Skip to content

Commit cdfb025

Browse files
aeneasrory-bot
authored andcommitted
chore: explicitly skip errnorows errors in token rotation
GitOrigin-RevId: 936036ee4fea605d581ac81265ee9cf86587508e
1 parent c14e538 commit cdfb025

File tree

2 files changed

+95
-3
lines changed

2 files changed

+95
-3
lines changed

oauth2/fosite_store_helpers_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,84 @@ func testHelperRotateRefreshToken(x oauth2.InternalRegistry) func(t *testing.T)
529529
assert.ErrorIs(t, err, fosite.ErrInactiveToken, "Token is no longer active because it was refreshed")
530530
})
531531

532+
t.Run("Rotation works when access token is already pruned", func(t *testing.T) {
533+
// Test both with and without grace period
534+
testCases := []struct {
535+
name string
536+
configureGrace bool
537+
expectTokenActive bool
538+
}{
539+
{
540+
name: "with grace period",
541+
configureGrace: true,
542+
expectTokenActive: true,
543+
},
544+
{
545+
name: "without grace period",
546+
configureGrace: false,
547+
expectTokenActive: false,
548+
},
549+
}
550+
551+
for _, tc := range testCases {
552+
t.Run(tc.name, func(t *testing.T) {
553+
if tc.configureGrace {
554+
x.Config().MustSet(ctx, config.KeyRefreshTokenRotationGracePeriod, "1s")
555+
} else {
556+
x.Config().Delete(ctx, config.KeyRefreshTokenRotationGracePeriod)
557+
}
558+
t.Cleanup(func() {
559+
x.Config().Delete(ctx, config.KeyRefreshTokenRotationGracePeriod)
560+
})
561+
562+
m := x.OAuth2Storage()
563+
r := newDefaultRequest(uuid.New())
564+
565+
// Create tokens
566+
refreshTokenSession := fmt.Sprintf("refresh_token_%s", uuid.New())
567+
accessTokenSession := fmt.Sprintf("access_token_%s", uuid.New())
568+
569+
// Create access token
570+
err := m.CreateAccessTokenSession(ctx, accessTokenSession, &r)
571+
require.NoError(t, err)
572+
573+
// Create refresh token linked to the access token
574+
err = m.CreateRefreshTokenSession(ctx, refreshTokenSession, accessTokenSession, &r)
575+
require.NoError(t, err)
576+
577+
// Verify tokens were created successfully
578+
req, err := m.GetRefreshTokenSession(ctx, refreshTokenSession, nil)
579+
require.NoError(t, err)
580+
require.Equal(t, r.GetID(), req.GetID())
581+
582+
req, err = m.GetAccessTokenSession(ctx, accessTokenSession, nil)
583+
require.NoError(t, err)
584+
require.Equal(t, r.GetID(), req.GetID())
585+
586+
// Delete the access token (simulating it being pruned)
587+
err = m.DeleteAccessTokenSession(ctx, accessTokenSession)
588+
require.NoError(t, err)
589+
590+
// Verify access token is gone
591+
_, err = m.GetAccessTokenSession(ctx, accessTokenSession, nil)
592+
assert.Error(t, err)
593+
594+
// Rotation should still work even though the access token is gone
595+
err = m.RotateRefreshToken(ctx, r.GetID(), refreshTokenSession)
596+
require.NoError(t, err)
597+
598+
// Check refresh token state based on grace period configuration
599+
req, err = m.GetRefreshTokenSession(ctx, refreshTokenSession, nil)
600+
if tc.expectTokenActive {
601+
assert.NoError(t, err)
602+
assert.Equal(t, r.GetID(), req.GetID())
603+
} else {
604+
assert.ErrorIs(t, err, fosite.ErrInactiveToken, "Token should be inactive when no grace period is configured")
605+
}
606+
})
607+
}
608+
})
609+
532610
t.Run("refresh token is valid until the grace period has ended", func(t *testing.T) {
533611
x.Config().MustSet(ctx, config.KeyRefreshTokenRotationGracePeriod, "1s")
534612

persistence/sql/persister_oauth2.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,9 @@ func (p *Persister) strictRefreshRotation(ctx context.Context, requestID string)
666666

667667
// In strict rotation we only have one token chain for every request. Therefore, we remove all
668668
// access tokens associated with the request ID.
669-
if err := p.deleteSessionByRequestID(ctx, requestID, sqlTableAccess); err != nil {
669+
if err := p.deleteSessionByRequestID(ctx, requestID, sqlTableAccess); errors.Is(err, fosite.ErrNotFound) {
670+
return nil // Tokens may have been pruned earlier, so we do not return an error here.
671+
} else if err != nil {
670672
return err
671673
}
672674

@@ -739,11 +741,23 @@ WHERE signature = ? AND nid = ?`
739741

740742
if !accessTokenSignature.Valid {
741743
// If the access token is not found, we fall back to deleting all access tokens associated with the request ID.
742-
return p.deleteSessionByRequestID(ctx, requestID, sqlTableAccess)
744+
if err := p.deleteSessionByRequestID(ctx, requestID, sqlTableAccess); errors.Is(err, fosite.ErrNotFound) {
745+
// Tokens may have been pruned earlier, so we do not return an error here.
746+
return nil
747+
} else if err != nil {
748+
return err
749+
}
743750
}
744751

745752
// We have the signature and we will only remove that specific access token as part of the rotation.
746-
return p.deleteSessionBySignature(ctx, accessTokenSignature.String, sqlTableAccess)
753+
if err := p.deleteSessionBySignature(ctx, accessTokenSignature.String, sqlTableAccess); errors.Is(err, fosite.ErrNotFound) {
754+
// Tokens may have been pruned earlier, so we do not return an error here.
755+
return nil
756+
} else if err != nil {
757+
return err
758+
}
759+
760+
return nil
747761
}
748762

749763
func (p *Persister) RotateRefreshToken(ctx context.Context, requestID, refreshTokenSignature string) (err error) {

0 commit comments

Comments
 (0)