Fix oauth2 builtin application logic (#30304) (#30327)

Backport #30304 by wxiaoguang

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.

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Giteabot 2024-04-08 15:46:21 +08:00 committed by GitHub
parent db370c47a6
commit c541616f1c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 107 additions and 17 deletions

View File

@ -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

View File

@ -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") {

View File

@ -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)
}

View File

@ -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

View File

@ -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"})
}

View File

@ -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)
}