Skip to content

fix(auth): land on portal after logout, drop dependency on OIDC_LOGOU…#4

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

fix(auth): land on portal after logout, drop dependency on OIDC_LOGOU…#4
UsamaSadiq merged 1 commit into
foss-mainfrom
feat/callback-independent-logout

Conversation

@awais786
Copy link
Copy Markdown

…T_URI

AuthStore.logout() was setting logoutRedirectUri from env.OIDC_LOGOUT_URI, which pointed at a Cognito hosted /logout endpoint. In this deployment the app client has no hosted /logout, so the Cognito session always survives logout and the env-wired redirect was dead weight when unset and pointed at an unregistered URL when set.

Simplified to derive the portal host from the current URL:

  • Rewrite "foss-." -> "foss." and assign that as logoutRedirectUri whenever the user initiates a logout.
  • 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.

Logout.tsx no longer branches on env.OIDC_LOGOUT_URI; AuthStore always sets logoutRedirectUri and the unauthenticated branch in Authenticated.tsx performs the navigation.

Motivation / Background

This Pull Request has been created because:

  • Resolves #issue-id

Detail

This Pull Request changes:

Additional information

TIP: Provide additional information such as screenshots, benchmarks, reference to other repositories or alternative solutions

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature
  • CHANGELOG files are updated for the behavior change or additional feature (minor bug fixes and documentation changes should not be included)
  • This PR contains API changes and API documentation is updated accordingly (for critical or behavior change, please inform related parties about them).

…T_URI

AuthStore.logout() was setting logoutRedirectUri from env.OIDC_LOGOUT_URI,
which pointed at a Cognito hosted /logout endpoint. In this deployment the
app client has no hosted /logout, so the Cognito session always survives
logout and the env-wired redirect was dead weight when unset and pointed
at an unregistered URL when set.

Simplified to derive the portal host from the current URL:

  - Rewrite "foss-<app>.<domain>" -> "foss.<domain>" and assign that as
    logoutRedirectUri whenever the user initiates a logout.
  - 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.

Logout.tsx no longer branches on env.OIDC_LOGOUT_URI; AuthStore always
sets logoutRedirectUri and the unauthenticated branch in
Authenticated.tsx performs the navigation.
@UsamaSadiq UsamaSadiq merged commit 8b703b1 into foss-main Apr 17, 2026
12 checks passed
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 logout flow to stop depending on OIDC_LOGOUT_URI (Cognito hosted logout) and instead redirect users to a derived “portal” host so they land outside ForwardAuth after logging out.

Changes:

  • Compute logoutRedirectUri during user-initiated logout by rewriting the current hostname to a portal host.
  • Simplify Logout.tsx to no longer branch on env.OIDC_LOGOUT_URI, relying on post-logout navigation elsewhere.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
app/stores/AuthStore.ts Derives a portal-based logoutRedirectUri from the current URL during user-initiated logout.
app/scenes/Logout.tsx Removes env-based branching and now returns null after invoking logout.

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

Comment thread app/scenes/Logout.tsx
Comment on lines 6 to +10
void auth.logout({ userInitiated: true });

if (env.OIDC_LOGOUT_URI) {
return null; // user will be redirected to logout URI after logout
}
return <Redirect to={logoutPath()} />;
// AuthStore.logout() always sets logoutRedirectUri to the portal host; the
// unauthenticated branch in Authenticated.tsx performs the actual navigation.
return null;
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.

/logout is routed outside the <Authenticated> wrapper (see app/routes/index.tsx), so returning null here leaves users on a blank page and the redirect in Authenticated.tsx will never run. This component should perform the navigation itself after auth.logout completes (e.g., window.location.href = auth.logoutRedirectUri or a router redirect fallback).

Copilot uses AI. Check for mistakes.
Comment thread app/stores/AuthStore.ts
Comment on lines +357 to +358
const portalHost = window.location.hostname.replace(/^[^.]*\./, "foss.");
this.logoutRedirectUri = `${window.location.protocol}//${portalHost}`;
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.

Building logoutRedirectUri from window.location.hostname drops any non-default port (e.g. localhost:3000) and the regex replacement will rewrite the first subdomain label for any hostname, not just the expected foss-<app>.<domain> pattern. Consider deriving from window.location.host (preserve port) and only rewriting when the current hostname matches the intended pattern (otherwise fall back to the current host) to avoid redirecting to a non-existent portal host.

Suggested change
const portalHost = window.location.hostname.replace(/^[^.]*\./, "foss.");
this.logoutRedirectUri = `${window.location.protocol}//${portalHost}`;
const { protocol, hostname, host } = window.location;
const portalHost = /^foss-[^.]+\./.test(hostname)
? host.replace(/^foss-[^.]+\./, "foss.")
: host;
this.logoutRedirectUri = `${protocol}//${portalHost}`;

Copilot uses AI. Check for mistakes.
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