Merge pull request #561 from oauth2-proxy/provider-urls-refactor

Move provider URLs to package level vars

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)

providers/azure.go

@@ -23,49 +23,84 @@ type AzureProvider struct {
 
 var _ Provider = (*AzureProvider)(nil)
 
-// NewAzureProvider initiates a new AzureProvider
+const (
-func NewAzureProvider(p *ProviderData) *AzureProvider {
+	azureProviderName = "Azure"
-	p.ProviderName = "Azure"
+	azureDefaultScope = "openid"
+)
 
-	if p.ProfileURL == nil || p.ProfileURL.String() == "" {
+var (
-		p.ProfileURL = &url.URL{
+	// 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",
 	}
-	}
+
-	if p.ProtectedResource == nil || p.ProtectedResource.String() == "" {
+	// Default ProtectedResource URL for Azure.
-		p.ProtectedResource = &url.URL{
+	// Pre-parsed URL of https://graph.microsoft.com.
+	azureDefaultProtectResourceURL = &url.URL{
 		Scheme: "https",
 		Host:   "graph.microsoft.com",
 	}
-	}
+)
-	if p.Scope == "" {
+
-		p.Scope = "openid"
+// NewAzureProvider initiates a new AzureProvider
+func NewAzureProvider(p *ProviderData) *AzureProvider {
+	p.setProviderDefaults(providerDefaults{
+		name:        azureProviderName,
+		loginURL:    azureDefaultLoginURL,
+		redeemURL:   azureDefaultRedeemURL,
+		profileURL:  azureDefaultProfileURL,
+		validateURL: nil,
+		scope:       azureDefaultScope,
+	})
+
+	if p.ProtectedResource == nil || p.ProtectedResource.String() == "" {
+		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 == "" || tenant == "common" {
-	if tenant == "" {
+		// tenant is empty or default, remain on the default "common" tenant
-		p.Tenant = "common"
+		return
 	}
 
-	if p.LoginURL == nil || p.LoginURL.String() == "" {
+	// Specific tennant specified, override the Login and RedeemURLs
-		p.LoginURL = &url.URL{
+	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"}
+			Path:   "/" + tenant + "/oauth2/" + path}
-	}
-	if p.RedeemURL == nil || p.RedeemURL.String() == "" {
-		p.RedeemURL = &url.URL{
-			Scheme: "https",
-			Host:   "login.microsoftonline.com",
-			Path:   "/" + p.Tenant + "/oauth2/token",
-		}
 	}
 }
 

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) {
+func TestNewAzureProvider(t *testing.T) {
-	p := testAzureProvider("")
+	g := NewWithT(t)
-	assert.NotEqual(t, nil, p)
+
-	p.Configure("")
+	// Test that defaults are set when calling for a new provider with nothing set
-	assert.Equal(t, "Azure", p.Data().ProviderName)
+	providerData := NewAzureProvider(&ProviderData{}).Data()
-	assert.Equal(t, "common", p.Tenant)
+	g.Expect(providerData.ProviderName).To(Equal("Azure"))
-	assert.Equal(t, "https://login.microsoftonline.com/common/oauth2/authorize",
+	g.Expect(providerData.LoginURL.String()).To(Equal("https://login.microsoftonline.com/common/oauth2/authorize"))
-		p.Data().LoginURL.String())
+	g.Expect(providerData.RedeemURL.String()).To(Equal("https://login.microsoftonline.com/common/oauth2/token"))
-	assert.Equal(t, "https://login.microsoftonline.com/common/oauth2/token",
+	g.Expect(providerData.ProfileURL.String()).To(Equal("https://graph.microsoft.com/v1.0/me"))
-		p.Data().RedeemURL.String())
+	g.Expect(providerData.ValidateURL.String()).To(Equal(""))
-	assert.Equal(t, "https://graph.microsoft.com/v1.0/me",
+	g.Expect(providerData.Scope).To(Equal("openid"))
-		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 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, "",
+	assert.Equal(t, "", p.Data().ValidateURL.String())
-		p.Data().ValidateURL.String())
 	assert.Equal(t, "openid", p.Data().Scope)
 }
 

providers/bitbucket.go

@@ -19,33 +19,49 @@ type BitbucketProvider struct {
 
 var _ Provider = (*BitbucketProvider)(nil)
 
-// NewBitbucketProvider initiates a new BitbucketProvider
+const (
-func NewBitbucketProvider(p *ProviderData) *BitbucketProvider {
+	bitbucketProviderName = "Bitbucket"
-	p.ProviderName = "Bitbucket"
+	bitbucketDefaultScope = "email"
-	if p.LoginURL == nil || p.LoginURL.String() == "" {
+)
-		p.LoginURL = &url.URL{
+
+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",
 	}
-	}
+
-	if p.RedeemURL == nil || p.RedeemURL.String() == "" {
+	// Default Redeem URL for Bitbucket.
-		p.RedeemURL = &url.URL{
+	// Pre-parsed URL of https://bitbucket.org/site/oauth2/access_token.
+	bitbucketDefaultRedeemURL = &url.URL{
 		Scheme: "https",
 		Host:   "bitbucket.org",
 		Path:   "/site/oauth2/access_token",
 	}
-	}
+
-	if p.ValidateURL == nil || p.ValidateURL.String() == "" {
+	// Default Validation URL for Bitbucket.
-		p.ValidateURL = &url.URL{
+	// 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",
 	}
-	}
+)
-	if p.Scope == "" {
+
-		p.Scope = "email"
+// NewBitbucketProvider initiates a new BitbucketProvider
-	}
+func NewBitbucketProvider(p *ProviderData) *BitbucketProvider {
+	p.setProviderDefaults(providerDefaults{
+		name:        bitbucketProviderName,
+		loginURL:    bitbucketDefaultLoginURL,
+		redeemURL:   bitbucketDefaultRedeemURL,
+		profileURL:  nil,
+		validateURL: bitbucketDefaultValidateURL,
+		scope:       bitbucketDefaultScope,
+	})
 	return &BitbucketProvider{ProviderData: p}
 }
 

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) {
+func TestNewBitbucketProvider(t *testing.T) {
-	p := testBitbucketProvider("", "", "")
+	g := NewWithT(t)
-	assert.NotEqual(t, nil, p)
+
-	assert.Equal(t, "Bitbucket", p.Data().ProviderName)
+	// Test that defaults are set when calling for a new provider with nothing set
-	assert.Equal(t, "https://bitbucket.org/site/oauth2/authorize",
+	providerData := NewBitbucketProvider(&ProviderData{}).Data()
-		p.Data().LoginURL.String())
+	g.Expect(providerData.ProviderName).To(Equal("Bitbucket"))
-	assert.Equal(t, "https://bitbucket.org/site/oauth2/access_token",
+	g.Expect(providerData.LoginURL.String()).To(Equal("https://bitbucket.org/site/oauth2/authorize"))
-		p.Data().RedeemURL.String())
+	g.Expect(providerData.RedeemURL.String()).To(Equal("https://bitbucket.org/site/oauth2/access_token"))
-	assert.Equal(t, "https://api.bitbucket.org/2.0/user/emails",
+	g.Expect(providerData.ProfileURL.String()).To(Equal(""))
-		p.Data().ValidateURL.String())
+	g.Expect(providerData.ValidateURL.String()).To(Equal("https://api.bitbucket.org/2.0/user/emails"))
-	assert.Equal(t, "email", p.Data().Scope)
+	g.Expect(providerData.Scope).To(Equal("email"))
 }
 
 func TestBitbucketProviderScopeAdjustForTeam(t *testing.T) {

providers/digitalocean.go

@@ -18,33 +18,47 @@ type DigitalOceanProvider struct {
 
 var _ Provider = (*DigitalOceanProvider)(nil)
 
-// NewDigitalOceanProvider initiates a new DigitalOceanProvider
+const (
-func NewDigitalOceanProvider(p *ProviderData) *DigitalOceanProvider {
+	digitalOceanProviderName = "DigitalOcean"
-	p.ProviderName = "DigitalOcean"
+	digitalOceanDefaultScope = "read"
-	if p.LoginURL.String() == "" {
+)
-		p.LoginURL = &url.URL{Scheme: "https",
+
+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",
 	}
-	}
+
-	if p.RedeemURL.String() == "" {
+	// Default Redeem URL for DigitalOcean.
-		p.RedeemURL = &url.URL{Scheme: "https",
+	// Pre-parsed URL of  https://cloud.digitalocean.com/v1/oauth/token.
+	digitalOceanDefaultRedeemURL = &url.URL{
+		Scheme: "https",
 		Host:   "cloud.digitalocean.com",
 		Path:   "/v1/oauth/token",
 	}
-	}
+
-	if p.ProfileURL.String() == "" {
+	// Default Profile URL for DigitalOcean.
-		p.ProfileURL = &url.URL{Scheme: "https",
+	// Pre-parsed URL of https://cloud.digitalocean.com/v2/account.
+	digitalOceanDefaultProfileURL = &url.URL{
+		Scheme: "https",
 		Host:   "api.digitalocean.com",
 		Path:   "/v2/account",
 	}
-	}
+)
-	if p.ValidateURL.String() == "" {
+
-		p.ValidateURL = p.ProfileURL
+// NewDigitalOceanProvider initiates a new DigitalOceanProvider
-	}
+func NewDigitalOceanProvider(p *ProviderData) *DigitalOceanProvider {
-	if p.Scope == "" {
+	p.setProviderDefaults(providerDefaults{
-		p.Scope = "read"
+		name:        digitalOceanProviderName,
-	}
+		loginURL:    digitalOceanDefaultLoginURL,
+		redeemURL:   digitalOceanDefaultRedeemURL,
+		profileURL:  digitalOceanDefaultProfileURL,
+		validateURL: digitalOceanDefaultProfileURL,
+		scope:       digitalOceanDefaultScope,
+	})
 	return &DigitalOceanProvider{ProviderData: p}
 }
 

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) {
+func TestNewDigitalOceanProvider(t *testing.T) {
-	p := testDigitalOceanProvider("")
+	g := NewWithT(t)
-	assert.NotEqual(t, nil, p)
+
-	assert.Equal(t, "DigitalOcean", p.Data().ProviderName)
+	// Test that defaults are set when calling for a new provider with nothing set
-	assert.Equal(t, "https://cloud.digitalocean.com/v1/oauth/authorize",
+	providerData := NewDigitalOceanProvider(&ProviderData{}).Data()
-		p.Data().LoginURL.String())
+	g.Expect(providerData.ProviderName).To(Equal("DigitalOcean"))
-	assert.Equal(t, "https://cloud.digitalocean.com/v1/oauth/token",
+	g.Expect(providerData.LoginURL.String()).To(Equal("https://cloud.digitalocean.com/v1/oauth/authorize"))
-		p.Data().RedeemURL.String())
+	g.Expect(providerData.RedeemURL.String()).To(Equal("https://cloud.digitalocean.com/v1/oauth/token"))
-	assert.Equal(t, "https://api.digitalocean.com/v2/account",
+	g.Expect(providerData.ProfileURL.String()).To(Equal("https://api.digitalocean.com/v2/account"))
-		p.Data().ProfileURL.String())
+	g.Expect(providerData.ValidateURL.String()).To(Equal("https://api.digitalocean.com/v2/account"))
-	assert.Equal(t, "https://api.digitalocean.com/v2/account",
+	g.Expect(providerData.Scope).To(Equal("read"))
-		p.Data().ValidateURL.String())
-	assert.Equal(t, "read", p.Data().Scope)
 }
 
 func TestDigitalOceanProviderOverrides(t *testing.T) {

providers/facebook.go

@@ -18,34 +18,48 @@ type FacebookProvider struct {
 
 var _ Provider = (*FacebookProvider)(nil)
 
-// NewFacebookProvider initiates a new FacebookProvider
+const (
-func NewFacebookProvider(p *ProviderData) *FacebookProvider {
+	facebookProviderName = "Facebook"
-	p.ProviderName = "Facebook"
+	facebookDefaultScope = "public_profile email"
-	if p.LoginURL.String() == "" {
+)
-		p.LoginURL = &url.URL{Scheme: "https",
+
+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
 	}
-	}
+
-	if p.RedeemURL.String() == "" {
+	// Default Redeem URL for Facebook.
-		p.RedeemURL = &url.URL{Scheme: "https",
+	// 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",
 	}
-	}
+
-	if p.ProfileURL.String() == "" {
+	// Default Profile URL for Facebook.
-		p.ProfileURL = &url.URL{Scheme: "https",
+	// Pre-parsed URL of https://graph.facebook.com/v2.5/me.
+	facebookDefaultProfileURL = &url.URL{
+		Scheme: "https",
 		Host:   "graph.facebook.com",
 		Path:   "/v2.5/me",
 	}
-	}
+)
-	if p.ValidateURL.String() == "" {
+
-		p.ValidateURL = p.ProfileURL
+// NewFacebookProvider initiates a new FacebookProvider
-	}
+func NewFacebookProvider(p *ProviderData) *FacebookProvider {
-	if p.Scope == "" {
+	p.setProviderDefaults(providerDefaults{
-		p.Scope = "public_profile email"
+		name:        facebookProviderName,
-	}
+		loginURL:    facebookDefaultLoginURL,
+		redeemURL:   facebookDefaultRedeemURL,
+		profileURL:  facebookDefaultProfileURL,
+		validateURL: facebookDefaultProfileURL,
+		scope:       facebookDefaultScope,
+	})
 	return &FacebookProvider{ProviderData: p}
 }
 

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"))
+}

providers/github.go

@@ -28,34 +28,49 @@ type GitHubProvider struct {
 
 var _ Provider = (*GitHubProvider)(nil)
 
-// NewGitHubProvider initiates a new GitHubProvider
+const (
-func NewGitHubProvider(p *ProviderData) *GitHubProvider {
+	githubProviderName = "GitHub"
-	p.ProviderName = "GitHub"
+	githubDefaultScope = "user:email"
-	if p.LoginURL == nil || p.LoginURL.String() == "" {
+)
-		p.LoginURL = &url.URL{
+
+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",
 	}
-	}
+
-	if p.RedeemURL == nil || p.RedeemURL.String() == "" {
+	// Default Redeem URL for GitHub.
-		p.RedeemURL = &url.URL{
+	// Pre-parsed URL of https://github.org/login/oauth/access_token.
+	githubDefaultRedeemURL = &url.URL{
 		Scheme: "https",
 		Host:   "github.com",
 		Path:   "/login/oauth/access_token",
 	}
-	}
+
-	// ValidationURL is the API Base URL
+	// Default Validation URL for GitHub.
-	if p.ValidateURL == nil || p.ValidateURL.String() == "" {
+	// ValidationURL is the API Base URL.
-		p.ValidateURL = &url.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:   "/",
 	}
-	}
+)
-	if p.Scope == "" {
+
-		p.Scope = "user:email"
+// NewGitHubProvider initiates a new GitHubProvider
-	}
+func NewGitHubProvider(p *ProviderData) *GitHubProvider {
+	p.setProviderDefaults(providerDefaults{
+		name:        githubProviderName,
+		loginURL:    githubDefaultLoginURL,
+		redeemURL:   githubDefaultRedeemURL,
+		profileURL:  nil,
+		validateURL: githubDefaultValidateURL,
+		scope:       githubDefaultScope,
+	})
 	return &GitHubProvider{ProviderData: p}
 }
 

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) {
+func TestNewGitHubProvider(t *testing.T) {
-	p := testGitHubProvider("")
+	g := NewWithT(t)
-	assert.NotEqual(t, nil, p)
+
-	assert.Equal(t, "GitHub", p.Data().ProviderName)
+	// Test that defaults are set when calling for a new provider with nothing set
-	assert.Equal(t, "https://github.com/login/oauth/authorize",
+	providerData := NewGitHubProvider(&ProviderData{}).Data()
-		p.Data().LoginURL.String())
+	g.Expect(providerData.ProviderName).To(Equal("GitHub"))
-	assert.Equal(t, "https://github.com/login/oauth/access_token",
+	g.Expect(providerData.LoginURL.String()).To(Equal("https://github.com/login/oauth/authorize"))
-		p.Data().RedeemURL.String())
+	g.Expect(providerData.RedeemURL.String()).To(Equal("https://github.com/login/oauth/access_token"))
-	assert.Equal(t, "https://api.github.com/",
+	g.Expect(providerData.ProfileURL.String()).To(Equal(""))
-		p.Data().ValidateURL.String())
+	g.Expect(providerData.ValidateURL.String()).To(Equal("https://api.github.com/"))
-	assert.Equal(t, "user:email", p.Data().Scope)
+	g.Expect(providerData.Scope).To(Equal("user:email"))
 }
 
 func TestGitHubProviderOverrides(t *testing.T) {

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}

providers/google.go

@@ -39,31 +39,49 @@ type claims struct {
 	EmailVerified bool   `json:"email_verified"`
 }
 
-// NewGoogleProvider initiates a new GoogleProvider
+const (
-func NewGoogleProvider(p *ProviderData) *GoogleProvider {
+	googleProviderName = "Google"
-	p.ProviderName = "Google"
+	googleDefaultScope = "profile email"
-	if p.LoginURL.String() == "" {
+)
-		p.LoginURL = &url.URL{Scheme: "https",
+
+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",
 	}
-	}
+
-	if p.RedeemURL.String() == "" {
+	// Default Redeem URL for Google.
-		p.RedeemURL = &url.URL{Scheme: "https",
+	// Pre-parsed URL of https://www.googleapis.com/oauth2/v3/token.
+	googleDefaultRedeemURL = &url.URL{
+		Scheme: "https",
 		Host:   "www.googleapis.com",
-			Path: "/oauth2/v3/token"}
+		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"
 	}
 
+	// 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

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) {
+func TestNewGoogleProvider(t *testing.T) {
-	p := newGoogleProvider()
+	g := NewWithT(t)
-	assert.NotEqual(t, nil, p)
+
-	assert.Equal(t, "Google", p.Data().ProviderName)
+	// Test that defaults are set when calling for a new provider with nothing set
-	assert.Equal(t, "https://accounts.google.com/o/oauth2/auth?access_type=offline",
+	providerData := NewGoogleProvider(&ProviderData{}).Data()
-		p.Data().LoginURL.String())
+	g.Expect(providerData.ProviderName).To(Equal("Google"))
-	assert.Equal(t, "https://www.googleapis.com/oauth2/v3/token",
+	g.Expect(providerData.LoginURL.String()).To(Equal("https://accounts.google.com/o/oauth2/auth?access_type=offline"))
-		p.Data().RedeemURL.String())
+	g.Expect(providerData.RedeemURL.String()).To(Equal("https://www.googleapis.com/oauth2/v3/token"))
-	assert.Equal(t, "https://www.googleapis.com/oauth2/v1/tokeninfo",
+	g.Expect(providerData.ProfileURL.String()).To(Equal(""))
-		p.Data().ValidateURL.String())
+	g.Expect(providerData.ValidateURL.String()).To(Equal("https://www.googleapis.com/oauth2/v1/tokeninfo"))
-	assert.Equal(t, "", p.Data().ProfileURL.String())
+	g.Expect(providerData.Scope).To(Equal("profile email"))
-	assert.Equal(t, "profile email", p.Data().Scope)
 }
 
 func TestGoogleProviderOverrides(t *testing.T) {

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
 }

providers/keycloak.go

@@ -16,32 +16,46 @@ type KeycloakProvider struct {
 
 var _ Provider = (*KeycloakProvider)(nil)
 
-func NewKeycloakProvider(p *ProviderData) *KeycloakProvider {
+const (
-	p.ProviderName = "Keycloak"
+	keycloakProviderName = "Keycloak"
-	if p.LoginURL == nil || p.LoginURL.String() == "" {
+	keycloakDefaultScope = "api"
-		p.LoginURL = &url.URL{
+)
+
+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",
 	}
-	}
+
-	if p.RedeemURL == nil || p.RedeemURL.String() == "" {
+	// Default Redeem URL for Keycloak.
-		p.RedeemURL = &url.URL{
+	// Pre-parsed URL of ttps://keycloak.org/oauth/token.
+	keycloakDefaultRedeemURL = &url.URL{
 		Scheme: "https",
 		Host:   "keycloak.org",
 		Path:   "/oauth/token",
 	}
-	}
+
-	if p.ValidateURL == nil || p.ValidateURL.String() == "" {
+	// Default Validation URL for Keycloak.
-		p.ValidateURL = &url.URL{
+	// Pre-parsed URL of https://keycloak.org/api/v3/user.
+	keycloakDefaultValidateURL = &url.URL{
 		Scheme: "https",
 		Host:   "keycloak.org",
 		Path:   "/api/v3/user",
 	}
-	}
+)
-	if p.Scope == "" {
+
-		p.Scope = "api"
+func NewKeycloakProvider(p *ProviderData) *KeycloakProvider {
-	}
+	p.setProviderDefaults(providerDefaults{
+		name:        keycloakProviderName,
+		loginURL:    keycloakDefaultLoginURL,
+		redeemURL:   keycloakDefaultRedeemURL,
+		profileURL:  nil,
+		validateURL: keycloakDefaultValidateURL,
+		scope:       keycloakDefaultScope,
+	})
 	return &KeycloakProvider{ProviderData: p}
 }
 

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{

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"
+	p.setProviderDefaults(providerDefaults{
-	if p.LoginURL.String() == "" {
+		name:        linkedinProviderName,
-		p.LoginURL = &url.URL{Scheme: "https",
+		loginURL:    linkedinDefaultLoginURL,
-			Host: "www.linkedin.com",
+		redeemURL:   linkedinDefaultRedeemURL,
-			Path: "/uas/oauth2/authorization"}
+		profileURL:  linkedinDefaultProfileURL,
-	}
+		validateURL: linkedinDefaultProfileURL,
-	if p.RedeemURL.String() == "" {
+		scope:       linkedinDefaultScope,
-		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"
-	}
 	return &LinkedInProvider{ProviderData: p}
 }
 

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) {
+func TestNewLinkedInProvider(t *testing.T) {
-	p := testLinkedInProvider("")
+	g := NewWithT(t)
-	assert.NotEqual(t, nil, p)
+
-	assert.Equal(t, "LinkedIn", p.Data().ProviderName)
+	// Test that defaults are set when calling for a new provider with nothing set
-	assert.Equal(t, "https://www.linkedin.com/uas/oauth2/authorization",
+	providerData := NewLinkedInProvider(&ProviderData{}).Data()
-		p.Data().LoginURL.String())
+	g.Expect(providerData.ProviderName).To(Equal("LinkedIn"))
-	assert.Equal(t, "https://www.linkedin.com/uas/oauth2/accessToken",
+	g.Expect(providerData.LoginURL.String()).To(Equal("https://www.linkedin.com/uas/oauth2/authorization"))
-		p.Data().RedeemURL.String())
+	g.Expect(providerData.RedeemURL.String()).To(Equal("https://www.linkedin.com/uas/oauth2/accessToken"))
-	assert.Equal(t, "https://www.linkedin.com/v1/people/~/email-address",
+	g.Expect(providerData.ProfileURL.String()).To(Equal("https://www.linkedin.com/v1/people/~/email-address"))
-		p.Data().ProfileURL.String())
+	g.Expect(providerData.ValidateURL.String()).To(Equal("https://www.linkedin.com/v1/people/~/email-address"))
-	assert.Equal(t, "https://www.linkedin.com/v1/people/~/email-address",
+	g.Expect(providerData.Scope).To(Equal("r_emailaddress r_basicprofile"))
-		p.Data().ValidateURL.String())
-	assert.Equal(t, "r_emailaddress r_basicprofile", p.Data().Scope)
 }
 
 func TestLinkedInProviderOverrides(t *testing.T) {

providers/logingov.go

@@ -43,35 +43,47 @@ func randSeq(n int) string {
 	return string(b)
 }
 
-// NewLoginGovProvider initiates a new LoginGovProvider
+const (
-func NewLoginGovProvider(p *ProviderData) *LoginGovProvider {
+	loginGovProviderName = "login.gov"
-	p.ProviderName = "login.gov"
+	loginGovDefaultScope = "email openid"
+)
 
-	if p.LoginURL == nil || p.LoginURL.String() == "" {
+var (
-		p.LoginURL = &url.URL{
+	// 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",
 	}
-	}
+
-	if p.RedeemURL == nil || p.RedeemURL.String() == "" {
+	// Default Redeem URL for LoginGov.
-		p.RedeemURL = &url.URL{
+	// 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",
 	}
-	}
+
-	if p.ProfileURL == nil || p.ProfileURL.String() == "" {
+	// Default Profile URL for LoginGov.
-		p.ProfileURL = &url.URL{
+	// 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",
 	}
-	}
+)
-	if p.Scope == "" {
-		p.Scope = "email openid"
-	}
 
+// NewLoginGovProvider initiates a new LoginGovProvider
+func NewLoginGovProvider(p *ProviderData) *LoginGovProvider {
+	p.setProviderDefaults(providerDefaults{
+		name:        loginGovProviderName,
+		loginURL:    loginGovDefaultLoginURL,
+		redeemURL:   loginGovDefaultRedeemURL,
+		profileURL:  loginGovDefaultProfileURL,
+		validateURL: nil,
+		scope:       loginGovDefaultScope,
+	})
 	return &LoginGovProvider{
 		ProviderData: p,
 		Nonce:        randSeq(32),

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) {
+func TestNewLoginGovProvider(t *testing.T) {
-	p, _, err := newLoginGovProvider()
+	g := NewWithT(t)
-	assert.NotEqual(t, nil, p)
+
-	assert.NoError(t, err)
+	// Test that defaults are set when calling for a new provider with nothing set
-	assert.Equal(t, "login.gov", p.Data().ProviderName)
+	providerData := NewLoginGovProvider(&ProviderData{}).Data()
-	assert.Equal(t, "https://secure.login.gov/openid_connect/authorize",
+	g.Expect(providerData.ProviderName).To(Equal("login.gov"))
-		p.Data().LoginURL.String())
+	g.Expect(providerData.LoginURL.String()).To(Equal("https://secure.login.gov/openid_connect/authorize"))
-	assert.Equal(t, "https://secure.login.gov/api/openid_connect/token",
+	g.Expect(providerData.RedeemURL.String()).To(Equal("https://secure.login.gov/api/openid_connect/token"))
-		p.Data().RedeemURL.String())
+	g.Expect(providerData.ProfileURL.String()).To(Equal("https://secure.login.gov/api/openid_connect/userinfo"))
-	assert.Equal(t, "https://secure.login.gov/api/openid_connect/userinfo",
+	g.Expect(providerData.ValidateURL.String()).To(Equal(""))
-		p.Data().ProfileURL.String())
+	g.Expect(providerData.Scope).To(Equal("email openid"))
-	assert.Equal(t, "email openid", p.Data().Scope)
 }
 
 func TestLoginGovProviderOverrides(t *testing.T) {

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}
 }
 

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{}
+}