Skip to content

Commit a3970c8

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 6dae990 commit a3970c8

File tree

5 files changed

+24
-32
lines changed

5 files changed

+24
-32
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
@@ -228,14 +228,10 @@ func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User, onlyPubli
228228

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

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: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
511511
err := db.Insert(db.DefaultContext, grant)
512512
require.NoError(t, err)
513513

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

516516
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
517517

@@ -592,7 +592,7 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
592592
err := db.Insert(db.DefaultContext, grant)
593593
require.NoError(t, err)
594594

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

597597
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
598598

@@ -786,7 +786,7 @@ func TestOAuth_GrantScopesClaimGroupsAll(t *testing.T) {
786786
}
787787
}
788788

789-
func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
789+
func TestOAuth_GrantScopesClaimGroupsPublicOnly(t *testing.T) {
790790
defer tests.PrepareTestEnv(t)()
791791

792792
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
@@ -815,7 +815,7 @@ func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
815815
err := db.Insert(db.DefaultContext, grant)
816816
require.NoError(t, err)
817817

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

820820
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
821821

0 commit comments

Comments
 (0)