Skip to content

Commit 1bf5527

Browse files
lunnylunny
authored
Refactor Find Sources and fix bug when view a user who belongs to an unactive auth source (#27798)
The steps to reproduce it. First, create a new oauth2 source. Then, a user login with this oauth2 source. Disable the oauth2 source. Visit users -> settings -> security, 500 will be displayed. This is because this page only load active Oauth2 sources but not all Oauth2 sources.
1 parent 80715ae commit 1bf5527

File tree

15 files changed

+115
-91
lines changed

15 files changed

+115
-91
lines changed

‎cmd/admin_auth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func runListAuth(c *cli.Context) error {
6262
return err
6363
}
6464

65-
authSources, err := auth_model.Sources(ctx)
65+
authSources, err := auth_model.FindSources(ctx, auth_model.FindSourcesOptions{})
6666
if err != nil {
6767
return err
6868
}

‎models/activities/statistic.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func GetStatistic(ctx context.Context) (stats Statistic) {
102102
stats.Counter.Follow, _ = e.Count(new(user_model.Follow))
103103
stats.Counter.Mirror, _ = e.Count(new(repo_model.Mirror))
104104
stats.Counter.Release, _ = e.Count(new(repo_model.Release))
105-
stats.Counter.AuthSource = auth.CountSources(ctx)
105+
stats.Counter.AuthSource = auth.CountSources(ctx, auth.FindSourcesOptions{})
106106
stats.Counter.Webhook, _ = e.Count(new(webhook.Webhook))
107107
stats.Counter.Milestone, _ = e.Count(new(issues_model.Milestone))
108108
stats.Counter.Label, _ = e.Count(new(issues_model.Label))

‎models/auth/oauth2.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -631,15 +631,6 @@ func (err ErrOAuthApplicationNotFound) Unwrap() error {
631631
return util.ErrNotExist
632632
}
633633

634-
// GetActiveOAuth2ProviderSources returns all actived LoginOAuth2 sources
635-
func GetActiveOAuth2ProviderSources(ctx context.Context) ([]*Source, error) {
636-
sources := make([]*Source, 0, 1)
637-
if err := db.GetEngine(ctx).Where("is_active = ? and type = ?", true, OAuth2).Find(&sources); err != nil {
638-
return nil, err
639-
}
640-
return sources, nil
641-
}
642-
643634
// GetActiveOAuth2SourceByName returns a OAuth2 AuthSource based on the given name
644635
func GetActiveOAuth2SourceByName(ctx context.Context, name string) (*Source, error) {
645636
authSource := new(Source)

‎models/auth/source.go

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/gitea/modules/timeutil"
1515
"code.gitea.io/gitea/modules/util"
1616

17+
"xorm.io/builder"
1718
"xorm.io/xorm"
1819
"xorm.io/xorm/convert"
1920
)
@@ -240,37 +241,26 @@ func CreateSource(ctx context.Context, source *Source) error {
240241
return err
241242
}
242243

243-
// Sources returns a slice of all login sources found in DB.
244-
func Sources(ctx context.Context) ([]*Source, error) {
245-
auths := make([]*Source, 0, 6)
246-
return auths, db.GetEngine(ctx).Find(&auths)
244+
type FindSourcesOptions struct {
245+
IsActive util.OptionalBool
246+
LoginType Type
247247
}
248248

249-
// SourcesByType returns all sources of the specified type
250-
func SourcesByType(ctx context.Context, loginType Type) ([]*Source, error) {
251-
sources := make([]*Source, 0, 1)
252-
if err := db.GetEngine(ctx).Where("type = ?", loginType).Find(&sources); err != nil {
253-
return nil, err
249+
func (opts FindSourcesOptions) ToConds() builder.Cond {
250+
conds := builder.NewCond()
251+
if !opts.IsActive.IsNone() {
252+
conds = conds.And(builder.Eq{"is_active": opts.IsActive.IsTrue()})
254253
}
255-
return sources, nil
256-
}
257-
258-
// AllActiveSources returns all active sources
259-
func AllActiveSources(ctx context.Context) ([]*Source, error) {
260-
sources := make([]*Source, 0, 5)
261-
if err := db.GetEngine(ctx).Where("is_active = ?", true).Find(&sources); err != nil {
262-
return nil, err
254+
if opts.LoginType != NoType {
255+
conds = conds.And(builder.Eq{"`type`": opts.LoginType})
263256
}
264-
return sources, nil
257+
return conds
265258
}
266259

267-
// ActiveSources returns all active sources of the specified type
268-
func ActiveSources(ctx context.Context, tp Type) ([]*Source, error) {
269-
sources := make([]*Source, 0, 1)
270-
if err := db.GetEngine(ctx).Where("is_active = ? and type = ?", true, tp).Find(&sources); err != nil {
271-
return nil, err
272-
}
273-
return sources, nil
260+
// FindSources returns a slice of login sources found in DB according to given conditions.
261+
func FindSources(ctx context.Context, opts FindSourcesOptions) ([]*Source, error) {
262+
auths := make([]*Source, 0, 6)
263+
return auths, db.GetEngine(ctx).Where(opts.ToConds()).Find(&auths)
274264
}
275265

276266
// IsSSPIEnabled returns true if there is at least one activated login
@@ -279,7 +269,10 @@ func IsSSPIEnabled(ctx context.Context) bool {
279269
if !db.HasEngine {
280270
return false
281271
}
282-
sources, err := ActiveSources(ctx, SSPI)
272+
sources, err := FindSources(ctx, FindSourcesOptions{
273+
IsActive: util.OptionalBoolTrue,
274+
LoginType: SSPI,
275+
})
283276
if err != nil {
284277
log.Error("ActiveSources: %v", err)
285278
return false
@@ -354,8 +347,8 @@ func UpdateSource(ctx context.Context, source *Source) error {
354347
}
355348

356349
// CountSources returns number of login sources.
357-
func CountSources(ctx context.Context) int64 {
358-
count, _ := db.GetEngine(ctx).Count(new(Source))
350+
func CountSources(ctx context.Context, opts FindSourcesOptions) int64 {
351+
count, _ := db.GetEngine(ctx).Where(opts.ToConds()).Count(new(Source))
359352
return count
360353
}
361354

‎routers/web/admin/auths.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ func Authentications(ctx *context.Context) {
4848
ctx.Data["PageIsAdminAuthentications"] = true
4949

5050
var err error
51-
ctx.Data["Sources"], err = auth.Sources(ctx)
51+
ctx.Data["Sources"], err = auth.FindSources(ctx, auth.FindSourcesOptions{})
5252
if err != nil {
5353
ctx.ServerError("auth.Sources", err)
5454
return
5555
}
5656

57-
ctx.Data["Total"] = auth.CountSources(ctx)
57+
ctx.Data["Total"] = auth.CountSources(ctx, auth.FindSourcesOptions{})
5858
ctx.HTML(http.StatusOK, tplAuths)
5959
}
6060

@@ -99,7 +99,7 @@ func NewAuthSource(ctx *context.Context) {
9999
ctx.Data["AuthSources"] = authSources
100100
ctx.Data["SecurityProtocols"] = securityProtocols
101101
ctx.Data["SMTPAuths"] = smtp.Authenticators
102-
oauth2providers := oauth2.GetOAuth2Providers()
102+
oauth2providers := oauth2.GetSupportedOAuth2Providers()
103103
ctx.Data["OAuth2Providers"] = oauth2providers
104104

105105
ctx.Data["SSPIAutoCreateUsers"] = true
@@ -242,7 +242,7 @@ func NewAuthSourcePost(ctx *context.Context) {
242242
ctx.Data["AuthSources"] = authSources
243243
ctx.Data["SecurityProtocols"] = securityProtocols
244244
ctx.Data["SMTPAuths"] = smtp.Authenticators
245-
oauth2providers := oauth2.GetOAuth2Providers()
245+
oauth2providers := oauth2.GetSupportedOAuth2Providers()
246246
ctx.Data["OAuth2Providers"] = oauth2providers
247247

248248
ctx.Data["SSPIAutoCreateUsers"] = true
@@ -284,7 +284,7 @@ func NewAuthSourcePost(ctx *context.Context) {
284284
ctx.RenderWithErr(err.Error(), tplAuthNew, form)
285285
return
286286
}
287-
existing, err := auth.SourcesByType(ctx, auth.SSPI)
287+
existing, err := auth.FindSources(ctx, auth.FindSourcesOptions{LoginType: auth.SSPI})
288288
if err != nil || len(existing) > 0 {
289289
ctx.Data["Err_Type"] = true
290290
ctx.RenderWithErr(ctx.Tr("admin.auths.login_source_of_type_exist"), tplAuthNew, form)
@@ -334,7 +334,7 @@ func EditAuthSource(ctx *context.Context) {
334334

335335
ctx.Data["SecurityProtocols"] = securityProtocols
336336
ctx.Data["SMTPAuths"] = smtp.Authenticators
337-
oauth2providers := oauth2.GetOAuth2Providers()
337+
oauth2providers := oauth2.GetSupportedOAuth2Providers()
338338
ctx.Data["OAuth2Providers"] = oauth2providers
339339

340340
source, err := auth.GetSourceByID(ctx, ctx.ParamsInt64(":authid"))
@@ -368,7 +368,7 @@ func EditAuthSourcePost(ctx *context.Context) {
368368
ctx.Data["PageIsAdminAuthentications"] = true
369369

370370
ctx.Data["SMTPAuths"] = smtp.Authenticators
371-
oauth2providers := oauth2.GetOAuth2Providers()
371+
oauth2providers := oauth2.GetSupportedOAuth2Providers()
372372
ctx.Data["OAuth2Providers"] = oauth2providers
373373

374374
source, err := auth.GetSourceByID(ctx, ctx.ParamsInt64(":authid"))

‎routers/web/admin/users.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,9 @@ func NewUser(ctx *context.Context) {
9090

9191
ctx.Data["login_type"] = "0-0"
9292

93-
sources, err := auth.Sources(ctx)
93+
sources, err := auth.FindSources(ctx, auth.FindSourcesOptions{
94+
IsActive: util.OptionalBoolTrue,
95+
})
9496
if err != nil {
9597
ctx.ServerError("auth.Sources", err)
9698
return
@@ -109,7 +111,9 @@ func NewUserPost(ctx *context.Context) {
109111
ctx.Data["DefaultUserVisibilityMode"] = setting.Service.DefaultUserVisibilityMode
110112
ctx.Data["AllowedUserVisibilityModes"] = setting.Service.AllowedUserVisibilityModesSlice.ToVisibleTypeSlice()
111113

112-
sources, err := auth.Sources(ctx)
114+
sources, err := auth.FindSources(ctx, auth.FindSourcesOptions{
115+
IsActive: util.OptionalBoolTrue,
116+
})
113117
if err != nil {
114118
ctx.ServerError("auth.Sources", err)
115119
return
@@ -230,7 +234,7 @@ func prepareUserInfo(ctx *context.Context) *user_model.User {
230234
ctx.Data["LoginSource"] = &auth.Source{}
231235
}
232236

233-
sources, err := auth.Sources(ctx)
237+
sources, err := auth.FindSources(ctx, auth.FindSourcesOptions{})
234238
if err != nil {
235239
ctx.ServerError("auth.Sources", err)
236240
return nil

‎routers/web/auth/auth.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,11 @@ func SignIn(ctx *context.Context) {
160160
return
161161
}
162162

163-
orderedOAuth2Names, oauth2Providers, err := oauth2.GetActiveOAuth2Providers(ctx)
163+
oauth2Providers, err := oauth2.GetOAuth2Providers(ctx, util.OptionalBoolTrue)
164164
if err != nil {
165165
ctx.ServerError("UserSignIn", err)
166166
return
167167
}
168-
ctx.Data["OrderedOAuth2Names"] = orderedOAuth2Names
169168
ctx.Data["OAuth2Providers"] = oauth2Providers
170169
ctx.Data["Title"] = ctx.Tr("sign_in")
171170
ctx.Data["SignInLink"] = setting.AppSubURL + "/user/login"
@@ -184,12 +183,11 @@ func SignIn(ctx *context.Context) {
184183
func SignInPost(ctx *context.Context) {
185184
ctx.Data["Title"] = ctx.Tr("sign_in")
186185

187-
orderedOAuth2Names, oauth2Providers, err := oauth2.GetActiveOAuth2Providers(ctx)
186+
oauth2Providers, err := oauth2.GetOAuth2Providers(ctx, util.OptionalBoolTrue)
188187
if err != nil {
189188
ctx.ServerError("UserSignIn", err)
190189
return
191190
}
192-
ctx.Data["OrderedOAuth2Names"] = orderedOAuth2Names
193191
ctx.Data["OAuth2Providers"] = oauth2Providers
194192
ctx.Data["Title"] = ctx.Tr("sign_in")
195193
ctx.Data["SignInLink"] = setting.AppSubURL + "/user/login"
@@ -408,13 +406,12 @@ func SignUp(ctx *context.Context) {
408406

409407
ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/sign_up"
410408

411-
orderedOAuth2Names, oauth2Providers, err := oauth2.GetActiveOAuth2Providers(ctx)
409+
oauth2Providers, err := oauth2.GetOAuth2Providers(ctx, util.OptionalBoolTrue)
412410
if err != nil {
413411
ctx.ServerError("UserSignUp", err)
414412
return
415413
}
416414

417-
ctx.Data["OrderedOAuth2Names"] = orderedOAuth2Names
418415
ctx.Data["OAuth2Providers"] = oauth2Providers
419416
context.SetCaptchaData(ctx)
420417

@@ -438,13 +435,12 @@ func SignUpPost(ctx *context.Context) {
438435

439436
ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/sign_up"
440437

441-
orderedOAuth2Names, oauth2Providers, err := oauth2.GetActiveOAuth2Providers(ctx)
438+
oauth2Providers, err := oauth2.GetOAuth2Providers(ctx, util.OptionalBoolTrue)
442439
if err != nil {
443440
ctx.ServerError("UserSignUp", err)
444441
return
445442
}
446443

447-
ctx.Data["OrderedOAuth2Names"] = orderedOAuth2Names
448444
ctx.Data["OAuth2Providers"] = oauth2Providers
449445
context.SetCaptchaData(ctx)
450446

‎routers/web/user/setting/security/security.go

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ package security
66

77
import (
88
"net/http"
9+
"sort"
910

1011
auth_model "code.gitea.io/gitea/models/auth"
1112
user_model "code.gitea.io/gitea/models/user"
1213
"code.gitea.io/gitea/modules/base"
1314
"code.gitea.io/gitea/modules/context"
1415
"code.gitea.io/gitea/modules/setting"
16+
"code.gitea.io/gitea/modules/util"
1517
"code.gitea.io/gitea/services/auth/source/oauth2"
1618
)
1719

@@ -105,11 +107,31 @@ func loadSecurityData(ctx *context.Context) {
105107
}
106108
ctx.Data["AccountLinks"] = sources
107109

108-
orderedOAuth2Names, oauth2Providers, err := oauth2.GetActiveOAuth2Providers(ctx)
110+
authSources, err := auth_model.FindSources(ctx, auth_model.FindSourcesOptions{
111+
IsActive: util.OptionalBoolNone,
112+
LoginType: auth_model.OAuth2,
113+
})
109114
if err != nil {
110-
ctx.ServerError("GetActiveOAuth2Providers", err)
115+
ctx.ServerError("FindSources", err)
111116
return
112117
}
118+
119+
var orderedOAuth2Names []string
120+
oauth2Providers := make(map[string]oauth2.Provider)
121+
for _, source := range authSources {
122+
provider, err := oauth2.CreateProviderFromSource(source)
123+
if err != nil {
124+
ctx.ServerError("CreateProviderFromSource", err)
125+
return
126+
}
127+
oauth2Providers[source.Name] = provider
128+
if source.IsActive {
129+
orderedOAuth2Names = append(orderedOAuth2Names, source.Name)
130+
}
131+
}
132+
133+
sort.Strings(orderedOAuth2Names)
134+
113135
ctx.Data["OrderedOAuth2Names"] = orderedOAuth2Names
114136
ctx.Data["OAuth2Providers"] = oauth2Providers
115137

‎services/auth/signin.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"code.gitea.io/gitea/models/db"
1212
user_model "code.gitea.io/gitea/models/user"
1313
"code.gitea.io/gitea/modules/log"
14+
"code.gitea.io/gitea/modules/util"
1415
"code.gitea.io/gitea/services/auth/source/oauth2"
1516
"code.gitea.io/gitea/services/auth/source/smtp"
1617

@@ -85,7 +86,9 @@ func UserSignIn(ctx context.Context, username, password string) (*user_model.Use
8586
}
8687
}
8788

88-
sources, err := auth.AllActiveSources(ctx)
89+
sources, err := auth.FindSources(ctx, auth.FindSourcesOptions{
90+
IsActive: util.OptionalBoolTrue,
91+
})
8992
if err != nil {
9093
return nil, nil, err
9194
}

‎services/auth/source/oauth2/init.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"code.gitea.io/gitea/models/auth"
1313
"code.gitea.io/gitea/modules/log"
1414
"code.gitea.io/gitea/modules/setting"
15+
"code.gitea.io/gitea/modules/util"
1516

1617
"github.com/google/uuid"
1718
"github.com/gorilla/sessions"
@@ -63,7 +64,13 @@ func ResetOAuth2(ctx context.Context) error {
6364

6465
// initOAuth2Sources is used to load and register all active OAuth2 providers
6566
func initOAuth2Sources(ctx context.Context) error {
66-
authSources, _ := auth.GetActiveOAuth2ProviderSources(ctx)
67+
authSources, err := auth.FindSources(ctx, auth.FindSourcesOptions{
68+
IsActive: util.OptionalBoolTrue,
69+
LoginType: auth.OAuth2,
70+
})
71+
if err != nil {
72+
return err
73+
}
6774
for _, source := range authSources {
6875
oauth2Source, ok := source.Cfg.(*Source)
6976
if !ok {

0 commit comments

Comments
 (0)