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

Feature Request: 2FA for logins #925

Open
NeilAtw opened this issue Jun 27, 2023 · 14 comments
Open

Feature Request: 2FA for logins #925

NeilAtw opened this issue Jun 27, 2023 · 14 comments

Comments

@NeilAtw
Copy link

NeilAtw commented Jun 27, 2023

I have read the previous thread about this, but wanted to suggest that 2FA using a tool like Google Authenticator seems like a better and more secure solution than SMS or even email.
Thoughts?

@tbar0970
Copy link
Owner

More secure, yes. Less convenient and more work to implement. The usual security tradeoff!

@NeilAtw
Copy link
Author

NeilAtw commented Jun 27, 2023

I certainly get that Tom - but is any 2FA on the roadmap for Jethro?

@jefft
Copy link
Contributor

jefft commented Jun 27, 2023

Previous thread: #858

TOTP (Authy, Google Authenticator) is more secure than SMS, and probably easier to implement, but requires users to install an app.

The new state of the art is WebAuthn / Passkeys, with GMail support announced by google and recently enabled for Google Workspace. The whole idea of logging in with a username and per-app password is becoming obsolete. Hence my 2c on #858, that it would be best to outsource auth completely, to an app like Keycloak that does 2FA, passkeys or whatever the latest hotness is.

@tbar0970
Copy link
Owner

but is any 2FA on the roadmap for Jethro?

It's certainly under discussion, as Jeff notes above!

@NeilAtw, in your context would Google Authenticator be preferable to SMS 2FA? Would syndicated auth from google etc be workable?

@tbar0970
Copy link
Owner

Using Google authenticator and TOPT apps are bit more dev work than SMS 2FA because there needs to be a mechanism for setting up the shared key. Whereas SMS just uses the mobile number which is data Jethro already possesses.

@NeilAtw
Copy link
Author

NeilAtw commented Jun 27, 2023

Thanks Tom (and others). I personally would be happy to see even SMS based 2FA. While I am aware of the potential to hijack SMS contents, in the context that Jethro is used in I think the risk is manageable, and the payoff should be seen as a significant positive.. :)

@tim-pearce
Copy link
Contributor

Ok, I think I am ready to try implementing 2FA using SMS/email/list depending on what the user has available or chooses.

SMS: The usual send OTP to the users mobile number
email: Send it to the users email address
list: For those who don't have SMS or email we print a list of codes and they are asked to enter one of those (eg enter the 85th code on the sheet). Will leave this option to the last.

I'll base the SMS and email functions based on the code in date_reminder.php
Using https://phppot.com/php/how-to-implement-otp-sms-mobile-verification-in-php-with-textlocal/ as a basis for the OTP coding.

Option to be enabled by the presence of constants in config.php. ie if the constants are not there nothing will change in the login process.

@s4069b
Copy link
Contributor

s4069b commented Aug 8, 2023

Nice! Thanks for putting in the work.

@tim-pearce
Copy link
Contributor

Fraid I haven't started this yet. I will ping Tom when I do get to it in case he is working on it.

I am thinking 2FA may not be required for member logins. Just staff logins. That could be parameterised in configurations.

@NeilAtw
Copy link
Author

NeilAtw commented Sep 7, 2023 via email

@tim-pearce
Copy link
Contributor

Neil,
That would be just fine in my proposal. The parameters would be something like 2FA_MEMBERS = true/false 2FA_STAFF = true/false - so entirely up to you.
I might also add a test to see if they are within the office if the site is hosted within the office and turn 2FA off in that case.

@NeilAtw
Copy link
Author

NeilAtw commented Sep 7, 2023 via email

@tbar0970
Copy link
Owner

I've started work on this.

@tim-pearce
Copy link
Contributor

Thanks for that. I shouldn't promise to do things during school terms!
I was going to add some suggestions but I see I already did so on September 7!
School term ends soon. If you want any help or 'crash testing' let me know.

tbar0970 added a commit that referenced this issue Mar 11, 2024
tbar0970 added a commit that referenced this issue Mar 11, 2024
…her 2FA is required, and how the SMS is sent.
tbar0970 added a commit that referenced this issue Mar 11, 2024
…her 2FA is required, and how the SMS is sent.
tbar0970 added a commit that referenced this issue Mar 11, 2024
…ile number is blank and that will block 2FA.
tbar0970 added a commit that referenced this issue Apr 6, 2024
…ion around changing it. Add CSRF token when editing person details, and if 2FA is relevant, send a notification of the change to the old number.
tbar0970 added a commit that referenced this issue Apr 9, 2024
…ing changed via the members interface. Some refactoring to suit.
tbar0970 added a commit that referenced this issue May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants