feat: harden study auth redirects

[ekassos]

Internal

Updates & Improvements

• Magic-link generation sanitizes and URL-encodes redirect query parameters.

[coderabbitai]

Warning

Rate limit exceeded

@ekassos has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 17 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 17 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1624ea01-3002-4ec4-9531-9a6160bfd676

📥 Commits

Reviewing files that changed from the base of the PR and between 5bbefa2 and 3c5a663.

📒 Files selected for processing (2)

• pingpong/auth.py
• pingpong/study/server.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ekassos/feat/harden-study-auth-redirects

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[greptile-apps]

Greptile Summary

This PR closes an open-redirect vulnerability in the magic-link and session-auth flows by introducing normalize_study_redirect, which validates that redirect targets are same-origin paths before they are embedded in auth links or session cookies. urlencode is also used to safely encode query parameters instead of raw f-string interpolation.

Confidence Score: 4/5

Safe to merge — the changes correctly harden redirect handling with no functional regressions introduced.

The normalization logic in normalize_study_redirect is sound: it handles null, non-absolute paths, protocol-relative prefixes, control characters, and uses urlsplit as a final guard. urlencode properly replaces raw f-string parameter embedding. The only notable finding is a redundant double-normalization call in login_magic and login_as which is harmless.

No files require special attention.

Vulnerabilities

No security issues were found. The PR fixes an open-redirect risk: normalize_study_redirect strips leading/trailing whitespace, rejects anything not starting with /, blocks // and /\\ protocol-relative prefixes, filters control characters (ord < 32), and uses urlsplit to reject any input that resolves to an absolute URI with a scheme or netloc. urlencode in generate_auth_link prevents parameter-injection via user-controlled redirect values.

Important Files Changed

Filename	Overview
pingpong/auth.py	Adds normalize_study_redirect with multi-layered open-redirect prevention (prefix checks, control-char filter, urlsplit scheme/netloc guard) and switches to urlencode for safe query-string construction in magic links.
pingpong/study/server.py	Applies normalize_study_redirect at all redirect-parameter ingestion points; introduces minor redundancy where normalization is called before generate_auth_link (which already normalizes internally).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Redirect input received] --> B{Is destination empty or None?}
    B -- Yes --> Z[Return '/']
    B -- No --> C[Strip whitespace]
    C --> D{Starts with '/'?}
    D -- No --> Z
    D -- Yes --> E{Starts with '//' or backslash?}
    E -- Yes --> Z
    E -- No --> F{Contains control chars below 32?}
    F -- Yes --> Z
    F -- No --> G[Run urlsplit]
    G --> H{Has scheme or netloc?}
    H -- Yes --> Z
    H -- No --> Y[Return sanitized path]

Loading

_{Reviews (1): Last reviewed commit: "feat: harden study auth redirects" | Re-trigger Greptile}

[greptile-apps]

Redundant double normalization

normalize_study_redirect is called here on body.forward, then called a second time inside generate_auth_link (line 165 of auth.py). The same pattern repeats at line 209 for login_as. Since generate_auth_link already normalizes its redirect argument internally, the pre-call normalization in the server layer is redundant. It's harmless and serves as defense-in-depth, but it may cause confusion about where the canonical sanitization is expected to live. Consider either removing the pre-call normalization in server.py or removing it from inside generate_auth_link and documenting the expectation.