Skip to content

[WEB-4900]: validated authentication redirection paths #7798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sriramveeraghanta merged 3 commits into from
Sep 15, 2025
Merged

[WEB-4900]: validated authentication redirection paths #7798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
49a5bfb
refactor: replace validate_next_path with get_safe_redirect_url for s…
pablohashescobar Sep 15, 2025
6a5ba15
refactor: use get_safe_redirect_url for improved URL redirection in S…
pablohashescobar Sep 15, 2025
767fe7e
fix: redirect paths
sriramveeraghanta Sep 15, 2025
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
105 changes: 55 additions & 50 deletions apps/api/plane/authentication/views/app/email.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# Python imports
from urllib.parse import urlencode, urljoin

# Django imports
from django.core.exceptions import ValidationError
from django.core.validators import validate_email
Expand All @@ -19,7 +16,7 @@
AuthenticationException,
AUTHENTICATION_ERROR_CODES,
)
from plane.utils.path_validator import validate_next_path
from plane.utils.path_validator import get_safe_redirect_url


class SignInAuthEndpoint(View):
Expand All @@ -34,11 +31,11 @@ def post(self, request):
error_message="INSTANCE_NOT_CONFIGURED",
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
# Base URL join
url = urljoin(
base_host(request=request, is_app=True), "sign-in?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)

Expand All @@ -58,10 +55,10 @@ def post(self, request):
)
params = exc.get_error_dict()
# Next path
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "sign-in?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)

Expand All @@ -76,10 +73,10 @@ def post(self, request):
payload={"email": str(email)},
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "sign-in?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)

Expand All @@ -92,10 +89,10 @@ def post(self, request):
payload={"email": str(email)},
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "sign-in?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)

Expand All @@ -112,19 +109,23 @@ def post(self, request):
user_login(request=request, user=user, is_app=True)
# Get the redirection path
if next_path:
path = str(validate_next_path(next_path))
path = next_path
else:
path = get_redirection_path(user=user)

# redirect to referer path
url = urljoin(base_host(request=request, is_app=True), path)
# Get the safe redirect URL
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=path,
params={},
)
return HttpResponseRedirect(url)
except AuthenticationException as e:
params = e.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "sign-in?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)

Expand All @@ -141,10 +142,10 @@ def post(self, request):
error_message="INSTANCE_NOT_CONFIGURED",
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)

Expand All @@ -161,10 +162,10 @@ def post(self, request):
payload={"email": str(email)},
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)
# Validate the email
Expand All @@ -179,10 +180,10 @@ def post(self, request):
payload={"email": str(email)},
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)

Expand All @@ -197,10 +198,10 @@ def post(self, request):
payload={"email": str(email)},
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)

Expand All @@ -217,17 +218,21 @@ def post(self, request):
user_login(request=request, user=user, is_app=True)
# Get the redirection path
if next_path:
path = str(validate_next_path(next_path))
path = next_path
else:
path = get_redirection_path(user=user)
# redirect to referer path
url = urljoin(base_host(request=request, is_app=True), path)

url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=path,
params={},
)
return HttpResponseRedirect(url)
except AuthenticationException as e:
params = e.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params,
)
return HttpResponseRedirect(url)
57 changes: 33 additions & 24 deletions apps/api/plane/authentication/views/app/github.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Python imports
import uuid
from urllib.parse import urlencode, urljoin

# Django import
from django.http import HttpResponseRedirect
Expand All @@ -16,8 +16,7 @@
AuthenticationException,
AUTHENTICATION_ERROR_CODES,
)
from plane.utils.path_validator import validate_next_path

from plane.utils.path_validator import get_safe_redirect_url

class GitHubOauthInitiateEndpoint(View):
def get(self, request):
Expand All @@ -35,10 +34,10 @@ def get(self, request):
error_message="INSTANCE_NOT_CONFIGURED",
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params
)
return HttpResponseRedirect(url)
try:
Expand All @@ -49,10 +48,10 @@ def get(self, request):
return HttpResponseRedirect(auth_url)
except AuthenticationException as e:
params = e.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(
base_host(request=request, is_app=True), "?" + urlencode(params)
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params
)
return HttpResponseRedirect(url)

Expand All @@ -70,9 +69,11 @@ def get(self, request):
error_message="GITHUB_OAUTH_PROVIDER_ERROR",
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(base_host, "?" + urlencode(params))
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params
)
return HttpResponseRedirect(url)
Comment on lines +73 to 77
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Shadowing base_host in callback breaks redirects.

base_host = request.session.get("host") shadows the function; subsequent base_host(...) calls will error.

Rename:

-        base_host = request.session.get("host")
+        host_url = request.session.get("host")

Also applies to: 86-90, 114-118

🤖 Prompt for AI Agents 
In apps/api/plane/authentication/views/app/github.py around lines 73-77 (and
similarly at 86-90 and 114-118), a local variable is assigned as base_host =
request.session.get("host"), which shadows the imported/function name base_host
and causes subsequent calls base_host(...) to fail; rename the session variable
to something non-conflicting (e.g., session_host or host_from_session), update
all uses of that variable in the callback to the new name, and keep all calls to
base_host(...) as function calls so redirects build correctly.


if not code:
Expand All @@ -81,9 +82,11 @@ def get(self, request):
error_message="GITHUB_OAUTH_PROVIDER_ERROR",
)
params = exc.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(base_host, "?" + urlencode(params))
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params
)
return HttpResponseRedirect(url)

try:
Expand All @@ -93,17 +96,23 @@ def get(self, request):
user = provider.authenticate()
# Login the user and record his device info
user_login(request=request, user=user, is_app=True)
# Get the redirection path
if next_path:
path = str(validate_next_path(next_path))
path = next_path
else:
path = get_redirection_path(user=user)
# redirect to referer path
url = urljoin(base_host, path)

# Get the safe redirect URL
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=path,
params={}
)
return HttpResponseRedirect(url)
except AuthenticationException as e:
params = e.get_error_dict()
if next_path:
params["next_path"] = str(validate_next_path(next_path))
url = urljoin(base_host, "?" + urlencode(params))
url = get_safe_redirect_url(
base_url=base_host(request=request, is_app=True),
next_path=next_path,
params=params
)
return HttpResponseRedirect(url)
Loading
Loading