diff --git a/runtime/routing/authenticatedLoader.test.ts b/runtime/routing/authenticatedLoader.test.ts index 1a31902a..f5e04b68 100644 --- a/runtime/routing/authenticatedLoader.test.ts +++ b/runtime/routing/authenticatedLoader.test.ts @@ -17,6 +17,13 @@ const mockRedirect = redirect as jest.MockedFunction; const mockLocationAssign = jest.fn(); +function makeLoaderArgs(url: string) { + return { + request: { url } as any, + params: {}, + }; +} + describe('authenticatedLoader', () => { const originalLocation = global.location; @@ -54,44 +61,44 @@ describe('authenticatedLoader', () => { avatar: 'https://example.com/avatar.jpg', }); - const result = authenticatedLoader(); + const result = authenticatedLoader(makeLoaderArgs('https://example.com/dashboard')); expect(result).toBeNull(); expect(mockLocationAssign).not.toHaveBeenCalled(); expect(mockGetUrlByRouteRole).not.toHaveBeenCalled(); }); - it('returns SPA redirect with relative ?next for internal login route', () => { + it('returns SPA redirect with ?next derived from request URL, not global.location', () => { mockGetAuthenticatedUser.mockReturnValue(null); mockGetUrlByRouteRole.mockReturnValue('/login'); - authenticatedLoader(); + authenticatedLoader(makeLoaderArgs('https://example.com/dashboard')); - expect(mockRedirect).toHaveBeenCalledWith('/login?next=%2Fcurrent-page'); + expect(mockRedirect).toHaveBeenCalledWith('/login?next=%2Fdashboard'); expect(mockLocationAssign).not.toHaveBeenCalled(); }); it('calls location.assign for a cross-origin login route', () => { mockGetAuthenticatedUser.mockReturnValue(null); mockGetUrlByRouteRole.mockReturnValue('https://auth.example.com/login'); - mockGetLoginRedirectUrl.mockReturnValue('https://auth.example.com/login?next=%2Fcurrent-page'); + mockGetLoginRedirectUrl.mockReturnValue('https://auth.example.com/login?next=%2Fdashboard'); - const result = authenticatedLoader(); + const result = authenticatedLoader(makeLoaderArgs('https://example.com/dashboard')); - expect(mockGetLoginRedirectUrl).toHaveBeenCalledWith('https://example.com/current-page'); - expect(mockLocationAssign).toHaveBeenCalledWith('https://auth.example.com/login?next=%2Fcurrent-page'); + expect(mockGetLoginRedirectUrl).toHaveBeenCalledWith('https://example.com/dashboard'); + expect(mockLocationAssign).toHaveBeenCalledWith('https://auth.example.com/login?next=%2Fdashboard'); expect(result).toBeInstanceOf(Promise); }); it('falls back to location.assign when no login role is found', () => { mockGetAuthenticatedUser.mockReturnValue(null); mockGetUrlByRouteRole.mockReturnValue(null); - mockGetLoginRedirectUrl.mockReturnValue('https://auth.example.com/login?next=%2Fcurrent-page'); + mockGetLoginRedirectUrl.mockReturnValue('https://auth.example.com/login?next=%2Fdashboard'); - const result = authenticatedLoader(); + const result = authenticatedLoader(makeLoaderArgs('https://example.com/dashboard')); - expect(mockGetLoginRedirectUrl).toHaveBeenCalledWith('https://example.com/current-page'); - expect(mockLocationAssign).toHaveBeenCalledWith('https://auth.example.com/login?next=%2Fcurrent-page'); + expect(mockGetLoginRedirectUrl).toHaveBeenCalledWith('https://example.com/dashboard'); + expect(mockLocationAssign).toHaveBeenCalledWith('https://auth.example.com/login?next=%2Fdashboard'); expect(result).toBeInstanceOf(Promise); }); }); diff --git a/runtime/routing/authenticatedLoader.ts b/runtime/routing/authenticatedLoader.ts index 56de0434..284cc93e 100644 --- a/runtime/routing/authenticatedLoader.ts +++ b/runtime/routing/authenticatedLoader.ts @@ -1,28 +1,29 @@ -import { redirect } from 'react-router'; +import { LoaderFunctionArgs, redirect } from 'react-router'; import { getAuthenticatedUser, getLoginRedirectUrl } from '../auth'; import { getUrlByRouteRole } from './utils'; -const LOGIN_ROLE = 'org.openedx.frontend.role.login'; +const loginRole = 'org.openedx.frontend.role.login'; -export default function authenticatedLoader() { +export default function authenticatedLoader({ request }: LoaderFunctionArgs) { const authenticatedUser = getAuthenticatedUser(); if (authenticatedUser !== null) { return null; } - const loginUrl = getUrlByRouteRole(LOGIN_ROLE); + const requestUrl = new URL(request.url); + const loginUrl = getUrlByRouteRole(loginRole); // Internal login route → SPA redirect with a relative ?next so the login // page can navigate() back without a full page refresh. if (loginUrl?.startsWith('/')) { - return redirect(`${loginUrl}?next=${encodeURIComponent(global.location.pathname)}`); + return redirect(`${loginUrl}?next=${encodeURIComponent(requestUrl.pathname)}`); } // No login role found (or it's defined as an external route, which is not // supported). Use loginUrl from siteConfig and the full href for the return // path so the login service redirects back to the correct origin after // login. - const fullLoginUrl = getLoginRedirectUrl(global.location.href); + const fullLoginUrl = getLoginRedirectUrl(requestUrl.href); // Return a never-resolving promise so React Router keeps waiting (and does // not attempt to render the route) while the browser navigates away.