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

Commitfa6dc8d

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 parentd00dd0b commitfa6dc8d

File tree

9 files changed

+318
-3
lines changed

9 files changed

+318
-3
lines changed

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,6 +1609,22 @@ func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCusto
16091609
returnq.db.DeleteCustomRole(ctx,arg)
16101610
}
16111611

1612+
func (q*querier)DeleteExpiredOAuth2ProviderAppCodes(ctx context.Context)error {
1613+
// System operation - only system can clean up expired authorization codes
1614+
iferr:=q.authorizeContext(ctx,policy.ActionDelete,rbac.ResourceSystem);err!=nil {
1615+
returnerr
1616+
}
1617+
returnq.db.DeleteExpiredOAuth2ProviderAppCodes(ctx)
1618+
}
1619+
1620+
func (q*querier)DeleteExpiredOAuth2ProviderAppTokens(ctx context.Context)error {
1621+
// System operation - only system can clean up expired access tokens
1622+
iferr:=q.authorizeContext(ctx,policy.ActionDelete,rbac.ResourceSystem);err!=nil {
1623+
returnerr
1624+
}
1625+
returnq.db.DeleteExpiredOAuth2ProviderAppTokens(ctx)
1626+
}
1627+
16121628
func (q*querier)DeleteExpiredOAuth2ProviderDeviceCodes(ctx context.Context)error {
16131629
// System operation - only system can clean up expired device codes
16141630
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
@@ -3977,6 +3977,9 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppCodes() {
39773977
})
39783978
check.Args(code.SecretPrefix).Asserts(code,policy.ActionUpdate).Returns(code)
39793979
}))
3980+
s.Run("DeleteExpiredOAuth2ProviderAppCodes",s.Subtest(func(db database.Store,check*expects) {
3981+
check.Args().Asserts(rbac.ResourceSystem,policy.ActionDelete)
3982+
}))
39803983
}
39813984

39823985
func (s*MethodTestSuite)TestOAuth2ProviderAppTokens() {
@@ -4050,6 +4053,9 @@ func (s *MethodTestSuite) TestOAuth2ProviderAppTokens() {
40504053
UserID:user.ID,
40514054
}).Asserts(rbac.ResourceOauth2AppCodeToken.WithOwner(user.ID.String()),policy.ActionDelete)
40524055
}))
4056+
s.Run("DeleteExpiredOAuth2ProviderAppTokens",s.Subtest(func(db database.Store,check*expects) {
4057+
check.Args().Asserts(rbac.ResourceSystem,policy.ActionDelete)
4058+
}))
40534059
}
40544060

40554061
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
@@ -71,6 +71,15 @@ func New(ctx context.Context, logger slog.Logger, db database.Store, clk quartz.
7171
iferr:=tx.ExpirePrebuildsAPIKeys(ctx,dbtime.Time(start));err!=nil {
7272
returnxerrors.Errorf("failed to expire prebuilds user api keys: %w",err)
7373
}
74+
iferr:=tx.DeleteExpiredOAuth2ProviderAppCodes(ctx);err!=nil {
75+
returnxerrors.Errorf("failed to delete expired oauth2 provider app codes: %w",err)
76+
}
77+
iferr:=tx.DeleteExpiredOAuth2ProviderAppTokens(ctx);err!=nil {
78+
returnxerrors.Errorf("failed to delete expired oauth2 provider app tokens: %w",err)
79+
}
80+
iferr:=tx.DeleteExpiredOAuth2ProviderDeviceCodes(ctx);err!=nil {
81+
returnxerrors.Errorf("failed to delete expired oauth2 provider device codes: %w",err)
82+
}
7483

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

‎coderd/database/dbpurge/dbpurge_test.go‎

Lines changed: 213 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,6 @@ func TestDeleteOldAuditLogConnectionEventsLimit(t *testing.T) {
639639

640640
require.Len(t,logs,0)
641641
}
642-
643642
funcTestExpireOldAPIKeys(t*testing.T) {
644643
t.Parallel()
645644

@@ -704,3 +703,216 @@ func TestExpireOldAPIKeys(t *testing.T) {
704703
// Out of an abundance of caution, we do not expire explicitly named prebuilds API keys.
705704
assertKeyActive(namedPrebuildsAPIKey.ID)
706705
}
706+
707+
//nolint:paralleltest // It uses LockIDDBPurge.
708+
funcTestDeleteExpiredOAuth2ProviderAppCodes(t*testing.T) {
709+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
710+
defercancel()
711+
712+
clk:=quartz.NewMock(t)
713+
db,_:=dbtestutil.NewDB(t,dbtestutil.WithDumpOnFailure())
714+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true})
715+
716+
now:=dbtime.Now()
717+
clk.Set(now).MustWait(ctx)
718+
719+
// Create test data
720+
user:=dbgen.User(t,db, database.User{})
721+
app:=dbgen.OAuth2ProviderApp(t,db, database.OAuth2ProviderApp{
722+
Name:fmt.Sprintf("test-codes-%d",time.Now().UnixNano()),
723+
})
724+
725+
// Create expired authorization code (should be deleted)
726+
expiredCode:=dbgen.OAuth2ProviderAppCode(t,db, database.OAuth2ProviderAppCode{
727+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
728+
AppID:app.ID,
729+
UserID:user.ID,
730+
SecretPrefix: []byte(fmt.Sprintf("expired-%d",time.Now().UnixNano())),
731+
})
732+
733+
// Create non-expired authorization code (should be retained)
734+
validCode:=dbgen.OAuth2ProviderAppCode(t,db, database.OAuth2ProviderAppCode{
735+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
736+
AppID:app.ID,
737+
UserID:user.ID,
738+
SecretPrefix: []byte(fmt.Sprintf("valid-%d",time.Now().UnixNano())),
739+
})
740+
741+
// Verify codes exist initially
742+
_,err:=db.GetOAuth2ProviderAppCodeByID(ctx,expiredCode.ID)
743+
require.NoError(t,err)
744+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,validCode.ID)
745+
require.NoError(t,err)
746+
747+
// Run cleanup
748+
done:=awaitDoTick(ctx,t,clk)
749+
closer:=dbpurge.New(ctx,logger,db,clk)
750+
defercloser.Close()
751+
<-done
752+
753+
// Verify expired code is deleted
754+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,expiredCode.ID)
755+
require.Error(t,err)
756+
require.ErrorIs(t,err,sql.ErrNoRows)
757+
758+
// Verify non-expired code is retained
759+
_,err=db.GetOAuth2ProviderAppCodeByID(ctx,validCode.ID)
760+
require.NoError(t,err)
761+
}
762+
763+
//nolint:paralleltest // It uses LockIDDBPurge.
764+
funcTestDeleteExpiredOAuth2ProviderAppTokens(t*testing.T) {
765+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
766+
defercancel()
767+
768+
clk:=quartz.NewMock(t)
769+
db,_:=dbtestutil.NewDB(t,dbtestutil.WithDumpOnFailure())
770+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true})
771+
772+
now:=dbtime.Now()
773+
clk.Set(now).MustWait(ctx)
774+
775+
// Create test data
776+
user:=dbgen.User(t,db, database.User{})
777+
app:=dbgen.OAuth2ProviderApp(t,db, database.OAuth2ProviderApp{
778+
Name:fmt.Sprintf("test-tokens-%d",time.Now().UnixNano()),
779+
})
780+
appSecret:=dbgen.OAuth2ProviderAppSecret(t,db, database.OAuth2ProviderAppSecret{
781+
AppID:app.ID,
782+
})
783+
784+
// Create API keys for the tokens
785+
expiredAPIKey,_:=dbgen.APIKey(t,db, database.APIKey{
786+
UserID:user.ID,
787+
ExpiresAt:now.Add(-1*time.Hour),
788+
})
789+
validAPIKey,_:=dbgen.APIKey(t,db, database.APIKey{
790+
UserID:user.ID,
791+
ExpiresAt:now.Add(24*time.Hour),// Valid for 24 hours
792+
})
793+
794+
// Create expired access token (should be deleted)
795+
expiredToken:=dbgen.OAuth2ProviderAppToken(t,db, database.OAuth2ProviderAppToken{
796+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
797+
AppSecretID:appSecret.ID,
798+
APIKeyID:expiredAPIKey.ID,
799+
UserID:user.ID,
800+
HashPrefix: []byte(fmt.Sprintf("expired-%d",time.Now().UnixNano())),
801+
})
802+
803+
// Create non-expired access token (should be retained)
804+
validToken:=dbgen.OAuth2ProviderAppToken(t,db, database.OAuth2ProviderAppToken{
805+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
806+
AppSecretID:appSecret.ID,
807+
APIKeyID:validAPIKey.ID,
808+
UserID:user.ID,
809+
HashPrefix: []byte(fmt.Sprintf("valid-%d",time.Now().UnixNano())),
810+
})
811+
812+
// Verify tokens exist initially
813+
_,err:=db.GetOAuth2ProviderAppTokenByPrefix(ctx,expiredToken.HashPrefix)
814+
require.NoError(t,err)
815+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,validToken.HashPrefix)
816+
require.NoError(t,err)
817+
818+
// Run cleanup
819+
done:=awaitDoTick(ctx,t,clk)
820+
closer:=dbpurge.New(ctx,logger,db,clk)
821+
defercloser.Close()
822+
<-done
823+
824+
// Verify expired token is deleted
825+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,expiredToken.HashPrefix)
826+
require.Error(t,err)
827+
require.ErrorIs(t,err,sql.ErrNoRows)
828+
829+
// Verify non-expired token is retained
830+
_,err=db.GetOAuth2ProviderAppTokenByPrefix(ctx,validToken.HashPrefix)
831+
require.NoError(t,err)
832+
}
833+
834+
//nolint:paralleltest // It uses LockIDDBPurge.
835+
funcTestDeleteExpiredOAuth2ProviderDeviceCodes(t*testing.T) {
836+
ctx,cancel:=context.WithTimeout(context.Background(),testutil.WaitShort)
837+
defercancel()
838+
839+
clk:=quartz.NewMock(t)
840+
db,_:=dbtestutil.NewDB(t,dbtestutil.WithDumpOnFailure())
841+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true})
842+
843+
now:=dbtime.Now()
844+
clk.Set(now).MustWait(ctx)
845+
846+
// Create test data
847+
app:=dbgen.OAuth2ProviderApp(t,db, database.OAuth2ProviderApp{
848+
Name:fmt.Sprintf("test-device-%d",time.Now().UnixNano()),
849+
})
850+
851+
nanoTime:=time.Now().UnixNano()
852+
853+
// Create expired device code with pending status (should be deleted)
854+
expiredPendingCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
855+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
856+
ClientID:app.ID,
857+
Status:database.OAuth2DeviceStatusPending,
858+
DeviceCodePrefix:fmt.Sprintf("EP%06d",nanoTime%1000000),
859+
UserCode:fmt.Sprintf("EP%06d",nanoTime%1000000),
860+
DeviceCodeHash:fmt.Appendf(nil,"hash-exp-pending-%d",nanoTime),
861+
})
862+
863+
// Create non-expired device code with pending status (should be retained)
864+
validPendingCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
865+
ExpiresAt:now.Add(1*time.Hour),// Expires in 1 hour
866+
ClientID:app.ID,
867+
Status:database.OAuth2DeviceStatusPending,
868+
DeviceCodePrefix:fmt.Sprintf("VP%06d", (nanoTime+1)%1000000),
869+
UserCode:fmt.Sprintf("VP%06d", (nanoTime+1)%1000000),
870+
DeviceCodeHash:fmt.Appendf(nil,"hash-val-pending-%d",nanoTime+1),
871+
})
872+
873+
// Create expired device code with authorized status (should be deleted - all expired codes are deleted)
874+
expiredAuthorizedCode:=dbgen.OAuth2ProviderDeviceCode(t,db, database.OAuth2ProviderDeviceCode{
875+
ExpiresAt:now.Add(-1*time.Hour),// Expired 1 hour ago
876+
ClientID:app.ID,
877+
DeviceCodePrefix:fmt.Sprintf("EA%06d", (nanoTime+2)%1000000),
878+
UserCode:fmt.Sprintf("EA%06d", (nanoTime+2)%1000000),
879+
DeviceCodeHash:fmt.Appendf(nil,"hash-exp-auth-%d",nanoTime+2),
880+
})
881+
882+
// Create a user and authorize the device code
883+
user:=dbgen.User(t,db, database.User{})
884+
expiredAuthorizedCode,err:=db.UpdateOAuth2ProviderDeviceCodeAuthorization(ctx, database.UpdateOAuth2ProviderDeviceCodeAuthorizationParams{
885+
ID:expiredAuthorizedCode.ID,
886+
UserID: uuid.NullUUID{UUID:user.ID,Valid:true},
887+
Status:database.OAuth2DeviceStatusAuthorized,
888+
})
889+
require.NoError(t,err)
890+
891+
// Verify device codes exist initially
892+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredPendingCode.ID)
893+
require.NoError(t,err)
894+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,validPendingCode.ID)
895+
require.NoError(t,err)
896+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredAuthorizedCode.ID)
897+
require.NoError(t,err)
898+
899+
// Run cleanup
900+
done:=awaitDoTick(ctx,t,clk)
901+
closer:=dbpurge.New(ctx,logger,db,clk)
902+
defercloser.Close()
903+
<-done
904+
905+
// Verify expired pending device code is deleted
906+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredPendingCode.ID)
907+
require.Error(t,err)
908+
require.ErrorIs(t,err,sql.ErrNoRows)
909+
910+
// Verify non-expired pending device code is retained
911+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,validPendingCode.ID)
912+
require.NoError(t,err)
913+
914+
// Verify expired authorized device code is deleted (all expired codes are deleted)
915+
_,err=db.GetOAuth2ProviderDeviceCodeByID(ctx,expiredAuthorizedCode.ID)
916+
require.Error(t,err)
917+
require.ErrorIs(t,err,sql.ErrNoRows)
918+
}

‎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