Skip to content

Commit d695600

Browse files
committed
refactor(auth): fold portal-logout into existing SignOutAuthEndpoint
Per review feedback: Plane already has SignOutAuthEndpoint that calls django.contrib.auth.logout(). Adding a parallel PortalSignOutEndpoint duplicated logic. Fold the cross-origin GET variant into the existing view instead — one endpoint, two methods, single logout primitive. - signout.py: add GET method with ?next= validation; share _flush_session() between POST and GET so both record last_logout_time/_ip and call logout() identically. - urls.py: drop the separate /portal-sign-out/ path; both methods now hit /auth/sign-out/. - portal_signout.py: deleted (functionality merged). - test_signout.py: add GET-side tests (allowlisted next, disallowed host, dot-boundary, empty PLATFORM_DOMAIN, fallback to MPASS_SIGNOUT_URL, malformed next). - test_portal_signout.py: deleted (tests merged into test_signout.py). Net: -95 lines vs the prior commit; single source of logout truth.
1 parent a68017f commit d695600

7 files changed

Lines changed: 197 additions & 292 deletions

File tree

apps/api/.env.example

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ API_KEY_RATE_LIMIT="60/minute"
7979
# MPASS_BYPASS_PATHS=/god-mode,/api/instances
8080
# Required for 3-layer logout (Django → oauth2-proxy → Cognito)
8181
# MPASS_SIGNOUT_URL=https://foss-auth.local.moneta.dev/oauth2/sign_out?rd=https%3A%2F%2Fcognito.example.com%2Flogout
82-
# Allowlist for /auth/portal-sign-out/?next= redirect targets — comma-separated
83-
# host suffixes. Used by the foss-bundle portal's "Log out of all apps" chain.
84-
# MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS=foss.arbisoft.com,localhost
82+
# Root domain of the foss-server-bundle deployment. Used by
83+
# /auth/sign-out/?next= as the redirect allowlist — only URLs whose
84+
# host equals PLATFORM_DOMAIN or is a subdomain of it are followed.
85+
# Set automatically by foss-server-bundle/platform.sh.
86+
# PLATFORM_DOMAIN=foss.arbisoft.com

apps/api/plane/authentication/urls.py

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
from .views import (
88
CSRFTokenEndpoint,
9-
PortalSignOutEndpoint,
109
SignOutAuthEndpoint,
1110
SignOutAuthSpaceEndpoint,
1211
)
@@ -34,17 +33,12 @@
3433
# GiteaOauthInitiateSpaceEndpoint, GiteaCallbackSpaceEndpoint,
3534

3635
urlpatterns = [
37-
# signout — kept active (used by frontend 3-layer logout)
36+
# signout — POST for in-app logout, GET (?next=) for the foss-bundle
37+
# portal's "Log out of all apps" redirect chain. Both methods flush
38+
# the Django session via the same django.contrib.auth.logout() call;
39+
# GET is CSRF-exempt with PLATFORM_DOMAIN-bounded ?next= validation.
3840
path("sign-out/", SignOutAuthEndpoint.as_view(), name="sign-out"),
3941
path("spaces/sign-out/", SignOutAuthSpaceEndpoint.as_view(), name="space-sign-out"),
40-
# portal-driven signout — GET-able, CSRF-exempt, used by the foss-bundle
41-
# portal's "Log out of all apps" redirect chain to clear the Django
42-
# session cookie while the browser is on this app's domain.
43-
path(
44-
"portal-sign-out/",
45-
PortalSignOutEndpoint.as_view(),
46-
name="portal-sign-out",
47-
),
4842
# csrf token — kept active (Django forms need it)
4943
path("get-csrf-token/", CSRFTokenEndpoint.as_view(), name="get_csrf_token"),
5044

apps/api/plane/authentication/views/__init__.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
from .app.magic import MagicGenerateEndpoint, MagicSignInEndpoint, MagicSignUpEndpoint
1515

1616
from .app.signout import SignOutAuthEndpoint
17-
from .app.portal_signout import PortalSignOutEndpoint
1817

1918

2019
from .space.email import SignInAuthSpaceEndpoint, SignUpAuthSpaceEndpoint

apps/api/plane/authentication/views/app/portal_signout.py

Lines changed: 0 additions & 83 deletions
This file was deleted.

apps/api/plane/authentication/views/app/signout.py

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,24 @@
22
# SPDX-License-Identifier: AGPL-3.0-only
33
# See the LICENSE file for details.
44

5+
# Standard library
6+
from urllib.parse import urlparse
7+
58
# Django imports
69
from django.views import View
710
from django.contrib.auth import logout
811
from django.conf import settings
9-
from django.http import HttpResponseRedirect
12+
from django.http import HttpResponseBadRequest, HttpResponseRedirect
1013
from django.utils import timezone
14+
from django.utils.decorators import method_decorator
15+
from django.views.decorators.csrf import csrf_exempt
1116

1217
# Module imports
1318
from plane.authentication.utils.host import user_ip, base_host
1419
from plane.db.models import User
1520

1621

22+
@method_decorator(csrf_exempt, name="dispatch")
1723
class SignOutAuthEndpoint(View):
1824
def post(self, request):
1925
try:
@@ -36,3 +42,62 @@ def post(self, request):
3642
return HttpResponseRedirect(mpass_signout_url)
3743

3844
return HttpResponseRedirect(base_host(request=request, is_app=True))
45+
46+
def get(self, request):
47+
"""
48+
Cross-origin redirect-chain entry-point for the foss-server-bundle
49+
portal's "Log out of all apps" flow.
50+
51+
Clears the Django session via django.contrib.auth.logout(), then
52+
302s to ?next= (validated against PLATFORM_DOMAIN to prevent open
53+
redirect). The chain works because the browser is on this app's
54+
own domain when this endpoint runs — so Django's Set-Cookie scope
55+
is correct.
56+
57+
CSRF-exempt (set at class level): no token is shared cross-origin
58+
with the portal. Residual risk is force-logout (attacker embeds
59+
<img src="…/sign-out"> ending the victim's session); low impact —
60+
only the session itself is lost, and re-auth via ForwardAuth is
61+
automatic.
62+
"""
63+
logout(request)
64+
65+
next_url = (request.GET.get("next") or "").strip()
66+
if next_url:
67+
if not self._is_allowed_next(next_url):
68+
return HttpResponseBadRequest(
69+
"next= target host is not a subdomain of PLATFORM_DOMAIN"
70+
)
71+
return HttpResponseRedirect(next_url)
72+
73+
# No ?next= — fall back to MPASS_SIGNOUT_URL so a manual hit still
74+
# chains through oauth2-proxy + Cognito.
75+
mpass_signout_url = getattr(settings, "MPASS_SIGNOUT_URL", None)
76+
if mpass_signout_url:
77+
return HttpResponseRedirect(mpass_signout_url)
78+
return HttpResponseRedirect(base_host(request=request, is_app=True))
79+
80+
@staticmethod
81+
def _is_allowed_next(url):
82+
"""True iff URL's host equals PLATFORM_DOMAIN or is a subdomain.
83+
84+
Suffix match enforces a dot boundary: foss.arbisoft.com matches
85+
foss.arbisoft.com and *.foss.arbisoft.com but NOT
86+
foss.arbisoft.com.evil. Unset PLATFORM_DOMAIN → False (every
87+
?next= rejected).
88+
"""
89+
platform_domain = (
90+
getattr(settings, "PLATFORM_DOMAIN", "") or ""
91+
).strip().lower().lstrip(".")
92+
if not platform_domain:
93+
return False
94+
95+
try:
96+
host = urlparse(url).hostname
97+
except ValueError:
98+
return False
99+
if not host:
100+
return False
101+
102+
host = host.lower()
103+
return host == platform_domain or host.endswith("." + platform_domain)

0 commit comments

Comments
 (0)