Fix oauth2 builtin application logic (#30304)

Fix #29074 (allow to disable all builtin apps) and don't make the doctor
command remove the builtin apps.

By the way, rename refobject and joincond to camel case.
This commit is contained in:
wxiaoguang 2024-04-08 12:13:34 +08:00 committed by GitHub
parent 0c7b0c5aca
commit 074a3e05f6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 107 additions and 17 deletions

@ -10,21 +10,21 @@ import (
)
// CountOrphanedObjects count subjects with have no existing refobject anymore
func CountOrphanedObjects(ctx context.Context, subject, refobject, joinCond string) (int64, error) {
func CountOrphanedObjects(ctx context.Context, subject, refObject, joinCond string) (int64, error) {
return GetEngine(ctx).
Table("`"+subject+"`").
Join("LEFT", "`"+refobject+"`", joinCond).
Where(builder.IsNull{"`" + refobject + "`.id"}).
Join("LEFT", "`"+refObject+"`", joinCond).
Where(builder.IsNull{"`" + refObject + "`.id"}).
Select("COUNT(`" + subject + "`.`id`)").
Count()
}
// DeleteOrphanedObjects delete subjects with have no existing refobject anymore
func DeleteOrphanedObjects(ctx context.Context, subject, refobject, joinCond string) error {
func DeleteOrphanedObjects(ctx context.Context, subject, refObject, joinCond string) error {
subQuery := builder.Select("`"+subject+"`.id").
From("`"+subject+"`").
Join("LEFT", "`"+refobject+"`", joinCond).
Where(builder.IsNull{"`" + refobject + "`.id"})
Join("LEFT", "`"+refObject+"`", joinCond).
Where(builder.IsNull{"`" + refObject + "`.id"})
b := builder.Delete(builder.In("id", subQuery)).From("`" + subject + "`")
_, err := GetEngine(ctx).Exec(b)
return err

@ -118,6 +118,10 @@ func loadOAuth2From(rootCfg ConfigProvider) {
return
}
if sec.HasKey("DEFAULT_APPLICATIONS") && sec.Key("DEFAULT_APPLICATIONS").String() == "" {
OAuth2.DefaultApplications = nil
}
// Handle the rename of ENABLE to ENABLED
deprecatedSetting(rootCfg, "oauth2", "ENABLE", "oauth2", "ENABLED", "v1.23.0")
if sec.HasKey("ENABLE") && !sec.HasKey("ENABLED") {

@ -32,3 +32,21 @@ JWT_SECRET = BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
assert.Len(t, actual, 32)
assert.EqualValues(t, expected, actual)
}
func TestOauth2DefaultApplications(t *testing.T) {
cfg, _ := NewConfigProviderFromData(``)
loadOAuth2From(cfg)
assert.Equal(t, []string{"git-credential-oauth", "git-credential-manager", "tea"}, OAuth2.DefaultApplications)
cfg, _ = NewConfigProviderFromData(`[oauth2]
DEFAULT_APPLICATIONS = tea
`)
loadOAuth2From(cfg)
assert.Equal(t, []string{"tea"}, OAuth2.DefaultApplications)
cfg, _ = NewConfigProviderFromData(`[oauth2]
DEFAULT_APPLICATIONS =
`)
loadOAuth2From(cfg)
assert.Nil(t, nil, OAuth2.DefaultApplications)
}

@ -61,26 +61,20 @@ func asFixer(fn func(ctx context.Context) error) func(ctx context.Context) (int6
}
}
func genericOrphanCheck(name, subject, refobject, joincond string) consistencyCheck {
func genericOrphanCheck(name, subject, refObject, joinCond string) consistencyCheck {
return consistencyCheck{
Name: name,
Counter: func(ctx context.Context) (int64, error) {
return db.CountOrphanedObjects(ctx, subject, refobject, joincond)
return db.CountOrphanedObjects(ctx, subject, refObject, joinCond)
},
Fixer: func(ctx context.Context) (int64, error) {
err := db.DeleteOrphanedObjects(ctx, subject, refobject, joincond)
err := db.DeleteOrphanedObjects(ctx, subject, refObject, joinCond)
return -1, err
},
}
}
func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) error {
// make sure DB version is uptodate
if err := db.InitEngineWithMigration(ctx, migrations.EnsureUpToDate); err != nil {
logger.Critical("Model version on the database does not match the current Gitea version. Model consistency will not be checked until the database is upgraded")
return err
}
func prepareDBConsistencyChecks() []consistencyCheck {
consistencyChecks := []consistencyCheck{
{
// find labels without existing repo or org
@ -210,7 +204,7 @@ func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) er
"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"),
"oauth2_application", "user", "oauth2_application.uid=0 OR 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"),
@ -224,7 +218,16 @@ func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) er
genericOrphanCheck("Orphaned Redirects without existing redirect user",
"user_redirect", "user", "user_redirect.redirect_user_id=`user`.id"),
)
return consistencyChecks
}
func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) error {
// make sure DB version is uptodate
if err := db.InitEngineWithMigration(ctx, migrations.EnsureUpToDate); err != nil {
logger.Critical("Model version on the database does not match the current Gitea version. Model consistency will not be checked until the database is upgraded")
return err
}
consistencyChecks := prepareDBConsistencyChecks()
for _, c := range consistencyChecks {
if err := c.Run(ctx, logger, autofix); err != nil {
return err

@ -0,0 +1,51 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package doctor
import (
"slices"
"testing"
"code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
"github.com/stretchr/testify/assert"
)
func TestConsistencyCheck(t *testing.T) {
checks := prepareDBConsistencyChecks()
idx := slices.IndexFunc(checks, func(check consistencyCheck) bool {
return check.Name == "Orphaned OAuth2Application without existing User"
})
if !assert.NotEqual(t, -1, idx) {
return
}
_ = db.TruncateBeans(db.DefaultContext, &auth.OAuth2Application{}, &user.User{})
_ = db.TruncateBeans(db.DefaultContext, &auth.OAuth2Application{}, &auth.OAuth2Application{})
err := db.Insert(db.DefaultContext, &user.User{ID: 1})
assert.NoError(t, err)
err = db.Insert(db.DefaultContext, &auth.OAuth2Application{Name: "test-oauth2-app-1", ClientID: "client-id-1"})
assert.NoError(t, err)
err = db.Insert(db.DefaultContext, &auth.OAuth2Application{Name: "test-oauth2-app-2", ClientID: "client-id-2", UID: 1})
assert.NoError(t, err)
err = db.Insert(db.DefaultContext, &auth.OAuth2Application{Name: "test-oauth2-app-3", ClientID: "client-id-3", UID: 99999999})
assert.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-1"})
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-2"})
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-3"})
oauth2AppCheck := checks[idx]
err = oauth2AppCheck.Run(db.DefaultContext, log.GetManager().GetLogger(log.DEFAULT), true)
assert.NoError(t, err)
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-1"})
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-2"})
unittest.AssertNotExistsBean(t, &auth.OAuth2Application{ClientID: "client-id-3"})
}

@ -0,0 +1,14 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package doctor
import (
"testing"
"code.gitea.io/gitea/models/unittest"
)
func TestMain(m *testing.M) {
unittest.MainTest(m)
}