diff --git a/internal/api/user.go b/internal/api/user.go index db0551473..121c5bd9e 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "net/http" + "time" "github.com/fatih/structs" "github.com/gofrs/uuid" @@ -68,10 +69,6 @@ func (p *UserUpdateParams) Validate(conn *storage.Connection, user *models.User, if len(*p.Password) < config.PasswordMinLength { return invalidPasswordLengthError(config.PasswordMinLength) } - // if password reauthentication is enabled, user can only update password together with a nonce sent - if config.Security.UpdatePasswordRequireReauthentication && p.Nonce == "" { - return unauthorizedError("Password update requires reauthentication.") - } } if p.AppData != nil { if !isAdmin(user, config) { @@ -129,8 +126,15 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { var terr error if params.Password != nil { if config.Security.UpdatePasswordRequireReauthentication { - if terr = a.verifyReauthentication(params.Nonce, tx, config, user); terr != nil { - return terr + now := time.Now() + // we require reauthentication if the user hasn't signed in recently in the current session + if session == nil || now.After(session.CreatedAt.Add(24*time.Hour)) { + if len(params.Nonce) == 0 { + return badRequestError("Password update requires reauthentication") + } + if terr = a.verifyReauthentication(params.Nonce, tx, config, user); terr != nil { + return terr + } } } diff --git a/internal/api/user_test.go b/internal/api/user_test.go index 4e6c304d3..c36d9cf63 100644 --- a/internal/api/user_test.go +++ b/internal/api/user_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/gofrs/uuid" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/supabase/gotrue/internal/conf" @@ -201,6 +202,23 @@ func (ts *UserTestSuite) TestUserUpdatePassword() { u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) + r, err := models.GrantAuthenticatedUser(ts.API.db, u, models.GrantParams{}) + require.NoError(ts.T(), err) + + r2, err := models.GrantAuthenticatedUser(ts.API.db, u, models.GrantParams{}) + require.NoError(ts.T(), err) + + // create a session and modify it's created_at time to simulate a session that is not recently logged in + notRecentlyLoggedIn, err := models.FindSessionByID(ts.API.db, *r2.SessionId) + require.NoError(ts.T(), err) + + // cannot use Update here because Update doesn't removes the created_at field + require.NoError(ts.T(), ts.API.db.RawQuery( + "update "+notRecentlyLoggedIn.TableName()+" set created_at = ? where id = ?", + time.Now().Add(-24*time.Hour), + notRecentlyLoggedIn.ID).Exec(), + ) + type expected struct { code int isAuthenticated bool @@ -211,36 +229,57 @@ func (ts *UserTestSuite) TestUserUpdatePassword() { newPassword string nonce string requireReauthentication bool + sessionId *uuid.UUID expected expected }{ - { - desc: "Valid password length", - newPassword: "newpassword", - nonce: "", - requireReauthentication: false, - expected: expected{code: http.StatusOK, isAuthenticated: true}, - }, { desc: "Invalid password length", newPassword: "", nonce: "", requireReauthentication: false, + sessionId: nil, expected: expected{code: http.StatusUnprocessableEntity, isAuthenticated: false}, }, { - desc: "No reauthentication provided", + desc: "No nonce provided", newPassword: "newpassword123", nonce: "", requireReauthentication: true, - expected: expected{code: http.StatusUnauthorized, isAuthenticated: false}, + sessionId: nil, + expected: expected{code: http.StatusBadRequest, isAuthenticated: false}, }, { - desc: "Invalid nonce", + desc: "Need reauthentication because outside of recently logged in window", newPassword: "newpassword123", + nonce: "", + requireReauthentication: true, + sessionId: r2.SessionId, + expected: expected{code: http.StatusBadRequest, isAuthenticated: false}, + }, + { + desc: "No need reauthentication because recently logged in", + newPassword: "newpassword123", + nonce: "", + requireReauthentication: true, + sessionId: r.SessionId, + expected: expected{code: http.StatusOK, isAuthenticated: true}, + }, + { + desc: "Invalid nonce", + newPassword: "newpassword1234", nonce: "123456", requireReauthentication: true, + sessionId: nil, expected: expected{code: http.StatusBadRequest, isAuthenticated: false}, }, + { + desc: "Valid password length", + newPassword: "newpassword", + nonce: "", + requireReauthentication: false, + sessionId: nil, + expected: expected{code: http.StatusOK, isAuthenticated: true}, + }, } for _, c := range cases { @@ -253,7 +292,8 @@ func (ts *UserTestSuite) TestUserUpdatePassword() { req.Header.Set("Content-Type", "application/json") var token string - token, err = generateAccessToken(ts.API.db, u, nil, &ts.Config.JWT) + + token, err = generateAccessToken(ts.API.db, u, c.sessionId, &ts.Config.JWT) require.NoError(ts.T(), err) req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))