fix(ui): only persist web sessions on routes that start a flow

Unauthenticated requests to any public route inserted a row in
web_sessions, even when nothing needed to be remembered. Shared entries,
the web manifest and the offline page only read the language and theme,
and the login form submission, WebAuthn login finish and OAuth2 callback
can only succeed with a session started by a previous request. Unknown
methods and disabled features also inserted a row before returning 404
or 405.

Only the login page, the WebAuthn login challenge and the OAuth2
redirect now persist a session for visitors without one. Other public
routes get an in-memory session seeded from the preferences cookie,
without setting a session cookie.

Also remove /healthcheck from the public routes, since it is served
before the UI handler.

internal/ui/routes.go

@@ -7,6 +7,8 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
+
+	"miniflux.app/v2/internal/config"
 )
 
 // isStaticAssetRoute checks if the request path corresponds to a static
@@ -37,8 +39,7 @@ func isPublicRoute(r *http.Request) bool {
 	path := r.URL.Path
 
 	switch path {
-	case "/", "/login", "/manifest.json",
-		"/healthcheck", "/offline",
+	case "/", "/login", "/manifest.json", "/offline",
 		"/webauthn/login/begin", "/webauthn/login/finish":
 		return true
 	}
@@ -47,6 +48,28 @@ func isPublicRoute(r *http.Request) bool {
 		strings.HasPrefix(path, "/share/")
 }
 
+// isSessionStartRoute checks if the request starts state that a later request
+// must find (login CSRF token, WebAuthn challenge, OAuth2 state), so a visitor
+// without a session needs one persisted.
+func isSessionStartRoute(r *http.Request) bool {
+	if r.Method != http.MethodGet && r.Method != http.MethodHead {
+		return false
+	}
+
+	path := r.URL.Path
+
+	switch {
+	case path == "/":
+		return true
+	case path == "/webauthn/login/begin":
+		return config.Opts.WebAuthn()
+	case strings.HasPrefix(path, "/oauth2/") && strings.HasSuffix(path, "/redirect"):
+		return config.Opts.OAuth2Provider() != ""
+	}
+
+	return false
+}
+
 // loginRedirectURL builds the login page URL with the given request URI
 // stored in the redirect_url query parameter.
 func loginRedirectURL(basePath, requestURI string) string {

internal/ui/routes_test.go

@@ -35,3 +35,38 @@ func TestIsStaticAssetRoute(t *testing.T) {
 		}
 	}
 }
+
+func TestIsSessionStartRoute(t *testing.T) {
+	scenarios := []struct {
+		name     string
+		env      map[string]string
+		method   string
+		path     string
+		expected bool
+	}{
+		{"login page", nil, http.MethodGet, "/", true},
+		{"login page head", nil, http.MethodHead, "/", true},
+		{"login page post", nil, http.MethodPost, "/", false},
+		{"login form submission", nil, http.MethodPost, "/login", false},
+		{"shared entry", nil, http.MethodGet, "/share/code", false},
+		{"web manifest", nil, http.MethodGet, "/manifest.json", false},
+		{"offline page", nil, http.MethodGet, "/offline", false},
+		{"webauthn begin disabled", nil, http.MethodGet, "/webauthn/login/begin", false},
+		{"webauthn begin enabled", map[string]string{"WEBAUTHN": "1"}, http.MethodGet, "/webauthn/login/begin", true},
+		{"webauthn finish", map[string]string{"WEBAUTHN": "1"}, http.MethodPost, "/webauthn/login/finish", false},
+		{"oauth2 redirect disabled", nil, http.MethodGet, "/oauth2/google/redirect", false},
+		{"oauth2 redirect enabled", map[string]string{"OAUTH2_PROVIDER": "google"}, http.MethodGet, "/oauth2/google/redirect", true},
+		{"oauth2 callback", map[string]string{"OAUTH2_PROVIDER": "google"}, http.MethodGet, "/oauth2/google/callback", false},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			configureTestOptions(t, scenario.env)
+
+			r := httptest.NewRequest(scenario.method, scenario.path, nil)
+			if result := isSessionStartRoute(r); result != scenario.expected {
+				t.Errorf(`Unexpected result for %s %q, got %v instead of %v`, scenario.method, scenario.path, result, scenario.expected)
+			}
+		})
+	}
+}

internal/ui/web_session_middleware.go

@@ -36,6 +36,7 @@ func (m *webSessionMiddleware) handle(next http.Handler) http.Handler {
 			response.HTMLServerError(w, r, err)
 			return
 		}
+		persisted := session != nil
 		if session == nil {
 			// Visitors without a session are sent to the login page and have
 			// nothing to remember yet, so don't persist a session for them.
@@ -49,11 +50,17 @@ func (m *webSessionMiddleware) handle(next http.Handler) http.Handler {
 			if language, theme, ok := preferencesFromCookie(r); ok {
 				session.SeedPreferences(language, theme)
 			}
-			if err := m.store.CreateWebSession(session); err != nil {
-				response.HTMLServerError(w, r, err)
-				return
+
+			// Other public routes either only render pages or expect a session
+			// started earlier, so an in-memory session is enough for them.
+			if isSessionStartRoute(r) {
+				if err := m.store.CreateWebSession(session); err != nil {
+					response.HTMLServerError(w, r, err)
+					return
+				}
+				setSessionCookie(w, session, secret)
+				persisted = true
 			}
-			setSessionCookie(w, session, secret)
 		}
 
 		ctx := context.WithValue(r.Context(), request.WebSessionContextKey, session)
@@ -66,7 +73,7 @@ func (m *webSessionMiddleware) handle(next http.Handler) http.Handler {
 
 		next.ServeHTTP(w, r)
 
-		if session.IsDirty() {
+		if persisted && session.IsDirty() {
 			if err := m.store.UpdateWebSession(session); err != nil {
 				slog.Error("Unable to persist web session changes",
 					slog.String("session_id", session.ID),

internal/ui/web_session_middleware_test.go

@@ -7,6 +7,8 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+
+	"miniflux.app/v2/internal/http/request"
 )
 
 func TestWebSessionMiddlewareRedirectsWithoutCreatingSession(t *testing.T) {
@@ -67,3 +69,55 @@ func TestWebSessionMiddlewareSkipsStaticAssets(t *testing.T) {
 		t.Fatalf(`No session cookie should be set, got: %q`, cookie)
 	}
 }
+
+func TestWebSessionMiddlewareUsesInMemorySessionForPublicRoutes(t *testing.T) {
+	scenarios := []struct {
+		method string
+		path   string
+	}{
+		{http.MethodGet, "/share/code"},
+		{http.MethodGet, "/manifest.json"},
+		{http.MethodGet, "/offline"},
+		{http.MethodGet, "/oauth2/google/callback"},
+		{http.MethodPost, "/webauthn/login/finish"},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.method+" "+scenario.path, func(t *testing.T) {
+			configureTestOptions(t, map[string]string{"WEBAUTHN": "1", "OAUTH2_PROVIDER": "google"})
+
+			// A nil store makes the test fail if the middleware tries to persist a session.
+			middleware := newWebSessionMiddleware("", nil)
+
+			called := false
+			handler := middleware.handle(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				called = true
+
+				session := request.WebSession(r)
+				if session == nil {
+					t.Fatal(`A web session should be available in the request context`)
+				}
+
+				if session.Language() != "fr_FR" || session.Theme() != "dark_serif" {
+					t.Errorf(`Preferences should be seeded from the cookie, got %q and %q`, session.Language(), session.Theme())
+				}
+
+				// Changes to an in-memory session must not be persisted either.
+				session.SetErrorMessage("error")
+			}))
+
+			r := httptest.NewRequest(scenario.method, scenario.path, nil)
+			r.AddCookie(&http.Cookie{Name: preferencesCookieName, Value: "fr_FR.dark_serif"})
+			w := httptest.NewRecorder()
+			handler.ServeHTTP(w, r)
+
+			if !called {
+				t.Fatal(`The next handler should be called for public routes`)
+			}
+
+			if cookie := w.Header().Get("Set-Cookie"); cookie != "" {
+				t.Fatalf(`No session cookie should be set, got: %q`, cookie)
+			}
+		})
+	}
+}