fix: validate host URL in GitHub integration OAuth callback#806
fix: validate host URL in GitHub integration OAuth callback#806
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| parsed = urlparse(host_url) | ||
| if parsed.scheme not in ("https", "http"): | ||
| return redirect( | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{original_url}?error=invalid_host_url" |
Check warning
Code scanning / CodeQL
URL redirection from remote source Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 22 days ago
In general, to fix untrusted URL redirection you must not pass user-controlled data directly to redirect as part of the URL path or query without constraining it. Instead, either (a) maintain a server-side allowlist of known valid redirect paths and map user input to that, or (b) enforce that the user-provided part is a safe relative URL (no scheme, no host, no leading //, etc.), rejecting or normalizing anything else.
For this function, the safest change without altering existing behavior too much is to sanitize original_url before any use in redirect. We can: (1) ensure it is treated as a relative path, not a full URL; (2) reject absolute URLs, protocol-relative URLs, or URLs with a non-empty netloc or scheme; and (3) fall back to / if it fails validation. We can implement a small helper inline in this function that uses urllib.parse.urlparse to parse original_url, strips backslashes (as in the background example), and checks that scheme, netloc, and dangerous prefixes are absent. Then we replace all uses of original_url in redirect targets with a sanitized version, e.g. safe_original_url. This requires adding an import for urlparse at the top of the file (or reusing an existing one if present in this file) and adding a few lines right after original_url is extracted.
Concretely in backend/api/views/auth.py, within github_integration_callback, right after original_url = state.get("returnUrl", "/") we should: (a) import urlparse if not already imported globally; (b) normalize and validate original_url into safe_original_url, defaulting to / on failure; and (c) use safe_original_url in all subsequent redirect constructions (lines 211, 218, 234, 241, and 276). This keeps the existing behavior for normal relative paths while blocking malicious or malformed redirect targets.
| validate_url_is_safe(host_url) | ||
| except Exception: | ||
| return redirect( | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{original_url}?error=invalid_host_url" |
Check warning
Code scanning / CodeQL
URL redirection from remote source Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 22 days ago
In general, the fix is to ensure that the untrusted original_url value cannot cause a redirect to an untrusted host. This is usually done either by (1) treating it strictly as a relative path and rejecting or normalizing any absolute URLs, or (2) using Django’s url_has_allowed_host_and_scheme to restrict redirects to a known set of allowed hosts or to relative URLs only.
The best low-impact fix here is to sanitize original_url once, right after it is extracted from state, and then use the sanitized value in all redirect calls. We can do this by importing url_has_allowed_host_and_scheme from django.utils.http, computing the current host from request.get_host(), and then:
- If
original_urlis not considered safe (i.e., it points to an external host or contains a disallowed scheme), fall back to a safe default such as/. - Otherwise, keep it as-is.
This preserves existing behavior for valid application paths but prevents user input from forcing redirects to arbitrary external sites. Concretely:
- Add an import for
url_has_allowed_host_and_scheme. - After
original_url = state.get("returnUrl", "/"), computesafe_original_urlusingurl_has_allowed_host_and_schemewithallowed_hosts={request.get_host()}andrequire_https=request.is_secure(). If not safe, setsafe_original_url = "/". - Replace all occurrences of
original_urlinside redirect calls in this function withsafe_original_url.
No changes to authentication logic, token exchange, or other behavior are required.
| @@ -4,6 +4,7 @@ | ||
| import os | ||
| from django.shortcuts import redirect | ||
| from django.views.decorators.http import require_POST | ||
| from django.utils.http import url_has_allowed_host_and_scheme | ||
| from api.utils.syncing.auth import store_oauth_token | ||
|
|
||
| from api.authentication.providers.authentik.views import AuthentikOpenIDConnectAdapter | ||
| @@ -205,17 +206,28 @@ | ||
| state = json.loads(state_decoded) | ||
| original_url = state.get("returnUrl", "/") | ||
|
|
||
| # Ensure original_url cannot cause an open redirect to an untrusted host | ||
| # Allow only URLs that are relative or that point back to this host with an allowed scheme | ||
| if url_has_allowed_host_and_scheme( | ||
| original_url, | ||
| allowed_hosts={request.get_host()}, | ||
| require_https=request.is_secure(), | ||
| ): | ||
| safe_original_url = original_url | ||
| else: | ||
| safe_original_url = "/" | ||
|
|
||
| if error: | ||
| # User denied the OAuth consent | ||
| return redirect( | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{original_url}?error=access_denied" | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{safe_original_url}?error=access_denied" | ||
| ) | ||
|
|
||
| code = request.GET.get("code") | ||
| if not code: | ||
| # Something went wrong (missing code) | ||
| return redirect( | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{original_url}?error=missing_code" | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{safe_original_url}?error=missing_code" | ||
| ) | ||
|
|
||
| is_enterprise = bool(state.get("isEnterprise", False)) | ||
| @@ -231,14 +234,14 @@ | ||
| parsed = urlparse(host_url) | ||
| if parsed.scheme not in ("https", "http"): | ||
| return redirect( | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{original_url}?error=invalid_host_url" | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{safe_original_url}?error=invalid_host_url" | ||
| ) | ||
|
|
||
| try: | ||
| validate_url_is_safe(host_url) | ||
| except Exception: | ||
| return redirect( | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{original_url}?error=invalid_host_url" | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{safe_original_url}?error=invalid_host_url" | ||
| ) | ||
|
|
||
| client_id = ( | ||
| @@ -267,10 +264,10 @@ | ||
| access_token = response.json().get("access_token") | ||
| if not access_token: | ||
| return redirect( | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{original_url}?error=token_exchange_failed" | ||
| f"{os.getenv('ALLOWED_ORIGINS')}{safe_original_url}?error=token_exchange_failed" | ||
| ) | ||
|
|
||
| store_oauth_token("github", name, access_token, host_url, api_url, org_id) | ||
|
|
||
| # Redirect back to Next.js app | ||
| return redirect(f"{os.getenv('ALLOWED_ORIGINS')}{original_url}") | ||
| return redirect(f"{os.getenv('ALLOWED_ORIGINS')}{safe_original_url}") |
2 participants
Adds
validate_url_is_safecheck to the GitHub integration OAuth callback to ensure thehostUrlfrom the state parameter is not pointing to internal/private network addresses before making outbound requests.