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

KeyError: 'password' when trying to disable another user's 2FA #3705

Closed
wes-otf opened this issue Jan 2, 2024 · 4 comments · Fixed by #3708
Closed

KeyError: 'password' when trying to disable another user's 2FA #3705

wes-otf opened this issue Jan 2, 2024 · 4 comments · Fixed by #3708
Assignees
Labels
Type: Bug Bugs! Things that are broken :-/

Comments

@wes-otf
Copy link
Contributor

wes-otf commented Jan 2, 2024

As an administrator with an instance that has ENFORCE_TWO_FACTOR=True, when going to disable another user's 2FA via wagtail admin, a KeyError: 'password' results from the apply users' views.py, attempting to set the label of the password form field.

This makes an administrator unable to disable 2FA for a user.

@wes-otf wes-otf added the Type: Bug Bugs! Things that are broken :-/ label Jan 2, 2024
@wes-otf
Copy link
Contributor Author

wes-otf commented Jan 2, 2024

I took a brief look at this already, it seems like only form field in this view is confirmation_text so I was going to get rid of it or wrap it in a conditional (I didn't see it being used anywhere else but don't have the historical context).

@wes-otf wes-otf changed the title KeyError: 'password' when trying to diable another user's 2FA KeyError: 'password' when trying to diable another user's 2FA Jan 2, 2024
@frjo
Copy link
Contributor

frjo commented Jan 3, 2024

Is this a new or old bug? I know OTF staff have used this function successfully in the past.

They might all have been logged in via Google so no 2FA however.

@wes-otf
Copy link
Contributor Author

wes-otf commented Jan 3, 2024

I believe this is a new bug as both Slammer & I couldn't disable 2FA on another user's account through Hypha, I had to do it manually in the heroku/python shell. I think this issue potentially came with #3531 as it removed the need for a password field in the 2FA disable prompt, but it's weird that a password field would be in the TWOFAAdminDisableView.

@sandeepsajan0 maybe you might have better insight as I think you did the initial implementation, is form.fields["password"].label = "Password" something that can be removed at this point?

@theskumar
Copy link
Member

I'll try take a stab at it. As I don't believe I tested the wagtail admin disable 2FA flow throughly in the #3531

@theskumar theskumar self-assigned this Jan 4, 2024
theskumar added a commit that referenced this issue Jan 4, 2024
- Use sudo mode instead of asking for password
- prompt confirmation text before final disable action
- Clean/Rename the confirmation form

closes: #3705
@wes-otf wes-otf changed the title KeyError: 'password' when trying to diable another user's 2FA KeyError: 'password' when trying to disable another user's 2FA Jan 4, 2024
@frjo frjo closed this as completed in #3708 Jan 8, 2024
frjo pushed a commit that referenced this issue Jan 8, 2024
- Use sudo mode instead of asking for password
- prompt confirmation text before final disable action
- Clean/Rename the confirmation form

closes: #3705
wes-otf pushed a commit that referenced this issue May 7, 2024
- Use sudo mode instead of asking for password
- prompt confirmation text before final disable action
- Clean/Rename the confirmation form

closes: #3705
wes-otf pushed a commit that referenced this issue May 8, 2024
- Use sudo mode instead of asking for password
- prompt confirmation text before final disable action
- Clean/Rename the confirmation form

closes: #3705
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bugs! Things that are broken :-/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants