From d4dd34a65a3392e63fb74429a537072fb124db51 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 25 May 2020 13:08:04 +0100 Subject: [PATCH] Move provider URLs to package level vars --- CHANGELOG.md | 1 + providers/azure.go | 93 +++++++++++++++++++++++---------- providers/azure_test.go | 32 +++++------- providers/bitbucket.go | 66 ++++++++++++++--------- providers/bitbucket_test.go | 26 ++++----- providers/digitalocean.go | 64 ++++++++++++++--------- providers/digitalocean_test.go | 25 +++++---- providers/facebook.go | 66 ++++++++++++++--------- providers/facebook_test.go | 20 +++++++ providers/github.go | 67 +++++++++++++++--------- providers/github_test.go | 23 ++++---- providers/gitlab.go | 9 +++- providers/google.go | 64 +++++++++++++++-------- providers/google_test.go | 25 +++++---- providers/internal_util_test.go | 3 ++ providers/keycloak.go | 64 ++++++++++++++--------- providers/keycloak_test.go | 17 +++++- providers/linkedin.go | 61 +++++++++++++-------- providers/linkedin_test.go | 25 +++++---- providers/logingov.go | 66 +++++++++++++---------- providers/logingov_test.go | 24 ++++----- providers/nextcloud.go | 4 +- providers/provider_data.go | 35 +++++++++++++ 23 files changed, 553 insertions(+), 327 deletions(-) create mode 100644 providers/facebook_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 74215868..5f473fcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ## Changes since v6.0.0 +- [#561](https://github.com/oauth2-proxy/oauth2-proxy/pull/561) Refactor provider URLs to package level vars (@JoelSpeed) - [#682](https://github.com/oauth2-proxy/oauth2-proxy/pull/682) Refactor persistent session store session ticket management (@NickMeves) - [#688](https://github.com/oauth2-proxy/oauth2-proxy/pull/688) Refactor session loading to make use of middleware pattern (@JoelSpeed) - [#593](https://github.com/oauth2-proxy/oauth2-proxy/pull/593) Integrate upstream package with OAuth2 Proxy (@JoelSpeed) diff --git a/providers/azure.go b/providers/azure.go index b38c1cc7..c713854c 100644 --- a/providers/azure.go +++ b/providers/azure.go @@ -23,49 +23,84 @@ type AzureProvider struct { var _ Provider = (*AzureProvider)(nil) +const ( + azureProviderName = "Azure" + azureDefaultScope = "openid" +) + +var ( + // Default Login URL for Azure. + // Pre-parsed URL of https://login.microsoftonline.com/common/oauth2/authorize. + azureDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "login.microsoftonline.com", + Path: "/common/oauth2/authorize", + } + + // Default Redeem URL for Azure. + // Pre-parsed URL of https://login.microsoftonline.com/common/oauth2/token. + azureDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "login.microsoftonline.com", + Path: "/common/oauth2/token", + } + + // Default Profile URL for Azure. + // Pre-parsed URL of https://graph.microsoft.com/v1.0/me. + azureDefaultProfileURL = &url.URL{ + Scheme: "https", + Host: "graph.microsoft.com", + Path: "/v1.0/me", + } + + // Default ProtectedResource URL for Azure. + // Pre-parsed URL of https://graph.microsoft.com. + azureDefaultProtectResourceURL = &url.URL{ + Scheme: "https", + Host: "graph.microsoft.com", + } +) + // NewAzureProvider initiates a new AzureProvider func NewAzureProvider(p *ProviderData) *AzureProvider { - p.ProviderName = "Azure" + p.setProviderDefaults(providerDefaults{ + name: azureProviderName, + loginURL: azureDefaultLoginURL, + redeemURL: azureDefaultRedeemURL, + profileURL: azureDefaultProfileURL, + validateURL: nil, + scope: azureDefaultScope, + }) - if p.ProfileURL == nil || p.ProfileURL.String() == "" { - p.ProfileURL = &url.URL{ - Scheme: "https", - Host: "graph.microsoft.com", - Path: "/v1.0/me", - } - } if p.ProtectedResource == nil || p.ProtectedResource.String() == "" { - p.ProtectedResource = &url.URL{ - Scheme: "https", - Host: "graph.microsoft.com", - } - } - if p.Scope == "" { - p.Scope = "openid" + p.ProtectedResource = azureDefaultProtectResourceURL } - return &AzureProvider{ProviderData: p} + return &AzureProvider{ + ProviderData: p, + Tenant: "common", + } } // Configure defaults the AzureProvider configuration options func (p *AzureProvider) Configure(tenant string) { - p.Tenant = tenant - if tenant == "" { - p.Tenant = "common" + if tenant == "" || tenant == "common" { + // tenant is empty or default, remain on the default "common" tenant + return } - if p.LoginURL == nil || p.LoginURL.String() == "" { - p.LoginURL = &url.URL{ + // Specific tennant specified, override the Login and RedeemURLs + p.Tenant = tenant + overrideTenantURL(p.LoginURL, azureDefaultLoginURL, tenant, "authorize") + overrideTenantURL(p.RedeemURL, azureDefaultRedeemURL, tenant, "token") +} + +func overrideTenantURL(current, defaultURL *url.URL, tenant, path string) { + if current == nil || current.String() == "" || current.String() == defaultURL.String() { + *current = url.URL{ Scheme: "https", Host: "login.microsoftonline.com", - Path: "/" + p.Tenant + "/oauth2/authorize"} - } - if p.RedeemURL == nil || p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{ - Scheme: "https", - Host: "login.microsoftonline.com", - Path: "/" + p.Tenant + "/oauth2/token", - } + Path: "/" + tenant + "/oauth2/" + path} } } diff --git a/providers/azure_test.go b/providers/azure_test.go index af364b77..fe9bbb42 100644 --- a/providers/azure_test.go +++ b/providers/azure_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" ) @@ -32,23 +33,17 @@ func testAzureProvider(hostname string) *AzureProvider { return p } -func TestAzureProviderDefaults(t *testing.T) { - p := testAzureProvider("") - assert.NotEqual(t, nil, p) - p.Configure("") - assert.Equal(t, "Azure", p.Data().ProviderName) - assert.Equal(t, "common", p.Tenant) - assert.Equal(t, "https://login.microsoftonline.com/common/oauth2/authorize", - p.Data().LoginURL.String()) - assert.Equal(t, "https://login.microsoftonline.com/common/oauth2/token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://graph.microsoft.com/v1.0/me", - p.Data().ProfileURL.String()) - assert.Equal(t, "https://graph.microsoft.com", - p.Data().ProtectedResource.String()) - assert.Equal(t, "", - p.Data().ValidateURL.String()) - assert.Equal(t, "openid", p.Data().Scope) +func TestNewAzureProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewAzureProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("Azure")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://login.microsoftonline.com/common/oauth2/authorize")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://login.microsoftonline.com/common/oauth2/token")) + g.Expect(providerData.ProfileURL.String()).To(Equal("https://graph.microsoft.com/v1.0/me")) + g.Expect(providerData.ValidateURL.String()).To(Equal("")) + g.Expect(providerData.Scope).To(Equal("openid")) } func TestAzureProviderOverrides(t *testing.T) { @@ -102,8 +97,7 @@ func TestAzureSetTenant(t *testing.T) { p.Data().ProfileURL.String()) assert.Equal(t, "https://graph.microsoft.com", p.Data().ProtectedResource.String()) - assert.Equal(t, "", - p.Data().ValidateURL.String()) + assert.Equal(t, "", p.Data().ValidateURL.String()) assert.Equal(t, "openid", p.Data().Scope) } diff --git a/providers/bitbucket.go b/providers/bitbucket.go index ffc52c79..8b6b71db 100644 --- a/providers/bitbucket.go +++ b/providers/bitbucket.go @@ -19,33 +19,49 @@ type BitbucketProvider struct { var _ Provider = (*BitbucketProvider)(nil) +const ( + bitbucketProviderName = "Bitbucket" + bitbucketDefaultScope = "email" +) + +var ( + // Default Login URL for Bitbucket. + // Pre-parsed URL of https://bitbucket.org/site/oauth2/authorize. + bitbucketDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "bitbucket.org", + Path: "/site/oauth2/authorize", + } + + // Default Redeem URL for Bitbucket. + // Pre-parsed URL of https://bitbucket.org/site/oauth2/access_token. + bitbucketDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "bitbucket.org", + Path: "/site/oauth2/access_token", + } + + // Default Validation URL for Bitbucket. + // This simply returns the email of the authenticated user. + // Bitbucket does not have a Profile URL to use. + // Pre-parsed URL of https://api.bitbucket.org/2.0/user/emails. + bitbucketDefaultValidateURL = &url.URL{ + Scheme: "https", + Host: "api.bitbucket.org", + Path: "/2.0/user/emails", + } +) + // NewBitbucketProvider initiates a new BitbucketProvider func NewBitbucketProvider(p *ProviderData) *BitbucketProvider { - p.ProviderName = "Bitbucket" - if p.LoginURL == nil || p.LoginURL.String() == "" { - p.LoginURL = &url.URL{ - Scheme: "https", - Host: "bitbucket.org", - Path: "/site/oauth2/authorize", - } - } - if p.RedeemURL == nil || p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{ - Scheme: "https", - Host: "bitbucket.org", - Path: "/site/oauth2/access_token", - } - } - if p.ValidateURL == nil || p.ValidateURL.String() == "" { - p.ValidateURL = &url.URL{ - Scheme: "https", - Host: "api.bitbucket.org", - Path: "/2.0/user/emails", - } - } - if p.Scope == "" { - p.Scope = "email" - } + p.setProviderDefaults(providerDefaults{ + name: bitbucketProviderName, + loginURL: bitbucketDefaultLoginURL, + redeemURL: bitbucketDefaultRedeemURL, + profileURL: nil, + validateURL: bitbucketDefaultValidateURL, + scope: bitbucketDefaultScope, + }) return &BitbucketProvider{ProviderData: p} } diff --git a/providers/bitbucket_test.go b/providers/bitbucket_test.go index e788b81e..917cf4ca 100644 --- a/providers/bitbucket_test.go +++ b/providers/bitbucket_test.go @@ -8,9 +8,9 @@ import ( "net/url" "testing" - "github.com/stretchr/testify/assert" - "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/assert" ) func testBitbucketProvider(hostname, team string, repository string) *BitbucketProvider { @@ -61,17 +61,17 @@ func testBitbucketBackend(payload string) *httptest.Server { })) } -func TestBitbucketProviderDefaults(t *testing.T) { - p := testBitbucketProvider("", "", "") - assert.NotEqual(t, nil, p) - assert.Equal(t, "Bitbucket", p.Data().ProviderName) - assert.Equal(t, "https://bitbucket.org/site/oauth2/authorize", - p.Data().LoginURL.String()) - assert.Equal(t, "https://bitbucket.org/site/oauth2/access_token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://api.bitbucket.org/2.0/user/emails", - p.Data().ValidateURL.String()) - assert.Equal(t, "email", p.Data().Scope) +func TestNewBitbucketProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewBitbucketProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("Bitbucket")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://bitbucket.org/site/oauth2/authorize")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://bitbucket.org/site/oauth2/access_token")) + g.Expect(providerData.ProfileURL.String()).To(Equal("")) + g.Expect(providerData.ValidateURL.String()).To(Equal("https://api.bitbucket.org/2.0/user/emails")) + g.Expect(providerData.Scope).To(Equal("email")) } func TestBitbucketProviderScopeAdjustForTeam(t *testing.T) { diff --git a/providers/digitalocean.go b/providers/digitalocean.go index 27ac60d0..a5314892 100644 --- a/providers/digitalocean.go +++ b/providers/digitalocean.go @@ -18,33 +18,47 @@ type DigitalOceanProvider struct { var _ Provider = (*DigitalOceanProvider)(nil) +const ( + digitalOceanProviderName = "DigitalOcean" + digitalOceanDefaultScope = "read" +) + +var ( + // Default Login URL for DigitalOcean. + // Pre-parsed URL of https://cloud.digitalocean.com/v1/oauth/authorize. + digitalOceanDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "cloud.digitalocean.com", + Path: "/v1/oauth/authorize", + } + + // Default Redeem URL for DigitalOcean. + // Pre-parsed URL of https://cloud.digitalocean.com/v1/oauth/token. + digitalOceanDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "cloud.digitalocean.com", + Path: "/v1/oauth/token", + } + + // Default Profile URL for DigitalOcean. + // Pre-parsed URL of https://cloud.digitalocean.com/v2/account. + digitalOceanDefaultProfileURL = &url.URL{ + Scheme: "https", + Host: "api.digitalocean.com", + Path: "/v2/account", + } +) + // NewDigitalOceanProvider initiates a new DigitalOceanProvider func NewDigitalOceanProvider(p *ProviderData) *DigitalOceanProvider { - p.ProviderName = "DigitalOcean" - if p.LoginURL.String() == "" { - p.LoginURL = &url.URL{Scheme: "https", - Host: "cloud.digitalocean.com", - Path: "/v1/oauth/authorize", - } - } - if p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{Scheme: "https", - Host: "cloud.digitalocean.com", - Path: "/v1/oauth/token", - } - } - if p.ProfileURL.String() == "" { - p.ProfileURL = &url.URL{Scheme: "https", - Host: "api.digitalocean.com", - Path: "/v2/account", - } - } - if p.ValidateURL.String() == "" { - p.ValidateURL = p.ProfileURL - } - if p.Scope == "" { - p.Scope = "read" - } + p.setProviderDefaults(providerDefaults{ + name: digitalOceanProviderName, + loginURL: digitalOceanDefaultLoginURL, + redeemURL: digitalOceanDefaultRedeemURL, + profileURL: digitalOceanDefaultProfileURL, + validateURL: digitalOceanDefaultProfileURL, + scope: digitalOceanDefaultScope, + }) return &DigitalOceanProvider{ProviderData: p} } diff --git a/providers/digitalocean_test.go b/providers/digitalocean_test.go index e7907eba..4c8d2a1d 100644 --- a/providers/digitalocean_test.go +++ b/providers/digitalocean_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" + . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" ) @@ -44,19 +45,17 @@ func testDigitalOceanBackend(payload string) *httptest.Server { })) } -func TestDigitalOceanProviderDefaults(t *testing.T) { - p := testDigitalOceanProvider("") - assert.NotEqual(t, nil, p) - assert.Equal(t, "DigitalOcean", p.Data().ProviderName) - assert.Equal(t, "https://cloud.digitalocean.com/v1/oauth/authorize", - p.Data().LoginURL.String()) - assert.Equal(t, "https://cloud.digitalocean.com/v1/oauth/token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://api.digitalocean.com/v2/account", - p.Data().ProfileURL.String()) - assert.Equal(t, "https://api.digitalocean.com/v2/account", - p.Data().ValidateURL.String()) - assert.Equal(t, "read", p.Data().Scope) +func TestNewDigitalOceanProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewDigitalOceanProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("DigitalOcean")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://cloud.digitalocean.com/v1/oauth/authorize")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://cloud.digitalocean.com/v1/oauth/token")) + g.Expect(providerData.ProfileURL.String()).To(Equal("https://api.digitalocean.com/v2/account")) + g.Expect(providerData.ValidateURL.String()).To(Equal("https://api.digitalocean.com/v2/account")) + g.Expect(providerData.Scope).To(Equal("read")) } func TestDigitalOceanProviderOverrides(t *testing.T) { diff --git a/providers/facebook.go b/providers/facebook.go index d3d123f2..00a5b55b 100644 --- a/providers/facebook.go +++ b/providers/facebook.go @@ -18,34 +18,48 @@ type FacebookProvider struct { var _ Provider = (*FacebookProvider)(nil) +const ( + facebookProviderName = "Facebook" + facebookDefaultScope = "public_profile email" +) + +var ( + // Default Login URL for Facebook. + // Pre-parsed URL of https://www.facebook.com/v2.5/dialog/oauth. + facebookDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "www.facebook.com", + Path: "/v2.5/dialog/oauth", + // ?granted_scopes=true + } + + // Default Redeem URL for Facebook. + // Pre-parsed URL of https://graph.facebook.com/v2.5/oauth/access_token. + facebookDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "graph.facebook.com", + Path: "/v2.5/oauth/access_token", + } + + // Default Profile URL for Facebook. + // Pre-parsed URL of https://graph.facebook.com/v2.5/me. + facebookDefaultProfileURL = &url.URL{ + Scheme: "https", + Host: "graph.facebook.com", + Path: "/v2.5/me", + } +) + // NewFacebookProvider initiates a new FacebookProvider func NewFacebookProvider(p *ProviderData) *FacebookProvider { - p.ProviderName = "Facebook" - if p.LoginURL.String() == "" { - p.LoginURL = &url.URL{Scheme: "https", - Host: "www.facebook.com", - Path: "/v2.5/dialog/oauth", - // ?granted_scopes=true - } - } - if p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{Scheme: "https", - Host: "graph.facebook.com", - Path: "/v2.5/oauth/access_token", - } - } - if p.ProfileURL.String() == "" { - p.ProfileURL = &url.URL{Scheme: "https", - Host: "graph.facebook.com", - Path: "/v2.5/me", - } - } - if p.ValidateURL.String() == "" { - p.ValidateURL = p.ProfileURL - } - if p.Scope == "" { - p.Scope = "public_profile email" - } + p.setProviderDefaults(providerDefaults{ + name: facebookProviderName, + loginURL: facebookDefaultLoginURL, + redeemURL: facebookDefaultRedeemURL, + profileURL: facebookDefaultProfileURL, + validateURL: facebookDefaultProfileURL, + scope: facebookDefaultScope, + }) return &FacebookProvider{ProviderData: p} } diff --git a/providers/facebook_test.go b/providers/facebook_test.go new file mode 100644 index 00000000..9e950457 --- /dev/null +++ b/providers/facebook_test.go @@ -0,0 +1,20 @@ +package providers + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestNewFacebookProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewFacebookProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("Facebook")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://www.facebook.com/v2.5/dialog/oauth")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://graph.facebook.com/v2.5/oauth/access_token")) + g.Expect(providerData.ProfileURL.String()).To(Equal("https://graph.facebook.com/v2.5/me")) + g.Expect(providerData.ValidateURL.String()).To(Equal("https://graph.facebook.com/v2.5/me")) + g.Expect(providerData.Scope).To(Equal("public_profile email")) +} diff --git a/providers/github.go b/providers/github.go index 014ae3cb..7d08ac0e 100644 --- a/providers/github.go +++ b/providers/github.go @@ -28,34 +28,49 @@ type GitHubProvider struct { var _ Provider = (*GitHubProvider)(nil) +const ( + githubProviderName = "GitHub" + githubDefaultScope = "user:email" +) + +var ( + // Default Login URL for GitHub. + // Pre-parsed URL of https://github.org/login/oauth/authorize. + githubDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "github.com", + Path: "/login/oauth/authorize", + } + + // Default Redeem URL for GitHub. + // Pre-parsed URL of https://github.org/login/oauth/access_token. + githubDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "github.com", + Path: "/login/oauth/access_token", + } + + // Default Validation URL for GitHub. + // ValidationURL is the API Base URL. + // Other API requests are based off of this (eg to fetch users/groups). + // Pre-parsed URL of https://api.github.com/. + githubDefaultValidateURL = &url.URL{ + Scheme: "https", + Host: "api.github.com", + Path: "/", + } +) + // NewGitHubProvider initiates a new GitHubProvider func NewGitHubProvider(p *ProviderData) *GitHubProvider { - p.ProviderName = "GitHub" - if p.LoginURL == nil || p.LoginURL.String() == "" { - p.LoginURL = &url.URL{ - Scheme: "https", - Host: "github.com", - Path: "/login/oauth/authorize", - } - } - if p.RedeemURL == nil || p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{ - Scheme: "https", - Host: "github.com", - Path: "/login/oauth/access_token", - } - } - // ValidationURL is the API Base URL - if p.ValidateURL == nil || p.ValidateURL.String() == "" { - p.ValidateURL = &url.URL{ - Scheme: "https", - Host: "api.github.com", - Path: "/", - } - } - if p.Scope == "" { - p.Scope = "user:email" - } + p.setProviderDefaults(providerDefaults{ + name: githubProviderName, + loginURL: githubDefaultLoginURL, + redeemURL: githubDefaultRedeemURL, + profileURL: nil, + validateURL: githubDefaultValidateURL, + scope: githubDefaultScope, + }) return &GitHubProvider{ProviderData: p} } diff --git a/providers/github_test.go b/providers/github_test.go index 46e6f1fc..ab2bb04e 100644 --- a/providers/github_test.go +++ b/providers/github_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" + . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" ) @@ -66,17 +67,17 @@ func testGitHubBackend(payloads map[string][]string) *httptest.Server { })) } -func TestGitHubProviderDefaults(t *testing.T) { - p := testGitHubProvider("") - assert.NotEqual(t, nil, p) - assert.Equal(t, "GitHub", p.Data().ProviderName) - assert.Equal(t, "https://github.com/login/oauth/authorize", - p.Data().LoginURL.String()) - assert.Equal(t, "https://github.com/login/oauth/access_token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://api.github.com/", - p.Data().ValidateURL.String()) - assert.Equal(t, "user:email", p.Data().Scope) +func TestNewGitHubProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewGitHubProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("GitHub")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://github.com/login/oauth/authorize")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://github.com/login/oauth/access_token")) + g.Expect(providerData.ProfileURL.String()).To(Equal("")) + g.Expect(providerData.ValidateURL.String()).To(Equal("https://api.github.com/")) + g.Expect(providerData.Scope).To(Equal("user:email")) } func TestGitHubProviderOverrides(t *testing.T) { diff --git a/providers/gitlab.go b/providers/gitlab.go index 8c1e1534..5836e3d6 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -25,12 +25,17 @@ type GitLabProvider struct { var _ Provider = (*GitLabProvider)(nil) +const ( + gitlabProviderName = "GitLab" + gitlabDefaultScope = "openid email" +) + // NewGitLabProvider initiates a new GitLabProvider func NewGitLabProvider(p *ProviderData) *GitLabProvider { - p.ProviderName = "GitLab" + p.ProviderName = gitlabProviderName if p.Scope == "" { - p.Scope = "openid email" + p.Scope = gitlabDefaultScope } return &GitLabProvider{ProviderData: p} diff --git a/providers/google.go b/providers/google.go index af2eebf3..85c064db 100644 --- a/providers/google.go +++ b/providers/google.go @@ -39,31 +39,49 @@ type claims struct { EmailVerified bool `json:"email_verified"` } -// NewGoogleProvider initiates a new GoogleProvider -func NewGoogleProvider(p *ProviderData) *GoogleProvider { - p.ProviderName = "Google" - if p.LoginURL.String() == "" { - p.LoginURL = &url.URL{Scheme: "https", - Host: "accounts.google.com", - Path: "/o/oauth2/auth", - // to get a refresh token. see https://developers.google.com/identity/protocols/OAuth2WebServer#offline - RawQuery: "access_type=offline", - } - } - if p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{Scheme: "https", - Host: "www.googleapis.com", - Path: "/oauth2/v3/token"} - } - if p.ValidateURL.String() == "" { - p.ValidateURL = &url.URL{Scheme: "https", - Host: "www.googleapis.com", - Path: "/oauth2/v1/tokeninfo"} - } - if p.Scope == "" { - p.Scope = "profile email" +const ( + googleProviderName = "Google" + googleDefaultScope = "profile email" +) + +var ( + // Default Login URL for Google. + // Pre-parsed URL of https://accounts.google.com/o/oauth2/auth?access_type=offline. + googleDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "accounts.google.com", + Path: "/o/oauth2/auth", + // to get a refresh token. see https://developers.google.com/identity/protocols/OAuth2WebServer#offline + RawQuery: "access_type=offline", } + // Default Redeem URL for Google. + // Pre-parsed URL of https://www.googleapis.com/oauth2/v3/token. + googleDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "www.googleapis.com", + Path: "/oauth2/v3/token", + } + + // Default Validation URL for Google. + // Pre-parsed URL of https://www.googleapis.com/oauth2/v1/tokeninfo. + googleDefaultValidateURL = &url.URL{ + Scheme: "https", + Host: "www.googleapis.com", + Path: "/oauth2/v1/tokeninfo", + } +) + +// NewGoogleProvider initiates a new GoogleProvider +func NewGoogleProvider(p *ProviderData) *GoogleProvider { + p.setProviderDefaults(providerDefaults{ + name: googleProviderName, + loginURL: googleDefaultLoginURL, + redeemURL: googleDefaultRedeemURL, + profileURL: nil, + validateURL: googleDefaultValidateURL, + scope: googleDefaultScope, + }) return &GoogleProvider{ ProviderData: p, // Set a default GroupValidator to just always return valid (true), it will diff --git a/providers/google_test.go b/providers/google_test.go index 63e5a9a8..35fc7f49 100644 --- a/providers/google_test.go +++ b/providers/google_test.go @@ -10,8 +10,8 @@ import ( "net/url" "testing" + . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" - admin "google.golang.org/api/admin/directory/v1" option "google.golang.org/api/option" ) @@ -35,18 +35,17 @@ func newGoogleProvider() *GoogleProvider { Scope: ""}) } -func TestGoogleProviderDefaults(t *testing.T) { - p := newGoogleProvider() - assert.NotEqual(t, nil, p) - assert.Equal(t, "Google", p.Data().ProviderName) - assert.Equal(t, "https://accounts.google.com/o/oauth2/auth?access_type=offline", - p.Data().LoginURL.String()) - assert.Equal(t, "https://www.googleapis.com/oauth2/v3/token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://www.googleapis.com/oauth2/v1/tokeninfo", - p.Data().ValidateURL.String()) - assert.Equal(t, "", p.Data().ProfileURL.String()) - assert.Equal(t, "profile email", p.Data().Scope) +func TestNewGoogleProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewGoogleProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("Google")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://accounts.google.com/o/oauth2/auth?access_type=offline")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://www.googleapis.com/oauth2/v3/token")) + g.Expect(providerData.ProfileURL.String()).To(Equal("")) + g.Expect(providerData.ValidateURL.String()).To(Equal("https://www.googleapis.com/oauth2/v1/tokeninfo")) + g.Expect(providerData.Scope).To(Equal("profile email")) } func TestGoogleProviderOverrides(t *testing.T) { diff --git a/providers/internal_util_test.go b/providers/internal_util_test.go index 0f6aa437..03579c01 100644 --- a/providers/internal_util_test.go +++ b/providers/internal_util_test.go @@ -13,6 +13,9 @@ import ( ) func updateURL(url *url.URL, hostname string) { + if url == nil { + return + } url.Scheme = "http" url.Host = hostname } diff --git a/providers/keycloak.go b/providers/keycloak.go index 77efc0c7..cb795fe2 100644 --- a/providers/keycloak.go +++ b/providers/keycloak.go @@ -16,32 +16,46 @@ type KeycloakProvider struct { var _ Provider = (*KeycloakProvider)(nil) +const ( + keycloakProviderName = "Keycloak" + keycloakDefaultScope = "api" +) + +var ( + // Default Login URL for Keycloak. + // Pre-parsed URL of https://keycloak.org/oauth/authorize. + keycloakDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "keycloak.org", + Path: "/oauth/authorize", + } + + // Default Redeem URL for Keycloak. + // Pre-parsed URL of ttps://keycloak.org/oauth/token. + keycloakDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "keycloak.org", + Path: "/oauth/token", + } + + // Default Validation URL for Keycloak. + // Pre-parsed URL of https://keycloak.org/api/v3/user. + keycloakDefaultValidateURL = &url.URL{ + Scheme: "https", + Host: "keycloak.org", + Path: "/api/v3/user", + } +) + func NewKeycloakProvider(p *ProviderData) *KeycloakProvider { - p.ProviderName = "Keycloak" - if p.LoginURL == nil || p.LoginURL.String() == "" { - p.LoginURL = &url.URL{ - Scheme: "https", - Host: "keycloak.org", - Path: "/oauth/authorize", - } - } - if p.RedeemURL == nil || p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{ - Scheme: "https", - Host: "keycloak.org", - Path: "/oauth/token", - } - } - if p.ValidateURL == nil || p.ValidateURL.String() == "" { - p.ValidateURL = &url.URL{ - Scheme: "https", - Host: "keycloak.org", - Path: "/api/v3/user", - } - } - if p.Scope == "" { - p.Scope = "api" - } + p.setProviderDefaults(providerDefaults{ + name: keycloakProviderName, + loginURL: keycloakDefaultLoginURL, + redeemURL: keycloakDefaultRedeemURL, + profileURL: nil, + validateURL: keycloakDefaultValidateURL, + scope: keycloakDefaultScope, + }) return &KeycloakProvider{ProviderData: p} } diff --git a/providers/keycloak_test.go b/providers/keycloak_test.go index 239d727f..856d2a22 100644 --- a/providers/keycloak_test.go +++ b/providers/keycloak_test.go @@ -7,9 +7,9 @@ import ( "net/url" "testing" - "github.com/stretchr/testify/assert" - "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/assert" ) func testKeycloakProvider(hostname, group string) *KeycloakProvider { @@ -65,6 +65,19 @@ func TestKeycloakProviderDefaults(t *testing.T) { assert.Equal(t, "api", p.Data().Scope) } +func TestNewKeycloakProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewKeycloakProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("Keycloak")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://keycloak.org/oauth/authorize")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://keycloak.org/oauth/token")) + g.Expect(providerData.ProfileURL.String()).To(Equal("")) + g.Expect(providerData.ValidateURL.String()).To(Equal("https://keycloak.org/api/v3/user")) + g.Expect(providerData.Scope).To(Equal("api")) +} + func TestKeycloakProviderOverrides(t *testing.T) { p := NewKeycloakProvider( &ProviderData{ diff --git a/providers/linkedin.go b/providers/linkedin.go index 7328dbbb..67e015e6 100644 --- a/providers/linkedin.go +++ b/providers/linkedin.go @@ -18,30 +18,47 @@ type LinkedInProvider struct { var _ Provider = (*LinkedInProvider)(nil) +const ( + linkedinProviderName = "LinkedIn" + linkedinDefaultScope = "r_emailaddress r_basicprofile" +) + +var ( + // Default Login URL for LinkedIn. + // Pre-parsed URL of https://www.linkedin.com/uas/oauth2/authorization. + linkedinDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "www.linkedin.com", + Path: "/uas/oauth2/authorization", + } + + // Default Redeem URL for LinkedIn. + // Pre-parsed URL of https://www.linkedin.com/uas/oauth2/accessToken. + linkedinDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "www.linkedin.com", + Path: "/uas/oauth2/accessToken", + } + + // Default Profile URL for LinkedIn. + // Pre-parsed URL of https://www.linkedin.com/v1/people/~/email-address. + linkedinDefaultProfileURL = &url.URL{ + Scheme: "https", + Host: "www.linkedin.com", + Path: "/v1/people/~/email-address", + } +) + // NewLinkedInProvider initiates a new LinkedInProvider func NewLinkedInProvider(p *ProviderData) *LinkedInProvider { - p.ProviderName = "LinkedIn" - if p.LoginURL.String() == "" { - p.LoginURL = &url.URL{Scheme: "https", - Host: "www.linkedin.com", - Path: "/uas/oauth2/authorization"} - } - if p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{Scheme: "https", - Host: "www.linkedin.com", - Path: "/uas/oauth2/accessToken"} - } - if p.ProfileURL.String() == "" { - p.ProfileURL = &url.URL{Scheme: "https", - Host: "www.linkedin.com", - Path: "/v1/people/~/email-address"} - } - if p.ValidateURL.String() == "" { - p.ValidateURL = p.ProfileURL - } - if p.Scope == "" { - p.Scope = "r_emailaddress r_basicprofile" - } + p.setProviderDefaults(providerDefaults{ + name: linkedinProviderName, + loginURL: linkedinDefaultLoginURL, + redeemURL: linkedinDefaultRedeemURL, + profileURL: linkedinDefaultProfileURL, + validateURL: linkedinDefaultProfileURL, + scope: linkedinDefaultScope, + }) return &LinkedInProvider{ProviderData: p} } diff --git a/providers/linkedin_test.go b/providers/linkedin_test.go index 6d70d57c..ffbb8b73 100644 --- a/providers/linkedin_test.go +++ b/providers/linkedin_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" + . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" ) @@ -44,19 +45,17 @@ func testLinkedInBackend(payload string) *httptest.Server { })) } -func TestLinkedInProviderDefaults(t *testing.T) { - p := testLinkedInProvider("") - assert.NotEqual(t, nil, p) - assert.Equal(t, "LinkedIn", p.Data().ProviderName) - assert.Equal(t, "https://www.linkedin.com/uas/oauth2/authorization", - p.Data().LoginURL.String()) - assert.Equal(t, "https://www.linkedin.com/uas/oauth2/accessToken", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://www.linkedin.com/v1/people/~/email-address", - p.Data().ProfileURL.String()) - assert.Equal(t, "https://www.linkedin.com/v1/people/~/email-address", - p.Data().ValidateURL.String()) - assert.Equal(t, "r_emailaddress r_basicprofile", p.Data().Scope) +func TestNewLinkedInProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewLinkedInProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("LinkedIn")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://www.linkedin.com/uas/oauth2/authorization")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://www.linkedin.com/uas/oauth2/accessToken")) + g.Expect(providerData.ProfileURL.String()).To(Equal("https://www.linkedin.com/v1/people/~/email-address")) + g.Expect(providerData.ValidateURL.String()).To(Equal("https://www.linkedin.com/v1/people/~/email-address")) + g.Expect(providerData.Scope).To(Equal("r_emailaddress r_basicprofile")) } func TestLinkedInProviderOverrides(t *testing.T) { diff --git a/providers/logingov.go b/providers/logingov.go index 79eb1827..def6043b 100644 --- a/providers/logingov.go +++ b/providers/logingov.go @@ -43,35 +43,47 @@ func randSeq(n int) string { return string(b) } +const ( + loginGovProviderName = "login.gov" + loginGovDefaultScope = "email openid" +) + +var ( + // Default Login URL for LoginGov. + // Pre-parsed URL of https://secure.login.gov/openid_connect/authorize. + loginGovDefaultLoginURL = &url.URL{ + Scheme: "https", + Host: "secure.login.gov", + Path: "/openid_connect/authorize", + } + + // Default Redeem URL for LoginGov. + // Pre-parsed URL of https://secure.login.gov/api/openid_connect/token. + loginGovDefaultRedeemURL = &url.URL{ + Scheme: "https", + Host: "secure.login.gov", + Path: "/api/openid_connect/token", + } + + // Default Profile URL for LoginGov. + // Pre-parsed URL of https://graph.loginGov.com/v2.5/me. + loginGovDefaultProfileURL = &url.URL{ + Scheme: "https", + Host: "secure.login.gov", + Path: "/api/openid_connect/userinfo", + } +) + // NewLoginGovProvider initiates a new LoginGovProvider func NewLoginGovProvider(p *ProviderData) *LoginGovProvider { - p.ProviderName = "login.gov" - - if p.LoginURL == nil || p.LoginURL.String() == "" { - p.LoginURL = &url.URL{ - Scheme: "https", - Host: "secure.login.gov", - Path: "/openid_connect/authorize", - } - } - if p.RedeemURL == nil || p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{ - Scheme: "https", - Host: "secure.login.gov", - Path: "/api/openid_connect/token", - } - } - if p.ProfileURL == nil || p.ProfileURL.String() == "" { - p.ProfileURL = &url.URL{ - Scheme: "https", - Host: "secure.login.gov", - Path: "/api/openid_connect/userinfo", - } - } - if p.Scope == "" { - p.Scope = "email openid" - } - + p.setProviderDefaults(providerDefaults{ + name: loginGovProviderName, + loginURL: loginGovDefaultLoginURL, + redeemURL: loginGovDefaultRedeemURL, + profileURL: loginGovDefaultProfileURL, + validateURL: nil, + scope: loginGovDefaultScope, + }) return &LoginGovProvider{ ProviderData: p, Nonce: randSeq(32), diff --git a/providers/logingov_test.go b/providers/logingov_test.go index 96934c68..2c0f8357 100644 --- a/providers/logingov_test.go +++ b/providers/logingov_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/dgrijalva/jwt-go" + . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" "gopkg.in/square/go-jose.v2" ) @@ -65,18 +66,17 @@ func newLoginGovProvider() (l *LoginGovProvider, serverKey *MyKeyData, err error return } -func TestLoginGovProviderDefaults(t *testing.T) { - p, _, err := newLoginGovProvider() - assert.NotEqual(t, nil, p) - assert.NoError(t, err) - assert.Equal(t, "login.gov", p.Data().ProviderName) - assert.Equal(t, "https://secure.login.gov/openid_connect/authorize", - p.Data().LoginURL.String()) - assert.Equal(t, "https://secure.login.gov/api/openid_connect/token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://secure.login.gov/api/openid_connect/userinfo", - p.Data().ProfileURL.String()) - assert.Equal(t, "email openid", p.Data().Scope) +func TestNewLoginGovProvider(t *testing.T) { + g := NewWithT(t) + + // Test that defaults are set when calling for a new provider with nothing set + providerData := NewLoginGovProvider(&ProviderData{}).Data() + g.Expect(providerData.ProviderName).To(Equal("login.gov")) + g.Expect(providerData.LoginURL.String()).To(Equal("https://secure.login.gov/openid_connect/authorize")) + g.Expect(providerData.RedeemURL.String()).To(Equal("https://secure.login.gov/api/openid_connect/token")) + g.Expect(providerData.ProfileURL.String()).To(Equal("https://secure.login.gov/api/openid_connect/userinfo")) + g.Expect(providerData.ValidateURL.String()).To(Equal("")) + g.Expect(providerData.Scope).To(Equal("email openid")) } func TestLoginGovProviderOverrides(t *testing.T) { diff --git a/providers/nextcloud.go b/providers/nextcloud.go index b70fd07c..2fea1fbe 100644 --- a/providers/nextcloud.go +++ b/providers/nextcloud.go @@ -16,9 +16,11 @@ type NextcloudProvider struct { var _ Provider = (*NextcloudProvider)(nil) +const nextCloudProviderName = "Nextcloud" + // NewNextcloudProvider initiates a new NextcloudProvider func NewNextcloudProvider(p *ProviderData) *NextcloudProvider { - p.ProviderName = "Nextcloud" + p.ProviderName = nextCloudProviderName return &NextcloudProvider{ProviderData: p} } diff --git a/providers/provider_data.go b/providers/provider_data.go index de5bc0d3..1371dd39 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -44,3 +44,38 @@ func (p *ProviderData) GetClientSecret() (clientSecret string, err error) { } return string(fileClientSecret), nil } + +type providerDefaults struct { + name string + loginURL *url.URL + redeemURL *url.URL + profileURL *url.URL + validateURL *url.URL + scope string +} + +func (p *ProviderData) setProviderDefaults(defaults providerDefaults) { + p.ProviderName = defaults.name + p.LoginURL = defaultURL(p.LoginURL, defaults.loginURL) + p.RedeemURL = defaultURL(p.RedeemURL, defaults.redeemURL) + p.ProfileURL = defaultURL(p.ProfileURL, defaults.profileURL) + p.ValidateURL = defaultURL(p.ValidateURL, defaults.validateURL) + + if p.Scope == "" { + p.Scope = defaults.scope + } +} + +// defaultURL will set return a default value if the given value is not set. +func defaultURL(u *url.URL, d *url.URL) *url.URL { + if u != nil && u.String() != "" { + // The value is already set + return u + } + + // If the default is given, return that + if d != nil { + return d + } + return &url.URL{} +}