The REST API Access Token module provides a Drupal authentication provider that uses tokens (in headers) as the primary factor of authentication.

Project link

https://www.drupal.org/project/rest_api_access_token

Git instructions

git clone --branch 8.x-1.x https://git.drupalcode.org/project/rest_api_access_token.git

PAReview checklist

https://pareview.sh/pareview/https-git.drupal.org-project-rest_api_acces...

Comments

marcinkazmierski created an issue. See original summary.

marcinkazmierski’s picture

Status: Active » Needs review
marcinkazmierski’s picture

Title: Request for 'security advisory policy' » [D8] REST API Access Token
Issue summary: View changes
avpaderno’s picture

Assigned: marcinkazmierski » Unassigned
Category: Support request » Task
Issue summary: View changes

Thank you for applying! I added the Git instructions for non-maintainer users. Reviewers will check the project and post comments to list what should be changed.

If you haven't done it, yet, please check the PAReview report and fix what needs to be fixed. There could be some false positives; verify that what reported is correct, before making any change.

avpaderno’s picture

Status: Needs review » Needs work
  • What follows is a quick review of the project; it doesn't mean to be complete
  • For every point, I didn't make a complete list of where the code should be fixed, but an example of what is wrong in the code
  • Not all the points are application stoppers; some of them describe changes that would be preferable to make
  1. The repository is using wrong branch names. See Release naming conventions, which explains which are the correct branch and tag names
  2. The code should not use the Drupal class or t() when dependency injection is available, for example inside plugin classes
  3. The code doesn't follow the Drupal coding standards in many points
marcinkazmierski’s picture

Assigned: Unassigned » marcinkazmierski
Status: Needs work » Needs review

Hi kiamlaluno,
Thanks for review!
I fix all of errors from PAReview report.
Is there anything else I can fix? :)

marcinkazmierski’s picture

Assigned: marcinkazmierski » Unassigned
vuil’s picture

Status: Needs review » Needs work

Hi! Thank you for the submission.

Please review the 8.x-1.x branch and fix the following issues:

  • User::load calls should be avoided in classes, use dependency injection instead
  • \Drupal calls should be avoided in classes, use dependency injection instead
marcinkazmierski’s picture

Status: Needs work » Needs review

Done :)

vuil’s picture

Status: Needs review » Needs work

Thank you for the contribution!

Please, fill the uncompleted sections in README.md with the required content (text) at first:
## INTRODUCTION
todo...
## CONFIGURATION
todo...

The descriptive documentation, and code comments, are the main requirements for each code. Please update also the module page with the required content (text) with the specific (h3 & other) tags inside.

And, be patience through the whole review process.

marcinkazmierski’s picture

Status: Needs work » Needs review

Thank you for review!

hook_help, readme and module description - updated :)

vuil’s picture

Status: Needs review » Needs work

Thank you for the contribution!

Could enable some "Automated tests" here? (example)
I want to see what will resulted there at first.

Please, be patience through the whole review process.

marcinkazmierski’s picture

Status: Needs work » Needs review

Thank you for review!

Added: "Automated tests".

seroton’s picture

Thanks for your module!

Haven't installed yet but here's 2 things I noticed:

-You've added a hook_help but also there is an open issue about it on your module's issue queue since almost a month ago that looks ignored. Please review and close if fixed as a release shouldn't have open issues, also please try to be on top of that as a module maintainer.
-Not too sure about this but there is a .gitignore left in your repo that shouldn't be there.

marcinkazmierski’s picture

.gitignore - deleted
opened issues - they were closed

vuil’s picture

Status: Needs review » Reviewed & tested by the community

I have not additional security issues found. Thank you!

marcinkazmierski’s picture

Status: Reviewed & tested by the community » Needs review

@ilchovuchkov - Thank you for review!
What is the next step? :)

vuil’s picture

Status: Needs review » Reviewed & tested by the community

Please, be patience through the whole process... Thanks.

* Note: Please, don't change the current status.

avpaderno’s picture

Modules don't need to implement hook_uninstall() to delete the database tables they use; that is done by Drupal core using the hook_schema() implementation.

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.

avpaderno’s picture

Status: Reviewed & tested by the community » Fixed
marcinkazmierski’s picture

Thank you!
@kiamlaluno Is everything alright?
What I have to do to get a green bar with text: "Stable release covered by the Drupal Security Team" in download section on module page? (https://www.drupal.org/project/rest_api_access_token) :)

avpaderno’s picture

You need to edit the project to opt into security coverage.

marcinkazmierski’s picture

@kiamlaluno, it works, thank you for your help! :)

avpaderno’s picture

Assigned: Unassigned » avpaderno
vuil’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.