diff --git a/apps/frontend/src/components/layout/layout.context.tsx b/apps/frontend/src/components/layout/layout.context.tsx index b9eeaccd70..efc81c849d 100644 --- a/apps/frontend/src/components/layout/layout.context.tsx +++ b/apps/frontend/src/components/layout/layout.context.tsx @@ -5,7 +5,10 @@ import { FetchWrapperComponent } from '@gitroom/helpers/utils/custom.fetch'; import { deleteDialog } from '@gitroom/react/helpers/delete.dialog'; import { useReturnUrl } from '@gitroom/frontend/app/(app)/auth/return.url.component'; import { useVariables } from '@gitroom/react/helpers/variable.context'; -import { shouldPreserveOAuthConsentUnauthorized } from './oauth-consent-unauthorized'; +import { + shouldHandleGlobalLogout, + shouldPreserveOAuthConsentUnauthorized, +} from './oauth-consent-unauthorized'; export default function LayoutContext(params: { children: ReactNode }) { if (params?.children) { // eslint-disable-next-line react/no-children-prop @@ -43,6 +46,15 @@ function LayoutContextInner(params: { children: ReactNode }) { response?.headers?.get('Impersonate'); const logout = response?.headers?.get('logout') || response?.headers?.get('Logout'); + if ( + shouldPreserveOAuthConsentUnauthorized( + url, + options.method, + response.status + ) + ) { + return true; + } if (headerAuth) { setCookie('auth', headerAuth, 365); } @@ -52,7 +64,15 @@ function LayoutContextInner(params: { children: ReactNode }) { if (impersonate) { setCookie('impersonate', impersonate, 365); } - if (logout && !isSecured) { + if ( + shouldHandleGlobalLogout( + url, + options.method, + response.status, + Boolean(logout) + ) && + !isSecured + ) { setCookie('auth', '', -10); setCookie('showorg', '', -10); setCookie('impersonate', '', -10); @@ -82,11 +102,12 @@ function LayoutContextInner(params: { children: ReactNode }) { } if ( - (response.status === 401 || response?.headers?.get('logout')) && - !shouldPreserveOAuthConsentUnauthorized( + response.status === 401 || + shouldHandleGlobalLogout( url, + options.method, response.status, - Boolean(response?.headers?.get('logout')) + Boolean(logout) ) ) { if (!isSecured) { diff --git a/apps/frontend/src/components/layout/oauth-consent-unauthorized.ts b/apps/frontend/src/components/layout/oauth-consent-unauthorized.ts index 499352718d..cef98f0b4f 100644 --- a/apps/frontend/src/components/layout/oauth-consent-unauthorized.ts +++ b/apps/frontend/src/components/layout/oauth-consent-unauthorized.ts @@ -1,11 +1,26 @@ export function shouldPreserveOAuthConsentUnauthorized( url: string, + method: string | undefined, + status: number +) { + if (status !== 401 || method?.toUpperCase() !== 'POST') return false; + try { + return ( + new URL(url, 'https://oauth.invalid').pathname === '/oauth/authorize' + ); + } catch { + return false; + } +} + +export function shouldHandleGlobalLogout( + url: string, + method: string | undefined, status: number, hasLogoutHeader: boolean ) { return ( - !hasLogoutHeader && - status === 401 && - url.split('?', 1)[0] === '/oauth/authorize' + hasLogoutHeader && + !shouldPreserveOAuthConsentUnauthorized(url, method, status) ); } diff --git a/tests/bootstrap-oauth-consent-error.spec.ts b/tests/bootstrap-oauth-consent-error.spec.ts index 32293623c6..b780628b12 100644 --- a/tests/bootstrap-oauth-consent-error.spec.ts +++ b/tests/bootstrap-oauth-consent-error.spec.ts @@ -2,7 +2,10 @@ import { authorizationActionResult, CONSENT_SESSION_ERROR, } from '../apps/frontend/src/app/(app)/oauth/authorize/authorization-action-result'; -import { shouldPreserveOAuthConsentUnauthorized } from '../apps/frontend/src/components/layout/oauth-consent-unauthorized'; +import { + shouldHandleGlobalLogout, + shouldPreserveOAuthConsentUnauthorized, +} from '../apps/frontend/src/components/layout/oauth-consent-unauthorized'; describe('First-party OAuth consent error UI', () => { it('keeps superseded tab A on a visible safe error instead of redirecting', () => { @@ -23,20 +26,32 @@ describe('First-party OAuth consent error UI', () => { it('keeps a failed consent POST on the OAuth page instead of sending it to launches', () => { expect( - shouldPreserveOAuthConsentUnauthorized('/oauth/authorize', 401, false) + shouldPreserveOAuthConsentUnauthorized('/oauth/authorize', 'POST', 401) ).toBe(true); expect( shouldPreserveOAuthConsentUnauthorized( - '/oauth/authorize?state=test', - 401, - false + 'https://beta-post.crove.com/oauth/authorize?state=test', + 'POST', + 401 ) ).toBe(true); expect( - shouldPreserveOAuthConsentUnauthorized('/user/profile', 401, false) + shouldPreserveOAuthConsentUnauthorized('/oauth/authorize', 'GET', 401) + ).toBe(false); + expect( + shouldPreserveOAuthConsentUnauthorized('/user/profile', 'POST', 401) + ).toBe(false); + expect( + shouldPreserveOAuthConsentUnauthorized('/oauth/authorize', 'POST', 500) ).toBe(false); expect( - shouldPreserveOAuthConsentUnauthorized('/oauth/authorize', 401, true) + shouldHandleGlobalLogout('/oauth/authorize', 'POST', 401, true) ).toBe(false); + expect(shouldHandleGlobalLogout('/oauth/authorize', 'GET', 401, true)).toBe( + true + ); + expect(shouldHandleGlobalLogout('/user/profile', 'POST', 401, true)).toBe( + true + ); }); });