TheOdinProject / theodinproject

Main Website for The Odin Project
http://www.theodinproject.com
MIT License
3.72k stars 2.07k forks source link

Feature: Admin V2 - Two factor authentication #4631

Closed KevinMulhern closed 2 months ago

KevinMulhern commented 2 months ago

Because

This commit:

KevinMulhern commented 2 months ago

Note, this can only be tested locally, we can't send emails from review apps.

Steps for QA:

Setting up 2fa

  1. Visit /admin_v2 and sign in with - admin@odin.com, password123
  2. Invite a new team member
  3. Within the invitation email, open the "Accept invitation" link in a new incognito window
  4. Fill in the new password and submit
  5. You should be redirected to a two factor authentication set up screen
  6. Try clicking one of the links on the sidebar - a notice should appear asking you to set up 2fa first
  7. Scan the QR code with your favourite authenticator app ( 1Password, Google Authenticator or Authy)
  8. Confirm by filling in the input with the one time code and clicking "Confirm"
  9. You should be redirected to the admin v2 dashboard

Using 2fa

  1. Sign out of the account you just invited.
  2. Sign into it again
  3. You should be prompted to enter an authentication code in a second step screen
  4. Use your authenticator to get the code and sign in
  5. You should be redirected to the admin dashboard
Asartea commented 2 months ago

QA:

Setting up 2fa

  1. Visit /admin_v2 and sign in with - admin@odin.com, password123 ✅ [1]
  2. Invite a new team member ✅
  3. Within the invitation email, open the "Accept invitation" link in a new incognito window ✅
  4. Fill in the new password and submit ✅
  5. You should be redirected to a two factor authentication set up screen image
  6. Try clicking one of the links on the sidebar - a notice should appear asking you to set up 2fa first ✅
  7. Scan the QR code with your favourite authenticator app ( 1Password, Google Authenticator or Authy) ✅
  8. Confirm by filling in the input with the one time code and clicking "Confirm" ✅
  9. You should be redirected to the admin v2 dashboard ✅

Using 2fa

  1. Sign out of the account you just invited. ✅
  2. Sign into it again ✅
  3. You should be prompted to enter an authentication code in a second step screen ✅
  4. Use your authenticator to get the code and sign in ✅
  5. You should be redirected to the admin dashboard ✅

Resetting 2FA

Additional comments

Left some feedback above, otherwise LGTM

KevinMulhern commented 2 months ago

Thanks for the feedback @Asartea, really good suggestions.

Currently, I can reset the 2FA of any number of admin accounts without relogging. Both resetting 2FA and other sensitive actions (anything that modified the state of another admin account: currently this also includes (re/de)activating an account) should IMO be locked behind a full relogin for the admin account, to ensure this isn't a session left open somewhere

Just to clarify this one, do you mean the admin account that is performing these actions should require a login challenge before performing them? - or the other admin account that the actions are being performed against should be logged out?

Asartea commented 2 months ago

Just to clarify this one, do you mean the admin account that is performing these actions should require a login challenge before performing them

This one

KevinMulhern commented 2 months ago

I can see what you mean. I'll have a think about password challenges. But we have a fairly aggressive 30 minute session timeout that should mitigate most issues with sessions being left open.

KevinMulhern commented 2 months ago

Got these done, thanks again for the feedback @Asartea!

While trying to test this locally I ran into trouble with not having key_derivation_salt and primary_key set. There should either be a step in the installation guide to add these, or they should just get set in developement.rb, given that leaking development keys isn't dangerous as there is no private info stored in them

Is it possible to make the default "account name" for 2FA slightly shorter? With Authy I ended up with a monstrosity of The Odin Project: The Odin Project_admin@odin.com, which is long enough that in the grid mode I can't see which account is which

The "please enable two-factor authentication" alert is currently green: is it possible to change this to red to accurately represent the fact its an error, not a confirmation of success?