diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dee4c05..cd42ac4e 100644 --- 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 index 65fade84..ca8af44c 100644 --- 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 index fbd48495..355fd939 100755 --- 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 index a510debf..4c9f2df0 100644 --- 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 index 9510701e..bc60858a 100644 --- 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 index 6e85136b..269fef4e 100644 --- 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 index 4f326f70..4eda8aa8 100644 --- 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 index 190275d3..2ac6acb0 100644 --- 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,