Skip to content

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Aug 20, 2025

This adds support for time based one time passwords (TOTP) to LORIS based on RFC6238 (the standard used by authenticator apps such as Authy or Microsoft Authenticator).

There is a new subpage of my_preferences where a user can register an authenticator app to their account using a QR code. After validating the code, the secret key used to generate it is saved to the users table. After this point, any attempts to log in will check if they have a valid 2FA code by using a new MFA middleware. The middleware intercepts requests and prompts for the code if it has not yet been provided.

Testing instructions:

  1. Using the same authenticator app that you use for other sites, set up MFA for your account using the "Configure MFA" link on my_preferences.
  2. Log out
  3. Attempt to log in

@driusan driusan added Critical to release PR or issue is key for the release to which it has been assigned Priority: High PR or issue should be prioritised over others for review and testing Language: PHP PR or issue that update PHP code Language: Javascript PR or issue that update Javascript code Project: C-BIG Issue or PR related to the C-BIG project Difficulty: Complex PR or issue that require a great effort to implementat, review, or test labels Aug 20, 2025
@github-actions github-actions bot added Module: api PR or issue related to api module Module: login PR or issue related to login module Module: my_preferences PR or issue related to my_preferences module labels Aug 20, 2025
@driusan driusan force-pushed the AddMFASupport branch 3 times, most recently from f626d63 to fdc812b Compare August 21, 2025 13:29
@github-actions github-actions bot added the Language: SQL PR or issue that update SQL code label Aug 21, 2025
@driusan driusan force-pushed the AddMFASupport branch 2 times, most recently from 63d07b4 to a40b8a8 Compare August 21, 2025 15:51
@driusan driusan marked this pull request as draft August 21, 2025 16:44
@driusan driusan marked this pull request as ready for review August 25, 2025 13:41
@driusan
Copy link
Collaborator Author

driusan commented Aug 26, 2025

@kongtiaowang is putting this on a VM for @charliehenrib to test.

@charliehenrib
Copy link
Contributor

@driusan It is working well following your testing steps above.
FYI, I used Microsoft authenticator.

@charliehenrib
Copy link
Contributor

charliehenrib commented Aug 26, 2025

@driusan I just noticed that the breadcrumbs for my preferences actually brings you back to the home (dashboard) page. I think this should be fixed.
image

Also, why do the breadcrumbs say "Configure 2FA" rather than keeping the same language as the previous page "Configure MFA"

@charliehenrib
Copy link
Contributor

charliehenrib commented Aug 26, 2025

Not a blocker for this PR, but something to consider, do we want to spell out MFA to improve user experience:
image

Same here:
image

@charliehenrib
Copy link
Contributor

charliehenrib commented Aug 26, 2025

I get the following error when adding manually...
image
On the app, it says my account was added successfully. I will try again with a new LORIS account to see if it works. --> Confirmed that manual setup is not working

Copy link
Contributor

@jeffersoncasimir jeffersoncasimir left a comment

Choose a reason for hiding this comment

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

Very cool feature!

@driusan
Copy link
Collaborator Author

driusan commented Aug 27, 2025

@charliehenrib I fixed the breadcrumbs and manual code input. I also changed the link to explicitly spell out "multi-factor authentication" but left the acronym on the page since you have to click on the link to get there and expanding it in some places would sound weird (ie. "multi-factor authentication authenticator")

@driusan
Copy link
Collaborator Author

driusan commented Aug 27, 2025

@jeffersoncasimir I have committed your suggested changes (and fixed a bug where they caused the automatic digit progression to stop working..)

Copy link
Contributor

@jeffersoncasimir jeffersoncasimir left a comment

Choose a reason for hiding this comment

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

LGTM!

@charliehenrib
Copy link
Contributor

LGTM for manual testing. The only thing that you could add, but is not essential for this PR is to update the test plan for my_preferences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical to release PR or issue is key for the release to which it has been assigned Difficulty: Complex PR or issue that require a great effort to implementat, review, or test Language: Javascript PR or issue that update Javascript code Language: PHP PR or issue that update PHP code Language: SQL PR or issue that update SQL code Module: api PR or issue related to api module Module: login PR or issue related to login module Module: my_preferences PR or issue related to my_preferences module Priority: High PR or issue should be prioritised over others for review and testing Project: C-BIG Issue or PR related to the C-BIG project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants