[WEB-4900] refactor: remove base_host retrieval from authentication views

[pablohashescobar]

Description

• Removed unnecessary base_host retrieval from GitHub, GitLab, and Google callback endpoints.
• Updated MagicSignUpEndpoint to use get_safe_redirect_url for URL construction.
• Refactored MagicSignInSpaceEndpoint to streamline URL redirection logic.

Type of Change

• Bug fix (non-breaking change which fixes an issue)

Test Scenarios

• test all authentication endpoints specifically all redirections

References

WEB-4900

Summary by CodeRabbit

• New Features
  • Safer redirect handling now supports passing error parameters, improving clarity on sign-in failures.
• Bug Fixes
  • Fixed OAuth callback redirects (GitHub, GitLab, Google) to reliably use the validated host, preventing misdirects.
  • Improved Magic link sign-up/sign-in flows with correct redirection and cleaner error parameters.
• Refactor
  • Standardized redirect URL construction through a single safe helper across authentication flows, reducing inconsistencies and potential URL issues.

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Removes local variables that shadow the base_host helper in GitHub/GitLab/Google app OAuth callbacks, adjusts magic sign-in/sign-up redirect construction to use get_safe_redirect_url with params, and updates get_safe_redirect_url to accept an optional params argument. Imports for manual URL assembly are removed where no longer needed.

Changes

Cohort / File(s)	Summary
App OAuth callbacks: shadowing fix apps/api/plane/authentication/views/app/github.py, .../app/gitlab.py, .../app/google.py	Deleted local base_host session assignments to avoid shadowing the helper function; URL construction continues to call base_host(request, is_app=True). No other logic changes.
App magic signup: error payload adjustment apps/api/plane/authentication/views/app/magic.py	Removed insertion of next_path into error payload for existing-user path; redirect still uses get_safe_redirect_url with next_path and params.
Space magic signin: unified redirect builder apps/api/plane/authentication/views/space/magic.py	Replaced manual urljoin/urlencode with get_safe_redirect_url(..., params=...). Removed related imports; updated variable naming in exception path.
Path validator utility: signature update plane/utils/path_validator.py	Changed get_safe_redirect_url signature to accept params: get_safe_redirect_url(base_url, next_path, params=None).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client (App)
  participant S as MagicSignInSpaceEndpoint
  participant U as get_safe_redirect_url
  Note over S: Sign-in attempt (space)
  C->>S: POST /magic/signin
  alt Success
    S->>U: Build redirect (base_url, next_path, params=None)
    U-->>S: Safe URL
    S-->>C: 302 Redirect to next_path
  else Error/Exception
    S->>U: Build redirect (base_url, next_path, params=error)
    U-->>S: Safe URL with query params
    S-->>C: 302 Redirect with error params
  end
  Note over U: New params argument used

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [WEB-4013]chore: publish login and standardize urls in common settings  #7013 — Refactors and centralizes base_host and URL utilities; relates to fixing base_host shadowing in OAuth callbacks.
• [WEB-4900]: validated authentication redirection paths #7798 — Centralizes redirects via get_safe_redirect_url; aligns with adopting the new params-based signature in this PR.

Suggested labels

ready to merge

Suggested reviewers

• dheeru0198
• sriramveeraghanta

Poem

I nibbled at URLs, neat and small,
No shadows on hosts, no trips to fall.
Hop-hop, params packed, safely we go—
One helper to guide every flow.
Ears up, redirects tidy and bright,
This bunny ships clean paths tonight. 🐇✨

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 title "[WEB-4900] refactor: remove base_host retrieval from authentication views" is concise, accurately reflects the main change (removing session-based base_host retrieval across authentication callback views), and includes the ticket reference and change type; it is specific enough for a teammate scanning history to understand the primary intent without listing files or unnecessary details.
Description Check	✅ Passed	The PR description follows the repository template and includes a Description, a checked Type of Change (Bug fix), Test Scenarios, and References, and it matches the changes summarized in the raw_summary and pr_objectives. The Test Scenarios entry is high-level ("test all authentication endpoints specifically all redirections") and would benefit from more concrete details (which tests, commands, or observed results), and the Screenshots/Media section is omitted (acceptable if not applicable).

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-redirections

---

📜 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 1f7eef5 and b136a0b.

📒 Files selected for processing (5)

• apps/api/plane/authentication/views/app/github.py (0 hunks)
• apps/api/plane/authentication/views/app/gitlab.py (0 hunks)
• apps/api/plane/authentication/views/app/google.py (0 hunks)
• apps/api/plane/authentication/views/app/magic.py (0 hunks)
• apps/api/plane/authentication/views/space/magic.py (1 hunks)

💤 Files with no reviewable changes (4)

• apps/api/plane/authentication/views/app/github.py
• apps/api/plane/authentication/views/app/magic.py
• apps/api/plane/authentication/views/app/gitlab.py
• apps/api/plane/authentication/views/app/google.py

🧰 Additional context used

🧬 Code graph analysis (1)

apps/api/plane/authentication/views/space/magic.py (1)

apps/api/plane/utils/path_validator.py (1)

• get_safe_redirect_url (76-97)

🔇 Additional comments (2)

apps/api/plane/authentication/views/space/magic.py (2)

104-108: LGTM: unified redirect construction via get_safe_redirect_url.

This removes the brittle manual urljoin/urlencode sequence and centralizes path validation. Looks good.

---

56-57: No change required — validate_next_path handles None safely.
validate_next_path returns an empty string for falsy or non-str inputs, so validate_next_path(request.POST.get("next_path")) is safe.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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

[makeplane]

Pull Request Linked with Plane Work Items

• [WEB-4900] Validated authentication redirection to avoid xss and fix all codeql vulnerabilities

Comment Automatically Generated by Plane

[Copilot]

Pull Request Overview

This PR refactors authentication callback endpoints to remove unnecessary base_host retrievals and standardizes URL redirection logic. The changes focus on cleaning up code by eliminating unused variables and consolidating URL construction methods.

• Removed unused base_host variable retrieval from GitHub, GitLab, and Google OAuth callback endpoints
• Updated MagicSignUpEndpoint to use get_safe_redirect_url instead of manual URL construction with urljoin
• Cleaned up redundant parameter handling in magic authentication

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
apps/api/plane/authentication/views/app/github.py	Removed unused base_host variable from callback endpoint
apps/api/plane/authentication/views/app/gitlab.py	Removed unused base_host variable from callback endpoint
apps/api/plane/authentication/views/app/google.py	Removed unused base_host variable from callback endpoint
apps/api/plane/authentication/views/app/magic.py	Removed redundant next_path parameter assignment
apps/api/plane/authentication/views/space/magic.py	Replaced manual URL construction with get_safe_redirect_url and removed unused imports

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}