Skip to content

Commit

Permalink
revert: "fix: only create or update the email / phone identity after …
Browse files Browse the repository at this point in the history 
…i… (supabase#1407)

…t's been verified (supabase#1403)"

This reverts commit 2d20729.

Reverting so it can be re-added again.
  • Loading branch information
@kangmingtay
kangmingtay authored Feb 8, 2024
1 parent 7763bd1 commit ff86849
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 69 deletions.
49 changes: 45 additions & 4 deletions internal/api/user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import (
"net/http"
"time"

"github.com/fatih/structs"
"github.com/gofrs/uuid"
"github.com/supabase/auth/internal/api/provider"
"github.com/supabase/auth/internal/api/sms_provider"
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/storage"
Expand Down Expand Up @@ -191,7 +193,29 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
}
}

var identities []models.Identity
if params.Email != "" && params.Email != user.GetEmail() {
identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "email")
if terr != nil {
if !models.IsNotFoundError(terr) {
return terr
}
// updating the user's email should create a new email identity since the user doesn't have one
identity, terr = a.createNewIdentity(tx, user, "email", structs.Map(provider.Claims{
Subject: user.ID.String(),
Email: params.Email,
}))
if terr != nil {
return terr
}
} else {
if terr := identity.UpdateIdentityData(tx, map[string]interface{}{
"email": params.Email,
}); terr != nil {
return terr
}
}
identities = append(identities, *identity)
mailer := a.Mailer(ctx)
referrer := utilities.GetReferrer(r, config)
flowType := getFlowFromChallenge(params.CodeChallenge)
Expand All @@ -214,13 +238,29 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
}

if params.Phone != "" && params.Phone != user.GetPhone() {
if config.Sms.Autoconfirm {
if _, terr := a.smsVerify(r, ctx, tx, user, &VerifyParams{
Type: phoneChangeVerification,
Phone: params.Phone,
identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "phone")
if terr != nil {
if !models.IsNotFoundError(terr) {
return terr
}
// updating the user's phone should create a new phone identity since the user doesn't have one
identity, terr = a.createNewIdentity(tx, user, "phone", structs.Map(provider.Claims{
Subject: user.ID.String(),
Phone: params.Phone,
}))
if terr != nil {
return terr
}
} else {
if terr := identity.UpdateIdentityData(tx, map[string]interface{}{
"phone": params.Phone,
}); terr != nil {
return terr
}
}
identities = append(identities, *identity)
if config.Sms.Autoconfirm {
return user.UpdatePhone(tx, params.Phone)
} else {
smsProvider, terr := sms_provider.GetSmsProvider(*config)
if terr != nil {
Expand All @@ -231,6 +271,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
}
}
}
user.Identities = append(user.Identities, identities...)

if terr = models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil {
return internalServerError("Error recording audit log entry").WithInternalError(terr)
Expand Down
82 changes: 17 additions & 65 deletions internal/api/verify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@ import (
"strings"
"time"

"github.com/fatih/structs"
"github.com/sethvargo/go-password/password"
"github.com/supabase/auth/internal/api/provider"
"github.com/supabase/auth/internal/api/sms_provider"
"github.com/supabase/auth/internal/crypto"
"github.com/supabase/auth/internal/models"
Expand Down Expand Up @@ -260,7 +258,7 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP
return nil
}
case smsVerification, phoneChangeVerification:
user, terr = a.smsVerify(r, ctx, tx, user, params)
user, terr = a.smsVerify(r, ctx, tx, user, params.Type)
default:
return unprocessableEntityError("Unsupported verification type")
}
Expand Down Expand Up @@ -369,53 +367,28 @@ func (a *API) recoverVerify(r *http.Request, ctx context.Context, conn *storage.
return user, nil
}

func (a *API) smsVerify(r *http.Request, ctx context.Context, conn *storage.Connection, user *models.User, params *VerifyParams) (*models.User, error) {
func (a *API) smsVerify(r *http.Request, ctx context.Context, conn *storage.Connection, user *models.User, otpType string) (*models.User, error) {
config := a.config

err := conn.Transaction(func(tx *storage.Connection) error {
if terr := triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil {
var terr error
if terr = models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", nil); terr != nil {
return terr
}

if params.Type == smsVerification {
if terr := models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", nil); terr != nil {
return terr
}
if terr := user.ConfirmPhone(tx); terr != nil {
if terr = triggerEventHooks(ctx, tx, SignupEvent, user, config); terr != nil {
return terr
}

if otpType == smsVerification {
if terr = user.ConfirmPhone(tx); terr != nil {
return internalServerError("Error confirming user").WithInternalError(terr)
}
} else if params.Type == phoneChangeVerification {
if terr := models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil {
return terr
}
if identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "phone"); terr != nil {
if !models.IsNotFoundError(terr) {
return terr
}
// confirming the phone change should create a new phone identity if the user doesn't have one
if _, terr = a.createNewIdentity(tx, user, "phone", structs.Map(provider.Claims{
Subject: user.ID.String(),
Phone: params.Phone,
PhoneVerified: true,
})); terr != nil {
return terr
}
} else {
if terr := identity.UpdateIdentityData(tx, map[string]interface{}{
"phone": params.Phone,
"phone_verified": true,
}); terr != nil {
return terr
}
}
if terr := user.ConfirmPhoneChange(tx); terr != nil {
} else if otpType == phoneChangeVerification {
if terr = user.ConfirmPhoneChange(tx); terr != nil {
return internalServerError("Error confirming user").WithInternalError(terr)
}
}

if terr := tx.Load(user, "Identities"); terr != nil {
return internalServerError("Error refetching identities").WithInternalError(terr)
}
return nil
})
if err != nil {
Expand Down Expand Up @@ -505,38 +478,17 @@ func (a *API) emailChangeVerify(r *http.Request, ctx context.Context, conn *stor

// one email is confirmed at this point if GOTRUE_MAILER_SECURE_EMAIL_CHANGE_ENABLED is enabled
err := conn.Transaction(func(tx *storage.Connection) error {
if terr := models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil {
var terr error

if terr = models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil {
return terr
}

if terr := triggerEventHooks(ctx, tx, EmailChangeEvent, user, config); terr != nil {
if terr = triggerEventHooks(ctx, tx, EmailChangeEvent, user, config); terr != nil {
return terr
}

if identity, terr := models.FindIdentityByIdAndProvider(tx, user.ID.String(), "email"); terr != nil {
if !models.IsNotFoundError(terr) {
return terr
}
// confirming the email change should create a new email identity if the user doesn't have one
if _, terr = a.createNewIdentity(tx, user, "email", structs.Map(provider.Claims{
Subject: user.ID.String(),
Email: params.Email,
EmailVerified: true,
})); terr != nil {
return terr
}
} else {
if terr := identity.UpdateIdentityData(tx, map[string]interface{}{
"email": params.Email,
"email_verified": true,
}); terr != nil {
return terr
}
}
if terr := tx.Load(user, "Identities"); terr != nil {
return internalServerError("Error refetching identities").WithInternalError(terr)
}
if terr := user.ConfirmEmailChange(tx, zeroConfirmation); terr != nil {
if terr = user.ConfirmEmailChange(tx, zeroConfirmation); terr != nil {
return internalServerError("Error confirm email").WithInternalError(terr)
}

Expand Down

0 comments on commit ff86849

Please sign in to comment.