Skip to content

fix(web): no-store per-user auth/user response (was cacheable PII) - #1113

Merged
Pa04rth merged 2 commits into
OWASP:mainfrom
skypank-coder:fix/auth-user-cache-headers
Oct 8, 2026
Merged

Pa04rth merged 2 commits into
OWASP:mainfrom
skypank-coder:fix/auth-user-cache-headers

Conversation

@skypank-coder

Copy link
Copy Markdown
Contributor

Per @northdpole's go-ahead. /rest/v1/auth/user (and the /rest/v1/user alias) returns the
logged-in user's email, but the after_request hook stamped it Cache-Control: max-age=300 — the
sibling /rest/v1/user/resources was already no-storeed in the same hook, auth/user + alias
were just missed.

Fix: add both per-user identity paths to the existing no-store branch. Reproduced before/after via
the test client (was max-age=300, now no-store), plus a regression test asserting no-store on
all three per-user endpoints (fails on unfixed main, passes with the fix).

Low severity — impact is shared-device / post-logout browser cache serving a prior user's email;
CDN vector is deployment-dependent.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository: OWASP/OpenCRE/.coderabbit.yml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 94835a27-2618-4057-9bb7-c66cd456f2a1
📥 Commits

Reviewing files that changed from the base of the PR and between 77c862e and ad92959.

📒 Files selected for processing (1)
  • application/tests/web_main_test.py
 _______________________________________________________________________________________________________________
< Make quality a requirements issue. Involve your users in determining the project's real quality requirements. >
 ---------------------------------------------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: OWASP/OpenCRE/.coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: dad283e9-ec27-421d-8761-0ac8995ac166

📥 Commits

Reviewing files that changed from the base of the PR and between fbacf55 and 77c862e.

📒 Files selected for processing (2)
  • application/tests/web_main_test.py
  • application/web/web_main.py

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


Summary by CodeRabbit

  • Bug Fixes
    • User-specific API responses are now marked as non-cacheable, reducing the risk of serving one user’s data to another. Other responses retain their existing caching behavior.

Walkthrough

The cache-control hook now applies no-store to /rest/v1/auth/user and /rest/v1/user, as well as /rest/v1/user/resources. A regression test checks that all three endpoints omit max-age=300.

Changes

Per-user endpoint cache control

Layer / File(s) Summary
Per-user endpoint cache headers
application/web/web_main.py, application/tests/web_main_test.py
The cache-control hook marks responses from the three per-user endpoints as no-store. A regression test checks their cache headers when NO_LOGIN=1.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 77c86

The per-user endpoints are marked no-store, and the regression test covers all three routes. No merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 77c86

The change reduces the chance that a browser or shared cache serves one user's identity to another. It does not change endpoint access, but cache behavior in deployed infrastructure has not been verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected exposure is caching of per-user HTTP responses, particularly the session email returned by the identity route. The change narrows that exposure for the two identity paths.

Trust Boundaries and Controls

  • observed — The identity route remains login-protected; this PR changes the post-response cache control rather than the authentication boundary.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preventing caching of per-user authentication responses.
Description check ✅ Passed The description directly explains the cache-control defect, affected endpoints, security impact, implementation, and regression test.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Pa04rth
Pa04rth merged commit 16cb560 into OWASP:main Oct 8, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants