Skip to content

Commit 87a15b3

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 5d05df4 commit 87a15b3

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
@@ -515,7 +515,7 @@ func TestOAuth_GrantScopesReadUserFailRepos(t *testing.T) {
515515
err := db.Insert(db.DefaultContext, grant)
516516
require.NoError(t, err)
517517

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

520520
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
521521

@@ -596,7 +596,7 @@ func TestOAuth_GrantScopesReadRepositoryFailOrganization(t *testing.T) {
596596
err := db.Insert(db.DefaultContext, grant)
597597
require.NoError(t, err)
598598

599-
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, " "))
600600

601601
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
602602

@@ -790,7 +790,7 @@ func TestOAuth_GrantScopesClaimGroupsAll(t *testing.T) {
790790
}
791791
}
792792

793-
func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
793+
func TestOAuth_GrantScopesClaimGroupsPublicOnly(t *testing.T) {
794794
defer tests.PrepareTestEnv(t)()
795795

796796
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
@@ -819,7 +819,7 @@ func TestOAuth_GrantScopesEnabledClaimGroups(t *testing.T) {
819819
err := db.Insert(db.DefaultContext, grant)
820820
require.NoError(t, err)
821821

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

824824
ctx := loginUserWithPasswordRemember(t, user.Name, "password", true)
825825

0 commit comments

Comments
 (0)