Skip to content
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

Support authentication manager #685

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

iisisrael
Copy link

Supports authentication listener creation when using Symfony's new authentication system.

# config/packages/security.yaml
security:
    enable_authenticator_manager: true

I didn't see any need to add to the config documentation. Whether you have the above config enabled or not, it just works (see SecurityExtension::createAuthenticationListeners()).

@iisisrael
Copy link
Author

Still working on this. Adding an authenticator that manages authentication via the new Passport with user and credential badges.

@iisisrael
Copy link
Author

Not sure about the user provider dependency injection yet, will be testing in the context of another app.

@iisisrael
Copy link
Author

Token storage is already taken care of in AuthenticatorManager::handleAuthenticationSuccess() when ::executeAuthenticator() is absolutely sure everything is kosher, so the token storage dependency in the OAuthAuthenticator was not needed.

@iisisrael
Copy link
Author

@deguif I noticed when testing this in an app that an unauthenticated request results in an AccessDeniedException rather than an AuthenticationCredentialsNotFoundException. Is this a result of other recent changes, or did I bork something here?

If you don't know off the top of your head, I can run the same test based on a different branch of our app.

@vladimir-light
Copy link

Does cleint_credentials/autherization_code grant work?

@victormacko
Copy link

@iisisrael was this work finished in the end by chance? ... was super keen to transition to Symfony v6 but can't without the new authentication system :/
Thanks for all your hard work btw, it's super appreciated!

@iisisrael
Copy link
Author

@victormacko yes, at least for this PR, or until changes are requested. Feel free to contribute by forking my fork, and I'll pull any changes into here.

Some other Symfony 5.3 deprecation fixes were made on a fork off of this PR branch here.

@iisisrael
Copy link
Author

Rebased to include commits "Remove support for Propel" and "Move Travis to GH Actions and fix CI builds".

@iisisrael
Copy link
Author

Removing support of Symfony 5.1 and 5.2 as a result of ProxyManager dependency change for ClassGenerator.

@tomazartack
Copy link

Why is this not getting merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants