From fc686086039b08ba6394b939886d6e4d738d0697 Mon Sep 17 00:00:00 2001 From: Harsh Sharma Date: Thu, 1 Oct 2026 12:38:33 +0530 Subject: [PATCH] fix(oauth2): allow users to approve scope changes (#38942) Lets users approve an OAuth2 scope change on an existing grant instead of failing with `a grant exists with different scope`. - Approving a different scope updates the existing grant. Issued tokens follow immediately, since their scope is read from the grant. - Confidential and trusted apps show the consent page when the scope set changes, instead of silently reusing the old grant. - An omitted `scope` reuses the existing grant's scope, like GitHub. - The consent page lists newly added scopes. Fixes: https://github.com/go-gitea/gitea/issues/38940 Co-authored-by: bircni Co-authored-by: Giteabot Co-authored-by: silverwind --- models/auth/oauth2.go | 6 ++++++ options/locale/locale_en-US.json | 1 + routers/web/auth/oauth2_provider.go | 24 +++++++++++++++++------- routers/web/auth/oauth_test.go | 28 ++++++++++++++++++++++++++++ templates/user/auth/grant.tmpl | 1 + 5 files changed, 53 insertions(+), 7 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index bff4b9db6d9..5a018922d31 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -564,6 +564,12 @@ func (grant *OAuth2Grant) SetNonce(ctx context.Context, nonce string) error { return nil } +func UpdateGrantScope(ctx context.Context, grant *OAuth2Grant, newScope string) error { + grant.Scope = newScope + _, err := db.GetEngine(ctx).ID(grant.ID).Cols("scope").Update(grant) + return err +} + // GetOAuth2GrantByID returns the grant with the given ID func GetOAuth2GrantByID(ctx context.Context, id int64) (grant *OAuth2Grant, err error) { grant = new(OAuth2Grant) diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index c6e574774ba..b66f4b3dc07 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -433,6 +433,7 @@ "auth.authorize_application_created_by": "This application was created by %s.", "auth.authorize_application_description": "If you grant access, it will be able to access and write to all your account information, including private repos and organizations.", "auth.authorize_application_with_scopes": "With scopes: %s", + "auth.authorize_application_new_scopes": "New scopes: %s", "auth.authorize_title": "Authorize \"%s\" to access your account?", "auth.authorization_failed": "Authorization failed", "auth.authorization_failed_desc": "The authorization failed because we detected an invalid request. Please contact the maintainer of the app you tried to authorize.", diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 58a3008c528..6a9b0b973cd 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" "strconv" + "strings" audit_model "gitea.dev/models/audit" "gitea.dev/models/auth" @@ -20,6 +21,7 @@ import ( "gitea.dev/modules/log" "gitea.dev/modules/setting" "gitea.dev/modules/templates" + "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/services/audit" auth_service "gitea.dev/services/auth" @@ -321,9 +323,18 @@ func AuthorizeOAuth(ctx *context.Context) { return } + var addedScopes, removedScopes []string + if grant != nil { + if form.Scope == "" { + form.Scope = grant.Scope + } + addedScopes, removedScopes = util.DiffSlice(strings.Fields(grant.Scope), strings.Fields(form.Scope)) + } + scopeChanged := len(addedScopes) > 0 || len(removedScopes) > 0 + // Redirect if user already granted access and the application is confidential or trusted otherwise // I.e. always require authorization for untrusted public clients as recommended by RFC 6749 Section 10.2 - if (app.ConfidentialClient || app.SkipSecondaryAuthorization) && grant != nil { + if (app.ConfidentialClient || app.SkipSecondaryAuthorization) && grant != nil && !scopeChanged { code, err := grant.GenerateNewAuthorizationCode(ctx, form.RedirectURI, form.CodeChallenge, form.CodeChallengeMethod) if err != nil { handleServerError(ctx, form.State, form.RedirectURI) @@ -347,6 +358,7 @@ func AuthorizeOAuth(ctx *context.Context) { // check if additional scopes ctx.Data["AdditionalScopes"] = oauth2_provider.GrantAdditionalScopes(form.Scope) != auth.AccessTokenScopeAll + ctx.Data["AddedScopes"] = addedScopes // show authorize page to grant access ctx.Data["Application"] = app @@ -432,12 +444,10 @@ func GrantApplicationOAuth(ctx *context.Context) { audit.Record(ctx, audit_model.UserOAuth2ApplicationGrant, ctx.Doer, "oauth2_application", app.Name, "granted_scope", form.Scope) } else if grant.Scope != form.Scope { - handleAuthorizeError(ctx, AuthorizeError{ - State: form.State, - ErrorDescription: "a grant exists with different scope", - ErrorCode: ErrorCodeServerError, - }, form.RedirectURI) - return + if err := auth.UpdateGrantScope(ctx, grant, form.Scope); err != nil { + handleServerError(ctx, form.State, form.RedirectURI) + return + } } if len(form.Nonce) > 0 { diff --git a/routers/web/auth/oauth_test.go b/routers/web/auth/oauth_test.go index 38ee20f41fe..3c2128eb998 100644 --- a/routers/web/auth/oauth_test.go +++ b/routers/web/auth/oauth_test.go @@ -13,6 +13,10 @@ import ( "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/egress/policy" + "gitea.dev/modules/session" + "gitea.dev/modules/web" + "gitea.dev/services/contexttest" + "gitea.dev/services/forms" "gitea.dev/services/oauth2_provider" "github.com/golang-jwt/jwt/v5" @@ -105,3 +109,27 @@ func TestOAuth2AvatarClientBlocksCloudMetadata(t *testing.T) { assert.ErrorIs(t, err, policy.ErrDenied, "avatar client must refuse a link-local cloud-metadata address") } + +func TestOAuth2ScopeChange(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + app := unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Application{ID: 1}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + mockOpt := contexttest.MockContextOption{SessionStore: session.NewMockMemStore("oauth2-scope-change")} + authorize := func(scope string) int { + ctx, resp := contexttest.MockContext(t, "/login/oauth/authorize", mockOpt) + ctx.Doer = doer + web.SetForm(ctx, &forms.AuthorizationForm{ResponseType: "code", ClientID: app.ClientID, RedirectURI: app.RedirectURIs[0], State: "state", Scope: scope}) + AuthorizeOAuth(ctx) + return resp.Code + } + assert.Equal(t, http.StatusSeeOther, authorize("")) + assert.Equal(t, http.StatusSeeOther, authorize("profile openid")) + assert.Equal(t, http.StatusOK, authorize("openid profile email")) + + ctx, resp := contexttest.MockContext(t, "/login/oauth/grant", mockOpt) + ctx.Doer = doer + web.SetForm(ctx, &forms.GrantApplicationForm{ClientID: app.ClientID, Granted: true, RedirectURI: app.RedirectURIs[0], State: "state", Scope: "openid profile email"}) + GrantApplicationOAuth(ctx) + assert.Equal(t, http.StatusSeeOther, resp.Code) + unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Grant{ID: 1, Scope: "openid profile email"}) +} diff --git a/templates/user/auth/grant.tmpl b/templates/user/auth/grant.tmpl index 198d388036f..6ad0c9febc4 100644 --- a/templates/user/auth/grant.tmpl +++ b/templates/user/auth/grant.tmpl @@ -12,6 +12,7 @@ {{end}} {{ctx.Locale.Tr "auth.authorize_application_created_by" .ApplicationCreatorLinkHTML}}
{{ctx.Locale.Tr "auth.authorize_application_with_scopes" (HTMLFormat "%s" .Scope)}} + {{if .AddedScopes}}
{{ctx.Locale.Tr "auth.authorize_application_new_scopes" (HTMLFormat "%s" (StringUtils.Join .AddedScopes " "))}}{{end}}