Skip to content

Conversation

@wouterj
Copy link
Member

@wouterj wouterj commented Nov 28, 2020

Q A
Branch? 5.2 (hopefully? sorry to keep pushing the barrier here)
Bug fix? no
New feature? yes (sort of)
Deprecations? no
Tickets -
License MIT
Doc PR -

These are 2 suggestions we found while implementing make:auth for the new system (symfony/maker-bundle#736):

Impact on a custom login form authenticator (as generated by the new maker):

  • Automatically add PasswordUpgradeBadge if there is a user password with valid password credentials.
     // ...
     return new Passport(
         new UserBadge($userIdentifier),
         new PasswordCredentials($password),
         [
    -        new PasswordUpgradeBadge($password),
             new CsrfTokenBadge('authenticate', $csrf),
         ]
     )
    Note that this does not automatically migrate all passwords: it still relies on PasswordUpgraderInterface to be implemented on the user loader/provider.
  • Add default implementation of AbstractFormLoginAuthenticator::support()
    - public function supports(Request $request): ?bool
    -  {
    -      return self::LOGIN_ROUTE === $request->attributes->get('_route')
    -          && $request->isMethod('POST');
    - }

cc @weaverryan @jrushlow

@wouterj wouterj requested a review from chalasr as a code owner November 28, 2020 16:29
@carsonbot carsonbot added Status: Needs Review Security DX DX = Developer eXperience (anything that improves the experience of using Symfony) Feature labels Nov 28, 2020
@carsonbot carsonbot changed the title [Security][DX] Automatically add PasswordUpgradeBadge + default support() impl in AbstractFormLoginAuthenticator [Security] [DX] Automatically add PasswordUpgradeBadge + default support() impl in AbstractFormLoginAuthenticator Nov 28, 2020
@wouterj wouterj changed the base branch from 5.x to 5.2 November 28, 2020 16:30
Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you have a look at the fabbot failures?

@wouterj
Copy link
Member Author

wouterj commented Nov 28, 2020

Done! Sorry, I forgot to check the PR status after creating it.

@derrabus derrabus added this to the 5.2 milestone Nov 29, 2020
@fabpot
Copy link
Member

fabpot commented Nov 30, 2020

Thank you @wouterj.

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

Labels

DX DX = Developer eXperience (anything that improves the experience of using Symfony) Feature Security Status: Reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants