Skip to content

Login as {some member} is impossible if this member has MFA enabled #3020

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

Closed
shbchk opened this issue Feb 16, 2023 · 6 comments · Fixed by #3301
Closed

Login as {some member} is impossible if this member has MFA enabled #3020

shbchk opened this issue Feb 16, 2023 · 6 comments · Fixed by #3301
Labels
Bug: Accepted Bug has been confirmed, is reproducible, and ready to work on. member-functionality

Comments

@shbchk
Copy link

shbchk commented Feb 16, 2023

Super admin feature Login as {some member} is not working when the target member has enabled MFA: EE asks for the MFA code.

@intoeetive
Copy link
Contributor

I can confirm this behavior, but also IDK if we should consider this as bug or limitation

I think that maybe if MFA is turned on for user, we need to show message to "login as..." page and disable that functionality.
Or, if we decide MFA to be skipped for this case, we need to at least require superadmin to be using MFA on their account, so that we wouldn't make it less secure

@TomJaeger at your discretion here

@intoeetive intoeetive added Bug: Accepted Bug has been confirmed, is reproducible, and ready to work on. member-functionality labels Mar 2, 2023
@shbchk
Copy link
Author

shbchk commented Mar 2, 2023

If I understand this right, the only way to restore an account for a user who lost his MFA key is "Login as ...". Or Superadmin can go to the database and change some values there, which is the same thing, but more complicated.

@TomJaeger
Copy link
Contributor

Okay, so after thinking on this one and getting some input from @matthewjohns0n, the way it is right now isn't a good solution.

I think we need to go the route of if a SA (Super Admin) wants to log in as a user that has MFA enabled, the SA also has to have MFA enabled and is required to use their (the SA) MFA. This would prevent a leaked SA account from being able to access other users' accounts that have MFA enabled.

@intoeetive
Copy link
Contributor

I don't think we can "fake" the user's MFA code with the SuperAdmin's MFA code. But assuming the SuperAdmin has already authorized themself with MFA when logging in to Control Panel, we should be ok with letting them "login as..." using password for confirmation - as MFA code has already been checked

@matthewjohns0n
Copy link
Member

It might be good to request the SuperAdmin's MFA code again at the time of logging in as another member. This ensures that the SA that is currently logged in is re-authenticated as the correct user, for the same reasons that it prompts you for the SuperAdmin password when performing "Login-as".

intoeetive added a commit that referenced this issue Apr 19, 2023
…d MFA enabled

If the member has MFA enabled, it only allows "login as" functionality if the superadmin also has MFA enabled. In that case, it's skipping the MFA check for the user (since superadmin is already MFA-authorized)
@bryannielsen
Copy link
Contributor

I think this is working as expected but just want to note that doing the Login As member prompts you for password and not the MFA code again like @matthewjohns0n suggested above. Do we want to change that?

I realize what @intoeetive is saying, that if the SuperAdmin is logged in they would have used an MFA code already, or if they just enabled MFA to access this account they had to verify the code. So maybe using the password is fine here?

matthewjohns0n added a commit that referenced this issue Jun 16, 2023
Resolved #3020 where "login as" functionality was broken if member had MFA enabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Accepted Bug has been confirmed, is reproducible, and ready to work on. member-functionality
Projects
None yet
5 participants