Fix redirects #27
Fix redirects #27
Conversation
- Introduced a new function to validate and normalize redirect URLs, ensuring only internal paths are allowed to prevent open redirect attacks. - Updated the GitHub login route to save validated redirect URLs from query parameters. - Enhanced the workflow trigger route to save relative paths for redirects when authentication is required. - Added comprehensive tests for OAuth redirect URL validation and preservation of query parameters. - Improved frontend to dynamically set the redirect URL for authentication links.
- Added a new active state for the primary button to indicate when a workflow can be run, improving user feedback. - Updated CSS to define styles for the active button state, including hover effects. - Modified JavaScript logic to manage the button's active state based on workflow permissions and conditions, ensuring accurate visual representation of button status.
🚀 Quick Actions🧪 Run Tests
📦 Backport
Choose branches to backport manually: ▶ - immediately runs the workflow with default parameters. ⚙️ - opens UI to review and modify parameters before running. These links will automatically comment on this PR with the workflow results. Tip: To open links in a new tab, use Ctrl+Click (Windows/Linux) or Cmd+Click (macOS). |
There was a problem hiding this comment.
Pull request overview
This PR implements security improvements and UX enhancements for OAuth redirect handling in a FastAPI application. The changes prevent open redirect attacks by validating redirect URLs and ensure query parameters are preserved during authentication flows.
Key changes:
- Adds URL validation to prevent external redirects during OAuth flows
- Updates redirect URL handling to use relative paths instead of full URLs
- Translates Russian UI text to English
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
backend/routes/auth.py |
Adds validate_redirect_url() function and integrates validation into OAuth login/callback flows |
backend/routes/workflow.py |
Changes redirect URL storage from full URL to relative path with query parameters |
frontend/templates/index.html |
Updates auth link to include redirect parameters and translates logout button; adds btn-active class management |
frontend/templates/result.html |
Translates Russian UI messages to English |
frontend/static/style.css |
Adds styling for active/inactive button states with new btn-active class |
tests/test_auth_redirect.py |
Adds comprehensive test coverage for OAuth redirect validation and parameter preservation |
tests/test_app.py |
Updates tests to handle redirect responses and skips complex session-dependent tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
frontend/templates/index.html
Outdated
| // Обновляем ссылку на авторизацию, чтобы она передавала текущий URL с параметрами | ||
| const authLink = document.getElementById('auth-link'); | ||
| if (authLink && window.location.search) { | ||
| const currentUrl = window.location.href; | ||
| const redirectAfter = encodeURIComponent(currentUrl); | ||
| authLink.href = `/auth/github?redirect_after=${redirectAfter}`; | ||
| } |
There was a problem hiding this comment.
This code comment is in Russian. It should be translated to English to match the rest of the codebase changes in this PR. Consider: 'Update the authorization link to pass the current URL with parameters'.
frontend/templates/index.html
Outdated
| authLink.href = `/auth/github?redirect_after=${redirectAfter}`; | ||
| } | ||
|
|
||
| // Инициализируем скрытое поле return_url из URL параметров, если его нет |
There was a problem hiding this comment.
This code comment is in Russian. It should be translated to English to match the rest of the codebase changes in this PR. Consider: 'Initialize the hidden return_url field from URL parameters if not present'.
| // Инициализируем скрытое поле return_url из URL параметров, если его нет | |
| // Initialize the hidden return_url field from URL parameters if not present |
frontend/templates/index.html
Outdated
| } | ||
| } | ||
|
|
||
| // Инициализация при загрузке |
There was a problem hiding this comment.
This code comment is in Russian. It should be translated to English to match the rest of the codebase changes in this PR. Consider: 'Initialize on page load'.
| // Инициализация при загрузке | |
| // Initialize on page load |
frontend/templates/result.html
Outdated
| secondRow.style.display = 'flex'; | ||
| } | ||
| } else if (attempts < maxAttempts) { | ||
| // Продолжаем опрос |
There was a problem hiding this comment.
This code comment is in Russian while the UI text in the same file has been translated to English. For consistency, translate to: 'Continue polling'.
| // Продолжаем опрос | |
| // Continue polling |
| if not url: | ||
| return "/" |
There was a problem hiding this comment.
The early return for empty URL in validate_redirect_url() lacks test coverage. Consider adding a test case in test_auth_redirect.py that explicitly tests empty string input to ensure this branch is covered.
| if not url.startswith('/'): | ||
| url = '/' + url |
There was a problem hiding this comment.
The normalization of paths without leading slashes lacks explicit test coverage. While test_oauth_login_accepts_internal_paths tests a path without leading slash at line 17, it doesn't verify the normalized output. Consider adding an assertion that specifically validates the normalized path.
|
✅ Tests completed successfully!
|
…ments - Translated comments in index.html and result.html from Russian to English for better accessibility. - Enhanced clarity of comments to accurately describe the functionality of JavaScript code related to authorization links and workflow actions. - Improved consistency in comment style across templates to maintain a uniform codebase.
🚀 Quick Actions🧪 Run Tests
📦 Backport
Choose branches to backport manually: ▶ - immediately runs the workflow with default parameters. ⚙️ - opens UI to review and modify parameters before running. These links will automatically comment on this PR with the workflow results. Tip: To open links in a new tab, use Ctrl+Click (Windows/Linux) or Cmd+Click (macOS). |
1 participant
No description provided.