From 8b77c9700979f2a249a9967204deecc63b91f6cf Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Fri, 23 Dec 2022 10:00:57 +0100 Subject: [PATCH] Fix default scope settings for none oidc providers like GitHub (#1927) * fix default scope settings for none oidc providers * add changelog for bugfix * fix scope test cases by producing and accessing correct result value --- CHANGELOG.md | 1 + providers/providers.go | 2 +- providers/providers_test.go | 28 ++++++++++++++++++++++------ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54a5d607..f9900432 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - [#1882](https://github.com/oauth2-proxy/oauth2-proxy/pull/1882) Make `htpasswd.GetUsers` racecondition safe - [#1883](https://github.com/oauth2-proxy/oauth2-proxy/pull/1883) Ensure v8 manifest variant is set on docker images - [#1906](https://github.com/oauth2-proxy/oauth2-proxy/pull/1906) Fix PKCE code verifier generation to never use UTF-8 characters +- [#1927](https://github.com/oauth2-proxy/oauth2-proxy/pull/1927) Fix default scope settings for none oidc providers # V7.4.0 diff --git a/providers/providers.go b/providers/providers.go index d2570081..dbac1d03 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -152,7 +152,7 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData, p.EmailClaim = providerConfig.OIDCConfig.UserIDClaim } - if p.Scope == "" { + if providerConfig.Type == "oidc" && p.Scope == "" { p.Scope = "openid email profile" if len(providerConfig.AllowedGroups) > 0 { diff --git a/providers/providers_test.go b/providers/providers_test.go index a69a0641..8d8aa297 100644 --- a/providers/providers_test.go +++ b/providers/providers_test.go @@ -125,32 +125,48 @@ func TestScope(t *testing.T) { testCases := []struct { name string + configuredType options.ProviderType configuredScope string expectedScope string allowedGroups []string }{ { - name: "with no scope provided", + name: "oidc: with no scope provided", + configuredType: "oidc", configuredScope: "", expectedScope: "openid email profile", }, { - name: "with no scope provided and groups", + name: "oidc: with no scope provided and groups", + configuredType: "oidc", configuredScope: "", expectedScope: "openid email profile groups", allowedGroups: []string{"foo"}, }, { - name: "with a configured scope provided", + name: "oidc: with a configured scope provided", + configuredType: "oidc", configuredScope: "openid", expectedScope: "openid", }, + { + name: "github: with no scope provided", + configuredType: "github", + configuredScope: "", + expectedScope: "user:email", + }, + { + name: "github: with a configured scope provided", + configuredType: "github", + configuredScope: "user:email org:read", + expectedScope: "user:email org:read", + }, } for _, tc := range testCases { providerConfig := options.Provider{ ID: providerID, - Type: "oidc", + Type: tc.configuredType, ClientID: clientID, ClientSecretFile: clientSecret, LoginURL: msAuthURL, @@ -164,10 +180,10 @@ func TestScope(t *testing.T) { }, } - pd, err := newProviderDataFromConfig(providerConfig) + pd, err := NewProvider(providerConfig) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(pd.Scope).To(Equal(tc.expectedScope)) + g.Expect(pd.Data().Scope).To(Equal(tc.expectedScope)) } }