Merge pull request #2421 from target/strict-url-validation

http: add --public-url flag to replace various URL validation approaches

Procfile

@@ -1,7 +1,7 @@
 build: while true; do make -qs bin/goalert || make bin/goalert || (echo '\033[0;31mBuild Failure'; sleep 3); sleep 0.1; done
 
 @watch-file=./bin/goalert
-goalert: ./bin/goalert -l=localhost:3030 --ui-dir=web/src/build --db-url=postgres://goalert@localhost --listen-sysapi=localhost:1234 --listen-prometheus=localhost:2112
+goalert: ./bin/goalert -l=localhost:3030 --ui-dir=web/src/build --db-url=postgres://goalert@localhost --listen-sysapi=localhost:1234 --listen-prometheus=localhost:2112 --public-url=http://localhost:3030$HTTP_PREFIX
 
 smtp: go run github.com/mailhog/MailHog -ui-bind-addr=localhost:8025 -api-bind-addr=localhost:8025 -smtp-bind-addr=localhost:1025 | grep -v KEEPALIVE
 prom: bin/tools/prometheus --log.level=warn --config.file=devtools/prometheus/prometheus.yml --storage.tsdb.path=bin/prom-data/ --web.listen-address=localhost:9090

Procfile.cypress

@@ -1,7 +1,7 @@
 build: while true; do make -qs bin/goalert || make bin/goalert || (echo '\033[0;31mBuild Failure'; sleep 3); sleep 0.1; done
 
 @watch-file=./bin/goalert
-goalert: go run ./devtools/waitfor postgres://postgres@localhost:5433 && go run ./devtools/procwrap -test=localhost:3042 bin/goalert -l=localhost:3042 --ui-dir=web/src/build --db-url=postgres://postgres@localhost:5433 --slack-base-url=http://localhost:3040/slack --log-errors-only
+goalert: go run ./devtools/waitfor postgres://postgres@localhost:5433 && go run ./devtools/procwrap -test=localhost:3042 bin/goalert -l=localhost:3042 --ui-dir=web/src/build --db-url=postgres://postgres@localhost:5433 --slack-base-url=http://localhost:3040/slack --log-errors-only --public-url=http://localhost:3040$HTTP_PREFIX
 
 @watch-file=./web/src/esbuild.config.js
 ui: yarn workspace goalert-web run esbuild --watch
@@ -14,6 +14,6 @@ slack: go run ./devtools/mockslack/cmd/mockslack -client-id=000000000000.0000000
 proxy: go run ./devtools/simpleproxy -addr=localhost:3040 /slack/=http://localhost:3046 http://localhost:3042
 
 @oneshot
-cypress: go run ./devtools/waitfor http://localhost:3042 && CYPRESS_DB_URL=postgres://postgres@localhost:5433 yarn workspace goalert-web --cwd=bin/build/integration cypress open --config baseUrl=http://localhost:3040$GOALERT_HTTP_PREFIX
+cypress: go run ./devtools/waitfor http://localhost:3042 && CYPRESS_DB_URL=postgres://postgres@localhost:5433 yarn workspace goalert-web --cwd=bin/build/integration cypress open --config baseUrl=http://localhost:3040$HTTP_PREFIX
 
 db: $CONTAINER_TOOL rm -f smoketest-postgres || true; $CONTAINER_TOOL run -it --rm --name smoketest-postgres -p5433:5432 -e=POSTGRES_HOST_AUTH_METHOD=trust postgres:13-alpine

Procfile.cypress.ci

@@ -1,7 +1,7 @@
 @oneshot
-cypress: go run ./devtools/waitfor http://localhost:3042 && CYPRESS_DB_URL=$DB_URL yarn workspace goalert-web --cwd=bin/build/integration cypress $CY_ACTION --config baseUrl=http://localhost:3040$GOALERT_HTTP_PREFIX
+cypress: go run ./devtools/waitfor http://localhost:3042 && CYPRESS_DB_URL=$DB_URL yarn workspace goalert-web --cwd=bin/build/integration cypress $CY_ACTION --config baseUrl=http://localhost:3040$HTTP_PREFIX
 
-goalert: go run ./devtools/waitfor $DB_URL && go run ./devtools/procwrap -test=localhost:3042 bin/goalert -l=localhost:3042 --db-url=$DB_URL --slack-base-url=http://localhost:3040/slack --log-errors-only
+goalert: go run ./devtools/waitfor $DB_URL && go run ./devtools/procwrap -test=localhost:3042 bin/goalert -l=localhost:3042 --db-url=$DB_URL --slack-base-url=http://localhost:3040/slack --log-errors-only --public-url=http://localhost:3040$HTTP_PREFIX
 
 slack: go run ./devtools/mockslack/cmd/mockslack -client-id=000000000000.000000000000 -client-secret=00000000000000000000000000000000 -access-token=xoxp-000000000000-000000000000-000000000000-00000000000000000000000000000000 -prefix=/slack -single-user=bob -addr=localhost:3046
 

Procfile.cypress.prod

@@ -1,14 +1,14 @@
 build: while true; do make -qs bin/goalert BUNDLE=1 >/dev/null || make bin/goalert BUNDLE=1 || (echo '\033[0;31mBuild Failure'; sleep 3); sleep 0.1; done
 
 @watch-file=./bin/goalert
-goalert: go run ./devtools/waitfor postgres://postgres@localhost:5433 && go run ./devtools/procwrap -test=localhost:3042 bin/goalert -l=localhost:3042 --db-url=postgres://postgres@localhost:5433 --slack-base-url=http://localhost:3040/slack --log-errors-only
+goalert: go run ./devtools/waitfor postgres://postgres@localhost:5433 && go run ./devtools/procwrap -test=localhost:3042 bin/goalert -l=localhost:3042 --db-url=postgres://postgres@localhost:5433 --slack-base-url=http://localhost:3040/slack --log-errors-only --public-url=http://localhost:3040$HTTP_PREFIX
 
 slack: go run ./devtools/mockslack/cmd/mockslack -client-id=000000000000.000000000000 -client-secret=00000000000000000000000000000000 -access-token=xoxp-000000000000-000000000000-000000000000-00000000000000000000000000000000 -prefix=/slack -single-user=bob -addr=localhost:3046
 
 proxy: go run ./devtools/simpleproxy -addr=localhost:3040 /slack/=http://localhost:3046 http://localhost:3042
 
 @oneshot
-cypress: go run ./devtools/waitfor http://localhost:3042 && CYPRESS_DB_URL=postgres://postgres@localhost:5433 yarn workspace goalert-web --cwd=bin/build/integration cypress $CY_ACTION --config baseUrl=http://localhost:3040$GOALERT_HTTP_PREFIX
+cypress: go run ./devtools/waitfor http://localhost:3042 && CYPRESS_DB_URL=postgres://postgres@localhost:5433 yarn workspace goalert-web --cwd=bin/build/integration cypress $CY_ACTION --config baseUrl=http://localhost:3040$HTTP_PREFIX
 
 db: $CONTAINER_TOOL rm -f smoketest-postgres || true; $CONTAINER_TOOL run -it --rm --name smoketest-postgres -p5433:5432 -e=POSTGRES_HOST_AUTH_METHOD=trust postgres:13-alpine
 

Procfile.prod

@@ -1,7 +1,7 @@
 build: while true; do make -qs bin/goalert BUNDLE=1 || make bin/goalert BUNDLE=1 || (echo '\033[0;31mBuild Failure'; sleep 3); sleep 0.1; done
 
 @watch-file=./bin/goalert
-goalert: ./bin/goalert -l=localhost:3030 --db-url=postgres://goalert@localhost --listen-sysapi=localhost:1234 --listen-prometheus=localhost:2112
+goalert: ./bin/goalert -l=localhost:3030 --db-url=postgres://goalert@localhost --listen-sysapi=localhost:1234 --listen-prometheus=localhost:2112 --public-url=http://localhost:3030$HTTP_PREFIX
 
 smtp: go run github.com/mailhog/MailHog -ui-bind-addr=localhost:8025 -api-bind-addr=localhost:8025 -smtp-bind-addr=localhost:1025 | grep -v KEEPALIVE
 prom: bin/tools/prometheus --log.level=warn --config.file=devtools/prometheus/prometheus.yml --storage.tsdb.path=bin/prom-data/ --web.listen-address=localhost:9090

app/cmd.go

@@ -263,7 +263,7 @@ Migration: %s (#%d)
 
 				ctx := cmd.Context()
 
-				store, err := config.NewStore(ctx, conn, cf.EncryptionKeys, "")
+				store, err := config.NewStore(ctx, conn, cf.EncryptionKeys, "", "")
 				if err != nil {
 					return fmt.Errorf("read config: %w", err)
 				}
@@ -623,6 +623,8 @@ func getConfig(ctx context.Context) (Config, error) {
 		DBMaxOpen: viper.GetInt("db-max-open"),
 		DBMaxIdle: viper.GetInt("db-max-idle"),
 
+		PublicURL: viper.GetString("public-url"),
+
 		MaxReqBodyBytes:   viper.GetInt64("max-request-body-bytes"),
 		MaxReqHeaderBytes: viper.GetInt("max-request-header-bytes"),
 
@@ -657,6 +659,17 @@ func getConfig(ctx context.Context) (Config, error) {
 		UIDir: viper.GetString("ui-dir"),
 	}
 
+	if cfg.PublicURL != "" {
+		u, err := url.Parse(cfg.PublicURL)
+		if err != nil {
+			return cfg, errors.Wrap(err, "parse public url")
+		}
+		if cfg.HTTPPrefix != "" {
+			return cfg, errors.New("public-url and http-prefix cannot be used together")
+		}
+		cfg.HTTPPrefix = u.Path
+	}
+
 	if cfg.DBURL == "" {
 		return cfg, ErrDBRequired
 	}
@@ -685,6 +698,8 @@ func init() {
 	RootCmd.Flags().String("sysapi-key-file", "", "(Experimental) Specifies a path to a PEM-encoded private key file use when connecting to plugin services.")
 	RootCmd.Flags().String("sysapi-ca-file", "", "(Experimental) Specifies a path to a PEM-encoded certificate(s) to authorize connections from plugin services.")
 
+	RootCmd.Flags().String("public-url", "", "Externally routable URL to the application. Used for validating callback requests, links, auth, and prefix calculation.")
+
 	RootCmd.PersistentFlags().StringP("listen-prometheus", "p", "", "Bind address for Prometheus metrics.")
 
 	RootCmd.Flags().String("tls-cert-file", "", "Specifies a path to a PEM-encoded certificate.  Has no effect if --listen-tls is unset.")
@@ -693,6 +708,7 @@ func init() {
 	RootCmd.Flags().String("tls-key-data", "", "Specifies a PEM-encoded private key.  Has no effect if --listen-tls is unset.")
 
 	RootCmd.Flags().String("http-prefix", def.HTTPPrefix, "Specify the HTTP prefix of the application.")
+	RootCmd.Flags().MarkDeprecated("http-prefix", "use --public-url instead")
 
 	RootCmd.Flags().Bool("api-only", def.APIOnly, "Starts in API-only mode (schedules & notifications will not be processed). Useful in clusters.")
 

app/config.go

@@ -19,6 +19,8 @@ type Config struct {
 	APIOnly     bool
 	LogEngine   bool
 
+	PublicURL string
+
 	TLSListenAddr string
 	TLSConfig     *tls.Config
 

app/getsetconfig.go

@@ -41,7 +41,7 @@ func getSetConfig(ctx context.Context, setCfg bool, data []byte) error {
 	}
 	defer tx.Rollback()
 
-	s, err := config.NewStore(ctx, db, c.EncryptionKeys, "")
+	s, err := config.NewStore(ctx, db, c.EncryptionKeys, "", "")
 	if err != nil {
 		return errors.Wrap(err, "init config store")
 	}

app/initstores.go

@@ -45,7 +45,7 @@ func (app *App) initStores(ctx context.Context) error {
 		fallback.Scheme = "http"
 		fallback.Host = app.l.Addr().String()
 		fallback.Path = app.cfg.HTTPPrefix
-		app.ConfigStore, err = config.NewStore(ctx, app.db, app.cfg.EncryptionKeys, fallback.String())
+		app.ConfigStore, err = config.NewStore(ctx, app.db, app.cfg.EncryptionKeys, app.cfg.PublicURL, fallback.String())
 	}
 	if err != nil {
 		return errors.Wrap(err, "init config store")

auth/cookies.go

@@ -2,7 +2,11 @@ package auth
 
 import (
 	"net/http"
+	"net/url"
+	"strings"
 	"time"
+
+	"github.com/target/goalert/config"
 )
 
 // SetCookie will set a cookie value for all API prefixes, respecting the current config parameters.
@@ -12,18 +16,25 @@ func SetCookie(w http.ResponseWriter, req *http.Request, name, value string) {
 
 // SetCookieAge behaves like SetCookie but also sets the MaxAge.
 func SetCookieAge(w http.ResponseWriter, req *http.Request, name, value string, age time.Duration) {
+	cfg := config.FromContext(req.Context())
+	u, err := url.Parse(cfg.PublicURL())
+	if err != nil {
+		panic(err)
+	}
+
+	cookiePath := "/"
+	secure := req.URL.Scheme == "https"
+	if cfg.ShouldUsePublicURL() {
+		cookiePath = strings.TrimSuffix(u.Path, "/") + "/"
+		secure = u.Scheme == "https"
+	}
+
 	http.SetCookie(w, &http.Cookie{
 		HttpOnly: true,
-		Secure:   req.URL.Scheme == "https",
+		Secure:   secure,
 		Name:     name,
 
-		// Until we can finish removing /v1 from all UI calls
-		// we need cookies available on both /api and /v1.
-		//
-		// Unfortunately we can't just set both paths without breaking integration tests...
-		// We'll keep this as `/` until Cypress fixes it's cookie handling, or we
-		// finish removing the `/v1` UI code. Whichever is sooner.
-		Path:   "/",
+		Path:   cookiePath,
 		Value:  value,
 		MaxAge: int(age.Seconds()),
 	})