fix: Fix multiple bugs with new authentication flow
This commit is contained in:
@@ -6,7 +6,54 @@ import {
|
||||
} from 'vitest';
|
||||
|
||||
import { UsersActions } from '../../../../store/users/users.actions';
|
||||
import { waitForAuthenticationOutcome } from './auth-navigation.rules';
|
||||
import {
|
||||
buildLoginReturnQueryParams,
|
||||
resolveSafeReturnUrl,
|
||||
waitForAuthenticationOutcome
|
||||
} from './auth-navigation.rules';
|
||||
|
||||
describe('resolveSafeReturnUrl', () => {
|
||||
it('returns the requested in-app path unchanged', () => {
|
||||
expect(resolveSafeReturnUrl('/servers')).toBe('/servers');
|
||||
expect(resolveSafeReturnUrl('/room/abc')).toBe('/room/abc');
|
||||
});
|
||||
|
||||
it('unwraps nested login returnUrl chains to the original destination', () => {
|
||||
const nested = '/login?returnUrl=%2Flogin%3FreturnUrl%3D%252Fservers';
|
||||
|
||||
expect(resolveSafeReturnUrl(nested)).toBe('/servers');
|
||||
expect(resolveSafeReturnUrl(`/login?returnUrl=${encodeURIComponent(nested)}`)).toBe('/servers');
|
||||
});
|
||||
|
||||
it('falls back to dashboard for auth-only return targets', () => {
|
||||
expect(resolveSafeReturnUrl('/login')).toBe('/dashboard');
|
||||
expect(resolveSafeReturnUrl('/register')).toBe('/dashboard');
|
||||
expect(resolveSafeReturnUrl(null)).toBe('/dashboard');
|
||||
});
|
||||
|
||||
it('rejects open redirects and protocol-relative paths', () => {
|
||||
expect(resolveSafeReturnUrl('//evil.example/phish')).toBe('/dashboard');
|
||||
expect(resolveSafeReturnUrl('https://evil.example/phish')).toBe('/dashboard');
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildLoginReturnQueryParams', () => {
|
||||
it('preserves a safe destination when redirecting from protected routes', () => {
|
||||
expect(buildLoginReturnQueryParams('/servers')).toEqual({ returnUrl: '/servers' });
|
||||
});
|
||||
|
||||
it('does not nest login returnUrl values', () => {
|
||||
expect(buildLoginReturnQueryParams('/login?returnUrl=%2Fservers')).toEqual({ returnUrl: '/servers' });
|
||||
expect(buildLoginReturnQueryParams('/login?returnUrl=%2Flogin%3FreturnUrl%3D%252Fservers')).toEqual({
|
||||
returnUrl: '/servers'
|
||||
});
|
||||
});
|
||||
|
||||
it('omits returnUrl when there is no meaningful destination', () => {
|
||||
expect(buildLoginReturnQueryParams('/login')).toEqual({});
|
||||
expect(buildLoginReturnQueryParams('/register')).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
describe('waitForAuthenticationOutcome', () => {
|
||||
it('resolves when authentication storage preparation succeeds', async () => {
|
||||
|
||||
@@ -8,10 +8,88 @@ import {
|
||||
import { UsersActions } from '../../../../store/users/users.actions';
|
||||
import type { User } from '../../../../shared-kernel';
|
||||
|
||||
export const DEFAULT_POST_AUTH_URL = '/dashboard';
|
||||
|
||||
const AUTH_ROUTE_PATHS = new Set(['/login', '/register']);
|
||||
const MAX_RETURN_URL_DEPTH = 10;
|
||||
|
||||
export type AuthenticationOutcome =
|
||||
| { kind: 'success'; user: User }
|
||||
| { kind: 'failure'; error: string };
|
||||
|
||||
export function isAuthRoutePath(path: string): boolean {
|
||||
return AUTH_ROUTE_PATHS.has(path);
|
||||
}
|
||||
|
||||
export function getRoutePathFromUrl(url: string): string {
|
||||
if (!url) {
|
||||
return '/';
|
||||
}
|
||||
|
||||
const [path] = url.split(/[?#]/, 1);
|
||||
|
||||
return path || '/';
|
||||
}
|
||||
|
||||
export function extractReturnUrlParam(url: string): string | null {
|
||||
const queryStart = url.indexOf('?');
|
||||
|
||||
if (queryStart === -1) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const hashStart = url.indexOf('#', queryStart + 1);
|
||||
const query = hashStart === -1
|
||||
? url.slice(queryStart + 1)
|
||||
: url.slice(queryStart + 1, hashStart);
|
||||
|
||||
return new URLSearchParams(query).get('returnUrl');
|
||||
}
|
||||
|
||||
export function resolveSafeReturnUrl(
|
||||
url: string | null | undefined,
|
||||
fallback = DEFAULT_POST_AUTH_URL
|
||||
): string {
|
||||
let candidate = url?.trim() ?? '';
|
||||
let depth = 0;
|
||||
|
||||
while (candidate && depth < MAX_RETURN_URL_DEPTH) {
|
||||
if (!candidate.startsWith('/') || candidate.startsWith('//')) {
|
||||
return fallback;
|
||||
}
|
||||
|
||||
const path = getRoutePathFromUrl(candidate);
|
||||
|
||||
if (!isAuthRoutePath(path)) {
|
||||
return candidate;
|
||||
}
|
||||
|
||||
const nestedReturnUrl = extractReturnUrlParam(candidate)?.trim();
|
||||
|
||||
if (!nestedReturnUrl) {
|
||||
return fallback;
|
||||
}
|
||||
|
||||
candidate = nestedReturnUrl;
|
||||
depth += 1;
|
||||
}
|
||||
|
||||
return fallback;
|
||||
}
|
||||
|
||||
export function buildLoginReturnQueryParams(
|
||||
currentUrl: string,
|
||||
fallback = DEFAULT_POST_AUTH_URL
|
||||
): { returnUrl?: string } {
|
||||
const safeReturnUrl = resolveSafeReturnUrl(currentUrl, fallback);
|
||||
|
||||
if (safeReturnUrl === fallback) {
|
||||
return {};
|
||||
}
|
||||
|
||||
return { returnUrl: safeReturnUrl };
|
||||
}
|
||||
|
||||
export function waitForAuthenticationOutcome(
|
||||
actions$: Observable<{ type: string; user?: User; error?: string }>
|
||||
): Observable<AuthenticationOutcome> {
|
||||
|
||||
Reference in New Issue
Block a user