[Security] decoupled UserPassword constraint from SecurityBundle #6892
Conversation
…lidator classes to fit the coding convention. Also added a bunch of unit tests to validate the UserPasswordValidator object and introduced a new UserPassword constraint class in the SecurityBundle. The base UserPassword constraint class in the Security component is now decoupled from the SecurityBundle.
… class constraint.
|
👎 Changing the class name is an unnecessary BC break, and your base class in the bridge is totalmy useless as it tells that its validator is a new instance UserPasswordValidator which won't work. |
|
I don't have a new class in the bridge namespace. I just added a new I can revert the BC break for the namespace if necessary. |
|
@hhamon the |
|
I don't really understand why it cannot be validated. How this constraint could be validated anyway by leaving it only in the security component? If I want to use the I can't understand why the current |
|
it cannot be validated by building a new instance of the validator when asked to validate the constraint, because the validator needs to have access to the SecurityContext. |
|
I understand but that means the |
| @@ -34,7 +34,7 @@ public function validate($password, Constraint $constraint) | |||
| $user = $this->securityContext->getToken()->getUser(); | |||
|
|
|||
| if (!$user instanceof UserInterface) { | |||
| throw new ConstraintDefinitionException('The User must extend UserInterface'); | |||
| throw new ConstraintDefinitionException('The User must be an instance of the UserInterface interface.'); | |||
There was a problem hiding this comment.
"implement" ? (instances of interface do not exist)
|
I agree with @stof comments. One more thing: the |
|
Hi Victor, Ok so I will drop this PR but I will open another one, which contains the unit tests for the validator. Cheers. |
|
Hi Hugo, It might also be interesting to get @fabpot and @bschussek opinions about dropping the "s" - it should not happen before 3.0 I think (if it should at all). |
|
Closing this PR in favor of #6947. |
|
I'm +1 to rename |
|
Ok will do. I'm opening a new PR. |
UserPasswordconstraint class in their code)