git.iko.soy/typeable/oauth2-proxy
on master help sign in
bugfix: OIDCConfig based providers are not respecting flags and configs (#2299)

* add full support for all oidc config based providers to use and respect all configs set via OIDCConfig * add changelog entry --------- Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

commitfc6e7fdbd11f979de2c4e5c1d3d7eb8b214566be authorJan Larwig <jan@larwig.com> dateSat 25 Nov 2023, 12:32 (3 years ago) parent0ddb5e7
8 files changed+36−29
compare with master
unifiedsplit
CHANGELOG.md+1 providers/adfs.go+3−3 providers/adfs_test.go+7−5 providers/gitlab.go+4−4 providers/gitlab_test.go+7−5 providers/keycloak_oidc.go+3−3 providers/keycloak_oidc_test.go+8−6 providers/providers.go+3−3
CHANGELOG.md+1
@@ -18,6 +18,7 @@
1818- [#1866](https://github.com/oauth2-proxy/oauth2-proxy/pull/1866) Add support for unix socker as upstream (@babs)
1919- [#1949](https://github.com/oauth2-proxy/oauth2-proxy/pull/1949) Allow cookie names with dots in redis sessions (@miguelborges99)
2020- [#2297](https://github.com/oauth2-proxy/oauth2-proxy/pull/2297) Add nightly build and push (@tuunit)
21+- [#2299](https://github.com/oauth2-proxy/oauth2-proxy/pull/2299) bugfix: OIDCConfig based providers are not respecting flags and configs (@tuunit)
2122- [#2248](https://github.com/oauth2-proxy/oauth2-proxy/pull/2248) Added support for semicolons in query strings.
2223
2324# V7.5.1
providers/adfs.go+3−3
@@ -29,7 +29,7 @@ const (
2929)
3030
3131// NewADFSProvider initiates a new ADFSProvider
32−func NewADFSProvider(p *ProviderData, opts options.ADFSOptions) *ADFSProvider {
32+func NewADFSProvider(p *ProviderData, opts options.Provider) *ADFSProvider {
3333 p.setProviderDefaults(providerDefaults{
3434 name: adfsProviderName,
3535 scope: adfsDefaultScope,
@@ -46,11 +46,11 @@ func NewADFSProvider(p *ProviderData, opts options.ADFSOptions) *ADFSProvider {
4646 }
4747 }
4848
49− oidcProvider := NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false})
49+ oidcProvider := NewOIDCProvider(p, opts.OIDCConfig)
5050
5151 return &ADFSProvider{
5252 OIDCProvider: oidcProvider,
53− skipScope: opts.SkipScope,
53+ skipScope: opts.ADFSConfig.SkipScope,
5454 oidcEnrichFunc: oidcProvider.EnrichSession,
5555 oidcRefreshFunc: oidcProvider.RefreshSession,
5656 }
providers/adfs_test.go+7−5
@@ -63,7 +63,7 @@ func testADFSProvider(hostname string) *ADFSProvider {
6363 Scope: "",
6464 Verifier: o,
6565 EmailClaim: options.OIDCEmailClaim,
66− }, options.ADFSOptions{})
66+ }, options.Provider{})
6767
6868 if hostname != "" {
6969 updateURL(p.Data().LoginURL, hostname)
@@ -134,12 +134,12 @@ var _ = Describe("ADFS Provider Tests", func() {
134134
135135 Context("New Provider Init", func() {
136136 It("uses defaults", func() {
137− providerData := NewADFSProvider(&ProviderData{}, options.ADFSOptions{}).Data()
137+ providerData := NewADFSProvider(&ProviderData{}, options.Provider{}).Data()
138138 Expect(providerData.ProviderName).To(Equal("ADFS"))
139139 Expect(providerData.Scope).To(Equal(oidcDefaultScope))
140140 })
141141 It("uses custom scope", func() {
142− providerData := NewADFSProvider(&ProviderData{Scope: "openid email"}, options.ADFSOptions{}).Data()
142+ providerData := NewADFSProvider(&ProviderData{Scope: "openid email"}, options.Provider{}).Data()
143143 Expect(providerData.ProviderName).To(Equal("ADFS"))
144144 Expect(providerData.Scope).To(Equal("openid email"))
145145 Expect(providerData.Scope).NotTo(Equal(oidcDefaultScope))
@@ -172,7 +172,9 @@ var _ = Describe("ADFS Provider Tests", func() {
172172 p := NewADFSProvider(&ProviderData{
173173 ProtectedResource: resource,
174174 Scope: "",
175− }, options.ADFSOptions{SkipScope: true})
175+ }, options.Provider{
176+ ADFSConfig: options.ADFSOptions{SkipScope: true},
177+ })
176178
177179 result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "", url.Values{})
178180 Expect(result).NotTo(ContainSubstring("scope="))
@@ -192,7 +194,7 @@ var _ = Describe("ADFS Provider Tests", func() {
192194 p := NewADFSProvider(&ProviderData{
193195 ProtectedResource: resource,
194196 Scope: in.scope,
195− }, options.ADFSOptions{})
197+ }, options.Provider{})
196198
197199 Expect(p.Data().Scope).To(Equal(in.expectedScope))
198200 result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "", url.Values{})
providers/gitlab.go+4−4
@@ -31,7 +31,7 @@ type GitLabProvider struct {
3131var _ Provider = (*GitLabProvider)(nil)
3232
3333// NewGitLabProvider initiates a new GitLabProvider
34−func NewGitLabProvider(p *ProviderData, opts options.GitLabOptions) (*GitLabProvider, error) {
34+func NewGitLabProvider(p *ProviderData, opts options.Provider) (*GitLabProvider, error) {
3535 p.setProviderDefaults(providerDefaults{
3636 name: gitlabProviderName,
3737 })
@@ -40,15 +40,15 @@ func NewGitLabProvider(p *ProviderData, opts options.GitLabOptions) (*GitLabProv
4040 p.Scope = gitlabDefaultScope
4141 }
4242
43− oidcProvider := NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false})
43+ oidcProvider := NewOIDCProvider(p, opts.OIDCConfig)
4444
4545 provider := &GitLabProvider{
4646 OIDCProvider: oidcProvider,
4747 oidcRefreshFunc: oidcProvider.RefreshSession,
4848 }
49− provider.setAllowedGroups(opts.Group)
49+ provider.setAllowedGroups(opts.GitLabConfig.Group)
5050
51− if err := provider.setAllowedProjects(opts.Projects); err != nil {
51+ if err := provider.setAllowedProjects(opts.GitLabConfig.Projects); err != nil {
5252 return nil, fmt.Errorf("could not configure allowed projects: %v", err)
5353 }
5454
providers/gitlab_test.go+7−5
@@ -14,7 +14,7 @@ import (
1414 . "github.com/onsi/gomega"
1515)
1616
17−func testGitLabProvider(hostname, scope string, opts options.GitLabOptions) (*GitLabProvider, error) {
17+func testGitLabProvider(hostname, scope string, opts options.Provider) (*GitLabProvider, error) {
1818 p, err := NewGitLabProvider(
1919 &ProviderData{
2020 ProviderName: "",
@@ -162,7 +162,7 @@ var _ = Describe("Gitlab Provider Tests", func() {
162162 bURL, err := url.Parse(b.URL)
163163 Expect(err).To(BeNil())
164164
165− p, err = testGitLabProvider(bURL.Host, "", options.GitLabOptions{})
165+ p, err = testGitLabProvider(bURL.Host, "", options.Provider{})
166166 Expect(err).ToNot(HaveOccurred())
167167 })
168168
@@ -237,9 +237,11 @@ var _ = Describe("Gitlab Provider Tests", func() {
237237 bURL, err := url.Parse(b.URL)
238238 Expect(err).To(BeNil())
239239
240− p, err := testGitLabProvider(bURL.Host, in.scope, options.GitLabOptions{
241− Group: in.allowedGroups,
242− Projects: in.allowedProjects,
240+ p, err := testGitLabProvider(bURL.Host, in.scope, options.Provider{
241+ GitLabConfig: options.GitLabOptions{
242+ Group: in.allowedGroups,
243+ Projects: in.allowedProjects,
244+ },
243245 })
244246 if in.expectedError == nil {
245247 Expect(err).To(BeNil())
providers/keycloak_oidc.go+3−3
@@ -16,16 +16,16 @@ type KeycloakOIDCProvider struct {
1616}
1717
1818// NewKeycloakOIDCProvider makes a KeycloakOIDCProvider using the ProviderData
19−func NewKeycloakOIDCProvider(p *ProviderData, opts options.KeycloakOptions) *KeycloakOIDCProvider {
19+func NewKeycloakOIDCProvider(p *ProviderData, opts options.Provider) *KeycloakOIDCProvider {
2020 p.setProviderDefaults(providerDefaults{
2121 name: keycloakOIDCProviderName,
2222 })
2323
2424 provider := &KeycloakOIDCProvider{
25− OIDCProvider: NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false}),
25+ OIDCProvider: NewOIDCProvider(p, opts.OIDCConfig),
2626 }
2727
28− provider.addAllowedRoles(opts.Roles)
28+ provider.addAllowedRoles(opts.KeycloakConfig.Roles)
2929 return provider
3030}
3131
providers/keycloak_oidc_test.go+8−6
@@ -40,11 +40,11 @@ func getAccessToken() string {
4040
4141func newTestKeycloakOIDCSetup() (*httptest.Server, *KeycloakOIDCProvider) {
4242 redeemURL, server := newOIDCServer([]byte(fmt.Sprintf(`{"email": "new@thing.com", "expires_in": 300, "access_token": "%v"}`, getAccessToken())))
43− provider := newKeycloakOIDCProvider(redeemURL, options.KeycloakOptions{})
43+ provider := newKeycloakOIDCProvider(redeemURL, options.Provider{})
4444 return server, provider
4545}
4646
47−func newKeycloakOIDCProvider(serverURL *url.URL, opts options.KeycloakOptions) *KeycloakOIDCProvider {
47+func newKeycloakOIDCProvider(serverURL *url.URL, opts options.Provider) *KeycloakOIDCProvider {
4848 verificationOptions := internaloidc.IDTokenVerificationOptions{
4949 AudienceClaims: []string{defaultAudienceClaim},
5050 ClientID: mockClientID,
@@ -90,7 +90,7 @@ func newKeycloakOIDCProvider(serverURL *url.URL, opts options.KeycloakOptions) *
9090var _ = Describe("Keycloak OIDC Provider Tests", func() {
9191 Context("New Provider Init", func() {
9292 It("creates new keycloak oidc provider with expected defaults", func() {
93− p := newKeycloakOIDCProvider(nil, options.KeycloakOptions{})
93+ p := newKeycloakOIDCProvider(nil, options.Provider{})
9494 providerData := p.Data()
9595 Expect(providerData.ProviderName).To(Equal(keycloakOIDCProviderName))
9696 Expect(providerData.LoginURL.String()).To(Equal("https://keycloak-oidc.com/oauth/auth"))
@@ -100,7 +100,7 @@ var _ = Describe("Keycloak OIDC Provider Tests", func() {
100100 Expect(providerData.Scope).To(Equal(oidcDefaultScope))
101101 })
102102 It("creates new keycloak oidc provider with custom scope", func() {
103− p := NewKeycloakOIDCProvider(&ProviderData{Scope: "openid email"}, options.KeycloakOptions{})
103+ p := NewKeycloakOIDCProvider(&ProviderData{Scope: "openid email"}, options.Provider{})
104104 providerData := p.Data()
105105
106106 Expect(providerData.ProviderName).To(Equal(keycloakOIDCProviderName))
@@ -111,8 +111,10 @@ var _ = Describe("Keycloak OIDC Provider Tests", func() {
111111
112112 Context("Allowed Roles", func() {
113113 It("should prefix allowed roles and add them to groups", func() {
114− p := newKeycloakOIDCProvider(nil, options.KeycloakOptions{
115− Roles: []string{"admin", "editor"},
114+ p := newKeycloakOIDCProvider(nil, options.Provider{
115+ KeycloakConfig: options.KeycloakOptions{
116+ Roles: []string{"admin", "editor"},
117+ },
116118 })
117119 Expect(p.AllowedGroups).To(HaveKey("role:admin"))
118120 Expect(p.AllowedGroups).To(HaveKey("role:editor"))
providers/providers.go+3−3
@@ -38,7 +38,7 @@ func NewProvider(providerConfig options.Provider) (Provider, error) {
3838 }
3939 switch providerConfig.Type {
4040 case options.ADFSProvider:
41− return NewADFSProvider(providerData, providerConfig.ADFSConfig), nil
41+ return NewADFSProvider(providerData, providerConfig), nil
4242 case options.AzureProvider:
4343 return NewAzureProvider(providerData, providerConfig.AzureConfig), nil
4444 case options.BitbucketProvider:
@@ -50,13 +50,13 @@ func NewProvider(providerConfig options.Provider) (Provider, error) {
5050 case options.GitHubProvider:
5151 return NewGitHubProvider(providerData, providerConfig.GitHubConfig), nil
5252 case options.GitLabProvider:
53− return NewGitLabProvider(providerData, providerConfig.GitLabConfig)
53+ return NewGitLabProvider(providerData, providerConfig)
5454 case options.GoogleProvider:
5555 return NewGoogleProvider(providerData, providerConfig.GoogleConfig)
5656 case options.KeycloakProvider:
5757 return NewKeycloakProvider(providerData, providerConfig.KeycloakConfig), nil
5858 case options.KeycloakOIDCProvider:
59− return NewKeycloakOIDCProvider(providerData, providerConfig.KeycloakConfig), nil
59+ return NewKeycloakOIDCProvider(providerData, providerConfig), nil
6060 case options.LinkedInProvider:
6161 return NewLinkedInProvider(providerData), nil
6262 case options.LoginGovProvider: