Skip to content

Commit

Permalink
bugfix: default scopes for OIDCProvider based providers
Browse files Browse the repository at this point in the history
  • Loading branch information
tuunit committed Sep 10, 2023
1 parent 3c2d67d commit ea8d441
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 15 deletions.
5 changes: 1 addition & 4 deletions providers/adfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 7 additions & 1 deletion providers/adfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
})

Expand Down
5 changes: 1 addition & 4 deletions providers/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions providers/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions providers/keycloak_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 10 additions & 2 deletions providers/keycloak_oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
})
})

Expand Down
8 changes: 7 additions & 1 deletion providers/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "oidc" {
name = p.ProviderName
}

oidcProviderDefaults := providerDefaults{
name: "OpenID Connect",
name: name,
loginURL: nil,
redeemURL: nil,
profileURL: nil,
Expand Down

0 comments on commit ea8d441

Please sign in to comment.