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 <bircni@icloud.com>
Co-authored-by: Giteabot <teabot@gitea.io>
Co-authored-by: silverwind <me@silverwind.io>

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)

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.",

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 {

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"})
+}

templates/user/auth/grant.tmpl

@@ -12,6 +12,7 @@
 				{{end}}
 				{{ctx.Locale.Tr "auth.authorize_application_created_by" .ApplicationCreatorLinkHTML}}<br>
 				{{ctx.Locale.Tr "auth.authorize_application_with_scopes" (HTMLFormat "<b>%s</b>" .Scope)}}
+				{{if .AddedScopes}}<br>{{ctx.Locale.Tr "auth.authorize_application_new_scopes" (HTMLFormat "<b>%s</b>" (StringUtils.Join .AddedScopes " "))}}{{end}}
 			</p>
 		</div>
 		<div class="ui attached segment">