Skip to content

Commit 0588744

Browse files
aeneasrory-bot
aeneasr
authored and
ory-bot
committed
fix(changelog-oel): update expires_at on token use
GitOrigin-RevId: c4ea129061ba34aaae5ed63403ee32221aee5556
1 parent a0a9062 commit 0588744

File tree

3 files changed

+221
-23
lines changed

3 files changed

+221
-23
lines changed

‎oauth2/fosite_store_helpers_test.go‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,3 +1500,188 @@ func createTestRequest(id string) *fosite.Request {
15001500
Session: &oauth2.Session{DefaultSession: &openid.DefaultSession{Subject: "bar"}},
15011501
}
15021502
}
1503+
1504+
func testHelperRefreshTokenExpiryUpdate(x oauth2.InternalRegistry) func(t *testing.T) {
1505+
return func(t *testing.T) {
1506+
ctx := context.Background()
1507+
1508+
// Create client
1509+
cl := &client.Client{ID: "refresh-expiry-client"}
1510+
require.NoError(t, x.ClientManager().CreateClient(ctx, cl))
1511+
1512+
// Create a request with a long expiry
1513+
initialRequest := fosite.Request{
1514+
ID: uuid.New(),
1515+
RequestedAt: time.Now().UTC().Round(time.Second),
1516+
Client: cl,
1517+
Session: oauth2.NewSession("sub"),
1518+
}
1519+
1520+
// Set a long expiry time (24 hours)
1521+
initialExpiry := time.Now().Add(24 * time.Hour)
1522+
initialRequest.Session.SetExpiresAt(fosite.RefreshToken, initialExpiry)
1523+
1524+
t.Run("regular rotation", func(t *testing.T) {
1525+
// Create original refresh token
1526+
regularSignature := uuid.New()
1527+
require.NoError(t, x.OAuth2Storage().CreateRefreshTokenSession(ctx, regularSignature, "", &initialRequest))
1528+
1529+
// Verify initial expiry is set correctly
1530+
originalToken, err := x.OAuth2Storage().GetRefreshTokenSession(ctx, regularSignature, oauth2.NewSession("sub"))
1531+
require.NoError(t, err)
1532+
require.Equal(t, initialExpiry.Unix(), originalToken.GetSession().GetExpiresAt(fosite.RefreshToken).Unix())
1533+
1534+
// Set up a connection to directly query the database
1535+
var actualExpiresAt time.Time
1536+
require.NoError(t, x.Persister().Connection(ctx).RawQuery("SELECT expires_at FROM hydra_oauth2_refresh WHERE signature=?", regularSignature).First(&actualExpiresAt))
1537+
require.Equal(t, initialExpiry.UTC().Round(time.Second), actualExpiresAt.UTC().Round(time.Second))
1538+
1539+
// Rotate the token
1540+
err = x.OAuth2Storage().RotateRefreshToken(ctx, initialRequest.ID, regularSignature)
1541+
require.NoError(t, err)
1542+
1543+
// Check that the original token's expiry was updated to be closer to now
1544+
var revokedData struct {
1545+
ExpiresAt time.Time `db:"expires_at"`
1546+
Active bool `db:"active"`
1547+
}
1548+
require.NoError(t, x.Persister().Connection(ctx).RawQuery("SELECT expires_at, active FROM hydra_oauth2_refresh WHERE signature=?", regularSignature).First(&revokedData))
1549+
1550+
// Verify the token is now inactive
1551+
require.False(t, revokedData.Active)
1552+
1553+
// Verify the expiry is updated to be closer to now than the original expiry
1554+
require.True(t, revokedData.ExpiresAt.Before(initialExpiry), "Expiry should be updated to be sooner than original")
1555+
require.True(t, revokedData.ExpiresAt.After(time.Now()), "Expiry should still be in the future")
1556+
require.True(t, time.Until(revokedData.ExpiresAt) < time.Until(initialExpiry), "New expiry should be closer to now than original expiry")
1557+
1558+
t.Logf("Original expiry: %v, Updated expiry: %v, Now: %v", initialExpiry, revokedData.ExpiresAt, time.Now())
1559+
})
1560+
1561+
t.Run("graceful rotation", func(t *testing.T) {
1562+
// Create refresh token for graceful rotation
1563+
gracefulSignature := uuid.New()
1564+
require.NoError(t, x.OAuth2Storage().CreateRefreshTokenSession(ctx, gracefulSignature, "", &initialRequest))
1565+
1566+
// Set config to graceful rotation
1567+
oldPeriod := x.Config().GracefulRefreshTokenRotation(ctx).Period
1568+
t.Cleanup(func() {
1569+
x.Config().MustSet(ctx, config.KeyRefreshTokenRotationGracePeriod, oldPeriod)
1570+
x.Config().MustSet(ctx, config.KeyRefreshTokenRotationGraceReuseCount, 0)
1571+
})
1572+
x.Config().MustSet(ctx, config.KeyRefreshTokenRotationGracePeriod, time.Minute*30)
1573+
x.Config().MustSet(ctx, config.KeyRefreshTokenRotationGraceReuseCount, 3)
1574+
1575+
// Record time before rotation
1576+
beforeRotation := time.Now().UTC().Add(-time.Second) // Ensure we have a different timestamp for first_used_at
1577+
1578+
// Rotate the token
1579+
err := x.OAuth2Storage().RotateRefreshToken(ctx, initialRequest.ID, gracefulSignature)
1580+
require.NoError(t, err)
1581+
1582+
// Check the token's expiry and status
1583+
var rotatedData struct {
1584+
ExpiresAt time.Time `db:"expires_at"`
1585+
Active bool `db:"active"`
1586+
FirstUsedAt sqlxx.NullTime `db:"first_used_at"`
1587+
UsedTimes sqlxx.NullInt64 `db:"used_times"`
1588+
}
1589+
require.NoError(t, x.Persister().Connection(ctx).RawQuery("SELECT expires_at, active, first_used_at, used_times FROM hydra_oauth2_refresh WHERE signature=?", gracefulSignature).First(&rotatedData))
1590+
1591+
// Token is used
1592+
require.False(t, rotatedData.Active)
1593+
1594+
// Verify first_used_at is set and reasonable
1595+
assert.True(t, time.Time(rotatedData.FirstUsedAt).After(beforeRotation) || time.Time(rotatedData.FirstUsedAt).Equal(beforeRotation), "%s should be after or equal to %s", time.Time(rotatedData.FirstUsedAt), beforeRotation)
1596+
1597+
now := time.Now().UTC().Add(time.Second)
1598+
assert.True(t, time.Time(rotatedData.FirstUsedAt).Before(now) || time.Time(rotatedData.FirstUsedAt).Equal(now), "%s should be before or equal to %s", time.Time(rotatedData.FirstUsedAt), now)
1599+
1600+
// Verify used_times was incremented
1601+
assert.True(t, rotatedData.UsedTimes.Valid)
1602+
assert.Equal(t, int64(1), rotatedData.UsedTimes.Int)
1603+
1604+
// Verify the expiry is updated and is in the future
1605+
assert.True(t, rotatedData.ExpiresAt.Before(initialExpiry), "Expiry should be updated to be sooner than original")
1606+
assert.True(t, rotatedData.ExpiresAt.After(time.Now().UTC()), "Expiry should still be in the future")
1607+
assert.True(t, time.Until(rotatedData.ExpiresAt) < time.Until(initialExpiry), "New expiry should be closer to now than original expiry")
1608+
1609+
t.Logf("Original expiry: %v, Updated expiry: %v, Now: %v", initialExpiry, rotatedData.ExpiresAt, time.Now())
1610+
})
1611+
}
1612+
}
1613+
1614+
func testHelperAuthorizeCodeInvalidation(x oauth2.InternalRegistry) func(t *testing.T) {
1615+
return func(t *testing.T) {
1616+
ctx := context.Background()
1617+
1618+
// Create client
1619+
cl := &client.Client{ID: "auth-code-client"}
1620+
require.NoError(t, x.ClientManager().CreateClient(ctx, cl))
1621+
1622+
// Create a request with a long expiry
1623+
initialRequest := fosite.Request{
1624+
ID: uuid.New(),
1625+
RequestedAt: time.Now().UTC().Round(time.Second),
1626+
Client: cl,
1627+
Session: oauth2.NewSession("sub"),
1628+
}
1629+
1630+
// Set a long expiry time (1 hour)
1631+
initialExpiry := time.Now().Add(1 * time.Hour)
1632+
initialRequest.Session.SetExpiresAt(fosite.AuthorizeCode, initialExpiry)
1633+
1634+
// Create authorize code session
1635+
authCodeSignature := uuid.New()
1636+
require.NoError(t, x.OAuth2Storage().CreateAuthorizeCodeSession(ctx, authCodeSignature, &initialRequest))
1637+
1638+
// Verify initial state
1639+
originalCode, err := x.OAuth2Storage().GetAuthorizeCodeSession(ctx, authCodeSignature, oauth2.NewSession("sub"))
1640+
require.NoError(t, err)
1641+
require.Equal(t, initialExpiry.Unix(), originalCode.GetSession().GetExpiresAt(fosite.AuthorizeCode).Unix())
1642+
1643+
// Check database directly
1644+
var codeData struct {
1645+
ExpiresAt time.Time `db:"expires_at"`
1646+
Active bool `db:"active"`
1647+
}
1648+
require.NoError(t, x.Persister().Connection(ctx).RawQuery(
1649+
"SELECT expires_at, active FROM hydra_oauth2_code WHERE signature=?",
1650+
authCodeSignature).First(&codeData))
1651+
require.Equal(t, initialExpiry.UTC().Round(time.Second), codeData.ExpiresAt.UTC().Round(time.Second))
1652+
require.True(t, codeData.Active)
1653+
1654+
// Invalidate the code
1655+
err = x.OAuth2Storage().InvalidateAuthorizeCodeSession(ctx, authCodeSignature)
1656+
require.NoError(t, err)
1657+
1658+
// Check that the code was invalidated but is still retrievable
1659+
invalidatedCode, err := x.OAuth2Storage().GetAuthorizeCodeSession(ctx, authCodeSignature, oauth2.NewSession("sub"))
1660+
require.Error(t, err)
1661+
require.ErrorIs(t, err, fosite.ErrInvalidatedAuthorizeCode)
1662+
require.NotNil(t, invalidatedCode) // Should still be retrievable
1663+
1664+
// Verify database state after invalidation
1665+
var invalidatedData struct {
1666+
ExpiresAt time.Time `db:"expires_at"`
1667+
Active bool `db:"active"`
1668+
}
1669+
require.NoError(t, x.Persister().Connection(ctx).RawQuery(
1670+
"SELECT expires_at, active FROM hydra_oauth2_code WHERE signature=?",
1671+
authCodeSignature).First(&invalidatedData))
1672+
1673+
// Verify the code is now inactive
1674+
require.False(t, invalidatedData.Active)
1675+
1676+
// Verify the expiry is updated to be closer to now than the original expiry
1677+
require.True(t, invalidatedData.ExpiresAt.Before(initialExpiry),
1678+
"Expiry should be updated to be sooner than original")
1679+
require.True(t, invalidatedData.ExpiresAt.After(time.Now()),
1680+
"Expiry should still be in the future")
1681+
require.True(t, time.Until(invalidatedData.ExpiresAt) < time.Until(initialExpiry),
1682+
"New expiry should be closer to now than original expiry")
1683+
1684+
t.Logf("Original expiry: %v, Updated expiry: %v, Now: %v",
1685+
initialExpiry, invalidatedData.ExpiresAt, time.Now())
1686+
}
1687+
}

‎oauth2/fosite_store_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ func TestManagers(t *testing.T) {
4040
t.Run("suite="+tc.name, func(t *testing.T) {
4141
for k, r := range testhelpers.ConnectDatabases(t, false) {
4242
t.Run("database="+k, func(t *testing.T) {
43-
t.Parallel()
44-
4543
store := testhelpers.NewRegistrySQLFromURL(t, r.Config().DSN(), true, &contextx.Default{})
4644
store.Config().MustSet(ctx, config.KeyEncryptSessionData, tc.enableSessionEncrypted)
4745

@@ -75,6 +73,8 @@ func TestManagers(t *testing.T) {
7573
t.Run("testHelperRevokeAccessToken", testHelperRevokeAccessToken(store))
7674
t.Run("testFositeJWTBearerGrantStorage", testFositeJWTBearerGrantStorage(store))
7775
t.Run("testHelperRotateRefreshToken", testHelperRotateRefreshToken(store))
76+
t.Run("testHelperRefreshTokenExpiryUpdate", testHelperRefreshTokenExpiryUpdate(store))
77+
t.Run("testHelperAuthorizeCodeInvalidation", testHelperAuthorizeCodeInvalidation(store))
7878
})
7979
}
8080
})

‎persistence/sql/persister_oauth2.go‎

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -321,22 +321,6 @@ func (p *Persister) deleteSessionByRequestID(ctx context.Context, id string, tab
321321
return nil
322322
}
323323

324-
func (p *Persister) deactivateSessionByRequestID(ctx context.Context, id string, table tableName) (err error) {
325-
ctx, span := p.r.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.deactivateSessionByRequestID")
326-
defer otelx.End(span, &err)
327-
328-
/* #nosec G201 table is static */
329-
return sqlcon.HandleError(
330-
p.Connection(ctx).
331-
RawQuery(
332-
fmt.Sprintf("UPDATE %s SET active=false WHERE request_id=? AND nid = ? AND active=true", OAuth2RequestSQL{Table: table}.TableName()),
333-
id,
334-
p.NetworkID(ctx),
335-
).
336-
Exec(),
337-
)
338-
}
339-
340324
func (p *Persister) CreateAuthorizeCodeSession(ctx context.Context, signature string, requester fosite.Requester) error {
341325
return otelx.WithSpan(ctx, "persistence.sql.CreateAuthorizeCodeSession", func(ctx context.Context) error {
342326
return p.createSession(ctx, signature, requester, sqlTableCode, requester.GetSession().GetExpiresAt(fosite.AuthorizeCode).UTC())
@@ -358,7 +342,13 @@ func (p *Persister) InvalidateAuthorizeCodeSession(ctx context.Context, signatur
358342
return sqlcon.HandleError(
359343
p.Connection(ctx).
360344
RawQuery(
361-
fmt.Sprintf("UPDATE %s SET active = false WHERE signature = ? AND nid = ?", OAuth2RequestSQL{Table: sqlTableCode}.TableName()),
345+
fmt.Sprintf(
346+
"UPDATE %s SET active = false, expires_at = ? WHERE signature = ? AND nid = ?",
347+
OAuth2RequestSQL{Table: sqlTableCode}.TableName(),
348+
),
349+
// We don't expire immediately, but in 30 minutes to avoid prematurely removing
350+
// rows while they may still be needed (e.g. for reuse detection).
351+
newUsedExpiry(),
362352
signature,
363353
p.NetworkID(ctx),
364354
).
@@ -674,7 +664,10 @@ func (p *Persister) strictRefreshRotation(ctx context.Context, requestID string)
674664

675665
// The same applies to refresh tokens in strict mode. We disable all old refresh tokens when rotating.
676666
count, err := c.RawQuery(
677-
"UPDATE hydra_oauth2_refresh SET active=false WHERE request_id=? AND nid = ? AND active",
667+
"UPDATE hydra_oauth2_refresh SET active=false, expires_at = ? WHERE request_id=? AND nid = ? AND active",
668+
// We don't expire immediately, but in 30 minutes to avoid prematurely removing
669+
// rows while they may still be needed (e.g. for reuse detection).
670+
newUsedExpiry(),
678671
requestID,
679672
p.NetworkID(ctx),
680673
).ExecWithCount()
@@ -696,15 +689,29 @@ func (p *Persister) gracefulRefreshRotation(ctx context.Context, requestID, refr
696689

697690
c := p.Connection(ctx)
698691
now := time.Now().UTC().Round(time.Millisecond)
692+
// The new expiry of the token starts now and ends at the end of the graceful token period.
693+
// After that, we can prune tokens from the store.
694+
expiresAt := newUsedExpiry().Add(p.r.Config().GracefulRefreshTokenRotation(ctx).Period)
699695

700696
// Signature is the primary key so no limit needed. We only update first_used_at if it is not set yet (otherwise
701697
// we would "refresh" the grace period again and again, and the refresh token would never "expire").
702698
query := `
703699
UPDATE hydra_oauth2_refresh
704-
SET active=false, first_used_at = COALESCE(first_used_at, ?), used_times = COALESCE(used_times, 0) + 1
700+
SET
701+
active=false,
702+
first_used_at = COALESCE(first_used_at, ?),
703+
used_times = COALESCE(used_times, 0) + 1,
704+
expires_at = ?
705705
WHERE signature = ? AND nid = ?`
706-
args := make([]any, 3, 4)
707-
args[0], args[1], args[2] = now, refreshSignature, p.NetworkID(ctx)
706+
707+
args := make([]any, 0, 5)
708+
args = append(args,
709+
now,
710+
expiresAt,
711+
refreshSignature,
712+
p.NetworkID(ctx),
713+
)
714+
708715
if l := p.r.Config().GracefulRefreshTokenRotation(ctx).Count; l > 0 {
709716
query += " AND (used_times IS NULL OR used_times < ?)"
710717
args = append(args, l)
@@ -771,3 +778,9 @@ func (p *Persister) RotateRefreshToken(ctx context.Context, requestID, refreshTo
771778

772779
return handleRetryError(p.strictRefreshRotation(ctx, requestID))
773780
}
781+
782+
func newUsedExpiry() time.Time {
783+
// Reuse detection is racy and would generally happen within seconds. Using 30 minutes here is a paranoid
784+
// setting but ensures that we do not prematurely remove rows while they may still be needed (e.g. for reuse detection).
785+
return time.Now().UTC().Round(time.Millisecond).Add(time.Minute * 30)
786+
}

0 commit comments

Comments
 (0)