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.

Sign up for GitHub

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

fix: only create or update the email / phone identity after it's been verified #1403

Merged
merged 3 commits into from
Feb 8, 2024
Merged

fix: only create or update the email / phone identity after it's been verified #1403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b7f44d8
fix: create & update email identity after verify
kangmingtay Feb 7, 2024
80e0262
fix: create & update phone identity after verify
kangmingtay Feb 7, 2024
fc2554b
fix: update audit log events for phone change
kangmingtay Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 4 additions & 45 deletions internal/api/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ 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 @@ -193,29 +191,7 @@ 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 @@ -238,29 +214,13 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
}

if params.Phone != "" && params.Phone != user.GetPhone() {
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,
if config.Sms.Autoconfirm {
if _, terr := a.smsVerify(r, ctx, tx, user, &VerifyParams{
Type: phoneChangeVerification,
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 @@ -271,7 +231,6 @@ 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: 65 additions & 17 deletions internal/api/verify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ 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 @@ -258,7 +260,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.Type)
user, terr = a.smsVerify(r, ctx, tx, user, params)
default:
return unprocessableEntityError("Unsupported verification type")
}
Expand Down Expand Up @@ -367,28 +369,53 @@ 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, otpType string) (*models.User, error) {
func (a *API) smsVerify(r *http.Request, ctx context.Context, conn *storage.Connection, user *models.User, params *VerifyParams) (*models.User, error) {
config := a.config

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

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

if otpType == smsVerification {
if terr = user.ConfirmPhone(tx); terr != nil {
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 {
return internalServerError("Error confirming user").WithInternalError(terr)
}
} else if otpType == phoneChangeVerification {
if terr = user.ConfirmPhoneChange(tx); terr != nil {
} 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 {
return internalServerError("Error confirming user").WithInternalError(terr)
}
}

if terr := tx.Load(user, "Identities"); terr != nil {
kangmingtay marked this conversation as resolved.
Show resolved Hide resolved
return internalServerError("Error refetching identities").WithInternalError(terr)
}
return nil
})
if err != nil {
Expand Down Expand Up @@ -478,17 +505,38 @@ 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 {
var terr error

if terr = models.NewAuditLogEntry(r, tx, user, models.UserModifiedAction, "", nil); terr != nil {
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 terr = user.ConfirmEmailChange(tx, zeroConfirmation); terr != nil {
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 {
kangmingtay marked this conversation as resolved.
Show resolved Hide resolved
return internalServerError("Error refetching identities").WithInternalError(terr)
}
if terr := user.ConfirmEmailChange(tx, zeroConfirmation); terr != nil {
return internalServerError("Error confirm email").WithInternalError(terr)
}

Expand Down
Loading