-
Notifications
You must be signed in to change notification settings - Fork 74
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
Badge support for AuthenticationManager and API 2fa-auth #67
Comments
To me it looks like, the only thing that you're using from the bundle is the TOTP authenticator. Actually, you don't even need that one, you could use the underlaying library directly and skip the bundle's glue code. Managing the intermediate state between identifying yourself (login) and completing authentication (2fa complete, fully authenticated) is the one main problem that the bundle is solving. In your use-case you don't have that state, since you're doing the whole authentication within a single request. So you have a single-step authentication process that is checking for multiple factors. With the new authenticator system this can actually be done in a very elegant way, adding badges to add additonal requirements to the authentication process. You demonstrated that very well in the example. So is there any other bundle features that you're using, that would actually make it worth using the bundle? Because to me it looks, with for a single-step authentication use-case you actually don't need the bundle ;) |
🤔 The more I think about this, the more it makes sense to use the bundle for such a use case - if you want extra features i.e. trusted IPs, trusted devices, backup codes. I remember that people have asked me before if they could pass the 2fa code together with username+password, but with the old security system it wasn't easy to implement, because its architecture allows only one authentication provider per request (except you're pulling off some really bad hacks). With the new authenticator system it should be possible and relatively easy to support "single step authentication with multiple factors". I think that would be a nice addition for bundle version 6, which would leverage the new possibilities of authenticator-based security. |
The reason we use the bundle is simply that we fully use it for the web clients which have session based auth, so there the full integration makes sense and works great. The API however is designed differently so I had to find a solution there. I assumed that backup codes would just work as an input when using totpAuhenticator->checkCode but I haven't set them up yet nor have I looked how it's implemented. If that isn't the case then it sounds like an additional feature request :) IMO it'd make sense that a backup code is just seen as a valid code but admittedly there might be extra considerations preventing this. Anyway i got this working so no rush for me. If you add something in v6 or whenever that lets me delete some code great but no pressure. I just felt this might benefit others. |
Ah, I see. Yea makes sense when you're also using it on the web application.
No, unfurtunately backup codes wouldn't work. The Trusted IPs and trusted devices are handled on a whole different layer. These are conditions if 2fa should even be started - in your implementation those could be additional conditions if the
Jep, accepted, I can see the value. |
I am actually currently also implementing this with creating a custom badge and listener. Are you still open to supporting this inside the bundler @scheb ? |
@michnovka It would definitely be interesting to see a working implementation. I myself haven't really looked into this. |
Context: I am currently migrating a project to the experimental AuthenticationManager as well as using this bundle instead of a homegrown 2fa solution.
We do have an API with an existing endpoint for logging in users which returns a special code to the client if 2fa is required, and then expects the login API call again with username, password, AND the 2fa code all present in one to do authentication in a single request.
This does not really play well with your API guidelines which require a two-request flow, which IMO is more complex as the client needs to keep even more state.
Anyway I ended up implementing this myself as it isn't that hard, but I also got pretty familiar with the new auth code as I had to do various authenticator implementations. So I am mostly posting this here for others who may need the help to figure this out.. and I am thinking if this bundle offered better support for it in the future it would maybe be good (could be a v6 thing for sure).
I now create this badge in my API authenticator, which gets added on the passport:
The badge class is a very simple value object:
And the auth listener which checks the badge is also not that complex, which is why I think it might be worth including here:
There are also two special exceptions MissingTwoFaCodeException and InvalidTwoFaCodeException both extending AuthenticationException that I added to be able to render these correctly in onAuthenticationFailure.
Note: Doing this way, I did not configure 2fa on the API firewall at all, as it's handled purely within the Authenticator via the badge.
The text was updated successfully, but these errors were encountered: