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

Commit4a7aa03

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 parentbdc94d5 commit4a7aa03

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
@@ -1521,6 +1521,22 @@ func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCusto
15211521
returnq.db.DeleteCustomRole(ctx,arg)
15221522
}
15231523

1524+
func (q*querier)DeleteExpiredOAuth2ProviderAppCodes(ctx context.Context)error {
1525+
// System operation - only system can clean up expired authorization codes
1526+
iferr:=q.authorizeContext(ctx,policy.ActionDelete,rbac.ResourceSystem);err!=nil {
1527+
returnerr
1528+
}
1529+
returnq.db.DeleteExpiredOAuth2ProviderAppCodes(ctx)
1530+
}
1531+
1532+
func (q*querier)DeleteExpiredOAuth2ProviderAppTokens(ctx context.Context)error {
1533+
// System operation - only system can clean up expired access tokens
1534+
iferr:=q.authorizeContext(ctx,policy.ActionDelete,rbac.ResourceSystem);err!=nil {
1535+
returnerr
1536+
}
1537+
returnq.db.DeleteExpiredOAuth2ProviderAppTokens(ctx)
1538+
}
1539+
15241540
func (q*querier)DeleteExpiredOAuth2ProviderDeviceCodes(ctx context.Context)error {
15251541
// System operation - only system can clean up expired device codes
15261542
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
@@ -5525,6 +5525,9 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppCodes() {
55255525
})
55265526
check.Args(code.SecretPrefix).Asserts(code,policy.ActionUpdate).Returns(code)
55275527
}))
5528+
s.Run("DeleteExpiredOAuth2ProviderAppCodes",s.Subtest(func(db database.Store,check*expects) {
5529+
check.Args().Asserts(rbac.ResourceSystem,policy.ActionDelete)
5530+
}))
55285531
}
55295532

55305533
func (s*MethodTestSuite)TestOAuth2ProviderAppTokens() {
@@ -5598,6 +5601,9 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppTokens() {
55985601
UserID:user.ID,
55995602
}).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()),policy.ActionDelete)
56005603
}))
5604+
s.Run("DeleteExpiredOAuth2ProviderAppTokens",s.Subtest(func(db database.Store,check*expects) {
5605+
check.Args().Asserts(rbac.ResourceSystem,policy.ActionDelete)
5606+
}))
56015607
}
56025608

56035609
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
@@ -68,6 +68,15 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz.
6868
iferr:=tx.DeleteOldNotificationMessages(ctx);err!=nil {
6969
returnxerrors.Errorf("failed to delete old notification messages: %w",err)
7070
}
71+
iferr:=tx.DeleteExpiredOAuth2ProviderAppCodes(ctx);err!=nil {
72+
returnxerrors.Errorf("failed to delete expired oauth2 provider app codes: %w",err)
73+
}
74+
iferr:=tx.DeleteExpiredOAuth2ProviderAppTokens(ctx);err!=nil {
75+
returnxerrors.Errorf("failed to delete expired oauth2 provider app tokens: %w",err)
76+
}
77+
iferr:=tx.DeleteExpiredOAuth2ProviderDeviceCodes(ctx);err!=nil {
78+
returnxerrors.Errorf("failed to delete expired oauth2 provider device codes: %w",err)
79+
}
7180

7281
deleteOldAuditLogConnectionEventsBefore:=start.Add(-maxAuditLogConnectionEventAge)
7382
iferr:=tx.DeleteOldAuditLogConnectionEvents(ctx, database.DeleteOldAuditLogConnectionEventsParams{

‎coderd/database/dbpurge/dbpurge_test.go‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,3 +635,216 @@ func TestDeleteOldAuditLogConnectionEventsLimit(t *testing.T) {
635635

636636
require.Len(t,logs,0)
637637
}
638+
639+
//nolint:paralleltest // It uses LockIDDBPurge.
640+
funcTestDeleteExpiredOAuth2ProviderAppCodes(t*testing.T) {
641+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
642+
defercancel()
643+
644+
clk:=quartz.NewMock(t)
645+
db,_:=dbtestutil.NewDB(t,dbtestutil.WithDumpOnFailure())
646+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true})
647+
648+
now:=dbtime.Now()
649+
clk.Set(now).MustWait(ctx)
650+
651+
// Create test data
652+
user:=dbgen.User(t,db, database.User{})
653+
app:=dbgen.OAuth2ProviderApp(t,db, database.OAuth2ProviderApp{
654+
Name:fmt.Sprintf("test-codes-%d",time.Now().UnixNano()),
655+
})
656+
657+
// Create expired authorization code (should be deleted)
658+
expiredCode:=dbgen.OAuth2ProviderAppCode(t,db, database.OAuth2ProviderAppCode{
659+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
660+
AppID:app.ID,
661+
UserID:user.ID,
662+
SecretPrefix: []byte(fmt.Sprintf("expired-%d",time.Now().UnixNano())),
663+
})
664+
665+
// Create non-expired authorization code (should be retained)
666+
validCode:=dbgen.OAuth2ProviderAppCode(t,db, database.OAuth2ProviderAppCode{
667+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
668+
AppID:app.ID,
669+
UserID:user.ID,
670+
SecretPrefix: []byte(fmt.Sprintf("valid-%d",time.Now().UnixNano())),
671+
})
672+
673+
// Verify codes exist initially
674+
_,err:=db.GetOAuth2ProviderAppCodeByID(ctx,expiredCode.ID)
675+
require.NoError(t,err)
676+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,validCode.ID)
677+
require.NoError(t,err)
678+
679+
// Run cleanup
680+
done:=awaitDoTick(ctx,t,clk)
681+
closer:=dbpurge.New(ctx,logger,db,clk)
682+
defercloser.Close()
683+
<-done
684+
685+
// Verify expired code is deleted
686+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,expiredCode.ID)
687+
require.Error(t,err)
688+
require.ErrorIs(t,err,sql.ErrNoRows)
689+
690+
// Verify non-expired code is retained
691+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,validCode.ID)
692+
require.NoError(t,err)
693+
}
694+
695+
//nolint:paralleltest // It uses LockIDDBPurge.
696+
funcTestDeleteExpiredOAuth2ProviderAppTokens(t*testing.T) {
697+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
698+
defercancel()
699+
700+
clk:=quartz.NewMock(t)
701+
db,_:=dbtestutil.NewDB(t,dbtestutil.WithDumpOnFailure())
702+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true})
703+
704+
now:=dbtime.Now()
705+
clk.Set(now).MustWait(ctx)
706+
707+
// Create test data
708+
user:=dbgen.User(t,db, database.User{})
709+
app:=dbgen.OAuth2ProviderApp(t,db, database.OAuth2ProviderApp{
710+
Name:fmt.Sprintf("test-tokens-%d",time.Now().UnixNano()),
711+
})
712+
appSecret:=dbgen.OAuth2ProviderAppSecret(t,db, database.OAuth2ProviderAppSecret{
713+
AppID:app.ID,
714+
})
715+
716+
// Create API keys for the tokens
717+
expiredAPIKey,_:=dbgen.APIKey(t,db, database.APIKey{
718+
UserID:user.ID,
719+
ExpiresAt:now.Add(-1*time.Hour),
720+
})
721+
validAPIKey,_:=dbgen.APIKey(t,db, database.APIKey{
722+
UserID:user.ID,
723+
ExpiresAt:now.Add(24*time.Hour),// Valid for 24 hours
724+
})
725+
726+
// Create expired access token (should be deleted)
727+
expiredToken:=dbgen.OAuth2ProviderAppToken(t,db, database.OAuth2ProviderAppToken{
728+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
729+
AppSecretID:appSecret.ID,
730+
APIKeyID:expiredAPIKey.ID,
731+
UserID:user.ID,
732+
HashPrefix: []byte(fmt.Sprintf("expired-%d",time.Now().UnixNano())),
733+
})
734+
735+
// Create non-expired access token (should be retained)
736+
validToken:=dbgen.OAuth2ProviderAppToken(t,db, database.OAuth2ProviderAppToken{
737+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
738+
AppSecretID:appSecret.ID,
739+
APIKeyID:validAPIKey.ID,
740+
UserID:user.ID,
741+
HashPrefix: []byte(fmt.Sprintf("valid-%d",time.Now().UnixNano())),
742+
})
743+
744+
// Verify tokens exist initially
745+
_,err:=db.GetOAuth2ProviderAppTokenByPrefix(ctx,expiredToken.HashPrefix)
746+
require.NoError(t,err)
747+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,validToken.HashPrefix)
748+
require.NoError(t,err)
749+
750+
// Run cleanup
751+
done:=awaitDoTick(ctx,t,clk)
752+
closer:=dbpurge.New(ctx,logger,db,clk)
753+
defercloser.Close()
754+
<-done
755+
756+
// Verify expired token is deleted
757+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,expiredToken.HashPrefix)
758+
require.Error(t,err)
759+
require.ErrorIs(t,err,sql.ErrNoRows)
760+
761+
// Verify non-expired token is retained
762+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,validToken.HashPrefix)
763+
require.NoError(t,err)
764+
}
765+
766+
//nolint:paralleltest // It uses LockIDDBPurge.
767+
funcTestDeleteExpiredOAuth2ProviderDeviceCodes(t*testing.T) {
768+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
769+
defercancel()
770+
771+
clk:=quartz.NewMock(t)
772+
db,_:=dbtestutil.NewDB(t,dbtestutil.WithDumpOnFailure())
773+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true})
774+
775+
now:=dbtime.Now()
776+
clk.Set(now).MustWait(ctx)
777+
778+
// Create test data
779+
app:=dbgen.OAuth2ProviderApp(t,db, database.OAuth2ProviderApp{
780+
Name:fmt.Sprintf("test-device-%d",time.Now().UnixNano()),
781+
})
782+
783+
nanoTime:=time.Now().UnixNano()
784+
785+
// Create expired device code with pending status (should be deleted)
786+
expiredPendingCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
787+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
788+
ClientID:app.ID,
789+
Status:database.OAuth2DeviceStatusPending,
790+
DeviceCodePrefix:fmt.Sprintf("EP%06d",nanoTime%1000000),
791+
UserCode:fmt.Sprintf("EP%06d",nanoTime%1000000),
792+
DeviceCodeHash:fmt.Appendf(nil,"hash-exp-pending-%d",nanoTime),
793+
})
794+
795+
// Create non-expired device code with pending status (should be retained)
796+
validPendingCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
797+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
798+
ClientID:app.ID,
799+
Status:database.OAuth2DeviceStatusPending,
800+
DeviceCodePrefix:fmt.Sprintf("VP%06d", (nanoTime+1)%1000000),
801+
UserCode:fmt.Sprintf("VP%06d", (nanoTime+1)%1000000),
802+
DeviceCodeHash:fmt.Appendf(nil,"hash-val-pending-%d",nanoTime+1),
803+
})
804+
805+
// Create expired device code with authorized status (should be deleted - all expired codes are deleted)
806+
expiredAuthorizedCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
807+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
808+
ClientID:app.ID,
809+
DeviceCodePrefix:fmt.Sprintf("EA%06d", (nanoTime+2)%1000000),
810+
UserCode:fmt.Sprintf("EA%06d", (nanoTime+2)%1000000),
811+
DeviceCodeHash:fmt.Appendf(nil,"hash-exp-auth-%d",nanoTime+2),
812+
})
813+
814+
// Create a user and authorize the device code
815+
user:=dbgen.User(t,db, database.User{})
816+
expiredAuthorizedCode,err:=db.UpdateOAuth2ProviderDeviceCodeAuthorization(ctx, database.UpdateOAuth2ProviderDeviceCodeAuthorizationParams{
817+
ID:expiredAuthorizedCode.ID,
818+
UserID: uuid.NullUUID{UUID:user.ID,Valid:true},
819+
Status:database.OAuth2DeviceStatusAuthorized,
820+
})
821+
require.NoError(t,err)
822+
823+
// Verify device codes exist initially
824+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredPendingCode.ID)
825+
require.NoError(t,err)
826+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,validPendingCode.ID)
827+
require.NoError(t,err)
828+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredAuthorizedCode.ID)
829+
require.NoError(t,err)
830+
831+
// Run cleanup
832+
done:=awaitDoTick(ctx,t,clk)
833+
closer:=dbpurge.New(ctx,logger,db,clk)
834+
defercloser.Close()
835+
<-done
836+
837+
// Verify expired pending device code is deleted
838+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredPendingCode.ID)
839+
require.Error(t,err)
840+
require.ErrorIs(t,err,sql.ErrNoRows)
841+
842+
// Verify non-expired pending device code is retained
843+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,validPendingCode.ID)
844+
require.NoError(t,err)
845+
846+
// Verify expired authorized device code is deleted (all expired codes are deleted)
847+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredAuthorizedCode.ID)
848+
require.Error(t,err)
849+
require.ErrorIs(t,err,sql.ErrNoRows)
850+
}

‎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
@@ -310,7 +310,15 @@ DELETE FROM oauth2_provider_device_codes WHERE id = $1;
310310

311311
-- name: DeleteExpiredOAuth2ProviderDeviceCodes :exec
312312
DELETEFROM oauth2_provider_device_codes
313-
WHERE expires_at< NOW()AND status='pending';
313+
WHERE expires_at< NOW();
314+
315+
-- name: DeleteExpiredOAuth2ProviderAppCodes :exec
316+
DELETEFROM oauth2_provider_app_codes
317+
WHERE expires_at< NOW();
318+
319+
-- name: DeleteExpiredOAuth2ProviderAppTokens :exec
320+
DELETEFROM oauth2_provider_app_tokens
321+
WHERE expires_at< NOW();
314322

315323
-- name: GetOAuth2ProviderDeviceCodesByClientID :many
316324
SELECT*FROM oauth2_provider_device_codes

0 commit comments

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