From f5c6fcd9c3fee0f793f96880a8caebc5b5cb0916 Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Mon, 17 Jun 2024 17:01:15 +0800 Subject: [PATCH] fix: admin user update should update is_anonymous field (#1623) ## What kind of change does this PR introduce? * Fixes #1578 --- internal/api/admin.go | 29 +++++++++--- internal/api/anonymous_test.go | 84 ++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 6 deletions(-) diff --git a/internal/api/admin.go b/internal/api/admin.go index 053a75d35..1cda8e264 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -214,8 +214,9 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { // if the user doesn't have an existing email // then updating the user's email should create a new email identity i, terr := a.createNewIdentity(tx, user, "email", structs.Map(provider.Claims{ - Subject: user.ID.String(), - Email: params.Email, + Subject: user.ID.String(), + Email: params.Email, + EmailVerified: params.EmailConfirm, })) if terr != nil { return terr @@ -224,11 +225,19 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { } else { // update the existing email identity if terr := identity.UpdateIdentityData(tx, map[string]interface{}{ - "email": params.Email, + "email": params.Email, + "email_verified": params.EmailConfirm, }); terr != nil { return terr } } + if user.IsAnonymous && params.EmailConfirm { + user.IsAnonymous = false + if terr := tx.UpdateOnly(user, "is_anonymous"); terr != nil { + return terr + } + } + if terr := user.SetEmail(tx, params.Email); terr != nil { return terr } @@ -241,8 +250,9 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { // if the user doesn't have an existing phone // then updating the user's phone should create a new phone identity identity, terr := a.createNewIdentity(tx, user, "phone", structs.Map(provider.Claims{ - Subject: user.ID.String(), - Phone: params.Phone, + Subject: user.ID.String(), + Phone: params.Phone, + PhoneVerified: params.PhoneConfirm, })) if terr != nil { return terr @@ -251,11 +261,18 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { } else { // update the existing phone identity if terr := identity.UpdateIdentityData(tx, map[string]interface{}{ - "phone": params.Phone, + "phone": params.Phone, + "phone_verified": params.PhoneConfirm, }); terr != nil { return terr } } + if user.IsAnonymous && params.PhoneConfirm { + user.IsAnonymous = false + if terr := tx.UpdateOnly(user, "is_anonymous"); terr != nil { + return terr + } + } if terr := user.SetPhone(tx, params.Phone); terr != nil { return terr } diff --git a/internal/api/anonymous_test.go b/internal/api/anonymous_test.go index fdee4cc07..92877cf19 100644 --- a/internal/api/anonymous_test.go +++ b/internal/api/anonymous_test.go @@ -8,6 +8,8 @@ import ( "net/http/httptest" "testing" + "github.com/gofrs/uuid" + jwt "github.com/golang-jwt/jwt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -222,3 +224,85 @@ func (ts *AnonymousTestSuite) TestRateLimitAnonymousSignups() { ts.API.handler.ServeHTTP(w, req) assert.Equal(ts.T(), http.StatusBadRequest, w.Code) } + +func (ts *AnonymousTestSuite) TestAdminUpdateAnonymousUser() { + claims := &AccessTokenClaims{ + Role: "supabase_admin", + } + adminJwt, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString([]byte(ts.Config.JWT.Secret)) + require.NoError(ts.T(), err) + + u1, err := models.NewUser("", "", "", ts.Config.JWT.Aud, nil) + require.NoError(ts.T(), err) + u1.IsAnonymous = true + require.NoError(ts.T(), ts.API.db.Create(u1)) + + u2, err := models.NewUser("", "", "", ts.Config.JWT.Aud, nil) + require.NoError(ts.T(), err) + u2.IsAnonymous = true + require.NoError(ts.T(), ts.API.db.Create(u2)) + + cases := []struct { + desc string + userId uuid.UUID + body map[string]interface{} + expected map[string]interface{} + expectedIdentities int + }{ + { + desc: "update anonymous user with email and email confirm true", + userId: u1.ID, + body: map[string]interface{}{ + "email": "foo@example.com", + "email_confirm": true, + }, + expected: map[string]interface{}{ + "email": "foo@example.com", + "is_anonymous": false, + }, + expectedIdentities: 1, + }, + { + desc: "update anonymous user with email and email confirm false", + userId: u2.ID, + body: map[string]interface{}{ + "email": "bar@example.com", + "email_confirm": false, + }, + expected: map[string]interface{}{ + "email": "bar@example.com", + "is_anonymous": true, + }, + expectedIdentities: 1, + }, + } + + for _, c := range cases { + ts.Run(c.desc, func() { + // Request body + var buffer bytes.Buffer + require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.body)) + + req := httptest.NewRequest(http.MethodPut, fmt.Sprintf("/admin/users/%s", c.userId), &buffer) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", adminJwt)) + + w := httptest.NewRecorder() + ts.API.handler.ServeHTTP(w, req) + require.Equal(ts.T(), http.StatusOK, w.Code) + + var data models.User + require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data)) + + require.NotNil(ts.T(), data) + require.Len(ts.T(), data.Identities, c.expectedIdentities) + + actual := map[string]interface{}{ + "email": data.GetEmail(), + "is_anonymous": data.IsAnonymous, + } + + require.Equal(ts.T(), c.expected, actual) + }) + } +}