[1.0.x] Node Inspector

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

Please read Review process for security advisory coverage: What to expect → for more details and Security advisory coverage application checklist → to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

To reviewers: Please read How to review security advisory coverage applications → , What to cover in an application review → , and Drupal.org security advisory coverage application workflow → .

While this application is open, only the user who opened the application can make commits to the project used for the application.

Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.

• The following points are just a start and don't necessarily encompass all of the changes that may be necessary
• A specific point may just be an example and may apply in other places
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; the single points aren't ordered, not even by importance

node_inspector.routing.yml

node_inspector.inspect_node:
  path: 'node/{node}/inspect'
  defaults:
    _title: 'Inspect node'
    _controller: '\Drupal\node_inspector\Controller\NodeInspector::inspectNode'
  options:
    _node_operation_route: TRUE
    parameters:
      node:
        type: entity:node
  requirements:
    _permission: 'access node inspector'

The node_inspector.inspect_node route is accessible to users who has the access node inspector permission. Users with that permission would be able to see data for unpublished nodes, even if they do not have the permission to see unpublished nodes. Users with that permission would be able to see data for nodes to which they do not have access; this makes the access node inspector permission very close to the Bypass content access control, since it allows them to bypass the content access control and see data for node to which they do not have access.
Furthermore, the node_inspector.inspect_node route also show information for all the revisions to users without the View all revisions permission or the permission to see revisions for a specific content type.

node_inspector.module

<?php

function node_inspector_theme($existing, $type, $theme, $path) {

The documentation comment is missing.

src/Controller/NodeInspector.php

/**
 * An example controller.
 */
class NodeInspector extends ControllerBase {

The class description is not the correct one.

  /**
   * Returns a render-able array for a test page.
   */

The short description does not seem correct.
The documentation comment is missing the parameters and the return value descriptions.

Fix phpcs issues.

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml node_inspector/

FILE: node_inspector/node_inspector.links.task.yml
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 5 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: node_inspector/node_inspector.module
--------------------------------------------------------------------------------
FOUND 6 ERRORS AND 4 WARNINGS AFFECTING 10 LINES
--------------------------------------------------------------------------------
  1 | ERROR   | [x] Missing file doc comment
  3 | ERROR   | [x] Missing function doc comment
 10 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected "NULL" but found "null"
 11 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
 17 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected "NULL" but found "null"
 18 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
 24 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected "NULL" but found "null"
 25 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
 31 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected "NULL" but found "null"
 32 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: node_inspector/node_inspector.permissions.yml
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 3 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: node_inspector/node_inspector.routing.yml
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 12 | ERROR | [x] Expected 1 newline at end of file; 0 found
-------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------


FILE: node_inspector/src/Controller/NodeInspector.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 29 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

I have fixed the coding standards issues and taken onboard the security recommendations.

By its nature this module is useful only for developers so rather than alter/add to the permissions needed to allow it to work I have made the following changes..

• Made the modules description more explicit as to its function.
• Moved it into the 'development' package.
• Added restrict access to the permission it creates.
• Added a warning the permission that informs the user of what it does and the implications of giving it to a user.

New release, now beta7.

I have reviewed the changes, and they look fine to me.

Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.

Thanks

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

These are some recommended readings to help with excellent maintainership:

• Dries → ' post on Responsible maintainers
• Best practices for creating and maintaining projects →
• Maintaining a drupal.org project with Git →
• Commit messages - providing history and credit →
• Release naming conventions → .
• Helping maintainers in the issue queues →

You can find more contributors chatting on the Slack → #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 reviewers.

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