Repository: supabase/gotrue
Pull request #812 (2022-11-15 14:26:34)

feat(saml): add session expiration (not after timestamp) support (disabled by default)
The SAML ACS endpoint will create sessions with a "not after" timestamp. Refresh tokens won't be refreshable past this timestamp.


Patch:
diff --git a/api/auth.go b/api/auth.go
index 56796924..eeec1f0f 100644
--- a/api/auth.go
+++ b/api/auth.go
@@ -113,7 +113,7 @@ func (a *API) maybeLoadUserOrSession(ctx context.Context) (context.Context, erro
 		if err != nil {
 			return ctx, err
 		}
-		session, err = models.FindSessionById(db, sessionId)
+		session, err = models.FindSessionByID(db, sessionId)
 		if err != nil && !models.IsNotFoundError(err) {
 			return ctx, err
 		}
diff --git a/api/samlacs.go b/api/samlacs.go
index cb0c7b12..c6f7827c 100644
--- a/api/samlacs.go
+++ b/api/samlacs.go
@@ -228,6 +228,14 @@ func (a *API) SAMLACS(w http.ResponseWriter, r *http.Request) error {
 	// refreshTokenParams.NotBefore = assertion.NotBefore()
 	// refreshTokenParams.NotAfter = assertion.NotAfter()
 
+	notAfter := assertion.NotAfter()
+
+	var grantParams models.GrantParams
+
+	if !notAfter.IsZero() {
+		grantParams.SessionNotAfter = &notAfter
+	}
+
 	var token *AccessTokenResponse
 
 	if err := db.Transaction(func(tx *storage.Connection) error {
@@ -238,7 +246,7 @@ func (a *API) SAMLACS(w http.ResponseWriter, r *http.Request) error {
 			return terr
 		}
 
-		token, terr = a.issueRefreshToken(ctx, tx, user, models.SSOSAML, models.GrantParams{})
+		token, terr = a.issueRefreshToken(ctx, tx, user, models.SSOSAML, grantParams)
 
 		if terr != nil {
 			return internalServerError("Unable to issue refresh token from SAML Assertion").WithInternalError(terr)
diff --git a/api/token.go b/api/token.go
index 93ee7d9d..142c0d18 100644
--- a/api/token.go
+++ b/api/token.go
@@ -276,16 +276,28 @@ func (a *API) RefreshTokenGrant(ctx context.Context, w http.ResponseWriter, r *h
 		return oauthError("invalid_request", "refresh_token required")
 	}
 
-	user, token, err := models.FindUserWithRefreshToken(db, params.RefreshToken)
+	user, token, session, err := models.FindUserWithRefreshToken(db, params.RefreshToken)
 	if err != nil {
 		if models.IsNotFoundError(err) {
-			return oauthError("invalid_grant", "Invalid Refresh Token")
+			return oauthError("invalid_grant", "Invalid Refresh Token: Refresh Token Not Found")
 		}
 		return internalServerError(err.Error())
 	}
 
 	if user.IsBanned() {
-		return oauthError("invalid_grant", "Invalid Refresh Token")
+		return oauthError("invalid_grant", "Invalid Refresh Token: User Banned")
+	}
+
+	if session != nil {
+		var notAfter time.Time
+
+		if session.NotAfter != nil {
+			notAfter = *session.NotAfter
+		}
+
+		if !notAfter.IsZero() && time.Now().UTC().After(notAfter) {
+			return oauthError("invalid_grant", "Invalid Refresh Token: Session Expired")
+		}
 	}
 
 	var newToken *models.RefreshToken
@@ -555,7 +567,7 @@ func generateAccessToken(tx *storage.Connection, user *models.User, sessionId *u
 	sid := ""
 	if sessionId != nil {
 		sid = sessionId.String()
-		session, terr := models.FindSessionById(tx, *sessionId)
+		session, terr := models.FindSessionByID(tx, *sessionId)
 		if terr != nil {
 			return "", terr
 		}
@@ -599,7 +611,7 @@ func (a *API) issueRefreshToken(ctx context.Context, conn *storage.Connection, u
 			return internalServerError("Database error granting user").WithInternalError(terr)
 		}
 
-		session, terr := models.FindSessionById(tx, *refreshToken.SessionId)
+		session, terr := models.FindSessionByID(tx, *refreshToken.SessionId)
 		if terr != nil {
 			return terr
 		}
@@ -639,7 +651,7 @@ func (a *API) updateMFASessionAndClaims(r *http.Request, tx *storage.Connection,
 		return nil, internalServerError("Cannot read SessionId claim as UUID").WithInternalError(err)
 	}
 	err = tx.Transaction(func(tx *storage.Connection) error {
-		session, terr := models.FindSessionById(tx, sessionId)
+		session, terr := models.FindSessionByID(tx, sessionId)
 		if terr != nil {
 			return terr
 		}
@@ -647,7 +659,7 @@ func (a *API) updateMFASessionAndClaims(r *http.Request, tx *storage.Connection,
 		if terr != nil {
 			return terr
 		}
-		session, terr = models.FindSessionById(tx, sessionId)
+		session, terr = models.FindSessionByID(tx, sessionId)
 		if terr != nil {
 			return terr
 		}
diff --git a/models/refresh_token.go b/models/refresh_token.go
index 1cf953ca..b86e5067 100644
--- a/models/refresh_token.go
+++ b/models/refresh_token.go
@@ -39,6 +39,8 @@ func (RefreshToken) TableName() string {
 // refresh token to authenticated users.
 type GrantParams struct {
 	FactorID *uuid.UUID
+
+	SessionNotAfter *time.Time
 }
 
 // GrantAuthenticatedUser creates a refresh token for the provided user.
@@ -125,14 +127,25 @@ func createRefreshToken(tx *storage.Connection, user *User, oldToken *RefreshTok
 	}
 
 	if token.SessionId == nil {
-		// Existing refresh tokens may have a null session_id if they were created before v2.15.3
-		var session *Session
-		var err error
-		session, err = CreateSession(tx, user, params.FactorID)
-
+		session, err := NewSession()
 		if err != nil {
-			return nil, errors.Wrap(err, "Error generated unique session id")
+			return nil, errors.Wrap(err, "error instantiating new session object")
+		}
+
+		session.UserID = user.ID
+
+		if params.FactorID != nil {
+			session.FactorID = params.FactorID
 		}
+
+		if params.SessionNotAfter != nil {
+			session.NotAfter = params.SessionNotAfter
+		}
+
+		if err := tx.Create(session); err != nil {
+			return nil, errors.Wrap(err, "error creating new session")
+		}
+
 		token.SessionId = &session.ID
 	}
 
diff --git a/models/sessions.go b/models/sessions.go
index 4bf59a11..aebf853c 100644
--- a/models/sessions.go
+++ b/models/sessions.go
@@ -70,7 +70,7 @@ func (Session) TableName() string {
 	return tableName
 }
 
-func NewSession(user *User, factorID *uuid.UUID) (*Session, error) {
+func NewSession() (*Session, error) {
 	id, err := uuid.NewV4()
 	if err != nil {
 		return nil, errors.Wrap(err, "Error generating unique session id")
@@ -79,27 +79,14 @@ func NewSession(user *User, factorID *uuid.UUID) (*Session, error) {
 	defaultAAL := AAL1.String()
 
 	session := &Session{
-		ID:        id,
-		UserID:    user.ID,
-		FactorID:  factorID,
-		AAL:       &defaultAAL,
-		AMRClaims: []AMRClaim{},
+		ID:  id,
+		AAL: &defaultAAL,
 	}
-	return session, nil
-}
 
-func CreateSession(tx *storage.Connection, user *User, factorID *uuid.UUID) (*Session, error) {
-	session, err := NewSession(user, factorID)
-	if err != nil {
-		return nil, err
-	}
-	if err := tx.Create(session); err != nil {
-		return nil, errors.Wrap(err, "error creating session")
-	}
 	return session, nil
 }
 
-func FindSessionById(tx *storage.Connection, id uuid.UUID) (*Session, error) {
+func FindSessionByID(tx *storage.Connection, id uuid.UUID) (*Session, error) {
 	session := &Session{}
 	if err := tx.Eager().Q().Where("id = ?", id).First(session); err != nil {
 		if errors.Cause(err) == sql.ErrNoRows {
diff --git a/models/user.go b/models/user.go
index a3245cd1..d7856347 100644
--- a/models/user.go
+++ b/models/user.go
@@ -354,21 +354,38 @@ func FindUserByEmailChangeToken(tx *storage.Connection, token string) (*User, er
 }
 
 // FindUserWithRefreshToken finds a user from the provided refresh token.
-func FindUserWithRefreshToken(tx *storage.Connection, token string) (*User, *RefreshToken, error) {
+func FindUserWithRefreshToken(tx *storage.Connection, token string) (*User, *RefreshToken, *Session, error) {
 	refreshToken := &RefreshToken{}
 	if err := tx.Where("token = ?", token).First(refreshToken); err != nil {
 		if errors.Cause(err) == sql.ErrNoRows {
-			return nil, nil, RefreshTokenNotFoundError{}
+			return nil, nil, nil, RefreshTokenNotFoundError{}
 		}
-		return nil, nil, errors.Wrap(err, "error finding refresh token")
+		return nil, nil, nil, errors.Wrap(err, "error finding refresh token")
 	}
 
-	user, err := findUser(tx, "id = ?", refreshToken.UserID)
+	user, err := FindUserByID(tx, refreshToken.UserID)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 
-	return user, refreshToken, nil
+	var session *Session
+
+	if refreshToken.SessionId != nil {
+		sessionId := *refreshToken.SessionId
+
+		if sessionId != uuid.Nil {
+			session, err = FindSessionByID(tx, sessionId)
+			if err != nil {
+				if !IsNotFoundError(err) {
+					return nil, nil, nil, errors.Wrap(err, "error finding session from refresh token")
+				}
+
+				// otherwise, there's no session for this refresh token
+			}
+		}
+	}
+
+	return user, refreshToken, session, nil
 }
 
 // FindUsersInAudience finds users with the matching audience.


Test patch:
diff --git a/api/auth_test.go b/api/auth_test.go
index ad00f532..9d616718 100644
--- a/api/auth_test.go
+++ b/api/auth_test.go
@@ -75,9 +75,12 @@ func (ts *AuthTestSuite) TestMaybeLoadUserOrSession() {
 	u, err := models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
 
-	var s *models.Session
-	s, err = models.CreateSession(ts.API.db, u, &uuid.Nil)
+	s, err := models.NewSession()
 	require.NoError(ts.T(), err)
+	s.UserID = u.ID
+	require.NoError(ts.T(), ts.API.db.Create(s))
+
+	require.NoError(ts.T(), ts.API.db.Load(s))
 
 	cases := []struct {
 		Desc            string
diff --git a/api/mfa_test.go b/api/mfa_test.go
index fd2b9f5e..b27c5b56 100644
--- a/api/mfa_test.go
+++ b/api/mfa_test.go
@@ -54,8 +54,10 @@ func (ts *MFATestSuite) SetupTest() {
 	require.NoError(ts.T(), err, "Error creating test factor model")
 	require.NoError(ts.T(), ts.API.db.Create(f), "Error saving new test factor")
 	// Create corresponding sessoin
-	s, err := models.NewSession(u, &f.ID)
+	s, err := models.NewSession()
 	require.NoError(ts.T(), err, "Error creating test session")
+	s.UserID = u.ID
+	s.FactorID = &f.ID
 	require.NoError(ts.T(), ts.API.db.Create(s), "Error saving test session")
 
 	// Generate TOTP related settings
@@ -214,8 +216,10 @@ func (ts *MFATestSuite) TestMFAVerifyFactor() {
 			require.NoError(ts.T(), ts.API.db.Update(f), "Error updating new test factor")
 
 			// Create session to be invalidated
-			secondarySession, err := models.NewSession(user, &f.ID)
+			secondarySession, err := models.NewSession()
 			require.NoError(ts.T(), err, "Error creating test session")
+			secondarySession.UserID = user.ID
+			secondarySession.FactorID = &f.ID
 			require.NoError(ts.T(), ts.API.db.Create(secondarySession), "Error saving test session")
 
 			token, err := generateAccessToken(ts.API.db, user, r.SessionId, time.Second*time.Duration(ts.Config.JWT.Exp), ts.Config.JWT.Secret)
@@ -254,7 +258,7 @@ func (ts *MFATestSuite) TestMFAVerifyFactor() {
 
 			if v.expectedHTTPCode == http.StatusOK {
 				// Ensure alternate session has been deleted
-				_, err = models.FindSessionById(ts.API.db, secondarySession.ID)
+				_, err = models.FindSessionByID(ts.API.db, secondarySession.ID)
 				require.EqualError(ts.T(), err, models.SessionNotFoundError{}.Error())
 			}
 			if !v.validChallenge {
@@ -300,8 +304,10 @@ func (ts *MFATestSuite) TestUnenrollVerifiedFactor() {
 			factors, err := models.FindFactorsByUser(ts.API.db, u)
 			require.NoError(ts.T(), err, "error finding factors")
 			f := factors[0]
-			secondarySession, err = models.NewSession(u, &f.ID)
+			secondarySession, err = models.NewSession()
 			require.NoError(ts.T(), err, "Error creating test session")
+			secondarySession.UserID = u.ID
+			secondarySession.FactorID = &f.ID
 			require.NoError(ts.T(), ts.API.db.Create(secondarySession), "Error saving test session")
 
 			sharedSecret := ts.TestOTPKey.Secret()
@@ -324,7 +330,7 @@ func (ts *MFATestSuite) TestUnenrollVerifiedFactor() {
 			if v.expectedHTTPCode == http.StatusOK {
 				_, err = models.FindFactorByFactorID(ts.API.db, f.ID)
 				require.EqualError(ts.T(), err, models.FactorNotFoundError{}.Error())
-				session, _ := models.FindSessionById(ts.API.db, secondarySession.ID)
+				session, _ := models.FindSessionByID(ts.API.db, secondarySession.ID)
 				require.Equal(ts.T(), models.AAL1.String(), session.GetAAL())
 				require.Nil(ts.T(), session.FactorID)
 
@@ -343,8 +349,10 @@ func (ts *MFATestSuite) TestUnenrollUnverifiedFactor() {
 	factors, err := models.FindFactorsByUser(ts.API.db, u)
 	require.NoError(ts.T(), err, "error finding factors")
 	f := factors[0]
-	secondarySession, err = models.NewSession(u, &f.ID)
+	secondarySession, err = models.NewSession()
 	require.NoError(ts.T(), err, "Error creating test session")
+	secondarySession.UserID = u.ID
+	secondarySession.FactorID = &f.ID
 	require.NoError(ts.T(), ts.API.db.Create(secondarySession), "Error saving test session")
 
 	sharedSecret := ts.TestOTPKey.Secret()
@@ -365,7 +373,7 @@ func (ts *MFATestSuite) TestUnenrollUnverifiedFactor() {
 	require.Equal(ts.T(), http.StatusOK, w.Code)
 	_, err = models.FindFactorByFactorID(ts.API.db, f.ID)
 	require.EqualError(ts.T(), err, models.FactorNotFoundError{}.Error())
-	session, _ := models.FindSessionById(ts.API.db, secondarySession.ID)
+	session, _ := models.FindSessionByID(ts.API.db, secondarySession.ID)
 	require.Equal(ts.T(), models.AAL1.String(), session.GetAAL())
 	require.Nil(ts.T(), session.FactorID)
 
diff --git a/api/token_test.go b/api/token_test.go
index 0fefca40..acecc256 100644
--- a/api/token_test.go
+++ b/api/token_test.go
@@ -22,6 +22,7 @@ type TokenTestSuite struct {
 	Config *conf.GlobalConfiguration
 
 	RefreshToken *models.RefreshToken
+	User         *models.User
 }
 
 func TestToken(t *testing.T) {
@@ -50,6 +51,7 @@ func (ts *TokenTestSuite) SetupTest() {
 	u.BannedUntil = nil
 	require.NoError(ts.T(), ts.API.db.Create(u), "Error saving new test user")
 
+	ts.User = u
 	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, u, models.GrantParams{})
 	require.NoError(ts.T(), err, "Error creating refresh token")
 }
@@ -251,3 +253,49 @@ func (ts *TokenTestSuite) createBannedUser() *models.User {
 
 	return u
 }
+
+func (ts *TokenTestSuite) TestTokenRefreshWithExpiredSession() {
+	var err error
+
+	now := time.Now().UTC().Add(-1 * time.Second)
+
+	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, ts.User, models.GrantParams{
+		SessionNotAfter: &now,
+	})
+	require.NoError(ts.T(), err, "Error creating refresh token")
+
+	var buffer bytes.Buffer
+	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+		"refresh_token": ts.RefreshToken.Token,
+	}))
+
+	req := httptest.NewRequest(http.MethodPost, "http://localhost/token?grant_type=refresh_token", &buffer)
+	req.Header.Set("Content-Type", "application/json")
+
+	w := httptest.NewRecorder()
+	ts.API.handler.ServeHTTP(w, req)
+	assert.Equal(ts.T(), http.StatusBadRequest, w.Code)
+}
+
+func (ts *TokenTestSuite) TestTokenRefreshWithUnexpiredSession() {
+	var err error
+
+	now := time.Now().UTC().Add(1 * time.Second)
+
+	ts.RefreshToken, err = models.GrantAuthenticatedUser(ts.API.db, ts.User, models.GrantParams{
+		SessionNotAfter: &now,
+	})
+	require.NoError(ts.T(), err, "Error creating refresh token")
+
+	var buffer bytes.Buffer
+	require.NoError(ts.T(), json.NewEncoder(&buffer).Encode(map[string]interface{}{
+		"refresh_token": ts.RefreshToken.Token,
+	}))
+
+	req := httptest.NewRequest(http.MethodPost, "http://localhost/token?grant_type=refresh_token", &buffer)
+	req.Header.Set("Content-Type", "application/json")
+
+	w := httptest.NewRecorder()
+	ts.API.handler.ServeHTTP(w, req)
+	assert.Equal(ts.T(), http.StatusOK, w.Code)
+}
diff --git a/models/refresh_token_test.go b/models/refresh_token_test.go
index 6da738ba..4f3e6b16 100644
--- a/models/refresh_token_test.go
+++ b/models/refresh_token_test.go
@@ -52,7 +52,7 @@ func (ts *RefreshTokenTestSuite) TestGrantRefreshTokenSwap() {
 	s, err := GrantRefreshTokenSwap(&http.Request{}, ts.db, u, r)
 	require.NoError(ts.T(), err)
 
-	_, nr, err := FindUserWithRefreshToken(ts.db, r.Token)
+	_, nr, _, err := FindUserWithRefreshToken(ts.db, r.Token)
 	require.NoError(ts.T(), err)
 
 	require.Equal(ts.T(), r.ID, nr.ID)
@@ -68,7 +68,7 @@ func (ts *RefreshTokenTestSuite) TestLogout() {
 	require.NoError(ts.T(), err)
 
 	require.NoError(ts.T(), Logout(ts.db, u.ID))
-	u, r, err = FindUserWithRefreshToken(ts.db, r.Token)
+	u, r, _, err = FindUserWithRefreshToken(ts.db, r.Token)
 	require.Errorf(ts.T(), err, "expected error when there are no refresh tokens to authenticate. user: %v token: %v", u, r)
 
 	require.True(ts.T(), IsNotFoundError(err), "expected NotFoundError")
diff --git a/models/sessions_test.go b/models/sessions_test.go
index 7534e257..7e4eb647 100644
--- a/models/sessions_test.go
+++ b/models/sessions_test.go
@@ -1,7 +1,6 @@
 package models
 
 import (
-	"github.com/gofrs/uuid"
 	"github.com/netlify/gotrue/conf"
 	"github.com/netlify/gotrue/storage"
 	"github.com/netlify/gotrue/storage/test"
@@ -44,8 +43,10 @@ func (ts *SessionsTestSuite) TestCalculateAALAndAMR() {
 	totalDistinctClaims := 2
 	u, err := FindUserByEmailAndAudience(ts.db, "test@example.com", ts.Config.JWT.Aud)
 	require.NoError(ts.T(), err)
-	session, err := CreateSession(ts.db, u, &uuid.Nil)
+	session, err := NewSession()
 	require.NoError(ts.T(), err)
+	session.UserID = u.ID
+	require.NoError(ts.T(), ts.db.Create(session))
 
 	err = AddClaimToSession(ts.db, session, PasswordGrant)
 	require.NoError(ts.T(), err)
@@ -53,7 +54,7 @@ func (ts *SessionsTestSuite) TestCalculateAALAndAMR() {
 	firstClaimAddedTime := time.Now()
 	err = AddClaimToSession(ts.db, session, TOTPSignIn)
 	require.NoError(ts.T(), err)
-	session, err = FindSessionById(ts.db, session.ID)
+	session, err = FindSessionByID(ts.db, session.ID)
 	require.NoError(ts.T(), err)
 
 	aal, amr := session.CalculateAALAndAMR()
@@ -63,7 +64,7 @@ func (ts *SessionsTestSuite) TestCalculateAALAndAMR() {
 	err = AddClaimToSession(ts.db, session, TOTPSignIn)
 	require.NoError(ts.T(), err)
 
-	session, err = FindSessionById(ts.db, session.ID)
+	session, err = FindSessionByID(ts.db, session.ID)
 	require.NoError(ts.T(), err)
 
 	aal, amr = session.CalculateAALAndAMR()
diff --git a/models/user_test.go b/models/user_test.go
index e6de8c6d..db664faf 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -151,10 +151,12 @@ func (ts *UserTestSuite) TestFindUserWithRefreshToken() {
 	r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})
 	require.NoError(ts.T(), err)
 
-	n, nr, err := FindUserWithRefreshToken(ts.db, r.Token)
+	n, nr, s, err := FindUserWithRefreshToken(ts.db, r.Token)
 	require.NoError(ts.T(), err)
 	require.Equal(ts.T(), r.ID, nr.ID)
 	require.Equal(ts.T(), u.ID, n.ID)
+	require.NotNil(ts.T(), s)
+	require.Equal(ts.T(), *r.SessionId, s.ID)
 }
 
 func (ts *UserTestSuite) TestIsDuplicatedEmail() {