Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0393465

Browse files
committed
feat: add cleanup for expired OAuth2 provider app codes and tokens
Change-Id: I07e7c229efa6e92282885464d2193dfc4c2e1c98Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent962c22c commit0393465

File tree

9 files changed

+318
-2
lines changed

9 files changed

+318
-2
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
returnq.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+
iferr:=q.authorizeContext(ctx,policy.ActionDelete,rbac.ResourceSystem);err!=nil {
1452+
returnerr
1453+
}
1454+
returnq.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+
iferr:=q.authorizeContext(ctx,policy.ActionDelete,rbac.ResourceSystem);err!=nil {
1460+
returnerr
1461+
}
1462+
returnq.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
iferr:=q.authorizeContext(ctx,policy.ActionDelete,rbac.ResourceSystem);err!=nil {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5460,6 +5460,9 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppCodes() {
54605460
})
54615461
check.Args(code.SecretPrefix).Asserts(code,policy.ActionUpdate).Returns(code)
54625462
}))
5463+
s.Run("DeleteExpiredOAuth2ProviderAppCodes",s.Subtest(func(db database.Store,check*expects) {
5464+
check.Args().Asserts(rbac.ResourceSystem,policy.ActionDelete)
5465+
}))
54635466
}
54645467

54655468
func (s*MethodTestSuite)TestOAuth2ProviderAppTokens() {
@@ -5533,6 +5536,9 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppTokens() {
55335536
UserID:user.ID,
55345537
}).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()),policy.ActionDelete)
55355538
}))
5539+
s.Run("DeleteExpiredOAuth2ProviderAppTokens",s.Subtest(func(db database.Store,check*expects) {
5540+
check.Args().Asserts(rbac.ResourceSystem,policy.ActionDelete)
5541+
}))
55365542
}
55375543

55385544
func (s*MethodTestSuite)TestOAuth2ProviderDeviceCodes() {

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing 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 aboutcustomizing 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
iferr:=tx.DeleteOldNotificationMessages(ctx);err!=nil {
6363
returnxerrors.Errorf("failed to delete old notification messages: %w",err)
6464
}
65+
iferr:=tx.DeleteExpiredOAuth2ProviderAppCodes(ctx);err!=nil {
66+
returnxerrors.Errorf("failed to delete expired oauth2 provider app codes: %w",err)
67+
}
68+
iferr:=tx.DeleteExpiredOAuth2ProviderAppTokens(ctx);err!=nil {
69+
returnxerrors.Errorf("failed to delete expired oauth2 provider app tokens: %w",err)
70+
}
71+
iferr:=tx.DeleteExpiredOAuth2ProviderDeviceCodes(ctx);err!=nil {
72+
returnxerrors.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: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,3 +490,216 @@ func containsProvisionerDaemon(daemons []database.ProvisionerDaemon, name string
490490
returnd.Name==name
491491
})
492492
}
493+
494+
//nolint:paralleltest // It uses LockIDDBPurge.
495+
funcTestDeleteExpiredOAuth2ProviderAppCodes(t*testing.T) {
496+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
497+
defercancel()
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+
SecretPrefix: []byte(fmt.Sprintf("expired-%d",time.Now().UnixNano())),
518+
})
519+
520+
// Create non-expired authorization code (should be retained)
521+
validCode:=dbgen.OAuth2ProviderAppCode(t,db, database.OAuth2ProviderAppCode{
522+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
523+
AppID:app.ID,
524+
UserID:user.ID,
525+
SecretPrefix: []byte(fmt.Sprintf("valid-%d",time.Now().UnixNano())),
526+
})
527+
528+
// Verify codes exist initially
529+
_,err:=db.GetOAuth2ProviderAppCodeByID(ctx,expiredCode.ID)
530+
require.NoError(t,err)
531+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,validCode.ID)
532+
require.NoError(t,err)
533+
534+
// Run cleanup
535+
done:=awaitDoTick(ctx,t,clk)
536+
closer:=dbpurge.New(ctx,logger,db,clk)
537+
defercloser.Close()
538+
<-done
539+
540+
// Verify expired code is deleted
541+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,expiredCode.ID)
542+
require.Error(t,err)
543+
require.ErrorIs(t,err,sql.ErrNoRows)
544+
545+
// Verify non-expired code is retained
546+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,validCode.ID)
547+
require.NoError(t,err)
548+
}
549+
550+
//nolint:paralleltest // It uses LockIDDBPurge.
551+
funcTestDeleteExpiredOAuth2ProviderAppTokens(t*testing.T) {
552+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
553+
defercancel()
554+
555+
clk:=quartz.NewMock(t)
556+
db,_:=dbtestutil.NewDB(t,dbtestutil.WithDumpOnFailure())
557+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true})
558+
559+
now:=dbtime.Now()
560+
clk.Set(now).MustWait(ctx)
561+
562+
// Create test data
563+
user:=dbgen.User(t,db, database.User{})
564+
app:=dbgen.OAuth2ProviderApp(t,db, database.OAuth2ProviderApp{
565+
Name:fmt.Sprintf("test-tokens-%d",time.Now().UnixNano()),
566+
})
567+
appSecret:=dbgen.OAuth2ProviderAppSecret(t,db, database.OAuth2ProviderAppSecret{
568+
AppID:app.ID,
569+
})
570+
571+
// Create API keys for the tokens
572+
expiredAPIKey,_:=dbgen.APIKey(t,db, database.APIKey{
573+
UserID:user.ID,
574+
ExpiresAt:now.Add(-1*time.Hour),
575+
})
576+
validAPIKey,_:=dbgen.APIKey(t,db, database.APIKey{
577+
UserID:user.ID,
578+
ExpiresAt:now.Add(24*time.Hour),// Valid for 24 hours
579+
})
580+
581+
// Create expired access token (should be deleted)
582+
expiredToken:=dbgen.OAuth2ProviderAppToken(t,db, database.OAuth2ProviderAppToken{
583+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
584+
AppSecretID:appSecret.ID,
585+
APIKeyID:expiredAPIKey.ID,
586+
UserID:user.ID,
587+
HashPrefix: []byte(fmt.Sprintf("expired-%d",time.Now().UnixNano())),
588+
})
589+
590+
// Create non-expired access token (should be retained)
591+
validToken:=dbgen.OAuth2ProviderAppToken(t,db, database.OAuth2ProviderAppToken{
592+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
593+
AppSecretID:appSecret.ID,
594+
APIKeyID:validAPIKey.ID,
595+
UserID:user.ID,
596+
HashPrefix: []byte(fmt.Sprintf("valid-%d",time.Now().UnixNano())),
597+
})
598+
599+
// Verify tokens exist initially
600+
_,err:=db.GetOAuth2ProviderAppTokenByPrefix(ctx,expiredToken.HashPrefix)
601+
require.NoError(t,err)
602+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,validToken.HashPrefix)
603+
require.NoError(t,err)
604+
605+
// Run cleanup
606+
done:=awaitDoTick(ctx,t,clk)
607+
closer:=dbpurge.New(ctx,logger,db,clk)
608+
defercloser.Close()
609+
<-done
610+
611+
// Verify expired token is deleted
612+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,expiredToken.HashPrefix)
613+
require.Error(t,err)
614+
require.ErrorIs(t,err,sql.ErrNoRows)
615+
616+
// Verify non-expired token is retained
617+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,validToken.HashPrefix)
618+
require.NoError(t,err)
619+
}
620+
621+
//nolint:paralleltest // It uses LockIDDBPurge.
622+
funcTestDeleteExpiredOAuth2ProviderDeviceCodes(t*testing.T) {
623+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
624+
defercancel()
625+
626+
clk:=quartz.NewMock(t)
627+
db,_:=dbtestutil.NewDB(t,dbtestutil.WithDumpOnFailure())
628+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true})
629+
630+
now:=dbtime.Now()
631+
clk.Set(now).MustWait(ctx)
632+
633+
// Create test data
634+
app:=dbgen.OAuth2ProviderApp(t,db, database.OAuth2ProviderApp{
635+
Name:fmt.Sprintf("test-device-%d",time.Now().UnixNano()),
636+
})
637+
638+
nanoTime:=time.Now().UnixNano()
639+
640+
// Create expired device code with pending status (should be deleted)
641+
expiredPendingCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
642+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
643+
ClientID:app.ID,
644+
Status:database.OAuth2DeviceStatusPending,
645+
DeviceCodePrefix:fmt.Sprintf("EP%06d",nanoTime%1000000),
646+
UserCode:fmt.Sprintf("EP%06d",nanoTime%1000000),
647+
DeviceCodeHash:fmt.Appendf(nil,"hash-exp-pending-%d",nanoTime),
648+
})
649+
650+
// Create non-expired device code with pending status (should be retained)
651+
validPendingCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
652+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
653+
ClientID:app.ID,
654+
Status:database.OAuth2DeviceStatusPending,
655+
DeviceCodePrefix:fmt.Sprintf("VP%06d", (nanoTime+1)%1000000),
656+
UserCode:fmt.Sprintf("VP%06d", (nanoTime+1)%1000000),
657+
DeviceCodeHash:fmt.Appendf(nil,"hash-val-pending-%d",nanoTime+1),
658+
})
659+
660+
// Create expired device code with authorized status (should be deleted - all expired codes are deleted)
661+
expiredAuthorizedCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
662+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
663+
ClientID:app.ID,
664+
DeviceCodePrefix:fmt.Sprintf("EA%06d", (nanoTime+2)%1000000),
665+
UserCode:fmt.Sprintf("EA%06d", (nanoTime+2)%1000000),
666+
DeviceCodeHash:fmt.Appendf(nil,"hash-exp-auth-%d",nanoTime+2),
667+
})
668+
669+
// Create a user and authorize the device code
670+
user:=dbgen.User(t,db, database.User{})
671+
expiredAuthorizedCode,err:=db.UpdateOAuth2ProviderDeviceCodeAuthorization(ctx, database.UpdateOAuth2ProviderDeviceCodeAuthorizationParams{
672+
ID:expiredAuthorizedCode.ID,
673+
UserID: uuid.NullUUID{UUID:user.ID,Valid:true},
674+
Status:database.OAuth2DeviceStatusAuthorized,
675+
})
676+
require.NoError(t,err)
677+
678+
// Verify device codes exist initially
679+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredPendingCode.ID)
680+
require.NoError(t,err)
681+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,validPendingCode.ID)
682+
require.NoError(t,err)
683+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredAuthorizedCode.ID)
684+
require.NoError(t,err)
685+
686+
// Run cleanup
687+
done:=awaitDoTick(ctx,t,clk)
688+
closer:=dbpurge.New(ctx,logger,db,clk)
689+
defercloser.Close()
690+
<-done
691+
692+
// Verify expired pending device code is deleted
693+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredPendingCode.ID)
694+
require.Error(t,err)
695+
require.ErrorIs(t,err,sql.ErrNoRows)
696+
697+
// Verify non-expired pending device code is retained
698+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,validPendingCode.ID)
699+
require.NoError(t,err)
700+
701+
// Verify expired authorized device code is deleted (all expired codes are deleted)
702+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredAuthorizedCode.ID)
703+
require.Error(t,err)
704+
require.ErrorIs(t,err,sql.ErrNoRows)
705+
}

‎coderd/database/querier.go

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

‎coderd/database/queries.sql.go

Lines changed: 21 additions & 1 deletion
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/database/queries/oauth2.sql

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,15 @@ DELETE FROM oauth2_provider_device_codes WHERE id = $1;
305305

306306
-- name: DeleteExpiredOAuth2ProviderDeviceCodes :exec
307307
DELETEFROM oauth2_provider_device_codes
308-
WHERE expires_at< NOW()AND status='pending';
308+
WHERE expires_at< NOW();
309+
310+
-- name: DeleteExpiredOAuth2ProviderAppCodes :exec
311+
DELETEFROM oauth2_provider_app_codes
312+
WHERE expires_at< NOW();
313+
314+
-- name: DeleteExpiredOAuth2ProviderAppTokens :exec
315+
DELETEFROM oauth2_provider_app_tokens
316+
WHERE expires_at< NOW();
309317

310318
-- name: GetOAuth2ProviderDeviceCodesByClientID :many
311319
SELECT*FROM oauth2_provider_device_codes

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp