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>
This commit is contained in:
Jan Larwig
2023-11-25 11:32:31 +00:00
committed by GitHub
co-authored by Joel Speed
parent 0ddb5e7b61
commit fc6e7fdbd1
8 changed files with 36 additions and 29 deletions
+3 -3
View File
@@ -29,7 +29,7 @@ const (
)
// NewADFSProvider initiates a new ADFSProvider
func NewADFSProvider(p *ProviderData, opts options.ADFSOptions) *ADFSProvider {
func NewADFSProvider(p *ProviderData, opts options.Provider) *ADFSProvider {
p.setProviderDefaults(providerDefaults{
name: adfsProviderName,
scope: adfsDefaultScope,
@@ -46,11 +46,11 @@ func NewADFSProvider(p *ProviderData, opts options.ADFSOptions) *ADFSProvider {
}
}
oidcProvider := NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false})
oidcProvider := NewOIDCProvider(p, opts.OIDCConfig)
return &ADFSProvider{
OIDCProvider: oidcProvider,
skipScope: opts.SkipScope,
skipScope: opts.ADFSConfig.SkipScope,
oidcEnrichFunc: oidcProvider.EnrichSession,
oidcRefreshFunc: oidcProvider.RefreshSession,
}
+7 -5
View File
@@ -63,7 +63,7 @@ func testADFSProvider(hostname string) *ADFSProvider {
Scope: "",
Verifier: o,
EmailClaim: options.OIDCEmailClaim,
}, options.ADFSOptions{})
}, options.Provider{})
if hostname != "" {
updateURL(p.Data().LoginURL, hostname)
@@ -134,12 +134,12 @@ var _ = Describe("ADFS Provider Tests", func() {
Context("New Provider Init", func() {
It("uses defaults", func() {
providerData := NewADFSProvider(&ProviderData{}, options.ADFSOptions{}).Data()
providerData := NewADFSProvider(&ProviderData{}, options.Provider{}).Data()
Expect(providerData.ProviderName).To(Equal("ADFS"))
Expect(providerData.Scope).To(Equal(oidcDefaultScope))
})
It("uses custom scope", func() {
providerData := NewADFSProvider(&ProviderData{Scope: "openid email"}, options.ADFSOptions{}).Data()
providerData := NewADFSProvider(&ProviderData{Scope: "openid email"}, options.Provider{}).Data()
Expect(providerData.ProviderName).To(Equal("ADFS"))
Expect(providerData.Scope).To(Equal("openid email"))
Expect(providerData.Scope).NotTo(Equal(oidcDefaultScope))
@@ -172,7 +172,9 @@ var _ = Describe("ADFS Provider Tests", func() {
p := NewADFSProvider(&ProviderData{
ProtectedResource: resource,
Scope: "",
}, options.ADFSOptions{SkipScope: true})
}, options.Provider{
ADFSConfig: options.ADFSOptions{SkipScope: true},
})
result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "", url.Values{})
Expect(result).NotTo(ContainSubstring("scope="))
@@ -192,7 +194,7 @@ var _ = Describe("ADFS Provider Tests", func() {
p := NewADFSProvider(&ProviderData{
ProtectedResource: resource,
Scope: in.scope,
}, options.ADFSOptions{})
}, options.Provider{})
Expect(p.Data().Scope).To(Equal(in.expectedScope))
result := p.GetLoginURL("https://example.com/adfs/oauth2/", "", "", url.Values{})
+4 -4
View File
@@ -31,7 +31,7 @@ type GitLabProvider struct {
var _ Provider = (*GitLabProvider)(nil)
// NewGitLabProvider initiates a new GitLabProvider
func NewGitLabProvider(p *ProviderData, opts options.GitLabOptions) (*GitLabProvider, error) {
func NewGitLabProvider(p *ProviderData, opts options.Provider) (*GitLabProvider, error) {
p.setProviderDefaults(providerDefaults{
name: gitlabProviderName,
})
@@ -40,15 +40,15 @@ func NewGitLabProvider(p *ProviderData, opts options.GitLabOptions) (*GitLabProv
p.Scope = gitlabDefaultScope
}
oidcProvider := NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false})
oidcProvider := NewOIDCProvider(p, opts.OIDCConfig)
provider := &GitLabProvider{
OIDCProvider: oidcProvider,
oidcRefreshFunc: oidcProvider.RefreshSession,
}
provider.setAllowedGroups(opts.Group)
provider.setAllowedGroups(opts.GitLabConfig.Group)
if err := provider.setAllowedProjects(opts.Projects); err != nil {
if err := provider.setAllowedProjects(opts.GitLabConfig.Projects); err != nil {
return nil, fmt.Errorf("could not configure allowed projects: %v", err)
}
+7 -5
View File
@@ -14,7 +14,7 @@ import (
. "github.com/onsi/gomega"
)
func testGitLabProvider(hostname, scope string, opts options.GitLabOptions) (*GitLabProvider, error) {
func testGitLabProvider(hostname, scope string, opts options.Provider) (*GitLabProvider, error) {
p, err := NewGitLabProvider(
&ProviderData{
ProviderName: "",
@@ -162,7 +162,7 @@ var _ = Describe("Gitlab Provider Tests", func() {
bURL, err := url.Parse(b.URL)
Expect(err).To(BeNil())
p, err = testGitLabProvider(bURL.Host, "", options.GitLabOptions{})
p, err = testGitLabProvider(bURL.Host, "", options.Provider{})
Expect(err).ToNot(HaveOccurred())
})
@@ -237,9 +237,11 @@ var _ = Describe("Gitlab Provider Tests", func() {
bURL, err := url.Parse(b.URL)
Expect(err).To(BeNil())
p, err := testGitLabProvider(bURL.Host, in.scope, options.GitLabOptions{
Group: in.allowedGroups,
Projects: in.allowedProjects,
p, err := testGitLabProvider(bURL.Host, in.scope, options.Provider{
GitLabConfig: options.GitLabOptions{
Group: in.allowedGroups,
Projects: in.allowedProjects,
},
})
if in.expectedError == nil {
Expect(err).To(BeNil())
+3 -3
View File
@@ -16,16 +16,16 @@ type KeycloakOIDCProvider struct {
}
// NewKeycloakOIDCProvider makes a KeycloakOIDCProvider using the ProviderData
func NewKeycloakOIDCProvider(p *ProviderData, opts options.KeycloakOptions) *KeycloakOIDCProvider {
func NewKeycloakOIDCProvider(p *ProviderData, opts options.Provider) *KeycloakOIDCProvider {
p.setProviderDefaults(providerDefaults{
name: keycloakOIDCProviderName,
})
provider := &KeycloakOIDCProvider{
OIDCProvider: NewOIDCProvider(p, options.OIDCOptions{InsecureSkipNonce: false}),
OIDCProvider: NewOIDCProvider(p, opts.OIDCConfig),
}
provider.addAllowedRoles(opts.Roles)
provider.addAllowedRoles(opts.KeycloakConfig.Roles)
return provider
}
+8 -6
View File
@@ -40,11 +40,11 @@ func getAccessToken() string {
func newTestKeycloakOIDCSetup() (*httptest.Server, *KeycloakOIDCProvider) {
redeemURL, server := newOIDCServer([]byte(fmt.Sprintf(`{"email": "new@thing.com", "expires_in": 300, "access_token": "%v"}`, getAccessToken())))
provider := newKeycloakOIDCProvider(redeemURL, options.KeycloakOptions{})
provider := newKeycloakOIDCProvider(redeemURL, options.Provider{})
return server, provider
}
func newKeycloakOIDCProvider(serverURL *url.URL, opts options.KeycloakOptions) *KeycloakOIDCProvider {
func newKeycloakOIDCProvider(serverURL *url.URL, opts options.Provider) *KeycloakOIDCProvider {
verificationOptions := internaloidc.IDTokenVerificationOptions{
AudienceClaims: []string{defaultAudienceClaim},
ClientID: mockClientID,
@@ -90,7 +90,7 @@ func newKeycloakOIDCProvider(serverURL *url.URL, opts options.KeycloakOptions) *
var _ = Describe("Keycloak OIDC Provider Tests", func() {
Context("New Provider Init", func() {
It("creates new keycloak oidc provider with expected defaults", func() {
p := newKeycloakOIDCProvider(nil, options.KeycloakOptions{})
p := newKeycloakOIDCProvider(nil, options.Provider{})
providerData := p.Data()
Expect(providerData.ProviderName).To(Equal(keycloakOIDCProviderName))
Expect(providerData.LoginURL.String()).To(Equal("https://keycloak-oidc.com/oauth/auth"))
@@ -100,7 +100,7 @@ var _ = Describe("Keycloak OIDC Provider Tests", func() {
Expect(providerData.Scope).To(Equal(oidcDefaultScope))
})
It("creates new keycloak oidc provider with custom scope", func() {
p := NewKeycloakOIDCProvider(&ProviderData{Scope: "openid email"}, options.KeycloakOptions{})
p := NewKeycloakOIDCProvider(&ProviderData{Scope: "openid email"}, options.Provider{})
providerData := p.Data()
Expect(providerData.ProviderName).To(Equal(keycloakOIDCProviderName))
@@ -111,8 +111,10 @@ var _ = Describe("Keycloak OIDC Provider Tests", func() {
Context("Allowed Roles", func() {
It("should prefix allowed roles and add them to groups", func() {
p := newKeycloakOIDCProvider(nil, options.KeycloakOptions{
Roles: []string{"admin", "editor"},
p := newKeycloakOIDCProvider(nil, options.Provider{
KeycloakConfig: options.KeycloakOptions{
Roles: []string{"admin", "editor"},
},
})
Expect(p.AllowedGroups).To(HaveKey("role:admin"))
Expect(p.AllowedGroups).To(HaveKey("role:editor"))
+3 -3
View File
@@ -38,7 +38,7 @@ func NewProvider(providerConfig options.Provider) (Provider, error) {
}
switch providerConfig.Type {
case options.ADFSProvider:
return NewADFSProvider(providerData, providerConfig.ADFSConfig), nil
return NewADFSProvider(providerData, providerConfig), nil
case options.AzureProvider:
return NewAzureProvider(providerData, providerConfig.AzureConfig), nil
case options.BitbucketProvider:
@@ -50,13 +50,13 @@ func NewProvider(providerConfig options.Provider) (Provider, error) {
case options.GitHubProvider:
return NewGitHubProvider(providerData, providerConfig.GitHubConfig), nil
case options.GitLabProvider:
return NewGitLabProvider(providerData, providerConfig.GitLabConfig)
return NewGitLabProvider(providerData, providerConfig)
case options.GoogleProvider:
return NewGoogleProvider(providerData, providerConfig.GoogleConfig)
case options.KeycloakProvider:
return NewKeycloakProvider(providerData, providerConfig.KeycloakConfig), nil
case options.KeycloakOIDCProvider:
return NewKeycloakOIDCProvider(providerData, providerConfig.KeycloakConfig), nil
return NewKeycloakOIDCProvider(providerData, providerConfig), nil
case options.LinkedInProvider:
return NewLinkedInProvider(providerData), nil
case options.LoginGovProvider: