Просмотр исходного кода

fix(ui): keep logged-out preferences in a cookie instead of the session

Logging out cleared the user from the web session but kept the row, with
the user's language and theme copied into it so the login page kept
using them. That made logged-out sessions indistinguishable from
sessions that never signed in, so anonymous sessions couldn't be
expired any sooner than signed-in ones.

Logout now deletes the session and stores the language and theme in a
MinifluxPreferences cookie. When a new session is created for a public
route, it's seeded from that cookie, after checking the values against
the known languages and themes. Anonymous sessions now only hold
short-lived state (CSRF token, OAuth2 and WebAuthn flows, etc), so
cleanup removes them after 24 hours.
Dave Marquard 1 неделя назад
Родитель
Сommit
030ecebf91

+ 8 - 0
internal/model/web_session.go

@@ -201,6 +201,14 @@ func (s *WebSession) ConsumeMessages() (string, string) {
 	return successMessage, errorMessage
 }
 
+// SeedPreferences sets the language and theme of a session that hasn't been
+// persisted yet. Unlike SetLanguage and SetTheme, it doesn't mark the session
+// dirty, since the values are saved when the session is created.
+func (s *WebSession) SeedPreferences(language, theme string) {
+	s.state.Language = language
+	s.state.Theme = theme
+}
+
 // SetLanguage updates the language.
 func (s *WebSession) SetLanguage(language string) {
 	s.dirty = true

+ 15 - 0
internal/model/web_session_test.go

@@ -250,6 +250,21 @@ func TestWebSession_LanguageAndThemeDefaults(t *testing.T) {
 	}
 }
 
+func TestWebSession_SeedPreferences(t *testing.T) {
+	session, _ := NewWebSession("", "")
+	session.SeedPreferences("de_DE", "light_sans_serif")
+
+	if got := session.Language(); got != "de_DE" {
+		t.Errorf("Language() = %q, want %q", got, "de_DE")
+	}
+	if got := session.Theme(); got != "light_sans_serif" {
+		t.Errorf("Theme() = %q, want %q", got, "light_sans_serif")
+	}
+	if session.IsDirty() {
+		t.Error("SeedPreferences must not mark the session dirty")
+	}
+}
+
 func TestWebSession_OAuth2FlowLifecycle(t *testing.T) {
 	session := &WebSession{}
 

+ 11 - 3
internal/storage/web_session.go

@@ -226,18 +226,26 @@ func (s *Storage) RemoveUserWebSession(userID int64, sessionID string) error {
 	return nil
 }
 
-// CleanOldWebSessions removes web sessions older than the specified interval (24h minimum).
+// anonymousWebSessionLifetime is how long a web session that never signed in
+// is kept. These are created for every visitor of the login page, including
+// crawlers and scanners that never send the cookie back.
+const anonymousWebSessionLifetime = 24 * time.Hour
+
+// CleanOldWebSessions removes web sessions older than the specified interval (24h minimum),
+// and anonymous web sessions older than anonymousWebSessionLifetime.
 func (s *Storage) CleanOldWebSessions(interval time.Duration) (int64, error) {
 	query := `
 		DELETE FROM
 			web_sessions
 		WHERE
-			created_at < now() - $1::interval
+			created_at < now() - $1::interval OR
+			(user_id IS NULL AND created_at < now() - $2::interval)
 	`
 
 	days := max(int(interval/(24*time.Hour)), 1)
+	anonymousHours := int(anonymousWebSessionLifetime / time.Hour)
 
-	result, err := s.db.Exec(query, fmt.Sprintf("%d days", days))
+	result, err := s.db.Exec(query, fmt.Sprintf("%d days", days), fmt.Sprintf("%d hours", anonymousHours))
 	if err != nil {
 		return 0, fmt.Errorf(`store: unable to clean old web sessions: %v`, err)
 	}

+ 54 - 7
internal/ui/auth.go

@@ -6,16 +6,22 @@ package ui // import "miniflux.app/v2/internal/ui"
 import (
 	"context"
 	"net/http"
+	"strings"
 	"time"
 
 	"miniflux.app/v2/internal/config"
 	"miniflux.app/v2/internal/http/request"
+	"miniflux.app/v2/internal/locale"
 	"miniflux.app/v2/internal/model"
 	"miniflux.app/v2/internal/oauth2"
 	"miniflux.app/v2/internal/storage"
 )
 
-const sessionCookieName = "MinifluxSessionID"
+const (
+	sessionCookieName         = "MinifluxSessionID"
+	preferencesCookieName     = "MinifluxPreferences"
+	preferencesCookieLifetime = 365 * 24 * time.Hour
+)
 
 // authenticateWebSession binds the current browser session to the given user,
 // rotates its identifier and secret, and refreshes the client cookie.
@@ -36,15 +42,10 @@ func authenticateWebSession(w http.ResponseWriter, r *http.Request, store *stora
 // security attributes used by miniflux (HttpOnly, SameSite=Lax, Secure
 // when HTTPS).
 func setSessionCookie(w http.ResponseWriter, session *model.WebSession, secret string) {
-	path := config.Opts.BasePath()
-	if path == "" {
-		path = "/"
-	}
-
 	http.SetCookie(w, &http.Cookie{
 		Name:     sessionCookieName,
 		Value:    session.ID + "." + secret,
-		Path:     path,
+		Path:     cookiePath(),
 		Secure:   config.Opts.HTTPS(),
 		HttpOnly: true,
 		Expires:  time.Now().Add(config.Opts.CleanupRemoveSessionsInterval()),
@@ -52,6 +53,52 @@ func setSessionCookie(w http.ResponseWriter, session *model.WebSession, secret s
 	})
 }
 
+// setPreferencesCookie remembers the language and theme of a user who logs
+// out, so the login page keeps using them without keeping a session around.
+func setPreferencesCookie(w http.ResponseWriter, language, theme string) {
+	http.SetCookie(w, &http.Cookie{
+		Name:     preferencesCookieName,
+		Value:    language + "." + theme,
+		Path:     cookiePath(),
+		Secure:   config.Opts.HTTPS(),
+		HttpOnly: true,
+		Expires:  time.Now().Add(preferencesCookieLifetime),
+		SameSite: http.SameSiteLaxMode,
+	})
+}
+
+// preferencesFromCookie returns the language and theme stored by
+// setPreferencesCookie. It reports false when the cookie is missing or holds
+// an unknown language or theme.
+func preferencesFromCookie(r *http.Request) (language, theme string, ok bool) {
+	cookie, err := r.Cookie(preferencesCookieName)
+	if err != nil {
+		return "", "", false
+	}
+
+	language, theme, ok = strings.Cut(cookie.Value, ".")
+	if !ok {
+		return "", "", false
+	}
+
+	if _, found := locale.AvailableLanguages[language]; !found {
+		return "", "", false
+	}
+
+	if _, found := model.Themes()[theme]; !found {
+		return "", "", false
+	}
+
+	return language, theme, true
+}
+
+func cookiePath() string {
+	if path := config.Opts.BasePath(); path != "" {
+		return path
+	}
+	return "/"
+}
+
 func getOAuth2Manager(ctx context.Context) *oauth2.Manager {
 	return oauth2.NewManager(
 		ctx,

+ 102 - 0
internal/ui/auth_test.go

@@ -0,0 +1,102 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package ui // import "miniflux.app/v2/internal/ui"
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"miniflux.app/v2/internal/config"
+)
+
+func configureTestOptions(t *testing.T, env map[string]string) {
+	t.Helper()
+
+	for key, value := range env {
+		t.Setenv(key, value)
+	}
+
+	parsedOptions, err := config.NewConfigParser().ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Unable to configure test options: %v`, err)
+	}
+
+	previousOptions := config.Opts
+	config.Opts = parsedOptions
+	t.Cleanup(func() {
+		config.Opts = previousOptions
+	})
+}
+
+func TestSetPreferencesCookie(t *testing.T) {
+	scenarios := []struct {
+		name       string
+		env        map[string]string
+		wantPath   string
+		wantSecure bool
+	}{
+		{"defaults", map[string]string{}, "/", false},
+		{"base path over HTTPS", map[string]string{"BASE_URL": "https://example.org/reader", "HTTPS": "1"}, "/reader", true},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			configureTestOptions(t, scenario.env)
+
+			w := httptest.NewRecorder()
+			setPreferencesCookie(w, "de_DE", "dark_serif")
+
+			cookies := w.Result().Cookies()
+			if len(cookies) != 1 {
+				t.Fatalf(`Expected one cookie, got %d`, len(cookies))
+			}
+
+			cookie := cookies[0]
+			if cookie.Name != preferencesCookieName || cookie.Value != "de_DE.dark_serif" {
+				t.Errorf(`Unexpected cookie: %s=%s`, cookie.Name, cookie.Value)
+			}
+			if cookie.Path != scenario.wantPath {
+				t.Errorf(`Unexpected path, got %q, want %q`, cookie.Path, scenario.wantPath)
+			}
+			if cookie.Secure != scenario.wantSecure {
+				t.Errorf(`Unexpected Secure attribute, got %v, want %v`, cookie.Secure, scenario.wantSecure)
+			}
+			if !cookie.HttpOnly || cookie.SameSite != http.SameSiteLaxMode {
+				t.Errorf(`Expected an HttpOnly cookie with SameSite=Lax`)
+			}
+		})
+	}
+}
+
+func TestPreferencesFromCookie(t *testing.T) {
+	scenarios := []struct {
+		name         string
+		value        string
+		wantLanguage string
+		wantTheme    string
+		wantOK       bool
+	}{
+		{"valid", "de_DE.dark_serif", "de_DE", "dark_serif", true},
+		{"unknown language", "xx_XX.dark_serif", "", "", false},
+		{"unknown theme", "de_DE.neon", "", "", false},
+		{"missing separator", "de_DE", "", "", false},
+		{"empty", "", "", "", false},
+		{"no cookie", "", "", "", false},
+	}
+
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			r := httptest.NewRequest(http.MethodGet, "/", nil)
+			if scenario.name != "no cookie" {
+				r.AddCookie(&http.Cookie{Name: preferencesCookieName, Value: scenario.value})
+			}
+
+			language, theme, ok := preferencesFromCookie(r)
+			if language != scenario.wantLanguage || theme != scenario.wantTheme || ok != scenario.wantOK {
+				t.Errorf(`Got (%q, %q, %v), want (%q, %q, %v)`, language, theme, ok, scenario.wantLanguage, scenario.wantTheme, scenario.wantOK)
+			}
+		})
+	}
+}

+ 6 - 3
internal/ui/logout.go

@@ -18,10 +18,13 @@ func (h *handler) logout(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if s := request.WebSession(r); s != nil {
-		s.ClearUser()
-		s.SetLanguage(user.Language)
-		s.SetTheme(user.Theme)
+		if err := h.store.RemoveUserWebSession(user.ID, s.ID); err != nil {
+			response.HTMLServerError(w, r, err)
+			return
+		}
 	}
 
+	setPreferencesCookie(w, user.Language, user.Theme)
+
 	response.HTMLRedirect(w, r, h.routePath("/"))
 }

+ 3 - 0
internal/ui/web_session_middleware.go

@@ -39,6 +39,9 @@ func (m *webSessionMiddleware) handle(next http.Handler) http.Handler {
 		if session == nil {
 			var secret string
 			session, secret = model.NewWebSession(r.UserAgent(), request.ClientIP(r))
+			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