Easier Custom Authentication errors #15882
Conversation
weaverryan
force-pushed
the
custom-authentication-exception
branch
from
8397fc9 to
1773c01
Compare
| * | ||
| * @author Ryan Weaver <ryan@knpuniversity.com> | ||
| */ | ||
| class CustomAuthenticationException extends AuthenticationException |
There was a problem hiding this comment.
I would convey the "safe" (not sure if safe is the right word here) notion in the class name.
There was a problem hiding this comment.
I suppose that safe here means: can be displayed to the end user.
weaverryan
force-pushed
the
custom-authentication-exception
branch
from
1773c01 to
556abe0
Compare
|
I've just made all the suggested changes. To avoid having the static method call, I've made the |
| */ | ||
|
|
||
| namespace Symfony\Component\Security\Core\Exception; | ||
| use Exception; |
There was a problem hiding this comment.
This should be inlined as a FQCN
|
Status: Needs work |
weaverryan
force-pushed
the
custom-authentication-exception
branch
from
f9a00b0 to
32d9a6a
Compare
|
Updated! Status: Needs Review |
|
|
||
| if ('' !== $message) { | ||
| $this->messageKey = $message; | ||
| } |
There was a problem hiding this comment.
hmm, no you removed the complete method call. In such cases, the Symfony standard is to remove any pre-configured defaults in the property definition and just call $this->messageKey = $message; $this->messageData = $messageData;
weaverryan
force-pushed
the
custom-authentication-exception
branch
from
d616938 to
84dd924
Compare
|
@wouterj check it out now - I just simplified things - I think this makes more sense. |
| * | ||
| * @author Ryan Weaver <ryan@knpuniversity.com> | ||
| */ | ||
| class SafeMessageAuthenticationException extends AuthenticationException |
There was a problem hiding this comment.
I'm really not comfortable with the naming. The idea is that the message is safe to be displayed to the end user. So, what about UserMessageAuthenticationException, or UserFacingAuthenticationException, UserOrientedAuthenticationException? You get my point; the naming should indicate that the message is "for end users". My proposals are probably not the right ones, but we need to find a better name.
There was a problem hiding this comment.
More proposals: PublicAuthenticationException, ExposedAuthenticationException, VisibleAuthenticationException
There was a problem hiding this comment.
I've just tried CustomUserMessageAuthenticationException. It's the the longest of all ideas, but it's clear to me: this is how you show a custom message... to your user. Leanna liked it better than UserFacingMessageAuthenticationException, fwiw ;)
weaverryan
force-pushed
the
custom-authentication-exception
branch
2 times, most recently
from
2d16ab0 to
af25c3b
Compare
weaverryan
force-pushed
the
custom-authentication-exception
branch
from
af25c3b to
d7c1463
Compare
|
Thank you @weaverryan. |
This PR was merged into the 2.8 branch.
Discussion
----------
Easier Custom Authentication errors
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | n/a
| License | MIT
| Doc PR | not yet
This makes failing authentication with a custom message much easier:
```php
throw CustomAuthenticationException::createWithSafeMessage(
'That was a ridiculous username'
);
// or
$e = new CustomAuthenticationException();
$e->setSafeMessage('That was a ridiculous username');
throw $e;
```
Currently, to do this, you'd need to create a new sub-class of `AuthenticationException`, which is way more work than it needs to be. The original design was so that all messages exposed are safe, which is why I've named the methods like I have.
Thanks!
Commits
-------
d7c1463 Adding a class to make it easier to set custom authentication error messages
This makes failing authentication with a custom message much easier:
Currently, to do this, you'd need to create a new sub-class of
AuthenticationException, which is way more work than it needs to be. The original design was so that all messages exposed are safe, which is why I've named the methods like I have.Thanks!