Skip to content

fix(auth): land on portal after logout, drop unreachable Cognito hop#5

Merged
UsamaSadiq merged 2 commits into
foss-mainfrom
feat/callback-independent-logout
Apr 17, 2026
Merged

fix(auth): land on portal after logout, drop unreachable Cognito hop#5
UsamaSadiq merged 2 commits into
foss-mainfrom
feat/callback-independent-logout

Conversation

@awais786
Copy link
Copy Markdown

The logout event was redirecting to MPASS_SIGNOUT_URL, which pointed at oauth2-proxy's /sign_out with an rd= that depended on Cognito hosted /logout being available. In this deployment the app client has no hosted /logout endpoint, so the Cognito session always survives logout and the extra redirect layer was dead weight. The URL was injected at container start via nginx-entrypoint.sh and read from a browser global (penpotMpassSignoutUrl), coupling the frontend bundle to identity- provider config baked in by an external script.

Simplified to derive the portal host from the current URL:

  • Rewrite "foss-." -> "foss." and redirect there after the native :logout cmd clears the Penpot session.
  • The portal host is outside ForwardAuth, so the user lands on the landing page instead of being silently re-authed into the dashboard. Re-auth still happens the next time the user clicks into a gated app, which is the expected behavior while Cognito hosted /logout is absent.

The cf/mpass-signout-url var remains defined in config.cljs for backwards compatibility with the nginx injection script but is no longer consumed by the logout flow.

Related Ticket

Summary

Steps to reproduce

Checklist

  • Choose the correct target branch; use develop by default.
  • Provide a brief summary of the changes introduced.
  • Add a detailed explanation of how to reproduce the issue and/or verify the fix, if applicable.
  • Include screenshots or videos, if applicable.
  • Add or modify existing integration tests in case of bugs or new features, if applicable.
  • Refactor any modified SCSS files following the refactor guide.
  • Check CI passes successfully.
  • Update the CHANGES.md file, referencing the related GitHub issue, if applicable.

The logout event was redirecting to MPASS_SIGNOUT_URL, which pointed at
oauth2-proxy's /sign_out with an rd= that depended on Cognito hosted
/logout being available. In this deployment the app client has no
hosted /logout endpoint, so the Cognito session always survives logout
and the extra redirect layer was dead weight. The URL was injected at
container start via nginx-entrypoint.sh and read from a browser global
(penpotMpassSignoutUrl), coupling the frontend bundle to identity-
provider config baked in by an external script.

Simplified to derive the portal host from the current URL:

  - Rewrite "foss-<app>.<domain>" -> "foss.<domain>" and redirect there
    after the native :logout cmd clears the Penpot session.
  - The portal host is outside ForwardAuth, so the user lands on the
    landing page instead of being silently re-authed into the
    dashboard. Re-auth still happens the next time the user clicks
    into a gated app, which is the expected behavior while Cognito
    hosted /logout is absent.

The cf/mpass-signout-url var remains defined in config.cljs for
backwards compatibility with the nginx injection script but is no
longer consumed by the logout flow.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the frontend logout flow to stop relying on an externally injected MPASS/Cognito signout URL and instead redirect users to a derived “portal” host after the native :logout completes.

Changes:

  • Remove usage of cf/mpass-signout-url for logout redirection.
  • Derive a portal redirect target from js/location by rewriting the current hostname, and always redirect there on logout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread frontend/src/app/main/data/auth.cljs Outdated
Comment on lines +262 to +268
;; Rewrite "foss-<app>.<domain>" → "foss.<domain>" so we land on the portal
;; (outside ForwardAuth) instead of Penpot's own root, which would silently re-auth.
hostname (.-hostname js/location)
protocol (.-protocol js/location)
portal-host (.replace hostname #"^[^.]*\." "foss.")
portal-uri (str protocol "//" portal-host)
logged-out-ev (logged-out {:redirect-uri portal-uri})]
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hostname rewrite is applied unconditionally to any hostname (replaces the first label with "foss"), but the comment/PR intent is specifically "foss-." → "foss.". As-is, deployments served from e.g. "penpot.example.com" would redirect logout to "foss.example.com" (likely wrong/nonexistent). Consider guarding this transform (only rewrite when the host starts with the expected prefix or already equals the portal host), and otherwise fall back to the previous behavior (no redirect-uri → /auth/login).

Suggested change
;; Rewrite "foss-<app>.<domain>" → "foss.<domain>" so we land on the portal
;; (outside ForwardAuth) instead of Penpot's own root, which would silently re-auth.
hostname (.-hostname js/location)
protocol (.-protocol js/location)
portal-host (.replace hostname #"^[^.]*\." "foss.")
portal-uri (str protocol "//" portal-host)
logged-out-ev (logged-out {:redirect-uri portal-uri})]
;; Rewrite only "foss-<app>.<domain>" → "foss.<domain>" so we land on the portal
;; (outside ForwardAuth) instead of Penpot's own root, which would silently re-auth.
;; For any other hostname, keep the previous behavior and fall back to /auth/login.
hostname (.-hostname js/location)
protocol (.-protocol js/location)
portal-host (cond
(re-matches #"^foss\..+" hostname)
hostname
(re-matches #"^foss-[^.]+\..+" hostname)
(.replace hostname #"^foss-[^.]+\." "foss.")
:else
nil)
portal-uri (when portal-host
(str protocol "//" portal-host))
logged-out-ev (logged-out (cond-> {}
portal-uri (assoc :redirect-uri portal-uri)))]

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Usama Sadiq <usama7274@gmail.com>
@UsamaSadiq UsamaSadiq merged commit 0e29cd8 into foss-main Apr 17, 2026
0 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants