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

Multi factor and step up proposal #5

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@AlistairDoswald
Copy link

commented Apr 16, 2019

This design document is based on my post in the mailing list on the administration for multi factor authentication. It has been amended from the following discussions, and has had it scope increased to include step-up authentication.

@ynojima
Copy link

left a comment

Thank you for writing down detailed design doc. It's a good start point for discussion.
I'm developing webauthn4j, and let me join the discussion. I've added some comments.

be separate from other executions like "reset password".

Thus, instead of directly adding "cookie" or "password" as alternatives in the authentication flow, the administrator can add the execution "authentication factor", and under
authentication factor he can add only the valid authenticator executions. Each of the

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

Is there any distinction between "authenticator executions" and "authentication execution"?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 24, 2019

Author

no, it's just the plural

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 24, 2019

I'm not pointing about plural or not.
My focus is 'authenticator executions' or 'authentication execution'.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 25, 2019

Author

Sorry, my mistake, will correct.

|- OTP Form
|- FIDO-2
```
Potentially we could also introduce another type of authentication execution, we could call it "Authenticated on", to simplify the authenticators that bypass the authentication factors. We could then have:

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

It seems the "Authenticated on" is not the "authentication execution" you defined at L. 50 but the "execution" like "authentication factor" defined at L. 49. right?

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

What is the difference between the "Authenticated on" execution and the "authentication factor" execution? Will you introduce new type (class) for it? or will it be just an instance of the "authentication factor" execution?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 24, 2019

Author

The "authenticated on" is a special "authentication factor". For any authentication execution in the "authenticated on", if a user logs in with it, he will skip all other declared authentication factors.
Not sure how I'm going to code it yet though. I was thinking another class, but considering how similar the "authenticated on" and "authentication factor" are, maybe a single class can be used for both.

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 24, 2019

OK. Then, please rewrite the sentence "Potentially we could also introduce another type of authentication execution" to "Potentially we could also introduce another type of authentication factor".

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 25, 2019

Author

You're quite correct, I'll also correct this.

if either the 2nd factor is disabled, or if all authentication executions are disabled in
the second factor, then the login will be single factor only.

For example, for the browser flow this would be:

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

For supporting both FIDO2 password-less authentication and password + FIDO2 as 2nd factor authentication at the same time, how it should be configured?
Is FIDO-2 added to "Authentication factor (1)" and "Authentication factor (2)"?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 24, 2019

Author

I can't see a use case for having FIDO2 as passwordless AND as second factor from the security standpoint. Either it's sufficient as passwordless, in which case you don't need a second factor, or you require 2 factors, and FIDO2 is one of the factors. Did you have a specific use case in mind?

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 24, 2019

I meant a case when two type of users are need to be supported at the same time.
One user has FIDO2 user-verifying authenticator(password-less authenticator) like Feitian BioPass, and the other user has non-user-verigying U2F authenticator.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 25, 2019

Author

I've actually already had a similar discussion in the past with my colleagues, and we've just re-discussed the matter now. If you can really consider the user-verifying authenticator as two factors (which we've debated), then I believe that it must be part of a separate authentication category, as otherwise there's no clear way to differentiate between the Feitian BioPass-like credentials and the non-user-verifying U2F credentials.

In this case there would be "Authentication category user-verifying FIDO2" which would be set in authentication factor (1) and in authentication factor (2) (or in "authenticated on"), and " Authentication category Non-user-verifying U2F" which would be only set in authentication factor (2).

This comment has been minimized.

Copy link
@ynojima
Identity Provider Redirector
Authenticated on
|- Cookie
|- Kerberos [ ] enabled [x] disabled

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

It seems supported options are changed from current implementation (alternatives, required, and disabled).
Why do you need to change?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 24, 2019

Author

Initially, I didn't put them because it wasn't the focus of the explanation. However, to me it may be worthwhile evaluating how useful alternative / required / optional / disabled are in the multi-factor authentication flow, and how they should be used.

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 24, 2019

it wasn't the focus of the explanation

I see.
This is a breaking change, and I suppose it is unacceptable for existing user.

---------------------------------------------------------------------------
Identity Provider Redirector
Authenticated on
|- Cookie

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

Why there is no options for "Identity Provider Redirector" and "Cookie"?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 24, 2019

Author

same as for the comment for design/multi-factor-admin-and-step-up.md


The password policy for the realm should remain, but the OTP policy should disappear, as a user
could have several alternative OTP devices with different settings. As such, the information
about the OTP settings should probably move to information associated with the credential.

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

I suppose the policies should move to information associated with the "authentication category", not the credential.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 24, 2019

Author

Not necessarily. You could have an TOTP token with 6 digits, and another TOTP token with 8 digits. In this case the configuration is linked to the token itself. However, for other cases maybe a policy could be global for an entire authentication category, but I can't think of a specific use case for one.

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 27, 2019

I see. There are two kinds of settings for authentication method.
Some settings are to be associated with credentials and it is better to let it user configurable, and some are to be associated with authentication categories and leave it admins only.
As for TOTP, number of digits is the former as you said, and I think look ahead of window is the latter. Even if let the look ahead of window user configurable, authentication category wide policy for look ahead of window should be remained.

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 2, 2019

Isn't the OTP policy a set of security constraints imposed by administrators? In general, I think users to follow the policies defined by the organization in regards to authentication.

Is a good practice to allow users to set up multiple OTP devices ?

This comment has been minimized.

Copy link
@stianst

stianst May 6, 2019

Contributor

The configuration of the OTP policy for the realm should be configured in the OTP authenticator, not the realm as it is today. Further, the policy would need to be associated with the users credentials. Otherwise, if the realm policy changes current user credentials may no longer work.

I don't think there is a need for users themselves to be able to change the settings. If that is needed that would make the authenticator more complex. The admin would have to be able to define the acceptable options and the user would have to select from the acceptable options when registering. If this is needed it may be better to register multiple OTP authenticators with different policies allowing users to select from different authenticators.

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

With regards to multiple Software OTP per user I don't think there's much more to be discussed and since we're not reached a mutual conclusion I'm going to make a decision here and say we will support multiple software OTP per user. There just is no reason to make this as a limitation. That being said I don't have any issue to make this configurable, but that raises a question on how it would be configurable.

For policies I see the point that there may be multiple OTP authenticators and they should in most cases have the same policy. However, having this on the realm as it is today is not good. Other authentication types will have a similar issue and we don't want to continue to expand on the realm to have "shared config" for authenticators.

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

Further, making sure it is possible to configure multiple Software OTP goes quite far to make sure we don't have any arbitrary limitations in the flow.

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

For policies around OTP perhaps the authentication category may be a place to share common configuration between multiple policies. Not sure if this makes sense though.

This comment has been minimized.

Copy link
@mposolda

mposolda May 14, 2019

I think that long term, if we want to have true flexibility with regards to credentials and types, we may need a possibility for people to introduce their own "credential types" as well as "credential policy types" . Rather than support just policies for few "concrete" specific credential types "password" and "totp" . In other words, maybe something like this:

  • Under left tab "Authentication" in admin console have subtab "Credentials" .

  • Admin can create instances of Credentials based on available Credential Provider Types. Available Credential providers will be by default: password, otp, WebAuthn

  • By default, there will be 1 instance of "password", 1 instance of OTP and 1 instance of WebAuthn created.

  • In case that credential provider supports credential provider policies (which both password and OTP obviously support), there will be another UI page shown when admin can configure credential policies for given credential instance

  • In other words, admin can for example have setup with 1 instance of "password" credential and 2 instances of "otp" credential called "otp-credential-1" and "otp-credential-2" where each of the OTPs will be configured with different OTP policies

  • In the authentication flow configuration, I can "link" the specified authenticator execution (or authentication category?) with the given credential instance. In other words, in the "browser" flow, I can add "OTP Form" authenticator and point to "otp-credential-1" . In the DirectGrant flow, I will create "ValidateOTP" authenticator and point it to "otp-credential-1" as well. This will allow me to share same TOTP policies for both the "browser" and "direct grant" flow as policies are configured for the instance of "otp-credential-1"

  • Migration from previous version should be fine as after migration, we will have 1 instance of "password" credential with password policies, which are in current version configured at realm level. Same for TOTP .

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 16, 2019

This seems a good and flexible approach to credentials in Keycloak. This probably deserves its own design document.

authentication, and the design of the admin GUI for this task. We will also be using the following concepts:
* **Authentication factor**: A new type of execution in the authentication flow, to which only authentication executions can be assigned. This is designed to allow N factors to be added to an authentication flow.
* **Authentication execution**: An execution that can only be set within an authentication factor. This is the actual code that is run to authenticate a user. Examples of authentication executions in Keycloak would be the classes OTPFormAuthenticator, UsernamePasswordForm or ValidateOTP.
* **Authentication category**: A logical grouping between authentication executions and credentials. This design is to allow for multi-tokens.

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

Do you mean "associating multiple OTP devices with an account" by "multi-token"?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 24, 2019

Author

Yes, but not necessarily linked to multiple OTP devices. It could also be for multiple fido2 devices, for example.

This comment has been minimized.

Copy link
@ynojima
} else {
Ask for a login with the first credential in the user's list that corresponds to the first allowed category
}
```

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 21, 2019

Do we need to select one credential from the candidates?
If a user has multiple credentials in an selected authentication category, why don't you try login with all the credentials in the category at once?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald Apr 24, 2019

Author

It may be useful to have such a functionality for some authentication categories, but not others. For example, for FIDO2 it would make sense: you select the category, and any valid FIDO2 token allows you access.
However, imagine that you have a credential that pushes a request to a smartphone. If the user has multiple such devices recorded, you don't necessarily want to push to all those devices.
Also, if you have multiple OTP devices recorded (say multiple RSA tokens), here you create a small security issue, as you multiply by the number of recorded OTP devices the chances an attacker has of success by guessing the code.

This comment has been minimized.

Copy link
@ynojima

ynojima Apr 27, 2019

I see. Thank you for clarification.

This comment has been minimized.

Copy link
@stianst

stianst May 6, 2019

Contributor

We should not allow trying to match against multiple registered credentials. User should always select a specific credential to validate from the list of configured credentials.

This comment has been minimized.

Copy link
@ynojima

ynojima May 6, 2019

How do you think about privacy problem?
In order to let users to select a specific credential, Keycloak need to provide a list of credentials associated with an account to unauthenticated users.
When presenting a list, an attacker may use it as an evidence for a username exists in the directory.

This kind username enumeration risk already exists in a username duplication check of user self sign-up, and many sites do not regard the risk as a problem, and I don't think it is a mistake, but I think we should make an explicit design decision.

This comment has been minimized.

Copy link
@stianst

stianst May 6, 2019

Contributor

I agree this is something that is worth highlighting and explicitly mentioning how it is handled. Preventing username enumeration is in direct conflict with usability and smarter authentication flows. I think it's only feasible to prevent username enumeration if there is a required authentication mechanism (password, can't think of anything else realistic here). With a more dynamic flow it is just not possible anymore. Further it's more and more common to do identity first flows (for instance to redirect to a specific IdP for a given domain name, or not ask for password for a user that has configured passwordless).

In summary my take on this is that username enumeration can be prevent if and only if the first page asks for username and password.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 7, 2019

Author

So the consensus is that we allow username enumeration in order to allow passwordless authentication, and that I'll amend the text to make this explicit?

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

Perhaps word it something like:

"Username enumeration can only be prevent if the first authentication execution collects both username and a credential. This will typically mean that username enumeration requires all users to have a password which is requested as a first step, with optional passwordless experience it is not possible to prevent username enumeration."

@stianst stianst requested review from mposolda and pedroigor Apr 24, 2019

@stianst

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

Sorry for the late response - I've been on PTO + had to catch-up from PTO!

This is a great start - I've only had a chance to read through it quickly, but will go in more depth the next few days and give you some more detailed feedback. Will also try to encourage more people from the team and community to review this as it is a very important part to get right.

@stianst stianst requested review from hmlnarik and stianst Apr 29, 2019

and modifications to the account console, as other design proposals and JIRAs already
specify those modifications.

## Design proposal for [KEYCLOAK-9693](https://issues.jboss.org/browse/KEYCLOAK-9693)

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 2, 2019

I'm not sure about the concepts you are proposing. Isn't the Authentication Flow enough to accommodate the necessary changes to enable MFA?

The Authentication Flow is what users configure to set up user authentication where these flows can be nested so that you can build more complex and dynamic flows. As such, I envision something like that when enabling MFA in Keycloak:

Auth type                         | Requirement                               | Actions
---------------------------------------------------------------------------------------
Cookie
MFA                                [X] ALTERNATIVE  [ ] REQUIRED  [ ] DISABLED  Actions >
  |- Username/Password             [X] REQUIRED
  |- OTP                           [ ] ALTERNATIVE  [X] REQUIRED  [ ] DISABLED
  |- Security Key                  [X] ALTERNATIVE  [ ] REQUIRED  [ ] DISABLED
  |- Passwordless                  [X] ALTERNATIVE  [ ] REQUIRED  [ ] DISABLED

The example above can be understood as:

  • Users using a browser are automatically re-authenticated if a valid session is active on the server
  • Users should use their username/password as a 1st factor and OTP as a 2nd factor, by default.
  • Users are able to choose between two alternative authentication methods when at the OTP page: Security Key and Passwordless as alternative methods of authentication.

Another option would be to only ask the user for his username first, then show a page with the different authentication methods supported by the MFA authentication flow.

The Actions button on the MFA authentication flow redirects the admin to a configuration page where additional configuration could be set in regards to any custom setting we want to support for MFA.

I don't understand yet the need for the concepts you are proposing. I understand the motivation behind some of them such as the category, but I'm still not sure about it. For instance, in regards to users using multiple security keys:

  • Isn't just a matter of allowing users to select the device they want to use ? Considering the user has properly registered the devices each one with a specific alias.

In case you are using "multiple-tokens":

  • Considering that the admin has configured an authentication flow with both OTP and FIDO2 as 2nd factor, isn't just a matter of allowing the user to choose between these two options right after the first log in ?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 3, 2019

Author

The problems I see that the current authentication flow is taken sequentially, and doesn't allow to switch between authentication categories. One of the reasons I suggest to make authentication factors explicit is to clarify the flow, and simplify the manner in which an administrator can define which type of credentials can be used at a specific part of the general authentication flow. Combined with authentication categories, it allows keycloak to know which credential options it can allow the user to present during the user's authentication at which step.

In the scheme you propose, if "passwordless" is for example a fido2 token with biometric identification, I don't know how an admin would decide that this is enough for two factors, or will decide only to set is as 2nd factor. Also, I may misunderstand what you're saying, but the way you describe your flow you can only choose passwordless after entering the username and password, which defeats the purpose. Also, I don't see in your flow any way to have an alternative to username password as a 1st factor. Currently it is required, but making explicit authentication factors would allow any alternative factor, say a biometric device as 1st factor, as long as it provides the necessary context for the following factors. Generally, by making authentication factors explicit, I believe that you can have this sort of flexibility.

The final advantage is that it can be easily tied in with the step-up, allowing to set an level on the authentication factor. It simplifies the matter for keycloak to know which set of authentication executions must be presented for the step up, since they are already bundled together.

You're right to suggest that authentication factors are more similar to a flow than to an executor however, and in the prototype I'm working on I'm currently extending authentication flow (I may refactor this later as I advance though, I've only just been able to start on prototyping this). The current default flow would anyway be needed to reworked to act as you suggest, both for allowing the user to switch between OTP and Fido2 (or any other 1st or 2nd factor), and for configuring the flow through the Actions button . In a way, what I suggest is exactly that: specialised flows that allow for alternative authentication executions to be set, with other benefits that are linked to their design.

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 3, 2019

The problems I see that the current authentication flow is taken sequentially, and doesn't allow to switch between authentication categories. One of the reasons I suggest to make authentication factors explicit is to clarify the flow, and simplify the manner in which an administrator can define which type of credentials can be used at a specific part of the general authentication flow. Combined with authentication categories, it allows keycloak to know which credential options it can allow the user to present during the user's authentication at which step.

In the scheme you propose, if "passwordless" is for example a fido2 token with biometric identification, I don't know how an admin would decide that this is enough for two factors, or will decide only to set is as 2nd factor. Also, I may misunderstand what you're saying, but the way you describe your flow you can only choose passwordless after entering the username and password, which defeats the purpose. Also, I don't see in your flow any way to have an alternative to username password as a 1st factor. Currently, it is required, but making explicit authentication factors would allow any alternative factor, say a biometric device as 1st factor, as long as it provides the necessary context for the following factors. Generally, by making authentication factors explicit, I believe that you can have this sort of flexibility.

Yeah, it is a sequential flow but this is something we can change in order to accommodate flows that rely on user preference as well as on the characteristics of the defined authentication methods.

My example is not clear about this but somehow we have an association between executions and their corresponding factor, this can be something that an admin can configure when at the configuration page of the flow (e.g.: after clicking Actions -> Config) or even by explicitly defining a factor to each execution definition in Keycloak.

Considering this, you could:

  • Define which executions are required by default, sequentially, as a 1st and 2nd-factor authentication. In that case UsernamePassword + OTP
  • Define which executions are alternatives as a 1st or 2nd factor. In that example, Passwordless (e.g.: FIDO+Bio) and Security Token are 1st and 2nd-factor alternatives, respectively.

In an ideal flow, the login page would prompt the username (or show some avatar he can click). After that, the user is redirected to the 1st step which is set to prompt the user for his password but with options to select an alternative authentication method. In this case, the available option is "Passwordless".

After choosing one of the authentication methods in this 1st step the user is redirected to the 2nd step. In this step, the user is presented with OTP by default, but with options to select an alternative authentication method. In this case, Security Key.

However, there is a catch here that I'm not sure how you are addressing in your proposal. Considering the previous example where both UsernamePassword and Passwordless are defined as 1st-factor authenticators. How to configure that when users authenticate using Passwordless they don't need to inform a 2nd-factor ?

In a nutshell, I like your idea I'm just not sure if the best way to push the "Authenticator Factor" concept to much into the Flows but instead provide either more options in "Requirements" to configure the factors or, probably better, have a set of options in the Flow configuration that are specific for MFA (which I think gives much more flexibility in terms of configuration).

The final advantage is that it can be easily tied in with the step-up, allowing to set an level on the authentication factor. It simplifies the matter for keycloak to know which set of authentication executions must be presented for the step up, since they are already bundled together.

I agree with setting the level on the authentication factor/execution. But I think there are some nuances that we need to discuss regarding that.

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 3, 2019

Another point I would like to make is regarding the association between the different authentication methods and whether or not they are considered a 1st, 2nd or Nth factor.

Taking as an example passwords and OTP, I think password is always a SFA while OTP will be always a 2FA. The same goes with a security key, which is always a 2nd factor.

Passwordless (e.g.: SMS, FIDO2+Bio, Notification) on the other hand can be considered a 1st factor as they don't require from users another factor.

Considering that, do we really need to distinguish in the UI the different mechanisms and whether or not they are a 1st or 2nd factor?

This comment has been minimized.

Copy link
@stianst

stianst May 6, 2019

Contributor

What we have today is rather limiting, so I'm confident that we need to extend on that. I'm not yet convinced that authentication categories completely solves the problem though. Take WebAuthn for instance. That is a rather tricky one as it can act as a single factor, a 2nd factor or as multiple factors on its own, depending on the type of WebAuthn device used. Another example here is an external authenticator mechanism that can provide multiple factors, but is visible in Keycloak as a single authenticator.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 13, 2019

Author

OK, I think I see where we're going with this. Being able to select between executions simply replaces sequential execution when "selectable" authentication executions are available. If this is the case, then I think that the following logic for authentication executions makes sense:

  • ALTERNATIVE for authentication execution denotes a selectable choice that can validate a subflow, and is the normal case.
  • OPTIONAL for an authentication execution no longer makes sense in a multi-token, multi-factor setting, as it is superseded by the "select a credential" concept.
  • REQUIRED is strange in a setting in which you can switch between authentication executions, as it implies that it must be done. Something like Username (selection of the user) has to be set to "required", but I'm not sure how you process the logic if other authentication executions can be set to "required".

I'm not sure what the logic should be if there's some non-user interactive authentication executions in the flow. Either they should always be executed first, or it's the responsibility of the administrator to make sure that the subflow makes sense, as otherwise, once a user-interactive authentication execution is encountered, they are no longer automatically executed.

The other case which is ambiguous is what happens if there there's a flow with subflows mixed with authentication executions, something like this:

Auth type
------------------------------------------------------------
Identity Provider Redirector
Auto                              [x] Alternative
  |- Cookie                       [x] Alternative
  |- Kerberos                     [x] Alternative
Authenticate                      [x] Alternative
  |- Username                     [x] Required
  |- 1st factor                   [x] Required
         |- Password              [x] Alternative
         |- Subflow 1
               |- ...
         |- Webauthn              [x] Alternative
         |- Subflow 2
               |- ...  
         |- OTP                   [x] Alternative

Then Subflow 1 and Subflow 2 will never be executed if they are set to Alternative, as they are not selectable. If they are set to Required (or Optional with conditions), then the other alternatives (password, Webauthn or OTP) are never executed, as Subflow 1 and Subflow 2 are a necessary condition to validate the 1st factor, no matter what they contain. So it's the responsibility of the administrator for making subflows that make sense. Does this seem correct?

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 13, 2019

Yeah, I think so.

I see OPTIONAL as "if the user has the credential, then prompt him". So if the user does not have the credentials set, the corresponding choice won't' be shown. This is different than ALTERNATIVE, where you actually try to challenge the user regardless.

I see REQUIRED as "among these options, prompt for this credential first". So when the user lands at the login page, it will be shown the "required"/default challenge and the alternatives. Maybe REQUIRED could be changed to DEFAULT.

This comment has been minimized.

Copy link
@stianst

stianst May 14, 2019

Contributor

I read the above example as 1st factor is required, with the following options password, subflow-1, webauthn, subflow-2 and otp.

Alternative as I interpret it should mean within a sub-flow there is a list of executions or sub-flows that are alternatives to each other where one has to be executed. They can be tried in order, or by user preference, or selected directly.

The most flexible would be if we allow executions and sub-flows to be alternatives to each other within a sub-flow. I can see cases where a step consists of multiple executions. For example Kerberos can collection username and authenticate, WebAuthn devices can contain username, but password requires username. Perhaps a bit fictional, but you could have something like:

[1st]
[Kerberos]
[WebAuthn]
[Pass Flow]
- [Username]
- [Password]

Basically I want us to think about all options and crazy authentication schemes folks can come up with and make sure we design something that can be extended, rather than re-designed in the future.

This comment has been minimized.

Copy link
@stianst

stianst May 14, 2019

Contributor

I find it slightly hard to interpret optional. I guess it means it is required if configured? I've never really liked it though. I would rather have a new requirement conditional, where the condition is clearly stated.

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 16, 2019

Indeed, the OPTIONAL could be replaced by CONDITIONAL while still keeping the same behavior.

I don't quite understand your example but I think I got the idea. In a nutshell, administrators have the responsibility to use flows that make sense while we should do exactly what was defined (respecting order, requirements, etc). Like @AlistairDoswald suggested.

In his flow, we should still be able to provide selectable executions by looking at the executions within the subflows. Or even allow subflow selection by users, which I think would help in your case.

SAML supports LoA with the [Identity Assurance Profiles](http://docs.oasis-open.org/security/saml/Post2.0/sstc-saml-assurance-profile.html) part of the protocol, and with the
`<RequestedAuthnContext>` element.

#### Specifying authenticator categories

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 2, 2019

Regarding OIDC, I think it should be enough to use acr_values as the mean to enforce a specific authentication method, and as a consequence a security level.

The amr claim should hold values for each authentication method used to assert the user's identity.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 3, 2019

Author

The protocol specifies both the acr_values and requesting the acr claim as an essential claim as valid methods for requesting a security level, so why not implement both? Aside from the fact that it is more work, obviously. It would allow for greater compatibility with whatever SPs may wish to use this feature.

For the amr you are correct, I thought I described it thus in the document, but if you think that it's not clear enough I'll rework the text to make it clearer.

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 3, 2019

Yeah, we should do both. Where acr_values is what we would use to request tokens with a specific acr value.

### Extending protocol implementation within Keycloak

The following protocols or optional parts of protocols must be implemented for OIDC:
* Handling of the `acr` claim, `acr_values` parameter and `prompt` parameter as

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 2, 2019

I think the prompt parameter is already implemented so that you can force users to log in even though they have an active session on the server. Or are you referring to something more specific?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 3, 2019

Author

I confess that I don't know what is already implemented here, just that it's useful for the step-up. But if it's already implemented, all the better.

This comment has been minimized.

Copy link
@stianst

stianst May 6, 2019

Contributor

An interesting thing here is that OIDC has prompt=login which says "SHOULD prompt the End-User for reauthentication". What reauthentication means may differ though. It may be asking for password, otp, or both. Not sure how we would handle that and make it configurable what reauthentication means.

This comment has been minimized.

Copy link
@mposolda

mposolda May 6, 2019

Currently "prompt=none" just skips the cookie. I think we can just preserve this behaviour and ensure that all authentication factors, which are marked as REQUIRED (and eventually OPTIONAL in case that user has particular credentials set) will need to be re-authenticated.

So for example, there is a flow, which has "password" authentication factor as REQUIRED and "otp" factor as OPTIONAL. Then using parameter combinations can work like this:

  • "prompt-login" will make sure that user needs to re-authenticate with password. Just if he has OTP set, he will need to re-authenticate with OTP as well.

  • "acr_values=otp" will ensure that user needs to authenticate (or re-authenticate) only with the OTP (No password re-authentication required). IMO it will be good if using "acr_values" will ensure that user needs to re-authenticate with OTP even if he already authenticated with it before in this session?

  • "prompt=login&acr_values=otp" will ensure that user needs to authenticate (or re-authenticate) with both password as REQUIRED and OTP as REQUIRED. Password is REQUIRED as it is REQUIRED authentication factor configured and OTP is required as it is requested by the "acr_values" parameter.

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 6, 2019

I'm not sure if is a good idea to use acr_values to reference authentication methods (amr). IMO, we should try to request re-authentication based on security levels as much as possible. Or even use a custom amr_values claims to indicate the expected authentication methods.

This comment has been minimized.

Copy link
@mposolda

mposolda May 14, 2019

I am sorry for the confusion. I didn't mean to mix acr_values (authentication factors/subflows) with the amr claims (which reference authenticators). In the example, when I used acr_values=otp I meant to use the authentication subflow (or authentication factor), which contains OTP authenticator, not directly the OTP authenticator itself. I should probably use some more explicit naming like acr_values=otp-subflow or something like that.

In other words, in my previous comment, I meant to use prompt=login together with specific acr_values to specify which authentication subflows/factors need to be re-authenticated.

IMO We don't need something like custom field amr_values as you can always model the flow with the acr_values . In other words, if you ever need to have something like request authentication with OTP, you should model subflow otp-flow, which will contain the OTP authenticator as the only authenticator and then reference this subflow with the acr_values claim like acr_values=otp-flow

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 15, 2019

Author

I think I understand what you mean with your examples.

IMO it will be good if using "acr_values" will ensure that user needs to re-authenticate with OTP even if he already authenticated with it before in this session?

I'm not sure that just having acr_values=otp_flow should make keycloak re-authenticate with the otp_flow LoA. For me, if the user is already connected with the otp_flow LoA, then if the client requests it, keycloak should send over the token without asking the user for an new authentication. This is for the purpose of SSO: if client 1 and client 2 require otp_flow LoA, then logging with that level to the first should automatically grant access to the 2nd.

Rather, I think that re authentication should only be done on login=prompt, as the protocol suggests. However, the protocol is not super clear on what should be done when prompt=login is used with acr_values (and more generally with LoA). But I think that keycloak can act exactly as is asked by the request from the client. Perhaps it's best to illustrate it with the following example:
Keycloak is configured with two authentication factor subflows, 1st factor which provides LoA lvl1 and 2nd factor which provides LoA lvl2. Imagining a user already logged in with a LoA lvl2, then

  1. the client asks prompt=login -> keycloak re-asks the user to log in with 1st factor subflow, as it would do on a normal login with no acr_values specified.
  2. the client asks prompt=login&acr_values=lvl1 --> same as case 1, but this time because it's explicitly specified.
  3. the client asks prompt=login&acr_values=lvl2 --> keycloak only executes the subflow for 2nd factor. Note that if the user hadn't already been logged in, keycloak would also have executed the subflow for 1st factor.

The only problem I see with this is that there's no way for a client to force keycloak to execute the subflow for the 1st factor and for the 2nd factor. The protocol does allow for having a list of acr_values (this is also true for if you're using the acr claim set to essential), but states that it's for "values appearing in order of preference". I suppose that you could have the following

  1. the client asks prompt=login&acr_value=lvl1 lvl2 --> keycloak executes the subflow for 1st factor and 2nd factor

But this doesn't seem to follow the specification.

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 16, 2019

I agree with the usage of acr_values that you and Marek are proposing. Depending on the values and user's authentication state we perform the necessary steps to gain the requested levels.

I think what you described in #4 is covered by the spec. Why do you think it is not ? The prompt=login should ask user for re-authentication and acr_values or acr claim (when using the claims request parameter) should instruct the LoA you want for a user. If you provide both lvl1 and lvl2 we would end up considering the highest.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 17, 2019

Author

I think that the protocol doesn't clearly state what was defined in this situation. Following the definition of acr_values and prompt=login in the OIDC protocol, the declaration prompt=login&acr_value=lvl1 lvl2 from the client to the IdP should translate as "I want you to force the user to re-authenticate, I would prefer if he was logged in with LoA lvl1, but LoA lvl2 is OK as well".

Making the user log in with the subflows corresonding to LoA lvl1 and LoA lvl2 is not wrong, but it's not completely correct either. But I don't see any other way in the protocol that tells the idp "first make the user authenticate with LoA lvl1, and then with LoA lvl2", aside from sending two authentication requests. If everyone is happy with the behaviour "the client asks prompt=login&acr_value=lvl1 lvl2 --> keycloak executes the subflow for 1st factor and 2nd factor", then it can be implemented that way.

This comment has been minimized.

Copy link
@pedroigor

pedroigor May 17, 2019

Based on this statement from the spec, where the Authorization Server MUST return an acr Claim Value that matches one of the requested values, I assume that if you request prompt=login&acr_value=lvl1 lvl2, we as a AS, are free to process whatever we find is the best as long as we return one of them. IMO, if you have N and N+1 we should always enforce the highest level. If the user is already on N we don't need to enforce N, but only N+1. If the request was for N only we do N even if the user is authenticated and set with N. If you request N+1 only we only do N+1 if user already is set with N, etc.

Does it make sense ? @mposolda , @stianst wdyt ?

@pedroigor

This comment has been minimized.

Copy link

commented May 2, 2019

@AlistairDoswald, that is a great start and document. Thanks for all the details in the document, not an easy task.

I'll be helping with this and my initial comments should be enough to understand more your proposal.

@mposolda
Copy link

left a comment

Thanks for the design, it is very good work IMO. I left comments inline.

and modifications to the account console, as other design proposals and JIRAs already
specify those modifications.

## Design proposal for [KEYCLOAK-9693](https://issues.jboss.org/browse/KEYCLOAK-9693)

This comment has been minimized.

Copy link
@mposolda

mposolda May 6, 2019

I am not sure if it is better to introduce Authentication factor or if we can just "enhance" Authentication execution and add the "LoA" to it (and maybe some other configuration options and tweaks).

Just few points:

  • Migration: I think it is possible to change the default KC behaviour, however during migration from previous versions (EG. usecase when admin used KC 5 and now he is migrating to KC 7), the behaviour needs to be preserved.

  • Some flows can be currently configured in a way that you can nest multiple alternative flows to more levels. For example default configuration of "First Broker Login" flow currently uses 3 levels. Not sure if this would be possible if you change ALTERNATIVE authentication executions to "authentication factors" as those are always 1st level if I understand correctly?

In other words, I think that support for ALTERNATIVE executions needs to be probably kept.

----------------------- ----------------------
```

This schema serves as a base for all credentials and allows them to be

This comment has been minimized.

Copy link
@mposolda

mposolda May 6, 2019

If I understand correctly, it will be good if we can support the possibility when you have same authenticator multiple times in the authentication flow in different authentication factors.

So for example you will have the flow with the authentication factor with LoA 2, which will contain OTP authenticator with requirement of having 6 digits. And in the same flow, you will have authentication factor with LoA 3, which will contain OTP authenticator with requirement of having 8 digits.

Will this be possible with the current design?

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 8, 2019

Author

With the current design no, I imagined that if you have the same authentication factor within two authentication factors (or N authentication factors), then validating it once will make it valid for all the others. This was to allow a type of credential to count for N factors.

I have an idea of how to reconcile your idea with the authentication factors (or with the improved authentication flows proposed by @stianst), but do you feel that this is a necessary feature?

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

Not sure I see a use-case for this with regards to OTP. I'm a bit concerned that it highlights some limitation in the design though.

used for the appropriate authentication executions. This link allows Keycloak to present
alternative credentials for an execution during login.

To simplify selection in the GUI, it would also be useful to do some refactoring to ensure

This comment has been minimized.

Copy link
@mposolda

mposolda May 6, 2019

+1 for improving this. Currently when you're adding the Authentication Execution, there is no way for "restrict" authenticator implementations based on the context (flow type). So for example ValidateOTP Authenticator is visible even if you're creating "browser" flow, but it shouldn't be displayed as it is applicable just for the direct grant flow (Browser flow has OTPFormAuthenticator).

Perhaps we should add more flow types (Currently there is just "generic" and "client" as you pointed) and each applicable Authenticator (or AuthenticatorFactory?) can have method like "getApplicableFlowTypes", which will return list of flow types for which the Authenticator implementation is valid. Some implementations are valid for more flow types (EG. OTPFormAuthenticator is applicable for "browser" or "post-broker-login" flows and maybe even in some others).

I am not sure if this can be done as part of your work or rather as separate thing?

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

+1 I was thinking we should introduce flow types where an authenticator can advertise which flows it is compatible with. We can introduce it as a new optional method that returns all flow types, and update the built-in authenticators to only support those that they should be used with. I'd say that is general enhancements and doesn't need to be covered by this design proposal though.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 8, 2019

Author

I also feel that this shouldn't be part of the design proposal. Should I leave the suggestion of the improvement in the text, but explicitly say that it is out of scope for this design document, or just remove the text?

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

Perhaps add a section to the Appendix to mention future enhancements that can be made?

and would simplify the selection of authentication executions within a authentication
factor.

It may also allow a simplification of the naming when selecting an authentication

This comment has been minimized.

Copy link
@mposolda

mposolda May 6, 2019

Not sure about that. I see your point, but I would rather still keep the naming like "OTP" + "OTP Form" to have it more clear which Authenticator implementation will be used. But maybe it's just me... Not sure what is better regarding usability for admins.

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

If we introduce a way to limit what flow types an authenticator is compatible with Form is no longer needed in the name as only OTP Form would be available in the browser flow.

In addition to its function, an authentication category will also have a set of
data/metadata that describes it. This can be a general field that will be found
in all authenticator categories, for example description information (internationalised),
an icon or something more functional like data for OIDC's amr claims (see section on

This comment has been minimized.

Copy link
@mposolda

mposolda May 6, 2019

I think it will be good if values of "amr" claims are configurable as well. For example some admin may want something like "amr" with value "otp" in the token, but some other admin will prefer to have something like "urn:something:otp". So maybe we need some configurable option on authentication category (or authentication execution) for this? Current userSession contains list of used authentication executions during the authentication. Not sure if this is sufficient information or something more needs to be added to userSession to being able to properly add the "amr" values into the token..

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

We should have an internal scheme, which can be mapped to other schemes on a per-client basis. Different clients within the same realm may expect different values, which means we need to support some way of mapping them regardless.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 8, 2019

Author

The amr values are defined in RFC 8176, and I thought this should be sufficient. I was initially intending to have the authentication category statically defined in the code and in resource files, but if we are moving towards more configurable values (for this and for OTP policies for example, although this is still being discussed), then it will also require a table in the database.

I don't see why not having this configurable, for the purpose of greater compatibility, with the default values being those from the RFC.

This comment has been minimized.

Copy link
@mposolda

mposolda May 14, 2019

+1 for having it configurable. It will be probably good to have the protocol mapper, which will somehow support the mapping of "internal" execution to the amr value. As @stianst pointed, we can have different clients wanting different values, but I guess that usual case will be more clients sharing same value. We can probably introduce some new default "Client scope" and add the protocol mapper for adding amr as part of this client scope. That will allow more clients share the same amr values, but will also other clients to use specific values.

IMO this is something, which can be surely added in some later stage - will be nice to avoid have one big PR containing all the changes, but ideally divide it as much as possible to more PRs for easier review :)

-----------------------------
```

With `ID` the primary key of the credential, `authenticator_category` a string

This comment has been minimized.

Copy link
@mposolda

mposolda May 6, 2019

I am not sure about the "secret_data" and "credential_data" and have their format as JSON stored in the DB.

IMO it will be good if we have possibility to introduce custom credential types and if the whole credentials page is "auto-rerendered" in the admin console (and maybe in the account mgmt too). In other words, we can have something like "Credential Storage SPI" and each CredentialStorageProviderFactory can contains list of available configuration options and based on that, the particular credentials page is automatically rendered in the admin console. In other words, the individual options of Credential will be just key-value pairs. I can imagine something similar to what we use for various Component providers (or even for Authenticator Configurations, or UserStorage providers which are also rendered generically). The fact whether credential option is secret or not could be handled with some flag in credential metadata. Just a note that ProviderConfigProperty class already contains the "secret" flag on it and some parts in admin console are rendered based on it. Maybe this can be re-used for the credentials directly.

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

How it's stored in the DB is independent on how you edit in the UI. Unless there is a need to query the DB for individual attributes it should just be stored as a blob, just like the component model should have been stored as a blob.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 8, 2019

Author

Describing these values as json is actually a result of an earlier discussion with @stianst. Functionally, I don't see any difference between your proposition and using json, aside from the fact that json allows sub-elements. I think that what you propose works both with key-values or json.

Having parts marked secret (however it is done), is necessary I believe. First because it marks which part of the credential should not be exposed or transmitted, but also because it is best (security) practice to have confidential data in a database to be encrypted by an external key to protect against a potential sql injection attacks. I didn't necessarily want to bring this up as part of the design proposal, but no matter which scheme is used, encrypting the secret data should be considered.

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

+1 To splitting secret/confidential and other data

In the future we are planning to refine how we do storage and the plan is to store things in the DB as a blob when there is no need to index or query on what is within the blob. It simplifies the DB schema as well as migration. An authenticator may for instance have a version added to the credential data that allows it to read and update values stored in an old format. That would be much harder if stored as individual attributes.

This comment has been minimized.

Copy link
@mposolda

mposolda May 14, 2019

As you pointed, key-value pairs has avantage for querying. This is probably not an advantage if you always know the user as you can retrieve the credentials of given user and filter them programatically (for example based on credential type and device). However if you don't know the user in advance, it may be beneficial. ATM I don't know about any use-case when you don't know the user, but just wanted to point it in case you can thing about some. For example with X509 or SPNEGO authentication, you don't know the user, but it still works fine as you can retrieve the user from the sent SPNEGO token / X509 certificate.

For the versioning/upgrades and nested attributes, it would work with key-value pairs as well, even if JSON is more friendly for those use-cases.

In shortcut, I don't insist on key-value pairs. JSON would probably works just fine unless we have use-case for a need to query credentials of unknown user as I mentioned above (which we probably don't have).

### Extending protocol implementation within Keycloak

The following protocols or optional parts of protocols must be implemented for OIDC:
* Handling of the `acr` claim, `acr_values` parameter and `prompt` parameter as

This comment has been minimized.

Copy link
@mposolda

mposolda May 6, 2019

Currently "prompt=none" just skips the cookie. I think we can just preserve this behaviour and ensure that all authentication factors, which are marked as REQUIRED (and eventually OPTIONAL in case that user has particular credentials set) will need to be re-authenticated.

So for example, there is a flow, which has "password" authentication factor as REQUIRED and "otp" factor as OPTIONAL. Then using parameter combinations can work like this:

  • "prompt-login" will make sure that user needs to re-authenticate with password. Just if he has OTP set, he will need to re-authenticate with OTP as well.

  • "acr_values=otp" will ensure that user needs to authenticate (or re-authenticate) only with the OTP (No password re-authentication required). IMO it will be good if using "acr_values" will ensure that user needs to re-authenticate with OTP even if he already authenticated with it before in this session?

  • "prompt=login&acr_values=otp" will ensure that user needs to authenticate (or re-authenticate) with both password as REQUIRED and OTP as REQUIRED. Password is REQUIRED as it is REQUIRED authentication factor configured and OTP is required as it is requested by the "acr_values" parameter.

and modifications to the account console, as other design proposals and JIRAs already
specify those modifications.

## Design proposal for [KEYCLOAK-9693](https://issues.jboss.org/browse/KEYCLOAK-9693)

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

There is no need to introduce a new concept called authentication factor, but rather it would be better to expand on the concept of sub-flows. Sub-flows would only need to have the optional requirement and a way to set the LoA value.

Here's an example on how that could look like:
image

Rather than hard-wire or expand on the logic behind the flows for setting LoA values, deciding when to do 2nd factor, etc. we should rather have conditional markers in the flow.

In the above example there would be a "Set LoA value" marker at the end of 1st and 2nd sub-flows.

Similarly to skip 1st sub-flow when we are doing a step-up to two factor we'd introduce a new requirement called Conditional where conditions within the sub-flow decides if the sub-flow should be executed or not.

|- OTP Form
|- FIDO-2
```
Potentially we could also introduce another type of authentication execution, we could call it "Authenticated on", to simplify the authenticators that bypass the authentication factors. We could then have:

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

I somewhat get the idea of the Authenticated on, but I would rather have a basic set of building blocks like sub-flows rather than introduce completely new concepts. The flow itself and its logic should be configurable, but if it's built of highly specialised blocks like authenticated on where the logic on how that is handled is hard-coded it is no longer configurable/customizable.

I vote to remove the Authenticated on concept from the proposal.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 8, 2019

Author

You're quite right about this, even with a normal flow, simply having an authentication execution outside an authentication factor (or the sub-flows you describe) is enough to have keycloak authenticate and skip the rest of the factors.

used for the appropriate authentication executions. This link allows Keycloak to present
alternative credentials for an execution during login.

To simplify selection in the GUI, it would also be useful to do some refactoring to ensure

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

+1 I was thinking we should introduce flow types where an authenticator can advertise which flows it is compatible with. We can introduce it as a new optional method that returns all flow types, and update the built-in authenticators to only support those that they should be used with. I'd say that is general enhancements and doesn't need to be covered by this design proposal though.

and would simplify the selection of authentication executions within a authentication
factor.

It may also allow a simplification of the naming when selecting an authentication

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

If we introduce a way to limit what flow types an authenticator is compatible with Form is no longer needed in the name as only OTP Form would be available in the browser flow.

|- FIDO-2
```

#### Authentication categories

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

I somewhat follow the idea of authentication categories and the need for something like it. There's some work to be done here though to expand on this concept to complete it. How are they created? How are they used? Etc.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 8, 2019

Author

I'll describe this in greater detail. My main dilemma was, "are authentication categories something statically created in the code?", or "are authentication categories something that can be created and managed by an administrator, with a database presence?". Comments up until now indicate the latter.

This comment has been minimized.

Copy link
@stianst

stianst May 8, 2019

Contributor

Perhaps in reality its both? One thing I'm not to keen on with the current flows is that the "static" data from code is copied into the DB. It makes upgrades complex.


No changes are necessary here.

#### Required Actions

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

I'm not following this section at all. I would say that what is needed is a required action to configure a valid 2nd factor, where the required action somehow lists what 2nd factor mechanisms are available and allows the user to pick. When the user picks it would somehow trigger the corresponding required action.

This comment has been minimized.

Copy link
@AlistairDoswald

AlistairDoswald May 8, 2019

Author

Having a "set 2nd factor required action" was actually the initial suggestion that my colleagues had. However, we saw the following difficulties:

  • How to find from the code the 2nd factor authentication categories to be set, when there is no explicit "2nd factor" type. And this is actually more difficult with the proposed subflow system, as there's even less to denote what a 2nd factor authentication is.
  • Even if you can distinguish all authentication categories that are a within a 2nd factor, what do you do when there are multiple flows defined, each with different authentication executions?

This is why we came up with the idea of configurable required actions, in which an administrator can explicitly say something like "I want the user to be required to set a credential for authentication category OTP or authentication category FIDO2". However, I suppose that we can simplify the configuration of such a required action to get the list of all authentication factors (or sub-flows) and the administrator simply chooses the one he wants. The required action can then infer the required authentication categories from the authentication executions within the selected flow.


The password policy for the realm should remain, but the OTP policy should disappear, as a user
could have several alternative OTP devices with different settings. As such, the information
about the OTP settings should probably move to information associated with the credential.

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

With two factor authentication you need at least one backup option. Personally I have a phone and a tablet, where I use the tablet as backup (both devices are just as secure as each other, well the tablet is more secure as it rarely leaves the house ;)). Of course alternatives could be SMS, email, backup codes, etc.. However, why should we limit it? It's up to the admin of the realm and the user to choose what they want, not us.

bypasses the login actions (e.g. session cookie, kerberos,...) he will arrive at a
login prompt that corresponds to his authentication category.

The rule for selecting which credential is chosen for a user is

This comment has been minimized.

Copy link
@stianst

stianst May 7, 2019

Contributor

Authenticators (authentication executions) is what displays forms. That needs to be made obvious in this section. I'm also having a hard time to decipher the logic here, both the bullets and the pseudo code.

@stianst

This comment has been minimized.

Copy link
Contributor

commented May 7, 2019

See my inline comments for more details, but here is a list of things that needs to be addressed:

  • Authentication factor is not needed as a new concept. We already have sub-flows that can be utilised for this purpose
  • I would like as little as possible of the logic be hardcoded in the authentication processor, but rather have configurable conditions/markers in the flow itself
  • Need better explanation of authentication categories. What are they? How will they be defined? How will they be used?
  • How will the list of available credentials / categories be retrieved? How is the default execution for the default credential/category for the user found in the sub-flow (or factor as it is currently) and invoked. How is a specific execution invoked if the user selects an alternative.
  • The concept of authenticated on should be removed. I would prefer to not have special elements like authentication factor and authenticated on when sub-flows can do the job

I have more comments with regards to step-up, but I believe it's covered sufficiently in its current form so we can focus on the multiple authentication choices issue (password, passwordless, multiple alternative two factor, etc.)

@stianst

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

Q - At this point does it make sense that we wait for a second draft, then continue the discussion afterwards?

@stianst stianst self-assigned this May 8, 2019

@AlistairDoswald

This comment has been minimized.

Copy link
Author

commented May 8, 2019

A - I'd like to address a couple of points before starting on a second draft, namely using using sub-flows instead of authentication factors and the comment on required actions. I'll try to write down my comments before tomorrow.

@mposolda
Copy link

left a comment

Added few more comments . Sorry for being late..

### Extending protocol implementation within Keycloak

The following protocols or optional parts of protocols must be implemented for OIDC:
* Handling of the `acr` claim, `acr_values` parameter and `prompt` parameter as

This comment has been minimized.

Copy link
@mposolda

mposolda May 14, 2019

I am sorry for the confusion. I didn't mean to mix acr_values (authentication factors/subflows) with the amr claims (which reference authenticators). In the example, when I used acr_values=otp I meant to use the authentication subflow (or authentication factor), which contains OTP authenticator, not directly the OTP authenticator itself. I should probably use some more explicit naming like acr_values=otp-subflow or something like that.

In other words, in my previous comment, I meant to use prompt=login together with specific acr_values to specify which authentication subflows/factors need to be re-authenticated.

IMO We don't need something like custom field amr_values as you can always model the flow with the acr_values . In other words, if you ever need to have something like request authentication with OTP, you should model subflow otp-flow, which will contain the OTP authenticator as the only authenticator and then reference this subflow with the acr_values claim like acr_values=otp-flow

-----------------------------
```

With `ID` the primary key of the credential, `authenticator_category` a string

This comment has been minimized.

Copy link
@mposolda

mposolda May 14, 2019

As you pointed, key-value pairs has avantage for querying. This is probably not an advantage if you always know the user as you can retrieve the credentials of given user and filter them programatically (for example based on credential type and device). However if you don't know the user in advance, it may be beneficial. ATM I don't know about any use-case when you don't know the user, but just wanted to point it in case you can thing about some. For example with X509 or SPNEGO authentication, you don't know the user, but it still works fine as you can retrieve the user from the sent SPNEGO token / X509 certificate.

For the versioning/upgrades and nested attributes, it would work with key-value pairs as well, even if JSON is more friendly for those use-cases.

In shortcut, I don't insist on key-value pairs. JSON would probably works just fine unless we have use-case for a need to query credentials of unknown user as I mentioned above (which we probably don't have).


The password policy for the realm should remain, but the OTP policy should disappear, as a user
could have several alternative OTP devices with different settings. As such, the information
about the OTP settings should probably move to information associated with the credential.

This comment has been minimized.

Copy link
@mposolda

mposolda May 14, 2019

I think that long term, if we want to have true flexibility with regards to credentials and types, we may need a possibility for people to introduce their own "credential types" as well as "credential policy types" . Rather than support just policies for few "concrete" specific credential types "password" and "totp" . In other words, maybe something like this:

  • Under left tab "Authentication" in admin console have subtab "Credentials" .

  • Admin can create instances of Credentials based on available Credential Provider Types. Available Credential providers will be by default: password, otp, WebAuthn

  • By default, there will be 1 instance of "password", 1 instance of OTP and 1 instance of WebAuthn created.

  • In case that credential provider supports credential provider policies (which both password and OTP obviously support), there will be another UI page shown when admin can configure credential policies for given credential instance

  • In other words, admin can for example have setup with 1 instance of "password" credential and 2 instances of "otp" credential called "otp-credential-1" and "otp-credential-2" where each of the OTPs will be configured with different OTP policies

  • In the authentication flow configuration, I can "link" the specified authenticator execution (or authentication category?) with the given credential instance. In other words, in the "browser" flow, I can add "OTP Form" authenticator and point to "otp-credential-1" . In the DirectGrant flow, I will create "ValidateOTP" authenticator and point it to "otp-credential-1" as well. This will allow me to share same TOTP policies for both the "browser" and "direct grant" flow as policies are configured for the instance of "otp-credential-1"

  • Migration from previous version should be fine as after migration, we will have 1 instance of "password" credential with password policies, which are in current version configured at realm level. Same for TOTP .

In addition to its function, an authentication category will also have a set of
data/metadata that describes it. This can be a general field that will be found
in all authenticator categories, for example description information (internationalised),
an icon or something more functional like data for OIDC's amr claims (see section on

This comment has been minimized.

Copy link
@mposolda

mposolda May 14, 2019

+1 for having it configurable. It will be probably good to have the protocol mapper, which will somehow support the mapping of "internal" execution to the amr value. As @stianst pointed, we can have different clients wanting different values, but I guess that usual case will be more clients sharing same value. We can probably introduce some new default "Client scope" and add the protocol mapper for adding amr as part of this client scope. That will allow more clients share the same amr values, but will also other clients to use specific values.

IMO this is something, which can be surely added in some later stage - will be nice to avoid have one big PR containing all the changes, but ideally divide it as much as possible to more PRs for easier review :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.