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

Implement 2FA During Login #1919

Open
bennpham opened this issue Oct 19, 2020 · 16 comments
Open

Implement 2FA During Login #1919

bennpham opened this issue Oct 19, 2020 · 16 comments

Comments

@bennpham
Copy link
Member

Description

I think it'd be nice to have 2FA after logging in on https://www.if-me.org/users/sign_in. Maybe we can have a modal popup for users to input that 2FA security code when they login normally via email or password.

This should not applied to sign in with google or facebook since those sites should have their own 2FA implementation if enabled.

A few gems we could check out that seems the most up to date for 2FA would be https://github.com/Houdini/two_factor_authentication or https://github.com/tinfoil/devise-two-factor so it'd be best to evaluate what would be the best 2FA gem to use before implementing.

Note

There will be new gems to implement with this PR so it is best to pick the best suited most up to date 2FA gem.


Please assign yourself (via the Assignees dropdown), if you do want to work on this issue. Can't find yourself? You need to join our organization.

Check out our Picking Up Issues guide if you haven't already!

@bennpham
Copy link
Member Author

bennpham commented Nov 30, 2020

Peeking through Tinfoil's Devise Two Factor, I think that gem's is kind of dying and deprecated. I remember attempting to implement this as a Spike story at work and ran into problems, so I think Tinfoil's Devise Two Factor might be a bit out of the question :/ devise-two-factor/devise-two-factor#170

I had my eyes on Houdini's two factor authentication which is related to Tinfoil's but more maintained at time. Activity been a bit low on there as well: Houdini/two_factor_authentication#193. Maybe someone might step in to take ownership of the project.


Two factor wise, I've yet to find a reliable 2FA package right off the box for Rails. I've found something working with Laravel though. Luckily I did ran into a blog that implemented 2FA on rails app written 5 months ago: https://oozou.com/blog/otp-2fa-in-ruby-on-rails-with-rotp-42

RTOP gem seems like the simplest to use gem although I suppose I'm curious how to get it with Devise (which Houdini's and Tinfoil's 2FA were made to be implemented with Devise).

Do note to encrypt otp_secret when going with this implementation mentioned in additional notes. Also to make sure that users aren't logged in and bypass the OTP part. I think some extra work in the Session Controller would need to be done on this part. I might grab this and give it a go if I get around to it and am not busy unless someone wants it first.


I think this list would be suitable for OTP:

  • Encrypt otp_secret
  • Make sure that users don't get logged into until they actually enter the correct OTP
  • Adds ability to enable OTP in user settings
  • Show OTP after/during login when OTP is enabled for users who signed up via email

@sebassebas1313
Copy link
Contributor

I would like to take this one. It's seems a nice way to start my contribution.

@bennpham
Copy link
Member Author

bennpham commented Dec 9, 2020

@sebassebas1313 Go ahead and assign yourself if no one is assigned to it here! Also you can join the slack channel for IFME.

@sebassebas1313 sebassebas1313 self-assigned this Dec 9, 2020
@sebassebas1313
Copy link
Contributor

Sadly, This issue goes beyond the scope I was thinking. So, I am going to leave open to someone with more time. Thank you for your help anyways @julianguyen

@sebassebas1313 sebassebas1313 removed their assignment Jan 1, 2021
@faithngetich faithngetich self-assigned this Jan 27, 2021
@akp2603
Copy link
Contributor

akp2603 commented May 3, 2021

Hi @faithngetich! Did you pick this up?

@faithngetich
Copy link

No @akp2603 I'll remove myself from the issue.

@faithngetich faithngetich removed their assignment May 3, 2021
@adang48
Copy link
Contributor

adang48 commented Jul 16, 2021

Could I give this a try? @julianguyen

@julianguyen
Copy link
Member

Yeah of course that would be great, thanks @adang48 !

@adang48 adang48 self-assigned this Jul 16, 2021
@MuraraAllan
Copy link

Can I help with this ticket?

@adang48 adang48 removed their assignment Dec 22, 2021
@MuraraAllan MuraraAllan self-assigned this Dec 23, 2021
@julianguyen
Copy link
Member

Yes that would be great, thanks @MuraraAllan! Let us know if you have any questions!

@MuraraAllan
Copy link

@bennpham @julianguyen hey \o

Some commits were made into Tinfoil's, should we reconsider it?

I'm also coming forward with my planning, OTP_SMS and OTP_QRCODE ( String )

proposalç (3)

I'm considering adding a custom strategy to Devise::Warden, or a Custom Controller, implementation details are further provided.

Should implement OTP_SMS? Any suggestions about providers?

Which way sounds more scalable for you ?
Which way would you choose here?

Thanks,
Allan

@julianguyen
Copy link
Member

@MuraraAllan Hey Allan! I think it would be worth looking into Tinfoil again before setting up a custom implementation! Hmm I think we should hold off implementing OTP_SMS because we don't have the financial means to pay for SMS services. Hope that helps! Great questions and thanks sketching things out!

@bennpham
Copy link
Member Author

@MuraraAllan @julianguyen Agreed on that. Sorry for late response, been on a big project and had a lot of plans recently so been out the loop. I think I looked into Tinfoil before way back but that was while it was still kind of dead-ish, but looks like activity picked up in 2021 👍

@MuraraAllan
Copy link

MuraraAllan commented Feb 2, 2022

@bennpham nice to meet you \o @julianguyen both thanks for answers

People, sorry my long time no see :)

ETA of POC : feb 11

@sfayyad sfayyad self-assigned this Nov 10, 2022
@shivansh84ya
Copy link

I would like to work on this issue. Could you please assign it to me?
Thanks!

@MuraraAllan
Copy link

MuraraAllan commented Jul 17, 2024

can we have @shivansh84ya assigned ? @julianguyen @bennpham

before opening a new request, could we also change the calendar permission to when a user actually wants to use the calendar / remember feature?

Today we ask for calendar ( google oauth ) right away, it seems a bit invasive for asking right away access to calendars.
Maybe shivansh also wants to work on that :P

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

9 participants