feat(auth): add GET /auth/portal-sign-out/ for cross-app logout chain#34
Closed
awais786 wants to merge 3 commits into
Closed
feat(auth): add GET /auth/portal-sign-out/ for cross-app logout chain#34awais786 wants to merge 3 commits into
awais786 wants to merge 3 commits into
Conversation
The foss-server-bundle portal's "Log out of all apps" button needs to
clear each app's session cookie in addition to the shared _oauth2_proxy
cookie and the Cognito SSO session. Cross-origin Set-Cookie isn't a
thing — only the app whose origin owns the cookie can clear it — so
the portal has to navigate the browser through each app's domain.
This endpoint is the Plane participant in that chain:
GET /auth/portal-sign-out/?next=<absolute_url>
1. logout(request) — flushes the Django session
2. Validates next against MPASS_SIGNOUT_NEXT_ALLOWED_HOSTS (suffix
match on a dot boundary; ".foss.arbisoft.com" matches subdomains
but not "foss.arbisoft.com.evil")
3. 302 to next
CSRF-exempt because the portal cannot obtain Plane's CSRF token cross-
origin. The residual risk is force-logout (an attacker embeds
<img src="…/portal-sign-out"> and ends the victim's session). That's
low-impact: the only state lost is the session itself, and re-auth via
ForwardAuth is automatic on the next request.
When ?next= is omitted or invalid, falls back to MPASS_SIGNOUT_URL
(if set) so a direct hit to the endpoint still chains through
oauth2-proxy + Cognito.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…STS to PLATFORM_DOMAIN Reuses the bundle-wide PLATFORM_DOMAIN env var (set by foss-server-bundle/platform.sh) instead of inventing a per-app allowlist env. Same suffix-match semantics with dot-boundary enforcement; less surface area for operator misconfiguration. PLATFORM_DOMAIN unset → endpoint still flushes the Django session, just rejects every ?next=. Tests updated to mock the new attribute.
5ee0985 to
c1f3b36
Compare
| return HttpResponseBadRequest( | ||
| "next= target host is not a subdomain of PLATFORM_DOMAIN" | ||
| ) | ||
| return HttpResponseRedirect(next_url) |
c1f3b36 to
ea370b0
Compare
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.
ea370b0 to
d695600
Compare
Author
|
Closing — too much force-push churn. Reopening as fresh PR with a clean single-commit history. |
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
SignOutAuthEndpointso the foss-server-bundle portal's "Log out of all apps" redirect chain can include Plane while reusing Plane's existing logout primitive (django.contrib.auth.logout()).Single endpoint, two methods, single source of logout truth. No new view, no duplicated logic.
What changed
Behaviour
`GET /auth/sign-out/?next=<absolute_url>`:
Why GET + CSRF-exempt
The portal builds a chained URL (`docs. → pm. → design. → ...`) and navigates the browser through each app. Cross-origin POSTs without CORS handshake fail; cross-origin CSRF tokens aren't shareable. The only chain-friendly shape is GET. Residual force-logout (`
`) loses only the session — auto re-auth via ForwardAuth — accepted as low impact.
Diff size
+212 / -11 across 5 files, of which:
Tests
`apps/api/plane/tests/unit/views/test_signout.py` — 6 new GET cases (existing 4 POST cases unchanged):
Config
Out of scope
History
Previously opened as #31, closed. Reopened as #34 with a new view + duplicated logic. Refactored per review feedback to fold into the existing endpoint — net diff dropped from ~440 lines to +212 / -11.