Repository navigation
Conversation
|
@rmartinc / @mposolda - I learned that this can be a blocker for those who want to use Keycloak as an IDP for EntraID where EntraID is the master tenant that only supports SAML (not OIDC), and the master tenant enforces a 2nd factor. So I'm giving it a go. I chose to make this a mapper as this is specific to EntraID, and to keep the logic encapsulated. I tried to add some logic to detect if two factors have been used to authenticate the user. This works in the test, not sure if I miss something here. An alternative could be to configure a list of authenticators to be considered as part of the 2FA ensemble, or even two groups one for the first group, and another one for the second group. Once we agree on the design, I'll prepare additional documentation and test it with the community. |
|
@ahus1 We have seen several requests similar to this. The |
|
Thank you for the feedback. I will see that I make it more generic in the set value. Still I will keep its use case for detecting multifactor authentication. For me, leaving it to users to write Java code is the least favourite option. I want Keycloak to work with EntraID out of the box and with great docs. This will make Keycloak relevant for a wider community. For now I hope this change is good enough for the problem at hand, and that it will not break in corner cases. |
630e377 to
686d2b5
Compare
|
I pushed some more changes to make it generic. This is a screenshot of the new mapper that can be added a SAML client. It is designed to add one of such a mapper to a client. To ensure that those authenticators have been triggered for the user, update your authentication flows as necessary, and if needed have a custom flow for the specific client in case you want to enforce MFA for a specific client. To build a provider JAR, build this branch locally using Then copy the JAR file to the |
a1de5ca to
dd818d8
Compare
dd818d8 to
325c90f
Compare
|
@ahus1 The previous weeks I was playing a bit with issue #10155 and it's not incredibly complicated. At least for the server part. So I would have preferred to not implement this and to go with the generic approach of using step-up implementation. I think it is more or less the same effort that you have spent here. The main idea about step-up implementation is that the admin configures an authentication workflow with the values they consider. Each value is mapped to a AuthnContextClassRef. This way the client can request a specific auth context class ref in the rquest, the authentication flow should have that value mapped to a authn level (1, 2 or whatever), and after that we need to add a similar mapper to the one implemented here to attach the class ref to the response. It's generic and covers also the step-up implementation. You can also also set a minimum level for a client to ensure that client uses the level you want. But as you have already done this I don't know what to say. I prefer to not support this extra mapper which IMHO makes no much sense once we have the generic step-up. But if you insists in adding this I will do the review. |
|
@rmartinc - I'd say this is a working solution for some of the users, which has been waiting for a while to have support for Entra ID (and others). Note that the step-up authentication setup sounds more like a realm-level configuration, while it IMHO would need to contain a client-specific configuration as each client might want to see different values of the AuthnContext. If you think that the step-up authentication will make it into a release soon (say, 26.5), we can make this feature preview-only, and then deprecate it once the more comprehensive solution is available. Please let me know your thoughts. |
|
@rmartinc - once you go down the ACR route and mappings, my knowledge is limited and looking at the code won't help much I fear. I'm happy to discuss your changes if you walk me through them if you think this is can help. Please go ahead and schedule a 1:1 for this. |
Closes keycloak#11721 Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
325c90f to
687dd6e
Compare
|
With SAML step-up authentication implemented in #44185, this issue should now be resolved as well. Therefore I'm closing it and and I will also close the associated PR. |
Closes #11721