Skip to content

Conversation

@tusharpandey13
Copy link
Contributor

@tusharpandey13 tusharpandey13 commented Nov 17, 2025

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

Note: This PR originally contained an attribution error in the commit message. The security issue was discovered and fixed by Joshua Rogers (@MegaManSec) in #2381. See correction comment below.

- 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>
@codecov-commenter
Copy link

codecov-commenter commented Nov 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.39%. Comparing base (0055cc4) to head (35eb321).
⚠️ Report is 22 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2413   +/-   ##
=======================================
  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 tusharpandey13 merged commit 50d25c1 into main Nov 17, 2025
9 checks passed
@tusharpandey13 tusharpandey13 deleted the fix/oauth-param-injection-with-tests branch November 17, 2025 04:15
@tusharpandey13 tusharpandey13 mentioned this pull request Nov 17, 2025
@tusharpandey13
Copy link
Contributor Author

I need to correct an attribution error in this PR. The original security fix was contributed by Joshua Rogers (@MegaManSec) in PR #2381, not "Simen A. W. Olsen" as incorrectly stated in the commit message.

We will update the changelog accordingly.

tusharpandey13 added a commit that referenced this pull request Nov 17, 2025
Credit Joshua Rogers (@MegaManSec) as the original author who
discovered and fixed the OAuth parameter injection vulnerability
in PR #2381.

This corrects an attribution error in PR #2413 where the commit
message incorrectly credited a different person.
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.

4 participants