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.

Sign up for GitHub

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

Jump to bottom

feat: allow unverified email signins #1301

Merged
merged 13 commits into from
Nov 16, 2023
Merged

feat: allow unverified email signins #1301

merged 13 commits into from
Nov 16, 2023

Conversation

kangmingtay
Copy link
Member

@kangmingtay kangmingtay commented Nov 8, 2023 •
edited
Loading

What kind of change does this PR introduce?

  • If GOTRUE_ALLOW_UNVERIFIED_EMAIL_SIGN_INS is enabled, it will allow a user with an unverified email address to sign in and obtain an access token JWT
  • This is particularly useful for OAuth in cases where the oauth provider doesn't return an email address / the oauth user didn't verify their email address with the OAuth provider.
  • Tests that broke and needed fixing were due to these reasons:
    • RemoveUnconfirmedIdentities was previously buggy and shouldn't be retaining the user metadata of a previously unconfirmed identity
    • GOTRUE_ALLOW_UNVERIFIED_EMAIL_SIGN_INS is enabled by default which caused some tests to return an access token instead of an error for a user with an unverified email

Modifications made to automatic linking algorithm

  • If the candidate identity doesn't have a verified email, the decision should be to create a new account.
    • If the email belongs to a user already, then we opt to create a new user with no email. Previously, we would attempt to create a new user and the db will return an error due to the partial unique constraint on email violation. In order to add an email to the new user, they would have to call update user (PUT /user) to add a new email.

@kangmingtay kangmingtay requested a review from a team as a code owner November 8, 2023 06:26
@kangmingtay kangmingtay marked this pull request as draft November 8, 2023 06:26
@kangmingtay kangmingtay changed the title feat: allow unverified email signins [wip] feat: allow unverified email signins Nov 8, 2023
@kangmingtay kangmingtay force-pushed the km/feat-allow-unverified-email-signins branch from 72765b0 to 659b46f Compare November 13, 2023 22:55
@kangmingtay kangmingtay marked this pull request as ready for review November 14, 2023 16:33
@kangmingtay kangmingtay changed the title [wip] feat: allow unverified email signins feat: allow unverified email signins Nov 14, 2023
hf
hf reviewed Nov 14, 2023
View reviewed changes
internal/api/external.go Outdated Show resolved Hide resolved
hf
hf reviewed Nov 14, 2023
View reviewed changes
internal/conf/configuration.go Outdated Show resolved Hide resolved
hf
hf reviewed Nov 14, 2023
View reviewed changes
internal/models/linking.go Outdated Show resolved Hide resolved
hf
hf reviewed Nov 14, 2023
View reviewed changes
internal/models/linking.go Outdated Show resolved Hide resolved
hf
hf reviewed Nov 14, 2023
View reviewed changes
Copy link
Contributor

@hf hf left a comment

Choose a reason for hiding this comment

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

Code seems mostly OK, I want to have a go at testing it a bit later.

@kangmingtay kangmingtay force-pushed the km/feat-allow-unverified-email-signins branch from d63e10e to ad52a15 Compare November 15, 2023 18:32
@kangmingtay kangmingtay force-pushed the km/feat-allow-unverified-email-signins branch from 8510fd3 to 10edffe Compare November 15, 2023 18:42
@kangmingtay kangmingtay merged commit 94293b7 into master Nov 16, 2023
@kangmingtay kangmingtay deleted the km/feat-allow-unverified-email-signins branch November 16, 2023 00:21
@github-actions GitHub Actions
Copy link
Contributor

🎉 This PR is included in version 2.114.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
@kangmingtay
feat: allow unverified email signins (supabase#1301)
eaae7a7 
## What kind of change does this PR introduce?
* If `GOTRUE_ALLOW_UNVERIFIED_EMAIL_SIGN_INS` is enabled, it will allow
a user with an unverified email address to sign in and obtain an access
token JWT
* This is particularly useful for OAuth in cases where the oauth
provider doesn't return an email address / the oauth user didn't verify
their email address with the OAuth provider.
* Tests that broke and needed fixing were due to these reasons:
* `RemoveUnconfirmedIdentities` was previously buggy and shouldn't be
retaining the user metadata of a previously unconfirmed identity
* `GOTRUE_ALLOW_UNVERIFIED_EMAIL_SIGN_INS` is enabled by default which
caused some tests to return an access token instead of an error for a
user with an unverified email

## Modifications made to automatic linking algorithm
* If the candidate identity doesn't have a verified email, the decision
should be to create a new account.
* If the email belongs to a user already, then we opt to create a new
user with no email. Previously, we would attempt to create a new user
and the db will return an error due to the partial unique constraint on
email violation. In order to add an email to the new user, they would
have to call update user (`PUT /user`) to add a new email.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
@kangmingtay @LashaJini
feat: allow unverified email signins (supabase#1301)
a1382db 
## What kind of change does this PR introduce?
* If `GOTRUE_ALLOW_UNVERIFIED_EMAIL_SIGN_INS` is enabled, it will allow
a user with an unverified email address to sign in and obtain an access
token JWT
* This is particularly useful for OAuth in cases where the oauth
provider doesn't return an email address / the oauth user didn't verify
their email address with the OAuth provider.
* Tests that broke and needed fixing were due to these reasons:
* `RemoveUnconfirmedIdentities` was previously buggy and shouldn't be
retaining the user metadata of a previously unconfirmed identity
* `GOTRUE_ALLOW_UNVERIFIED_EMAIL_SIGN_INS` is enabled by default which
caused some tests to return an access token instead of an error for a
user with an unverified email

## Modifications made to automatic linking algorithm
* If the candidate identity doesn't have a verified email, the decision
should be to create a new account.
* If the email belongs to a user already, then we opt to create a new
user with no email. Previously, we would attempt to create a new user
and the db will return an error due to the partial unique constraint on
email violation. In order to add an email to the new user, they would
have to call update user (`PUT /user`) to add a new email.
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
@kangmingtay @LashaJini
feat: allow unverified email signins (supabase#1301)
02cf38f 
## What kind of change does this PR introduce?
* If `GOTRUE_ALLOW_UNVERIFIED_EMAIL_SIGN_INS` is enabled, it will allow
a user with an unverified email address to sign in and obtain an access
token JWT
* This is particularly useful for OAuth in cases where the oauth
provider doesn't return an email address / the oauth user didn't verify
their email address with the OAuth provider.
* Tests that broke and needed fixing were due to these reasons:
* `RemoveUnconfirmedIdentities` was previously buggy and shouldn't be
retaining the user metadata of a previously unconfirmed identity
* `GOTRUE_ALLOW_UNVERIFIED_EMAIL_SIGN_INS` is enabled by default which
caused some tests to return an access token instead of an error for a
user with an unverified email

## Modifications made to automatic linking algorithm
* If the candidate identity doesn't have a verified email, the decision
should be to create a new account.
* If the email belongs to a user already, then we opt to create a new
user with no email. Previously, we would attempt to create a new user
and the db will return an error due to the partial unique constraint on
email violation. In order to add an email to the new user, they would
have to call update user (`PUT /user`) to add a new email.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@J0 J0 J0 left review comments

@hf hf hf approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kangmingtay @hf @J0