From 0734596eaa59b115fd5409b96dac0153458224d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Baltaz=C3=A1r=20Radics?= Date: Wed, 27 Nov 2024 07:48:32 +0000 Subject: [PATCH] Use user.FullName in Oauth2 id_token response (#6071) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of [gitea#32542](https://github.com/go-gitea/gitea/pull/32542). This makes /login/oauth/authorize behave the same way as the /login/oauth/userinfo endpoint. Previously, `name` property of the returned OIDCToken used to depend on the UI.DefaultShowFullName setting (I don't think this is desired behavior). Even worse, the `userinfo` endpoint can return basically the same data, but the `name` value there always returned `FullName`, even if it's empty (no fallback to `Name`). A few notes: I'm not sure what branch to target with this PR, please correct me if I'm chose the wrong one. The deleted lines in the tests are duplicates, there's a copy of the whole thing just below, the only difference being the `Name` field (used to test the dependency on the UI.DefaultShowFullName setting) ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [ ] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [ ] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6071 Reviewed-by: Earl Warren Reviewed-by: Gusted Co-authored-by: Baltazár Radics Co-committed-by: Baltazár Radics --- routers/web/auth/oauth.go | 4 ++-- routers/web/auth/oauth_test.go | 36 +++++++++------------------------- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index bf197bd14d..e329729dcd 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -231,7 +231,7 @@ func newAccessTokenResponse(ctx go_context.Context, grant *auth.OAuth2Grant, ser Nonce: grant.Nonce, } if grant.ScopeContains("profile") { - idToken.Name = user.GetDisplayName() + idToken.Name = user.DisplayName() idToken.PreferredUsername = user.Name idToken.Profile = user.HTMLURL() idToken.Picture = user.AvatarLink(ctx) @@ -305,7 +305,7 @@ func InfoOAuth(ctx *context.Context) { response := &userInfoResponse{ Sub: fmt.Sprint(ctx.Doer.ID), - Name: ctx.Doer.FullName, + Name: ctx.Doer.DisplayName(), Username: ctx.Doer.Name, Email: ctx.Doer.Email, Picture: ctx.Doer.AvatarLink(ctx), diff --git a/routers/web/auth/oauth_test.go b/routers/web/auth/oauth_test.go index 5a4a646577..a5f2dd7713 100644 --- a/routers/web/auth/oauth_test.go +++ b/routers/web/auth/oauth_test.go @@ -10,7 +10,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/services/auth/source/oauth2" "github.com/golang-jwt/jwt/v5" @@ -67,32 +67,14 @@ func TestNewAccessTokenResponse_OIDCToken(t *testing.T) { // Scopes: openid profile email oidcToken = createAndParseToken(t, grants[0]) - assert.Equal(t, user.Name, oidcToken.Name) - assert.Equal(t, user.Name, oidcToken.PreferredUsername) - assert.Equal(t, user.HTMLURL(), oidcToken.Profile) - assert.Equal(t, user.AvatarLink(db.DefaultContext), oidcToken.Picture) - assert.Equal(t, user.Website, oidcToken.Website) - assert.Equal(t, user.UpdatedUnix, oidcToken.UpdatedAt) - assert.Equal(t, user.Email, oidcToken.Email) - assert.Equal(t, user.IsActive, oidcToken.EmailVerified) - - // set DefaultShowFullName to true - oldDefaultShowFullName := setting.UI.DefaultShowFullName - setting.UI.DefaultShowFullName = true - defer func() { - setting.UI.DefaultShowFullName = oldDefaultShowFullName - }() - - // Scopes: openid profile email - oidcToken = createAndParseToken(t, grants[0]) - assert.Equal(t, user.FullName, oidcToken.Name) - assert.Equal(t, user.Name, oidcToken.PreferredUsername) - assert.Equal(t, user.HTMLURL(), oidcToken.Profile) - assert.Equal(t, user.AvatarLink(db.DefaultContext), oidcToken.Picture) - assert.Equal(t, user.Website, oidcToken.Website) - assert.Equal(t, user.UpdatedUnix, oidcToken.UpdatedAt) - assert.Equal(t, user.Email, oidcToken.Email) - assert.Equal(t, user.IsActive, oidcToken.EmailVerified) + assert.Equal(t, "User Five", oidcToken.Name) + assert.Equal(t, "user5", oidcToken.PreferredUsername) + assert.Equal(t, "https://try.gitea.io/user5", oidcToken.Profile) + assert.Equal(t, "https://try.gitea.io/assets/img/avatar_default.png", oidcToken.Picture) + assert.Equal(t, "", oidcToken.Website) + assert.Equal(t, timeutil.TimeStamp(0), oidcToken.UpdatedAt) + assert.Equal(t, "user5@example.com", oidcToken.Email) + assert.True(t, oidcToken.EmailVerified) } func TestEncodeCodeChallenge(t *testing.T) {