Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix oauth2 builtin application logic #30304

Merged
merged 2 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions models/db/consistency.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions modules/setting/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
18 changes: 18 additions & 0 deletions modules/setting/oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
25 changes: 14 additions & 11 deletions services/doctor/dbconsistency.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"),
Expand All @@ -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
Expand Down
51 changes: 51 additions & 0 deletions services/doctor/dbconsistency_test.go
Original file line number Diff line number Diff line change
@@ -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"})
}
14 changes: 14 additions & 0 deletions services/doctor/main_test.go
Original file line number Diff line number Diff line change
@@ -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)
}