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

Should the admin create user handler of Auth create identities based on the provider under the hood? #1577

Closed
2 tasks done
rexwangcc opened this issue May 8, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@rexwangcc
Copy link

rexwangcc commented May 8, 2024

Bug report

  • (actually, not really) I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

POST /admin/users does not really respect provider fields when they are not email.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Call POST /admin/users endpoint with any HTTP requests or SDK, such as in Python:
from supabase import create_client

supabase = create_client(SUPABASE_URL, SUPABASE_SERVICE_KEY)

payload = {
    "email": "foo@gmail.com",
    "email_confirm": True,
    "app_metadata": {"provider": "google", "providers": ["google"], "my_custom_field": "baz"},
    "user_metadata": {},
}

supabase.auth.admin.create_user(payload)

the key is to use a provider other than the default email.

  1. Navigate either to Auth UI of supabase or to the underlying PG instance, a correct row is inserted into auth.users, the field raw_app_meta_data looks like:
{
  "provider": "google",
  "providers": [
    "google"
  ],
  "my_custom_field": "baz"
}
  1. However, one somewhat weird row is created in auth.identities looking like:
{
  "sub": "483fb8f4-6b12-4c20-af6e-797b6ec1bcd4",
  "email": "foo@gmail.com",
  "email_verified": false,
  "phone_verified": false
}
  1. If you login with Google using that foo@gmail.com now, the aforementioned "normal auth.users` record will get overwritten into:
{
  "provider": "email",
  "providers": [
    "email"
  ],
  "my_custom_field": "baz"
}

which is defo not what we want?

  1. But if you then login with Email (OTP in our context), the field raw_app_meta_data gets to normal with the right providers in it, except that the default provider now sticks in email:
{
  "provider": "email",
  "providers": [
    "email",
    "google"
  ],
  "my_custom_field": "baz"
}

while the identities table now have one extra record with provoder=google, still, the other one is left untouched (even the last_sign_in_at didn't change after a successful email-OTP login)

Expected behavior

I would expect the identities table has a row with provider=google inserted if the POST /admin/users got provider payload that is not a email provider. After some digging, this is not feasible with the current implementation that harding the provider in the transaction:

auth/internal/api/admin.go

Lines 361 to 377 in ed2b490

err = db.Transaction(func(tx *storage.Connection) error {
if terr := tx.Create(user); terr != nil {
return terr
}
var identities []models.Identity
if user.GetEmail() != "" {
identity, terr := a.createNewIdentity(tx, user, "email", structs.Map(provider.Claims{
Subject: user.ID.String(),
Email: user.GetEmail(),
}))
if terr != nil {
return terr
}
identities = append(identities, *identity)
}
.

My question now becomes:

  1. Is this by design? I.e. Supabase Auth always tends to create a email identity no matter what provider is specified when creating a user via admin endpoints.
  2. If this is by design, can we assume that we can just let it be and don't have to poke anything with the auth.identities table, and simply let the user's first login updates the DB?
  3. Then what does the provider=email record in auth.identities do at all? I tried deleting it from the underlying DB table and still being able to login with Email OTP.

Thank you so much!

@rexwangcc rexwangcc added the bug Something isn't working label May 8, 2024
@rexwangcc rexwangcc changed the title Should the admin create user func of Auth create identities based on the provider under the hood? Should the admin create user handler of Auth create identities based on the provider under the hood? May 8, 2024
@kangmingtay
Copy link
Member

hi @rexwangcc,

Is this by design? I.e. Supabase Auth always tends to create a email identity no matter what provider is specified when creating a user via admin endpoints.

The admin user create endpoint requires either an email or phone number to be passed in - there is an assumption made here that the user created will have either an email / phone identity, depending on which one was provided initially.

If this is by design, can we assume that we can just let it be and don't have to poke anything with the auth.identities table, and simply let the user's first login updates the DB?

Yes, that's fine because when the user signs in with google, there will be a new google identity created and it will be linked to the user since they share the same email.

Then what does the provider=email record in auth.identities do at all? I tried deleting it from the underlying DB table and still being able to login with Email OTP.

Currently, it doesn't enforce anything but eventually, the goal is to decouple having unique emails for OAuth identities (see #214).

@kangmingtay
Copy link
Member

closing this issue since it's more of a question

@rexwangcc
Copy link
Author

closing this issue since it's more of a question

Thanks for the reply / help! We ended up leaving the "redundant" provider=email record in auth.identities table for now.

@ham-evans
Copy link

@kangmingtay I think being able to specify provider when using supabase.auth.admin.create_user() would be incredibly helpful. Implementing SSO with invites and running into this. Having to modify identities table under the hood to get the expected behavior... Also can only link OAuth identities, not SSO or email to another one causing a lot of these issues.

Happy to discuss further as well but would love to see more attention to these methods

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants