-
Notifications
You must be signed in to change notification settings - Fork 546
fix: shorten email otp #446
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good just a couple of questions
crypto/crypto.go
Outdated
|
||
// GenerateEmailOtp generates a random n-length alphanumeric otp | ||
func GenerateEmailOtp(length int) (string, error) { | ||
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok but are there any situations where upper case chars could get squashed? (I can't think of any)
block on this for the time being because we need to run the migration to create the new indices concurrently which isn't supported by the current migration tool (soda) will be swapping that out for sql-migrate |
A typical user journey is to look at the OTP in phone while doing it in web/iPad. This is quite common to not context switch, as people are often comfortable checking email (like SMS) in phone. On that note, can we have a consistent experience, where we do not assume that it will be copy pasted? Most providers do lowercase alphabets only, for example. Making them a multiple of 4 is also common for the same reason, it makes it possible to remember chunks and type it out. |
🎉 This PR is included in version 2.6.22 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
* fix: add unique indices to user token columns * fix: shorten email otp * remove phone_change_token from migration * fix: use enums for each token type * refactor: use RawURLEncoding instead of removePadding * fix: update token format & verify * add description for v1 otp
* fix: add unique indices to user token columns * fix: shorten email otp * remove phone_change_token from migration * fix: use enums for each token type * refactor: use RawURLEncoding instead of removePadding * fix: update token format & verify * add description for v1 otp
* fix: add unique indices to user token columns * fix: shorten email otp * remove phone_change_token from migration * fix: use enums for each token type * refactor: use RawURLEncoding instead of removePadding * fix: update token format & verify * add description for v1 otp
* fix: add unique indices to user token columns * fix: shorten email otp * remove phone_change_token from migration * fix: use enums for each token type * refactor: use RawURLEncoding instead of removePadding * fix: update token format & verify * add description for v1 otp
* fix: add unique indices to user token columns * fix: shorten email otp * remove phone_change_token from migration * fix: use enums for each token type * refactor: use RawURLEncoding instead of removePadding * fix: update token format & verify * add description for v1 otp
* fix: add unique indices to user token columns * fix: shorten email otp * remove phone_change_token from migration * fix: use enums for each token type * refactor: use RawURLEncoding instead of removePadding * fix: update token format & verify * add description for v1 otp
What kind of change does this PR introduce?
confirmation_token
,recovery_token
,email_change_token_new
,email_change_token_current
,reauthentication_token