Skip to content

Conversation

mposolda
Copy link
Contributor

@mposolda mposolda commented Apr 25, 2025

…ng threshold

closes #39214

Draft for now. I will add some documentation (including deprecation of password policy) if this approach is ok

…ng threshold

closes keycloak#39214

Signed-off-by: mposolda <mposolda@gmail.com>
@mposolda mposolda self-assigned this Apr 25, 2025
Copy link
Contributor

@rmartinc rmartinc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO the approach is OK. But maybe we can extend this to also configure the total number of codes (now it's 12 fixed, we can also add that number to the configuration). And yes, this should be documented to comment the property in the recovery codes section and the deprecation of the password policy too. Note that this was tech preview, so we can be more drastic.

@mposolda
Copy link
Contributor Author

@rmartinc Thanks for the feedback!

Yes, I was also thinking about this (Adding possibility of configurable count of recovery codes).

Also regarding the "Warning threshold", I was also thinking that we can add some screen during authentication when user has small amount of recovery codes remaining. So after user successfully authenticates with recovery code and he has less than "Warning threshold", there would be some screen like You have only 3 remaining recovery codes left. Do you want to generate new set of codes? with options to generate new codes (Which would add required action to authenticationSession or UserModel probably) or continue with authentication. Warnings in account console is maybe sufficient, but it is a possibility that not everyone uses account console. On the other hand, maybe it is sufficient what we have today (user is forced to generate new codes just after last recovery code is used). WDYT? Maybe we can check if other vendors like Google or Github are doing something like this (Will probably require to manually try many authentication attempts to simulate having lower threshold... :) )

Could we perhaps address additional things as a follow-up instead of doing everything in this PR? So far, I can see follow-ups like:

  • Ability to configure count of recovery codes

  • Documentation (As it can be better to update docs in one step after we have all follow-ups done)

  • Additional screen during authentication like "Only 3 recovery codes remaining. Do you want to generate new?" . But not 100% sure about this, so would appreciate feedback.

Considering that, I've marked this PR as "Ready for review" as we may do docs in the follow-up PR though.

Copy link
Contributor

@rmartinc rmartinc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mposolda! Approved! Please create the issue for the documentation change and add it to the epic (I was about to create it, but I cannot link it to the epic, so I had to bother you anyway).

@mposolda
Copy link
Contributor Author

@rmartinc Thanks! Will create the documentation task.

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

Successfully merging this pull request may close these issues.

Use required action configuration instead of password policy for warning threshold
2 participants