Merge branch 'master' into docs/add-search

tuunit · Sep 11, 2023 · b1fde8b · b1fde8b
2 parents 4bb4f32 + 854401e
commit b1fde8b
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
 - [#2221](https://github.com/oauth2-proxy/oauth2-proxy/pull/2221) Backwards compatible fix for wrong environment variable name (OAUTH2_PROXY_GOOGLE_GROUPS) (@kvanzuijlen)
 - [#1989](https://github.com/oauth2-proxy/oauth2-proxy/pull/1989) Fix default scope for keycloak-oidc provider
 - [#2217](https://github.com/oauth2-proxy/oauth2-proxy/pull/2217) Upgrade alpine to version 3.18 (@polarctos)
+- [#2229](https://github.com/oauth2-proxy/oauth2-proxy/pull/2229) bugfix: default scopes for OIDCProvider based providers
 
 # V7.5.0
 

diff --git a/providers/adfs.go b/providers/adfs.go
@@ -46,10 +46,7 @@ func NewADFSProvider(p *ProviderData, opts options.ADFSOptions) *ADFSProvider {
 		}
 	}
 
-	oidcProvider := &OIDCProvider{
-		ProviderData: p,
-		SkipNonce:    false,
-	}
+	oidcProvider := NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false})
 
 	return &ADFSProvider{
 		OIDCProvider:    oidcProvider,

diff --git a/providers/adfs_test.go b/providers/adfs_test.go
@@ -136,7 +136,13 @@ var _ = Describe("ADFS Provider Tests", func() {
 		It("uses defaults", func() {
 			providerData := NewADFSProvider(&ProviderData{}, options.ADFSOptions{}).Data()
 			Expect(providerData.ProviderName).To(Equal("ADFS"))
-			Expect(providerData.Scope).To(Equal("openid email profile"))
+			Expect(providerData.Scope).To(Equal(oidcDefaultScope))
+		})
+		It("uses custom scope", func() {
+			providerData := NewADFSProvider(&ProviderData{Scope: "openid email"}, options.ADFSOptions{}).Data()
+			Expect(providerData.ProviderName).To(Equal("ADFS"))
+			Expect(providerData.Scope).To(Equal("openid email"))
+			Expect(providerData.Scope).NotTo(Equal(oidcDefaultScope))
 		})
 	})
 

diff --git a/providers/gitlab.go b/providers/gitlab.go
@@ -40,10 +40,7 @@ func NewGitLabProvider(p *ProviderData, opts options.GitLabOptions) (*GitLabProv
 		p.Scope = gitlabDefaultScope
 	}
 
-	oidcProvider := &OIDCProvider{
-		ProviderData: p,
-		SkipNonce:    false,
-	}
+	oidcProvider := NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false})
 
 	provider := &GitLabProvider{
 		OIDCProvider:    oidcProvider,

diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go
@@ -170,6 +170,15 @@ var _ = Describe("Gitlab Provider Tests", func() {
 		b.Close()
 	})
 
+	Context("New Provider Init", func() {
+		It("creates new keycloak oidc provider with expected defaults", func() {
+			providerData := p.Data()
+			Expect(providerData.ProviderName).To(Equal(gitlabProviderName))
+			Expect(providerData.Scope).To(Equal(gitlabDefaultScope))
+			Expect(providerData.ProviderName).NotTo(Equal(oidcDefaultScope))
+		})
+	})
+
 	Context("with bad token", func() {
 		It("should trigger an error", func() {
 			p.AllowUnverifiedEmail = false

diff --git a/providers/keycloak_oidc.go b/providers/keycloak_oidc.go
@@ -22,9 +22,7 @@ func NewKeycloakOIDCProvider(p *ProviderData, opts options.KeycloakOptions) *Key
 	})
 
 	provider := &KeycloakOIDCProvider{
-		OIDCProvider: &OIDCProvider{
-			ProviderData: p,
-		},
+		OIDCProvider: NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false}),
 	}
 
 	provider.addAllowedRoles(opts.Roles)

diff --git a/providers/keycloak_oidc_test.go b/providers/keycloak_oidc_test.go
@@ -67,7 +67,7 @@ func newKeycloakOIDCProvider(serverURL *url.URL, opts options.KeycloakOptions) *
 				Scheme: "https",
 				Host:   "keycloak-oidc.com",
 				Path:   "/api/v3/user"},
-			Scope: "openid email profile"},
+		},
 		opts)
 
 	if serverURL != nil {
@@ -97,7 +97,15 @@ var _ = Describe("Keycloak OIDC Provider Tests", func() {
 			Expect(providerData.RedeemURL.String()).To(Equal("https://keycloak-oidc.com/oauth/token"))
 			Expect(providerData.ProfileURL.String()).To(Equal("https://keycloak-oidc.com/api/v3/user"))
 			Expect(providerData.ValidateURL.String()).To(Equal("https://keycloak-oidc.com/api/v3/user"))
-			Expect(providerData.Scope).To(Equal("openid email profile"))
+			Expect(providerData.Scope).To(Equal(oidcDefaultScope))
+		})
+		It("creates new keycloak oidc provider with custom scope", func() {
+			p := NewKeycloakOIDCProvider(&ProviderData{Scope: "openid email"}, options.KeycloakOptions{})
+			providerData := p.Data()
+
+			Expect(providerData.ProviderName).To(Equal(keycloakOIDCProviderName))
+			Expect(providerData.Scope).To(Equal("openid email"))
+			Expect(providerData.Scope).NotTo(Equal(oidcDefaultScope))
 		})
 	})
 

diff --git a/providers/oidc.go b/providers/oidc.go
@@ -24,8 +24,14 @@ const oidcDefaultScope = "openid email profile"
 
 // NewOIDCProvider initiates a new OIDCProvider
 func NewOIDCProvider(p *ProviderData, opts options.OIDCOptions) *OIDCProvider {
+	name := "OpenID Connect"
+
+	if p.ProviderName != "" {
+		name = p.ProviderName
+	}
+
 	oidcProviderDefaults := providerDefaults{
-		name:        "OpenID Connect",
+		name:        name,
 		loginURL:    nil,
 		redeemURL:   nil,
 		profileURL:  nil,