fix: use pointer for user.EncryptedPassword (supabase#1637)

Makes sure that `NULL` values in the `auth.users.encrypted_password` column are not met with SQL serialization errors.
LashaJini · Jun 27, 2024 · bbecbd6 · bbecbd6
1 parent d8b47f9
commit bbecbd6
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 13 deletions.
diff --git a/internal/api/invite_test.go b/internal/api/invite_test.go
@@ -207,7 +207,7 @@ func (ts *InviteTestSuite) TestVerifyInvite() {
 			now := time.Now()
 			user.InvitedAt = &now
 			user.ConfirmationSentAt = &now
-			user.EncryptedPassword = ""
+			user.EncryptedPassword = nil
 			user.ConfirmationToken = crypto.GenerateTokenHash(c.email, c.requestBody["token"].(string))
 			require.NoError(ts.T(), err)
 			require.NoError(ts.T(), ts.API.db.Create(user))

diff --git a/internal/api/user.go b/internal/api/user.go
@@ -155,7 +155,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
 		if password != "" {
 			isSamePassword := false
 
-			if user.EncryptedPassword != "" {
+			if user.HasPassword() {
 				auth, _, err := user.Authenticate(ctx, password, config.Security.DBEncryption.DecryptionKeys, false, "")
 				if err != nil {
 					return err

diff --git a/internal/api/verify.go b/internal/api/verify.go
@@ -306,7 +306,7 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP
 func (a *API) signupVerify(r *http.Request, ctx context.Context, conn *storage.Connection, user *models.User) (*models.User, error) {
 	config := a.config
 
-	if user.EncryptedPassword == "" && user.InvitedAt != nil {
+	if !user.HasPassword() && user.InvitedAt != nil {
 		// sign them up with temporary password, and require application
 		// to present the user with a password set form
 		password, err := password.Generate(64, 10, 0, false, true)
@@ -322,7 +322,7 @@ func (a *API) signupVerify(r *http.Request, ctx context.Context, conn *storage.C
 
 	err := conn.Transaction(func(tx *storage.Connection) error {
 		var terr error
-		if user.EncryptedPassword == "" && user.InvitedAt != nil {
+		if !user.HasPassword() && user.InvitedAt != nil {
 			if terr = user.UpdatePassword(tx, nil); terr != nil {
 				return internalServerError("Error storing password").WithInternalError(terr)
 			}

diff --git a/internal/models/user.go b/internal/models/user.go
@@ -25,7 +25,7 @@ type User struct {
 	Email     storage.NullString `json:"email" db:"email"`
 	IsSSOUser bool               `json:"-" db:"is_sso_user"`
 
-	EncryptedPassword string     `json:"-" db:"encrypted_password"`
+	EncryptedPassword *string    `json:"-" db:"encrypted_password"`
 	EmailConfirmedAt  *time.Time `json:"email_confirmed_at,omitempty" db:"email_confirmed_at"`
 	InvitedAt         *time.Time `json:"invited_at,omitempty" db:"invited_at"`
 
@@ -95,7 +95,7 @@ func NewUser(phone, email, password, aud string, userData map[string]interface{}
 		Email:             storage.NullString(strings.ToLower(email)),
 		Phone:             storage.NullString(phone),
 		UserMetaData:      userData,
-		EncryptedPassword: passwordHash,
+		EncryptedPassword: &passwordHash,
 	}
 	return user, nil
 }
@@ -106,6 +106,16 @@ func (User) TableName() string {
 	return tableName
 }
 
+func (u *User) HasPassword() bool {
+	var pwd string
+
+	if u.EncryptedPassword != nil {
+		pwd = *u.EncryptedPassword
+	}
+
+	return pwd != ""
+}
+
 // BeforeSave is invoked before the user is saved to the database
 func (u *User) BeforeSave(tx *pop.Connection) error {
 	if u.EmailConfirmedAt != nil && u.EmailConfirmedAt.IsZero() {
@@ -285,7 +295,7 @@ func (u *User) SetPhone(tx *storage.Connection, phone string) error {
 
 func (u *User) SetPassword(ctx context.Context, password string, encrypt bool, encryptionKeyID, encryptionKey string) error {
 	if password == "" {
-		u.EncryptedPassword = ""
+		u.EncryptedPassword = nil
 		return nil
 	}
 
@@ -294,14 +304,15 @@ func (u *User) SetPassword(ctx context.Context, password string, encrypt bool, e
 		return err
 	}
 
-	u.EncryptedPassword = pw
+	u.EncryptedPassword = &pw
 	if encrypt {
 		es, err := crypto.NewEncryptedString(u.ID.String(), []byte(pw), encryptionKeyID, encryptionKey)
 		if err != nil {
 			return err
 		}
 
-		u.EncryptedPassword = es.String()
+		encryptedPassword := es.String()
+		u.EncryptedPassword = &encryptedPassword
 	}
 
 	return nil
@@ -341,9 +352,13 @@ func (u *User) UpdatePassword(tx *storage.Connection, sessionID *uuid.UUID) erro
 
 // Authenticate a user from a password
 func (u *User) Authenticate(ctx context.Context, password string, decryptionKeys map[string]string, encrypt bool, encryptionKeyID string) (bool, bool, error) {
-	hash := u.EncryptedPassword
+	if u.EncryptedPassword == nil {
+		return false, false, nil
+	}
+
+	hash := *u.EncryptedPassword
 
-	es := crypto.ParseEncryptedString(u.EncryptedPassword)
+	es := crypto.ParseEncryptedString(hash)
 	if es != nil {
 		h, err := es.Decrypt(u.ID.String(), decryptionKeys)
 		if err != nil {
@@ -719,7 +734,7 @@ func (u *User) UpdateBannedUntil(tx *storage.Connection) error {
 func (u *User) RemoveUnconfirmedIdentities(tx *storage.Connection, identity *Identity) error {
 	if identity.Provider != "email" && identity.Provider != "phone" {
 		// user is unconfirmed so the password should be reset
-		u.EncryptedPassword = ""
+		u.EncryptedPassword = nil
 		if terr := tx.UpdateOnly(u, "encrypted_password"); terr != nil {
 			return terr
 		}
@@ -755,7 +770,7 @@ func (u *User) SoftDeleteUser(tx *storage.Connection) error {
 	u.Phone = storage.NullString(obfuscatePhone(u, u.GetPhone()))
 	u.EmailChange = obfuscateEmail(u, u.EmailChange)
 	u.PhoneChange = obfuscatePhone(u, u.PhoneChange)
-	u.EncryptedPassword = ""
+	u.EncryptedPassword = nil
 	u.ConfirmationToken = ""
 	u.RecoveryToken = ""
 	u.EmailChangeTokenCurrent = ""