-
-
Notifications
You must be signed in to change notification settings - Fork 13
refactor middleware authentication failure handler #101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
refactor middleware authentication failure handler #101
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #101 +/- ##
============================================
- Coverage 95.52% 95.47% -0.05%
Complexity 77 77
============================================
Files 10 10
Lines 201 199 -2
============================================
- Hits 192 190 -2
Misses 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * Default authentication failure handler. Responds with "401 Unauthorized" HTTP status code. | ||
| */ | ||
| final class AuthenticationFailureHandler implements RequestHandlerInterface | ||
| final class AuthenticationFailureHandler implements AuthenticationFailureHandlerInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the benefit in having a dedicated interface? Ability to configure it with a single line only? @vjik is there a better way?
|
@olegbaturin thanks for the pull request. Discussion with the code in front of you is way more productive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the middleware authentication failure handler by introducing a dedicated interface and updating the constructor signature. It also bumps the minimum PHP version requirement from 8.0 to 8.1.
- Introduces
AuthenticationFailureHandlerInterfaceto formalize the failure handler contract - Updates
Authenticationmiddleware constructor to require the interface instead of optional parameter - Moves
AuthenticationFailureHandlerfromHandlernamespace to rootAuthnamespace
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/AuthenticationFailureHandlerInterface.php | New interface defining the authentication failure handler contract |
| src/AuthenticationFailureHandler.php | Moved to root namespace and implements new interface |
| src/Middleware/Authentication.php | Updated constructor to require failure handler interface |
| tests/AuthenticationMiddlewareTest.php | Updated test methods to use new interface |
| tests/AuthenticationFailureHandlerTest.php | Updated import for moved class |
| tests/ConfigTest.php | New test for DI configuration |
| config/di-web.php | New DI configuration file |
| composer.json | Updated PHP version constraint and added dependencies |
| CHANGELOG.md | Updated with changes |
| .github/workflows/*.yml | Updated PHP versions in CI workflows |
Comments suppressed due to low confidence (1)
tests/AuthenticationMiddlewareTest.php:171
- The method name 'handle' doesn't match the interface being implemented. Since this implements AuthenticationFailureHandlerInterface, verify that the interface method is also named 'handle'.
public function handle(ServerRequestInterface $request): ResponseInterface
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Refactor middleware authentication failure handler
Bump minimum PHP version to 8.1