refactor(handler/default): redirect auto-login requests instead of inlining login handler

pkg/handler/handler.go

@@ -25,6 +25,8 @@ type Handler struct {
 	OpenIDConfig  openidconfig.Config
 	Provider      provider.Provider
 	Sessions      session.Store
+
+	path string
 }
 
 func NewHandler(
@@ -54,5 +56,10 @@ func NewHandler(
 		OpenIDConfig:  openidConfig,
 		Provider:      openidProvider,
 		Sessions:      sessionStore,
+		path:          config.ParseIngress(cfg.Ingress),
 	}, nil
 }
+
+func (h *Handler) Path() string {
+	return h.path
+}

pkg/handler/handler_default.go

@@ -6,6 +6,7 @@ import (
 
 	logentry "github.com/nais/wonderwall/pkg/middleware"
 	"github.com/nais/wonderwall/pkg/session"
+	urlpkg "github.com/nais/wonderwall/pkg/url"
 )
 
 // Default proxies all requests upstream.
@@ -31,8 +32,10 @@ func (h *Handler) Default(w http.ResponseWriter, r *http.Request) {
 	if h.AutoLogin.NeedsLogin(r, isAuthenticated) {
 		logger.Debug("default: auto-login is enabled; request does not match skippable path")
 
-		r.Header.Add("Referer", r.URL.String())
-		h.Login(w, r)
+		redirectTarget := r.URL.String()
+		loginUrl := urlpkg.LoginURL(h.Path(), redirectTarget)
+
+		http.Redirect(w, r, loginUrl, http.StatusTemporaryRedirect)
 		return
 	}
 

pkg/handler/handler_test.go

@@ -187,6 +187,14 @@ func TestHandler_Default(t *testing.T) {
 		resp := get(t, rpClient, target)
 		assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
 
+		// redirect should point to local login endpoint
+		loginLocation := resp.Location
+		assert.Equal(t, idp.RelyingPartyServer.URL+"/oauth2/login?redirect=/", loginLocation.String())
+
+		// follow redirect to local login endpoint
+		resp = get(t, rpClient, loginLocation.String())
+		assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
+
 		// redirect should point to identity provider
 		authorizeLocation := resp.Location
 

pkg/middleware/logentry.go

@@ -22,7 +22,7 @@ func LogEntryHandler(provider string) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		fn := func(w http.ResponseWriter, r *http.Request) {
 			entry := logger.NewLogEntry(r)
-			entry.WithRequestLogFields(r).Infof("request")
+			entry.WithRequestLogFields(r).Infof("%s - %s", r.Method, r.URL.Path)
 
 			ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor)
 			t1 := time.Now()

pkg/router/router.go

@@ -4,7 +4,6 @@ import (
 	"github.com/go-chi/chi/v5"
 	chi_middleware "github.com/go-chi/chi/v5/middleware"
 
-	"github.com/nais/wonderwall/pkg/config"
 	"github.com/nais/wonderwall/pkg/handler"
 	"github.com/nais/wonderwall/pkg/middleware"
 	"github.com/nais/wonderwall/pkg/router/paths"
@@ -16,7 +15,7 @@ func New(handler *handler.Handler) chi.Router {
 	r.Use(chi_middleware.Recoverer)
 	prometheusMiddleware := middleware.NewPrometheusMiddleware("wonderwall", handler.OpenIDConfig.Provider().Name())
 
-	prefix := config.ParseIngress(handler.Config.Ingress)
+	prefix := handler.Path()
 
 	r.Group(func(r chi.Router) {
 		r.Use(middleware.LogEntryHandler(handler.OpenIDConfig.Provider().Name()))

pkg/url/url.go

@@ -59,9 +59,14 @@ func Retry(r *http.Request, ingress string, loginCookie *openid.LoginCookie) str
 		redirect = loginCookie.Referer
 	}
 
-	retryURI = fmt.Sprintf(prefix + paths.OAuth2 + paths.Login)
-	retryURI = retryURI + fmt.Sprintf("?%s=%s", RedirectURLParameter, redirect)
-	return retryURI
+	return LoginURL(prefix, redirect)
+}
+
+func LoginURL(prefix, redirectTarget string) string {
+	loginPath := prefix + paths.OAuth2 + paths.Login
+	redirectParam := fmt.Sprintf("?%s=%s", RedirectURLParameter, redirectTarget)
+
+	return loginPath + redirectParam
 }
 
 func defaultRedirectURL(ingress string) string {

pkg/url/url_test.go

@@ -299,3 +299,42 @@ func TestRetry(t *testing.T) {
 		})
 	}
 }
+
+func TestLoginURL(t *testing.T) {
+	for _, test := range []struct {
+		name           string
+		prefix         string
+		redirectTarget string
+		want           string
+	}{
+		{
+			name:           "no prefix",
+			prefix:         "",
+			redirectTarget: "https://test.example.com?some=param&other=param2",
+			want:           "/oauth2/login?redirect=https://test.example.com?some=param&other=param2",
+		},
+		{
+			name:           "with prefix",
+			prefix:         "/path",
+			redirectTarget: "https://test.example.com?some=param&other=param2",
+			want:           "/path/oauth2/login?redirect=https://test.example.com?some=param&other=param2",
+		},
+		{
+			name:           "we need to go deeper",
+			prefix:         "/deeper/path",
+			redirectTarget: "https://test.example.com?some=param&other=param2",
+			want:           "/deeper/path/oauth2/login?redirect=https://test.example.com?some=param&other=param2",
+		},
+		{
+			name:           "relative target",
+			prefix:         "",
+			redirectTarget: "/path?some=param&other=param2",
+			want:           "/oauth2/login?redirect=/path?some=param&other=param2",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			loginUrl := urlpkg.LoginURL(test.prefix, test.redirectTarget)
+			assert.Equal(t, test.want, loginUrl)
+		})
+	}
+}