[WEB-4943]: add url has allowed host or scheme for validating valid redirections #7809
[WEB-4943]: add url has allowed host or scheme for validating valid redirections #7809sriramveeraghanta merged 3 commits intopreviewfrom
Conversation
* Added get_allowed_hosts function to retrieve allowed hosts from settings. * Updated get_safe_redirect_url to validate URLs against allowed hosts. * Improved URL construction logic for safer redirection handling.
* Added url_has_allowed_host_and_scheme checks in SignUpAuthSpaceEndpoint and MagicSignInSpaceEndpoint for safer redirection. * Updated redirect logic to fallback to base host if the constructed URL is not allowed. * Improved overall URL safety and handling in authentication flows.
* Updated get_allowed_hosts to extract only the host from ADMIN_BASE_URL and SPACE_BASE_URL settings for better URL validation. * Enhanced overall safety and clarity in allowed hosts retrieval.
|
Caution Review failedThe pull request is closed. WalkthroughIntroduces host-allowlist validation for redirect URLs in space authentication flows. Updates SignUp and magic-link endpoints to validate constructed redirect URLs and fall back to a base host when disallowed. Adds get_allowed_hosts utility and updates get_safe_redirect_url to enforce host checks. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant E as Auth Endpoint (SignUp / Magic)
participant PV as Path Validator
participant DJ as Django Host Validator
U->>E: Request with next_path
E->>PV: get_safe_redirect_url(base_url, next_path)
PV->>PV: validate_next_path(next_path)
PV->>DJ: url_has_allowed_host_and_scheme(url, allowed_hosts)
alt Host allowed
DJ-->>PV: true
PV-->>E: safe redirect URL
E-->>U: HTTP 302 to next URL
else Host not allowed
DJ-->>PV: false
PV-->>E: base_url
E-->>U: HTTP 302 to base host
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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.
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. Comment |
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR implements URL validation for redirection safety by introducing host and scheme validation to prevent open redirect vulnerabilities. The changes add a new function to retrieve allowed hosts from settings and integrate Django's built-in URL validation across authentication endpoints.
- Added
get_allowed_hosts()function to extract allowed hosts from application settings - Enhanced
get_safe_redirect_url()with URL validation using Django'surl_has_allowed_host_and_scheme - Updated space authentication endpoints to validate redirect URLs before performing redirections
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/api/plane/utils/path_validator.py | Added host validation logic and enhanced redirect URL safety checks |
| apps/api/plane/authentication/views/space/magic.py | Integrated URL validation in magic link authentication endpoints |
| apps/api/plane/authentication/views/space/email.py | Added URL validation to email authentication endpoint |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| base_origin = settings.WEB_URL or settings.APP_BASE_URL | ||
| allowed_hosts = [base_origin] | ||
| if settings.ADMIN_BASE_URL: | ||
| # Get only the host | ||
| host = urlparse(settings.ADMIN_BASE_URL).netloc | ||
| allowed_hosts.append(host) | ||
| if settings.SPACE_BASE_URL: | ||
| # Get only the host | ||
| host = urlparse(settings.SPACE_BASE_URL).netloc | ||
| allowed_hosts.append(host) |
There was a problem hiding this comment.
Inconsistent host extraction logic. base_origin is added as a full URL while ADMIN_BASE_URL and SPACE_BASE_URL are parsed to extract only the netloc. This creates a mismatch where base_origin should also be parsed to extract only the host portion for consistent validation.
| from django.core.validators import validate_email | ||
| from django.http import HttpResponseRedirect | ||
| from django.views import View | ||
| from django.utils.http import url_has_allowed_host_and_scheme |
There was a problem hiding this comment.
Missing import for get_allowed_hosts function. The function is used on line 204 but not imported, which will cause a NameError at runtime.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on September 20
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| # Get only the host | ||
| host = urlparse(settings.SPACE_BASE_URL).netloc | ||
| allowed_hosts.append(host) | ||
| return allowed_hosts |
There was a problem hiding this comment.
Bug: Inconsistent Host Handling in URL Validation
The get_allowed_hosts function inconsistently populates the allowed_hosts list. It adds base_origin as a full URL, but extracts only the hostname for ADMIN_BASE_URL and SPACE_BASE_URL. This causes url_has_allowed_host_and_scheme to fail validation, as it expects only hostnames.
…edirections (makeplane#7809) * feat: enhance path validation and URL safety in path_validator.py * Added get_allowed_hosts function to retrieve allowed hosts from settings. * Updated get_safe_redirect_url to validate URLs against allowed hosts. * Improved URL construction logic for safer redirection handling. * feat: enhance URL validation in authentication views * Added url_has_allowed_host_and_scheme checks in SignUpAuthSpaceEndpoint and MagicSignInSpaceEndpoint for safer redirection. * Updated redirect logic to fallback to base host if the constructed URL is not allowed. * Improved overall URL safety and handling in authentication flows. * fix: improve host extraction in get_allowed_hosts function * Updated get_allowed_hosts to extract only the host from ADMIN_BASE_URL and SPACE_BASE_URL settings for better URL validation. * Enhanced overall safety and clarity in allowed hosts retrieval.
Description
Type of Change
Test Scenarios
References
WEB-4943
Summary by CodeRabbit