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: Add OTP for Email in Auth #2651

Open
osseonews opened this issue Apr 9, 2024 · 14 comments
Open

Feature Request: Add OTP for Email in Auth #2651

osseonews opened this issue Apr 9, 2024 · 14 comments
Labels
enhancement New feature or request

Comments

@osseonews
Copy link

Is your feature request related to a problem? Please describe.
Currently, for Auth you only allow an OTP code (6 digit number) to be sent by SMS. However, there is no way to send this code to a user's email to facilitate a login. This is not ideal because, for various reasons, virtually all auth systems now, will also allow for OTP verification and login with a code that can also be sent to an email. In fact, generally the user is given a choice to send the OTP to email or phone.

Describe the solution you'd like
While it's not necessary to offer a choice per above, there should at least be an option to send an OTP code to an email. I imagine the code is exactly the same as is used for an SMS. It's just that the code is sent by email and not SMS. So I don't see that this will entail alot of changes, though admittingly I have not looked at Auth code at all in NHost.

Describe alternatives you've considered
Nhost comes with a Magic Link, but magic links are not a great solution particularly given the reluctance nowadays to not want to click on any link. Just sending the Magic Link code in the email, instead of the link is a terrible UI, because the code is unintelligible to the user and so virtually impossible to remember. This means that you would have to cut/paste the code into the website, which is a bad UI and also in certain instances not possible. One of the benefits of OTP codes, in general, is precisely that they obviate these UI issues, as most people can easily remember a 6 digit number and just type it in immediately.

Additional context
OTP codes are commonplace now in virtually every single Auth application nowadays (see Shopify's new Customer Accounts API, which just sends a code). At the same time, Magic Links actually have never really taken off, and I doubt they ever will, given concerns of clicking on links. Offering the OTP code to an email also, in addition to the SMS, will bring the Auth library up to speed with other providers.

@osseonews osseonews added the enhancement New feature or request label Apr 9, 2024
@dbarrosop
Copy link
Contributor

Thanks for the detailed feature request, we will look into it when planning future work.

@osseonews
Copy link
Author

Curious if you guys plan on implementing this? Honestly, in our project, we are just going to have to reluctantly use Clerk for Auth, instead of the built in Nhost Auth because there is no way we want to tether our application to a bunch of links and redirects. There are so many problems with links nowadays (this is why you even wrote this: https://nhost.io/blog/protect-magic-links-from-email-clients), it's just a an antiquated solution and I really don't see any Auth library using them anymore as a first choice. Everyone sends a 6 digit code by email, to verify email and then another code for changing password. We are tempted to just follow this example from Lucia Auth (https://lucia-auth.com/guides/email-and-password/email-verification-codes) and hook it into Nhost Auth, but honestly we don't know enough about the inner workings of your code to feel confident this can work, so seems like a waste effort. But, I'm sure someone who is more familiar with your codebase can easily implement the type of solution Lucia recommends. All the code is on that page, it's just a matter of putting the functions into your Auth package. Thanks for listening.

@dbarrosop
Copy link
Contributor

Curious if you guys plan on implementing this?

Yes, this is something we will certainly want to add.

there is no way we want to tether our application to a bunch of links and redirects

Remember you can already use codes (even though longer). I know you prefer 6-digits codes but as a temporary solution it might be easier than using clerk as migrating from clerk might be difficult.

In any case, if what you want is security and convenience nothing beats weabuthn/keypass so you might want to look into that instead.

@osseonews
Copy link
Author

"Remember you can already use codes (even though longer)". what do you mean by this? Thanks.

@dbarrosop
Copy link
Contributor

This:

#2642 (comment)

So basically what you are describing can already be implemented by tweaking the email template. The difference between your proposal and what we are providing already is that you want a 6 digits code (or similar) and what we offer is a more difficult to guess uuid.

@osseonews
Copy link
Author

osseonews commented May 21, 2024

Oh yeah, OK. Would never send UUID as a code to type in, impossible for most people to type in correctly. Better off with link then, which is probably why links were used in the first place with this auth flow, because asking someone to type in a UUID would be a total disaster. Can't imagine how many calls we would get, for basic typos.

@dbarrosop
Copy link
Contributor

Yes, the expectation in this case would be to copy & paste.

@osseonews
Copy link
Author

So we've done a ton of testing on various email clients, and firewall/anti-virus and have consistently run into problems with email clients invalidating links, even when you use your own app to do a redirect. Ultimately, it requires constant tweaking of code to battle these clients and provide a good UX.

My suggestion, which would be simple and completely backwards compatible, is to simply replace the uuidv4() which you use everywhere for generating the ticket code to a different code which just generates a random 6 digit number. See the example below. By doing it this way, nothing will break for anyone still using links because there is still a code. At the same time, anyone who wants to move away from links, and provide an OTP-like form can easily do so by grabbing the digits and creating the OTP form. Incidentally, if you do it this way, you are in good company, as this is exactly how Cloudflare Pages now does it's Access for dev pages. They generate a link with a 6-digit code as the code, so you can use the link (presumably for backwards compatibility), but actually their frontend now has a form, where you type in the 6-digit code.

Example

https://github.com/nhost/hasura-auth/blob/main/src/utils/user/email-verification.ts
line 117
const ticket = `verifyEmail:${uuidv4()}`;

Change this to:

import { generateRandomString, alphabet } from "oslo/crypto"; //obviously you don't have to use this library and you can just create your own way to generate random digits 
const code = generateRandomString(6, alphabet("0-9"));
const ticket = `verifyEmail:${code}`;

There are a few places where you'd have to implement this fix, https://github.com/nhost/hasura-auth/blob/155357345a10b785c69e19cb5522818ed6cd56c5/src/routes/user/email/send-verification-email.ts
line 51 and https://github.com/nhost/hasura-auth/blob/155357345a10b785c69e19cb5522818ed6cd56c5/src/routes/user/password-reset.ts#L44
Line 44

@dbarrosop
Copy link
Contributor

Hi,
thanks for the suggestion. Unfortunately the solution isn't as simple. Two reasons:

  1. Users may want to retain the more secure option of using a uuid, so this should be configurable at the very least, most likely it will need to be a new endpoint due to the next point.
  2. We need to asses its security considerations/impact as we are vastly reducing the robustness against brute-force attacks/collissions. For instance, at the very least it'd require a new parameter to the verify endpoint to request the email, which we don't today and would be a breaking change.

Regards

@osseonews
Copy link
Author

Thanks for the quick reply. My counterpoints are below.

  1. A properly generated random 6 digit number is no less secure than a UUID for the purposes of email verification. Of course, in other use cases a UUID is more secure, but not for this use case. If it was how is it that every single auth company uses this nowadays for verification instead of UUID's, including companies like Cloudflare who entire business revolves around security. The new ID.me from the US government for verifying identity, probably the most targeted service in the world, is also entirely based around digits vs UUIDs, though they require 8 digit code. You also use OTP for SMS. Fact is that if you use a good crypto library and hash the code in the database (that's what the oslo package does, by the way), have an expiration, there is no security difference whatsoever between a UUID and 6 digit code for this particular use case. I don't see why anyone would ever prefer a UUID over a 6-digit (or 8 digit) code.

  2. There is zero security impact using a random 6 or 8 digit code vs UUID if done properly, and it in no way reduces the robustness of brute force. I don't even understand this point actually, as the protection against brute force attacks would follow the same exact pattern as you currently have. Is your OTP with SMS subject vastly reducing the robustness against brute force and undermining your security? If you can send OTP via SMS, you can send it via email. There is no difference. Probably you can just copy your code for OTP with SMS and instead of sending via SMS, send via email and it would be fine. But I haven't looked at your OTP for SMS code yet, to be sure this is possible.

  3. At the end of the day, using links with UUID is a thing of the past and any auth system that uses it will be abandoned in due time (I give it about a year). Everybody is moving away from it. Users are increasingly less likely to click on links for fear of phishing attacks, and increasingly filters block emails with them or send them to spam. Just yesterday, I had 2 phishing attacks myself with fake magic links from Asana.

Given #3 above, it is really crucial for Nhost Auth to make the change and move away from clicking links. If it means, you need to create another enpoint, of course, that's a perfectly viable solution, but I think the implementation is much easier than that, especially since you already have OTP with SMS.

@dbarrosop
Copy link
Contributor

if done properly

That's the thing, there are a lot of considerations so it isn't as simple as just replacing a uuid with a 6 digit number. Nobody is arguing against the proposal, just saying it isn't as simple as replacing one type with another

@osseonews
Copy link
Author

OK, thanks. BTW, I did look at the code quickly. You have a function getNewOneTimePasswordData in otp.ts that generates the OTP. Upon quick glance it seems perfectly fine and follows all the best practices. You then use it in sms.ts to send the sms. Why couldn't you just duplicate the sms.ts file and instead of twilioClient.messages.create, just have a postmark message sent or some other email provider? How is sending this by SMS any different than email? it's just a different API for sending, but the underlying OTP functionality is the same.

@dbarrosop
Copy link
Contributor

From my previous message:

For instance, at the very least it'd require a new parameter to the verify endpoint to request the email, which we don't today and would be a breaking change

We basically need a new endpoint as this will need to work more like the otp endpoint and less like the verify ticket endpoint

@osseonews
Copy link
Author

Thanks. Yeah, after reviewing the code a bit more, I see that now that new endpoint is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants