internal/config: pass urls by value

Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>

Makefile

@@ -30,7 +30,7 @@ GOOSARCHES = linux/amd64 darwin/amd64 windows/amd64
 
 
 .PHONY: all
-all: clean build lint spellcheck test ## Runs a clean, build, fmt, lint, test, and vet.
+all: clean lint spellcheck test build ## Runs a clean, build, fmt, lint, test, and vet.
 
 .PHONY: tag
 tag: ## Create a new git tag to prepare to build a release

authenticate/authenticate.go

@@ -18,7 +18,7 @@ import (
 // The checks do not modify the internal state of the Option structure. Returns
 // on first error found.
 func ValidateOptions(o config.Options) error {
-	if o.AuthenticateURL == nil {
+	if o.AuthenticateURL.String() == "" {
 		return errors.New("authenticate: 'AUTHENTICATE_SERVICE_URL' missing")
 	}
 	if o.ClientID == "" {
@@ -97,7 +97,7 @@ func New(opts config.Options) (*Authenticate, error) {
 	}
 	return &Authenticate{
 		SharedKey:    opts.SharedKey,
-		RedirectURL:  redirectURL,
+		RedirectURL:  &redirectURL,
 		templates:    templates.New(),
 		csrfStore:    cookieStore,
 		sessionStore: cookieStore,

authenticate/authenticate_test.go

@@ -1,6 +1,7 @@
 package authenticate
 
 import (
+	"net/url"
 	"testing"
 
 	"github.com/pomerium/pomerium/internal/config"
@@ -21,7 +22,7 @@ func newTestOptions(t *testing.T) *config.Options {
 func TestOptions_Validate(t *testing.T) {
 	good := newTestOptions(t)
 	badRedirectURL := newTestOptions(t)
-	badRedirectURL.AuthenticateURL = nil
+	badRedirectURL.AuthenticateURL = url.URL{}
 	emptyClientID := newTestOptions(t)
 	emptyClientID.ClientID = ""
 	emptyClientSecret := newTestOptions(t)
@@ -35,7 +36,7 @@ func TestOptions_Validate(t *testing.T) {
 	badSharedKey := newTestOptions(t)
 	badSharedKey.SharedKey = ""
 	badAuthenticateURL := newTestOptions(t)
-	badAuthenticateURL.AuthenticateURL = nil
+	badAuthenticateURL.AuthenticateURL = url.URL{}
 
 	tests := []struct {
 		name    string
@@ -66,7 +67,13 @@ func TestNew(t *testing.T) {
 	good := newTestOptions(t)
 
 	badRedirectURL := newTestOptions(t)
-	badRedirectURL.AuthenticateURL = nil
+	badRedirectURL.AuthenticateURL = url.URL{}
+
+	badCookieName := newTestOptions(t)
+	badCookieName.CookieName = ""
+
+	badProvider := newTestOptions(t)
+	badProvider.Provider = ""
 
 	tests := []struct {
 		name string
@@ -77,6 +84,8 @@ func TestNew(t *testing.T) {
 		{"good", good, false},
 		{"empty opts", &config.Options{}, true},
 		{"fails to validate", badRedirectURL, true},
+		{"bad cookie name", badCookieName, true},
+		{"bad provider", badProvider, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

cmd/pomerium/main_test.go

@@ -146,8 +146,8 @@ func Test_newProxyeService(t *testing.T) {
 			AuthenticateURL, _ := url.Parse("https://authenticate.example.com")
 			AuthorizeURL, _ := url.Parse("https://authorize.example.com")
 
-			testOpts.AuthenticateURL = AuthenticateURL
-			testOpts.AuthorizeURL = AuthorizeURL
+			testOpts.AuthenticateURL = *AuthenticateURL
+			testOpts.AuthorizeURL = *AuthorizeURL
 			testOpts.CookieSecret = "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM="
 			testOpts.Services = tt.s
 

internal/config/options.go

@@ -73,7 +73,7 @@ type Options struct {
 	// AuthenticateURL represents the externally accessible http endpoints
 	// used for authentication requests and callbacks
 	AuthenticateURLString string `mapstructure:"authenticate_service_url"`
-	AuthenticateURL       *url.URL
+	AuthenticateURL       url.URL
 
 	// Session/Cookie management
 	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie
@@ -103,13 +103,13 @@ type Options struct {
 	// NOTE: As many load balancers do not support externally routed gRPC so
 	// this may be an internal location.
 	AuthenticateInternalAddrString string `mapstructure:"authenticate_internal_url"`
-	AuthenticateInternalAddr       *url.URL
+	AuthenticateInternalAddr       url.URL
 
 	// AuthorizeURL is the routable destination of the authorize service's
 	// gRPC endpoint. NOTE: As many load balancers do not support
 	// externally routed gRPC so this may be an internal location.
 	AuthorizeURLString string `mapstructure:"authorize_service_url"`
-	AuthorizeURL       *url.URL
+	AuthorizeURL       url.URL
 
 	// Settings to enable custom behind-the-ingress service communication
 	OverrideCertificateName string `mapstructure:"override_certificate_name"`
@@ -230,7 +230,7 @@ func (o *Options) Validate() error {
 		if err != nil {
 			return fmt.Errorf("bad authenticate-url %s : %v", o.AuthenticateURLString, err)
 		}
-		o.AuthenticateURL = u
+		o.AuthenticateURL = *u
 	}
 
 	if o.AuthorizeURLString != "" {
@@ -238,7 +238,7 @@ func (o *Options) Validate() error {
 		if err != nil {
 			return fmt.Errorf("bad authorize-url %s : %v", o.AuthorizeURLString, err)
 		}
-		o.AuthorizeURL = u
+		o.AuthorizeURL = *u
 	}
 
 	if o.AuthenticateInternalAddrString != "" {
@@ -246,7 +246,7 @@ func (o *Options) Validate() error {
 		if err != nil {
 			return fmt.Errorf("bad authenticate-internal-addr %s : %v", o.AuthenticateInternalAddrString, err)
 		}
-		o.AuthenticateInternalAddr = u
+		o.AuthenticateInternalAddr = *u
 	}
 	if o.PolicyFile != "" {
 		return errors.New("policy file setting is deprecated")
@@ -373,7 +373,7 @@ func ParseOptions(configFile string) (*Options, error) {
 
 	checksumDec, err := strconv.ParseUint(o.Checksum(), 16, 64)
 	if err != nil {
-		log.Warn().Err(err).Msg("Could not parse config checksum into decimal")
+		log.Warn().Err(err).Msg("internal/config: could not parse config checksum into decimal")
 	}
 	metrics.SetConfigChecksum(o.Services, checksumDec)
 
@@ -383,26 +383,22 @@ func ParseOptions(configFile string) (*Options, error) {
 func HandleConfigUpdate(configFile string, opt *Options, services []OptionsUpdater) *Options {
 	newOpt, err := ParseOptions(configFile)
 	if err != nil {
-		log.Error().Err(err).Msg("cmd/pomerium: could not reload configuration")
+		log.Error().Err(err).Msg("config: could not reload configuration")
 		return opt
 	}
 	optChecksum := opt.Checksum()
 	newOptChecksum := newOpt.Checksum()
 
-	log.Debug().
-		Str("old-checksum", optChecksum).
-		Str("new-checksum", newOptChecksum).
-		Msg("cmd/pomerium: configuration file changed")
+	log.Debug().Str("old-checksum", optChecksum).Str("new-checksum", newOptChecksum).Msg("internal/config: checksum change")
 
 	if newOptChecksum == optChecksum {
-		log.Debug().Msg("cmd/pomerium: loaded configuration has not changed")
+		log.Debug().Msg("internal/config: loaded configuration has not changed")
 		return opt
 	}
 
-	log.Info().Str("checksum", newOptChecksum).Msg("cmd/pomerium: checksum changed")
 	for _, service := range services {
 		if err := service.UpdateOptions(*newOpt); err != nil {
-			log.Error().Err(err).Msg("cmd/pomerium: could not update options")
+			log.Error().Err(err).Msg("internal/config: could not update options")
 		}
 	}
 

internal/config/options_test.go

@@ -150,8 +150,8 @@ func Test_OptionsFromViper(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	goodOptions.AuthorizeURL = authorize
-	goodOptions.AuthenticateURL = authenticate
+	goodOptions.AuthorizeURL = *authorize
+	goodOptions.AuthenticateURL = *authenticate
 	if err := goodOptions.Validate(); err != nil {
 		t.Fatal(err)
 	}

internal/identity/providers.go

@@ -84,7 +84,7 @@ func New(providerName string, p *Provider) (a Authenticator, err error) {
 type Provider struct {
 	ProviderName string
 
-	RedirectURL  *url.URL
+	RedirectURL  url.URL
 	ClientID     string
 	ClientSecret string
 	ProviderURL  string

proxy/proxy.go

@@ -47,13 +47,13 @@ func ValidateOptions(o config.Options) error {
 	if len(decoded) != 32 {
 		return fmt.Errorf("`SHARED_SECRET` want 32 but got %d bytes", len(decoded))
 	}
-	if o.AuthenticateURL == nil || o.AuthenticateURL.String() == "" {
+	if o.AuthenticateURL.String() == "" {
 		return errors.New("missing setting: authenticate-service-url")
 	}
 	if o.AuthenticateURL.Scheme != "https" {
 		return errors.New("authenticate-service-url must be a valid https url")
 	}
-	if o.AuthorizeURL == nil || o.AuthorizeURL.String() == "" {
+	if o.AuthorizeURL.String() == "" {
 		return errors.New("missing setting: authorize-service-url")
 	}
 	if o.AuthorizeURL.Scheme != "https" {
@@ -143,7 +143,7 @@ func New(opts config.Options) (*Proxy, error) {
 
 		routeConfigs: make(map[string]*routeConfig),
 		// services
-		AuthenticateURL: opts.AuthenticateURL,
+		AuthenticateURL: &opts.AuthenticateURL,
 
 		cipher:                 cipher,
 		cookieName:             opts.CookieName,
@@ -165,8 +165,8 @@ func New(opts config.Options) (*Proxy, error) {
 	})
 	p.AuthenticateClient, err = clients.NewAuthenticateClient("grpc",
 		&clients.Options{
-			Addr:                    opts.AuthenticateURL,
-			InternalAddr:            opts.AuthenticateInternalAddr,
+			Addr:                    &opts.AuthenticateURL,
+			InternalAddr:            &opts.AuthenticateInternalAddr,
 			OverrideCertificateName: opts.OverrideCertificateName,
 			SharedSecret:            opts.SharedKey,
 			CA:                      opts.CA,
@@ -177,7 +177,7 @@ func New(opts config.Options) (*Proxy, error) {
 	}
 	p.AuthorizeClient, err = clients.NewAuthorizeClient("grpc",
 		&clients.Options{
-			Addr:                    opts.AuthorizeURL,
+			Addr:                    &opts.AuthorizeURL,
 			OverrideCertificateName: opts.OverrideCertificateName,
 			SharedSecret:            opts.SharedKey,
 			CA:                      opts.CA,

proxy/proxy_test.go

@@ -96,8 +96,8 @@ func testOptions(t *testing.T) config.Options {
 	opts := newTestOptions(t)
 	testPolicy := config.Policy{From: "https://corp.example.example", To: "https://example.example"}
 	opts.Policies = []config.Policy{testPolicy}
-	opts.AuthenticateURL = authenticateService
-	opts.AuthorizeURL = authorizeService
+	opts.AuthenticateURL = *authenticateService
+	opts.AuthorizeURL = *authorizeService
 	opts.SharedKey = "80ldlrU2d7w+wVpKNfevk6fmb8otEx6CqOfshj2LwhQ="
 	opts.CookieSecret = "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw="
 	opts.CookieName = "pomerium"
@@ -120,8 +120,8 @@ func testOptionsTestServer(t *testing.T, uri string) config.Options {
 	}
 	opts := newTestOptions(t)
 	opts.Policies = []config.Policy{testPolicy}
-	opts.AuthenticateURL = authenticateService
-	opts.AuthorizeURL = authorizeService
+	opts.AuthenticateURL = *authenticateService
+	opts.AuthorizeURL = *authorizeService
 	opts.SharedKey = "80ldlrU2d7w+wVpKNfevk6fmb8otEx6CqOfshj2LwhQ="
 	opts.CookieSecret = "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw="
 	opts.CookieName = "pomerium"
@@ -165,14 +165,14 @@ func testOptionsWithEmptyPolicies(t *testing.T, uri string) config.Options {
 func TestOptions_Validate(t *testing.T) {
 	good := testOptions(t)
 	badAuthURL := testOptions(t)
-	badAuthURL.AuthenticateURL = nil
+	badAuthURL.AuthenticateURL = url.URL{}
 	authurl, _ := url.Parse("http://authenticate.corp.beyondperimeter.com")
 	authenticateBadScheme := testOptions(t)
-	authenticateBadScheme.AuthenticateURL = authurl
+	authenticateBadScheme.AuthenticateURL = *authurl
 	authorizeBadSCheme := testOptions(t)
-	authorizeBadSCheme.AuthorizeURL = authurl
+	authorizeBadSCheme.AuthorizeURL = *authurl
 	authorizeNil := testOptions(t)
-	authorizeNil.AuthorizeURL = nil
+	authorizeNil.AuthorizeURL = url.URL{}
 	emptyCookieSecret := testOptions(t)
 	emptyCookieSecret.CookieSecret = ""
 	invalidCookieSecret := testOptions(t)