remark42 icon indicating copy to clipboard operation
remark42 copied to clipboard

Fix login persistence with AUTH_SEND_JWT_HEADER enabled

Open paskal opened this issue 10 months ago • 3 comments

Summary

  • When using AUTH_SEND_JWT_HEADER=true, the JWT token in header needed to be persisted to maintain login after page reload
  • The frontend now properly handles JWT tokens received in headers by:
    • Storing them in client-side cookies with the name 'JWT' to match what backend expects
    • Extracting and storing the JWT ID as XSRF token in 'XSRF-TOKEN' cookie
    • Setting cookies with Secure flag when on HTTPS connections
  • Documentation has been updated to clarify that with this flag enabled, the frontend stores the JWT in a client-side cookie

First part of the fix for #1877, pre-requirement for fixing OAUTH login in #241

Technical Details

The issue was occurring because the JWT token was received in X-JWT header with AUTH_SEND_JWT_HEADER=true, but the token was only stored in memory. When the page reloaded, the token would be lost, causing users to be logged out.

This implementation allows the frontend to handle authentication consistently regardless of whether AUTH_SEND_JWT_HEADER is enabled or not, making it transparent to users.

Demo

Before:

https://github.com/user-attachments/assets/7990ca72-5e7c-4dcf-aacc-ad9b6b9d3aa7

After:

https://github.com/user-attachments/assets/c9d7bbf0-a83c-4881-be36-a114a27a39d7

Concerns

Stored cookie is not HTTPOnly as it's written from JS, so could be read from JS on the page as well. I thought it would be better than storing it in local storage, at least we know that the only sensitive stuff is still in the cookie.

paskal avatar Apr 27 '25 21:04 paskal

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 62.19%. Comparing base (1f3a868) to head (abdca90). Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #1929    +/-   ##
========================================
  Coverage   62.19%   62.19%            
========================================
  Files         132      132            
  Lines        3026     3026            
  Branches      721      764    +43     
========================================
  Hits         1882     1882            
+ Misses       1140     1030   -110     
- Partials        4      114   +110     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Apr 27 '25 21:04 codecov[bot]

size-limit report 📦

Path Size
public/embed.mjs 2.03 KB (0%)
public/remark.mjs 73.84 KB (+0.44% 🔺)
public/remark.css 8.26 KB (0%)
public/last-comments.mjs 36.12 KB (+1.03% 🔺)
public/last-comments.css 3.75 KB (-0.03% 🔽)
public/deleteme.mjs 12.45 KB (+3.14% 🔺)
public/counter.mjs 751 B (0%)

github-actions[bot] avatar Apr 27 '25 21:04 github-actions[bot]

Pull Request Test Coverage Report for Build 14742789946

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.03%) to 84.471%

Files with Coverage Reduction New Missed Lines %
backend/app/providers/telegram.go 2 87.88%
<!-- Total: 2
Totals Coverage Status
Change from base Build 14725791312: -0.03%
Covered Lines: 6049
Relevant Lines: 7161

💛 - Coveralls

coveralls avatar Apr 27 '25 21:04 coveralls