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

2fa disabling ask password #6408

Merged
merged 19 commits into from
Aug 24, 2019

Conversation

yeraydiazdiaz
Copy link
Contributor

@yeraydiazdiaz yeraydiazdiaz commented Aug 10, 2019

Fixes #5825

Here's a capture of the new functionality:

2fa_remove_ask_password

@yeraydiazdiaz yeraydiazdiaz requested review from nlhkabu and di August 11, 2019 14:34
@nlhkabu
Copy link
Contributor

nlhkabu commented Aug 12, 2019

Hi @yeraydiazdiaz, thanks for this PR 😁

A couple of items:

Could we please update the error text (when a user enters an incorrect password) to be more meaningful than 'invalid credentials'?

Screenshot from 2019-08-12 08-48-36

Maybe something like:
"Incorrect password. Try again." ??

Can we also please use this functionality for the 'remove API token' modal as well?

Thanks!

@brainwane
Copy link
Contributor

@yeraydiazdiaz is this ready for re-review?

@yeraydiazdiaz
Copy link
Contributor Author

Yes, it is. Here is a capture of the changes requested by @nlhkabu

token_remove_ask_password

warehouse/manage/forms.py Outdated Show resolved Hide resolved
@yeraydiazdiaz yeraydiazdiaz force-pushed the 2fa-disabling-ask-password branch from c1af5ac to 46fc81f Compare August 15, 2019 09:10
@yeraydiazdiaz
Copy link
Contributor Author

yeraydiazdiaz commented Aug 15, 2019

I've implemented the change suggested by @di and rebased on master, this is ready for re-review.

@yeraydiazdiaz
Copy link
Contributor Author

I merged master onto this branch and had to update the some of the tests, I believe everything's correct but @woodruffw could you review again just in case?

Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

LGTM!

@nlhkabu nlhkabu merged commit 443cecc into pypi:master Aug 24, 2019
@nlhkabu
Copy link
Contributor

nlhkabu commented Aug 24, 2019

Thank you @yeraydiazdiaz ! 🎉

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.

2FA Ask for password not username when disabling 2FA
5 participants