refactor: replace relative canonical redirect with handler

This also ensure that we clean any urls that may stem from user input (e.g.
url parameter or login cookie) before performing redirects.

cmd/wonderwall/main.go

@@ -96,7 +96,7 @@ func ssoServerHandler(ctx context.Context, cfg *config.Config, crypt crypto.Cryp
 		WithPath("/").
 		WithDomain(cfg.SSO.Domain)
 
-	return handler.NewSSOServerHandler(h), nil
+	return handler.NewSSOServerHandler(h)
 }
 
 func ssoProxyHandler(cfg *config.Config) (*handler.SSOProxyHandler, error) {

pkg/config/config.go

@@ -50,10 +50,11 @@ type Session struct {
 }
 
 type SSO struct {
-	Enabled   bool    `json:"enabled"`
-	Domain    string  `json:"domain"`
-	Mode      SSOMode `json:"mode"`
-	ServerURL string  `json:"server-url"`
+	Enabled                  bool    `json:"enabled"`
+	Domain                   string  `json:"domain"`
+	Mode                     SSOMode `json:"mode"`
+	ServerURL                string  `json:"server-url"`
+	ServerDefaultRedirectURL string  `json:"server-default-redirect-url"`
 }
 
 type SSOMode string
@@ -87,10 +88,11 @@ const (
 	LoginstatusResourceIndicator = "loginstatus.resource-indicator"
 	LoginstatusTokenURL          = "loginstatus.token-url"
 
-	SSOEnabled   = "sso.enabled"
-	SSODomain    = "sso.domain"
-	SSOModeFlag  = "sso.mode"
-	SSOServerURL = "sso.server-url"
+	SSOEnabled                  = "sso.enabled"
+	SSODomain                   = "sso.domain"
+	SSOServerDefaultRedirectURL = "sso.server-default-redirect-url"
+	SSOModeFlag                 = "sso.mode"
+	SSOServerURL                = "sso.server-url"
 )
 
 func Initialize() (*Config, error) {
@@ -122,7 +124,8 @@ func Initialize() (*Config, error) {
 	flag.Bool(SSOEnabled, false, "Enable single sign-on mode; one server acting as the OIDC Relying Party, and N proxies. The proxies delegate most endpoint operations to the server, and only implements a reverse proxy that reads the user's session data from the shared store.")
 	flag.String(SSODomain, "", "The domain that the session cookies should be set for, usually the second-level domain name (e.g. example.com).")
 	flag.String(SSOModeFlag, string(SSOModeServer), "The SSO mode for this instance. Must be one of 'server' or 'proxy'.")
-	flag.String(SSOServerURL, "", "The URL that points to the SSO server instance.")
+	flag.String(SSOServerDefaultRedirectURL, "", "The URL that the SSO server should redirect to by default if the a given redirect query parameter is invalid.")
+	flag.String(SSOServerURL, "", "The URL used by the proxy to point to the SSO server instance.")
 
 	redisFlags()
 	openIDFlags()
@@ -186,10 +189,6 @@ func (c *Config) Validate() error {
 	if c.SSO.Enabled {
 		switch c.SSO.Mode {
 		case SSOModeProxy:
-			if len(c.SSO.ServerURL) == 0 {
-				return fmt.Errorf("%q cannot be empty", SSOServerURL)
-			}
-
 			_, err := url.ParseRequestURI(c.SSO.ServerURL)
 			if err != nil {
 				return fmt.Errorf("%q must be a valid url: %w", SSOServerURL, err)
@@ -198,6 +197,11 @@ func (c *Config) Validate() error {
 			if len(c.SSO.Domain) == 0 {
 				return fmt.Errorf("%q cannot be empty", SSODomain)
 			}
+
+			_, err := url.ParseRequestURI(c.SSO.ServerDefaultRedirectURL)
+			if err != nil {
+				return fmt.Errorf("%q must be a valid url: %w", SSOServerDefaultRedirectURL, err)
+			}
 		default:
 			return fmt.Errorf("%q must be one of [%q, %q]", SSOModeFlag, SSOModeServer, SSOModeProxy)
 		}

pkg/handler/error/error.go

@@ -16,6 +16,7 @@ import (
 	"github.com/nais/wonderwall/pkg/handler/templates"
 	mw "github.com/nais/wonderwall/pkg/middleware"
 	"github.com/nais/wonderwall/pkg/openid"
+	"github.com/nais/wonderwall/pkg/redirect"
 	"github.com/nais/wonderwall/pkg/router/paths"
 	urlpkg "github.com/nais/wonderwall/pkg/url"
 )
@@ -30,6 +31,7 @@ type Source interface {
 	GetCrypter() crypto.Crypter
 	GetErrorPath() string
 	GetPath(r *http.Request) string
+	GetRedirectHandler() redirect.Handler
 }
 
 type Page struct {
@@ -71,13 +73,14 @@ func (h Handler) Retry(r *http.Request, loginCookie *openid.LoginCookie) string
 		}
 	}
 
-	redirect := urlpkg.CanonicalRedirect(r)
+	redirectHandler := h.GetRedirectHandler()
+	rd := redirectHandler.Canonical(r)
 
 	if loginCookie != nil && len(loginCookie.Referer) > 0 {
-		redirect = loginCookie.Referer
+		rd = redirectHandler.Clean(r, loginCookie.Referer)
 	}
 
-	return urlpkg.LoginRelative(ingressPath, redirect)
+	return urlpkg.LoginRelative(ingressPath, rd)
 }
 
 func (h Handler) respondError(w http.ResponseWriter, r *http.Request, statusCode int, cause error, level log.Level) {

pkg/handler/handler_default.go

@@ -14,6 +14,7 @@ import (
 	"github.com/nais/wonderwall/pkg/middleware"
 	openidclient "github.com/nais/wonderwall/pkg/openid/client"
 	openidconfig "github.com/nais/wonderwall/pkg/openid/config"
+	"github.com/nais/wonderwall/pkg/redirect"
 	"github.com/nais/wonderwall/pkg/router"
 	"github.com/nais/wonderwall/pkg/session"
 )
@@ -21,16 +22,17 @@ import (
 var _ router.Source = &DefaultHandler{}
 
 type DefaultHandler struct {
-	AutoLogin     *autologin.AutoLogin
-	Client        *openidclient.Client
-	Config        *config.Config
-	CookieOptions cookie.Options
-	Crypter       crypto.Crypter
-	Ingresses     *ingress.Ingresses
-	Loginstatus   *loginstatus.Loginstatus
-	OpenidConfig  openidconfig.Config
-	Sessions      *session.Handler
-	UpstreamProxy *ReverseProxy
+	AutoLogin       *autologin.AutoLogin
+	Client          *openidclient.Client
+	Config          *config.Config
+	CookieOptions   cookie.Options
+	Crypter         crypto.Crypter
+	Ingresses       *ingress.Ingresses
+	Loginstatus     *loginstatus.Loginstatus
+	OpenidConfig    openidconfig.Config
+	RedirectHandler redirect.Handler
+	Sessions        *session.Handler
+	UpstreamProxy   *ReverseProxy
 }
 
 func NewDefaultHandler(
@@ -64,17 +66,20 @@ func NewDefaultHandler(
 		return nil, err
 	}
 
+	redirectHandler := redirect.NewDefaultHandler(ingresses)
+
 	return &DefaultHandler{
-		AutoLogin:     autoLogin,
-		Client:        openidClient,
-		Config:        cfg,
-		CookieOptions: cookieOpts,
-		Crypter:       crypter,
-		Ingresses:     ingresses,
-		Loginstatus:   loginstatusClient,
-		OpenidConfig:  openidConfig,
-		Sessions:      sessionHandler,
-		UpstreamProxy: NewReverseProxy(cfg.UpstreamHost),
+		AutoLogin:       autoLogin,
+		Client:          openidClient,
+		Config:          cfg,
+		CookieOptions:   cookieOpts,
+		Crypter:         crypter,
+		Ingresses:       ingresses,
+		Loginstatus:     loginstatusClient,
+		OpenidConfig:    openidConfig,
+		Sessions:        sessionHandler,
+		UpstreamProxy:   NewReverseProxy(cfg.UpstreamHost),
+		RedirectHandler: redirectHandler,
 	}, nil
 }
 
@@ -128,6 +133,10 @@ func (d *DefaultHandler) GetPath(r *http.Request) string {
 	return path
 }
 
+func (d *DefaultHandler) GetRedirectHandler() redirect.Handler {
+	return d.RedirectHandler
+}
+
 func (d *DefaultHandler) GetSessions() *session.Handler {
 	return d.Sessions
 }

pkg/handler/handler_sso_proxy.go

@@ -5,13 +5,10 @@ import (
 	"net/http"
 	urllib "net/url"
 
-	log "github.com/sirupsen/logrus"
-
 	"github.com/nais/wonderwall/pkg/config"
-	"github.com/nais/wonderwall/pkg/handler/url"
 	"github.com/nais/wonderwall/pkg/ingress"
-	mw "github.com/nais/wonderwall/pkg/middleware"
 	openidclient "github.com/nais/wonderwall/pkg/openid/client"
+	"github.com/nais/wonderwall/pkg/redirect"
 	"github.com/nais/wonderwall/pkg/router"
 	"github.com/nais/wonderwall/pkg/router/paths"
 )
@@ -19,9 +16,10 @@ import (
 var _ router.Source = &SSOProxyHandler{}
 
 type SSOProxyHandler struct {
-	Config       *config.Config
-	Ingresses    *ingress.Ingresses
-	SSOServerURL *urllib.URL
+	Config          *config.Config
+	Ingresses       *ingress.Ingresses
+	RedirectHandler redirect.Handler
+	SSOServerURL    *urllib.URL
 }
 
 func NewSSOProxyHandler(cfg *config.Config) (*SSOProxyHandler, error) {
@@ -30,6 +28,8 @@ func NewSSOProxyHandler(cfg *config.Config) (*SSOProxyHandler, error) {
 		return nil, err
 	}
 
+	redirectHandler := redirect.NewSSOProxyHandler(ingresses)
+
 	u, err := urllib.ParseRequestURI(cfg.SSO.ServerURL)
 	if err != nil {
 		return nil, fmt.Errorf("parsing sso server url: %w", err)
@@ -48,47 +48,15 @@ func NewSSOProxyHandler(cfg *config.Config) (*SSOProxyHandler, error) {
 	u.RawQuery = query.Encode()
 
 	return &SSOProxyHandler{
-		Config:       cfg,
-		Ingresses:    ingresses,
-		SSOServerURL: u,
+		Config:          cfg,
+		Ingresses:       ingresses,
+		SSOServerURL:    u,
+		RedirectHandler: redirectHandler,
 	}, nil
 }
 
 func (s *SSOProxyHandler) Login(w http.ResponseWriter, r *http.Request) {
-	target := *s.SSOServerURL
-	targetQuery := target.Query()
-
-	reqQuery := r.URL.Query()
-
-	if reqQuery.Has(openidclient.SecurityLevelURLParameter) {
-		targetQuery.Set(openidclient.SecurityLevelURLParameter, reqQuery.Get(openidclient.SecurityLevelURLParameter))
-	}
-
-	if reqQuery.Has(openidclient.LocaleURLParameter) {
-		targetQuery.Set(openidclient.LocaleURLParameter, reqQuery.Get(openidclient.LocaleURLParameter))
-	}
-
-	target.RawQuery = reqQuery.Encode()
-
-	redirect, err := url.Ingress(r)
-	if err != nil {
-		redirect = s.Ingresses.Single().NewURL()
-	}
-	parsedRedirect, err := urllib.ParseRequestURI(reqQuery.Get(url.RedirectURLParameter))
-	if err == nil {
-		redirect = redirect.JoinPath(parsedRedirect.Path)
-	}
-
-	ssoServerLoginURL := url.Login(&target, redirect.String())
-
-	mw.LogEntryFrom(r).
-		WithFields(log.Fields{
-			"redirect_to":          ssoServerLoginURL,
-			"redirect_after_login": redirect.String(),
-		}).
-		Info("login: redirecting to sso server")
-
-	http.Redirect(w, r, ssoServerLoginURL, http.StatusTemporaryRedirect)
+	LoginSSOProxy(s, w, r)
 }
 
 func (s *SSOProxyHandler) LoginCallback(w http.ResponseWriter, r *http.Request) {
@@ -131,3 +99,12 @@ func (s *SSOProxyHandler) ReverseProxy(w http.ResponseWriter, r *http.Request) {
 func (s *SSOProxyHandler) GetIngresses() *ingress.Ingresses {
 	return s.Ingresses
 }
+
+func (s *SSOProxyHandler) GetRedirectHandler() redirect.Handler {
+	return s.RedirectHandler
+}
+
+func (s *SSOProxyHandler) GetSSOServerURL() *urllib.URL {
+	u := *s.SSOServerURL
+	return &u
+}

pkg/handler/handler_sso_server.go

@@ -3,6 +3,7 @@ package handler
 import (
 	"net/http"
 
+	"github.com/nais/wonderwall/pkg/redirect"
 	"github.com/nais/wonderwall/pkg/router"
 )
 
@@ -12,8 +13,13 @@ type SSOServerHandler struct {
 	DefaultHandler
 }
 
-func NewSSOServerHandler(handler *DefaultHandler) *SSOServerHandler {
-	return &SSOServerHandler{DefaultHandler: *handler}
+func NewSSOServerHandler(handler *DefaultHandler) (*SSOServerHandler, error) {
+	rdHandler, err := redirect.NewSSOServerHandler(handler.Config)
+	if err != nil {
+		return nil, err
+	}
+	handler.RedirectHandler = rdHandler
+	return &SSOServerHandler{DefaultHandler: *handler}, nil
 }
 
 func (s *SSOServerHandler) ReverseProxy(w http.ResponseWriter, r *http.Request) {

pkg/handler/login.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"time"
 
 	log "github.com/sirupsen/logrus"
@@ -15,6 +16,8 @@ import (
 	logentry "github.com/nais/wonderwall/pkg/middleware"
 	"github.com/nais/wonderwall/pkg/openid"
 	openidclient "github.com/nais/wonderwall/pkg/openid/client"
+	"github.com/nais/wonderwall/pkg/redirect"
+	urlpkg "github.com/nais/wonderwall/pkg/url"
 )
 
 const (
@@ -26,9 +29,11 @@ type LoginSource interface {
 	GetCookieOptsPathAware(r *http.Request) cookie.Options
 	GetCrypter() crypto.Crypter
 	GetErrorHandler() errorhandler.Handler
+	GetRedirectHandler() redirect.Handler
 }
 
 func Login(src LoginSource, w http.ResponseWriter, r *http.Request) {
+	canonicalRedirect := src.GetRedirectHandler().Canonical(r)
 	login, err := src.GetClient().Login(r)
 	if err != nil {
 		if errors.Is(err, openidclient.ErrInvalidSecurityLevel) || errors.Is(err, openidclient.ErrInvalidLocale) {
@@ -40,19 +45,52 @@ func Login(src LoginSource, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	err = setLoginCookies(src, w, r, login.Cookie())
+	err = setLoginCookies(src, w, r, login.Cookie(canonicalRedirect))
 	if err != nil {
 		src.GetErrorHandler().InternalError(w, r, fmt.Errorf("login: setting cookie: %w", err))
 		return
 	}
 
 	fields := log.Fields{
-		"redirect_after_login": login.CanonicalRedirect(),
+		"redirect_after_login": canonicalRedirect,
 	}
-	logentry.LogEntryFrom(r).WithFields(fields).Debug("login: redirecting to identity provider")
+	logentry.LogEntryFrom(r).WithFields(fields).Info("login: redirecting to identity provider")
 	http.Redirect(w, r, login.AuthCodeURL(), http.StatusTemporaryRedirect)
 }
 
+type LoginSSOProxySource interface {
+	GetSSOServerURL() *url.URL
+	GetRedirectHandler() redirect.Handler
+}
+
+func LoginSSOProxy(src LoginSSOProxySource, w http.ResponseWriter, r *http.Request) {
+	logger := logentry.LogEntryFrom(r)
+
+	target := src.GetSSOServerURL()
+	targetQuery := target.Query()
+
+	// override default query parameters
+	reqQuery := r.URL.Query()
+	if reqQuery.Has(openidclient.SecurityLevelURLParameter) {
+		targetQuery.Set(openidclient.SecurityLevelURLParameter, reqQuery.Get(openidclient.SecurityLevelURLParameter))
+	}
+	if reqQuery.Has(openidclient.LocaleURLParameter) {
+		targetQuery.Set(openidclient.LocaleURLParameter, reqQuery.Get(openidclient.LocaleURLParameter))
+	}
+
+	target.RawQuery = reqQuery.Encode()
+
+	canonicalRedirect := src.GetRedirectHandler().Canonical(r)
+	ssoServerLoginURL := urlpkg.Login(target, canonicalRedirect)
+
+	logger.WithFields(log.Fields{
+		"redirect_to":          ssoServerLoginURL,
+		"redirect_after_login": canonicalRedirect,
+	}).Info("login: redirecting to sso server")
+
+	http.Redirect(w, r, ssoServerLoginURL, http.StatusTemporaryRedirect)
+}
+
 func setLoginCookies(src LoginSource, w http.ResponseWriter, r *http.Request, loginCookie *openid.LoginCookie) error {
 	loginCookieJson, err := json.Marshal(loginCookie)
 	if err != nil {

pkg/handler/login_callback.go

@@ -18,6 +18,7 @@ import (
 	logentry "github.com/nais/wonderwall/pkg/middleware"
 	"github.com/nais/wonderwall/pkg/openid"
 	openidclient "github.com/nais/wonderwall/pkg/openid/client"
+	"github.com/nais/wonderwall/pkg/redirect"
 	retrypkg "github.com/nais/wonderwall/pkg/retry"
 	"github.com/nais/wonderwall/pkg/session"
 )
@@ -29,6 +30,7 @@ type LoginCallbackSource interface {
 	GetCrypter() crypto.Crypter
 	GetErrorHandler() errorhandler.Handler
 	GetLoginstatus() *loginstatus.Loginstatus
+	GetRedirectHandler() redirect.Handler
 	GetSessions() *session.Handler
 	GetSessionConfig() config.Session
 }
@@ -96,9 +98,11 @@ func LoginCallback(src LoginCallbackSource, w http.ResponseWriter, r *http.Reque
 		logentry.LogEntryFrom(r).Debug("callback: successfully fetched loginstatus token")
 	}
 
-	logSuccessfulLogin(r, tokens, loginCookie.Referer)
+	rd := src.GetRedirectHandler().Clean(r, loginCookie.Referer)
+
+	logSuccessfulLogin(r, tokens, rd)
 	cookie.Clear(w, cookie.Retry, src.GetCookieOptsPathAware(r))
-	http.Redirect(w, r, loginCookie.Referer, http.StatusTemporaryRedirect)
+	http.Redirect(w, r, rd, http.StatusTemporaryRedirect)
 }
 
 func clearLoginCookies(src LogoutCallbackSource, w http.ResponseWriter, r *http.Request) {

pkg/openid/client/login.go

@@ -53,32 +53,25 @@ func NewLogin(c *Client, r *http.Request) (*Login, error) {
 		return nil, fmt.Errorf("generating auth code url: %w", err)
 	}
 
-	referer := urlpkg.CanonicalRedirect(r)
-	cookie := params.cookie(referer, callbackURL)
+	cookie := params.cookie(callbackURL)
 
 	return &Login{
-		authCodeURL:       url,
-		canonicalRedirect: referer,
-		cookie:            cookie,
-		params:            params,
+		authCodeURL: url,
+		cookie:      cookie,
+		params:      params,
 	}, nil
 }
 
 type Login struct {
-	authCodeURL       string
-	canonicalRedirect string
-	cookie            *openid.LoginCookie
-	params            *loginParameters
+	authCodeURL string
+	cookie      *openid.LoginCookie
+	params      *loginParameters
 }
 
 func (l *Login) AuthCodeURL() string {
 	return l.authCodeURL
 }
 
-func (l *Login) CanonicalRedirect() string {
-	return l.canonicalRedirect
-}
-
 func (l *Login) CodeChallenge() string {
 	return l.params.CodeChallenge
 }
@@ -87,7 +80,8 @@ func (l *Login) CodeVerifier() string {
 	return l.params.CodeVerifier
 }
 
-func (l *Login) Cookie() *openid.LoginCookie {
+func (l *Login) Cookie(canonicalRedirect string) *openid.LoginCookie {
+	l.cookie.Referer = canonicalRedirect
 	return l.cookie
 }
 
@@ -159,12 +153,11 @@ func (in *loginParameters) authCodeURL(r *http.Request, callbackURL string, logi
 	return authCodeUrl, nil
 }
 
-func (in *loginParameters) cookie(referer, redirectURI string) *openid.LoginCookie {
+func (in *loginParameters) cookie(redirectURI string) *openid.LoginCookie {
 	return &openid.LoginCookie{
 		State:        in.State,
 		Nonce:        in.Nonce,
 		CodeVerifier: in.CodeVerifier,
-		Referer:      referer,
 		RedirectURI:  redirectURI,
 	}
 }