fix: Session management issue that causes malformed redirect URLs

[mtrezza]

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

Invalid redirect URL in this scenario:

1. Open two browser tabs with dashboard.
2. In tab A click the logout button.
3. In tab B click a link to a Parse Pointer which opens in a new tab. Since the user is now logged out, the new tab C that is opened shows the login page.
4. In tab A, log into dashboard again.
5. Reload tab C which shows the login page. Since the user already logged in tab A, the dashboard should directly show the dashboard page, instead it opens an invalid URL.

The problem is that the session check recognizes you're already authenticated and tries to redirect, but the redirect URL is malformed.

Approach

Fix redirect handlers to properly strip leading slashes before concatenation. This ensures that regardless of whether the user submits the login form or reloads a login page while already authenticated, the redirect URLs are always properly formatted without double slashes or malformed relative paths.

Summary by CodeRabbit

• Security Improvements

  • Enhanced redirect URL validation to prevent open redirect vulnerabilities, rejecting absolute and protocol-relative URLs while ensuring only safe internal redirects are processed.

• Bug Fixes

  • Improved URL parsing with enhanced error handling for invalid or malformed redirect parameters.

[parse-github-assistant]

🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review.

[parseplatformorg]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds redirect parsing, validation, and normalization for /login flows: parses redirect via URL, rejects absolute/protocol-relative URLs, stores a validated originalRedirect, strips a leading slash to form internal paths, and uses the normalized value for failure/success redirects with added error handling and comments.

Changes

Cohort / File(s)	Summary
Redirect validation & normalization Parse-Dashboard/Authentication.js, Parse-Dashboard/app.js	Introduces originalRedirect storage and validation to reject absolute (://) and protocol-relative (//) URLs; parses redirect via URL in app.js with error handling; strips leading slash from valid redirects to compute internal paths; uses normalized originalRedirect for failureRedirect and redirect responses; adds explanatory comment.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant App as app.js
    participant Auth as Authentication.js
    Note over App,Auth: Login request handling & redirect validation
    Client->>App: GET /login?redirect=/path or body.redirect
    App->>App: parse req.url via URL(...)\nextract searchParams 'redirect'\nsanitize: reject '://' or startsWith('//')\nstrip leading '/'
    alt invalid redirect
        App->>App: set redirect -> 'apps'\nclear originalRedirect
    else valid redirect
        App->>Auth: pass normalized redirect / originalRedirect
    end
    Auth->>Auth: validate/normalize redirect again\nset failureRedirect using originalRedirect
    Auth-->>Client: redirect to computed internal path or failureRedirect

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Areas to check closely:
  • Parse-Dashboard/Authentication.js — correctness of originalRedirect handling and where it's used for failureRedirect.
  • Parse-Dashboard/app.js — URL parsing error handling and sanitization edge cases (leading slash stripping, protocol-relative detection).

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The PR title "fix: Session management issue that causes malformed redirect URLs" is directly related to the main changes in the pull request. The raw summary confirms that the modifications address URL validation, sanitization, and proper formatting of redirect paths to prevent malformed URLs. The title is concise (65 characters, 9 words), uses clear and specific language that explicitly mentions the key issue (malformed redirect URLs), and follows standard commit message conventions with the "fix:" prefix. A teammate scanning the history would immediately understand that this PR addresses a bug related to redirect URL formatting.
Description Check	✅ Passed	The PR description includes the core substantive sections from the template: a completed New Pull Request Checklist with both checkboxes marked, a detailed Issue Description that clearly explains the problem scenario and root cause, and a well-articulated Approach section describing the fix. However, the description is missing two template elements: the "Closes: #ISSUE_NUMBER" reference in the Issue Description section (though the PR objectives indicate issue #3011 is referenced elsewhere), and the "TODOs before merging" section with its associated checklist. Despite these template compliance gaps, the description provides complete and meaningful information about what the issue is, how it manifests, and how it was resolved.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10142cc and 1deb81f.

📒 Files selected for processing (2)

• Parse-Dashboard/Authentication.js (1 hunks)
• Parse-Dashboard/app.js (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• Parse-Dashboard/Authentication.js

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Docker linux/amd64

🔇 Additional comments (2)

Parse-Dashboard/app.js (2)

1065-1071: Well-structured URL parsing with proper error handling.

The switch from string splitting to URL constructor and searchParams.get() is a significant improvement:

• Properly handles URL-encoded values
• Provides automatic decoding
• Gracefully handles malformed URLs without crashes

---

1073-1084: Open redirect validation is equivalent in both files, but verify handler escaping for query parameters.

Authentication.js implements the same validation logic at line 83, correctly preventing :// and // patterns. However, two items require verification:

1. Unblocked protocol schemes: javascript: URLs bypass the current check and could pose an XSS risk if the login page renders the redirect query parameter without proper escaping. Verify that the login template escapes this parameter in HTML context.

2. Query parameter usage: Line 94 in Authentication.js passes the original (pre-sanitization) originalRedirect back to the client as ?redirect=${originalRedirect}. Confirm that this parameter is properly escaped when rendered on the login page to prevent injection.

---

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.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

Parse-Dashboard/app.js (1)

1065-1065: Refactor for clarity and efficiency.

The chained boolean expression is complex and calls split three times. Additionally, if the URL contains multiple query parameters (e.g., ?redirect=/apps&other=value), the redirect value would include the extra parameters.

Consider using URLSearchParams for proper query string parsing:

-    let redirectURL = req.url.includes('?redirect=') && req.url.split('?redirect=')[1].length > 1 && req.url.split('?redirect=')[1];
+    const url = new URL(req.url, `http://${req.headers.host}`);
+    let redirectURL = url.searchParams.get('redirect');

This approach:

• Properly parses query parameters regardless of order
• Handles multiple query parameters correctly
• Improves readability
• Is more efficient

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 043850b and 10142cc.

📒 Files selected for processing (2)

• Parse-Dashboard/Authentication.js (1 hunks)
• Parse-Dashboard/app.js (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Docker linux/amd64

[parseplatformorg]

🎉 This change has been released in version 8.0.0-alpha.6