Add new Authenticator service to decouple authentication from panel

[giuscris]

This pull request introduces a new centralized authentication system for the application, replacing the previous access limiting and authentication logic in the panel. The main changes include the addition of the Authenticator service, a new rate limiting mechanism, updates to configuration to support these features, and refactoring of controllers and exception handling to use the new system.

Authentication System Overhaul

• Added a new Authenticator class (formwork/src/Authentication/Authenticator.php) to manage user login, logout, session handling, and user retrieval, centralizing authentication logic.
• Introduced a new RateLimiter class (formwork/src/Authentication/RateLimiter.php, renamed from AccessLimiter) for access attempt tracking and rate limiting, including improved reset logic and exception throwing when limits are exceeded. [1] [2] [3] [4]

Configuration Updates

• Added an authentication section to formwork/config/system.yaml for registry path and rate limit settings, and removed legacy panel login attempt settings. [1] [2]

Controller Refactoring

• Refactored AuthenticationController to use the new Authenticator and RateLimiter, improving error handling and response codes for authentication failures and rate limits. [1] [2] [3] [4] [5]
• Removed obsolete last access registry logic from UsersController as user access tracking is now handled by the new system. [1] [2]

Exception Handling Improvements

• Moved and updated exception classes to the new authentication namespace, and added a new RateLimitExceededException for clearer error reporting. [1] [2] [3]

Service Registration

• Registered the new authentication services and loader in the application container, ensuring they are available throughout the app. [1] [2] [3] [4] [5] [6]

[Copilot]

Pull request overview

This pull request introduces a centralized authentication system for the application by adding a new Authenticator service and refactoring existing authentication logic out of the panel and user models. The changes improve separation of concerns and consolidate authentication-related functionality.

Key Changes:

• Introduced Authenticator class to centralize login, logout, and session management operations
• Renamed and refactored AccessLimiter to RateLimiter with improved exception handling for rate limiting
• Updated configuration structure to support the new authentication system with dedicated settings under system.authentication

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
formwork/src/Authentication/Authenticator.php	New centralized authentication service handling login, logout, and user session management
formwork/src/Authentication/RateLimiter.php	Renamed from AccessLimiter, moved to authentication namespace with improved rate limit exception throwing
formwork/src/Authentication/Exceptions/*.php	Moved exception classes to authentication namespace for better organization
formwork/src/Services/Loaders/AuthenticationServiceLoader.php	New service loader for registering authentication services in the container
formwork/src/Services/Loaders/PanelServiceLoader.php	Added deprecation warnings for old panel configuration options and conditional RateLimiter registration
formwork/src/Panel/Controllers/AuthenticationController.php	Refactored to use new Authenticator service with improved error handling and rate limiting
formwork/src/Panel/Controllers/AbstractController.php	Updated CSRF token retrieval to use auto-generation
formwork/src/Users/User.php	Delegated authentication methods to Authenticator service and added lastAccess to default data
formwork/src/Panel/Controllers/UsersController.php	Removed obsolete last access registry cleanup logic
formwork/src/Panel/Panel.php	Updated exception import to new authentication namespace
formwork/src/Cms/App.php	Registered new authentication services and session in the container
formwork/config/system.yaml	Added authentication configuration section and removed deprecated panel login settings

Comments suppressed due to low confidence (3)

formwork/src/Authentication/RateLimiter.php:49

• The RateLimitExceededException receives $this->resetTime (the configured reset duration), but this doesn't account for the time already elapsed since the last attempt. For accurate retry timing, consider calculating the remaining time until reset: $this->resetTime - (time() - $this->lastAttemptTime) instead of just passing $this->resetTime.
  formwork/src/Panel/Controllers/AuthenticationController.php:81
• CSRF token is not generated for GET requests to the login page (lines 78-81). When a user first visits the login page via GET, no CSRF token is generated, which means the form will not have a valid token. The AbstractController::defaults() now uses autoGenerate: true (line 88 in AbstractController.php), but this relies on implicit behavior. Consider explicitly generating the token for clarity and to ensure the token is available for the login form.

        return new Response($this->view('@panel.authentication.login', [
            'title'  => $this->translate('panel.login.login'),
            'fields' => $fields,
        ]));

formwork/src/Authentication/RateLimiter.php:51

• The rate limit check logic is flawed. In assertAllowed(), registerAttempt() is called first (line 47), which increments the counter, and then hasReachedLimit() checks if attempts exceed the limit (line 48). This means if the limit is 5, the exception is thrown on the 6th attempt instead of after 5 failed attempts. Consider checking the limit before registering the attempt, or adjusting the comparison in hasReachedLimit() to use >= instead of >.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (1)

formwork/src/Authentication/RateLimiter.php:73

• RateLimiter derives its attemptHash using the unvalidated Host header (via Request::host() in the constructor), so an attacker can bypass the login attempt limit by changing the Host header and causing each request to be recorded under a different key. This effectively disables login rate limiting for a single IP, enabling unrestricted online brute-force attempts. Derive the rate-limit key only from stable, trusted attributes (e.g., IP address and/or account identifier) or enforce strict host validation before including it in the hash.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 13 comments.

Comments suppressed due to low confidence (2)

formwork/src/Authentication/RateLimiter.php:51

• The logic in assertAllowed() is incorrect. It registers an attempt first and then checks if the limit is reached. This means when a user reaches exactly the limit (e.g., 10 attempts), the 10th attempt is registered but doesn't throw an exception. The exception only gets thrown on the 11th attempt. The check should occur before registering the attempt, or the comparison in hasReachedLimit() should use >= instead of >.
  formwork/src/Authentication/RateLimiter.php:72
• The guard condition to prevent registering attempts after the limit is reached returns silently. This means subsequent calls to registerAttempt() won't update the lastAttemptTime, which could affect the reset logic. When the reset time elapses, the first check at line 66 won't reset the attempts because lastAttemptTime hasn't been updated. Consider removing this guard or updating lastAttemptTime even when the limit is reached.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.