Request to dto transition. #20
Conversation
WalkthroughThe PR refactors core authentication services by moving and renaming classes (AuthManager→Authentication, CsrfTokenManager→CsrfToken, PasswordResetManager→PasswordResetter, EmailVerificationManager→EmailVerifier) into a new Neuron\Cms\Services\Auth namespace. It introduces DTO-based request handling with new UsesDtos trait and DtoFactoryService, simplifies DTO validation schemas, and updates all dependents including controllers, initializers, and filters to use the restructured services and registry keys. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Cms/Controllers/Auth/PasswordReset.php (1)
183-190: Invalid CSRF redirect inresetPasswordshould target the reset/forgot flow, notadmin_posts
resetPassword()now uses the newCsrfTokenservice to validate the CSRF token, but on failure it redirects to theadmin_postsroute:if( !$this->_csrfToken->validate( $csrfToken ) ) { $this->redirect( 'admin_posts', [], ['error', 'Invalid CSRF token.'] ); }For a public password-reset route, this is almost certainly the wrong target and will send users to an admin page instead of back to the reset flow.
A more consistent behavior would be something like:
- if( !$this->_csrfToken->validate( $csrfToken ) ) - { - $this->redirect( 'admin_posts', [], ['error', 'Invalid CSRF token.'] ); - } + if( !$this->_csrfToken->validate( $csrfToken ) ) + { + $this->redirect( 'forgot_password', [], ['error', 'Invalid CSRF token. Please try again.'] ); + }(or redirect back to the reset form URL with the token if you prefer).
The call to
$this->_passwordResetter->resetPassword( $token, $password )otherwise fits the new service contract.Also applies to: 209-213
src/Cms/Services/Member/RegistrationService.php (1)
149-222: DTO-based registration currently bypasses password policy enforced byPasswordHasher
registerWithDto()validates uniqueness viavalidateUserBusinessRules()and checks password confirmation, but it never calls_passwordHasher->meetsRequirements()(orvalidateRegistration()). That means any runtime-configured password policy inPasswordHasher::configure()(min length, uppercase/lowercase/number/special requirements) is enforced inregister()but not in the DTO path.That’s a security-relevant inconsistency: users registering through the DTO flow can end up with weaker passwords than allowed elsewhere.
You can align both paths by adding the same password-strength check used in
validateRegistration():@@ public function registerWithDto( Dto $dto ): User - // Validate business rules (uniqueness checks) - $this->validateUserBusinessRules( $username, $email ); - - // Validate password confirmation - if( $password !== $passwordConfirmation ) - { - throw new Exception( 'Passwords do not match.' ); - } + // Validate business rules (uniqueness checks) + $this->validateUserBusinessRules( $username, $email ); + + // Validate password confirmation + if( $password !== $passwordConfirmation ) + { + throw new Exception( 'Passwords do not match.' ); + } + + // Enforce configured password strength requirements + if( !$this->_passwordHasher->meetsRequirements( $password ) ) + { + $errors = $this->_passwordHasher->getValidationErrors( $password ); + throw new Exception( implode( ', ', $errors ) ); + }This keeps the DTO responsible for structural validation while still honoring the centralized password policy.
🧹 Nitpick comments (19)
resources/views/member/profile/edit.php (3)
44-47: Consider adding validation feedback for email field.While the
requiredattribute provides basic client-side validation, consider adding Bootstrap's validation feedback classes to provide clearer user feedback when validation fails.Example:
<div class="mb-3"> <label for="email" class="form-label">Email</label> - <input type="email" class="form-control" id="email" name="email" value="<?= htmlspecialchars( $User->getEmail() ) ?>" required> + <input type="email" class="form-control" id="email" name="email" value="<?= htmlspecialchars( $User->getEmail() ) ?>" required> + <div class="invalid-feedback">Please provide a valid email address.</div> </div>
115-131: Add autocomplete attributes to password fields for better UX and security.Password fields should include appropriate
autocompleteattributes to help password managers correctly identify and fill these fields per the HTML specification.Apply this diff:
<div class="mb-3"> <label for="current_password" class="form-label">Current Password</label> - <input type="password" class="form-control" id="current_password" name="current_password"> + <input type="password" class="form-control" id="current_password" name="current_password" autocomplete="current-password"> </div> <div class="mb-3"> <label for="new_password" class="form-label">New Password</label> - <input type="password" class="form-control" id="new_password" name="new_password"> + <input type="password" class="form-control" id="new_password" name="new_password" autocomplete="new-password"> <small class="form-text text-muted"> Password requirements: min 8 chars, uppercase, lowercase, number, special character </small> </div> <div class="mb-3"> <label for="confirm_password" class="form-label">Confirm New Password</label> - <input type="password" class="form-control" id="confirm_password" name="confirm_password"> + <input type="password" class="form-control" id="confirm_password" name="confirm_password" autocomplete="new-password"> </div>
34-34: Consider using separate routes for profile update and password change.Both forms currently submit to
member_profile_update, which requires the server to distinguish intent based on which fields are populated. Using separate routes (e.g.,member_profile_updateandmember_password_update) would provide clearer separation of concerns and make the code more maintainable.Also applies to: 109-109
tests/Cms/Auth/MemberAuthenticationFilterTest.php (1)
129-129: Unused variable can be removed.The
$filtervariable is created but never used in this test. Since the test is marked incomplete and doesn't invoke$filter->pre(), you can either remove the variable or use$this->_filterinstead if you plan to complete the test later.Apply this diff:
- $filter = new MemberAuthenticationFilter( $this->_authentication, '/login', true ); + // Filter configuration verified via constructor, actual pre() call skipped due to exit() behaviorsrc/Cms/Dtos/CreateUserDto.yaml (1)
1-19: Verify thatroleretain their previous non-empty validation behaviorThe DTO refactor looks consistent overall, and
lengthconstraints forusernameandpasswordmirror the old string-length validators.However:
type: emailand dropped explicitIsNotEmpty/IsEmail.roleis now onlytype: string+required: true(previousIsNotEmptyremoved).If your DTO engine treats
required: trueas “key present” rather than “non-empty string”, empty values might now slip through forrole. If that’s not desired, consider adding simplelengthconstraints (e.g.,min: 1) or equivalent.src/Cms/Dtos/RegisterUserDto.yaml (1)
1-22: Confirm intended relaxation of username/email validationThe DTO structure and length constraints for
username,password, andpassword_confirmationlook coherent.Two behavioral changes to double‑check:
- Previous config enforced
IsAlphanumericforusername; the new schema only constrains length. If you still want to forbid characters like spaces or symbols, you may need an explicit validator or pattern.type: emailwithout explicitIsNotEmpty/IsEmail. As with other DTOs, this relies on your DTO engine’s handling oftype: emailandrequired: trueto reject empty or malformed addresses.If these relaxations are not intentional, consider adding back a stricter rule set (e.g., pattern/regex for username,
length/ explicit email validator).src/Cms/Controllers/Admin/Users.php (1)
208-209: Consider normalizingrole/passwordinputs before passing to updaterUsing
$request->post()forroleandpasswordis an improvement. To make the updater’s behavior clearer and avoid edge cases, you might want to normalize the inputs, for example:
- Trim
$roleto avoid accidental whitespace.- Convert an empty password string to
nullbefore callingupdate, ifnullis your “no password change” signal.For example:
- $role = $request->post( 'role', $user->getRole() ); - $password = $request->post( 'password', null ); + $roleInput = $request->post( 'role', $user->getRole() ); + $role = is_string( $roleInput ) ? trim( $roleInput ) : $user->getRole(); + + $passwordInput = $request->post( 'password', null ); + $password = (is_string( $passwordInput ) && $passwordInput !== '') + ? $passwordInput + : null;This keeps the updater’s contract explicit without changing behavior for typical requests.
src/Cms/Controllers/Auth/PasswordReset.php (1)
6-9: Migration toPasswordResetterandCsrfTokenis structurally soundFetching
PasswordResetterfrom the Registry with a hard failure when missing, and initializing a sharedCsrfTokenusinggetSessionManager(), cleanly aligns this controller with the new auth services.You could make
_passwordResetternon‑nullable since you always throw when it’s absent, but that’s purely a type-hint cleanup:- private ?PasswordResetter $_passwordResetter; + private PasswordResetter $_passwordResetter;Also applies to: 27-29, 38-48
tests/Cms/Services/Dto/DtoFactoryServiceTest.php (2)
14-19: Consider using a more robust path resolution.The relative path
__DIR__ . '/../../../../src/Cms/Dtos'is fragile and may break if the test file is moved. Consider using a constant or configuration value for the DTO directory path.Apply this diff:
protected function setUp(): void { - // Point to the actual DTO directory - $this->_testDtoPath = __DIR__ . '/../../../../src/Cms/Dtos'; + // Point to the actual DTO directory using realpath for robustness + $this->_testDtoPath = realpath( __DIR__ . '/../../../../src/Cms/Dtos' ); $this->_factory = new DtoFactoryService( $this->_testDtoPath ); }
116-134: Consider more specific exception handling in validation tests.The test catches a generic
\Exception, which is broad. Based on the UsesDtos trait (lines 78-82, 100-103), the DTO validation throwsNeuron\Core\Exceptions\Validationexceptions. Consider catching the specific exception type for more precise test assertions.Apply this diff:
+use Neuron\Core\Exceptions\Validation; + public function testValidationFailsForInvalidData(): void { $dto = $this->_factory->createRegisterUser(); // Set invalid username (too short) try { $dto->username = 'ab'; // Less than 3 characters $dto->validate(); $errors = $dto->getErrors(); $this->assertNotEmpty( $errors ); } - catch( \Exception $e ) + catch( Validation $e ) { // Validation exception expected $this->assertTrue( true ); } }src/Cms/Controllers/Traits/UsesDtos.php (2)
51-57: Inconsistent request data access.Line 56 directly accesses
$_POSTto get field keys, while line 61 uses$request->post($field)to get values. For consistency and to respect the Request abstraction, consider using the Request object for both operations.However, this would require the Request object to expose a method to get all POST keys. If such a method doesn't exist, this inconsistency is acceptable, but consider adding a comment explaining why direct
$_POSTaccess is necessary here.protected function populateDtoFromRequest( Dto $dto, Request $request, array $fields = [] ): Dto { - // If no specific fields provided, use all POST data keys + // If no specific fields provided, use all POST data keys + // Note: Direct $_POST access needed since Request doesn't expose getAllKeys() if( empty( $fields ) ) { $fields = array_keys( $_POST ); }
115-124: Generic exception loses type information.The
validateDtoOrFail()method throws a generic\Exceptioninstead of preserving theValidationexception type (line 122). This loses valuable type information that callers might want to handle differently from other exceptions.Apply this diff to preserve the exception type:
protected function validateDtoOrFail( Dto $dto ): void { $errors = $this->validateDto( $dto ); if( !empty( $errors ) ) { $message = implode( ', ', $errors ); - throw new \Exception( $message ); + throw new Validation( $message ); } }tests/Cms/Services/AuthenticationTest.php (3)
41-48: Reflection-based access to_pdomakes the test brittleUsing
ReflectionClassto grab the private_pdoproperty couples this test tightly toDatabaseUserRepositoryinternals. A rename or visibility change will break the test even though behavior is unchanged. Consider exposing a minimal getter for the PDO (behind a test-only interface) or constructing the repository around an injectable PDO in tests so you can avoid reflection entirely.
96-105: Clean up unused$userlocals flagged by PHPMDIn several tests (
testAttemptWithCorrectCredentials,testAttemptWithIncorrectPassword,testCheckReturnsTrue,testUserReturnsAuthenticatedUser) the$uservariable is assigned but never read, which PHPMD reports asUnusedLocalVariable. You can either remove the assignment (just call$this->createTestUser(...)for setup) or use the variable in assertions (e.g., to assert IDs or roles) to clarify intent and silence the warnings.Also applies to: 109-110, 119-120, 234-235, 247-248
305-318:testPasswordRehashingOnLogindoes not actually assert rehash behaviorThis test currently only checks that
getPasswordHash()is non-empty after login. Because the user is created with the samePasswordHasherused byAuthentication,needsRehash()will typically return false, so the test never proves the rehash path is exercised.To make this meaningful, consider forcing the “needs rehash” branch, e.g.:
- Inject a stubbed
PasswordHasherintoAuthenticationthat always returnstruefromneedsRehash()and records whenhash()is called; or- Manually set a legacy hash on the user before persisting, in a format that
needsRehash()will definitely flag.That will let you assert that the password hash actually changes on login instead of just being non-empty.
src/Cms/Services/Dto/DtoFactoryService.php (1)
26-34: Solid DTO factory; consider minor safety and ergonomics tweaksThe factory logic (default directory, caching, and cloning) looks correct and matches the YAML layout. A few optional improvements you might consider:
- If
$namecould ever come from outside trusted code, guard against path traversal (e.g., restrict to[A-Za-z0-9_]and strip separators) before concatenating into$filePath.- Instead of throwing a generic
Exceptionfor missing files, you could introduce a more specific exception type (e.g.,DtoNotFoundException) to make caller-side error handling clearer.- If DTO names grow over time, you may eventually want to replace the specific helper methods (
createCreateUser,createCreatePost, etc.) with a more generic convention or constants to avoid having to touch this class for every new DTO.These are non-blocking; the current implementation is functionally sound.
Also applies to: 43-69, 77-146
resources/app/Initializers/RegistrationInitializer.php (1)
49-56: Initializer wiring is correct; add small robustness checksThe updated initializer correctly:
- Retrieves
Authenticationfrom the Registry and passes it intoMemberAuthenticationFilter.- Constructs
EmailVerifierwith the right dependencies and stores it under theEmailVerifierkey.- Applies the token expiration from settings and wires
RegistrationServiceaccordingly.Two minor robustness improvements to consider:
- Add an
instanceof Authenticationcheck for$authenticationbefore using it, to fail fast if the Registry is misconfigured.- For consistency with
PasswordResetInitializer, you could cast$tokenExpirationto(int)when callingsetTokenExpirationMinutes, even though coercive typing will already handle numeric strings.None of this is blocking—the current logic is functionally sound.
Also applies to: 78-90, 103-116
src/Cms/Services/Member/RegistrationService.php (2)
24-52: EmailVerifier integration and event emission look good, but catch block comment is misleadingInjecting
EmailVerifierand emittingUserCreatedEventafter successful registration aligns the service with the new verification and event model. Swallowing email-sending failures so registration can succeed also makes sense.However, the catch block comment says “Log error but don't fail registration”, but no logging actually occurs. Either add a log call (e.g., via a logger dependency) or adjust the comment so it doesn’t imply logging is happening when it isn’t.
Also applies to: 119-140
233-296: Unify business-rule checks betweenvalidateRegistration()andvalidateUserBusinessRules()
validateRegistration()andvalidateUserBusinessRules()both perform the same username/email uniqueness checks, which risks the two paths diverging over time.You can reduce duplication and keep behavior in sync by having
validateRegistration()delegate tovalidateUserBusinessRules()for the uniqueness part, e.g.:@@ private function validateRegistration( - // Check if username is already taken - if( $this->_userRepository->findByUsername( $username ) ) - { - throw new Exception( 'Username is already taken.' ); - } - … - // Check if email is already taken - if( $this->_userRepository->findByEmail( $email ) ) - { - throw new Exception( 'Email is already registered.' ); - } + // Check business rules (uniqueness) + $this->validateUserBusinessRules( $username, $email );This way any future changes to uniqueness rules are made in one place and automatically apply to both the classic and DTO registration flows.
Also applies to: 298-318
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
composer.json(1 hunks)resources/app/Initializers/AuthInitializer.php(2 hunks)resources/app/Initializers/PasswordResetInitializer.php(4 hunks)resources/app/Initializers/RegistrationInitializer.php(6 hunks)resources/app/Initializers/ViewDataInitializer.php(1 hunks)resources/views/layouts/member.php(1 hunks)resources/views/member/dashboard/index.php(0 hunks)resources/views/member/profile/edit.php(1 hunks)src/Cms/Auth/Filters/AuthenticationFilter.php(3 hunks)src/Cms/Auth/Filters/CsrfFilter.php(3 hunks)src/Cms/Auth/Filters/MemberAuthenticationFilter.php(3 hunks)src/Cms/Controllers/Admin/Dashboard.php(2 hunks)src/Cms/Controllers/Admin/Posts.php(5 hunks)src/Cms/Controllers/Admin/Profile.php(2 hunks)src/Cms/Controllers/Admin/Users.php(6 hunks)src/Cms/Controllers/Auth/Login.php(7 hunks)src/Cms/Controllers/Auth/PasswordReset.php(10 hunks)src/Cms/Controllers/Member/Dashboard.php(3 hunks)src/Cms/Controllers/Member/Profile.php(3 hunks)src/Cms/Controllers/Member/Registration.php(8 hunks)src/Cms/Controllers/Traits/UsesDtos.php(1 hunks)src/Cms/Dtos/CreateCategoryDto.yaml(1 hunks)src/Cms/Dtos/CreatePostDto.yaml(2 hunks)src/Cms/Dtos/CreateUserDto.yaml(1 hunks)src/Cms/Dtos/RegisterUserDto.yaml(1 hunks)src/Cms/Dtos/UpdateCategoryDto.yaml(1 hunks)src/Cms/Dtos/UpdatePostDto.yaml(2 hunks)src/Cms/Dtos/UpdateUserDto.yaml(1 hunks)src/Cms/Services/Auth/Authentication.php(1 hunks)src/Cms/Services/Auth/CsrfToken.php(1 hunks)src/Cms/Services/Auth/EmailVerifier.php(2 hunks)src/Cms/Services/Auth/PasswordResetter.php(2 hunks)src/Cms/Services/Dto/DtoFactoryService.php(1 hunks)src/Cms/Services/Member/RegistrationService.php(5 hunks)tests/Cms/Auth/MemberAuthenticationFilterTest.php(7 hunks)tests/Cms/Services/AuthenticationTest.php(10 hunks)tests/Cms/Services/CsrfTokenTest.php(7 hunks)tests/Cms/Services/Dto/DtoFactoryServiceTest.php(1 hunks)tests/Cms/Services/EmailVerifierTest.php(4 hunks)tests/Cms/Services/PasswordResetterTest.php(4 hunks)tests/Cms/Services/RegistrationServiceTest.php(9 hunks)tests/bootstrap.php(1 hunks)versionlog.md(1 hunks)
💤 Files with no reviewable changes (1)
- resources/views/member/dashboard/index.php
🧰 Additional context used
🧬 Code graph analysis (29)
src/Cms/Controllers/Member/Profile.php (3)
src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)getToken(38-46)src/Cms/Controllers/Content.php (1)
getSessionManager(222-230)src/Cms/Auth/SessionManager.php (1)
set(99-103)
src/Cms/Auth/Filters/AuthenticationFilter.php (4)
src/Cms/Services/Auth/Authentication.php (3)
Authentication(19-323)__construct(27-36)user(167-190)src/Cms/Controllers/Auth/Login.php (1)
__construct(30-44)src/Cms/Auth/Filters/MemberAuthenticationFilter.php (1)
__construct(27-41)src/Cms/Auth/helpers.php (1)
user(28-31)
resources/views/member/profile/edit.php (3)
src/Cms/View/helpers.php (2)
gravatar_url(23-27)route_path(70-79)src/Cms/Auth/helpers.php (1)
csrf_field(122-126)src/Cms/Models/User.php (3)
getUsername(78-81)getTimezone(396-399)getRole(129-132)
src/Cms/Auth/Filters/CsrfFilter.php (4)
src/Cms/Services/Auth/CsrfToken.php (3)
CsrfToken(15-71)__construct(20-23)validate(51-62)src/Cms/Controllers/Auth/Login.php (1)
__construct(30-44)src/Cms/Controllers/Auth/PasswordReset.php (1)
__construct(34-48)src/Cms/Controllers/Member/Registration.php (1)
__construct(40-61)
src/Cms/Controllers/Admin/Profile.php (3)
src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)getToken(38-46)src/Cms/Controllers/Content.php (1)
getSessionManager(222-230)src/Cms/Auth/SessionManager.php (1)
set(99-103)
src/Cms/Services/Auth/PasswordResetter.php (1)
src/Cms/Auth/PasswordHasher.php (1)
PasswordHasher(13-200)
src/Cms/Controllers/Member/Registration.php (6)
src/Cms/Services/Auth/CsrfToken.php (3)
CsrfToken(15-71)getToken(38-46)validate(51-62)src/Cms/Services/Auth/EmailVerifier.php (3)
EmailVerifier(21-247)verifyEmail(123-164)resendVerification(173-190)src/Cms/Services/Member/RegistrationService.php (2)
RegistrationService(22-319)registerWithDto(149-222)src/Cms/Auth/SessionManager.php (2)
get(108-112)set(99-103)src/Cms/Controllers/Content.php (2)
getSessionManager(222-230)redirect(240-251)src/Cms/Controllers/Traits/UsesDtos.php (2)
createDtoFromRequest(137-141)validateDtoOrFail(115-124)
src/Cms/Controllers/Admin/Posts.php (3)
src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)getToken(38-46)src/Cms/Auth/SessionManager.php (1)
set(99-103)src/Cms/Controllers/Content.php (1)
getSessionManager(222-230)
src/Cms/Controllers/Auth/PasswordReset.php (4)
src/Cms/Services/Auth/PasswordResetter.php (4)
PasswordResetter(21-226)requestReset(76-107)validateToken(115-127)resetPassword(137-170)src/Cms/Auth/SessionManager.php (3)
SessionManager(13-188)get(108-112)set(99-103)src/Cms/Services/Auth/CsrfToken.php (3)
CsrfToken(15-71)getToken(38-46)validate(51-62)src/Cms/Controllers/Content.php (1)
getSessionManager(222-230)
src/Cms/Controllers/Traits/UsesDtos.php (1)
src/Cms/Services/Dto/DtoFactoryService.php (2)
DtoFactoryService(16-167)create(43-69)
src/Cms/Services/Member/RegistrationService.php (4)
src/Cms/Services/Auth/EmailVerifier.php (3)
EmailVerifier(21-247)__construct(39-52)sendVerificationEmail(72-94)src/Cms/Controllers/Member/Registration.php (1)
__construct(40-61)src/Cms/Events/UserCreatedEvent.php (1)
UserCreatedEvent(13-23)src/Cms/Models/User.php (4)
setUsername(86-90)setPasswordHash(120-124)setRole(137-141)setEmailVerified(211-215)
src/Cms/Controllers/Auth/Login.php (4)
src/Cms/Services/Auth/Authentication.php (4)
Authentication(19-323)check(147-162)attempt(41-97)logout(121-142)src/Cms/Services/Auth/CsrfToken.php (3)
CsrfToken(15-71)getToken(38-46)validate(51-62)src/Cms/Auth/SessionManager.php (2)
get(108-112)set(99-103)src/Cms/Controllers/Content.php (2)
getSessionManager(222-230)redirect(240-251)
tests/Cms/Services/AuthenticationTest.php (5)
src/Cms/Services/Auth/Authentication.php (8)
Authentication(19-323)user(167-190)attempt(41-97)check(147-162)logout(121-142)loginUsingRememberToken(243-259)isAdmin(291-294)hasRole(282-286)src/Cms/Repositories/DatabaseUserRepository.php (1)
DatabaseUserRepository(19-263)src/Cms/Auth/SessionManager.php (1)
SessionManager(13-188)src/Cms/Auth/PasswordHasher.php (2)
PasswordHasher(13-200)hash(24-30)src/Cms/Models/User.php (8)
setPasswordHash(120-124)setLockedUntil(323-327)getFailedLoginAttempts(279-282)isLockedOut(332-340)getLockedUntil(315-318)getRememberToken(220-223)getUsername(78-81)getPasswordHash(112-115)
resources/app/Initializers/AuthInitializer.php (3)
src/Cms/Services/Auth/Authentication.php (1)
Authentication(19-323)src/Cms/Auth/Filters/AuthenticationFilter.php (1)
AuthenticationFilter(18-68)src/Cms/Auth/SessionManager.php (1)
set(99-103)
resources/app/Initializers/ViewDataInitializer.php (1)
src/Cms/Services/Auth/Authentication.php (1)
check(147-162)
src/Cms/Controllers/Admin/Users.php (5)
src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)getToken(38-46)src/Cms/Controllers/Content.php (1)
getSessionManager(222-230)src/Cms/Auth/SessionManager.php (1)
set(99-103)src/Cms/Controllers/Admin/Posts.php (1)
edit(206-247)src/Cms/Controllers/Admin/Profile.php (1)
edit(50-81)
resources/app/Initializers/PasswordResetInitializer.php (1)
src/Cms/Services/Auth/PasswordResetter.php (2)
PasswordResetter(21-226)setTokenExpirationMinutes(61-65)
src/Cms/Auth/Filters/MemberAuthenticationFilter.php (3)
src/Cms/Services/Auth/Authentication.php (3)
Authentication(19-323)__construct(27-36)user(167-190)src/Cms/Controllers/Auth/Login.php (1)
__construct(30-44)src/Cms/Auth/Filters/AuthenticationFilter.php (1)
__construct(24-33)
tests/Cms/Services/CsrfTokenTest.php (2)
src/Cms/Services/Auth/CsrfToken.php (5)
CsrfToken(15-71)generate(28-33)getToken(38-46)validate(51-62)regenerate(67-70)src/Cms/Auth/SessionManager.php (1)
SessionManager(13-188)
tests/Cms/Services/Dto/DtoFactoryServiceTest.php (1)
src/Cms/Services/Dto/DtoFactoryService.php (11)
DtoFactoryService(16-167)createRegisterUser(77-80)createCreateUser(88-91)createUpdateUser(99-102)createCreateCategory(110-113)createUpdateCategory(121-124)createCreatePost(132-135)createUpdatePost(143-146)clearCache(153-156)getDtoDirectory(163-166)create(43-69)
src/Cms/Services/Auth/Authentication.php (2)
src/Cms/Auth/SessionManager.php (1)
SessionManager(13-188)src/Cms/Auth/PasswordHasher.php (1)
PasswordHasher(13-200)
tests/Cms/Services/EmailVerifierTest.php (1)
src/Cms/Services/Auth/EmailVerifier.php (2)
EmailVerifier(21-247)setTokenExpirationMinutes(57-61)
tests/Cms/Services/PasswordResetterTest.php (2)
src/Cms/Auth/PasswordHasher.php (1)
PasswordHasher(13-200)src/Cms/Services/Auth/PasswordResetter.php (2)
PasswordResetter(21-226)setTokenExpirationMinutes(61-65)
src/Cms/Controllers/Admin/Dashboard.php (2)
src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)getToken(38-46)src/Cms/Auth/SessionManager.php (1)
set(99-103)
tests/Cms/Services/RegistrationServiceTest.php (1)
src/Cms/Services/Auth/EmailVerifier.php (1)
EmailVerifier(21-247)
src/Cms/Controllers/Member/Dashboard.php (3)
src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)getToken(38-46)src/Cms/Controllers/Content.php (1)
getSessionManager(222-230)src/Cms/Auth/SessionManager.php (1)
set(99-103)
resources/app/Initializers/RegistrationInitializer.php (3)
src/Cms/Services/Auth/EmailVerifier.php (2)
EmailVerifier(21-247)setTokenExpirationMinutes(57-61)src/Cms/Auth/Filters/MemberAuthenticationFilter.php (1)
MemberAuthenticationFilter(19-105)src/Cms/Services/Auth/Authentication.php (1)
Authentication(19-323)
tests/Cms/Auth/MemberAuthenticationFilterTest.php (2)
src/Cms/Services/Auth/Authentication.php (2)
Authentication(19-323)user(167-190)src/Cms/Auth/Filters/MemberAuthenticationFilter.php (1)
MemberAuthenticationFilter(19-105)
src/Cms/Services/Auth/CsrfToken.php (1)
src/Cms/Auth/SessionManager.php (1)
SessionManager(13-188)
🪛 PHPMD (2.15.0)
src/Cms/Controllers/Member/Registration.php
129-129: Avoid unused local variables such as '$user'. (undefined)
(UnusedLocalVariable)
tests/Cms/Services/AuthenticationTest.php
109-109: Avoid unused local variables such as '$user'. (undefined)
(UnusedLocalVariable)
119-119: Avoid unused local variables such as '$user'. (undefined)
(UnusedLocalVariable)
234-234: Avoid unused local variables such as '$user'. (undefined)
(UnusedLocalVariable)
247-247: Avoid unused local variables such as '$user'. (undefined)
(UnusedLocalVariable)
tests/Cms/Auth/MemberAuthenticationFilterTest.php
129-129: Avoid unused local variables such as '$filter'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (50)
resources/views/member/profile/edit.php (2)
1-10: LGTM!The header and alert blocks are properly structured with correct output escaping.
12-25: Profile picture section looks good.The Gravatar integration is properly implemented with secure external links using
rel="noopener".composer.json (2)
15-15: LGTM: Version constraint properly tightened.The MVC version constraint update to
^0.9.5aligns with the version log's reference to enhancedurlFor()method with fallback parameter from MVC 0.9.5+.
19-19: neuron-php/dto dependency is available and properly versioned.The package is published on Packagist with recent releases, including v0.0.7. The version constraint
0.0.*is consistent with the other neuron-php dependencies in your project (cli0.8.*, jobs0.2.*, orm0.1.*) and will properly resolve to the latest available 0.0.x release. No concerns identified.versionlog.md (1)
1-21: LGTM: Comprehensive release documentation.The new version entries (0.8.10, 0.8.9, 0.8.8) thoroughly document the login/logout flow improvements, MVC compatibility updates, and DTO-based validation changes.
src/Cms/Services/Auth/CsrfToken.php (1)
3-15: LGTM: Clean refactor to Services namespace.The class has been successfully renamed from
CsrfTokenManagertoCsrfTokenand moved to theNeuron\Cms\Services\Authnamespace. The functionality remains unchanged, and theSessionManagerdependency is properly imported.src/Cms/Dtos/CreatePostDto.yaml (2)
28-29: Good addition: type specification for array items.Adding
items: type: integerfor thecategoryIdsarray provides better type safety and validation for the array elements.
1-15: Length constraints properly enforce validation with neuron-php/dto framework.Verification confirms that the declarative
lengthconstraints in the YAML file are equivalent to the previous explicit validators. The neuron-php/dto framework (version 0.0.*) usesIsStringLengthinternally forlength: {min, max}constraints and enforcesrequired: truefor non-empty validation. Test cases in the codebase validate both length constraints and required fields correctly.src/Cms/Controllers/Member/Profile.php (2)
9-9: LGTM: CSRF service usage updated correctly.The import and instantiation have been properly updated to use the new
CsrfTokenclass from theNeuron\Cms\Services\Authnamespace, and the token is correctly retrieved usinggetToken().Also applies to: 60-61
79-79: Template path change verified as correct and consistent.The
'member'layout exists and properly renders views via the$contentvariable. All member-related controllers (Dashboard, Registration, Profile) consistently use this exact pattern—passing'member'as the layout parameter and the specific view name as a separate parameter (e.g.,'edit'for Profile,'index'for Dashboard). This is the established and intentional pattern across the codebase, not an error.resources/app/Initializers/AuthInitializer.php (1)
5-5: LGTM: Authentication service properly initialized.The initializer has been correctly updated to:
- Import the new
Authenticationclass fromNeuron\Cms\Services\Auth- Instantiate
Authenticationwith the correct dependencies- Pass the
Authenticationinstance toAuthenticationFilter- Store it in the Registry under the updated
'Authentication'keyAll changes are consistent with the authentication service refactoring.
Also applies to: 58-58, 61-61, 66-67
resources/app/Initializers/ViewDataInitializer.php (1)
68-69: Registry key migration verified—no remaining AuthManager references in codebase.Verification confirms the refactoring is complete. A comprehensive search for "AuthManager" across all PHP files returned no results, indicating all references have been successfully updated to the new "Authentication" registry key. The code at lines 68-69 correctly uses
$registry->get( 'Authentication' )and properly calls thecheck()method on the Authentication instance.src/Cms/Dtos/UpdatePostDto.yaml (2)
5-7: LGTM! Validation migrated to length constraints.The migration from
IsNotEmptyandIsStringLengthvalidators to thelengthconstraint is consistent with the DTO modernization across the codebase.
28-29: Good addition of item type constraint.Explicitly declaring
type: integerfor array items improves validation and type safety for thecategoryIdsfield.src/Cms/Controllers/Member/Dashboard.php (1)
48-49: LGTM! CSRF token handling updated correctly.The migration from
CsrfTokenManagertoCsrfTokenis implemented correctly, using the new API withgetToken().tests/Cms/Auth/MemberAuthenticationFilterTest.php (1)
5-5: LGTM! Test updated for Authentication service.The test correctly updates all references from
AuthManagertoAuthentication, maintaining consistency with the service rename.Also applies to: 18-18, 27-27, 30-30
src/Cms/Services/Auth/Authentication.php (1)
3-3: LGTM! Service successfully migrated to new namespace.The class has been correctly renamed from
AuthManagertoAuthenticationand moved to theNeuron\Cms\Services\Authnamespace, consistent with the broader service reorganization.Also applies to: 19-19
src/Cms/Controllers/Admin/Profile.php (1)
60-61: LGTM! CSRF token handling migrated correctly.The update from
CsrfTokenManagertoCsrfTokenfollows the correct pattern, properly instantiating with the session manager and retrieving the token.src/Cms/Dtos/UpdateUserDto.yaml (2)
5-7: LGTM! Username validation modernized.The migration to
lengthconstraints is consistent with the DTO refactoring pattern across the codebase.
9-9: Good semantic improvement with email type.Changing from
type: stringtotype: emailprovides better semantic validation for email fields.src/Cms/Controllers/Admin/Posts.php (2)
88-89: LGTM! CSRF token handling consistently updated.All three CSRF token instantiation points correctly migrate from
CsrfTokenManagertoCsrfToken, following the new API pattern.Also applies to: 134-135, 230-231
170-177: LGTM! Form data extraction modernized.The migration from direct
$_POSTaccess to$request->post()improves consistency with the framework's request abstraction.src/Cms/Controllers/Admin/Dashboard.php (1)
51-52: LGTM! CSRF token handling updated correctly.The migration from
CsrfTokenManagertoCsrfTokenis implemented correctly, consistent with the broader refactoring.src/Cms/Services/Auth/EmailVerifier.php (1)
3-21: Namespace and service rename toServices\Auth\EmailVerifierlooks consistentThe move to
Neuron\Cms\Services\Auth\EmailVerifierand updated docblock/package are coherent with the new auth service structure; implementation remains sound and self-contained.src/Cms/Auth/Filters/AuthenticationFilter.php (1)
7-8: Filter now correctly depends on theAuthenticationserviceSwitching to
Neuron\Cms\Services\Auth\Authenticationand routing all user lookups through$this->_authentication->user()is consistent with the new auth service. Just ensure all instantiation/wiring points now injectAuthenticationinstead of the oldAuthManager.Also applies to: 20-27, 40-40
src/Cms/Controllers/Admin/Users.php (2)
12-12: CSRF token generation now correctly uses the sharedCsrfTokenserviceSwitching to
Neuron\Cms\Services\Auth\CsrfTokenand storingAuth.CsrfTokenfrom$csrfToken->getToken()inindex,create, andeditaligns with the new CSRF infrastructure and matches the pattern in other controllers.Also applies to: 60-63, 94-96, 161-163
146-150: Return type foreditstreamlined tostringUpdating the PHPDoc and method signature to return
stringis sensible here; it matches actual usage (rendering a view) and avoids the unsupportedstring|neverstyle return type.src/Cms/Controllers/Auth/PasswordReset.php (1)
135-136: Token validation and CSRF setup inshowResetFormlook goodUsing
PasswordResetter::validateToken()and, on success, settingAuth.CsrfTokenfrom$this->_csrfToken->getToken()matches the intended flow and mirrors the forgot-password form behavior. The error paths correctly short-circuit with a redirect when the token is missing or invalid.Also applies to: 147-159
tests/Cms/Services/RegistrationServiceTest.php (1)
5-291: LGTM! Class rename properly reflected throughout the test.The test file has been consistently updated to reflect the rename from
EmailVerificationManagertoEmailVerifier. All imports, property declarations, mock creation, and method calls have been properly updated while maintaining the same test logic and assertions.src/Cms/Services/Auth/PasswordResetter.php (1)
3-21: LGTM! Namespace and class rename applied correctly.The class has been properly renamed from
PasswordResetManagertoPasswordResetterand moved to theNeuron\Cms\Services\Authnamespace. The import statement forPasswordHasherand documentation updates are correct.tests/Cms/Services/CsrfTokenTest.php (1)
3-172: LGTM! Test updated consistently for class rename and relocation.The test file has been properly updated to reflect the rename from
CsrfTokenManagertoCsrfTokenand the namespace change. The property naming now follows the underscore prefix convention ($_csrfToken), and all method calls have been updated accordingly.tests/Cms/Services/EmailVerifierTest.php (1)
3-61: LGTM! Test properly reflects class rename and relocation.The test file has been consistently updated to reflect the rename from
EmailVerificationManagertoEmailVerifierand the namespace move toNeuron\Cms\Services\Auth. All property declarations, instance checks, and assertions are correctly updated.src/Cms/Auth/Filters/CsrfFilter.php (1)
7-56: LGTM! Dependency updated consistently throughout the filter.The filter has been properly updated to use
CsrfTokeninstead ofCsrfTokenManager. The import statement, property declaration, constructor signature, and usage are all correctly updated.tests/Cms/Services/Dto/DtoFactoryServiceTest.php (1)
21-97: Test coverage is solid, but could be enhanced.The tests verify that DTO properties exist, which is good basic coverage. However, they don't validate property types, constraints, or default values. While this is acceptable for the current scope, consider adding more detailed property validation tests in the future to ensure DTOs are properly configured.
src/Cms/Controllers/Traits/UsesDtos.php (2)
27-38: LGTM! Registry pattern with lazy initialization.The
getDtoFactory()method properly implements lazy initialization with Registry fallback. This ensures a single instance is created and reused across controllers while allowing for explicit configuration via Registry.
51-86: Review comment is valid but code is currently safe—use .Verification confirms the security concern is architecturally sound: the code relies on DTO schema as an implicit whitelist. However, the current RegisterUserDto schema contains only
username,password, andpassword_confirmation—no sensitive fields. Additionally, the DTO'sgetProperty()validation (lines 66-70) prevents setting undefined properties. Only Registration.php controller uses this pattern without explicit fields, and that specific DTO is safe.The recommendation to use explicit field lists remains a best practice for future-proofing, especially if sensitive fields are added to DTO schemas later. Current risk is low given the whitelist-via-schema design, but developers should remain aware of this implicit pattern.
src/Cms/Auth/Filters/MemberAuthenticationFilter.php (1)
7-48: LGTM! Dependency updated consistently throughout the filter.The filter has been properly updated to use
Authenticationinstead ofAuthManager. The import statement, property declaration, constructor signature, and usage in thecheckAuthentication()method are all correctly updated.tests/Cms/Services/PasswordResetterTest.php (1)
47-55: PasswordResetter tests align well with the new service behaviorThe test setup and assertions match the
PasswordResetterimplementation: token hashing, repository interactions, password hashing, and weak-password handling are all exercised correctly. UsingMemorysettings withemail.test_modeavoids accidental email sends while still driving the realSenderpath. No changes needed here.Also applies to: 57-66, 68-116, 176-223, 242-275
resources/app/Initializers/PasswordResetInitializer.php (1)
62-77: Password reset initializer wiring looks correctThe initializer now constructs
PasswordResetterwith all required dependencies (token repo, user repo, hasher, settings, base path, reset URL), applies the token expiration, and registers it underPasswordResetterin theRegistry. This is consistent with the service API and should be safe in environments where database settings are present.src/Cms/Controllers/Member/Registration.php (5)
6-9: LGTM! Clean integration of DTO support.The
UsesDtostrait and updated service imports properly support the DTO-based request handling introduced in this PR.Also applies to: 29-29
30-61: LGTM! Service initialization updated correctly.The constructor properly initializes the renamed services (
EmailVerifierandCsrfToken) and updates the Registry lookup keys accordingly.
88-88: LGTM! CSRF token handling migrated correctly.The CSRF token is now properly retrieved from the new
CsrfTokenservice.
208-208: LGTM! Email verification service updated correctly.The call to
verifyEmail()now uses the renamedEmailVerifierservice.
272-297: LGTM! Verification resend updated correctly with good security practices.The method correctly uses the renamed services and maintains the security practice of not revealing whether an email exists in the system.
src/Cms/Controllers/Auth/Login.php (6)
6-7: LGTM! Service imports updated correctly.The imports reflect the renamed authentication services in the new namespace.
23-44: LGTM! Authentication service initialization updated correctly.The constructor properly initializes the renamed
AuthenticationandCsrfTokenservices with the correct Registry keys and error messages.
53-87: LGTM! Enhanced redirect URL handling with better security.The login form now:
- Uses explicit default redirects with fallbacks
- Validates redirect URLs via
isValidRedirectUrl()before use- Prevents potential open redirect vulnerabilities
This is a security improvement over the previous implementation.
95-132: LGTM! Login flow updated with consistent redirect handling.The login method correctly uses the renamed
Authenticationservice and applies the same secure redirect validation pattern asshowLoginForm().
139-143: LGTM! Logout flow updated with appropriate redirect.The logout method now redirects to the home page instead of the admin dashboard, which is a sensible UX improvement since users need to log in again to access admin areas.
152-181: LGTM! Excellent security validation for redirect URLs.The
isValidRedirectUrl()method implements comprehensive checks to prevent open redirect vulnerabilities:
- Only allows relative URLs starting with
/- Blocks protocol-relative URLs (
//)- Prevents phishing patterns with
@and backslash charactersThis is a solid security implementation.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Cms/Controllers/Member/Registration.php(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Cms/Controllers/Member/Registration.php (6)
src/Cms/Services/Auth/CsrfToken.php (3)
CsrfToken(15-71)getToken(38-46)validate(51-62)src/Cms/Services/Auth/EmailVerifier.php (3)
EmailVerifier(21-247)verifyEmail(123-164)resendVerification(173-190)src/Cms/Services/Member/RegistrationService.php (2)
RegistrationService(22-319)registerWithDto(149-222)src/Cms/Auth/SessionManager.php (2)
get(108-112)set(99-103)src/Cms/Controllers/Content.php (2)
getSessionManager(222-230)redirect(240-251)src/Cms/Controllers/Traits/UsesDtos.php (2)
createDtoFromRequest(137-141)validateDtoOrFail(115-124)
🔇 Additional comments (3)
src/Cms/Controllers/Member/Registration.php (3)
6-8: Service wiring and DTO trait integration look consistentUsing
UsesDtos, injectingEmailVerifierfrom the registry, and instantiatingCsrfTokenwithgetSessionManager()all align with the new auth/services layer and DTO infrastructure. The combined null-check on$_registrationServiceand$_emailVerifierin the constructor is also fine.Also applies to: 29-32, 44-48, 54-61
122-129: DTO-based registration flow is clean and matches the service APICreating the
RegisterUserDTO from the request, validating viavalidateDtoOrFail(), then delegating toRegistrationService::registerWithDto()is a clear and cohesive flow. Removing the unused$userassignment (from earlier iterations) also resolves the previous PHPMD warning without losing behavior.
204-211: Email verification and resend flows correctly delegate toEmailVerifierUsing
$this->_emailVerifier->verifyEmail($token)and$this->_emailVerifier->resendVerification($email)matches the new service contract. Ignoring the resend return value while always showing a generic success message preserves anti-enumeration behavior, and the surrounding throttling and error handling look appropriate.Also applies to: 293-299
…s into feature/dto-implementation
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/Cms/Controllers/Auth/PasswordReset.php (2)
90-91: Consider using the redirect() helper for consistency.While functional, directly using
header()+exitis inconsistent with theredirect()helper method used elsewhere in this controller (e.g., lines 100, 189, 216).Apply this diff for consistency:
- $this->_sessionManager->flash( 'error', 'Invalid CSRF token. Please try again.' ); - header( 'Location: /forgot-password' ); - exit; + $this->redirect( 'forgot_password', [], ['error', 'Invalid CSRF token. Please try again.'] );
143-144: Consider using redirect() helper with named routes.Lines 143 and 153 use hardcoded URLs with
header()+exit, which is inconsistent with theredirect()helper and named routes used elsewhere (lines 100, 189, 216, 220).For consistency, consider:
- $this->_sessionManager->flash( 'error', 'Invalid or missing reset token.' ); - header( 'Location: /forgot-password' ); - exit; + $this->redirect( 'forgot_password', [], ['error', 'Invalid or missing reset token.'] );And similarly for line 153.
Also applies to: 153-154
src/Cms/Controllers/Member/Registration.php (2)
112-126: CSRF validation now defensively handles missing/invalid tokensNormalizing
$tokenRawand explicitly rejectingnull/empty values before calling$this->_csrfToken->validate()correctly avoidsTypeErrors and routes all CSRF failures through the existing redirect flow in both actions. Given the duplication betweenprocessRegistration()andresendVerification(), consider extracting this into a small private helper (e.g.,assertValidCsrfOrRedirect(Request $request, string $redirectRoute)) to keep the behavior in sync over time.Also applies to: 275-288
214-247: Email verification viaEmailVerifieris correctly integrated; consider logging on failureSwitching to
$this->_emailVerifier->verifyEmail($token)and branching on the boolean result matches the describedEmailVerifierbehavior, with a generic error page on exceptions to avoid leaking details. IfEmailVerifierdoesn’t already log failures internally, consider adding a log entry in thecatch (Exception $e)block to aid debugging operational issues.src/Cms/Controllers/Admin/Profile.php (1)
99-103: Email default handling improved; consider consolidating shared logicUsing
$request->post('email', $user->getEmail())avoids clearing the email when not posted and is consistent with the separate password-change form. Given the near-identical update logic in the member controller, you might later extract this profile update input parsing into a shared helper or base method to reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
resources/views/admin/profile/edit.php(1 hunks)resources/views/member/profile/edit.php(1 hunks)src/Cms/Controllers/Admin/Profile.php(3 hunks)src/Cms/Controllers/Auth/PasswordReset.php(10 hunks)src/Cms/Controllers/Member/Profile.php(4 hunks)src/Cms/Controllers/Member/Registration.php(8 hunks)src/Cms/Dtos/CreateCategoryDto.yaml(1 hunks)src/Cms/Dtos/UpdateCategoryDto.yaml(1 hunks)src/Cms/View/helpers.php(1 hunks)versionlog.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- versionlog.md
- src/Cms/Dtos/CreateCategoryDto.yaml
- src/Cms/Dtos/UpdateCategoryDto.yaml
🧰 Additional context used
🧬 Code graph analysis (5)
src/Cms/Controllers/Auth/PasswordReset.php (3)
src/Cms/Services/Auth/PasswordResetter.php (4)
PasswordResetter(21-226)requestReset(76-107)validateToken(115-127)resetPassword(137-170)src/Cms/Services/Auth/CsrfToken.php (3)
CsrfToken(15-71)getToken(38-46)validate(51-62)src/Cms/Controllers/Content.php (2)
getSessionManager(222-230)redirect(240-251)
src/Cms/Controllers/Member/Profile.php (5)
src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)getToken(38-46)src/Cms/Controllers/Content.php (1)
getSessionManager(222-230)src/Cms/Auth/SessionManager.php (1)
set(99-103)src/Cms/View/helpers.php (1)
group_timezones_for_select(229-256)src/Cms/Models/User.php (1)
getTimezone(396-399)
resources/views/member/profile/edit.php (3)
src/Cms/View/helpers.php (2)
gravatar_url(23-27)route_path(70-79)src/Cms/Auth/helpers.php (1)
csrf_field(122-126)src/Cms/Models/User.php (2)
getUsername(78-81)getRole(129-132)
src/Cms/Controllers/Member/Registration.php (6)
src/Cms/Services/Auth/CsrfToken.php (3)
CsrfToken(15-71)getToken(38-46)validate(51-62)src/Cms/Services/Auth/EmailVerifier.php (3)
EmailVerifier(21-247)verifyEmail(123-164)resendVerification(173-190)src/Cms/Services/Member/RegistrationService.php (2)
RegistrationService(22-319)registerWithDto(149-222)src/Cms/Auth/SessionManager.php (2)
get(108-112)set(99-103)src/Cms/Controllers/Content.php (2)
getSessionManager(222-230)redirect(240-251)src/Cms/Controllers/Traits/UsesDtos.php (2)
createDtoFromRequest(137-141)validateDtoOrFail(115-124)
src/Cms/Controllers/Admin/Profile.php (5)
src/Cms/Services/Auth/CsrfToken.php (2)
CsrfToken(15-71)getToken(38-46)src/Cms/Controllers/Content.php (1)
getSessionManager(222-230)src/Cms/Auth/SessionManager.php (1)
set(99-103)src/Cms/View/helpers.php (1)
group_timezones_for_select(229-256)src/Cms/Models/User.php (1)
getTimezone(396-399)
🔇 Additional comments (20)
src/Cms/Controllers/Auth/PasswordReset.php (9)
6-8: LGTM! Service imports updated correctly.The imports correctly reference the refactored authentication services in their new namespace.
27-28: LGTM! Properties correctly updated.Property names and types correctly reflect the refactored services.
38-47: LGTM! Constructor properly initializes refactored services.The constructor correctly retrieves
PasswordResetterfrom the registry and initializesCsrfTokenwith appropriate error handling.
56-75: LGTM! CSRF token handling updated correctly.The method correctly uses the refactored
CsrfTokenservice to retrieve and store the token.
80-80: Previous review comment addressed.The docblock has been correctly updated to
@param Request $request.
87-106: Password reset logic correctly refactored.The method properly validates the CSRF token and delegates to the
PasswordResetterservice. Line 100 now correctly redirects to theforgot_passwordroute instead of the unrelatedadmin_postsroute, addressing the previous review concern.
148-158: LGTM! Token validation and CSRF handling correct.The method properly validates the password reset token and retrieves the CSRF token using the refactored services.
187-212: LGTM! Password reset logic correctly refactored.CSRF validation and password reset operations properly use the refactored
CsrfTokenandPasswordResetterservices.
200-200: Method exists and usage is correct.Verification confirms that
redirectToUrl()is defined in the parent classContent.phpat line 260 with the signature:protected function redirectToUrl( string $url, ?array $flash = null ): neverThe calls on lines 200, 206, and 224 correctly pass a URL string and optional flash array, matching the method signature.
src/Cms/Controllers/Member/Registration.php (3)
6-8: Auth/DTO wiring and CSRF token initialization look consistentUsing
UsesDtos, fetchingRegistrationServiceandEmailVerifierfrom theRegistry, instantiatingCsrfTokenwith the session-backedSessionManager, and exposing the token viaRegistry::set('Auth.CsrfToken', ...)all align cleanly with the new auth/DTO architecture; no issues spotted here.Also applies to: 29-32, 44-55, 88-88
129-137: DTO-based registration flow is clean and matches the service APICreating the
RegisterUserDTO from the request, validating it viavalidateDtoOrFail(), and then delegating toRegistrationService::registerWithDto()gives a clear separation between input mapping, validation, and business logic, and matches theRegistrationServicecontract.
300-320: Resend verification flow preserves privacy and applies throttling correctlyCombining
ResendVerificationThrottlewithEmailVerifier::resendVerification()and always redirecting with a generic success message (both on success and in thecatchblock) maintains good anti-enumeration behavior while still rate-limiting abuse; the flow looks solid.Also applies to: 310-320
resources/views/admin/profile/edit.php (1)
52-60: Grouped timezone select rendering is soundThe select now cleanly iterates over
groupedTimezones, properly escaping region/labels and relying on the precomputedselectedflag; this keeps the view simple and matches the helper’s structure.src/Cms/Controllers/Member/Profile.php (2)
9-9: CSRF token and timezone preparation moved to controller correctlyUsing
CsrfTokenwithgetSessionManager()and storing the token in the registry, plus precomputinggroup_timezones_for_select(..., $user->getTimezone())and passinggroupedTimezonesto the view, is a clean separation-of-concerns and keeps templates dumb.Also applies to: 59-62, 63-66, 71-71, 80-81
100-102: Safer default for email in update flowDefaulting
$user->getEmail()when the POST key is absent avoids overwriting the email with an empty string and aligns with the separate password-change form that omits the email field, while still allowing explicit profile email updates.src/Cms/View/helpers.php (1)
207-257: Timezone grouping helper is correct and reusable
group_timezones_for_selectcleanly encapsulates the region grouping and selection logic into a reusable helper with a well-defined structure that matches how the views consumegroupedTimezones.src/Cms/Controllers/Admin/Profile.php (1)
9-9: CSRF integration and timezone setup are consistent and correctCreating
CsrfTokenwith the session manager, storing the token in the registry, and preparinggroupedTimezonesfromDateTimeZone::listIdentifiers()with the user’s timezone produces the expected data for the admin profile view and mirrors the member implementation.Also applies to: 59-62, 63-66, 71-71
resources/views/member/profile/edit.php (3)
2-10: Header, alerts, and profile picture card are clean and safeThe updated header and flash alerts correctly escape
$success/$error, and the new Gravatar-based profile card uses the existinggravatar_urlhelper with sensible alt text and external-link attributes.Also applies to: 12-25
34-36: Account Information form and timezone select are well-structuredKeeping username/role disabled, making email required, and switching the timezone select to iterate over
groupedTimezones(with escaped region/labels and a precomputedselectedflag) greatly simplifies the view and relies on the shared helper instead of inline processing.Also applies to: 40-47, 49-63, 65-71
76-84: Password-change form separation and CSRF usage look goodThe dedicated Change Password form includes CSRF protection, no longer sends a hidden email field, and cleanly collects current/new/confirmation passwords with explanatory text, matching the controller’s logic that only processes a password change when a new password is supplied.
Also applies to: 88-96, 99-104
Implemented the now dto based request format for data objects.
Summary by CodeRabbit
New Features
Improvements