fix(auth): keep logout on portal when Cognito hosted /logout is absent#4
Merged
Merged
Conversation
When VITE_OIDC_LOGOUT_URL is empty (deployments without a Cognito hosted /logout endpoint), the signOut else-branch used window.location.origin as the post-logout `rd=` target. That's the Plane host itself, which is behind ForwardAuth — oauth2-proxy immediately re-authed via the still-valid Cognito session cookie and landed the user back on the dashboard. Use VITE_LOGOUT_REDIRECT_URL (the platform landing page, not behind auth) as the fallback instead, matching the with-Cognito branch's landing target.
UsamaSadiq
approved these changes
Apr 16, 2026
awais786
added a commit
that referenced
this pull request
May 16, 2026
All four comments from the PR #32 review are valid. Fixes: #1 (sso-audit.sh:29) — Row 14 description claimed three checks (no /oauth2/sign_out, portal-host redirect, no POST /auth/sign-out/) but the function only verifies the first. Narrows the comment to match what's actually implemented. The other two properties from logout-flow spec need runtime context or AST analysis; not in scope for this deterministic gate. #2 (sso-auth.sh:131) — Row 20 detection was brittle: - Single-line regex missed parenthesized multiline imports (`from django.contrib.auth import (\n login,\n logout,\n)`) - Call regex required exact `logout(request)` — missed whitespace (`logout( request )`), keyword form (`logout(request=request)`), and aliased call sites (`auth_logout(request)`) Could false-fail on legitimate fixes → block merges spuriously. New detection uses Python's `ast` module to parse the proxy_auth.py import block and extract the in-scope name for django.contrib.auth.logout (alias or plain). Then a whitespace-tolerant grep checks for a call to that name. Handles every variant in one pass without writing multi-line regex in bash. Verified against a synthetic fixture with aliased + parenthesized + whitespaced call. #3 (sso-audit.sh:191) — Output referenced `skills/app-rules/SKILL.md`, which lives in awais786/sso-rules-moneta, not in Pressingly/plane. Confused readers of the sticky PR comment. Replaced both repo- relative references with full GitHub URLs so the links work from the PR view. #4 (.github/workflows/sso-audit.yml:62) — `marocchino/sticky-pull- request-comment@v2` needs `pull-requests: write`. PRs from external forks get a read-only GITHUB_TOKEN regardless of declared permissions, and the action errors out, marking the whole workflow failed even when the audit passed. Two-layer fix: - Skip the comment step on fork PRs via `github.event.pull_request.head.repo.full_name == github.repository` - Add `continue-on-error: true` as a defensive belt so other comment-posting failures (rate limit, GitHub API blip) don't fail the audit gate either The audit gate itself (final exit-1 step) is unaffected — still blocks merge on security-critical violations regardless of comment posting. Local dry-run on foss-main still produces the expected red ❌ on row 20 with the new alias-aware detection. Switching to the fix branch detects the import (as `logout`) and the call site correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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
2 participants
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.
When VITE_OIDC_LOGOUT_URL is empty (deployments without a Cognito hosted /logout endpoint), the signOut else-branch used window.location.origin as the post-logout
rd=target. That's the Plane host itself, which is behind ForwardAuth — oauth2-proxy immediately re-authed via the still-valid Cognito session cookie and landed the user back on the dashboard.Use VITE_LOGOUT_REDIRECT_URL (the platform landing page, not behind auth) as the fallback instead, matching the with-Cognito branch's landing target.
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References