diff --git a/models/auth/access_token_scope.go b/models/auth/access_token_scope.go index 94833f18744..1585bcbcd06 100644 --- a/models/auth/access_token_scope.go +++ b/models/auth/access_token_scope.go @@ -54,7 +54,8 @@ type AccessTokenScope string // for all categories, write implies read const ( AccessTokenScopeAll AccessTokenScope = "all" - AccessTokenScopePublicOnly AccessTokenScope = "public-only" // limited to public orgs/repos + AccessTokenScopeNone AccessTokenScope = "no-token-scope" // no token permission scopes, so no API permissions + AccessTokenScopePublicOnly AccessTokenScope = "public-only" // limited to public orgs/repos AccessTokenScopeReadActivityPub AccessTokenScope = "read:activitypub" AccessTokenScopeWriteActivityPub AccessTokenScope = "write:activitypub" @@ -147,6 +148,7 @@ var allAccessTokenScopes = []AccessTokenScope{ // allAccessTokenScopeBits contains all access token scopes. var allAccessTokenScopeBits = map[AccessTokenScope]accessTokenScopeBitmap{ AccessTokenScopeAll: accessTokenScopeAllBits, + AccessTokenScopeNone: 0, AccessTokenScopePublicOnly: accessTokenScopePublicOnlyBits, AccessTokenScopeReadActivityPub: accessTokenScopeReadActivityPubBits, AccessTokenScopeWriteActivityPub: accessTokenScopeWriteActivityPubBits, @@ -211,11 +213,6 @@ func GetRequiredScopes(level AccessTokenScopeLevel, scopeCategories ...AccessTok return scopes } -// ContainsCategory checks if a list of categories contains a specific category -func ContainsCategory(categories []AccessTokenScopeCategory, category AccessTokenScopeCategory) bool { - return slices.Contains(categories, category) -} - // GetScopeLevelFromAccessMode converts permission access mode to scope level func GetScopeLevelFromAccessMode(mode perm.AccessMode) AccessTokenScopeLevel { switch mode { @@ -286,12 +283,14 @@ func (s AccessTokenScope) Normalize() (AccessTokenScope, error) { if err != nil { return "", err } - + if bitmap == 0 && slices.Contains(s.StringSlice(), string(AccessTokenScopeNone)) { + return AccessTokenScopeNone, nil + } return bitmap.toScope(), nil } func (s AccessTokenScope) HasPermissionScope() bool { - return s != "" && s != AccessTokenScopePublicOnly + return s != "" && s != AccessTokenScopeNone && s != AccessTokenScopePublicOnly } // PublicOnly checks if this token scope is limited to public resources diff --git a/models/auth/access_token_scope_test.go b/models/auth/access_token_scope_test.go index a54284e45f5..9bf49a6b215 100644 --- a/models/auth/access_token_scope_test.go +++ b/models/auth/access_token_scope_test.go @@ -18,8 +18,14 @@ type scopeTestNormalize struct { func TestAccessTokenScope_Normalize(t *testing.T) { assert.Equal(t, []string{"activitypub", "admin", "issue", "misc", "notification", "organization", "package", "repository", "user"}, GetAccessTokenCategories()) + assert.False(t, AccessTokenScopeNone.HasPermissionScope()) tests := []scopeTestNormalize{ {"", "", nil}, + {"no-token-scope", "no-token-scope", nil}, + {"no-token-scope,no-token-scope", "no-token-scope", nil}, + {"no-token-scope,read:user", "read:user", nil}, + {"no-token-scope,public-only", "public-only", nil}, + {"no-token-scope,all", "all", nil}, {"write:misc,write:notification,read:package,write:notification,public-only", "public-only,write:misc,write:notification,read:package", nil}, {"all", "all", nil}, {"write:activitypub,write:admin,write:misc,write:notification,write:organization,write:package,write:issue,write:repository,write:user", "all", nil}, @@ -58,10 +64,20 @@ func TestAccessTokenScope_HasScope(t *testing.T) { {"all", "write:package", true, nil}, {"write:package", "all", false, nil}, {"public-only", "read:issue", false, nil}, + {"no-token-scope", "all", false, nil}, + {"no-token-scope", "no-token-scope", true, nil}, } for _, scope := range GetAccessTokenCategories() { tests = append(tests, + scopeTestHasScope{ + AccessTokenScopeNone, + AccessTokenScope("read:" + scope), false, nil, + }, + scopeTestHasScope{ + AccessTokenScopeNone, + AccessTokenScope("write:" + scope), false, nil, + }, scopeTestHasScope{ AccessTokenScope("read:" + scope), AccessTokenScope("read:" + scope), true, nil, diff --git a/routers/web/auth/oauth2_provider.go b/routers/web/auth/oauth2_provider.go index 6a9b0b973cd..e969f81d0e1 100644 --- a/routers/web/auth/oauth2_provider.go +++ b/routers/web/auth/oauth2_provider.go @@ -197,7 +197,7 @@ func IntrospectOAuth(ctx *context.Context) { } response.Active = true - response.Scope = grant.Scope + response.Scope = oauth2_provider.NormalizeGrantScope(grant.Scope) response.RegisteredClaims = oauth2_provider.NewJwtRegisteredClaimsFromUser(introspectingApp.ClientID, grant.UserID, nil /*exp*/) user, err := user_model.GetUserByID(ctx, grant.UserID) if err != nil { @@ -323,12 +323,15 @@ func AuthorizeOAuth(ctx *context.Context) { return } + if grant != nil && form.Scope == "" { + form.Scope = grant.Scope + } + form.Scope = oauth2_provider.NormalizeGrantScope(form.Scope) + 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)) + grantedScope := oauth2_provider.NormalizeGrantScope(grant.Scope) + addedScopes, removedScopes = util.DiffSlice(strings.Fields(grantedScope), strings.Fields(form.Scope)) } scopeChanged := len(addedScopes) > 0 || len(removedScopes) > 0 @@ -431,6 +434,8 @@ func GrantApplicationOAuth(ctx *context.Context) { handleServerError(ctx, form.State, form.RedirectURI) return } + form.Scope = oauth2_provider.NormalizeGrantScope(form.Scope) + if grant == nil { grant, err = app.CreateGrant(ctx, ctx.Doer.ID, form.Scope) if err != nil { diff --git a/routers/web/auth/oauth_test.go b/routers/web/auth/oauth_test.go index 3c2128eb998..6037f6e206c 100644 --- a/routers/web/auth/oauth_test.go +++ b/routers/web/auth/oauth_test.go @@ -6,6 +6,7 @@ package auth import ( "net/http" "net/http/httptest" + "net/url" "sync/atomic" "testing" @@ -32,6 +33,7 @@ func createAndParseToken(t *testing.T, grant *auth.OAuth2Grant) *oauth2_provider response, terr := oauth2_provider.NewAccessTokenResponse(t.Context(), grant, signingKey, signingKey) assert.Nil(t, terr) assert.NotNil(t, response) + assert.Equal(t, grant.Scope, response.Scope) parsedToken, err := jwt.ParseWithClaims(response.IDToken, &oauth2_provider.OIDCToken{}, func(token *jwt.Token) (any, error) { assert.NotNil(t, token.Method) @@ -115,21 +117,56 @@ func TestOAuth2ScopeChange(t *testing.T) { 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 { + authorize := func(scope string) *httptest.ResponseRecorder { 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 + return resp } - assert.Equal(t, http.StatusSeeOther, authorize("")) - assert.Equal(t, http.StatusSeeOther, authorize("profile openid")) - assert.Equal(t, http.StatusOK, authorize("openid profile email")) + assert.Equal(t, http.StatusSeeOther, authorize("").Code) + assert.Equal(t, http.StatusSeeOther, authorize("profile openid").Code) + grant := unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Grant{ID: 1}) + require.NoError(t, auth.UpdateGrantScope(t.Context(), grant, "openid profile read:user write:user")) + assert.Equal(t, http.StatusSeeOther, authorize("openid profile write:user").Code) + require.NoError(t, auth.UpdateGrantScope(t.Context(), grant, "openid invalid:scope")) + assert.Equal(t, http.StatusOK, authorize("openid profile").Code) + require.NoError(t, auth.UpdateGrantScope(t.Context(), grant, "openid profile")) + assert.Equal(t, http.StatusOK, authorize("openid profile email").Code) - 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"}) + for _, tt := range []struct { + name, scope, wantScope string + }{ + {"only invalid scopes", "invalid:scope unsupported", "no-token-scope"}, + {"OIDC with invalid scope", "openid invalid:scope", "openid no-token-scope"}, + {"comma-only scope", ",", "no-token-scope"}, + {"OIDC with comma-only scope", "openid ,,", "openid no-token-scope"}, + {"OIDC scopes", "openid profile email", "openid profile email"}, + {"mixed valid and invalid scopes", "openid groups read:user,invalid:scope public-only unsupported", "openid groups public-only read:user"}, + {"normalized access scopes", " read:user openid profile email read:user ", "openid profile email read:user"}, + {"redundant read scope", "openid read:user write:user", "openid write:user"}, + {"redundant scope with all", "openid all read:user", "openid all"}, + } { + t.Run(tt.name, func(t *testing.T) { + resp := authorize(tt.scope) + require.Equal(t, http.StatusOK, resp.Code) + 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: tt.scope}) + GrantApplicationOAuth(ctx) + assert.Equal(t, http.StatusSeeOther, resp.Code) + redirect, err := url.Parse(resp.Header().Get("Location")) + require.NoError(t, err) + assert.Empty(t, redirect.Query().Get("error")) + assert.Equal(t, "state", redirect.Query().Get("state")) + unittest.AssertExistsAndLoadBean(t, &auth.OAuth2Grant{ID: 1, Scope: tt.wantScope}) + assert.NotEmpty(t, redirect.Query().Get("code")) + resp = authorize(tt.scope) + assert.Equal(t, http.StatusSeeOther, resp.Code) + redirect, err = url.Parse(resp.Header().Get("Location")) + require.NoError(t, err) + assert.NotEmpty(t, redirect.Query().Get("code")) + assert.Empty(t, redirect.Query().Get("error")) + }) + } } diff --git a/routers/web/user/setting/security/openid.go b/routers/web/user/setting/security/openid.go index c4e7962380d..06a620a84fc 100644 --- a/routers/web/user/setting/security/openid.go +++ b/routers/web/user/setting/security/openid.go @@ -5,11 +5,14 @@ package security import ( "errors" + "fmt" "net/http" + "time" audit_model "gitea.dev/models/audit" user_model "gitea.dev/models/user" "gitea.dev/modules/auth/openid" + "gitea.dev/modules/base" "gitea.dev/modules/log" "gitea.dev/modules/setting" "gitea.dev/modules/util" @@ -19,6 +22,8 @@ import ( "gitea.dev/services/forms" ) +const openIDAssociationCodeMinutes = 10 + // OpenIDPost response for change user's openid func OpenIDPost(ctx *context.Context) { if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageCredentials) { @@ -70,7 +75,8 @@ func OpenIDPost(ctx *context.Context) { } } - redirectTo := setting.AppURL + "user/settings/security" + state := base.CreateTimeLimitCode(fmt.Sprintf("openid_association:%d", ctx.Doer.ID), openIDAssociationCodeMinutes, time.Now(), nil) + redirectTo := setting.AppURL + "user/settings/security?state=" + state url, err := openid.RedirectURL(id, redirectTo, setting.AppURL) if err != nil { loadSecurityData(ctx) @@ -82,6 +88,11 @@ func OpenIDPost(ctx *context.Context) { } func settingsOpenIDVerify(ctx *context.Context) { + if !base.VerifyTimeLimitCode(time.Now(), fmt.Sprintf("openid_association:%d", ctx.Doer.ID), openIDAssociationCodeMinutes, ctx.FormString("state")) { + ctx.HTTPError(http.StatusForbidden) + return + } + log.Trace("Incoming call to: " + ctx.Req.URL.String()) fullURL := setting.AppURL + ctx.Req.URL.String()[1:] diff --git a/routers/web/user/setting/security/openid_test.go b/routers/web/user/setting/security/openid_test.go index f00506effaf..2490272e76e 100644 --- a/routers/web/user/setting/security/openid_test.go +++ b/routers/web/user/setting/security/openid_test.go @@ -4,15 +4,103 @@ package security import ( + "fmt" "net/http" + "net/http/httptest" + "net/url" "testing" + "time" "gitea.dev/models/unittest" + user_model "gitea.dev/models/user" + "gitea.dev/modules/base" + "gitea.dev/modules/setting" + "gitea.dev/modules/test" + "gitea.dev/modules/web" "gitea.dev/services/contexttest" + "gitea.dev/services/forms" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestOpenIDAssociation(t *testing.T) { + unittest.PrepareTestEnv(t) + defer test.MockVariableValue(&setting.AppURL, "https://gitea.example/")() + defer test.MockVariableValue(&setting.AppSubURL, "")() + defer test.MockVariableValue(&setting.Security.AllowedHostList, "127.0.0.1:*")() + + provider := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + assert.Equal(t, "check_authentication", r.FormValue("openid.mode")) + w.Header().Set("Content-Type", "text/plain") + fmt.Fprint(w, "ns:http://specs.openid.net/auth/2.0\nis_valid:true\n") + return + } + w.Header().Set("Content-Type", "text/html") + fmt.Fprintf(w, ``, r.Host) + })) + defer provider.Close() + identity := provider.URL + "/identity" + ctx, resp := contexttest.MockContext(t, "POST /user/settings/security/openid") + contexttest.LoadUser(t, ctx, 2) + web.SetForm(ctx, &forms.AddOpenIDForm{Openid: identity}) + OpenIDPost(ctx) + require.Equal(t, http.StatusSeeOther, resp.Code) + redirect, err := url.Parse(resp.Header().Get("Location")) + require.NoError(t, err) + callback, err := url.Parse(redirect.Query().Get("openid.return_to")) + require.NoError(t, err) + state := callback.Query().Get("state") + require.NotEmpty(t, state) + + params := url.Values{ + "openid.ns": {"http://specs.openid.net/auth/2.0"}, + "openid.mode": {"id_res"}, + "openid.op_endpoint": {provider.URL + "/provider"}, + "openid.claimed_id": {identity}, + "openid.identity": {identity}, + "openid.return_to": {callback.String()}, + "openid.response_nonce": {time.Now().UTC().Format("2006-01-02T15:04:05Z") + "association"}, + "openid.assoc_handle": {"test-association"}, + "openid.signed": {"op_endpoint,claimed_id,identity,return_to,response_nonce,assoc_handle"}, + "openid.sig": {"test-signature"}, + } + for _, tc := range []struct { + name string + uid int64 + state string + }{ + {"unsolicited", 2, ""}, + {"wrong state", 2, "wrong"}, + {"other user", 1, state}, + {"expired", 2, base.CreateTimeLimitCode("openid_association:2", openIDAssociationCodeMinutes, time.Now().Add(-time.Hour), nil)}, + {"other purpose", 2, base.CreateTimeLimitCode("reset_password:2", openIDAssociationCodeMinutes, time.Now(), nil)}, + } { + t.Run(tc.name, func(t *testing.T) { + params.Set("state", tc.state) + ctx, resp := contexttest.MockContext(t, "GET /user/settings/security?"+params.Encode()) + contexttest.LoadUser(t, ctx, tc.uid) + Security(ctx) + assert.Equal(t, http.StatusForbidden, resp.Code) + unittest.AssertNotExistsBean(t, &user_model.UserOpenID{URI: identity}) + }) + } + + params.Set("state", state) + ctx, resp = contexttest.MockContext(t, "GET /user/settings/security?"+params.Encode()) + contexttest.LoadUser(t, ctx, 2) + Security(ctx) + require.Equal(t, http.StatusSeeOther, resp.Code) + unittest.AssertExistsAndLoadBean(t, &user_model.UserOpenID{UID: 2, URI: identity}) + + ctx, resp = contexttest.MockContext(t, "GET /user/settings/security?"+params.Encode()) + contexttest.LoadUser(t, ctx, 2) + Security(ctx) + assert.Equal(t, http.StatusOK, resp.Code) + assert.Equal(t, "Nonce already used", ctx.Flash.ErrorMsg) +} + func TestDeleteOpenIDReturnsNotFoundForOtherUsersAddress(t *testing.T) { unittest.PrepareTestEnv(t) ctx, _ := contexttest.MockContext(t, "POST /user/settings/security?id=1") diff --git a/services/oauth2_provider/access_token.go b/services/oauth2_provider/access_token.go index 7dbd90749c5..0ec10891bcc 100644 --- a/services/oauth2_provider/access_token.go +++ b/services/oauth2_provider/access_token.go @@ -72,6 +72,7 @@ type AccessTokenResponse struct { ExpiresIn int64 `json:"expires_in"` RefreshToken string `json:"refresh_token"` IDToken string `json:"id_token,omitempty"` + Scope string `json:"scope,omitempty"` } func GeneralScopesSupported() []string { @@ -83,29 +84,53 @@ func GeneralScopesSupported() []string { } } -// GrantAdditionalScopes returns valid scopes coming from grant -func GrantAdditionalScopes(grantScopes string) auth.AccessTokenScope { +func splitGrantScopes(grantScopes string) (generalScopes []string, accessScope auth.AccessTokenScope) { generalScopesSupported := GeneralScopesSupported() - var accessScopes []string // the scopes for access control, but not for general information + var accessScopes []string + hasAccessScopes := false for scope := range strings.SplitSeq(grantScopes, " ") { - if scope != "" && !slices.Contains(generalScopesSupported, scope) { - accessScopes = append(accessScopes, scope) + if scope == "" { + continue } + if slices.Contains(generalScopesSupported, scope) { + generalScopes = append(generalScopes, scope) + continue + } + hasAccessScopes = true + for accessScope := range strings.SplitSeq(scope, ",") { + if normalizedScope, err := auth.AccessTokenScope(accessScope).Normalize(); err == nil { + accessScopes = append(accessScopes, normalizedScope.StringSlice()...) + } + } + } + if !hasAccessScopes { + return generalScopes, "" } - // since version 1.22, access tokens grant full access to the API - // with this access is reduced only if additional scopes are provided - if len(accessScopes) > 0 { - accessTokenScope := auth.AccessTokenScope(strings.Join(accessScopes, ",")) - if normalizedAccessTokenScope, err := accessTokenScope.Normalize(); err == nil { - return normalizedAccessTokenScope - } - // TODO: if there are invalid access scopes (err != nil), - // then it is treated as "all", maybe in the future we should make it stricter to return an error - // at the moment, to avoid breaking 1.22 behavior, invalid tokens are also treated as "all" + // Since version 1.22, if no access scope is provided, then "all" access is granted. + // + // OAuth also permits ignoring requested scopes under server policy: https://www.rfc-editor.org/rfc/rfc6749.html#section-3.3 + // If after the removal the invalid scopes, there are no valid scopes left, we return no-token-scope to prevent granting "all" access. + accessScope, _ = auth.AccessTokenScope(strings.Join(accessScopes, ",")).Normalize() + if accessScope == "" { + accessScope = auth.AccessTokenScopeNone } - // fallback, empty access scope is treated as "all" access - return auth.AccessTokenScopeAll + return generalScopes, accessScope +} + +// NormalizeGrantScope retains OIDC scopes and uses no-token-scope to prevent discarded scopes from granting full access. +func NormalizeGrantScope(grantScopes string) string { + generalScopes, accessScope := splitGrantScopes(grantScopes) + return strings.Join(append(generalScopes, accessScope.StringSlice()...), " ") +} + +// GrantAdditionalScopes returns valid scopes coming from grant. +func GrantAdditionalScopes(grantScopes string) auth.AccessTokenScope { + _, accessScope := splitGrantScopes(grantScopes) + if accessScope == "" { + return auth.AccessTokenScopeAll // grants without access scopes retain the legacy default + } + return accessScope } func NewJwtRegisteredClaimsFromUser(clientID string, grantUserID int64, exp *jwt.NumericDate) jwt.RegisteredClaims { @@ -241,6 +266,7 @@ func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, server return &AccessTokenResponse{ AccessToken: signedAccessToken, + Scope: NormalizeGrantScope(grant.Scope), TokenType: TokenTypeBearer, ExpiresIn: setting.OAuth2.AccessTokenExpirationTime, RefreshToken: signedRefreshToken, diff --git a/services/oauth2_provider/additional_scopes_test.go b/services/oauth2_provider/additional_scopes_test.go index 5f375346dce..3b0d0077e9b 100644 --- a/services/oauth2_provider/additional_scopes_test.go +++ b/services/oauth2_provider/additional_scopes_test.go @@ -23,10 +23,15 @@ func TestGrantAdditionalScopes(t *testing.T) { {"read:user read:repository", "read:repository,read:user"}, {"read:user write:issue public-only", "public-only,write:issue,read:user"}, {"openid profile email read:user", "read:user"}, - - // TODO: at the moment invalid tokens are treated as "all" to avoid breaking 1.22 behavior (more details are in GrantAdditionalScopes) - {"read:invalid_scope", "all"}, - {"read:invalid_scope,write:scope_invalid,just-plain-wrong", "all"}, + {"no-token-scope", "no-token-scope"}, + {"openid no-token-scope", "no-token-scope"}, + {"no-token-scope read:user", "read:user"}, + {"read:invalid_scope", "no-token-scope"}, + {"read:invalid_scope,write:scope_invalid,just-plain-wrong", "no-token-scope"}, + {"openid invalid:scope", "no-token-scope"}, + {"openid ,,", "no-token-scope"}, + {"read:user invalid:scope", "read:user"}, + {"read:user,invalid:scope public-only", "public-only,read:user"}, } for _, test := range tests { diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 7ce463b361b..38b473ad281 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -803,7 +803,7 @@ func testOAuthIntrospectionCrossClientIsolation(t *testing.T) { func testOAuthGrantScopesReadUserFailRepos(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - accessToken := issueOAuthAccessTokenForScope(t, user, "openid read:user") + accessToken := issueOAuthAccessTokenResponseForScope(t, user, "openid read:user").AccessToken userReq := NewRequest(t, "GET", "/api/v1/user") userReq.SetHeader("Authorization", "Bearer "+accessToken) userResp := MakeRequest(t, userReq, http.StatusOK) @@ -832,7 +832,7 @@ func testOAuthGrantScopesReadUserFailRepos(t *testing.T) { func testOAuthGrantScopesBasicRespectsWriteUser(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - accessToken := issueOAuthAccessTokenForScope(t, user, "openid read:user") + accessToken := issueOAuthAccessTokenResponseForScope(t, user, "openid read:user").AccessToken fullName := "oauth2-basic-scope-test" updateBody := &api.UserSettingsOptions{ @@ -861,7 +861,7 @@ func testOAuthGrantScopesBasicRespectsWriteUser(t *testing.T) { assert.Contains(t, basicError.Message, "required=[write:user]") } -func issueOAuthAccessTokenForScope(t *testing.T, user *user_model.User, scope string) string { +func issueOAuthAccessTokenResponseForScope(t *testing.T, user *user_model.User, scope string) *oauth2_provider.AccessTokenResponse { t.Helper() appBody := api.CreateOAuth2ApplicationOptions{ @@ -899,15 +899,9 @@ func issueOAuthAccessTokenForScope(t *testing.T, user *user_model.User, scope st "code": authcode, }) accessTokenResp := ctx.MakeRequest(t, accessTokenReq, http.StatusOK) - type response struct { - AccessToken string `json:"access_token"` - TokenType string `json:"token_type"` - ExpiresIn int64 `json:"expires_in"` - RefreshToken string `json:"refresh_token"` - } - parsed := new(response) + parsed := new(oauth2_provider.AccessTokenResponse) require.NoError(t, json.Unmarshal(accessTokenResp.Body.Bytes(), parsed)) - return parsed.AccessToken + return parsed } func testOAuthGrantScopesReadRepositoryFailOrganization(t *testing.T) { @@ -1445,4 +1439,23 @@ func TestOAuthUserInfoTokenScope(t *testing.T) { req.SetHeader("Authorization", "Bearer "+userToken) resp := MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Body.String(), "user2@example.com") + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + for _, tc := range []struct { + scope, wantScope string + status int + }{ + {"invalid:scope", "no-token-scope", http.StatusForbidden}, + {"openid invalid:scope", "openid no-token-scope", http.StatusForbidden}, + {"no-token-scope", "no-token-scope", http.StatusForbidden}, + {"openid read:user invalid:scope", "openid read:user", http.StatusOK}, + } { + t.Run(tc.scope, func(t *testing.T) { + response := issueOAuthAccessTokenResponseForScope(t, user, tc.scope) + assert.Equal(t, tc.wantScope, response.Scope) + req := NewRequest(t, "GET", "/login/oauth/userinfo") + req.SetHeader("Authorization", "Bearer "+response.AccessToken) + MakeRequest(t, req, tc.status) + }) + } }