cmd/pomerium : refactor main to more testable (#112)

- cmd/pomerium: refactor global timeouts to be configurable
- cmd/pomerium: add tests
- cmd/pomerium: remove debug flag, set with env vars only
- cmd/pomerium: global ping now returns version not OK
- proxy: validate shared secret encoding and length
- docs: add timeout to example policy
- docs: document timeouts and cors
- docs: update pomerium logo
- docs: add policy authorization docs

cmd/pomerium/main.go

@@ -1,6 +1,7 @@
 package main // import "github.com/pomerium/pomerium/cmd/pomerium"
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"net/http"
@@ -21,116 +22,148 @@ import (
 	"github.com/pomerium/pomerium/proxy"
 )
 
-var (
-	debugFlag   = flag.Bool("debug", false, "run server in debug mode, changes log output to STDOUT and level to info")
-	versionFlag = flag.Bool("version", false, "prints the version")
-)
+var versionFlag = flag.Bool("version", false, "prints the version")
 
 func main() {
 	flag.Parse()
 	if *versionFlag {
-		fmt.Printf("%s\n", version.FullVersion())
+		fmt.Println(version.FullVersion())
 		os.Exit(0)
 	}
-	mainOpts, err := optionsFromEnvConfig()
+	opt, err := parseOptions()
 	if err != nil {
-		log.Fatal().Err(err).Msg("cmd/pomerium: settings error")
+		log.Fatal().Err(err).Msg("cmd/pomerium: options")
 	}
-	if *debugFlag || mainOpts.Debug {
-		log.SetDebugMode()
-	}
-	if mainOpts.LogLevel != "" {
-		log.SetLevel(mainOpts.LogLevel)
-	}
-	log.Info().Str("version", version.FullVersion()).Str("user-agent", version.UserAgent()).Str("service", mainOpts.Services).Msg("cmd/pomerium")
 
-	grpcAuth := middleware.NewSharedSecretCred(mainOpts.SharedKey)
+	grpcAuth := middleware.NewSharedSecretCred(opt.SharedKey)
 	grpcOpts := []grpc.ServerOption{grpc.UnaryInterceptor(grpcAuth.ValidateRequest)}
 	grpcServer := grpc.NewServer(grpcOpts...)
 
-	var authenticateService *authenticate.Authenticate
-	var authHost string
-	if mainOpts.Services == "all" || mainOpts.Services == "authenticate" {
-		opts, err := authenticate.OptionsFromEnvConfig()
-		if err != nil {
-			log.Fatal().Err(err).Msg("cmd/pomerium: authenticate settings")
-		}
-		authenticateService, err = authenticate.New(opts)
-		if err != nil {
-			log.Fatal().Err(err).Msg("cmd/pomerium: new authenticate")
-		}
-		authHost = urlutil.StripPort(opts.AuthenticateURL.Host)
-		pbAuthenticate.RegisterAuthenticatorServer(grpcServer, authenticateService)
-	}
-
-	var authorizeService *authorize.Authorize
-	if mainOpts.Services == "all" || mainOpts.Services == "authorize" {
-		opts, err := authorize.OptionsFromEnvConfig()
-		if err != nil {
-			log.Fatal().Err(err).Msg("cmd/pomerium: authorize settings")
-		}
-		authorizeService, err = authorize.New(opts)
-		if err != nil {
-			log.Fatal().Err(err).Msg("cmd/pomerium: new authorize")
-		}
-		pbAuthorize.RegisterAuthorizerServer(grpcServer, authorizeService)
-	}
-
-	var proxyService *proxy.Proxy
-	if mainOpts.Services == "all" || mainOpts.Services == "proxy" {
-		proxyOpts, err := proxy.OptionsFromEnvConfig()
-		if err != nil {
-			log.Fatal().Err(err).Msg("cmd/pomerium: proxy settings")
-		}
-
-		proxyService, err = proxy.New(proxyOpts)
-		if err != nil {
-			log.Fatal().Err(err).Msg("cmd/pomerium: new proxy")
-		}
-		// cleanup our RPC services
-		defer proxyService.AuthenticateClient.Close()
-		defer proxyService.AuthorizeClient.Close()
-
-	}
-
-	topMux := http.NewServeMux()
-	topMux.HandleFunc("/ping", func(rw http.ResponseWriter, _ *http.Request) {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/ping", func(rw http.ResponseWriter, _ *http.Request) {
 		rw.WriteHeader(http.StatusOK)
-		fmt.Fprintf(rw, "OK")
+		fmt.Fprintf(rw, version.UserAgent())
 	})
-	if authenticateService != nil {
-		topMux.Handle(authHost+"/", authenticateService.Handler())
+
+	_, err = newAuthenticateService(opt.Services, mux, grpcServer)
+	if err != nil {
+		log.Fatal().Err(err).Msg("cmd/pomerium: authenticate")
 	}
-	if proxyService != nil {
-		topMux.Handle("/", proxyService.Handler())
+
+	_, err = newAuthorizeService(opt.Services, grpcServer)
+	if err != nil {
+		log.Fatal().Err(err).Msg("cmd/pomerium: authorize")
 	}
 
+	_, err = newProxyService(opt.Services, mux)
+	if err != nil {
+		log.Fatal().Err(err).Msg("cmd/pomerium: proxy")
+	}
+	// defer statements ignored anyway :  https://stackoverflow.com/a/17888654
+	// defer proxyService.AuthenticateClient.Close()
+	// defer proxyService.AuthorizeClient.Close()
+
 	httpOpts := &https.Options{
-		Addr:     mainOpts.Addr,
-		Cert:     mainOpts.Cert,
-		Key:      mainOpts.Key,
-		CertFile: mainOpts.CertFile,
-		KeyFile:  mainOpts.KeyFile,
+		Addr:              opt.Addr,
+		Cert:              opt.Cert,
+		Key:               opt.Key,
+		CertFile:          opt.CertFile,
+		KeyFile:           opt.KeyFile,
+		ReadTimeout:       opt.ReadTimeout,
+		WriteTimeout:      opt.WriteTimeout,
+		ReadHeaderTimeout: opt.ReadHeaderTimeout,
+		IdleTimeout:       opt.IdleTimeout,
 	}
 
-	if mainOpts.HTTPRedirectAddr != "" {
-		// stand up another http server that just redirect HTTP to HTTPS traffic
-		srv := &http.Server{
-			Addr:         mainOpts.HTTPRedirectAddr,
-			ReadTimeout:  5 * time.Second,
-			WriteTimeout: 5 * time.Second,
-			Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-				w.Header().Set("Connection", "close")
-				url := fmt.Sprintf("https://%s%s", urlutil.StripPort(r.Host), r.URL.String())
-				http.Redirect(w, r, url, http.StatusMovedPermanently)
-			}),
-		}
-		log.Info().Str("Addr", mainOpts.HTTPRedirectAddr).Msg("cmd/pomerium: http redirect server started")
-		go func() { log.Fatal().Err(srv.ListenAndServe()).Msg("cmd/pomerium: http server") }()
+	if srv, err := startRedirectServer(opt.HTTPRedirectAddr); err != nil {
+		log.Debug().Err(err).Msg("cmd/pomerium: http redirect server not started")
 	} else {
-		log.Debug().Msg("cmd/pomerium: http redirect server not started")
+		defer srv.Close()
+	}
+	if err := https.ListenAndServeTLS(httpOpts, mux, grpcServer); err != nil {
+		log.Fatal().Err(err).Msg("cmd/pomerium: https server")
 	}
-
-	log.Fatal().Err(https.ListenAndServeTLS(httpOpts, topMux, grpcServer)).Msg("cmd/pomerium: https server")
-
+}
+
+// startRedirectServer starts a http server that redirect HTTP to HTTPS traffic
+func startRedirectServer(addr string) (*http.Server, error) {
+	if addr == "" {
+		return nil, errors.New("no http redirect addr provided")
+	}
+	srv := &http.Server{
+		Addr:         addr,
+		ReadTimeout:  5 * time.Second,
+		WriteTimeout: 5 * time.Second,
+		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Connection", "close")
+			url := fmt.Sprintf("https://%s%s", urlutil.StripPort(r.Host), r.URL.String())
+			http.Redirect(w, r, url, http.StatusMovedPermanently)
+		}),
+	}
+	log.Info().Str("Addr", addr).Msg("cmd/pomerium: http redirect server started")
+	go func() { log.Error().Err(srv.ListenAndServe()).Msg("cmd/pomerium: http server closed") }()
+	return srv, nil
+}
+
+func newAuthenticateService(s string, mux *http.ServeMux, rpc *grpc.Server) (*authenticate.Authenticate, error) {
+	if !isAuthenticate(s) {
+		return nil, nil
+	}
+	opts, err := authenticate.OptionsFromEnvConfig()
+	if err != nil {
+		return nil, err
+	}
+	service, err := authenticate.New(opts)
+	if err != nil {
+		return nil, err
+	}
+	pbAuthenticate.RegisterAuthenticatorServer(rpc, service)
+	mux.Handle(urlutil.StripPort(opts.AuthenticateURL.Host)+"/", service.Handler())
+	return service, nil
+}
+
+func newAuthorizeService(s string, rpc *grpc.Server) (*authorize.Authorize, error) {
+	if !isAuthorize(s) {
+		return nil, nil
+	}
+	opts, err := authorize.OptionsFromEnvConfig()
+	if err != nil {
+		return nil, err
+	}
+	service, err := authorize.New(opts)
+	if err != nil {
+		return nil, err
+	}
+	pbAuthorize.RegisterAuthorizerServer(rpc, service)
+	return service, nil
+}
+
+func newProxyService(s string, mux *http.ServeMux) (*proxy.Proxy, error) {
+	if !isProxy(s) {
+		return nil, nil
+	}
+	opts, err := proxy.OptionsFromEnvConfig()
+	if err != nil {
+		return nil, err
+	}
+	service, err := proxy.New(opts)
+	if err != nil {
+		return nil, err
+	}
+	mux.Handle("/", service.Handler())
+	return service, nil
+}
+
+func parseOptions() (*Options, error) {
+	o, err := optionsFromEnvConfig()
+	if err != nil {
+		return nil, err
+	}
+	if o.Debug {
+		log.SetDebugMode()
+	}
+	if o.LogLevel != "" {
+		log.SetLevel(o.LogLevel)
+	}
+	return o, nil
 }

cmd/pomerium/main_test.go

@@ -0,0 +1,199 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/pomerium/pomerium/internal/middleware"
+	"google.golang.org/grpc"
+)
+
+func Test_startRedirectServer(t *testing.T) {
+
+	tests := []struct {
+		name    string
+		addr    string
+		want    string
+		wantErr bool
+	}{
+		{"empty", "", "", true},
+		{":http", ":http", ":http", false},
+		{"localhost:80", "localhost:80", "localhost:80", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := startRedirectServer(tt.addr)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("startRedirectServer() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if got != nil {
+				defer got.Close()
+				ts := httptest.NewServer(got.Handler)
+				defer ts.Close()
+				_, err := http.Get(ts.URL)
+				if !strings.Contains(err.Error(), "https") {
+					t.Errorf("startRedirectServer() = %v, want %v", err, tt.want)
+					return
+				}
+			}
+		})
+	}
+}
+
+func Test_newAuthenticateService(t *testing.T) {
+	os.Clearenv()
+	grpcAuth := middleware.NewSharedSecretCred("test")
+	grpcOpts := []grpc.ServerOption{grpc.UnaryInterceptor(grpcAuth.ValidateRequest)}
+	grpcServer := grpc.NewServer(grpcOpts...)
+	mux := http.NewServeMux()
+
+	tests := []struct {
+		name     string
+		s        string
+		envKey   string
+		envValue string
+
+		wantHostname string
+		wantErr      bool
+	}{
+		{"wrong service", "proxy", "", "", "", false},
+		{"bad", "authenticate", "SHARED_SECRET", "error!", "", true},
+		{"bad emv", "authenticate", "COOKIE_REFRESH", "error!", "", true},
+		{"good", "authenticate", "IDP_CLIENT_ID", "test", "auth.server.com", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			os.Setenv("IDP_PROVIDER", "google")
+			os.Setenv("IDP_CLIENT_SECRET", "TEST")
+			os.Setenv("SHARED_SECRET", "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=")
+			os.Setenv("COOKIE_SECRET", "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=")
+			os.Setenv("AUTHENTICATE_SERVICE_URL", "http://auth.server.com")
+			defer os.Unsetenv("IDP_CLIENT_ID")
+			defer os.Unsetenv("IDP_CLIENT_SECRET")
+			defer os.Unsetenv("SHARED_SECRET")
+			defer os.Unsetenv("COOKIE_SECRET")
+
+			os.Setenv(tt.envKey, tt.envValue)
+			defer os.Unsetenv(tt.envKey)
+
+			_, err := newAuthenticateService(tt.s, mux, grpcServer)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("newAuthenticateService() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+		})
+	}
+}
+
+func Test_newAuthorizeService(t *testing.T) {
+	os.Clearenv()
+	grpcAuth := middleware.NewSharedSecretCred("test")
+	grpcOpts := []grpc.ServerOption{grpc.UnaryInterceptor(grpcAuth.ValidateRequest)}
+	grpcServer := grpc.NewServer(grpcOpts...)
+
+	tests := []struct {
+		name     string
+		s        string
+		envKey   string
+		envValue string
+
+		wantErr bool
+	}{
+		{"wrong service", "proxy", "", "", false},
+		{"bad option parsing", "authorize", "SHARED_SECRET", "false", true},
+		{"bad env", "authorize", "POLICY", "error!", true},
+		{"good", "authorize", "SHARED_SECRET", "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			os.Setenv("POLICY", "LSBmcm9tOiBodHRwYmluLmNvcnAuYmV5b25kcGVyaW1ldGVyLmNvbQogIHRvOiBodHRwOi8vaHR0cGJpbgogIGFsbG93ZWRfZG9tYWluczoKICAgIC0gcG9tZXJpdW0uaW8KICBjb3JzX2FsbG93X3ByZWZsaWdodDogdHJ1ZQogIHRpbWVvdXQ6IDMwcwotIGZyb206IGV4dGVybmFsLWh0dHBiaW4uY29ycC5iZXlvbmRwZXJpbWV0ZXIuY29tCiAgdG86IGh0dHBiaW4ub3JnCiAgYWxsb3dlZF9kb21haW5zOgogICAgLSBnbWFpbC5jb20KLSBmcm9tOiB3ZWlyZGx5c3NsLmNvcnAuYmV5b25kcGVyaW1ldGVyLmNvbQogIHRvOiBodHRwOi8vbmV2ZXJzc2wuY29tCiAgYWxsb3dlZF91c2VyczoKICAgIC0gYmRkQHBvbWVyaXVtLmlvCiAgYWxsb3dlZF9ncm91cHM6CiAgICAtIGFkbWlucwogICAgLSBkZXZlbG9wZXJzCi0gZnJvbTogaGVsbG8uY29ycC5iZXlvbmRwZXJpbWV0ZXIuY29tCiAgdG86IGh0dHA6Ly9oZWxsbzo4MDgwCiAgYWxsb3dlZF9ncm91cHM6CiAgICAtIGFkbWlucw==")
+			os.Setenv("COOKIE_SECRET", "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=")
+			defer os.Unsetenv("SHARED_SECRET")
+			defer os.Unsetenv("COOKIE_SECRET")
+			os.Setenv(tt.envKey, tt.envValue)
+			defer os.Unsetenv(tt.envKey)
+			_, err := newAuthorizeService(tt.s, grpcServer)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("newAuthorizeService() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+		})
+	}
+}
+
+func Test_newProxyeService(t *testing.T) {
+	os.Clearenv()
+	tests := []struct {
+		name     string
+		s        string
+		envKey   string
+		envValue string
+
+		wantErr bool
+	}{
+		{"wrong service", "authenticate", "", "", false},
+		{"bad option parsing", "proxy", "SHARED_SECRET", "false", true},
+		{"bad env", "proxy", "POLICY", "error!", true},
+		{"bad encoding for envar", "proxy", "COOKIE_REFRESH", "error!", true},
+		{"good", "proxy", "SHARED_SECRET", "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mux := http.NewServeMux()
+
+			os.Setenv("AUTHENTICATE_SERVICE_URL", "https://authenticate.example.com")
+			os.Setenv("AUTHORIZE_SERVICE_URL", "https://authorize.example.com")
+			os.Setenv("POLICY", "LSBmcm9tOiBodHRwYmluLmNvcnAuYmV5b25kcGVyaW1ldGVyLmNvbQogIHRvOiBodHRwOi8vaHR0cGJpbgogIGFsbG93ZWRfZG9tYWluczoKICAgIC0gcG9tZXJpdW0uaW8KICBjb3JzX2FsbG93X3ByZWZsaWdodDogdHJ1ZQogIHRpbWVvdXQ6IDMwcwotIGZyb206IGV4dGVybmFsLWh0dHBiaW4uY29ycC5iZXlvbmRwZXJpbWV0ZXIuY29tCiAgdG86IGh0dHBiaW4ub3JnCiAgYWxsb3dlZF9kb21haW5zOgogICAgLSBnbWFpbC5jb20KLSBmcm9tOiB3ZWlyZGx5c3NsLmNvcnAuYmV5b25kcGVyaW1ldGVyLmNvbQogIHRvOiBodHRwOi8vbmV2ZXJzc2wuY29tCiAgYWxsb3dlZF91c2VyczoKICAgIC0gYmRkQHBvbWVyaXVtLmlvCiAgYWxsb3dlZF9ncm91cHM6CiAgICAtIGFkbWlucwogICAgLSBkZXZlbG9wZXJzCi0gZnJvbTogaGVsbG8uY29ycC5iZXlvbmRwZXJpbWV0ZXIuY29tCiAgdG86IGh0dHA6Ly9oZWxsbzo4MDgwCiAgYWxsb3dlZF9ncm91cHM6CiAgICAtIGFkbWlucw==")
+			os.Setenv("COOKIE_SECRET", "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=")
+			defer os.Unsetenv("AUTHENTICATE_SERVICE_URL")
+			defer os.Unsetenv("AUTHORIZE_SERVICE_URL")
+			defer os.Unsetenv("SHARED_SECRET")
+			defer os.Unsetenv("COOKIE_SECRET")
+			os.Setenv(tt.envKey, tt.envValue)
+			defer os.Unsetenv(tt.envKey)
+			_, err := newProxyService(tt.s, mux)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("newProxyService() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+		})
+	}
+}
+
+func Test_parseOptions(t *testing.T) {
+	tests := []struct {
+		name     string
+		envKey   string
+		envValue string
+
+		want    *Options
+		wantErr bool
+	}{
+		{"no shared secret", "", "", nil, true},
+		{"good", "SHARED_SECRET", "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=", &Options{Services: "all", SharedKey: "YixWi1MYh77NMECGGIJQevoonYtVF+ZPRkQZrrmeRqM=", LogLevel: "debug"}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			os.Setenv(tt.envKey, tt.envValue)
+			defer os.Unsetenv(tt.envKey)
+
+			got, err := parseOptions()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("parseOptions() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("parseOptions()\n")
+				t.Errorf("got: %+v\n", got)
+				t.Errorf("want: %+v\n", tt.want)
+
+			}
+		})
+	}
+}

cmd/pomerium/options.go

@@ -3,6 +3,7 @@ package main // import "github.com/pomerium/pomerium/cmd/pomerium"
 import (
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/pomerium/envconfig"
 )
@@ -43,6 +44,12 @@ type Options struct {
 	// to HTTPS redirect server on. For example, ":http" would start a server
 	// on port 80.  If empty, no redirect server is started.
 	HTTPRedirectAddr string `envconfig:"HTTP_REDIRECT_ADDR"`
+
+	// Timeout settings : https://github.com/pomerium/pomerium/issues/40
+	ReadTimeout       time.Duration `envconfig:"TIMEOUT_READ"`
+	WriteTimeout      time.Duration `envconfig:"TIMEOUT_WRITE"`
+	ReadHeaderTimeout time.Duration `envconfig:"TIMEOUT_READ_HEADER"`
+	IdleTimeout       time.Duration `envconfig:"TIMEOUT_IDLE"`
 }
 
 var defaultOptions = &Options{
@@ -68,8 +75,8 @@ func optionsFromEnvConfig() (*Options, error) {
 }
 
 // isValidService checks to see if a service is a valid service mode
-func isValidService(service string) bool {
-	switch service {
+func isValidService(s string) bool {
+	switch s {
 	case
 		"all",
 		"proxy",
@@ -79,3 +86,33 @@ func isValidService(service string) bool {
 	}
 	return false
 }
+
+func isAuthenticate(s string) bool {
+	switch s {
+	case
+		"all",
+		"authenticate":
+		return true
+	}
+	return false
+}
+
+func isAuthorize(s string) bool {
+	switch s {
+	case
+		"all",
+		"authorize":
+		return true
+	}
+	return false
+}
+
+func isProxy(s string) bool {
+	switch s {
+	case
+		"all",
+		"proxy":
+		return true
+	}
+	return false
+}

cmd/pomerium/options_test.go

@@ -66,3 +66,26 @@ func Test_isValidService(t *testing.T) {
 		})
 	}
 }
+
+func Test_isAuthenticate(t *testing.T) {
+	tests := []struct {
+		name    string
+		service string
+		want    bool
+	}{
+		{"proxy", "proxy", false},
+		{"all", "all", true},
+		{"authenticate", "authenticate", true},
+		{"authenticate bad case", "AuThenticate", false},
+		{"authorize implemented", "authorize", false},
+		{"jiberish", "xd23", false},
+	}
+	for _, tt := range tests {
+
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isAuthenticate(tt.service); got != tt.want {
+				t.Errorf("isAuthenticate() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}