Preventing warning about expected login check

Preventing warning about expected login check

by David Mudrák -
Number of replies: 2
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Particularly helpful Moodlers Picture of Peer reviewers Picture of Plugin developers Picture of Plugins guardians Picture of Testers Picture of Translators

Some time ago, as a part of MDLSITE-3688, Dan Poltawski implemented a new sniff into the Moodle code checker tool. It helps to prevent some security problems by checking that scripts trigger appropriate authentication mechanisms. In other words, most scripts should call something like require_login() or require_course_login() or admin_externalpage_setup() right after including the main config.php file (which boots up the whole Moodle framework). Note that here, the authentication also includes authenticating as a guest user, if the given context allows access for them.

If the script does not require user/guest authentication, there is a chance for a security bug in the code. So a warning like this is triggered:

(#xyz) Expected login check (require_login, require_course_login, admin_externalpage_setup) following config inclusion. None found.

Where the xyz shows the line number in the script where config.php is included and no login check follows.

As you can imagine, there are valid cases when the page should not require any authentication at all (although they are pretty rare). A typical example may be the login or signup page. To declare that the lack of the authentication is intended, you can ask the prechecker to ignore the line where the warning would otherwise be reported:

// No login check is expected here bacause ... (explain here why anonymous
// internet users should have access to this script).
// @codingStandardsIgnoreLine
require(__DIR__.'/../../config.php');

// Do what you need to do.

Unfortunately, the PHP CodeSniffer does not yet provide a way to ignore just individual rules. So any other coding style violation on the given line would be ignored, too.

More details can be found at Ignoring Parts of a File section of the CodeSniffer wiki.

Let me remind that these statements should not be abused. In most cases, the checks are valid. Turning off the checker is not a way to deal with raised warnings and errors. In this particular case, automatic login of the guest user is probably what you want when you think of anonymous access to a page. And that is what require_login() does by default.

Average of ratings: Useful (3)
In reply to David Mudrák

Re: Preventing warning about expected login check

by Rex Lorenzo -

Thanks for the cool tip about using @codingStandardsIgnoreLine. 

We have been using it to ignore certain edges with the coding standards. Just an problem that we ran into.

In our Behat custom steps we have a long line necessary for creating our own @Then steps and we put in @codingStandardsIgnoreLine to ignore the coding standard for having too long of a line.

However, the PHPdoc checker complains saying that @codingStandardsIgnoreLine is an invalid phpdoc tag.

Can we have @codingStandardsIgnoreLine as one of the valid phpdoc tag?

Average of ratings: Useful (1)