Document how to bypass 2FA entirely in a given context #74
Conversation
|
So you can't make the decision whether 2fa should be applied or nor based on the security token, but you'd want to make it based on the authenticator that was used to generate that security token? I guess you have multiple authenticators all using the same security token class. Do I understand that right? |
|
Yes that is correct. I could for sure also create a new token, but IMO the token should be used to represent a user and shouldn't have to change here. The passport/badge system is used to configure the auth flow and so it would be much more natural to do it at that level. This could be a feature request I guess, but I needed a solution now, and this attribute on the token appears to work :) |
|
Attribute on token is in fact a valid solution imo. I'd need to look into what the flow with the new authenticator system is like. The decision is based on the token mostly because that was the only way to distinguish authentication methods in the classic security system. |
|
Yup works fine for current setup. In a future world where there is only authenticator. I think having a badge makes more sense, actually this is kinda covered by #67 already - if the TwoFactorAuthBadge was part of this bundle and recognized by it, then I could have added the badge, called markResolved() on it already in the authenticator to make it skip the badge auth step. |
|
I'll add a note that this approach is only working with the authenticator system :) If the PR for Symfony is accepted I can include the passport in the |
…vent (scheb) This PR was merged into the 5.3-dev branch. Discussion ---------- [Security] Add passport to AuthenticationTokenCreatedEvent | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | yes | Deprecations? | no | License | MIT This is a follow-up to my previous PR #37359, which added `AuthenticationTokenCreatedEvent` to the new authenticator-based security system to inspect the security token before it becomes effective to the security system. It **adds the passport** that was used to generate that token to the event, so that it can be inspected as well. Reasoning: 1) It makes the event more aligned with other security events (which are also providing the passport) 2) I see valid use-cases when you'd want to look into the passport/badges to decide if you'd want to make modifications to the security token. @Seldaek mentioned to me in scheb/2fa#74 that he'd like to have the ability to add a badge from his custom authenticator class, which then influences 2fa being triggered or not. Having the passport in the event would make that a straight forward task. I would like to add this to Symfony 5.3, since @wouterj plans to stabilize the authenticator security system for that release, so I believe this is worth adding it now rather than later. The constructor change could be considered a BC break, but since authenticator system is experimental, I believe it's fair to make that change now before declaring it "stable". Commits ------- 74196e0 Add passport to AuthenticationTokenCreatedEvent
This is partially a question too.. is this the best way?
I first tried setting up a custom 2fa condition class but I did not have access to the Passport via the AuthenticationContextInterface, so it made things tricky as I couldn't easily add a TwoFactorBypassBadge or something to look at. I had to use the token, then when looking at internals further I found out that I may not even need a custom condition at all if I'm modifying the token already.. But IMO a passport badge would be more natural in the new authenticator.