From 3356c823dafda02e8b79e9686a71a8946c0b73c3 Mon Sep 17 00:00:00 2001 From: Code Fix Bot Date: Sat, 29 Aug 2026 14:36:13 +0100 Subject: [PATCH] fix(auth, a11y): oauth redirect validation and focus-trap test typing Issue #785 - W2-F-034: Validate OAuth callback redirect target - Added getSafeOAuthReturnPath() validator to prevent open redirect vulnerabilities - Validates returnPath starts with / and is not protocol-relative (//) or absolute URL - Rejects javascript:, data:, http://, https:// schemes - Falls back to / for any invalid redirect path Issue #779 - W2-F-028: Clean up focus-trap hook test typing - Removed unused 'vi' import from vitest - Replaced 6 'any' type casts with proper React.RefObject typing - Improved type safety across all 6 test cases - Ensures test code passes strict linting requirements --- app/[locale]/auth/oauth/callback/page.tsx | 29 ++- hooks/__tests__/use-focus-trap.test.ts | 225 ++++++++++++---------- 2 files changed, 146 insertions(+), 108 deletions(-) diff --git a/app/[locale]/auth/oauth/callback/page.tsx b/app/[locale]/auth/oauth/callback/page.tsx index b3dadb31..3f218d53 100644 --- a/app/[locale]/auth/oauth/callback/page.tsx +++ b/app/[locale]/auth/oauth/callback/page.tsx @@ -1,7 +1,28 @@ "use client"; -import { useSearchParams, useRouter } from 'next/navigation'; -import { Suspense, useEffect, useRef, useState } from 'react'; +import { useSearchParams, useRouter } from "next/navigation"; +import { Suspense, useEffect, useRef, useState } from "react"; + +export function getSafeOAuthReturnPath(value: string | null): string { + if (!value) { + return "/"; + } + + const trimmedValue = value.trim(); + + if ( + !trimmedValue || + !trimmedValue.startsWith("/") || + trimmedValue.startsWith("//") || + /^https?:\/\//i.test(trimmedValue) || + /^javascript:/i.test(trimmedValue) || + /^data:/i.test(trimmedValue) + ) { + return "/"; + } + + return trimmedValue; +} function OAuthCallbackContent() { const searchParams = useSearchParams(); @@ -37,7 +58,9 @@ function OAuthCallbackContent() { setStatus("success"); - const returnPath = sessionStorage.getItem("oauth_return_path") || "/"; + const returnPath = getSafeOAuthReturnPath( + sessionStorage.getItem("oauth_return_path"), + ); sessionStorage.removeItem("oauth_return_path"); router.replace(returnPath); }, [searchParams, router]); diff --git a/hooks/__tests__/use-focus-trap.test.ts b/hooks/__tests__/use-focus-trap.test.ts index e960e7f9..22e6c3dd 100644 --- a/hooks/__tests__/use-focus-trap.test.ts +++ b/hooks/__tests__/use-focus-trap.test.ts @@ -1,192 +1,207 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' -import { renderHook, act } from '@testing-library/react' -import { useFocusTrap } from '../use-focus-trap' -import React from 'react' +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useFocusTrap } from "../use-focus-trap"; +import React from "react"; -describe('useFocusTrap', () => { - let container: HTMLDivElement +describe("useFocusTrap", () => { + let container: HTMLDivElement; beforeEach(() => { // Create a test container with focusable elements - container = document.createElement('div') + container = document.createElement("div"); container.innerHTML = ` Link 1 - ` - document.body.appendChild(container) - }) + `; + document.body.appendChild(container); + }); afterEach(() => { - document.body.removeChild(container) - }) + document.body.removeChild(container); + }); - it('traps focus on Tab from last element to first', () => { - const containerRef = React.createRef() - // Mock the ref - ;(containerRef as any).current = container + it("traps focus on Tab from last element to first", () => { + const containerRef: React.RefObject = { + current: container, + }; - renderHook(() => useFocusTrap(containerRef, { isActive: true })) + renderHook(() => useFocusTrap(containerRef, { isActive: true })); - const lastButton = container.querySelector('#button-2') as HTMLButtonElement - lastButton.focus() - expect(document.activeElement).toBe(lastButton) + const lastButton = container.querySelector( + "#button-2", + ) as HTMLButtonElement; + lastButton.focus(); + expect(document.activeElement).toBe(lastButton); // Simulate Tab key on last element - const event = new KeyboardEvent('keydown', { - key: 'Tab', + const event = new KeyboardEvent("keydown", { + key: "Tab", bubbles: true, - }) + }); act(() => { - container.dispatchEvent(event) - }) + container.dispatchEvent(event); + }); // Focus should wrap to first element - const firstButton = container.querySelector('#button-1') as HTMLButtonElement - expect(document.activeElement).toBe(firstButton) - }) + const firstButton = container.querySelector( + "#button-1", + ) as HTMLButtonElement; + expect(document.activeElement).toBe(firstButton); + }); - it('traps focus on Shift+Tab from first element to last', () => { - const containerRef = React.createRef() - ;(containerRef as any).current = container + it("traps focus on Shift+Tab from first element to last", () => { + const containerRef: React.RefObject = { + current: container, + }; - renderHook(() => useFocusTrap(containerRef, { isActive: true })) + renderHook(() => useFocusTrap(containerRef, { isActive: true })); - const firstButton = container.querySelector('#button-1') as HTMLButtonElement - firstButton.focus() - expect(document.activeElement).toBe(firstButton) + const firstButton = container.querySelector( + "#button-1", + ) as HTMLButtonElement; + firstButton.focus(); + expect(document.activeElement).toBe(firstButton); // Simulate Shift+Tab key on first element - const event = new KeyboardEvent('keydown', { - key: 'Tab', + const event = new KeyboardEvent("keydown", { + key: "Tab", shiftKey: true, bubbles: true, - }) + }); act(() => { - container.dispatchEvent(event) - }) + container.dispatchEvent(event); + }); // Focus should wrap to last element - const lastButton = container.querySelector('#button-2') as HTMLButtonElement - expect(document.activeElement).toBe(lastButton) - }) + const lastButton = container.querySelector( + "#button-2", + ) as HTMLButtonElement; + expect(document.activeElement).toBe(lastButton); + }); - it('does not trap focus when inactive', () => { - const containerRef = React.createRef() - ;(containerRef as any).current = container + it("does not trap focus when inactive", () => { + const containerRef: React.RefObject = { + current: container, + }; - renderHook(() => useFocusTrap(containerRef, { isActive: false })) + renderHook(() => useFocusTrap(containerRef, { isActive: false })); - const lastButton = container.querySelector('#button-2') as HTMLButtonElement - lastButton.focus() + const lastButton = container.querySelector( + "#button-2", + ) as HTMLButtonElement; + lastButton.focus(); // Simulate Tab key - const event = new KeyboardEvent('keydown', { - key: 'Tab', + const event = new KeyboardEvent("keydown", { + key: "Tab", bubbles: true, - }) + }); // Should not prevent default when inactive - expect(event.defaultPrevented).toBe(false) - }) + expect(event.defaultPrevented).toBe(false); + }); - it('finds various focusable element types', () => { - const containerRef = React.createRef() - ;(containerRef as any).current = container + it("finds various focusable element types", () => { + const containerRef: React.RefObject = { + current: container, + }; - renderHook(() => useFocusTrap(containerRef, { isActive: true })) + renderHook(() => useFocusTrap(containerRef, { isActive: true })); // Container has button, input, link, button - all focusable // If we can Tab to different elements, the focus trap is finding them - expect(true).toBe(true) // Placeholder assertion - }) + expect(true).toBe(true); // Placeholder assertion + }); - it('ignores disabled elements', () => { - const disabledContainer = document.createElement('div') + it("ignores disabled elements", () => { + const disabledContainer = document.createElement("div"); disabledContainer.innerHTML = ` - ` - document.body.appendChild(disabledContainer) + `; + document.body.appendChild(disabledContainer); - const containerRef = React.createRef() - ;(containerRef as any).current = disabledContainer + const containerRef: React.RefObject = { + current: disabledContainer, + }; - renderHook(() => useFocusTrap(containerRef, { isActive: true })) + renderHook(() => useFocusTrap(containerRef, { isActive: true })); const firstButton = disabledContainer.querySelector( - '#disabled-button-1', - ) as HTMLButtonElement + "#disabled-button-1", + ) as HTMLButtonElement; const thirdButton = disabledContainer.querySelector( - '#disabled-button-3', - ) as HTMLButtonElement + "#disabled-button-3", + ) as HTMLButtonElement; if (!firstButton || !thirdButton) { - document.body.removeChild(disabledContainer) - return + document.body.removeChild(disabledContainer); + return; } - firstButton.focus() + firstButton.focus(); // Simulate Shift+Tab from first to wrap around - const event = new KeyboardEvent('keydown', { - key: 'Tab', + const event = new KeyboardEvent("keydown", { + key: "Tab", shiftKey: true, bubbles: true, - }) + }); act(() => { - disabledContainer.dispatchEvent(event) - }) + disabledContainer.dispatchEvent(event); + }); // Should skip disabled button and focus on the last focusable (button-3) - expect(document.activeElement).toBe(thirdButton) + expect(document.activeElement).toBe(thirdButton); - document.body.removeChild(disabledContainer) - }) + document.body.removeChild(disabledContainer); + }); - it('ignores hidden elements', () => { - const hiddenContainer = document.createElement('div') + it("ignores hidden elements", () => { + const hiddenContainer = document.createElement("div"); hiddenContainer.innerHTML = ` - ` - document.body.appendChild(hiddenContainer) + `; + document.body.appendChild(hiddenContainer); - const containerRef = React.createRef() - ;(containerRef as any).current = hiddenContainer + const containerRef: React.RefObject = { + current: hiddenContainer, + }; - renderHook(() => useFocusTrap(containerRef, { isActive: true })) + renderHook(() => useFocusTrap(containerRef, { isActive: true })); const firstButton = hiddenContainer.querySelector( - '#hidden-button-1', - ) as HTMLButtonElement + "#hidden-button-1", + ) as HTMLButtonElement; const thirdButton = hiddenContainer.querySelector( - '#hidden-button-3', - ) as HTMLButtonElement + "#hidden-button-3", + ) as HTMLButtonElement; if (!firstButton || !thirdButton) { - document.body.removeChild(hiddenContainer) - return + document.body.removeChild(hiddenContainer); + return; } - firstButton.focus() + firstButton.focus(); // Simulate Shift+Tab from first to wrap around - const event = new KeyboardEvent('keydown', { - key: 'Tab', + const event = new KeyboardEvent("keydown", { + key: "Tab", shiftKey: true, bubbles: true, - }) + }); act(() => { - hiddenContainer.dispatchEvent(event) - }) + hiddenContainer.dispatchEvent(event); + }); // Should skip hidden button and focus on the last focusable (button-3) - expect(document.activeElement).toBe(thirdButton) + expect(document.activeElement).toBe(thirdButton); - document.body.removeChild(hiddenContainer) - }) -}) + document.body.removeChild(hiddenContainer); + }); +});