mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-11-27 04:43:59 +01:00
[BUG] Don't remove builtin OAuth2 applications
- When the database consistency is being run it would check for any OAuth2 applications that don't have an existing user. However there are few special OAuth2 applications that don't have an user set, because they are global applications. - This was not taken into account by the database consistency checker and were removed if the database consistency check was being run with autofix enabled. - Take into account to ignore these global OAuth2 applications when running the database consistency check. - Add unit tests. - Ref: https://codeberg.org/Codeberg/Community/issues/1530
This commit is contained in:
parent
af47c583b4
commit
6af8f3a3f2
4 changed files with 93 additions and 3 deletions
|
@ -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
|
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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})
|
||||
}
|
||||
|
|
|
@ -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"),
|
||||
|
|
Loading…
Reference in a new issue