diff --git a/authenticate/authenticate.go b/authenticate/authenticate.go index 662ee39f3..cc9ea31dc 100644 --- a/authenticate/authenticate.go +++ b/authenticate/authenticate.go @@ -18,8 +18,8 @@ import ( // ValidateOptions checks to see if configuration values are valid for the authenticate service. // 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 || o.AuthenticateURL.Hostname() == "" { +func ValidateOptions(o config.Options) error { + if o.AuthenticateURL.Hostname() == "" { return errors.New("authenticate: 'AUTHENTICATE_SERVICE_URL' missing") } if o.ClientID == "" { @@ -54,10 +54,7 @@ type Authenticate struct { } // New validates and creates a new authenticate service from a set of Options -func New(opts *config.Options) (*Authenticate, error) { - if opts == nil { - return nil, errors.New("authenticate: options cannot be nil") - } +func New(opts config.Options) (*Authenticate, error) { if err := ValidateOptions(opts); err != nil { return nil, err } @@ -83,7 +80,7 @@ func New(opts *config.Options) (*Authenticate, error) { provider, err := identity.New( opts.Provider, &identity.Provider{ - RedirectURL: redirectURL, + RedirectURL: &redirectURL, ProviderName: opts.Provider, ProviderURL: opts.ProviderURL, ClientID: opts.ClientID, @@ -97,7 +94,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 da81e3cd9..6bc0ea182 100644 --- a/authenticate/authenticate_test.go +++ b/authenticate/authenticate_test.go @@ -8,10 +8,10 @@ import ( "github.com/pomerium/pomerium/internal/config" ) -func testOptions() *config.Options { +func testOptions() config.Options { redirectURL, _ := url.Parse("https://example.com/oauth2/callback") - return &config.Options{ - AuthenticateURL: redirectURL, + return config.Options{ + AuthenticateURL: *redirectURL, SharedKey: "80ldlrU2d7w+wVpKNfevk6fmb8otEx6CqOfshj2LwhQ=", ClientID: "test-client-id", ClientSecret: "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw=", @@ -25,7 +25,7 @@ func testOptions() *config.Options { func TestOptions_Validate(t *testing.T) { good := testOptions() badRedirectURL := testOptions() - badRedirectURL.AuthenticateURL = nil + badRedirectURL.AuthenticateURL = url.URL{} emptyClientID := testOptions() emptyClientID.ClientID = "" emptyClientSecret := testOptions() @@ -39,15 +39,15 @@ func TestOptions_Validate(t *testing.T) { badSharedKey := testOptions() badSharedKey.SharedKey = "" badAuthenticateURL := testOptions() - badAuthenticateURL.AuthenticateURL = new(url.URL) + badAuthenticateURL.AuthenticateURL = url.URL{} tests := []struct { name string - o *config.Options + o config.Options wantErr bool }{ {"minimum options", good, false}, - {"nil options", &config.Options{}, true}, + {"nil options", config.Options{}, true}, {"bad redirect url", badRedirectURL, true}, {"no cookie secret", emptyCookieSecret, true}, {"invalid cookie secret", invalidCookieSecret, true}, @@ -72,16 +72,16 @@ func TestNew(t *testing.T) { good.Provider = "google" badRedirectURL := testOptions() - badRedirectURL.AuthenticateURL = nil + badRedirectURL.AuthenticateURL = url.URL{} tests := []struct { name string - opts *config.Options + opts config.Options // want *Authenticate wantErr bool }{ {"good", good, false}, - {"empty opts", nil, true}, + {"empty opts", config.Options{}, true}, {"fails to validate", badRedirectURL, true}, } for _, tt := range tests { diff --git a/authorize/authorize.go b/authorize/authorize.go index f276458e5..1fbff172c 100644 --- a/authorize/authorize.go +++ b/authorize/authorize.go @@ -14,7 +14,7 @@ import ( // ValidateOptions checks to see if configuration values are valid for the // authorize service. Returns first error, if found. -func ValidateOptions(o *config.Options) error { +func ValidateOptions(o config.Options) error { decoded, err := base64.StdEncoding.DecodeString(o.SharedKey) if err != nil { return fmt.Errorf("authorize: `SHARED_SECRET` setting is invalid base64: %v", err) @@ -39,10 +39,7 @@ type Authorize struct { } // New validates and creates a new Authorize service from a set of Options -func New(opts *config.Options) (*Authorize, error) { - if opts == nil { - return nil, errors.New("authorize: options cannot be nil") - } +func New(opts config.Options) (*Authorize, error) { if err := ValidateOptions(opts); err != nil { return nil, err } @@ -67,7 +64,7 @@ func (a *Authorize) ValidIdentity(route string, identity *Identity) bool { } // UpdateOptions updates internal structres based on config.Options -func (a *Authorize) UpdateOptions(o *config.Options) error { +func (a *Authorize) UpdateOptions(o config.Options) error { log.Info().Msg("authorize: updating options") a.identityAccess = NewIdentityWhitelist(o.Policies, o.Administrators) return nil diff --git a/authorize/authorize_test.go b/authorize/authorize_test.go index 71be228e5..decddae92 100644 --- a/authorize/authorize_test.go +++ b/authorize/authorize_test.go @@ -22,14 +22,14 @@ func TestNew(t *testing.T) { {"bad shared secret", "AZA85podM73CjLCjViDNz1EUvvejKpWp7Hysr0knXA==", policies, true}, {"really bad shared secret", "sup", policies, true}, {"validation error, short secret", "AZA85podM73CjLCjViDNz1EUvvejKpWp7Hysr0knXA==", policies, true}, - {"nil options", "", []policy.Policy{}, true}, // special case + {"empty options", "", []policy.Policy{}, true}, // special case {"missing policies", "gXK6ggrlIW2HyKyUF9rUO4azrDgxhDPWqw9y+lJU7B8=", []policy.Policy{}, true}, // special case } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - o := &config.Options{SharedKey: tt.SharedKey, Policies: tt.Policies} - if tt.name == "nil options" { - o = nil + o := config.Options{SharedKey: tt.SharedKey, Policies: tt.Policies} + if tt.name == "empty options" { + o = config.Options{} } _, err := New(o) if (err != nil) != tt.wantErr { @@ -76,7 +76,7 @@ func Test_UpdateOptions(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - o := &config.Options{SharedKey: tt.SharedKey, Policies: tt.Policies} + o := config.Options{SharedKey: tt.SharedKey, Policies: tt.Policies} authorize, _ := New(o) o.Policies = tt.newPolices authorize.UpdateOptions(o) diff --git a/cmd/pomerium/main.go b/cmd/pomerium/main.go index ce7a45dd8..c56816ac1 100644 --- a/cmd/pomerium/main.go +++ b/cmd/pomerium/main.go @@ -114,8 +114,8 @@ func startRedirectServer(addr string) (*http.Server, error) { return srv, nil } -func newAuthenticateService(opt *config.Options, mux *http.ServeMux, rpc *grpc.Server) (*authenticate.Authenticate, error) { - if opt == nil || !config.IsAuthenticate(opt.Services) { +func newAuthenticateService(opt config.Options, mux *http.ServeMux, rpc *grpc.Server) (*authenticate.Authenticate, error) { + if !config.IsAuthenticate(opt.Services) { return nil, nil } service, err := authenticate.New(opt) @@ -127,8 +127,8 @@ func newAuthenticateService(opt *config.Options, mux *http.ServeMux, rpc *grpc.S return service, nil } -func newAuthorizeService(opt *config.Options, rpc *grpc.Server) (*authorize.Authorize, error) { - if opt == nil || !config.IsAuthorize(opt.Services) { +func newAuthorizeService(opt config.Options, rpc *grpc.Server) (*authorize.Authorize, error) { + if !config.IsAuthorize(opt.Services) { return nil, nil } service, err := authorize.New(opt) @@ -139,8 +139,8 @@ func newAuthorizeService(opt *config.Options, rpc *grpc.Server) (*authorize.Auth return service, nil } -func newProxyService(opt *config.Options, mux *http.ServeMux) (*proxy.Proxy, error) { - if opt == nil || !config.IsProxy(opt.Services) { +func newProxyService(opt config.Options, mux *http.ServeMux) (*proxy.Proxy, error) { + if !config.IsProxy(opt.Services) { return nil, nil } service, err := proxy.New(opt) @@ -151,7 +151,7 @@ func newProxyService(opt *config.Options, mux *http.ServeMux) (*proxy.Proxy, err return service, nil } -func wrapMiddleware(o *config.Options, mux *http.ServeMux) http.Handler { +func wrapMiddleware(o config.Options, mux *http.ServeMux) http.Handler { c := middleware.NewChain() c = c.Append(log.NewHandler(log.Logger)) c = c.Append(log.AccessHandler(func(r *http.Request, status, size int, duration time.Duration) { @@ -166,7 +166,7 @@ func wrapMiddleware(o *config.Options, mux *http.ServeMux) http.Handler { Str("url", r.URL.String()). Msg("http-request") })) - if o != nil && len(o.Headers) != 0 { + if len(o.Headers) != 0 { c = c.Append(middleware.SetHeaders(o.Headers)) } c = c.Append(log.ForwardedAddrHandler("fwd_ip")) @@ -178,10 +178,10 @@ func wrapMiddleware(o *config.Options, mux *http.ServeMux) http.Handler { return c.Then(mux) } -func parseOptions(configFile string) (*config.Options, error) { +func parseOptions(configFile string) (config.Options, error) { o, err := config.OptionsFromViper(configFile) if err != nil { - return nil, err + return o, err } if o.Debug { log.SetDebugMode() @@ -193,7 +193,7 @@ func parseOptions(configFile string) (*config.Options, error) { return o, nil } -func handleConfigUpdate(opt *config.Options, services []config.OptionsUpdater) *config.Options { +func handleConfigUpdate(opt config.Options, services []config.OptionsUpdater) config.Options { newOpt, err := parseOptions(*configFile) optChecksum := opt.Checksum() newOptChecksum := newOpt.Checksum() diff --git a/cmd/pomerium/main_test.go b/cmd/pomerium/main_test.go index 6aa27fe15..b3212a629 100644 --- a/cmd/pomerium/main_test.go +++ b/cmd/pomerium/main_test.go @@ -78,11 +78,11 @@ func Test_newAuthenticateService(t *testing.T) { testOpts.ClientSecret = "TEST" testOpts.SharedKey = "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=" testOpts.CookieSecret = "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=" - testOpts.AuthenticateURL = authURL + testOpts.AuthenticateURL = *authURL testOpts.Services = tt.s if tt.Field != "" { - testOptsField := reflect.ValueOf(testOpts).Elem().FieldByName(tt.Field) + testOptsField := reflect.ValueOf(&testOpts).Elem().FieldByName(tt.Field) testOptsField.Set(reflect.ValueOf(tt).FieldByName("Value")) } @@ -126,7 +126,7 @@ func Test_newAuthorizeService(t *testing.T) { } if tt.Field != "" { - testOptsField := reflect.ValueOf(testOpts).Elem().FieldByName(tt.Field) + testOptsField := reflect.ValueOf(&testOpts).Elem().FieldByName(tt.Field) testOptsField.Set(reflect.ValueOf(tt).FieldByName("Value")) } @@ -162,13 +162,17 @@ func Test_newProxyeService(t *testing.T) { testOpts.Policies = []policy.Policy{ testPolicy, } - testOpts.AuthenticateURL, _ = url.Parse("https://authenticate.example.com") - testOpts.AuthorizeURL, _ = url.Parse("https://authorize.example.com") + + AuthenticateURL, _ := url.Parse("https://authenticate.example.com") + AuthorizeURL, _ := url.Parse("https://authorize.example.com") + + testOpts.AuthenticateURL = *AuthenticateURL + testOpts.AuthorizeURL = *AuthorizeURL testOpts.CookieSecret = "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=" testOpts.Services = tt.s if tt.Field != "" { - testOptsField := reflect.ValueOf(testOpts).Elem().FieldByName(tt.Field) + testOptsField := reflect.ValueOf(&testOpts).Elem().FieldByName(tt.Field) testOptsField.Set(reflect.ValueOf(tt).FieldByName("Value")) } _, err := newProxyService(testOpts, mux) @@ -181,7 +185,7 @@ func Test_newProxyeService(t *testing.T) { } func Test_wrapMiddleware(t *testing.T) { - o := &config.Options{ + o := config.Options{ Services: "all", Headers: map[string]string{ "X-Content-Type-Options": "nosniff", @@ -232,7 +236,7 @@ func Test_parseOptions(t *testing.T) { t.Errorf("parseOptions() error = %v, wantErr %v", err, tt.wantErr) return } - if got != nil && got.SharedKey != tt.wantSharedKey { + if got.SharedKey != tt.wantSharedKey { t.Errorf("parseOptions()\n") t.Errorf("got: %+v\n", got.SharedKey) t.Errorf("want: %+v\n", tt.wantSharedKey) @@ -247,7 +251,7 @@ type mockService struct { Updated bool } -func (m *mockService) UpdateOptions(o *config.Options) error { +func (m *mockService) UpdateOptions(o config.Options) error { m.Updated = true if m.fail { @@ -266,7 +270,7 @@ func Test_handleConfigUpdate(t *testing.T) { tests := []struct { name string service *mockService - oldOpts *config.Options + oldOpts config.Options wantUpdate bool }{ {"good", &mockService{fail: false}, blankOpts, true}, diff --git a/internal/config/options.go b/internal/config/options.go index ad62fbd5e..923671933 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -70,8 +70,8 @@ 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 `hash:"ignore"` + AuthenticateURLString string `mapstructure:"authenticate_service_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"` @@ -136,8 +136,8 @@ type Options struct { } // NewOptions returns a new options struct with default values -func NewOptions() *Options { - o := &Options{ +func NewOptions() Options { + o := Options{ Debug: false, LogLevel: "debug", Services: "all", @@ -153,25 +153,22 @@ func NewOptions() *Options { "X-XSS-Protection": "1; mode=block", "Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload", }, - Addr: ":https", - CertFile: filepath.Join(findPwd(), "cert.pem"), - KeyFile: filepath.Join(findPwd(), "privkey.pem"), - ReadHeaderTimeout: 10 * time.Second, - ReadTimeout: 30 * time.Second, - WriteTimeout: 0, // support streaming by default - IdleTimeout: 5 * time.Minute, - AuthenticateURL: new(url.URL), - AuthenticateInternalAddr: new(url.URL), - AuthorizeURL: new(url.URL), - RefreshCooldown: time.Duration(5 * time.Minute), - AllowWebsockets: false, + Addr: ":https", + CertFile: filepath.Join(findPwd(), "cert.pem"), + KeyFile: filepath.Join(findPwd(), "privkey.pem"), + ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, + WriteTimeout: 0, // support streaming by default + IdleTimeout: 5 * time.Minute, + RefreshCooldown: time.Duration(5 * time.Minute), + AllowWebsockets: false, } return o } // OptionsFromViper builds the main binary's configuration // options by parsing environmental variables and config file -func OptionsFromViper(configFile string) (*Options, error) { +func OptionsFromViper(configFile string) (Options, error) { o := NewOptions() // Load up config @@ -184,25 +181,25 @@ func OptionsFromViper(configFile string) (*Options, error) { viper.SetConfigFile(configFile) err := viper.ReadInConfig() if err != nil { - return nil, fmt.Errorf("failed to read config: %s", err) + return o, fmt.Errorf("failed to read config: %s", err) } } - err := viper.Unmarshal(o) + err := viper.Unmarshal(&o) if err != nil { - return nil, fmt.Errorf("failed to load options from config: %s", err) + return o, fmt.Errorf("failed to load options from config: %s", err) } // Turn URL strings into url structs err = o.parseURLs() if err != nil { - return nil, fmt.Errorf("failed to parse URLs: %s", err) + return o, fmt.Errorf("failed to parse URLs: %s", err) } // Load and initialize policy err = o.parsePolicy() if err != nil { - return nil, fmt.Errorf("failed to parse Policy: %s", err) + return o, fmt.Errorf("failed to parse Policy: %s", err) } if o.Debug { @@ -217,7 +214,7 @@ func OptionsFromViper(configFile string) (*Options, error) { err = o.validate() if err != nil { - return nil, err + return o, err } log.Debug(). @@ -302,7 +299,7 @@ func (o *Options) parseURLs() error { if err != nil { return fmt.Errorf("internal/config: bad authenticate-url %s : %v", o.AuthenticateURLString, err) } - o.AuthenticateURL = AuthenticateURL + o.AuthenticateURL = *AuthenticateURL } if o.AuthorizeURLString != "" { @@ -310,7 +307,7 @@ func (o *Options) parseURLs() error { if err != nil { return fmt.Errorf("internal/config: bad authorize-url %s : %v", o.AuthorizeURLString, err) } - o.AuthorizeURL = AuthorizeURL + o.AuthorizeURL = *AuthorizeURL } if o.AuthenticateInternalAddrString != "" { @@ -318,7 +315,7 @@ func (o *Options) parseURLs() error { if err != nil { return fmt.Errorf("internal/config: bad authenticate-internal-addr %s : %v", o.AuthenticateInternalAddrString, err) } - o.AuthenticateInternalAddr = AuthenticateInternalAddr + o.AuthenticateInternalAddr = *AuthenticateInternalAddr } return nil @@ -396,7 +393,7 @@ func IsProxy(s string) bool { // OptionsUpdater updates local state based on an Options struct type OptionsUpdater interface { - UpdateOptions(*Options) error + UpdateOptions(Options) error } // Checksum returns the checksum of the current options struct diff --git a/internal/config/options_test.go b/internal/config/options_test.go index a1623b80d..0adb30fab 100644 --- a/internal/config/options_test.go +++ b/internal/config/options_test.go @@ -16,7 +16,7 @@ import ( func Test_validate(t *testing.T) { - testOptions := func() *Options { + testOptions := func() Options { o := NewOptions() o.SharedKey = "test" o.Services = "all" @@ -34,7 +34,7 @@ func Test_validate(t *testing.T) { tests := []struct { name string - testOpts *Options + testOpts Options wantErr bool }{ {"good default with no env settings", good, false}, @@ -194,10 +194,10 @@ func Test_parseURLs(t *testing.T) { if (err != nil) != test.wantErr { t.Errorf("Failed to parse URLs %v: %s", test, err) } - if o.AuthenticateURL != nil && o.AuthenticateURL.String() != test.authenticateURL { + if err == nil && o.AuthenticateURL.String() != test.authenticateURL { t.Errorf("Failed to update AuthenticateURL: %v", test) } - if o.AuthorizeURL != nil && o.AuthorizeURL.String() != test.authorizeURL { + if err == nil && o.AuthorizeURL.String() != test.authorizeURL { t.Errorf("Failed to update AuthorizeURL: %v", test) } } @@ -230,8 +230,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 badConfigBytes := []byte("badjson!") badUnmarshalConfigBytes := []byte(`"debug": "blue"`) @@ -239,12 +239,12 @@ func Test_OptionsFromViper(t *testing.T) { tests := []struct { name string configBytes []byte - want *Options + want Options wantErr bool }{ {"good", goodConfigBytes, goodOptions, false}, - {"bad json", badConfigBytes, nil, true}, - {"bad unmarshal", badUnmarshalConfigBytes, nil, true}, + {"bad json", badConfigBytes, NewOptions(), true}, + {"bad unmarshal", badUnmarshalConfigBytes, NewOptions(), true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -268,8 +268,8 @@ func Test_OptionsFromViper(t *testing.T) { } // Test for missing config file - o, err := OptionsFromViper("filedoesnotexist") - if o != nil || err == nil { + _, err = OptionsFromViper("filedoesnotexist") + if err == nil { t.Errorf("OptionsFromViper(): Did when loading missing file") } } diff --git a/proxy/handlers.go b/proxy/handlers.go index 82e9ae582..35c562936 100644 --- a/proxy/handlers.go +++ b/proxy/handlers.go @@ -9,6 +9,7 @@ import ( "time" "github.com/pomerium/pomerium/internal/config" + "github.com/pomerium/pomerium/internal/cryptutil" "github.com/pomerium/pomerium/internal/httputil" "github.com/pomerium/pomerium/internal/log" @@ -529,7 +530,7 @@ func extendDeadline(ttl time.Duration) time.Time { } // websocketHandlerFunc splits request serving with timeouts depending on the protocol -func websocketHandlerFunc(baseHandler http.Handler, timeoutHandler http.Handler, o *config.Options) http.Handler { +func websocketHandlerFunc(baseHandler http.Handler, timeoutHandler http.Handler, o config.Options) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Do not use timeouts for websockets because they are long-lived connections. diff --git a/proxy/handlers_test.go b/proxy/handlers_test.go index 7545e93d9..9a8213d27 100644 --- a/proxy/handlers_test.go +++ b/proxy/handlers_test.go @@ -290,7 +290,7 @@ func TestProxy_Proxy(t *testing.T) { tests := []struct { name string - options *config.Options + options config.Options method string header http.Header host string @@ -356,7 +356,7 @@ func TestProxy_UserDashboard(t *testing.T) { opts := testOptions() tests := []struct { name string - options *config.Options + options config.Options method string cipher cryptutil.Cipher session sessions.SessionStore @@ -408,7 +408,7 @@ func TestProxy_Refresh(t *testing.T) { tests := []struct { name string - options *config.Options + options config.Options method string cipher cryptutil.Cipher session sessions.SessionStore @@ -452,7 +452,7 @@ func TestProxy_Impersonate(t *testing.T) { tests := []struct { name string malformed bool - options *config.Options + options config.Options method string email string groups string diff --git a/proxy/proxy.go b/proxy/proxy.go index bb4883aa6..e16f5d7f1 100755 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -35,7 +35,7 @@ const ( // ValidateOptions checks that proper configuration settings are set to create // a proper Proxy instance -func ValidateOptions(o *config.Options) error { +func ValidateOptions(o config.Options) error { decoded, err := base64.StdEncoding.DecodeString(o.SharedKey) if err != nil { return fmt.Errorf("authorize: `SHARED_SECRET` setting is invalid base64: %v", err) @@ -46,13 +46,13 @@ func ValidateOptions(o *config.Options) error { if len(o.Policies) == 0 { return errors.New("missing setting: no policies defined") } - if o.AuthenticateURL == nil { + 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 { + if o.AuthorizeURL.String() == "" { return errors.New("missing setting: authorize-service-url") } if o.AuthorizeURL.Scheme != "https" { @@ -106,10 +106,7 @@ type routeConfig struct { // New takes a Proxy service from options and a validation function. // Function returns an error if options fail to validate. -func New(opts *config.Options) (*Proxy, error) { - if opts == nil { - return nil, errors.New("options cannot be nil") - } +func New(opts config.Options) (*Proxy, error) { if err := ValidateOptions(opts); err != nil { return nil, err } @@ -137,7 +134,7 @@ func New(opts *config.Options) (*Proxy, error) { p := &Proxy{ routeConfigs: make(map[string]*routeConfig), // services - AuthenticateURL: opts.AuthenticateURL, + AuthenticateURL: &opts.AuthenticateURL, // session state cipher: cipher, csrfStore: cookieStore, @@ -177,7 +174,7 @@ func New(opts *config.Options) (*Proxy, error) { } // UpdatePolicies updates the handlers based on the configured policies -func (p *Proxy) UpdatePolicies(opts *config.Options) error { +func (p *Proxy) UpdatePolicies(opts config.Options) error { routeConfigs := make(map[string]*routeConfig) for _, route := range opts.Policies { proxy := NewReverseProxy(route.Destination) @@ -254,7 +251,7 @@ func NewReverseProxy(to *url.URL) *httputil.ReverseProxy { } // NewReverseProxyHandler applies handler specific options to a given route. -func NewReverseProxyHandler(o *config.Options, proxy *httputil.ReverseProxy, route *policy.Policy) (http.Handler, error) { +func NewReverseProxyHandler(o config.Options, proxy *httputil.ReverseProxy, route *policy.Policy) (http.Handler, error) { up := &UpstreamProxy{ name: route.Destination.Host, handler: proxy, @@ -287,7 +284,7 @@ func urlParse(uri string) (*url.URL, error) { } // UpdateOptions updates internal structres based on config.Options -func (p *Proxy) UpdateOptions(o *config.Options) error { +func (p *Proxy) UpdateOptions(o config.Options) error { log.Info().Msg("proxy: updating options") err := p.UpdatePolicies(o) if err != nil { diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index 3445a3cc3..de2b63f5c 100644 --- a/proxy/proxy_test.go +++ b/proxy/proxy_test.go @@ -80,7 +80,7 @@ func TestNewReverseProxyHandler(t *testing.T) { } } -func testOptions() *config.Options { +func testOptions() config.Options { authenticateService, _ := url.Parse("https://authenticate.corp.beyondperimeter.com") authorizeService, _ := url.Parse("https://authorize.corp.beyondperimeter.com") @@ -88,15 +88,15 @@ func testOptions() *config.Options { testPolicy := policy.Policy{From: "corp.example.notatld", To: "example.notatld"} testPolicy.Validate() opts.Policies = []policy.Policy{testPolicy} - opts.AuthenticateURL = authenticateService - opts.AuthorizeURL = authorizeService + opts.AuthenticateURL = *authenticateService + opts.AuthorizeURL = *authorizeService opts.SharedKey = "80ldlrU2d7w+wVpKNfevk6fmb8otEx6CqOfshj2LwhQ=" opts.CookieSecret = "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw=" opts.CookieName = "pomerium" return opts } -func testOptionsTestServer(uri string) *config.Options { +func testOptionsTestServer(uri string) config.Options { authenticateService, _ := url.Parse("https://authenticate.corp.beyondperimeter.com") authorizeService, _ := url.Parse("https://authorize.corp.beyondperimeter.com") // RFC 2606 @@ -107,15 +107,15 @@ func testOptionsTestServer(uri string) *config.Options { testPolicy.Validate() opts := config.NewOptions() opts.Policies = []policy.Policy{testPolicy} - opts.AuthenticateURL = authenticateService - opts.AuthorizeURL = authorizeService + opts.AuthenticateURL = *authenticateService + opts.AuthorizeURL = *authorizeService opts.SharedKey = "80ldlrU2d7w+wVpKNfevk6fmb8otEx6CqOfshj2LwhQ=" opts.CookieSecret = "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw=" opts.CookieName = "pomerium" return opts } -func testOptionsWithCORS(uri string) *config.Options { +func testOptionsWithCORS(uri string) config.Options { testPolicy := policy.Policy{ From: "httpbin.corp.example", To: uri, @@ -127,7 +127,7 @@ func testOptionsWithCORS(uri string) *config.Options { return opts } -func testOptionsWithPublicAccess(uri string) *config.Options { +func testOptionsWithPublicAccess(uri string) config.Options { testPolicy := policy.Policy{ From: "httpbin.corp.example", To: uri, @@ -139,7 +139,7 @@ func testOptionsWithPublicAccess(uri string) *config.Options { return opts } -func testOptionsWithPublicAccessAndWhitelist(uri string) *config.Options { +func testOptionsWithPublicAccessAndWhitelist(uri string) config.Options { testPolicy := policy.Policy{ From: "httpbin.corp.example", To: uri, @@ -155,14 +155,14 @@ func testOptionsWithPublicAccessAndWhitelist(uri string) *config.Options { func TestOptions_Validate(t *testing.T) { good := testOptions() badAuthURL := testOptions() - badAuthURL.AuthenticateURL = nil + badAuthURL.AuthenticateURL = url.URL{} authurl, _ := url.Parse("http://authenticate.corp.beyondperimeter.com") authenticateBadScheme := testOptions() - authenticateBadScheme.AuthenticateURL = authurl + authenticateBadScheme.AuthenticateURL = *authurl authorizeBadSCheme := testOptions() - authorizeBadSCheme.AuthorizeURL = authurl + authorizeBadSCheme.AuthorizeURL = *authurl authorizeNil := testOptions() - authorizeNil.AuthorizeURL = nil + authorizeNil.AuthorizeURL = url.URL{} emptyCookieSecret := testOptions() emptyCookieSecret.CookieSecret = "" invalidCookieSecret := testOptions() @@ -178,11 +178,11 @@ func TestOptions_Validate(t *testing.T) { tests := []struct { name string - o *config.Options + o config.Options wantErr bool }{ {"good - minimum options", good, false}, - {"nil options", &config.Options{}, true}, + {"nil options", config.Options{}, true}, {"authenticate service url", badAuthURL, true}, {"authenticate service url not https", authenticateBadScheme, true}, {"authorize service url not https", authorizeBadSCheme, true}, @@ -213,14 +213,13 @@ func TestNew(t *testing.T) { badRoutedProxy.SigningKey = "YmFkIGtleQo=" tests := []struct { name string - opts *config.Options + opts config.Options wantProxy bool numRoutes int wantErr bool }{ {"good", good, true, 1, false}, - {"empty options", &config.Options{}, false, 0, true}, - {"nil options", nil, false, 0, true}, + {"empty options", config.Options{}, false, 0, true}, {"short secret/validate sanity check", shortCookieLength, false, 0, true}, {"invalid ec key, valid base64 though", badRoutedProxy, false, 0, true}, } @@ -296,7 +295,7 @@ func Test_UpdateOptions(t *testing.T) { } tests := []struct { name string - opts *config.Options + opts config.Options newPolicy []policy.Policy host string wantErr bool