[1.0.x] Secure Magic Login Link

1. FILE: magic_login_link.info.yml

package: Custom

This line is used by custom modules created for specific sites. It is not a package name used for projects hosted on drupal.org.

Thank you for applying!

Please review the following documentation before continuing:

• Review process for security advisory coverage: What to expect
• Security advisory coverage application checklist
• Tips for ensuring a smooth review

Important notes for the applicant:

• Enable GitLab CI and fix any PHP_CodeSniffer errors or warnings.
• While this application is open, only commits from the applicant should be made.
• This application grants the applicant permission to opt eligible projects into security advisory coverage.
• Only the person who opened this application receives that permission.
• If a different project should be reviewed, update the issue summary and title with the correct project and branch.

To reviewers

Please review:

• How to review security advisory coverage applications
• Application workflow
• What to cover in an application review
• Tools to use for reviews

For new reviewers, also read how this issue queue differs from other project queues.

The important notes are the following.

• If you have not done it yet, you should enable GitLab CI for the project and fix the PHP_CodeSniffer errors/warnings it reports.
• For the time this application is open, only your commits are allowed.
• The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status will not be changed by this application; once this application is closed, you will be able to change the project status from Not covered to Opt into security advisory coverage. This is possible only 14 days after the project is created.
  
  
  Keep in mind that once the project is opted into security advisory coverage, only Security Team members may change coverage.
• Only the person who created the application will get the permission to opt projects into security advisory coverage. No other person will get the same permission from the same application; that applies also to co-maintainers/maintainers of the project used for the application.
• We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.

I reviewed the 1.0.x branch.

Main blocker

The module declares Drupal core compatibility with ^10.2 || ^11 || ^12, but the implementation relies on OOP hook attributes:

• src/Hook/FormHooks.php
• src/Hook/MailHooks.php

The OOP hook system using Drupal\Core\Hook\Attribute\Hook was introduced in Drupal 11.1.0. For Drupal 10.x / Drupal 11.0 compatibility, the Drupal core change record recommends adding procedural hook shims with #[LegacyHook], or otherwise limiting the supported core version. See Support for object oriented hook implementations using autowired services.

As written, on older advertised core versions the login form alter and mail hook may not run, so the module’s main functionality can fail.

Please either:

• change core_version_requirement and composer constraints to the actually supported Drupal versions, or
• add backward-compatible procedural hook implementations for the advertised versions.

Coding standards

I ran:

vendor/bin/phpcs --standard=Drupal,DrupalPractice web/modules/contrib/magic_login_link

PHPCS reports the following errors/warnings:

FILE: tests/src/Kernel/MagicLoginTest.php
--------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------
  36 | ERROR | Missing member variable doc comment
  38 | ERROR | Missing member variable doc comment
 109 | ERROR | Object operator not indented correctly; expected 6 spaces but found 8
 148 | ERROR | Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
--------------------------------------------------------------------------------

FILE: src/Controller/MagicLoginController.php
--------------------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES
--------------------------------------------------------------------------------
 15 | ERROR   | Missing class doc comment
 23 | ERROR   | Missing function doc comment
 83 | ERROR   | Missing function doc comment
 84 | WARNING | Only string literals should be passed to t() where possible
--------------------------------------------------------------------------------

FILE: src/Service/MagicLoginThrottler.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 64 | ERROR | Missing function doc comment
--------------------------------------------------------------------------------

FILE: src/Hook/FormHooks.php
--------------------------------------------------------------------------------
FOUND 4 ERRORS AND 5 WARNINGS AFFECTING 9 LINES
--------------------------------------------------------------------------------
 17 | WARNING | Unused use statement
 22 | ERROR   | Missing class doc comment
 38 | ERROR   | Missing function doc comment
 47 | WARNING | t() calls should be avoided in classes, use StringTranslationTrait and $this->t() instead
 52 | WARNING | t() calls should be avoided in classes, use StringTranslationTrait and $this->t() instead
 53 | WARNING | t() calls should be avoided in classes, use StringTranslationTrait and $this->t() instead
 54 | WARNING | t() calls should be avoided in classes, use StringTranslationTrait and $this->t() instead
 70 | ERROR   | Missing function doc comment
 82 | ERROR   | Missing function doc comment
--------------------------------------------------------------------------------

FILE: src/Hook/MailHooks.php
--------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------------
 11 | ERROR | Missing class doc comment
 16 | ERROR | Multi-line function declarations must have a trailing comma after the last parameter
 20 | ERROR | Missing function doc comment
--------------------------------------------------------------------------------

Additional coding-standard/documentation cleanup:

• magic_login_link.info.yml: package: Custom should be changed to an appropriate contrib package.
• README.md says the expiration period is configurable, but the code hardcodes 900 seconds.
• README.md says expiration can be altered by decorating MagicLoginThrottler, but expiration is implemented in the controller, not that service.
• The test command points to web/modules/custom/magic_login_link; for this project it should use the contrib path.
• The table of contents links to “Maintainers”, but there is no Maintainers section.

I also attempted to run the kernel tests locally, but the local environment does not define SIMPLETEST_DB, so PHPUnit could not run here. Please enable GitLab CI and ensure the test suite passes there.