diff --git a/models/auth/TestOrphanedOAuth2Applications/oauth2_application.yaml b/models/auth/TestOrphanedOAuth2Applications/oauth2_application.yaml new file mode 100644 index 0000000000..b188770a30 --- /dev/null +++ b/models/auth/TestOrphanedOAuth2Applications/oauth2_application.yaml @@ -0,0 +1,25 @@ +- + id: 1000 + uid: 0 + name: "Git Credential Manager" + client_id: "e90ee53c-94e2-48ac-9358-a874fb9e0662" + redirect_uris: '["http://127.0.0.1", "https://127.0.0.1"]' + created_unix: 1712358091 + updated_unix: 1712358091 +- + id: 1001 + uid: 0 + name: "git-credential-oauth" + client_id: "a4792ccc-144e-407e-86c9-5e7d8d9c3269" + redirect_uris: '["http://127.0.0.1", "https://127.0.0.1"]' + created_unix: 1712358091 + updated_unix: 1712358091 + +- + id: 1002 + uid: 1234567890 + name: "Should be removed" + client_id: "deadc0de-badd-dd11-fee1-deaddecafbad" + redirect_uris: '["http://127.0.0.1", "https://127.0.0.1"]' + created_unix: 1712358091 + updated_unix: 1712358091 diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 9d53fffc78..83d60e3abe 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -74,6 +74,13 @@ func BuiltinApplications() map[string]*BuiltinOAuth2Application { return m } +func BuiltinApplicationsClientIDs() (clientIDs []string) { + for clientID := range BuiltinApplications() { + clientIDs = append(clientIDs, clientID) + } + return clientIDs +} + func Init(ctx context.Context) error { builtinApps := BuiltinApplications() var builtinAllClientIDs []string @@ -637,3 +644,27 @@ func DeleteOAuth2RelictsByUserID(ctx context.Context, userID int64) error { return nil } + +// CountOrphanedOAuth2Applications returns the amount of orphaned OAuth2 applications. +func CountOrphanedOAuth2Applications(ctx context.Context) (int64, error) { + return db.GetEngine(ctx). + Table("`oauth2_application`"). + Join("LEFT", "`user`", "`oauth2_application`.`uid` = `user`.`id`"). + Where(builder.IsNull{"`user`.id"}). + Where(builder.NotIn("`oauth2_application`.`client_id`", BuiltinApplicationsClientIDs())). + Select("COUNT(`oauth2_application`.`id`)"). + Count() +} + +// DeleteOrphanedOAuth2Applications deletes orphaned OAuth2 applications. +func DeleteOrphanedOAuth2Applications(ctx context.Context) (int64, error) { + subQuery := builder.Select("`oauth2_application`.id"). + From("`oauth2_application`"). + Join("LEFT", "`user`", "`oauth2_application`.`uid` = `user`.`id`"). + Where(builder.IsNull{"`user`.id"}). + Where(builder.NotIn("`oauth2_application`.`client_id`", BuiltinApplicationsClientIDs())) + + b := builder.Delete(builder.In("id", subQuery)).From("`oauth2_application`") + _, err := db.GetEngine(ctx).Exec(b) + return -1, err +} diff --git a/models/auth/oauth2_test.go b/models/auth/oauth2_test.go index 122d43098c..9a818a0bb3 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -4,11 +4,13 @@ package auth_test import ( + "path/filepath" "testing" auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" ) @@ -265,3 +267,31 @@ func TestOAuth2AuthorizationCode_Invalidate(t *testing.T) { func TestOAuth2AuthorizationCode_TableName(t *testing.T) { assert.Equal(t, "oauth2_authorization_code", new(auth_model.OAuth2AuthorizationCode).TableName()) } + +func TestBuiltinApplicationsClientIDs(t *testing.T) { + assert.EqualValues(t, []string{"a4792ccc-144e-407e-86c9-5e7d8d9c3269", "e90ee53c-94e2-48ac-9358-a874fb9e0662", "d57cb8c4-630c-4168-8324-ec79935e18d4"}, auth_model.BuiltinApplicationsClientIDs()) +} + +func TestOrphanedOAuth2Applications(t *testing.T) { + defer unittest.OverrideFixtures( + unittest.FixturesOptions{ + Dir: filepath.Join(setting.AppWorkPath, "models/fixtures/"), + Base: setting.AppWorkPath, + Dirs: []string{"models/auth/TestOrphanedOAuth2Applications/"}, + }, + )() + assert.NoError(t, unittest.PrepareTestDatabase()) + + count, err := auth_model.CountOrphanedOAuth2Applications(db.DefaultContext) + assert.NoError(t, err) + assert.EqualValues(t, 1, count) + unittest.AssertExistsIf(t, true, &auth_model.OAuth2Application{ID: 1002}) + + _, err = auth_model.DeleteOrphanedOAuth2Applications(db.DefaultContext) + assert.NoError(t, err) + + count, err = auth_model.CountOrphanedOAuth2Applications(db.DefaultContext) + assert.NoError(t, err) + assert.EqualValues(t, 0, count) + unittest.AssertExistsIf(t, false, &auth_model.OAuth2Application{ID: 1002}) +} diff --git a/services/doctor/dbconsistency.go b/services/doctor/dbconsistency.go index e2dcb63f33..0903ecc2a6 100644 --- a/services/doctor/dbconsistency.go +++ b/services/doctor/dbconsistency.go @@ -8,6 +8,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" activities_model "code.gitea.io/gitea/models/activities" + auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/migrations" @@ -164,6 +165,12 @@ func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) er Fixer: repo_model.DeleteOrphanedTopics, FixedMessage: "Removed", }, + { + Name: "Orphaned OAuth2Application without existing User", + Counter: auth_model.CountOrphanedOAuth2Applications, + Fixer: auth_model.DeleteOrphanedOAuth2Applications, + FixedMessage: "Removed", + }, } // TODO: function to recalc all counters @@ -208,9 +215,6 @@ func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) er // find OAuth2Grant without existing user genericOrphanCheck("Orphaned OAuth2Grant without existing User", "oauth2_grant", "user", "oauth2_grant.user_id=`user`.id"), - // find OAuth2Application without existing user - genericOrphanCheck("Orphaned OAuth2Application without existing User", - "oauth2_application", "user", "oauth2_application.uid=`user`.id"), // find OAuth2AuthorizationCode without existing OAuth2Grant genericOrphanCheck("Orphaned OAuth2AuthorizationCode without existing OAuth2Grant", "oauth2_authorization_code", "oauth2_grant", "oauth2_authorization_code.grant_id=oauth2_grant.id"),