Skip to content

MFA System - #2827

Merged
ssddanbrown merged 25 commits into
masterfrom
mfa
Aug 21, 2021
Merged

ssddanbrown merged 25 commits into
masterfrom
mfa

Conversation

@ssddanbrown

@ssddanbrown ssddanbrown commented Jun 28, 2021

Copy link
Copy Markdown
Member

TODO

  • TOTP Setup System
  • Backup Codes System
  • Role based enforcement
  • MFA setup link/visibility in user edit page
  • MFA indicator in users list
  • Admin clear MFA command - Should accept user id/email as option.
  • Access time MFA check
  • "Disable Method" button in setup
  • MFA setup instructions abstracted to own view for easier customization?
    • Skip this one for now, Better to get feedback as to where needed if there's a use-case.
  • Move new view text to translations.
  • Testing
  • Lib attribution
  • Intended Redirect on Login needs updating, Is leading to MFA setup after login when having to setup MFA for first time.

Notes

  • Encryption is used, will need to start to enforce consistency of the APP_KEY.
  • Likely change to email confirmation system, now validated at login event rather than on every request.
  • Libs

Also updated MFA setup view to have settings-like listed interface to
make it possible to extend with extra options in the future.
- Includes DB storage and code validation.
- Extracted TOTP work to its own service file.
- Still needs testing to cover this side of things.
- Includes testing to cover flow.
- Moved TOTP logic to its own controller.
- Added some extra totp tests.
- Added new DB column for control and role updated create/update actions.
- Created new middleware as a start to actual enforcement logic.
- Added indicator to role list of whether MFA is enforced.
Also fixed issue with showing incorrect MFA method count on user edit
page changes done in last commit
Includes tests to cover the command.
Discovered some difficult edge cases:
- User image loading in header bar when using local_secure storage
- 404s showing user-specific visible content due to content listing on
  404 page since user is in semi-logged in state. Maybe need to go
  through and change up how logins are handled to centralise and
  provide us better control at login time to prevent any auth level.
Instead of being soley middleware based.
Also continued a bit on the MFA verification system.
Moved some MFA routes to public space using updated login service to get
the current user that is either logged in or last attempted login (With
correct creds).
Also added testing to cover as part of this in addition to adding the
core backup code handling required.

Also added the standardised translations for switching mfa mode and
adding testing for this switching.
- Restructured some of the route naming to be a little more consistent.
- Moved the routes about to be more logically in one place.
- Created a new middleware to handle the auth of people that should be
  allowed access to mfa setup routes, since these could be used by
  existing logged in users or by people needing to setup MFA on access.
- Added testing to cover MFA setup required flow.
- Added TTL and method tracking to session last-login tracking system.
Email confirmations are now done within the guard during auth checking
instead of at the middleware layer.
Since sessions are invalidated upon logout.
@ssddanbrown ssddanbrown added this to the Next Feature Release milestone Aug 8, 2021
Also aligned mfa method delete route to align with others.
- Also hard-set TOTP algorithm with comment from testing others.
- Updated auth system for mfa to not update intended URL so that the
  user is not redirected to mfa setup after eventual login.
- Added notification for users setting up MFA, after setup when
  redirected back to login screen to advise that MFA setup was complete
  but they need to login again.
- Updated some bits of wording to display better.
@ssddanbrown
ssddanbrown merged commit cac31b2 into master Aug 21, 2021
@ssddanbrown
ssddanbrown deleted the mfa branch August 21, 2021 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

1 participant