diff --git a/apps/api/.env.example b/apps/api/.env.example index 6bed24003b4..44fdca1bc06 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -79,3 +79,6 @@ API_KEY_RATE_LIMIT="60/minute" # MPASS_BYPASS_PATHS=/god-mode,/api/instances # Required for 3-layer logout (Django → oauth2-proxy → Cognito) # MPASS_SIGNOUT_URL=https://foss-auth.local.moneta.dev/oauth2/sign_out?rd=https%3A%2F%2Fcognito.example.com%2Flogout +# Allowlist for /auth/portal-sign-out/?next= redirect targets — comma-separated +# host suffixes. Used by the foss-bundle portal's "Log out of all apps" chain. +# MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS=foss.arbisoft.com,localhost diff --git a/apps/api/plane/authentication/urls.py b/apps/api/plane/authentication/urls.py index bc736087fcb..2552a1d3c85 100644 --- a/apps/api/plane/authentication/urls.py +++ b/apps/api/plane/authentication/urls.py @@ -6,6 +6,7 @@ from .views import ( CSRFTokenEndpoint, + PortalSignOutEndpoint, SignOutAuthEndpoint, SignOutAuthSpaceEndpoint, ) @@ -36,6 +37,14 @@ # signout — kept active (used by frontend 3-layer logout) path("sign-out/", SignOutAuthEndpoint.as_view(), name="sign-out"), path("spaces/sign-out/", SignOutAuthSpaceEndpoint.as_view(), name="space-sign-out"), + # portal-driven signout — GET-able, CSRF-exempt, used by the foss-bundle + # portal's "Log out of all apps" redirect chain to clear the Django + # session cookie while the browser is on this app's domain. + path( + "portal-sign-out/", + PortalSignOutEndpoint.as_view(), + name="portal-sign-out", + ), # csrf token — kept active (Django forms need it) path("get-csrf-token/", CSRFTokenEndpoint.as_view(), name="get_csrf_token"), diff --git a/apps/api/plane/authentication/views/__init__.py b/apps/api/plane/authentication/views/__init__.py index a9c816ae9ea..45639d62211 100644 --- a/apps/api/plane/authentication/views/__init__.py +++ b/apps/api/plane/authentication/views/__init__.py @@ -14,6 +14,7 @@ from .app.magic import MagicGenerateEndpoint, MagicSignInEndpoint, MagicSignUpEndpoint from .app.signout import SignOutAuthEndpoint +from .app.portal_signout import PortalSignOutEndpoint from .space.email import SignInAuthSpaceEndpoint, SignUpAuthSpaceEndpoint diff --git a/apps/api/plane/authentication/views/app/portal_signout.py b/apps/api/plane/authentication/views/app/portal_signout.py new file mode 100644 index 00000000000..b6d8e891de7 --- /dev/null +++ b/apps/api/plane/authentication/views/app/portal_signout.py @@ -0,0 +1,86 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +from urllib.parse import urlparse + +from django.conf import settings +from django.contrib.auth import logout +from django.http import HttpResponseBadRequest, HttpResponseRedirect +from django.utils.decorators import method_decorator +from django.views import View +from django.views.decorators.csrf import csrf_exempt + + +@method_decorator(csrf_exempt, name="dispatch") +class PortalSignOutEndpoint(View): + """ + GET /auth/portal-sign-out/?next= + + Clears the Django session and 302-redirects the browser to ``next``. + + Designed for the foss-server-bundle portal's "Log out of all apps" + redirect chain — the portal can navigate the browser through each + app's portal-sign-out URL, each step clearing its own session cookie + while the browser is on that app's own domain (so the Set-Cookie + scope is correct). + + CSRF-exempt: no token is shared cross-origin with the portal, so the + POST + CSRF flow used by the in-app SignOutAuthEndpoint isn't usable + here. The residual risk is force-logout (an attacker embeds + ```` and the victim's session ends). + That's low impact (annoying, not destructive — the only state lost is + the session itself, and re-auth via ForwardAuth is automatic). + + The ``next`` URL is validated against + ``MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS`` to prevent this endpoint from + being weaponised as an open redirect. Each allowlist entry is a host + suffix; ``foss.arbisoft.com`` matches ``pm.foss.arbisoft.com``, + ``docs.foss.arbisoft.com``, etc., but does not match + ``foss.arbisoft.com.evil.example``. + """ + + def get(self, request): + logout(request) + + next_url = (request.GET.get("next") or "").strip() + + if next_url: + if not self._is_allowed_next(next_url): + return HttpResponseBadRequest( + "next= target host is not in MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS" + ) + return HttpResponseRedirect(next_url) + + # No next= supplied — fall back to MPASS_SIGNOUT_URL so a manual hit + # to this endpoint still chains through oauth2-proxy + Cognito. + fallback = getattr(settings, "MPASS_SIGNOUT_URL", "") or "/" + return HttpResponseRedirect(fallback) + + @staticmethod + def _is_allowed_next(url): + """True iff the URL's hostname matches an entry in the allowlist. + + Allowlist entries are matched as suffixes on a dot boundary: + ``foss.arbisoft.com`` matches ``foss.arbisoft.com`` and + ``*.foss.arbisoft.com``, but not ``foss.arbisoft.com.evil``. + """ + allowed = getattr(settings, "MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS", []) or [] + if not allowed: + return False + + try: + host = urlparse(url).hostname + except ValueError: + return False + if not host: + return False + + host = host.lower() + for entry in allowed: + entry = (entry or "").strip().lower().lstrip(".") + if not entry: + continue + if host == entry or host.endswith("." + entry): + return True + return False diff --git a/apps/api/plane/settings/common.py b/apps/api/plane/settings/common.py index 5f865dcb4f4..3715ecfc297 100644 --- a/apps/api/plane/settings/common.py +++ b/apps/api/plane/settings/common.py @@ -62,6 +62,16 @@ # mPass proxy auth MPASS_BYPASS_PATHS = [p.strip() for p in os.environ.get("MPASS_BYPASS_PATHS", "").split(",") if p.strip()] or None DEFAULT_EMAIL_DOMAIN = os.environ.get("DEFAULT_EMAIL_DOMAIN", "askii.ai") +# Allowlist for the ?next= parameter on /auth/portal-sign-out/ — comma- +# separated host suffixes. Each entry matches its exact host plus all +# subdomains (e.g. "foss.arbisoft.com" matches "pm.foss.arbisoft.com"). +# Empty list disables the endpoint's redirect behaviour (any ?next= is +# rejected); the endpoint still flushes the Django session. +MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = [ + h.strip().lower().lstrip(".") + for h in os.environ.get("MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS", "").split(",") + if h.strip() +] # Middlewares MIDDLEWARE = [ diff --git a/apps/api/plane/tests/unit/views/test_portal_signout.py b/apps/api/plane/tests/unit/views/test_portal_signout.py new file mode 100644 index 00000000000..1b8120eb06e --- /dev/null +++ b/apps/api/plane/tests/unit/views/test_portal_signout.py @@ -0,0 +1,194 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +""" +Unit tests for PortalSignOutEndpoint. + +SPEC (GIVEN / WHEN / THEN) +────────────────────────── + 1 GIVEN a valid ?next= URL on an allowlisted host + WHEN GET /auth/portal-sign-out/?next=… is called + THEN Django session is cleared and response 302s to that URL + + 2 GIVEN ?next= is omitted + WHEN GET /auth/portal-sign-out/ is called with MPASS_SIGNOUT_URL set + THEN response 302s to MPASS_SIGNOUT_URL (session still cleared) + + 3 GIVEN ?next= is omitted AND MPASS_SIGNOUT_URL is unset + WHEN GET /auth/portal-sign-out/ is called + THEN response 302s to "/" + + 4 GIVEN ?next= is on a host not in MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS + WHEN GET /auth/portal-sign-out/?next=https://evil.example/ is called + THEN response is 400 Bad Request — open-redirect protection + + 5 GIVEN allowlist contains "foss.arbisoft.com" + WHEN ?next= hostname is "foss.arbisoft.com.evil" + THEN response is 400 — suffix match enforces dot boundary + + 6 GIVEN allowlist contains "foss.arbisoft.com" + WHEN ?next= hostname is "docs.foss.arbisoft.com" (subdomain) + THEN response 302s to that URL — subdomain allowed + + 7 GIVEN allowlist is empty + WHEN any ?next= is supplied + THEN response is 400 — empty allowlist rejects every redirect + + 8 GIVEN ?next= contains a malformed URL + WHEN GET /auth/portal-sign-out/?next=:::garbage is called + THEN response is 400 — defensive; we don't redirect to junk +""" + +from unittest.mock import MagicMock, patch + +import pytest +from django.test import RequestFactory + +from plane.authentication.views.app.portal_signout import PortalSignOutEndpoint + +pytestmark = pytest.mark.unit + + +def _make_request(factory: RequestFactory, query: str = "") -> MagicMock: + path = "/auth/portal-sign-out/" + (f"?{query}" if query else "") + req = factory.get(path) + req.user = MagicMock(id="user-uuid-1234") + return req + + +@pytest.fixture +def factory(): + return RequestFactory() + + +@pytest.fixture +def view(): + return PortalSignOutEndpoint() + + +@pytest.mark.unit +class TestPortalSignOutEndpoint: + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_redirects_to_allowlisted_next( + self, mock_settings, mock_logout, factory, view + ): + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ["foss.arbisoft.com"] + next_url = "https://docs.foss.arbisoft.com/auth/portal-sign-out/" + + response = view.get(_make_request(factory, f"next={next_url}")) + + mock_logout.assert_called_once() + assert response.status_code == 302 + assert response["Location"] == next_url + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_falls_back_to_mpass_signout_url_when_no_next( + self, mock_settings, mock_logout, factory, view + ): + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ["foss.arbisoft.com"] + mock_settings.MPASS_SIGNOUT_URL = "https://auth.foss.arbisoft.com/oauth2/sign_out" + + response = view.get(_make_request(factory)) + + mock_logout.assert_called_once() + assert response.status_code == 302 + assert response["Location"] == "https://auth.foss.arbisoft.com/oauth2/sign_out" + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_falls_back_to_root_when_no_next_and_no_mpass_url( + self, mock_settings, mock_logout, factory, view + ): + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ["foss.arbisoft.com"] + mock_settings.MPASS_SIGNOUT_URL = "" + + response = view.get(_make_request(factory)) + + mock_logout.assert_called_once() + assert response.status_code == 302 + assert response["Location"] == "/" + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_rejects_next_on_disallowed_host( + self, mock_settings, mock_logout, factory, view + ): + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ["foss.arbisoft.com"] + + response = view.get( + _make_request(factory, "next=https://evil.example/steal") + ) + + mock_logout.assert_called_once() # session is still flushed + assert response.status_code == 400 + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_suffix_match_enforces_dot_boundary( + self, mock_settings, mock_logout, factory, view + ): + # "foss.arbisoft.com.evil" must not match the "foss.arbisoft.com" entry. + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ["foss.arbisoft.com"] + + response = view.get( + _make_request(factory, "next=https://foss.arbisoft.com.evil/x") + ) + + assert response.status_code == 400 + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_subdomain_matches_suffix_entry( + self, mock_settings, mock_logout, factory, view + ): + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ["foss.arbisoft.com"] + next_url = "https://pm.foss.arbisoft.com/portal/done" + + response = view.get(_make_request(factory, f"next={next_url}")) + + assert response.status_code == 302 + assert response["Location"] == next_url + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_empty_allowlist_rejects_all_next( + self, mock_settings, mock_logout, factory, view + ): + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = [] + + response = view.get( + _make_request(factory, "next=https://docs.foss.arbisoft.com/x") + ) + + assert response.status_code == 400 + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_malformed_next_is_rejected( + self, mock_settings, mock_logout, factory, view + ): + # Garbage URL: no hostname extractable → reject. + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = ["foss.arbisoft.com"] + + response = view.get(_make_request(factory, "next=not-a-url")) + + assert response.status_code == 400 + + @patch("plane.authentication.views.app.portal_signout.logout") + @patch("plane.authentication.views.app.portal_signout.settings") + def test_allowlist_entry_with_leading_dot_is_normalised( + self, mock_settings, mock_logout, factory, view + ): + # Operators sometimes write ".foss.arbisoft.com" — that leading dot + # is stripped and the entry treated as a host suffix. + mock_settings.MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS = [".foss.arbisoft.com"] + next_url = "https://pm.foss.arbisoft.com/done" + + response = view.get(_make_request(factory, f"next={next_url}")) + + assert response.status_code == 302 + assert response["Location"] == next_url