Skip to content

Commit dae9dfa

Browse files
ThomasK33ThomasK33
committed
feat: add cleanup for expired OAuth2 provider app codes and tokens
Change-Id: I07e7c229efa6e92282885464d2193dfc4c2e1c98 Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent e24c4b5 commit dae9dfa

File tree

8 files changed

+229
-0
lines changed

8 files changed

+229
-0
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,6 +1446,22 @@ func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCusto
14461446
return q.db.DeleteCustomRole(ctx, arg)
14471447
}
14481448

1449+
func (q *querier) DeleteExpiredOAuth2ProviderAppCodes(ctx context.Context) error {
1450+
// System operation - only system can clean up expired authorization codes
1451+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
1452+
return err
1453+
}
1454+
return q.db.DeleteExpiredOAuth2ProviderAppCodes(ctx)
1455+
}
1456+
1457+
func (q *querier) DeleteExpiredOAuth2ProviderAppTokens(ctx context.Context) error {
1458+
// System operation - only system can clean up expired access tokens
1459+
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
1460+
return err
1461+
}
1462+
return q.db.DeleteExpiredOAuth2ProviderAppTokens(ctx)
1463+
}
1464+
14491465
func (q *querier) DeleteExpiredOAuth2ProviderDeviceCodes(ctx context.Context) error {
14501466
// System operation - only system can clean up expired device codes
14511467
if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbpurge/dbpurge.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,15 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz.
6262
if err := tx.DeleteOldNotificationMessages(ctx); err != nil {
6363
return xerrors.Errorf("failed to delete old notification messages: %w", err)
6464
}
65+
if err := tx.DeleteExpiredOAuth2ProviderAppCodes(ctx); err != nil {
66+
return xerrors.Errorf("failed to delete expired oauth2 provider app codes: %w", err)
67+
}
68+
if err := tx.DeleteExpiredOAuth2ProviderAppTokens(ctx); err != nil {
69+
return xerrors.Errorf("failed to delete expired oauth2 provider app tokens: %w", err)
70+
}
71+
if err := tx.DeleteExpiredOAuth2ProviderDeviceCodes(ctx); err != nil {
72+
return xerrors.Errorf("failed to delete expired oauth2 provider device codes: %w", err)
73+
}
6574

6675
logger.Debug(ctx, "purged old database entries", slog.F("duration", clk.Since(start)))
6776

‎coderd/database/dbpurge/dbpurge_test.go

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,3 +490,135 @@ func containsProvisionerDaemon(daemons []database.ProvisionerDaemon, name string
490490
return d.Name == name
491491
})
492492
}
493+
494+
//nolint:paralleltest // It uses LockIDDBPurge.
495+
func TestDeleteExpiredOAuth2ProviderAppCodes(t *testing.T) {
496+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
497+
defer cancel()
498+
499+
clk := quartz.NewMock(t)
500+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
501+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
502+
503+
now := dbtime.Now()
504+
clk.Set(now).MustWait(ctx)
505+
506+
// Create test data
507+
user := dbgen.User(t, db, database.User{})
508+
app := dbgen.OAuth2ProviderApp(t, db, database.OAuth2ProviderApp{
509+
Name: fmt.Sprintf("test-codes-%d", time.Now().UnixNano()),
510+
})
511+
512+
// Create expired authorization code (should be deleted)
513+
expiredCode := dbgen.OAuth2ProviderAppCode(t, db, database.OAuth2ProviderAppCode{
514+
ExpiresAt: now.Add(-1 * time.Hour), // Expired 1 hour ago
515+
AppID: app.ID,
516+
UserID: user.ID,
517+
})
518+
519+
// Verify code exists initially
520+
_, err := db.GetOAuth2ProviderAppCodeByID(ctx, expiredCode.ID)
521+
require.NoError(t, err)
522+
523+
// Run cleanup
524+
done := awaitDoTick(ctx, t, clk)
525+
closer := dbpurge.New(ctx, logger, db, clk)
526+
defer closer.Close()
527+
<-done
528+
529+
// Verify expired code is deleted
530+
_, err = db.GetOAuth2ProviderAppCodeByID(ctx, expiredCode.ID)
531+
require.Error(t, err)
532+
require.ErrorIs(t, err, sql.ErrNoRows)
533+
}
534+
535+
//nolint:paralleltest // It uses LockIDDBPurge.
536+
func TestDeleteExpiredOAuth2ProviderAppTokens(t *testing.T) {
537+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
538+
defer cancel()
539+
540+
clk := quartz.NewMock(t)
541+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
542+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
543+
544+
now := dbtime.Now()
545+
clk.Set(now).MustWait(ctx)
546+
547+
// Create test data
548+
user := dbgen.User(t, db, database.User{})
549+
app := dbgen.OAuth2ProviderApp(t, db, database.OAuth2ProviderApp{
550+
Name: fmt.Sprintf("test-tokens-%d", time.Now().UnixNano()),
551+
})
552+
appSecret := dbgen.OAuth2ProviderAppSecret(t, db, database.OAuth2ProviderAppSecret{
553+
AppID: app.ID,
554+
})
555+
556+
// Create API keys for the tokens
557+
expiredAPIKey, _ := dbgen.APIKey(t, db, database.APIKey{
558+
UserID: user.ID,
559+
ExpiresAt: now.Add(-1 * time.Hour),
560+
})
561+
562+
// Create expired access token (should be deleted)
563+
expiredToken := dbgen.OAuth2ProviderAppToken(t, db, database.OAuth2ProviderAppToken{
564+
ExpiresAt: now.Add(-1 * time.Hour), // Expired 1 hour ago
565+
AppSecretID: appSecret.ID,
566+
APIKeyID: expiredAPIKey.ID,
567+
UserID: user.ID,
568+
})
569+
570+
// Verify token exists initially
571+
_, err := db.GetOAuth2ProviderAppTokenByPrefix(ctx, expiredToken.HashPrefix)
572+
require.NoError(t, err)
573+
574+
// Run cleanup
575+
done := awaitDoTick(ctx, t, clk)
576+
closer := dbpurge.New(ctx, logger, db, clk)
577+
defer closer.Close()
578+
<-done
579+
580+
// Verify expired token is deleted
581+
_, err = db.GetOAuth2ProviderAppTokenByPrefix(ctx, expiredToken.HashPrefix)
582+
require.Error(t, err)
583+
require.ErrorIs(t, err, sql.ErrNoRows)
584+
}
585+
586+
//nolint:paralleltest // It uses LockIDDBPurge.
587+
func TestDeleteExpiredOAuth2ProviderDeviceCodes(t *testing.T) {
588+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
589+
defer cancel()
590+
591+
clk := quartz.NewMock(t)
592+
db, _ := dbtestutil.NewDB(t, dbtestutil.WithDumpOnFailure())
593+
logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
594+
595+
now := dbtime.Now()
596+
clk.Set(now).MustWait(ctx)
597+
598+
// Create test data
599+
app := dbgen.OAuth2ProviderApp(t, db, database.OAuth2ProviderApp{
600+
Name: fmt.Sprintf("test-device-%d", time.Now().UnixNano()),
601+
})
602+
603+
// Create expired device code with pending status (should be deleted)
604+
expiredDeviceCode := dbgen.OAuth2ProviderDeviceCode(t, db, database.OAuth2ProviderDeviceCode{
605+
ExpiresAt: now.Add(-1 * time.Hour), // Expired 1 hour ago
606+
ClientID: app.ID,
607+
Status: database.OAuth2DeviceStatusPending,
608+
})
609+
610+
// Verify device code exists initially
611+
_, err := db.GetOAuth2ProviderDeviceCodeByID(ctx, expiredDeviceCode.ID)
612+
require.NoError(t, err)
613+
614+
// Run cleanup
615+
done := awaitDoTick(ctx, t, clk)
616+
closer := dbpurge.New(ctx, logger, db, clk)
617+
defer closer.Close()
618+
<-done
619+
620+
// Verify expired pending device code is deleted
621+
_, err = db.GetOAuth2ProviderDeviceCodeByID(ctx, expiredDeviceCode.ID)
622+
require.Error(t, err)
623+
require.ErrorIs(t, err, sql.ErrNoRows)
624+
}

‎coderd/database/querier.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/oauth2.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,14 @@ DELETE FROM oauth2_provider_device_codes WHERE id = $1;
307307
DELETE FROM oauth2_provider_device_codes
308308
WHERE expires_at < NOW() AND status = 'pending';
309309

310+
-- name: DeleteExpiredOAuth2ProviderAppCodes :exec
311+
DELETE FROM oauth2_provider_app_codes
312+
WHERE expires_at < NOW();
313+
314+
-- name: DeleteExpiredOAuth2ProviderAppTokens :exec
315+
DELETE FROM oauth2_provider_app_tokens
316+
WHERE expires_at < NOW();
317+
310318
-- name: GetOAuth2ProviderDeviceCodesByClientID :many
311319
SELECT * FROM oauth2_provider_device_codes
312320
WHERE client_id = $1

0 commit comments

Comments
 (0)