Skip to content

Commit

Permalink
fix: sms verify should update is_anonymous field (#1580)
Browse files Browse the repository at this point in the history 
## What kind of change does this PR introduce?
* verifying the phone number of a user should update the `is_anonymous`
field to false
* add test to prevent any future regression

---------

Co-authored-by: Joel Lee <lee.yi.jie.joel@gmail.com>
  • Loading branch information
@kangmingtay @J0
kangmingtay and J0 committed May 13, 2024
1 parent ed2b490 commit e5f98cb
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 59 deletions.
165 changes: 106 additions & 59 deletions internal/api/anonymous_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/supabase/auth/internal/conf"
mail "github.com/supabase/auth/internal/mailer"
"github.com/supabase/auth/internal/models"
)

Expand Down Expand Up @@ -77,66 +78,112 @@ func (ts *AnonymousTestSuite) TestAnonymousLogins() {

func (ts *AnonymousTestSuite) TestConvertAnonymousUserToPermanent() {
ts.Config.External.AnonymousUsers.Enabled = true
// Request body
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{}))

req := httptest.NewRequest(http.MethodPost, "/signup", &buffer)
req.Header.Set("Content-Type", "application/json")

w := httptest.NewRecorder()

ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusOK, w.Code)

signupResponse := &AccessTokenResponse{}
require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&signupResponse))

// Add email to anonymous user
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"email": "test@example.com",
}))

req = httptest.NewRequest(http.MethodPut, "/user", &buffer)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", signupResponse.Token))

w = httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusOK, w.Code)

// Check if anonymous user is still anonymous
user, err := models.FindUserByID(ts.API.db, signupResponse.User.ID)
require.NoError(ts.T(), err)
require.NotEmpty(ts.T(), user)
require.True(ts.T(), user.IsAnonymous)

// Verify email change
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"token_hash": user.EmailChangeTokenNew,
"type": "email_change",
}))

req = httptest.NewRequest(http.MethodPost, "/verify", &buffer)
req.Header.Set("Content-Type", "application/json")

w = httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusOK, w.Code)

data := &AccessTokenResponse{}
require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data))

// User is a permanent user and not anonymous anymore
assert.Equal(ts.T(), signupResponse.User.ID, data.User.ID)
assert.Equal(ts.T(), ts.Config.JWT.Aud, data.User.Aud)
assert.Equal(ts.T(), "test@example.com", data.User.GetEmail())
assert.Equal(ts.T(), models.JSONMap(models.JSONMap{"provider": "email", "providers": []interface{}{"email"}}), data.User.AppMetaData)
assert.False(ts.T(), data.User.IsAnonymous)
assert.NotEmpty(ts.T(), data.User.EmailConfirmedAt)
ts.Config.Sms.TestOTP = map[string]string{"1234567890": "000000"}
// test OTPs still require setting up an sms provider
ts.Config.Sms.Provider = "twilio"
ts.Config.Sms.Twilio.AccountSid = "fake-sid"
ts.Config.Sms.Twilio.AuthToken = "fake-token"
ts.Config.Sms.Twilio.MessageServiceSid = "fake-message-service-sid"

cases := []struct {
desc string
body map[string]interface{}
verificationType string
}{
{
desc: "convert anonymous user to permanent user with email",
body: map[string]interface{}{
"email": "test@example.com",
},
verificationType: "email_change",
},
{
desc: "convert anonymous user to permanent user with phone",
body: map[string]interface{}{
"phone": "1234567890",
},
verificationType: "phone_change",
},
}

// User should have an email identity
assert.Len(ts.T(), data.User.Identities, 1)
for _, c := range cases {
ts.Run(c.desc, func() {
// Request body
var buffer bytes.Buffer
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{}))

req := httptest.NewRequest(http.MethodPost, "/signup", &buffer)
req.Header.Set("Content-Type", "application/json")

w := httptest.NewRecorder()

ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusOK, w.Code)

signupResponse := &AccessTokenResponse{}
require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&signupResponse))

// Add email to anonymous user
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(c.body))

req = httptest.NewRequest(http.MethodPut, "/user", &buffer)
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", signupResponse.Token))

w = httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusOK, w.Code)

// Check if anonymous user is still anonymous
user, err := models.FindUserByID(ts.API.db, signupResponse.User.ID)
require.NoError(ts.T(), err)
require.NotEmpty(ts.T(), user)
require.True(ts.T(), user.IsAnonymous)

switch c.verificationType {
case mail.EmailChangeVerification:
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"token_hash": user.EmailChangeTokenNew,
"type": c.verificationType,
}))
case phoneChangeVerification:
require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
"phone": "1234567890",
"token": "000000",
"type": c.verificationType,
}))
}

req = httptest.NewRequest(http.MethodPost, "/verify", &buffer)
req.Header.Set("Content-Type", "application/json")

w = httptest.NewRecorder()
ts.API.handler.ServeHTTP(w, req)
require.Equal(ts.T(), http.StatusOK, w.Code)

data := &AccessTokenResponse{}
require.NoError(ts.T(), json.NewDecoder(w.Body).Decode(&data))

// User is a permanent user and not anonymous anymore
assert.Equal(ts.T(), signupResponse.User.ID, data.User.ID)
assert.Equal(ts.T(), ts.Config.JWT.Aud, data.User.Aud)
assert.False(ts.T(), data.User.IsAnonymous)

// User should have an identity
assert.Len(ts.T(), data.User.Identities, 1)

switch c.verificationType {
case mail.EmailChangeVerification:
assert.Equal(ts.T(), "test@example.com", data.User.GetEmail())
assert.Equal(ts.T(), models.JSONMap(models.JSONMap{"provider": "email", "providers": []interface{}{"email"}}), data.User.AppMetaData)
assert.NotEmpty(ts.T(), data.User.EmailConfirmedAt)
case phoneChangeVerification:
assert.Equal(ts.T(), "1234567890", data.User.GetPhone())
assert.Equal(ts.T(), models.JSONMap(models.JSONMap{"provider": "phone", "providers": []interface{}{"phone"}}), data.User.AppMetaData)
assert.NotEmpty(ts.T(), data.User.PhoneConfirmedAt)
}
})
}
}

func (ts *AnonymousTestSuite) TestRateLimitAnonymousSignups() {
Expand Down
7 changes: 7 additions & 0 deletions internal/api/verify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,13 @@ func (a *API) smsVerify(r *http.Request, conn *storage.Connection, user *models.
}
}

if user.IsAnonymous {
user.IsAnonymous = false
if terr := tx.UpdateOnly(user, "is_anonymous"); terr != nil {
return terr
}
}

if terr := tx.Load(user, "Identities"); terr != nil {
return internalServerError("Error refetching identities").WithInternalError(terr)
}
Expand Down

0 comments on commit e5f98cb

Please sign in to comment.