Skip to content

Conversation

@chalasr
Copy link
Member

@chalasr chalasr commented May 13, 2020

Q A
Branch? master
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #36804
License MIT
Doc PR -

@wouterj
Copy link
Member

wouterj commented May 13, 2020

Imho, we should do both suggestions of #36804: The always_authenticate_before_granting setting is completely ignored in the new system. It's better to make this explicit by not allowing it to be true when the new system is used.

Rationale: This option is heavily misused to reload user roles on the fly, which is not the intention of this option. This use-case has to be addressed by a new feature in Symfony Security, fixing all related issues with changing roles on the fly (such as being logged out since 4.4). As this was considered out of scope for the authenticator system PR, it was decided that if one needs to reload on the fly, one needs to stick with the old system in 5.1.

@chalasr chalasr force-pushed the noop-auth-retval branch from bfaf4bc to c7a04d7 Compare May 13, 2020 13:35
@chalasr
Copy link
Member Author

chalasr commented May 13, 2020

@wouterj Makes sense, exception added. Thanks.
Suggestions welcome to improve the error message

@chalasr chalasr force-pushed the noop-auth-retval branch 2 times, most recently from 7664179 to 39af372 Compare May 13, 2020 16:27
@nicolas-grekas
Copy link
Member

Thank you @chalasr.

@nicolas-grekas nicolas-grekas merged commit a86058c into symfony:master May 16, 2020
@fabpot fabpot mentioned this pull request May 16, 2020
@chalasr chalasr deleted the noop-auth-retval branch May 17, 2020 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Authenticators and always_authenticate_before_granting causes null TypeError

5 participants