Skip to content

Commit 8748000

Browse files
authored
fix: ensure legacy refresh token revoked (#2624)
## What kind of change does this PR introduce? Bug fix ## What is the current behavior? FindTokenBySessionID did not exclude revoked tokens from its lookup. Allowing a refresh token to be reused when switching legacy refresh tokensduring an MFA session update ## What is the new behavior? The revoked status of the legacy refresh token is checked when doing MFA upgrade
1 parent 94460b9 commit 8748000

3 files changed

Lines changed: 45 additions & 3 deletions

File tree

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ require (
186186
gopkg.in/yaml.v3 v3.0.1 // indirect
187187
)
188188

189-
go 1.25.11
189+
go 1.25.12
190190

191191
replace (
192192
github.com/joho/godotenv => ./internal/forks/godotenv
193-
)
193+
)

internal/models/refresh_token.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func RevokeTokenFamily(tx *storage.Connection, token *RefreshToken) error {
110110

111111
func FindTokenBySessionID(tx *storage.Connection, sessionId *uuid.UUID) (*RefreshToken, error) {
112112
refreshToken := &RefreshToken{}
113-
err := tx.Q().Where("instance_id = ? and session_id = ?", uuid.Nil, sessionId).Order("created_at asc").First(refreshToken)
113+
err := tx.Q().Where("instance_id = ? and session_id = ? and revoked = false", uuid.Nil, sessionId).Order("created_at asc").First(refreshToken)
114114
if err != nil {
115115
if errors.Cause(err) == sql.ErrNoRows {
116116
return nil, RefreshTokenNotFoundError{}

internal/models/refresh_token_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,48 @@ func (ts *RefreshTokenTestSuite) TestGrantRefreshTokenSwap() {
6767
require.Equal(ts.T(), u.ID, s.UserID)
6868
}
6969

70+
func (ts *RefreshTokenTestSuite) TestFindTokenBySessionID() {
71+
u := ts.createUser()
72+
r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})
73+
require.NoError(ts.T(), err)
74+
75+
found, err := FindTokenBySessionID(ts.db, r.SessionId)
76+
require.NoError(ts.T(), err)
77+
78+
require.Equal(ts.T(), r.ID, found.ID)
79+
require.Equal(ts.T(), r.Token, found.Token)
80+
require.False(ts.T(), found.Revoked)
81+
}
82+
83+
func (ts *RefreshTokenTestSuite) TestFindTokenBySessionIDExcludesRevokedToken() {
84+
u := ts.createUser()
85+
r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})
86+
require.NoError(ts.T(), err)
87+
88+
s, err := GrantRefreshTokenSwap(ts.config.AuditLog, &http.Request{}, ts.db, u, r)
89+
require.NoError(ts.T(), err)
90+
91+
found, err := FindTokenBySessionID(ts.db, r.SessionId)
92+
require.NoError(ts.T(), err)
93+
94+
require.Equal(ts.T(), s.ID, found.ID, "expected the active (post-swap) token, not the revoked one")
95+
require.NotEqual(ts.T(), r.ID, found.ID)
96+
require.False(ts.T(), found.Revoked)
97+
}
98+
99+
func (ts *RefreshTokenTestSuite) TestFindTokenBySessionIDNotFoundWhenAllRevoked() {
100+
u := ts.createUser()
101+
r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})
102+
require.NoError(ts.T(), err)
103+
104+
r.Revoked = true
105+
require.NoError(ts.T(), ts.db.UpdateOnly(r, "revoked"))
106+
107+
_, err = FindTokenBySessionID(ts.db, r.SessionId)
108+
require.Error(ts.T(), err)
109+
require.True(ts.T(), IsNotFoundError(err), "expected NotFoundError")
110+
}
111+
70112
func (ts *RefreshTokenTestSuite) TestLogout() {
71113
u := ts.createUser()
72114
r, err := GrantAuthenticatedUser(ts.db, u, GrantParams{})

0 commit comments

Comments
 (0)