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

fix: add confirmation to disable password login #3829

Merged
merged 7 commits into from
May 23, 2023

Conversation

nunogois
Copy link
Member

@nunogois nunogois commented May 22, 2023

https://linear.app/unleash/issue/2-1071/prevent-users-from-disabling-password-authentication-when-there-are-no

Improves the behavior of disabling password based login by adding some relevant information and a confirmation dialog with a warning. This felt better than trying to disable the toggle, by still allowing the end users to make the decision, except now it should be a properly informed decision with confirmation.

image

  • Password based administrators: Admin accounts that have a password set;
  • Other administrators: Other admin users that do not have a password. May be SSO, but may also be users that did not set a password yet;
  • Admin service accounts: Service accounts that have the admin root role. Depending on how you're using the SA this may not necessarily mean locking yourself out of an admin account, especially if you secured its token beforehand;
  • Admin API tokens: Similar to the above. If you secured an admin API token beforehand, you still have access to all features through the API;

Each one of them link to the respective page inside Unleash (e.g. users page, service accounts page, tokens page...);

If you try to disable and press "save", and only in that scenario, you are presented with the following confirmation dialog:

image

@vercel
Copy link

vercel bot commented May 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2023 1:33pm
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2023 1:33pm

@github-actions
Copy link

After enabling strictNullChecks this PR would be increasing the number of null check errors from 293 to 5814.
Make sure your branch is up-to-date with main and check the diff in the console output to pinpoint the offending files.

1 similar comment
@github-actions
Copy link

After enabling strictNullChecks this PR would be increasing the number of null check errors from 293 to 5814.
Make sure your branch is up-to-date with main and check the diff in the console output to pinpoint the offending files.

@github-actions
Copy link

After enabling strictNullChecks this PR would be increasing the number of null check errors from 293 to 5814.
Make sure your branch is up-to-date with main and check the diff in the console output to pinpoint the offending files.

Copy link
Contributor

@gastonfournier gastonfournier left a comment

Choose a reason for hiding this comment

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

I think this is good, I just have 2 small things I'd like to improve

src/lib/services/account-service.ts Show resolved Hide resolved
src/lib/types/stores/account-store.ts Outdated Show resolved Hide resolved
gastonfournier added a commit that referenced this pull request May 22, 2023
## About the changes
This action started to fail and despite being safe to ignore it's still
annoying. I'm disabling it for now until I can look into what's going on
with it

Example:
#3829 (comment)
Copy link
Contributor

@gastonfournier gastonfournier left a comment

Choose a reason for hiding this comment

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

This LGTM!

Copy link
Member

@sighphyre sighphyre left a comment

Choose a reason for hiding this comment

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

LGTM!

@nunogois nunogois merged commit c0bcc50 into main May 23, 2023
16 checks passed
@nunogois nunogois deleted the fix-disable-password-login-confirmation branch May 23, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants