Skip to content

Commit

Permalink
fix: admin user update should update is_anonymous field (supabase#1623)
Browse files Browse the repository at this point in the history
## What kind of change does this PR introduce?
* Fixes supabase#1578
  • Loading branch information
kangmingtay authored Jun 17, 2024
1 parent 3cd00ee commit f5c6fcd
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 6 deletions.
29 changes: 23 additions & 6 deletions internal/api/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
Expand All @@ -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
}
Expand Down
84 changes: 84 additions & 0 deletions internal/api/anonymous_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
})
}
}

0 comments on commit f5c6fcd

Please sign in to comment.