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

[Security] Adding a GuardAuthenticatorHandler alias #25274

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@weaverryan
Member

weaverryan commented Dec 2, 2017

Q A
Branch? master
Bug fix? kinda
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets none
License MIT
Doc PR This feature is not currently documented

The security.authentication.guard_handler service is actually meant to be available for users to use. Specifically, the authenticateUserAndHandleSuccess() method is useful to auto-login the user after, for example, registration, but maintain all the behavior of a normal login (success behavior, trigger the login event).

So, it should have an autowiring alias.

@Simperfit

This comment has been minimized.

Show comment
Hide comment
@Simperfit

Simperfit Dec 3, 2017

Contributor

It seems that the file loader can't load the file anymore, could you please check why ?

Contributor

Simperfit commented Dec 3, 2017

It seems that the file loader can't load the file anymore, could you please check why ?

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Dec 3, 2017

Member

@Simperfit yea, I borked it :). Let's try that again

Member

weaverryan commented Dec 3, 2017

@Simperfit yea, I borked it :). Let's try that again

@nicolas-grekas

3.4?

@nicolas-grekas nicolas-grekas added this to the 4.1 milestone Dec 4, 2017

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Dec 4, 2017

Member

Yea, it could be considered a bug fix: it really should be usable by the end-user. When the service became private in 4.0, that made it not easily usable. So, I think 3.4 makes sense (we could do 3.3... but I don't want to make a policy of back-porting all aliases to 3.3: we just need to make sure they exist on 3.4 so people can upgrade)

Member

weaverryan commented Dec 4, 2017

Yea, it could be considered a bug fix: it really should be usable by the end-user. When the service became private in 4.0, that made it not easily usable. So, I think 3.4 makes sense (we could do 3.3... but I don't want to make a policy of back-porting all aliases to 3.3: we just need to make sure they exist on 3.4 so people can upgrade)

@fabpot fabpot changed the base branch from master to 3.4 Dec 4, 2017

@fabpot fabpot changed the base branch from 3.4 to master Dec 4, 2017

@fabpot

fabpot approved these changes Dec 4, 2017

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Dec 4, 2017

Member

Thank you @weaverryan.

Member

fabpot commented Dec 4, 2017

Thank you @weaverryan.

fabpot added a commit that referenced this pull request Dec 4, 2017

bug #25274 [Security] Adding a GuardAuthenticatorHandler alias (weave…
…rryan)

This PR was submitted for the master branch but it was squashed and merged into the 3.4 branch instead (closes #25274).

Discussion
----------

[Security] Adding a GuardAuthenticatorHandler alias

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | kinda
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | This feature is not currently documented

The `security.authentication.guard_handler` service *is* actually meant to be available for users to use. Specifically, the `authenticateUserAndHandleSuccess()` method is useful to auto-login the user after, for example, registration, but maintain all the behavior of a normal login (success behavior, trigger the login event).

So, it should have an autowiring alias.

Commits
-------

844c402 [Security] Adding a GuardAuthenticatorHandler alias

@fabpot fabpot closed this Dec 4, 2017

@weaverryan weaverryan deleted the weaverryan-patch-1 branch Dec 4, 2017

This was referenced Dec 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment