Skip to content

Commit 074a3e0

Browse files
wxiaoguangwxiaoguang
authored
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.
1 parent 0c7b0c5 commit 074a3e0

File tree

6 files changed

+107
-17
lines changed

6 files changed

+107
-17
lines changed

‎models/db/consistency.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,21 @@ import (
1010
)
1111

1212
// CountOrphanedObjects count subjects with have no existing refobject anymore
13-
func CountOrphanedObjects(ctx context.Context, subject, refobject, joinCond string) (int64, error) {
13+
func CountOrphanedObjects(ctx context.Context, subject, refObject, joinCond string) (int64, error) {
1414
return GetEngine(ctx).
1515
Table("`"+subject+"`").
16-
Join("LEFT", "`"+refobject+"`", joinCond).
17-
Where(builder.IsNull{"`" + refobject + "`.id"}).
16+
Join("LEFT", "`"+refObject+"`", joinCond).
17+
Where(builder.IsNull{"`" + refObject + "`.id"}).
1818
Select("COUNT(`" + subject + "`.`id`)").
1919
Count()
2020
}
2121

2222
// DeleteOrphanedObjects delete subjects with have no existing refobject anymore
23-
func DeleteOrphanedObjects(ctx context.Context, subject, refobject, joinCond string) error {
23+
func DeleteOrphanedObjects(ctx context.Context, subject, refObject, joinCond string) error {
2424
subQuery := builder.Select("`"+subject+"`.id").
2525
From("`"+subject+"`").
26-
Join("LEFT", "`"+refobject+"`", joinCond).
27-
Where(builder.IsNull{"`" + refobject + "`.id"})
26+
Join("LEFT", "`"+refObject+"`", joinCond).
27+
Where(builder.IsNull{"`" + refObject + "`.id"})
2828
b := builder.Delete(builder.In("id", subQuery)).From("`" + subject + "`")
2929
_, err := GetEngine(ctx).Exec(b)
3030
return err

‎modules/setting/oauth2.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ func loadOAuth2From(rootCfg ConfigProvider) {
118118
return
119119
}
120120

121+
if sec.HasKey("DEFAULT_APPLICATIONS") && sec.Key("DEFAULT_APPLICATIONS").String() == "" {
122+
OAuth2.DefaultApplications = nil
123+
}
124+
121125
// Handle the rename of ENABLE to ENABLED
122126
deprecatedSetting(rootCfg, "oauth2", "ENABLE", "oauth2", "ENABLED", "v1.23.0")
123127
if sec.HasKey("ENABLE") && !sec.HasKey("ENABLED") {

‎modules/setting/oauth2_test.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,21 @@ JWT_SECRET = BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
3232
assert.Len(t, actual, 32)
3333
assert.EqualValues(t, expected, actual)
3434
}
35+
36+
func TestOauth2DefaultApplications(t *testing.T) {
37+
cfg, _ := NewConfigProviderFromData(``)
38+
loadOAuth2From(cfg)
39+
assert.Equal(t, []string{"git-credential-oauth", "git-credential-manager", "tea"}, OAuth2.DefaultApplications)
40+
41+
cfg, _ = NewConfigProviderFromData(`[oauth2]
42+
DEFAULT_APPLICATIONS = tea
43+
`)
44+
loadOAuth2From(cfg)
45+
assert.Equal(t, []string{"tea"}, OAuth2.DefaultApplications)
46+
47+
cfg, _ = NewConfigProviderFromData(`[oauth2]
48+
DEFAULT_APPLICATIONS =
49+
`)
50+
loadOAuth2From(cfg)
51+
assert.Nil(t, nil, OAuth2.DefaultApplications)
52+
}

‎services/doctor/dbconsistency.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,26 +61,20 @@ func asFixer(fn func(ctx context.Context) error) func(ctx context.Context) (int6
6161
}
6262
}
6363

64-
func genericOrphanCheck(name, subject, refobject, joincond string) consistencyCheck {
64+
func genericOrphanCheck(name, subject, refObject, joinCond string) consistencyCheck {
6565
return consistencyCheck{
6666
Name: name,
6767
Counter: func(ctx context.Context) (int64, error) {
68-
return db.CountOrphanedObjects(ctx, subject, refobject, joincond)
68+
return db.CountOrphanedObjects(ctx, subject, refObject, joinCond)
6969
},
7070
Fixer: func(ctx context.Context) (int64, error) {
71-
err := db.DeleteOrphanedObjects(ctx, subject, refobject, joincond)
71+
err := db.DeleteOrphanedObjects(ctx, subject, refObject, joinCond)
7272
return -1, err
7373
},
7474
}
7575
}
7676

77-
func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) error {
78-
// make sure DB version is uptodate
79-
if err := db.InitEngineWithMigration(ctx, migrations.EnsureUpToDate); err != nil {
80-
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")
81-
return err
82-
}
83-
77+
func prepareDBConsistencyChecks() []consistencyCheck {
8478
consistencyChecks := []consistencyCheck{
8579
{
8680
// find labels without existing repo or org
@@ -210,7 +204,7 @@ func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) er
210204
"oauth2_grant", "user", "oauth2_grant.user_id=`user`.id"),
211205
// find OAuth2Application without existing user
212206
genericOrphanCheck("Orphaned OAuth2Application without existing User",
213-
"oauth2_application", "user", "oauth2_application.uid=`user`.id"),
207+
"oauth2_application", "user", "oauth2_application.uid=0 OR oauth2_application.uid=`user`.id"),
214208
// find OAuth2AuthorizationCode without existing OAuth2Grant
215209
genericOrphanCheck("Orphaned OAuth2AuthorizationCode without existing OAuth2Grant",
216210
"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
224218
genericOrphanCheck("Orphaned Redirects without existing redirect user",
225219
"user_redirect", "user", "user_redirect.redirect_user_id=`user`.id"),
226220
)
221+
return consistencyChecks
222+
}
227223

224+
func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) error {
225+
// make sure DB version is uptodate
226+
if err := db.InitEngineWithMigration(ctx, migrations.EnsureUpToDate); err != nil {
227+
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")
228+
return err
229+
}
230+
consistencyChecks := prepareDBConsistencyChecks()
228231
for _, c := range consistencyChecks {
229232
if err := c.Run(ctx, logger, autofix); err != nil {
230233
return err

‎services/doctor/dbconsistency_test.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package doctor
5+
6+
import (
7+
"slices"
8+
"testing"
9+
10+
"code.gitea.io/gitea/models/auth"
11+
"code.gitea.io/gitea/models/db"
12+
"code.gitea.io/gitea/models/unittest"
13+
"code.gitea.io/gitea/models/user"
14+
"code.gitea.io/gitea/modules/log"
15+
16+
"github.com/stretchr/testify/assert"
17+
)
18+
19+
func TestConsistencyCheck(t *testing.T) {
20+
checks := prepareDBConsistencyChecks()
21+
idx := slices.IndexFunc(checks, func(check consistencyCheck) bool {
22+
return check.Name == "Orphaned OAuth2Application without existing User"
23+
})
24+
if !assert.NotEqual(t, -1, idx) {
25+
return
26+
}
27+
28+
_ = db.TruncateBeans(db.DefaultContext, &auth.OAuth2Application{}, &user.User{})
29+
_ = db.TruncateBeans(db.DefaultContext, &auth.OAuth2Application{}, &auth.OAuth2Application{})
30+
31+
err := db.Insert(db.DefaultContext, &user.User{ID: 1})
32+
assert.NoError(t, err)
33+
err = db.Insert(db.DefaultContext, &auth.OAuth2Application{Name: "test-oauth2-app-1", ClientID: "client-id-1"})
34+
assert.NoError(t, err)
35+
err = db.Insert(db.DefaultContext, &auth.OAuth2Application{Name: "test-oauth2-app-2", ClientID: "client-id-2", UID: 1})
36+
assert.NoError(t, err)
37+
err = db.Insert(db.DefaultContext, &auth.OAuth2Application{Name: "test-oauth2-app-3", ClientID: "client-id-3", UID: 99999999})
38+
assert.NoError(t, err)
39+
40+
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-1"})
41+
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-2"})
42+
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-3"})
43+
44+
oauth2AppCheck := checks[idx]
45+
err = oauth2AppCheck.Run(db.DefaultContext, log.GetManager().GetLogger(log.DEFAULT), true)
46+
assert.NoError(t, err)
47+
48+
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-1"})
49+
unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ClientID: "client-id-2"})
50+
unittest.AssertNotExistsBean(t, &auth.OAuth2Application{ClientID: "client-id-3"})
51+
}

‎services/doctor/main_test.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package doctor
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/unittest"
10+
)
11+
12+
func TestMain(m *testing.M) {
13+
unittest.MainTest(m)
14+
}

0 commit comments

Comments
 (0)