diff --git a/apps/api/.env.example b/apps/api/.env.example index 6bed24003b4..a0082a81b61 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -77,5 +77,5 @@ API_KEY_RATE_LIMIT="60/minute" # mPass proxy auth # Optional: comma-separated paths to skip proxy auth (default: /god-mode,/api/instances) # 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 +# PLATFORM_DOMAIN=foss.arbisoft.com diff --git a/apps/api/plane/authentication/views/app/signout.py b/apps/api/plane/authentication/views/app/signout.py index 65aa29aef4e..e85fd220348 100644 --- a/apps/api/plane/authentication/views/app/signout.py +++ b/apps/api/plane/authentication/views/app/signout.py @@ -2,18 +2,22 @@ # SPDX-License-Identifier: AGPL-3.0-only # See the LICENSE file for details. -# Django imports +from urllib.parse import urlparse + from django.views import View from django.contrib.auth import logout from django.conf import settings -from django.http import HttpResponseRedirect +from django.http import HttpResponseBadRequest, HttpResponseRedirect from django.utils import timezone +from django.utils.decorators import method_decorator +from django.utils.http import url_has_allowed_host_and_scheme +from django.views.decorators.csrf import csrf_exempt -# Module imports from plane.authentication.utils.host import user_ip, base_host from plane.db.models import User +@method_decorator(csrf_exempt, name="dispatch") class SignOutAuthEndpoint(View): def post(self, request): try: @@ -24,15 +28,64 @@ def post(self, request): except Exception: pass finally: - # Always clear the Django session, even if user lookup/save failed logout(request) - # If SSO (mPass) sign-out URL is configured, redirect there to also - # clear the shared oauth2-proxy session and Cognito session. - # Without this, the next request immediately re-authenticates the user - # via Traefik ForwardAuth. mpass_signout_url = getattr(settings, "MPASS_SIGNOUT_URL", None) if mpass_signout_url: return HttpResponseRedirect(mpass_signout_url) - return HttpResponseRedirect(base_host(request=request, is_app=True)) + + def get(self, request): + # Delegate to POST so last_logout_ip/_time get tracked and the + # session is flushed the same way. Only override the redirect + # target if ?next= was passed (portal logout-chain hop). + response = self.post(request) + + next_url = (request.GET.get("next") or "").strip() + if not next_url: + return response + + # Validate the redirect target against PLATFORM_DOMAIN. + allowed_hosts = self._get_allowed_hosts_for_url(next_url) + if not allowed_hosts: + return HttpResponseBadRequest( + "next= host is not a subdomain of PLATFORM_DOMAIN" + ) + + # Django's built-in open-redirect guard — CodeQL recognizes this as + # a sanitizer when it directly guards a redirect in the same scope. + if not url_has_allowed_host_and_scheme( + next_url, allowed_hosts=allowed_hosts, require_https=False + ): + return HttpResponseBadRequest( + "next= host is not a subdomain of PLATFORM_DOMAIN" + ) + + return HttpResponseRedirect(next_url) + + @staticmethod + def _get_allowed_hosts_for_url(url): + """Return allowed hosts set if *url*'s host is within PLATFORM_DOMAIN. + + Returns a set containing the URL's hostname (for use with + url_has_allowed_host_and_scheme) if it passes dot-boundary domain + validation; returns an empty set otherwise. + """ + platform_domain = ( + getattr(settings, "PLATFORM_DOMAIN", "") or "" + ).strip().lower().lstrip(".") + if not platform_domain: + return set() + + try: + host = urlparse(url).hostname + except ValueError: + return set() + if not host: + return set() + + host = host.lower() + # Dot-boundary enforcement: foss.arbisoft.com.evil does NOT match. + if host == platform_domain or host.endswith("." + platform_domain): + return {host} + return set() diff --git a/apps/api/plane/settings/common.py b/apps/api/plane/settings/common.py index 6d23c23a8d7..47effeb55c7 100644 --- a/apps/api/plane/settings/common.py +++ b/apps/api/plane/settings/common.py @@ -62,6 +62,7 @@ # 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") +PLATFORM_DOMAIN = os.environ.get("PLATFORM_DOMAIN", "") # SMB portal hostname segment (landing / logout redirects) vs default Plane workspace slug SMB_NAME = os.environ.get("SMB_NAME") diff --git a/apps/api/plane/tests/unit/views/test_signout.py b/apps/api/plane/tests/unit/views/test_signout.py index 6ba375e0988..2158b00351d 100644 --- a/apps/api/plane/tests/unit/views/test_signout.py +++ b/apps/api/plane/tests/unit/views/test_signout.py @@ -129,3 +129,130 @@ def test_falls_back_to_base_host_when_save_raises_and_no_mpass_url( assert response.status_code == 302 assert response["Location"] == _BASE_HOST + + + +def _make_get_request(factory: RequestFactory, qs: str = "") -> MagicMock: + url = "/auth/sign-out/" + (f"?{qs}" if qs else "") + req = factory.get(url) + req.user = MagicMock(id="user-uuid-1234") + return req + + +@pytest.mark.unit +class TestSignOutAuthEndpointGet: + + @patch("plane.authentication.views.app.signout.base_host", return_value=_BASE_HOST) + @patch("plane.authentication.views.app.signout.logout") + @patch("plane.authentication.views.app.signout.User") + @patch("plane.authentication.views.app.signout.settings") + def test_redirects_to_allowlisted_next( + self, mock_settings, mock_user_cls, mock_logout, mock_base_host, factory, view + ): + mock_settings.PLATFORM_DOMAIN = "foss.arbisoft.com" + mock_user_cls.objects.get.return_value = MagicMock() + next_url = "https://docs.foss.arbisoft.com/auth/sign-out/" + + response = view.get(_make_get_request(factory, f"next={next_url}")) + + mock_logout.assert_called_once() # GET still flushes the session + assert response.status_code == 302 + assert response["Location"] == next_url + + @patch("plane.authentication.views.app.signout.base_host", return_value=_BASE_HOST) + @patch("plane.authentication.views.app.signout.logout") + @patch("plane.authentication.views.app.signout.User") + @patch("plane.authentication.views.app.signout.settings") + def test_rejects_next_on_disallowed_host( + self, mock_settings, mock_user_cls, mock_logout, mock_base_host, factory, view + ): + mock_settings.PLATFORM_DOMAIN = "foss.arbisoft.com" + mock_user_cls.objects.get.return_value = MagicMock() + + response = view.get( + _make_get_request(factory, "next=https://evil.example/steal") + ) + + mock_logout.assert_called_once() # session still flushed + assert response.status_code == 400 + + @patch("plane.authentication.views.app.signout.base_host", return_value=_BASE_HOST) + @patch("plane.authentication.views.app.signout.logout") + @patch("plane.authentication.views.app.signout.User") + @patch("plane.authentication.views.app.signout.settings") + def test_suffix_match_enforces_dot_boundary( + self, mock_settings, mock_user_cls, mock_logout, mock_base_host, factory, view + ): + """foss.arbisoft.com.evil must not match foss.arbisoft.com.""" + mock_settings.PLATFORM_DOMAIN = "foss.arbisoft.com" + mock_user_cls.objects.get.return_value = MagicMock() + + response = view.get( + _make_get_request(factory, "next=https://foss.arbisoft.com.evil/x") + ) + + assert response.status_code == 400 + + @patch("plane.authentication.views.app.signout.base_host", return_value=_BASE_HOST) + @patch("plane.authentication.views.app.signout.logout") + @patch("plane.authentication.views.app.signout.User") + @patch("plane.authentication.views.app.signout.settings") + def test_empty_platform_domain_rejects_all_next( + self, mock_settings, mock_user_cls, mock_logout, mock_base_host, factory, view + ): + mock_settings.PLATFORM_DOMAIN = "" + mock_user_cls.objects.get.return_value = MagicMock() + + response = view.get( + _make_get_request(factory, "next=https://docs.foss.arbisoft.com/x") + ) + + assert response.status_code == 400 + + @patch("plane.authentication.views.app.signout.base_host", return_value=_BASE_HOST) + @patch("plane.authentication.views.app.signout.logout") + @patch("plane.authentication.views.app.signout.User") + @patch("plane.authentication.views.app.signout.settings") + def test_no_next_falls_back_to_mpass_url( + self, mock_settings, mock_user_cls, mock_logout, mock_base_host, factory, view + ): + mock_settings.PLATFORM_DOMAIN = "foss.arbisoft.com" + mock_settings.MPASS_SIGNOUT_URL = _MPASS_URL + mock_user_cls.objects.get.return_value = MagicMock() + + response = view.get(_make_get_request(factory)) + + mock_logout.assert_called_once() + assert response.status_code == 302 + assert response["Location"] == _MPASS_URL + + @patch("plane.authentication.views.app.signout.base_host", return_value=_BASE_HOST) + @patch("plane.authentication.views.app.signout.logout") + @patch("plane.authentication.views.app.signout.User") + @patch("plane.authentication.views.app.signout.settings") + def test_malformed_next_is_rejected( + self, mock_settings, mock_user_cls, mock_logout, mock_base_host, factory, view + ): + mock_settings.PLATFORM_DOMAIN = "foss.arbisoft.com" + mock_user_cls.objects.get.return_value = MagicMock() + + response = view.get(_make_get_request(factory, "next=:::garbage")) + + assert response.status_code == 400 + + @patch("plane.authentication.views.app.signout.base_host", return_value=_BASE_HOST) + @patch("plane.authentication.views.app.signout.logout") + @patch("plane.authentication.views.app.signout.User") + @patch("plane.authentication.views.app.signout.settings") + def test_non_http_scheme_is_rejected( + self, mock_settings, mock_user_cls, mock_logout, mock_base_host, factory, view + ): + """javascript: or other non-http(s) schemes must be rejected.""" + mock_settings.PLATFORM_DOMAIN = "foss.arbisoft.com" + mock_user_cls.objects.get.return_value = MagicMock() + + response = view.get( + _make_get_request(factory, "next=javascript://foss.arbisoft.com/x") + ) + + assert response.status_code == 400