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

website/docs: document the Password Uniqueness Policy #11000

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

verkaufer
Copy link

Relates to #10631

Details

Introduces public documentation for the Unique Password Policy feature added by #10631

Documentation changes were move into a separate PR to keep feedback conversations focused on the specific changes.

👉 This PR should not be merged before #10631.


Checklist

  • Local tests pass (ak test authentik/)
  • The code has been formatted (make lint-fix)

If an API change has been made

  • The API schema has been updated (make gen-build)

If changes to the frontend have been made

  • The code has been formatted (make web)

If applicable

  • The documentation has been updated
  • The documentation has been formatted (make website)

@verkaufer verkaufer requested a review from a team as a code owner August 21, 2024 02:49
Copy link

netlify bot commented Aug 21, 2024

Deploy Preview for authentik-docs ready!

Name Link
🔨 Latest commit ae05509
🔍 Latest deploy log https://app.netlify.com/sites/authentik-docs/deploys/66c6b1bb2d094f0008db494b
😎 Deploy Preview https://deploy-preview-11000--authentik-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Aug 21, 2024

Deploy Preview for authentik-storybook ready!

Name Link
🔨 Latest commit ae05509
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/66c6b1bb05be5a0008f7edfe
😎 Deploy Preview https://deploy-preview-11000--authentik-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tanberry tanberry changed the title policies: document the Password Uniqueness Policy website/docs: document the Password Uniqueness Policy Aug 21, 2024
Copy link
Contributor

@kensternberg-authentik kensternberg-authentik left a comment

Choose a reason for hiding this comment

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

A few language changes to help people be aware of the mechanics. I feel a solid mental model helps people feel more confident about the reliability of a system.

@@ -46,6 +46,21 @@ Starting with authentik 2022.11.0, the following checks can also be done with th
- Check the password hash against the database of [Have I Been Pwned](https://haveibeenpwned.com/). Only the first 5 characters of the hashed password are transmitted, the rest is compared in authentik
- Check the password against the password complexity checker [zxcvbn](https://github.com/dropbox/zxcvbn), which detects weak password on various metrics.

### Password Uniqueness Policy

Prevents users from reusing old passwords when changing their own password. The policy offers an option to limit the number many previous passwords to consider during evaluation.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this reads awkwardly. Suggestion: "This policy allows admins to specify how many previous password hashes should be kept to prevent re-use. By default, the password history depth is zero, permitting users to re-use any previous password."

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

I like that rephrasing a lot more and I'll get that changed!

The UI displays 1 as the default, which I can change to be 0 (I figured if you're setting up a the policy you want it to actually do something 😅 ). Is there a preference?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree about defaulting to 1, from a usability perspective. Also zeros can be unnerving, lol.

website/docs/policies/index.md Outdated Show resolved Hide resolved
When this policy is bound and enabled to at least one [User write stage](../flow/stages/user_write.md):

1. authentik compares the hashes of the new password and the old password for a match. The policy check fails and exits if the hashes match.
2. authentik copies the hashed form of the user's old password for future comparison
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: "When the policy succeeds, the user's current password hash is copied into the password history. Passwords hashes are removed, oldest first, from the user's password history if it has more entries than the current depth setting."

Copy link
Author

Choose a reason for hiding this comment

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

👍 Much cleaner

We could note if more than one Password Uniqueness Policy is bound & active anywhere in authentik, then the system maintains a history depth equal to the greatest configured depth of all Password Uniqueness Policies.

e.g. I create 2 copies of a Password Uniqueness Policy. I configure one policy with a depth of 1, and another with a depth of 10. authentik will maintain 10 old password hashes.

It's a minor detail and probably only important for someone wanting to fully understand how many old hashes are stored. I'll leave it up to you & @tanberry whether we include that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I remember you telling me about this. Yes I think we should indeed include this info in the docs. I predict it will avoid a few GitHub Issues in the future, where people ask what-the-heck.

I think that will also help people understand that there can be multiple instances on a policy.

Copy link
Contributor

@tanberry tanberry left a comment

Choose a reason for hiding this comment

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

Thanks @verkaufer for the docs! I agree with Ken's rewordings ( I almost always like them more than my own), and then I caught one typo, but looks great otherwise!

I'm pre-approving just so that I am not the blocker when you/we are ready to merge this.

website/docs/policies/index.md Outdated Show resolved Hide resolved
Copy link

codecov bot commented Aug 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.76%. Comparing base (b6cf889) to head (ae05509).
Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11000      +/-   ##
==========================================
+ Coverage   92.69%   92.76%   +0.07%     
==========================================
  Files         736      736              
  Lines       36360    36360              
==========================================
+ Hits        33703    33729      +26     
+ Misses       2657     2631      -26     
Flag Coverage Δ
e2e 49.28% <ø> (+0.10%) ⬆️
integration 25.05% <ø> (ø)
unit 90.23% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Successfully merging this pull request may close these issues.

3 participants