From 87809b234463ebcd06b8cbffb07d82ddb37af13e Mon Sep 17 00:00:00 2001 From: Alexander Klein Date: Wed, 16 Sep 2026 05:36:36 +0200 Subject: [PATCH] Preserve safe return paths through sign-in --- doc/ideen.md | 1 + internal/web/auth.go | 13 +++++- internal/web/flows_test.go | 56 ++++++++++++++++++++++-- internal/web/helpers.go | 2 +- internal/web/middleware.go | 14 +++++- internal/web/paths.go | 12 +++++ internal/web/paths_test.go | 23 ++++++++++ internal/web/templates/pages/signin.tmpl | 1 + 8 files changed, 116 insertions(+), 6 deletions(-) diff --git a/doc/ideen.md b/doc/ideen.md index 21678ac..ea3c418 100644 --- a/doc/ideen.md +++ b/doc/ideen.md @@ -1,6 +1,7 @@ # Muss # Demnächst und konkret +OpenApi einbauen # Vielleicht - Detailansicht und Bearbeitenansicht trennen? diff --git a/internal/web/auth.go b/internal/web/auth.go index 82f8393..8a3f020 100644 --- a/internal/web/auth.go +++ b/internal/web/auth.go @@ -15,6 +15,7 @@ type signInForm struct { Email string `form:"email"` Password string `form:"password"` RememberEmail bool `form:"remember_email"` + ReturnTo string `form:"return_to"` Errors map[string]string Message string } @@ -30,12 +31,17 @@ type activationForm struct { } func (app *application) signIn(w http.ResponseWriter, r *http.Request) { + returnTo := safeReturnPath(r.URL.Query().Get("return_to")) if app.isAuthenticated(r) { + if returnTo != "" { + http.Redirect(w, r, returnTo, http.StatusSeeOther) + return + } http.Redirect(w, r, app.authenticatedLandingPage(r), http.StatusSeeOther) return } data := app.newTemplateData(r) - form := signInForm{Errors: make(map[string]string)} + form := signInForm{ReturnTo: returnTo, Errors: make(map[string]string)} if cookie, err := r.Cookie("gardomatic_remembered_email"); err == nil { if decoded, decodeErr := base64.RawURLEncoding.DecodeString(cookie.Value); decodeErr == nil { form.Email, form.RememberEmail = string(decoded), true @@ -52,6 +58,7 @@ func (app *application) signInPost(w http.ResponseWriter, r *http.Request) { return } form.Email = strings.TrimSpace(form.Email) + form.ReturnTo = safeReturnPath(form.ReturnTo) form.Errors = make(map[string]string) if form.Email == "" { form.Errors["email"] = "E-Mail-Adresse ist erforderlich." @@ -99,6 +106,10 @@ func (app *application) signInPost(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, webPath("activate"), http.StatusSeeOther) return } + if form.ReturnTo != "" { + http.Redirect(w, r, form.ReturnTo, http.StatusSeeOther) + return + } http.Redirect(w, r, pathWithQuery(webPath("gardens"), "auto", 1), http.StatusSeeOther) } diff --git a/internal/web/flows_test.go b/internal/web/flows_test.go index c2b1532..e31efb4 100644 --- a/internal/web/flows_test.go +++ b/internal/web/flows_test.go @@ -272,8 +272,32 @@ func TestProtectedPageRedirectsWithoutSession(t *testing.T) { if response.Code != http.StatusSeeOther { t.Fatalf("status: got %d, want %d", response.Code, http.StatusSeeOther) } - if location := response.Header().Get("Location"); location != "/login" { - t.Errorf("Location: got %q, want %q", location, "/login") + if location := response.Header().Get("Location"); location != "/login?return_to=%2Fgardens" { + t.Errorf("Location: got %q, want login with return path", location) + } +} + +func TestProtectedInvitationRedirectPreservesToken(t *testing.T) { + apiHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v1/session" { + w.WriteHeader(http.StatusUnauthorized) + _, _ = w.Write([]byte(`{"error":"you must be authenticated"}`)) + return + } + http.NotFound(w, r) + }) + app := newAPIBackedTestApplication(t, apiHandler) + request := httptest.NewRequest(http.MethodGet, "/invite?token=garden-invite-token", nil) + response := httptest.NewRecorder() + + app.routes().ServeHTTP(response, request) + + if response.Code != http.StatusSeeOther { + t.Fatalf("status: got %d, want %d", response.Code, http.StatusSeeOther) + } + want := "/login?return_to=%2Finvite%3Ftoken%3Dgarden-invite-token" + if location := response.Header().Get("Location"); location != want { + t.Errorf("Location: got %q, want %q", location, want) } } @@ -288,7 +312,7 @@ func TestSignInForwardsAPISessionCookie(t *testing.T) { _, _ = w.Write([]byte(`{"user":{"id":7,"name":"Alice","activated":true}}`)) }) app := newAPIBackedTestApplication(t, apiHandler) - form := url.Values{"email": {"alice@example.com"}, "password": {"correct horse battery staple"}} + form := url.Values{"email": {"alice@example.com"}, "password": {"correct horse battery staple"}, "return_to": {"/invite?token=garden-invite-token"}} request := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(form.Encode())) request.Header.Set("Content-Type", "application/x-www-form-urlencoded") request = request.WithContext(client.NewContext(request.Context(), app.apiClient)) @@ -302,6 +326,32 @@ func TestSignInForwardsAPISessionCookie(t *testing.T) { if cookies := response.Result().Cookies(); len(cookies) != 1 || cookies[0].Value != "new-session" { t.Fatalf("forwarded cookies: got %+v", cookies) } + if location := response.Header().Get("Location"); location != "/invite?token=garden-invite-token" { + t.Errorf("Location: got %q, want invitation URL", location) + } +} + +func TestSignInRejectsExternalReturnURL(t *testing.T) { + apiHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/v1/session" { + http.NotFound(w, r) + return + } + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"user":{"id":7,"name":"Alice","activated":true}}`)) + }) + app := newAPIBackedTestApplication(t, apiHandler) + form := url.Values{"email": {"alice@example.com"}, "password": {"correct horse battery staple"}, "return_to": {"https://example.com/phishing"}} + request := httptest.NewRequest(http.MethodPost, "/login", strings.NewReader(form.Encode())) + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + request = request.WithContext(client.NewContext(request.Context(), app.apiClient)) + response := httptest.NewRecorder() + + app.signInPost(response, request) + + if location := response.Header().Get("Location"); location != "/gardens?auto=1" { + t.Errorf("Location: got %q, want default landing page", location) + } } func TestInactiveSignInRedirectsToActivation(t *testing.T) { diff --git a/internal/web/helpers.go b/internal/web/helpers.go index edc98d4..79e2418 100644 --- a/internal/web/helpers.go +++ b/internal/web/helpers.go @@ -184,7 +184,7 @@ func (app *application) handleAPIError(w http.ResponseWriter, r *http.Request, e if errors.As(err, &apiError) { switch apiError.StatusCode { case http.StatusUnauthorized: - http.Redirect(w, r, webPath("login"), http.StatusSeeOther) + http.Redirect(w, r, loginPathForRequest(r), http.StatusSeeOther) return case http.StatusForbidden: if user, ok := userFromContext(r.Context()); ok && !user.Activated { diff --git a/internal/web/middleware.go b/internal/web/middleware.go index 505d640..3bea972 100644 --- a/internal/web/middleware.go +++ b/internal/web/middleware.go @@ -43,7 +43,7 @@ func (app *application) recoverPanic(next http.Handler) http.Handler { func (app *application) requireAuthentication(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if !app.isAuthenticated(r) { - http.Redirect(w, r, webPath("login"), http.StatusSeeOther) + http.Redirect(w, r, loginPathForRequest(r), http.StatusSeeOther) return } w.Header().Set("Cache-Control", "no-store") @@ -51,6 +51,18 @@ func (app *application) requireAuthentication(next http.Handler) http.Handler { }) } +func loginPathForRequest(r *http.Request) string { + loginPath := webPath("login") + if r.Method != http.MethodGet { + return loginPath + } + returnTo := safeReturnPath(r.URL.RequestURI()) + if returnTo == "" { + return loginPath + } + return pathWithQuery(loginPath, "return_to", returnTo) +} + func (app *application) requireActivatedUser(next http.Handler) http.Handler { return app.requireAuthentication(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { user, ok := userFromContext(r.Context()) diff --git a/internal/web/paths.go b/internal/web/paths.go index 96b5a07..eeb3cfe 100644 --- a/internal/web/paths.go +++ b/internal/web/paths.go @@ -141,3 +141,15 @@ func pathWithQuery(path string, pairs ...any) string { } return path + "?" + values.Encode() } + +func safeReturnPath(value string) string { + value = strings.TrimSpace(value) + if value == "" { + return "" + } + target, err := url.Parse(value) + if err != nil || target.IsAbs() || target.Host != "" || !strings.HasPrefix(target.Path, "/") || strings.HasPrefix(target.Path, "//") || strings.Contains(target.Path, `\`) { + return "" + } + return target.RequestURI() +} diff --git a/internal/web/paths_test.go b/internal/web/paths_test.go index 26667f1..cad1109 100644 --- a/internal/web/paths_test.go +++ b/internal/web/paths_test.go @@ -33,6 +33,29 @@ func TestPathWithQueryEncodesValues(t *testing.T) { } } +func TestSafeReturnPath(t *testing.T) { + tests := []struct { + name string + value string + want string + }{ + {name: "local path", value: "/invite?token=abc", want: "/invite?token=abc"}, + {name: "absolute URL", value: "https://example.com/phishing"}, + {name: "scheme relative URL", value: "//example.com/phishing"}, + {name: "backslash", value: `/\\example.com/phishing`}, + {name: "encoded backslash", value: `/%5C%5Cexample.com/phishing`}, + {name: "encoded leading slashes", value: `/%2F%2Fexample.com/phishing`}, + {name: "relative path", value: "invite?token=abc"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := safeReturnPath(test.value); got != test.want { + t.Errorf("safeReturnPath(%q): got %q, want %q", test.value, got, test.want) + } + }) + } +} + func TestGardenAwareAdminPaths(t *testing.T) { garden := &client.Garden{ID: 3} if got := gardenAwarePath(webPath("admin.role.new"), garden); got != "/admin/roles/new?garden=3" { diff --git a/internal/web/templates/pages/signin.tmpl b/internal/web/templates/pages/signin.tmpl index 8777168..49e2036 100644 --- a/internal/web/templates/pages/signin.tmpl +++ b/internal/web/templates/pages/signin.tmpl @@ -7,6 +7,7 @@ {{with $form.Message}}

{{.}}

{{end}}
+ {{with $form.ReturnTo}}{{end}} {{with index $form.Errors "email"}}

{{.}}

{{end}}