Skip to content

Conversation

@tusharpandey13
Copy link
Contributor

Description

This PR addresses a security issue where OAuth parameters could be injected via the returnTo parameter in withPageAuthRequired.

Changes

  • URL encode the returnTo parameter to prevent injection of OAuth parameters like &prompt=login
  • Add comprehensive unit tests covering:
    • Basic returnTo encoding
    • OAuth parameter injection attempts
    • Special character handling
    • Function-based returnTo options

Related PRs

Testing

All existing tests pass, plus 3 new test cases specifically for this issue:

✓ should URL encode returnTo parameter to prevent OAuth param injection
✓ should URL encode returnTo with special characters
✓ should URL encode returnTo from function to prevent OAuth param injection

Co-authored-by: Simen A. W. Olsen my@simen.io

- URL encode returnTo parameter to prevent injection of OAuth parameters
- Add comprehensive unit tests for returnTo encoding scenarios
- Tests cover basic encoding, OAuth param injection attempts, and edge cases

Co-authored-by: Simen A. W. Olsen <my@simen.io>
@tusharpandey13 tusharpandey13 requested a review from a team as a code owner November 17, 2025 03:57
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.39%. Comparing base (0055cc4) to head (94670ea).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2411   +/-   ##
=======================================
  Coverage   90.39%   90.39%           
=======================================
  Files          39       39           
  Lines        4488     4488           
  Branches      912      911    -1     
=======================================
  Hits         4057     4057           
  Misses        425      425           
  Partials        6        6           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@tusharpandey13
Copy link
Contributor Author

Closing this PR in favor of #2413 which uses a branch on the main repo instead of a fork. The commits are identical.

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.

2 participants