Skip to content

Commit ac3c404

Browse files
committed
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.
1 parent 5d32ed0 commit ac3c404

File tree

5 files changed

+25
-37
lines changed

5 files changed

+25
-37
lines changed

modules/setting/oauth2.go

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -90,25 +90,23 @@ func parseScopes(sec ConfigSection, name string) []string {
9090
}
9191

9292
var OAuth2 = struct {
93-
Enabled bool
94-
AccessTokenExpirationTime int64
95-
RefreshTokenExpirationTime int64
96-
InvalidateRefreshTokens bool
97-
JWTSigningAlgorithm string `ini:"JWT_SIGNING_ALGORITHM"`
98-
JWTSigningPrivateKeyFile string `ini:"JWT_SIGNING_PRIVATE_KEY_FILE"`
99-
MaxTokenLength int
100-
DefaultApplications []string
101-
EnableAdditionalGrantScopes bool
93+
Enabled bool
94+
AccessTokenExpirationTime int64
95+
RefreshTokenExpirationTime int64
96+
InvalidateRefreshTokens bool
97+
JWTSigningAlgorithm string `ini:"JWT_SIGNING_ALGORITHM"`
98+
JWTSigningPrivateKeyFile string `ini:"JWT_SIGNING_PRIVATE_KEY_FILE"`
99+
MaxTokenLength int
100+
DefaultApplications []string
102101
}{
103-
Enabled: true,
104-
AccessTokenExpirationTime: 3600,
105-
RefreshTokenExpirationTime: 730,
106-
InvalidateRefreshTokens: false,
107-
JWTSigningAlgorithm: "RS256",
108-
JWTSigningPrivateKeyFile: "jwt/private.pem",
109-
MaxTokenLength: math.MaxInt16,
110-
DefaultApplications: []string{"git-credential-oauth", "git-credential-manager", "tea"},
111-
EnableAdditionalGrantScopes: false,
102+
Enabled: true,
103+
AccessTokenExpirationTime: 3600,
104+
RefreshTokenExpirationTime: 730,
105+
InvalidateRefreshTokens: false,
106+
JWTSigningAlgorithm: "RS256",
107+
JWTSigningPrivateKeyFile: "jwt/private.pem",
108+
MaxTokenLength: math.MaxInt16,
109+
DefaultApplications: []string{"git-credential-oauth", "git-credential-manager", "tea"},
112110
}
113111

114112
func loadOAuth2From(rootCfg ConfigProvider) {

routers/web/user/setting/applications.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,5 @@ func loadApplicationsData(ctx *context.Context) {
113113
ctx.ServerError("GetOAuth2GrantsByUserID", err)
114114
return
115115
}
116-
ctx.Data["EnableAdditionalGrantScopes"] = setting.OAuth2.EnableAdditionalGrantScopes
117116
}
118117
}

services/oauth2_provider/access_token.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -229,14 +229,10 @@ func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User, onlyPubli
229229

230230
var groups []string
231231
for _, org := range orgs {
232-
// process additional scopes only if enabled in settings
233-
// this could be removed once additional scopes get accepted
234-
if setting.OAuth2.EnableAdditionalGrantScopes {
235-
if onlyPublicGroups {
236-
if public, err := org_model.IsPublicMembership(ctx, org.ID, user.ID); err == nil {
237-
if !public || !org.Visibility.IsPublic() {
238-
continue
239-
}
232+
if onlyPublicGroups {
233+
if public, err := org_model.IsPublicMembership(ctx, org.ID, user.ID); err == nil {
234+
if !public || !org.Visibility.IsPublic() {
235+
continue
240236
}
241237
}
242238
}

services/oauth2_provider/additional_scopes_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
)
1111

1212
func TestGrantAdditionalScopes(t *testing.T) {
13-
setting.OAuth2.EnableAdditionalGrantScopes = true
1413
tests := []struct {
1514
grantScopes string
1615
expectedScopes string

tests/integration/oauth_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,6 @@ func TestOAuthIntrospection(t *testing.T) {
490490
func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
491491
defer tests.PrepareTestEnv(t)()
492492

493-
setting.OAuth2.EnableAdditionalGrantScopes = true
494493
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
495494
appBody := api.CreateOAuth2ApplicationOptions{
496495
Name: "oauth-provider-scopes-test",
@@ -516,7 +515,7 @@ func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
516515
err := db.Insert(db.DefaultContext, grant)
517516
require.NoError(t, err)
518517

519-
assert.Contains(t, grant.Scope, "openid profile email read:user")
518+
assert.ElementsMatch(t, []string{"openid", "profile", "email", "read:user"}, strings.Split(grant.Scope, " "))
520519

521520
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
522521

@@ -572,7 +571,6 @@ func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
572571
func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
573572
defer tests.PrepareTestEnv(t)()
574573

575-
setting.OAuth2.EnableAdditionalGrantScopes = true
576574
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
577575
appBody := api.CreateOAuth2ApplicationOptions{
578576
Name: "oauth-provider-scopes-test",
@@ -598,7 +596,7 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
598596
err := db.Insert(db.DefaultContext, grant)
599597
require.NoError(t, err)
600598

601-
assert.Contains(t, grant.Scope, "openid profile email read:user read:repository")
599+
assert.ElementsMatch(t, []string{"openid", "profile", "email", "read:user", "read:repository"}, strings.Split(grant.Scope, " "))
602600

603601
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
604602

@@ -814,10 +812,9 @@ func TestOAuth_GrantScopesReadRepositoriesPublicOnly(t *testing.T) {
814812
assert.Equal(t, reposExpected, reposCaptured)
815813
}
816814

817-
func TestOAuth_GrantScopesNotEnabledClaimGroups(t *testing.T) {
815+
func TestOAuth_GrantScopesClaimGroupsAll(t *testing.T) {
818816
defer tests.PrepareTestEnv(t)()
819817

820-
setting.OAuth2.EnableAdditionalGrantScopes = false
821818
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
822819

823820
appBody := api.CreateOAuth2ApplicationOptions{
@@ -894,10 +891,9 @@ func TestOAuth_GrantScopesNotEnabledClaimGroups(t *testing.T) {
894891
}
895892
}
896893

897-
func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
894+
func TestOAuth_GrantScopesClaimGroupsPublicOnly(t *testing.T) {
898895
defer tests.PrepareTestEnv(t)()
899896

900-
setting.OAuth2.EnableAdditionalGrantScopes = true
901897
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
902898

903899
appBody := api.CreateOAuth2ApplicationOptions{
@@ -924,7 +920,7 @@ func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
924920
err := db.Insert(db.DefaultContext, grant)
925921
require.NoError(t, err)
926922

927-
assert.Contains(t, grant.Scope, "openid profile email groups")
923+
assert.ElementsMatch(t, []string{"openid", "profile", "email", "groups"}, strings.Split(grant.Scope, " "))
928924

929925
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
930926

0 commit comments

Comments
 (0)