diff --git a/internal/web/session/session.go b/internal/web/session/session.go index ef2f07afa..e29a8d406 100644 --- a/internal/web/session/session.go +++ b/internal/web/session/session.go @@ -29,6 +29,9 @@ func SetLoginUser(c *gin.Context, user *model.User) error { return nil } s := sessions.Default(c) + // Anyone can mint the anonymous session this request carried and plant it in a browser, + // so nothing in it, the CSRF token above all, outlives the privilege change. + s.Clear() s.Set(loginUserKey, user.Id) s.Set(loginEpochKey, user.LoginEpoch) return s.Save() diff --git a/internal/web/session/session_test.go b/internal/web/session/session_test.go index ca7117956..870cb3509 100644 --- a/internal/web/session/session_test.go +++ b/internal/web/session/session_test.go @@ -45,3 +45,33 @@ func TestSessionUserIDSupportsLegacyUserPayload(t *testing.T) { t.Fatalf("legacy pointer session payload resolved to (%d, %v), want (12, true)", id, ok) } } + +// Anyone can fetch an anonymous session with its CSRF token and plant that cookie in a +// browser from a sibling host; the token must not survive the login it was planted for. +func TestSetLoginUserReplacesThePreLoginCSRFToken(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + router.Use(sessions.Sessions(sessionCookieName, cookie.NewStore([]byte("01234567890123456789012345678901")))) + var planted, afterLogin string + router.GET("/csrf", func(c *gin.Context) { + planted, _ = EnsureCSRFToken(c) + c.Status(http.StatusNoContent) + }) + router.GET("/login", func(c *gin.Context) { + if err := SetLoginUser(c, &model.User{Id: 7}); err != nil { + t.Fatal(err) + } + afterLogin, _ = EnsureCSRFToken(c) + c.Status(http.StatusNoContent) + }) + first := httptest.NewRecorder() + router.ServeHTTP(first, httptest.NewRequest(http.MethodGet, "/csrf", nil)) + req := httptest.NewRequest(http.MethodGet, "/login", nil) + for _, cookie := range first.Result().Cookies() { + req.AddCookie(cookie) + } + router.ServeHTTP(httptest.NewRecorder(), req) + if planted == "" || afterLogin == "" || afterLogin == planted { + t.Fatalf("CSRF token before login %q, after login %q; want a fresh token", planted, afterLogin) + } +}