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

refactor: update (app)-auth-v1-users-reset-token to new theming pattern. #200

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hnb-ku
Copy link
Collaborator

@hnb-ku hnb-ku commented Oct 2, 2024

This is part of #187, more details can be found there.

This refactor is a little bit different from the other ones.

Right now we have something like this:

image

This works great because we relay the password policy from Rauthy.

This PR proposes a slightly different way to do the same. Instead of doing the password validation again in the Svelte app, we just surface the errors from Rauthy directly.

The idea here is that it simplifies the app-side route a bit while also reducing the amount of information users have to process.

The PR makes the errors show one at a time and let users update the password (while keeping what they already typed). We get the errors directly from Rauthy.

It looks like this

image

and so on. Users can work through the errors one by one.

Server-load is not a concern here because that form won't show unless we have a csrf token (which is also generated by Rauthy) The form won't display if front-end doesn't get a csrf token from the backend.

The result is that we directly display the errors from Rauthy in the app, but we still control how things look.

As a side-note: I think the current password policy defaults lean too much on the "safe" side and we might be ok relaxing some of them to make the process a bit easier from a UX perspective.

@zicklag zicklag changed the title refactor: update (app)-auth-v1-users-reset-token to new theming pattern refactor: update (app)-auth-v1-users-reset-token to new theming pattern. Oct 2, 2024
@zicklag
Copy link
Collaborator

zicklag commented Oct 3, 2024

@erlend-sh I'd like to get your input on this one.

I agree that the default password policy is stricter than necessary, that's something we must configure after deployment in the Rauthy admin UI, and we do have it simpler on our production site ( same code, but less rules ):

image

I feel like having the user submit the form multiple times to figure out what the password rules are is frustrating, and it'd be better just to show them whether the password is valid as they type it.

It makes sense to me maybe not to show any rules at all until they get it wrong once, and then show them all the rules, but if you have to add an uppercase a lowercase, and have a certain number of characters, then having to submit the form 4 times isn't going to be fun.

@hnb-ku
Copy link
Collaborator Author

hnb-ku commented Oct 3, 2024

I'd honestly go one step further, but I wanted to keep this focused on "theming concerns."

Yes, passwords should have a minimum length. Passwords don't need to be complicated though.

Rauthy's password policy makes sense for Rauthy. I just don't think it applies here.

I would:

  1. enforce a min-length of 10 and a reasonable max-length (with HTML attributes on the client-side and reject via server validation)
  2. check it against something like this CommonPasswords (just an example, anything similar could work)
  3. relax all Rauthy's password rules and make the password policy an application-level concern.

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.

2 participants