Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions backend/api/views/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,23 @@
org_id = state.get("orgId")
name = state.get("name")

# Validate host_url to prevent SSRF
from urllib.parse import urlparse
from api.utils.network import validate_url_is_safe

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

Untrusted URL redirection depends on a
user-provided value
.

Copilot Autofix

AI about 2 months 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.


Suggested changeset 1
backend/api/views/auth.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/api/views/auth.py b/backend/api/views/auth.py
--- a/backend/api/views/auth.py
+++ b/backend/api/views/auth.py
@@ -43,6 +43,7 @@
 from ee.authentication.sso.oidc.okta.views import (
     OktaOpenIDConnectAdapter,
 )
+from urllib.parse import urlparse
 
 CLOUD_HOSTED = settings.APP_HOST == "cloud"
 
@@ -205,17 +206,28 @@
     state = json.loads(state_decoded)
     original_url = state.get("returnUrl", "/")
 
+    # Sanitize original_url to ensure it is a safe relative path
+    if not isinstance(original_url, str):
+        original_url = "/"
+    original_url = original_url.replace("\\", "")
+    parsed_original = urlparse(original_url)
+    if parsed_original.scheme or parsed_original.netloc or original_url.startswith("//"):
+        safe_original_url = "/"
+    else:
+        # Ensure it starts with a slash to keep redirection within the app
+        safe_original_url = original_url if original_url.startswith("/") else f"/{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))
@@ -225,20 +228,19 @@
     name = state.get("name")
 
     # Validate host_url to prevent SSRF
-    from urllib.parse import urlparse
     from api.utils.network import validate_url_is_safe
 
     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 +258,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}")
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
)

try:
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

Untrusted URL redirection depends on a
user-provided value
.

Copilot Autofix

AI about 2 months 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_url is 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", "/"), compute safe_original_url using url_has_allowed_host_and_scheme with allowed_hosts={request.get_host()} and require_https=request.is_secure(). If not safe, set safe_original_url = "/".
  • Replace all occurrences of original_url inside redirect calls in this function with safe_original_url.

No changes to authentication logic, token exchange, or other behavior are required.

Suggested changeset 1
backend/api/views/auth.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/api/views/auth.py b/backend/api/views/auth.py
--- a/backend/api/views/auth.py
+++ b/backend/api/views/auth.py
@@ -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}")
EOF
@@ -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}")
Copilot is powered by AI and may make mistakes. Always verify output.
)

client_id = (
os.getenv("GITHUB_ENTERPRISE_INTEGRATION_CLIENT_ID")
if is_enterprise
Expand Down
Empty file.
113 changes: 113 additions & 0 deletions backend/tests/api/views/test_github_callback.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import pytest
import json
import base64
from unittest.mock import patch, MagicMock
from django.test import RequestFactory
from api.views.auth import github_integration_callback


class TestGitHubIntegrationCallbackURLValidation:
"""Tests that the GitHub OAuth callback validates host URLs."""

def _make_state(self, host_url="https://github.com", **kwargs):
"""Create a base64-encoded state parameter."""
state = {
"returnUrl": "/",
"isEnterprise": True,
"hostUrl": host_url,
"apiUrl": host_url,
"orgId": "test-org",
"name": "test",
**kwargs,
}
return base64.b64encode(json.dumps(state).encode()).decode()

def test_rejects_private_ip_host_url(self):
"""host_url pointing to a private IP should be rejected."""
factory = RequestFactory()
state = self._make_state(host_url="http://169.254.169.254/latest/meta-data")
request = factory.get(
"/oauth/github/callback",
{"code": "test-code", "state": state},
)

with patch.dict("os.environ", {"ALLOWED_ORIGINS": "https://example.com"}):
response = github_integration_callback(request)

assert response.status_code == 302
assert "invalid_host_url" in response.url

def test_rejects_localhost_host_url(self):
"""host_url pointing to localhost should be rejected."""
factory = RequestFactory()
state = self._make_state(host_url="http://127.0.0.1:8080")
request = factory.get(
"/oauth/github/callback",
{"code": "test-code", "state": state},
)

with patch.dict("os.environ", {"ALLOWED_ORIGINS": "https://example.com"}):
response = github_integration_callback(request)

assert response.status_code == 302
assert "invalid_host_url" in response.url

def test_rejects_internal_network_host_url(self):
"""host_url pointing to internal network should be rejected."""
factory = RequestFactory()
state = self._make_state(host_url="http://10.0.0.1")
request = factory.get(
"/oauth/github/callback",
{"code": "test-code", "state": state},
)

with patch.dict("os.environ", {"ALLOWED_ORIGINS": "https://example.com"}):
response = github_integration_callback(request)

assert response.status_code == 302
assert "invalid_host_url" in response.url

def test_rejects_non_http_scheme(self):
"""host_url with non-http scheme should be rejected."""
factory = RequestFactory()
state = self._make_state(host_url="file:///etc/passwd")
request = factory.get(
"/oauth/github/callback",
{"code": "test-code", "state": state},
)

with patch.dict("os.environ", {"ALLOWED_ORIGINS": "https://example.com"}):
response = github_integration_callback(request)

assert response.status_code == 302
assert "invalid_host_url" in response.url

@patch("api.views.auth.requests.post")
@patch("api.views.auth.store_oauth_token")
@patch("api.views.auth.get_secret", return_value="fake-secret")
def test_allows_valid_github_host_url(self, mock_secret, mock_store, mock_post):
"""A valid public GitHub URL should be allowed through."""
factory = RequestFactory()
state = self._make_state(host_url="https://github.com")
request = factory.get(
"/oauth/github/callback",
{"code": "test-code", "state": state},
)

mock_post.return_value = MagicMock(
json=MagicMock(return_value={"access_token": "gho_test123"})
)

with patch.dict(
"os.environ",
{
"ALLOWED_ORIGINS": "https://example.com",
"GITHUB_ENTERPRISE_INTEGRATION_CLIENT_ID": "test-id",
},
):
with patch("api.views.auth.validate_url_is_safe"):
response = github_integration_callback(request)

# Should redirect back to app (not error)
assert response.status_code == 302
assert "invalid_host_url" not in response.url
Loading