Skip to content

Commit a5a3d9b

Browse files
wxiaoguangwxiaoguang
authored
Refactor OpenIDConnect to support SSH/FullName sync (#34978)
* Fix #26585 * Fix #28327 * Fix #34932
1 parent 6ab6d4e commit a5a3d9b

File tree

27 files changed

+459
-206
lines changed

27 files changed

+459
-206
lines changed

‎cmd/admin_auth_oauth.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ func oauthCLIFlags() []cli.Flag {
8787
Value: nil,
8888
Usage: "Scopes to request when to authenticate against this OAuth2 source",
8989
},
90+
&cli.StringFlag{
91+
Name: "ssh-public-key-claim-name",
92+
Usage: "Claim name that provides SSH public keys",
93+
},
94+
&cli.StringFlag{
95+
Name: "full-name-claim-name",
96+
Usage: "Claim name that provides user's full name",
97+
},
9098
&cli.StringFlag{
9199
Name: "required-claim-name",
92100
Value: "",
@@ -177,6 +185,8 @@ func parseOAuth2Config(c *cli.Command) *oauth2.Source {
177185
RestrictedGroup: c.String("restricted-group"),
178186
GroupTeamMap: c.String("group-team-map"),
179187
GroupTeamMapRemoval: c.Bool("group-team-map-removal"),
188+
SSHPublicKeyClaimName: c.String("ssh-public-key-claim-name"),
189+
FullNameClaimName: c.String("full-name-claim-name"),
180190
}
181191
}
182192

@@ -268,6 +278,12 @@ func (a *authService) runUpdateOauth(ctx context.Context, c *cli.Command) error
268278
if c.IsSet("group-team-map-removal") {
269279
oAuth2Config.GroupTeamMapRemoval = c.Bool("group-team-map-removal")
270280
}
281+
if c.IsSet("ssh-public-key-claim-name") {
282+
oAuth2Config.SSHPublicKeyClaimName = c.String("ssh-public-key-claim-name")
283+
}
284+
if c.IsSet("full-name-claim-name") {
285+
oAuth2Config.FullNameClaimName = c.String("full-name-claim-name")
286+
}
271287

272288
// update custom URL mapping
273289
customURLMapping := &oauth2.CustomURLMapping{}

‎cmd/admin_auth_oauth_test.go

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ func TestAddOauth(t *testing.T) {
8888
"--restricted-group", "restricted",
8989
"--group-team-map", `{"group1": [1,2]}`,
9090
"--group-team-map-removal=true",
91+
"--ssh-public-key-claim-name", "attr_ssh_pub_key",
92+
"--full-name-claim-name", "attr_full_name",
9193
},
9294
source: &auth_model.Source{
9395
Type: auth_model.OAuth2,
@@ -104,15 +106,17 @@ func TestAddOauth(t *testing.T) {
104106
EmailURL: "https://example.com/email",
105107
Tenant: "some_tenant",
106108
},
107-
IconURL: "https://example.com/icon",
108-
Scopes: []string{"scope1", "scope2"},
109-
RequiredClaimName: "claim_name",
110-
RequiredClaimValue: "claim_value",
111-
GroupClaimName: "group_name",
112-
AdminGroup: "admin",
113-
RestrictedGroup: "restricted",
114-
GroupTeamMap: `{"group1": [1,2]}`,
115-
GroupTeamMapRemoval: true,
109+
IconURL: "https://example.com/icon",
110+
Scopes: []string{"scope1", "scope2"},
111+
RequiredClaimName: "claim_name",
112+
RequiredClaimValue: "claim_value",
113+
GroupClaimName: "group_name",
114+
AdminGroup: "admin",
115+
RestrictedGroup: "restricted",
116+
GroupTeamMap: `{"group1": [1,2]}`,
117+
GroupTeamMapRemoval: true,
118+
SSHPublicKeyClaimName: "attr_ssh_pub_key",
119+
FullNameClaimName: "attr_full_name",
116120
},
117121
TwoFactorPolicy: "skip",
118122
},
@@ -223,15 +227,17 @@ func TestUpdateOauth(t *testing.T) {
223227
EmailURL: "https://old.example.com/email",
224228
Tenant: "old_tenant",
225229
},
226-
IconURL: "https://old.example.com/icon",
227-
Scopes: []string{"old_scope1", "old_scope2"},
228-
RequiredClaimName: "old_claim_name",
229-
RequiredClaimValue: "old_claim_value",
230-
GroupClaimName: "old_group_name",
231-
AdminGroup: "old_admin",
232-
RestrictedGroup: "old_restricted",
233-
GroupTeamMap: `{"old_group1": [1,2]}`,
234-
GroupTeamMapRemoval: true,
230+
IconURL: "https://old.example.com/icon",
231+
Scopes: []string{"old_scope1", "old_scope2"},
232+
RequiredClaimName: "old_claim_name",
233+
RequiredClaimValue: "old_claim_value",
234+
GroupClaimName: "old_group_name",
235+
AdminGroup: "old_admin",
236+
RestrictedGroup: "old_restricted",
237+
GroupTeamMap: `{"old_group1": [1,2]}`,
238+
GroupTeamMapRemoval: true,
239+
SSHPublicKeyClaimName: "old_ssh_pub_key",
240+
FullNameClaimName: "old_full_name",
235241
},
236242
TwoFactorPolicy: "",
237243
},
@@ -257,6 +263,8 @@ func TestUpdateOauth(t *testing.T) {
257263
"--restricted-group", "restricted",
258264
"--group-team-map", `{"group1": [1,2]}`,
259265
"--group-team-map-removal=false",
266+
"--ssh-public-key-claim-name", "new_ssh_pub_key",
267+
"--full-name-claim-name", "new_full_name",
260268
},
261269
authSource: &auth_model.Source{
262270
ID: 1,
@@ -274,15 +282,17 @@ func TestUpdateOauth(t *testing.T) {
274282
EmailURL: "https://example.com/email",
275283
Tenant: "new_tenant",
276284
},
277-
IconURL: "https://example.com/icon",
278-
Scopes: []string{"scope1", "scope2"},
279-
RequiredClaimName: "claim_name",
280-
RequiredClaimValue: "claim_value",
281-
GroupClaimName: "group_name",
282-
AdminGroup: "admin",
283-
RestrictedGroup: "restricted",
284-
GroupTeamMap: `{"group1": [1,2]}`,
285-
GroupTeamMapRemoval: false,
285+
IconURL: "https://example.com/icon",
286+
Scopes: []string{"scope1", "scope2"},
287+
RequiredClaimName: "claim_name",
288+
RequiredClaimValue: "claim_value",
289+
GroupClaimName: "group_name",
290+
AdminGroup: "admin",
291+
RestrictedGroup: "restricted",
292+
GroupTeamMap: `{"group1": [1,2]}`,
293+
GroupTeamMapRemoval: false,
294+
SSHPublicKeyClaimName: "new_ssh_pub_key",
295+
FullNameClaimName: "new_full_name",
286296
},
287297
TwoFactorPolicy: "skip",
288298
},

‎models/asymkey/ssh_key.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,13 +355,13 @@ func AddPublicKeysBySource(ctx context.Context, usr *user_model.User, s *auth.So
355355
return sshKeysNeedUpdate
356356
}
357357

358-
// SynchronizePublicKeys updates a users public keys. Returns true if there are changes.
358+
// SynchronizePublicKeys updates a user's public keys. Returns true if there are changes.
359359
func SynchronizePublicKeys(ctx context.Context, usr *user_model.User, s *auth.Source, sshPublicKeys []string) bool {
360360
var sshKeysNeedUpdate bool
361361

362362
log.Trace("synchronizePublicKeys[%s]: Handling Public SSH Key synchronization for user %s", s.Name, usr.Name)
363363

364-
// Get Public Keys from DB with current LDAP source
364+
// Get Public Keys from DB with the current auth source
365365
var giteaKeys []string
366366
keys, err := db.Find[PublicKey](ctx, FindPublicKeyOptions{
367367
OwnerID: usr.ID,

‎models/auth/oauth2.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -612,8 +612,8 @@ func (err ErrOAuthApplicationNotFound) Unwrap() error {
612612
return util.ErrNotExist
613613
}
614614

615-
// GetActiveOAuth2SourceByName returns a OAuth2 AuthSource based on the given name
616-
func GetActiveOAuth2SourceByName(ctx context.Context, name string) (*Source, error) {
615+
// GetActiveOAuth2SourceByAuthName returns a OAuth2 AuthSource based on the given name
616+
func GetActiveOAuth2SourceByAuthName(ctx context.Context, name string) (*Source, error) {
617617
authSource := new(Source)
618618
has, err := db.GetEngine(ctx).Where("name = ? and type = ? and is_active = ?", name, OAuth2, true).Get(authSource)
619619
if err != nil {

‎models/auth/source.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ func UpdateSource(ctx context.Context, source *Source) error {
334334

335335
err = registerableSource.RegisterSource()
336336
if err != nil {
337-
// restore original values since we cannot update the provider it self
337+
// restore original values since we cannot update the provider itself
338338
if _, err := db.GetEngine(ctx).ID(source.ID).AllCols().Update(originalSource); err != nil {
339339
log.Error("UpdateSource: Error while wrapOpenIDConnectInitializeError: %v", err)
340340
}

‎modules/setting/oauth2.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
"code.gitea.io/gitea/modules/log"
1313
)
1414

15-
// OAuth2UsernameType is enum describing the way gitea 'name' should be generated from oauth2 data
15+
// OAuth2UsernameType is enum describing the way gitea generates its 'username' from oauth2 data
1616
type OAuth2UsernameType string
1717

1818
const (

‎options/locale/locale_en-US.ini

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3251,6 +3251,8 @@ auths.oauth2_required_claim_name_helper = Set this name to restrict login from t
32513251
auths.oauth2_required_claim_value = Required Claim Value
32523252
auths.oauth2_required_claim_value_helper = Set this value to restrict login from this source to users with a claim with this name and value
32533253
auths.oauth2_group_claim_name = Claim name providing group names for this source. (Optional)
3254+
auths.oauth2_full_name_claim_name = Full Name Claim Name. (Optional, if set, the user's full name will always be synchronized with this claim)
3255+
auths.oauth2_ssh_public_key_claim_name = SSH Public Key Claim Name
32543256
auths.oauth2_admin_group = Group Claim value for administrator users. (Optional - requires claim name above)
32553257
auths.oauth2_restricted_group = Group Claim value for restricted users. (Optional - requires claim name above)
32563258
auths.oauth2_map_group_to_team = Map claimed groups to Organization teams. (Optional - requires claim name above)

‎routers/web/admin/auths.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source {
199199
AdminGroup: form.Oauth2AdminGroup,
200200
GroupTeamMap: form.Oauth2GroupTeamMap,
201201
GroupTeamMapRemoval: form.Oauth2GroupTeamMapRemoval,
202+
203+
SSHPublicKeyClaimName: form.Oauth2SSHPublicKeyClaimName,
204+
FullNameClaimName: form.Oauth2FullNameClaimName,
202205
}
203206
}
204207

‎routers/web/auth/2fa.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"code.gitea.io/gitea/modules/templates"
1515
"code.gitea.io/gitea/modules/web"
1616
"code.gitea.io/gitea/services/context"
17-
"code.gitea.io/gitea/services/externalaccount"
1817
"code.gitea.io/gitea/services/forms"
1918
)
2019

@@ -75,7 +74,7 @@ func TwoFactorPost(ctx *context.Context) {
7574
}
7675

7776
if ctx.Session.Get("linkAccount") != nil {
78-
err = externalaccount.LinkAccountFromStore(ctx, ctx.Session, u)
77+
err = linkAccountFromContext(ctx, u)
7978
if err != nil {
8079
ctx.ServerError("UserSignIn", err)
8180
return

‎routers/web/auth/auth.go

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
329329
"twofaUid",
330330
"twofaRemember",
331331
"linkAccount",
332+
"linkAccountData",
332333
}, map[string]any{
333334
session.KeyUID: u.ID,
334335
session.KeyUname: u.Name,
@@ -519,7 +520,7 @@ func SignUpPost(ctx *context.Context) {
519520
Passwd: form.Password,
520521
}
521522

522-
if !createAndHandleCreatedUser(ctx, tplSignUp, form, u, nil, nil, false) {
523+
if !createAndHandleCreatedUser(ctx, tplSignUp, form, u, nil, nil) {
523524
// error already handled
524525
return
525526
}
@@ -530,22 +531,22 @@ func SignUpPost(ctx *context.Context) {
530531

531532
// createAndHandleCreatedUser calls createUserInContext and
532533
// then handleUserCreated.
533-
func createAndHandleCreatedUser(ctx *context.Context, tpl templates.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, gothUser *goth.User, allowLink bool) bool {
534-
if !createUserInContext(ctx, tpl, form, u, overwrites, gothUser, allowLink) {
534+
func createAndHandleCreatedUser(ctx *context.Context, tpl templates.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, possibleLinkAccountData *LinkAccountData) bool {
535+
if !createUserInContext(ctx, tpl, form, u, overwrites, possibleLinkAccountData) {
535536
return false
536537
}
537-
return handleUserCreated(ctx, u, gothUser)
538+
return handleUserCreated(ctx, u, possibleLinkAccountData)
538539
}
539540

540541
// createUserInContext creates a user and handles errors within a given context.
541-
// Optionally a template can be specified.
542-
func createUserInContext(ctx *context.Context, tpl templates.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, gothUser *goth.User, allowLink bool) (ok bool) {
542+
// Optionally, a template can be specified.
543+
func createUserInContext(ctx *context.Context, tpl templates.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, possibleLinkAccountData *LinkAccountData) (ok bool) {
543544
meta := &user_model.Meta{
544545
InitialIP: ctx.RemoteAddr(),
545546
InitialUserAgent: ctx.Req.UserAgent(),
546547
}
547548
if err := user_model.CreateUser(ctx, u, meta, overwrites); err != nil {
548-
if allowLink && (user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err)) {
549+
if possibleLinkAccountData != nil && (user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err)) {
549550
switch setting.OAuth2Client.AccountLinking {
550551
case setting.OAuth2AccountLinkingAuto:
551552
var user *user_model.User
@@ -561,15 +562,15 @@ func createUserInContext(ctx *context.Context, tpl templates.TplName, form any,
561562
}
562563

563564
// TODO: probably we should respect 'remember' user's choice...
564-
linkAccount(ctx, user, *gothUser, true)
565+
oauth2LinkAccount(ctx, user, possibleLinkAccountData, true)
565566
return false // user is already created here, all redirects are handled
566567
case setting.OAuth2AccountLinkingLogin:
567-
showLinkingLogin(ctx, *gothUser)
568+
showLinkingLogin(ctx, &possibleLinkAccountData.AuthSource, possibleLinkAccountData.GothUser)
568569
return false // user will be created only after linking login
569570
}
570571
}
571572

572-
// handle error without template
573+
// handle error without a template
573574
if len(tpl) == 0 {
574575
ctx.ServerError("CreateUser", err)
575576
return false
@@ -610,7 +611,7 @@ func createUserInContext(ctx *context.Context, tpl templates.TplName, form any,
610611
// handleUserCreated does additional steps after a new user is created.
611612
// It auto-sets admin for the only user, updates the optional external user and
612613
// sends a confirmation email if required.
613-
func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth.User) (ok bool) {
614+
func handleUserCreated(ctx *context.Context, u *user_model.User, possibleLinkAccountData *LinkAccountData) (ok bool) {
614615
// Auto-set admin for the only user.
615616
hasUsers, err := user_model.HasUsers(ctx)
616617
if err != nil {
@@ -631,8 +632,8 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth.
631632
}
632633

633634
// update external user information
634-
if gothUser != nil {
635-
if err := externalaccount.EnsureLinkExternalToUser(ctx, u, *gothUser); err != nil {
635+
if possibleLinkAccountData != nil {
636+
if err := externalaccount.EnsureLinkExternalToUser(ctx, possibleLinkAccountData.AuthSource.ID, u, possibleLinkAccountData.GothUser); err != nil {
636637
log.Error("EnsureLinkExternalToUser failed: %v", err)
637638
}
638639
}

0 commit comments

Comments
 (0)