[WEB-4943] refactor: streamline URL construction in authentication views#7806
[WEB-4943] refactor: streamline URL construction in authentication views#7806sriramveeraghanta merged 3 commits intopreviewfrom
Conversation
* Updated MagicSignInSpaceEndpoint and MagicSignUpSpaceEndpoint to directly construct redirect URLs using formatted strings instead of the get_safe_redirect_url function. * Enhanced get_safe_redirect_url to use quote for safer URL encoding of parameters.
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughReplaces successful post-auth redirect construction across space auth views to validate next_path and concatenate with base_host(...).rstrip("/"). Retains get_safe_redirect_url only for error paths. Updates get_safe_redirect_url in path_validator to always emit next_path as a query param and avoid mutating params. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant V as Space Auth View
participant PV as Path Validator
participant H as base_host()
U->>V: Successful auth (email/github/gitlab/google/magic)
V->>PV: validate_next_path(next_path)
PV-->>V: sanitized_next_path
V->>H: base_host(request, is_space=true)
H-->>V: https://space.example.com/
V-->>U: Redirect to base_host.rstrip("/") + sanitized_next_path
note over V,U: Error paths still use get_safe_redirect_url(base_url, next_path, params)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 (6)
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors URL construction in authentication views by replacing the get_safe_redirect_url function with direct string formatting for specific endpoints, while simultaneously enhancing the remaining get_safe_redirect_url function with safer URL encoding using the quote function.
- Replaced
get_safe_redirect_urlcalls with direct string formatting in MagicSignInSpaceEndpoint and MagicSignUpSpaceEndpoint - Enhanced
get_safe_redirect_urlto usequotefor more secure URL parameter encoding - Streamlined URL construction logic in authentication flows
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/api/plane/utils/path_validator.py | Enhanced get_safe_redirect_url with quote import and safer URL encoding |
| apps/api/plane/authentication/views/space/magic.py | Replaced get_safe_redirect_url calls with direct string formatting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
* Added validate_next_path function to improve the safety of redirect URLs in MagicSignInSpaceEndpoint and MagicSignUpSpaceEndpoint. * Updated URL construction to ensure proper handling of next_path and base_url. * Streamlined the get_safe_redirect_url function for better parameter encoding.
* Introduced validate_next_path function to enhance URL safety in SignInAuthSpaceEndpoint, SignUpAuthSpaceEndpoint, GitHubCallbackSpaceEndpoint, GitLabCallbackSpaceEndpoint, and GoogleCallbackSpaceEndpoint. * Updated URL construction to directly format the redirect URL, improving clarity and consistency across multiple authentication views.
pablohashescobar
changed the title
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
|
|
||
| # Return the safe redirect URL | ||
| return f"{base_url.rstrip('/')}?{urlencode(params)}" | ||
| return f"{base_url}/?next_path={validated_path}" |
There was a problem hiding this comment.
Bug: Redirect URL Malformed with Empty Path
The get_safe_redirect_url function now unconditionally adds ?next_path={validated_path} to the URL, even when validated_path is empty. This differs from the previous behavior of omitting the parameter for invalid paths, potentially creating malformed URLs like base_url/?next_path=. Additionally, validated_path is directly inserted without URL encoding, which could lead to issues with special characters.
…ews (makeplane#7806) * refactor: streamline URL construction in authentication views * Updated MagicSignInSpaceEndpoint and MagicSignUpSpaceEndpoint to directly construct redirect URLs using formatted strings instead of the get_safe_redirect_url function. * Enhanced get_safe_redirect_url to use quote for safer URL encoding of parameters. * refactor: enhance URL validation and redirection in authentication views * Added validate_next_path function to improve the safety of redirect URLs in MagicSignInSpaceEndpoint and MagicSignUpSpaceEndpoint. * Updated URL construction to ensure proper handling of next_path and base_url. * Streamlined the get_safe_redirect_url function for better parameter encoding. * refactor: unify URL redirection logic across authentication views * Introduced validate_next_path function to enhance URL safety in SignInAuthSpaceEndpoint, SignUpAuthSpaceEndpoint, GitHubCallbackSpaceEndpoint, GitLabCallbackSpaceEndpoint, and GoogleCallbackSpaceEndpoint. * Updated URL construction to directly format the redirect URL, improving clarity and consistency across multiple authentication views.
Description
Type of Change
Test Scenarios
References
WEB-4943
Summary by CodeRabbit
Bug Fixes
Security
Refactor