feat(auth): add GET method to /auth/sign-out/ for portal logout chain#35
Open
awais786 wants to merge 4 commits into
Open
feat(auth): add GET method to /auth/sign-out/ for portal logout chain#35awais786 wants to merge 4 commits into
awais786 wants to merge 4 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
awais786
force-pushed
the
feat/sign-out-get-method
branch
from
dd79e1d to
7c90bf8
Compare
Lets the foss-server-bundle portal's "Log out of all apps" button include Plane in its cross-app redirect chain. POST behaviour is unchanged. GET clears the Django session via the same django.contrib.auth.logout() call and 302s to ?next= (validated against PLATFORM_DOMAIN to prevent open redirect; suffix match enforces dot boundary). CSRF-exempt at the class level: no token is shared cross-origin with the portal. The residual force-logout risk (image-tag triggers logout) is low — only the session itself is lost; re-auth via ForwardAuth is automatic.
awais786
force-pushed
the
feat/sign-out-get-method
branch
from
7c90bf8 to
afddc4c
Compare
This was referenced May 18, 2026
|
@copilot Review thoroughly, specially from security angle. |
… CodeQL open-redirect alert - Replace _is_allowed_next (boolean) with _sanitize_next (returns sanitized URL or None) - Reconstruct URL from urlparse components instead of passing raw user input to HttpResponseRedirect - Add scheme allowlist (http/https only) to block javascript: and data: URIs - Add test case for non-http scheme rejection Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/0c64d2ca-8166-43df-9115-a9c2135346ab Co-authored-by: jawad-khan <5320368+jawad-khan@users.noreply.github.com>
…nized sanitizer Adds Django's built-in url_has_allowed_host_and_scheme check which CodeQL recognizes as a proper open-redirect sanitizer. Combined with domain validation and URL reconstruction from parsed components. Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/0c64d2ca-8166-43df-9115-a9c2135346ab Co-authored-by: jawad-khan <5320368+jawad-khan@users.noreply.github.com>
… as redirect Move the Django sanitizer check directly into the get() method scope so CodeQL recognizes the guard pattern. Simplify to a single helper that returns the validated host set for url_has_allowed_host_and_scheme. Agent-Logs-Url: https://github.com/Pressingly/plane/sessions/0c64d2ca-8166-43df-9115-a9c2135346ab Co-authored-by: jawad-khan <5320368+jawad-khan@users.noreply.github.com>
Security review completed and CodeQL alert resolved in d9354fc. Changes made:
Security layers (defense in depth):
CodeQL now reports 0 alerts for URL redirection. |
There was a problem hiding this comment.
Pull request overview
Adds a GET handler to the existing /auth/sign-out/ endpoint to support cross-origin logout redirect chains (portal “log out of all apps”), validating ?next= redirects against an allowlisted PLATFORM_DOMAIN.
Changes:
- Implement
GET /auth/sign-out/by reusing the existing logout flow and conditionally redirecting to a validated?next=URL. - Introduce
PLATFORM_DOMAINconfiguration (settings +.env.example) to control the redirect allowlist. - Add unit tests covering GET redirect allowlisting and rejection cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| apps/api/plane/authentication/views/app/signout.py | Adds GET support, ?next= validation against PLATFORM_DOMAIN, and CSRF handling changes. |
| apps/api/plane/settings/common.py | Adds PLATFORM_DOMAIN env-backed setting. |
| apps/api/.env.example | Documents PLATFORM_DOMAIN (and adjusts MPASS-related comments). |
| apps/api/plane/tests/unit/views/test_signout.py | Adds GET-focused unit tests for redirect validation behavior. |
Comment on lines
+20
to
22
| @method_decorator(csrf_exempt, name="dispatch") | ||
| class SignOutAuthEndpoint(View): | ||
| def post(self, request): |
Comment on lines
+38
to
+46
| 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 |
Comment on lines
+149
to
+155
| 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/" | ||
|
|
Comment on lines
+149
to
+161
| 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 | ||
|
|
Comment on lines
77
to
+81
| # 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a GET method to the existing `SignOutAuthEndpoint`. POST behaviour unchanged. GET reuses the same `django.contrib.auth.logout()` call and 302s to a `?next=` URL (validated against `PLATFORM_DOMAIN`).
Lets the foss-server-bundle portal's "Log out of all apps" button include Plane in a cross-origin redirect chain that walks each app's domain and ends Plane's session along the way.
Why GET (not reuse POST)
Cross-origin redirect chains follow `Location:` headers, which are GET semantics. Cross-origin POST without CORS handshake fails; cross-origin CSRF tokens aren't shareable. GET on the same endpoint is the only chain-friendly shape, hence the addition.
Diff
Net: +153 / -9 across 4 files.
Tests
Out of scope