diff --git a/Makefile b/Makefile index 328f1ab51..6f4160200 100644 --- a/Makefile +++ b/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 diff --git a/authenticate/authenticate.go b/authenticate/authenticate.go index bb8be6c83..a553a2753 100644 --- a/authenticate/authenticate.go +++ b/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, diff --git a/authenticate/authenticate_test.go b/authenticate/authenticate_test.go index a7166ad6b..3fb9fd4cb 100644 --- a/authenticate/authenticate_test.go +++ b/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) { diff --git a/cmd/pomerium/main_test.go b/cmd/pomerium/main_test.go index 68f2a653b..6c6f24333 100644 --- a/cmd/pomerium/main_test.go +++ b/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 diff --git a/internal/config/options.go b/internal/config/options.go index 4162ae154..2c252035c 100644 --- a/internal/config/options.go +++ b/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") } } diff --git a/internal/config/options_test.go b/internal/config/options_test.go index 69f3e132e..318d58686 100644 --- a/internal/config/options_test.go +++ b/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) } diff --git a/internal/identity/providers.go b/internal/identity/providers.go index 394e69566..0e18a621d 100644 --- a/internal/identity/providers.go +++ b/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 diff --git a/proxy/proxy.go b/proxy/proxy.go index 683136126..857d84727 100755 --- a/proxy/proxy.go +++ b/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, diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index c8635d0b9..7eb3cc384 100644 --- a/proxy/proxy_test.go +++ b/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)