Skip to content

Commit c0a2316

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 57dbe82 commit c0a2316

File tree

5 files changed

+26
-39
lines changed

5 files changed

+26
-39
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
@@ -256,14 +256,10 @@ func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User, onlyPubli
256256

257257
var groups []string
258258
for _, org := range orgs {
259-
// process additional scopes only if enabled in settings
260-
// this could be removed once additional scopes get accepted
261-
if setting.OAuth2.EnableAdditionalGrantScopes {
262-
if onlyPublicGroups {
263-
if public, err := org_model.IsPublicMembership(ctx, org.ID, user.ID); err == nil {
264-
if !public || !org.Visibility.IsPublic() {
265-
continue
266-
}
259+
if onlyPublicGroups {
260+
if public, err := org_model.IsPublicMembership(ctx, org.ID, user.ID); err == nil {
261+
if !public || !org.Visibility.IsPublic() {
262+
continue
267263
}
268264
}
269265
}

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: 6 additions & 11 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

@@ -716,7 +714,6 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
716714
func TestOAuth_GrantScopesReadRepositoriesPublicOnly(t *testing.T) {
717715
defer tests.PrepareTestEnv(t)()
718716

719-
setting.OAuth2.EnableAdditionalGrantScopes = true
720717
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
721718

722719
appBody := api.CreateOAuth2ApplicationOptions{
@@ -743,7 +740,7 @@ func TestOAuth_GrantScopesReadRepositoriesPublicOnly(t *testing.T) {
743740
err := db.Insert(db.DefaultContext, grant)
744741
require.NoError(t, err)
745742

746-
assert.Contains(t, grant.Scope, "openid profile email groups public-only read:user read:repository")
743+
assert.ElementsMatch(t, []string{"openid", "profile", "email", "groups", "public-only", "read:user", "read:repository"}, strings.Split(grant.Scope, " "))
747744

748745
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
749746

@@ -815,10 +812,9 @@ func TestOAuth_GrantScopesReadRepositoriesPublicOnly(t *testing.T) {
815812
assert.Equal(t, reposExpected, reposCaptured)
816813
}
817814

818-
func TestOAuth_GrantScopesNotEnabledClaimGroups(t *testing.T) {
815+
func TestOAuth_GrantScopesClaimGroupsAll(t *testing.T) {
819816
defer tests.PrepareTestEnv(t)()
820817

821-
setting.OAuth2.EnableAdditionalGrantScopes = false
822818
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
823819

824820
appBody := api.CreateOAuth2ApplicationOptions{
@@ -895,10 +891,9 @@ func TestOAuth_GrantScopesNotEnabledClaimGroups(t *testing.T) {
895891
}
896892
}
897893

898-
func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
894+
func TestOAuth_GrantScopesClaimGroupsPublicOnly(t *testing.T) {
899895
defer tests.PrepareTestEnv(t)()
900896

901-
setting.OAuth2.EnableAdditionalGrantScopes = true
902897
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
903898

904899
appBody := api.CreateOAuth2ApplicationOptions{
@@ -925,7 +920,7 @@ func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
925920
err := db.Insert(db.DefaultContext, grant)
926921
require.NoError(t, err)
927922

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

930925
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
931926

0 commit comments

Comments
 (0)