no setting requirement for additional grant scopes

- the logic introduced with this PR will be applied by default, even though it
introduces breaking changes if anyone relied on the previous behavior
regarding personal access tokens or full access for OAuth2 third parties.

Author: marcellmars

modules/setting/oauth2.go

@@ -90,25 +90,23 @@ func parseScopes(sec ConfigSection, name string) []string {
 }
 
 var OAuth2 = struct {
-	Enabled                     bool
-	AccessTokenExpirationTime   int64
-	RefreshTokenExpirationTime  int64
-	InvalidateRefreshTokens     bool
-	JWTSigningAlgorithm         string `ini:"JWT_SIGNING_ALGORITHM"`
-	JWTSigningPrivateKeyFile    string `ini:"JWT_SIGNING_PRIVATE_KEY_FILE"`
-	MaxTokenLength              int
-	DefaultApplications         []string
-	EnableAdditionalGrantScopes bool
+	Enabled                    bool
+	AccessTokenExpirationTime  int64
+	RefreshTokenExpirationTime int64
+	InvalidateRefreshTokens    bool
+	JWTSigningAlgorithm        string `ini:"JWT_SIGNING_ALGORITHM"`
+	JWTSigningPrivateKeyFile   string `ini:"JWT_SIGNING_PRIVATE_KEY_FILE"`
+	MaxTokenLength             int
+	DefaultApplications        []string
 }{
-	Enabled:                     true,
-	AccessTokenExpirationTime:   3600,
-	RefreshTokenExpirationTime:  730,
-	InvalidateRefreshTokens:     false,
-	JWTSigningAlgorithm:         "RS256",
-	JWTSigningPrivateKeyFile:    "jwt/private.pem",
-	MaxTokenLength:              math.MaxInt16,
-	DefaultApplications:         []string{"git-credential-oauth", "git-credential-manager", "tea"},
-	EnableAdditionalGrantScopes: false,
+	Enabled:                    true,
+	AccessTokenExpirationTime:  3600,
+	RefreshTokenExpirationTime: 730,
+	InvalidateRefreshTokens:    false,
+	JWTSigningAlgorithm:        "RS256",
+	JWTSigningPrivateKeyFile:   "jwt/private.pem",
+	MaxTokenLength:             math.MaxInt16,
+	DefaultApplications:        []string{"git-credential-oauth", "git-credential-manager", "tea"},
 }
 
 func loadOAuth2From(rootCfg ConfigProvider) {

routers/web/user/setting/applications.go

@@ -113,6 +113,5 @@ func loadApplicationsData(ctx *context.Context) {
 			ctx.ServerError("GetOAuth2GrantsByUserID", err)
 			return
 		}
-		ctx.Data["EnableAdditionalGrantScopes"] = setting.OAuth2.EnableAdditionalGrantScopes
 	}
 }

services/auth/additional_scopes_test.go

@@ -12,7 +12,6 @@ import (
 )
 
 func TestGrantAdditionalScopes(t *testing.T) {
-	setting.OAuth2.EnableAdditionalGrantScopes = true
 	tests := []struct {
 		grantScopes    string
 		expectedScopes string

services/auth/oauth2.go

@@ -28,12 +28,6 @@ var (
 
 // GrantAdditionalScopes returns valid scopes coming from grant
 func GrantAdditionalScopes(grantScopes string) string {
-	// process additional scopes only if enabled in settings
-	// this could be removed once additional scopes get accepted
-	if !setting.OAuth2.EnableAdditionalGrantScopes {
-		return ""
-	}
-
 	// scopes_supported from templates/user/auth/oidc_wellknown.tmpl
 	scopesSupported := []string{
 		"openid",

services/oauth2_provider/access_token.go

@@ -216,14 +216,10 @@ func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User, onlyPubli
 
 	var groups []string
 	for _, org := range orgs {
-		// process additional scopes only if enabled in settings
-		// this could be removed once additional scopes get accepted
-		if setting.OAuth2.EnableAdditionalGrantScopes {
-			if onlyPublicGroups {
-				if public, err := org_model.IsPublicMembership(ctx, org.ID, user.ID); err == nil {
-					if !public || !org.Visibility.IsPublic() {
-						continue
-					}
+		if onlyPublicGroups {
+			if public, err := org_model.IsPublicMembership(ctx, org.ID, user.ID); err == nil {
+				if !public || !org.Visibility.IsPublic() {
+					continue
 				}
 			}
 		}

tests/integration/oauth_test.go

@@ -490,7 +490,6 @@ func TestOAuthIntrospection(t *testing.T) {
 func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	setting.OAuth2.EnableAdditionalGrantScopes = true
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
 	appBody := api.CreateOAuth2ApplicationOptions{
 		Name: "oauth-provider-scopes-test",
@@ -516,7 +515,7 @@ func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
 	err := db.Insert(db.DefaultContext, grant)
 	require.NoError(t, err)
 
-	assert.Contains(t, grant.Scope, "openid profile email read:user")
+	assert.ElementsMatch(t, []string{"openid", "profile", "email", "read:user"}, strings.Split(grant.Scope, " "))
 
 	ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
 
@@ -572,7 +571,6 @@ func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
 func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	setting.OAuth2.EnableAdditionalGrantScopes = true
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
 	appBody := api.CreateOAuth2ApplicationOptions{
 		Name: "oauth-provider-scopes-test",
@@ -598,7 +596,7 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
 	err := db.Insert(db.DefaultContext, grant)
 	require.NoError(t, err)
 
-	assert.Contains(t, grant.Scope, "openid profile email read:user read:repository")
+	assert.ElementsMatch(t, []string{"openid", "profile", "email", "read:user", "read:repository"}, strings.Split(grant.Scope, " "))
 
 	ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
 
@@ -716,7 +714,6 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
 func TestOAuth_GrantScopesReadRepositoriesPublicOnly(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	setting.OAuth2.EnableAdditionalGrantScopes = true
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
 
 	appBody := api.CreateOAuth2ApplicationOptions{
@@ -743,7 +740,7 @@ func TestOAuth_GrantScopesReadRepositoriesPublicOnly(t *testing.T) {
 	err := db.Insert(db.DefaultContext, grant)
 	require.NoError(t, err)
 
-	assert.Contains(t, grant.Scope, "openid profile email groups public-only read:user read:repository")
+	assert.ElementsMatch(t, []string{"openid", "profile", "email", "groups", "public-only", "read:user", "read:repository"}, strings.Split(grant.Scope, " "))
 
 	ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
 
@@ -815,10 +812,9 @@ func TestOAuth_GrantScopesReadRepositoriesPublicOnly(t *testing.T) {
 	assert.Equal(t, reposExpected, reposCaptured)
 }
 
-func TestOAuth_GrantScopesNotEnabledClaimGroups(t *testing.T) {
+func TestOAuth_GrantScopesClaimGroupsAll(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	setting.OAuth2.EnableAdditionalGrantScopes = false
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
 
 	appBody := api.CreateOAuth2ApplicationOptions{
@@ -839,13 +835,13 @@ func TestOAuth_GrantScopesNotEnabledClaimGroups(t *testing.T) {
 	grant := &auth_model.OAuth2Grant{
 		ApplicationID: app.ID,
 		UserID:        user.ID,
-		Scope:         "openid profile email groups",
+		Scope:         "openid profile email groups all",
 	}
 
 	err := db.Insert(db.DefaultContext, grant)
 	require.NoError(t, err)
 
-	assert.Contains(t, grant.Scope, "openid profile email groups")
+	assert.ElementsMatch(t, []string{"openid", "profile", "email", "groups", "all"}, strings.Split(grant.Scope, " "))
 
 	ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
 
@@ -895,10 +891,9 @@ func TestOAuth_GrantScopesNotEnabledClaimGroups(t *testing.T) {
 	}
 }
 
-func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
+func TestOAuth_GrantScopesClaimGroupsPublicOnly(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()
 
-	setting.OAuth2.EnableAdditionalGrantScopes = true
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
 
 	appBody := api.CreateOAuth2ApplicationOptions{
@@ -925,7 +920,7 @@ func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
 	err := db.Insert(db.DefaultContext, grant)
 	require.NoError(t, err)
 
-	assert.Contains(t, grant.Scope, "openid profile email groups")
+	assert.ElementsMatch(t, []string{"openid", "profile", "email", "groups"}, strings.Split(grant.Scope, " "))
 
 	ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)