2.21.12 (#423)
* chore: bump version to 2.21.12 and implement safe redirect path validation - Updated application version from 2.21.11 to 2.21.12 in package.json. - Introduced `getSafeRedirectPath` and `isSafeRedirectPath` utility functions to validate user-supplied redirect URLs, enhancing security against open redirect attacks. * fix: address page reload issue in Admin tests for CI
This commit is contained in:
committed by
GitHub
parent
2f78e16dfa
commit
44137016cb
@@ -103,6 +103,9 @@ test.describe('Admin', () => {
|
||||
),
|
||||
]);
|
||||
|
||||
// TODO: find out why we need to reload the page only in CI
|
||||
await page.reload();
|
||||
|
||||
await expect(page.getByText('Banned').first()).toBeVisible();
|
||||
|
||||
await page.context().clearCookies();
|
||||
@@ -149,7 +152,8 @@ test.describe('Admin', () => {
|
||||
),
|
||||
]);
|
||||
|
||||
await page.waitForTimeout(250);
|
||||
// TODO: find out why we need to reload the page only in CI
|
||||
await page.reload();
|
||||
|
||||
// Verify ban badge is removed
|
||||
await expect(page.getByText('Banned')).not.toBeVisible();
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import Link from 'next/link';
|
||||
|
||||
import { SignInMethodsContainer } from '@kit/auth/sign-in';
|
||||
import { getSafeRedirectPath } from '@kit/shared/utils';
|
||||
import { Button } from '@kit/ui/button';
|
||||
import { Heading } from '@kit/ui/heading';
|
||||
import { Trans } from '@kit/ui/trans';
|
||||
@@ -29,7 +30,7 @@ async function SignInPage({ searchParams }: SignInPageProps) {
|
||||
|
||||
const paths = {
|
||||
callback: pathsConfig.auth.callback,
|
||||
returnPath: next || pathsConfig.app.home,
|
||||
returnPath: getSafeRedirectPath(next, pathsConfig.app.home),
|
||||
joinTeam: pathsConfig.app.joinTeam,
|
||||
};
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { redirect } from 'next/navigation';
|
||||
|
||||
import { MultiFactorChallengeContainer } from '@kit/auth/mfa';
|
||||
import { getSafeRedirectPath } from '@kit/shared/utils';
|
||||
import { checkRequiresMultiFactorAuthentication } from '@kit/supabase/check-requires-mfa';
|
||||
import { getSupabaseServerClient } from '@kit/supabase/server-client';
|
||||
|
||||
@@ -38,7 +39,7 @@ async function VerifyPage(props: Props) {
|
||||
}
|
||||
|
||||
const nextPath = (await props.searchParams).next;
|
||||
const redirectPath = nextPath ?? pathsConfig.app.home;
|
||||
const redirectPath = getSafeRedirectPath(nextPath, pathsConfig.app.home);
|
||||
|
||||
return (
|
||||
<MultiFactorChallengeContainer
|
||||
|
||||
@@ -3,6 +3,7 @@ import { Metadata } from 'next';
|
||||
import { redirect } from 'next/navigation';
|
||||
|
||||
import { AuthLayoutShell } from '@kit/auth/shared';
|
||||
import { getSafeRedirectPath } from '@kit/shared/utils';
|
||||
import { requireUser } from '@kit/supabase/require-user';
|
||||
import { getSupabaseServerClient } from '@kit/supabase/server-client';
|
||||
import { Heading } from '@kit/ui/heading';
|
||||
@@ -96,7 +97,7 @@ async function fetchData(props: IdentitiesPageProps) {
|
||||
}
|
||||
|
||||
// Get the next path from URL params (where to redirect after setup)
|
||||
const nextPath = searchParams.next || pathsConfig.app.home;
|
||||
const nextPath = getSafeRedirectPath(searchParams.next, pathsConfig.app.home);
|
||||
|
||||
// Available auth methods to add
|
||||
const showPasswordOption = authConfig.providers.password;
|
||||
|
||||
@@ -130,7 +130,8 @@ export async function GET(request: NextRequest) {
|
||||
joinUrl.searchParams.set('is_new_user', 'true');
|
||||
}
|
||||
|
||||
authCallbackUrl.searchParams.set('next', joinUrl.href);
|
||||
// Use pathname + search to create a safe relative path for validation
|
||||
authCallbackUrl.searchParams.set('next', joinUrl.pathname + joinUrl.search);
|
||||
|
||||
logger.info(
|
||||
{
|
||||
|
||||
@@ -2,6 +2,7 @@ import { redirect } from 'next/navigation';
|
||||
|
||||
import { UpdatePasswordForm } from '@kit/auth/password-reset';
|
||||
import { AuthLayoutShell } from '@kit/auth/shared';
|
||||
import { getSafeRedirectPath } from '@kit/shared/utils';
|
||||
import { requireUser } from '@kit/supabase/require-user';
|
||||
import { getSupabaseServerClient } from '@kit/supabase/server-client';
|
||||
|
||||
@@ -38,7 +39,7 @@ async function UpdatePasswordPage(props: UpdatePasswordPageProps) {
|
||||
}
|
||||
|
||||
const { callback } = await props.searchParams;
|
||||
const redirectTo = callback ?? pathsConfig.app.home;
|
||||
const redirectTo = getSafeRedirectPath(callback, pathsConfig.app.home);
|
||||
|
||||
return (
|
||||
<AuthLayoutShell Logo={Logo}>
|
||||
|
||||
@@ -4,6 +4,7 @@ import { NextResponse } from 'next/server';
|
||||
import { CsrfError, createCsrfProtect } from '@edge-csrf/nextjs';
|
||||
|
||||
import { isSuperAdmin } from '@kit/admin';
|
||||
import { getSafeRedirectPath } from '@kit/shared/utils';
|
||||
import { checkRequiresMultiFactorAuthentication } from '@kit/supabase/check-requires-mfa';
|
||||
import { createMiddlewareClient } from '@kit/supabase/middleware-client';
|
||||
|
||||
@@ -158,8 +159,10 @@ async function getPatterns() {
|
||||
// If user is logged in and does not need to verify MFA,
|
||||
// redirect to home page.
|
||||
if (!isVerifyMfa) {
|
||||
const nextPath =
|
||||
req.nextUrl.searchParams.get('next') ?? pathsConfig.app.home;
|
||||
const nextPath = getSafeRedirectPath(
|
||||
req.nextUrl.searchParams.get('next'),
|
||||
pathsConfig.app.home,
|
||||
);
|
||||
|
||||
return NextResponse.redirect(
|
||||
new URL(nextPath, req.nextUrl.origin).href,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "next-supabase-saas-kit-turbo",
|
||||
"version": "2.21.11",
|
||||
"version": "2.21.12",
|
||||
"private": true,
|
||||
"sideEffects": false,
|
||||
"engines": {
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import type { Provider } from '@supabase/supabase-js';
|
||||
|
||||
import { isBrowser } from '@kit/shared/utils';
|
||||
import { isBrowser, isSafeRedirectPath } from '@kit/shared/utils';
|
||||
import { If } from '@kit/ui/if';
|
||||
import { Separator } from '@kit/ui/separator';
|
||||
import { Trans } from '@kit/ui/trans';
|
||||
@@ -114,7 +114,8 @@ function getCallbackUrl(props: {
|
||||
const searchParams = new URLSearchParams(window.location.search);
|
||||
const next = searchParams.get('next');
|
||||
|
||||
if (next) {
|
||||
// Only pass through the next param if it's a safe internal path
|
||||
if (next && isSafeRedirectPath(next)) {
|
||||
url.searchParams.set('next', next);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
'use client';
|
||||
|
||||
import { getSafeRedirectPath } from '@kit/shared/utils';
|
||||
import { useSignOut } from '@kit/supabase/hooks/use-sign-out';
|
||||
import { Button } from '@kit/ui/button';
|
||||
import { Trans } from '@kit/ui/trans';
|
||||
@@ -16,7 +17,11 @@ export function SignOutInvitationButton(
|
||||
variant={'ghost'}
|
||||
onClick={async () => {
|
||||
await signOut.mutateAsync();
|
||||
window.location.assign(props.nextPath);
|
||||
|
||||
// Validate the path to prevent open redirect attacks
|
||||
const safePath = getSafeRedirectPath(props.nextPath, '/');
|
||||
|
||||
window.location.assign(safePath);
|
||||
}}
|
||||
>
|
||||
<Trans i18nKey={'teams:signInWithDifferentAccount'} />
|
||||
|
||||
@@ -21,3 +21,43 @@ export function formatCurrency(params: {
|
||||
currency: params.currencyCode,
|
||||
}).format(Number(params.value));
|
||||
}
|
||||
|
||||
/**
|
||||
* @name isSafeRedirectPath
|
||||
* @description Checks if a path is safe for redirects (prevents open redirect attacks).
|
||||
* Safe paths must:
|
||||
* - Start with a single `/`
|
||||
* - NOT start with `//` (protocol-relative URLs)
|
||||
* - NOT contain `://` (absolute URLs)
|
||||
* - NOT contain backslash (URL normalization attacks)
|
||||
*/
|
||||
export function isSafeRedirectPath(path: string): boolean {
|
||||
if (!path || typeof path !== 'string') return false;
|
||||
|
||||
// Must start with exactly one forward slash (relative path)
|
||||
if (!path.startsWith('/') || path.startsWith('//')) return false;
|
||||
|
||||
// Must not contain protocol indicators
|
||||
if (path.includes('://')) return false;
|
||||
|
||||
// Must not contain backslashes (can be normalized to forward slashes)
|
||||
if (path.includes('\\')) return false;
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* @name getSafeRedirectPath
|
||||
* @description Returns the path if safe, otherwise returns the fallback.
|
||||
* Use this to validate user-supplied redirect URLs to prevent open redirect attacks.
|
||||
*/
|
||||
export function getSafeRedirectPath(
|
||||
path: string | null | undefined,
|
||||
fallback: string,
|
||||
): string {
|
||||
if (path && isSafeRedirectPath(path)) {
|
||||
return path;
|
||||
}
|
||||
|
||||
return fallback;
|
||||
}
|
||||
@@ -21,6 +21,9 @@
|
||||
"./auth": "./src/auth.ts",
|
||||
"./types": "./src/types.ts"
|
||||
},
|
||||
"dependencies": {
|
||||
"@kit/shared": "workspace:*"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@kit/eslint-config": "workspace:*",
|
||||
"@kit/prettier-config": "workspace:*",
|
||||
|
||||
@@ -6,6 +6,8 @@ import {
|
||||
SupabaseClient,
|
||||
} from '@supabase/supabase-js';
|
||||
|
||||
import { isSafeRedirectPath } from '@kit/shared/utils';
|
||||
|
||||
/**
|
||||
* @name createAuthCallbackService
|
||||
* @description Creates an instance of the AuthCallbackService
|
||||
@@ -71,9 +73,11 @@ class AuthCallbackService {
|
||||
|
||||
const errorPath = params.errorPath ?? '/auth/callback/error';
|
||||
|
||||
// remove the query params from the url
|
||||
// remove the auth-related query params from the url
|
||||
searchParams.delete('token_hash');
|
||||
searchParams.delete('type');
|
||||
searchParams.delete('next');
|
||||
searchParams.delete('callback');
|
||||
|
||||
// if we have a next path, we redirect to that path
|
||||
if (nextPath) {
|
||||
@@ -132,7 +136,11 @@ class AuthCallbackService {
|
||||
const nextUrlPathFromParams = searchParams.get('next');
|
||||
const errorPath = params.errorPath ?? '/auth/callback/error';
|
||||
|
||||
const nextUrl = nextUrlPathFromParams ?? params.redirectPath;
|
||||
// Validate the next URL to prevent open redirect attacks
|
||||
const nextUrl =
|
||||
nextUrlPathFromParams && isSafeRedirectPath(nextUrlPathFromParams)
|
||||
? nextUrlPathFromParams
|
||||
: params.redirectPath;
|
||||
|
||||
if (authCode) {
|
||||
try {
|
||||
@@ -188,6 +196,7 @@ class AuthCallbackService {
|
||||
/**
|
||||
* Parses a redirect URL and extracts the destination path and query params
|
||||
* Handles nested 'next' parameters for chained redirects
|
||||
* Validates paths to prevent open redirect attacks
|
||||
*/
|
||||
private parseRedirectDestination(redirectParam: string | null): {
|
||||
path: string;
|
||||
@@ -197,29 +206,45 @@ class AuthCallbackService {
|
||||
return null;
|
||||
}
|
||||
|
||||
// First, try as a simple relative path with optional query string
|
||||
const [pathPart, queryPart] = redirectParam.split('?') as [
|
||||
string,
|
||||
string | undefined,
|
||||
];
|
||||
|
||||
if (isSafeRedirectPath(pathPart)) {
|
||||
return {
|
||||
path: pathPart,
|
||||
params: new URLSearchParams(queryPart ?? ''),
|
||||
};
|
||||
}
|
||||
|
||||
// Handle full URLs (e.g., from Supabase callback parameter)
|
||||
try {
|
||||
const redirectUrl = new URL(redirectParam);
|
||||
const url = new URL(redirectParam);
|
||||
|
||||
// check for nested 'next' parameter (chained redirect)
|
||||
const nestedNext = redirectUrl.searchParams.get('next');
|
||||
// Check for nested 'next' parameter - this is the final destination
|
||||
const nestedNext = url.searchParams.get('next');
|
||||
|
||||
if (nestedNext) {
|
||||
// use the nested path as the final destination
|
||||
if (nestedNext && isSafeRedirectPath(nestedNext)) {
|
||||
return {
|
||||
path: nestedNext,
|
||||
params: redirectUrl.searchParams,
|
||||
params: url.searchParams,
|
||||
};
|
||||
}
|
||||
|
||||
// no nested redirect, use the pathname directly
|
||||
return {
|
||||
path: redirectUrl.pathname,
|
||||
params: redirectUrl.searchParams,
|
||||
};
|
||||
// No nested next, use pathname if safe
|
||||
if (isSafeRedirectPath(url.pathname)) {
|
||||
return {
|
||||
path: url.pathname,
|
||||
params: url.searchParams,
|
||||
};
|
||||
}
|
||||
} catch {
|
||||
// invalid URL, ignore
|
||||
return null;
|
||||
// Invalid URL, ignore
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
private isLocalhost(host: string | null) {
|
||||
|
||||
@@ -2,6 +2,8 @@ import type { Provider } from '@supabase/supabase-js';
|
||||
|
||||
import { useMutation } from '@tanstack/react-query';
|
||||
|
||||
import { getSafeRedirectPath } from '@kit/shared/utils';
|
||||
|
||||
import { useSupabase } from './use-supabase';
|
||||
|
||||
export function useLinkIdentityWithProvider(
|
||||
@@ -14,7 +16,12 @@ export function useLinkIdentityWithProvider(
|
||||
|
||||
const mutationFn = async (provider: Provider) => {
|
||||
const origin = window.location.origin;
|
||||
const redirectToPath = props.redirectToPath ?? '/home/settings';
|
||||
|
||||
// Validate the redirect path to prevent open redirect attacks
|
||||
const redirectToPath = getSafeRedirectPath(
|
||||
props.redirectToPath,
|
||||
'/home/settings',
|
||||
);
|
||||
|
||||
const url = new URL('/auth/callback', origin);
|
||||
url.searchParams.set('redirectTo', redirectToPath);
|
||||
|
||||
10
pnpm-lock.yaml
generated
10
pnpm-lock.yaml
generated
@@ -1409,6 +1409,10 @@ importers:
|
||||
version: 19.2.7
|
||||
|
||||
packages/supabase:
|
||||
dependencies:
|
||||
'@kit/shared':
|
||||
specifier: workspace:*
|
||||
version: link:../shared
|
||||
devDependencies:
|
||||
'@kit/eslint-config':
|
||||
specifier: workspace:*
|
||||
@@ -14427,7 +14431,7 @@ snapshots:
|
||||
eslint: 9.39.1(jiti@2.6.1)
|
||||
eslint-import-resolver-node: 0.3.9
|
||||
eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1))
|
||||
eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.1(jiti@2.6.1))
|
||||
eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1))
|
||||
eslint-plugin-jsx-a11y: 6.10.2(eslint@9.39.1(jiti@2.6.1))
|
||||
eslint-plugin-react: 7.37.5(eslint@9.39.1(jiti@2.6.1))
|
||||
eslint-plugin-react-hooks: 7.0.1(eslint@9.39.1(jiti@2.6.1))
|
||||
@@ -14466,7 +14470,7 @@ snapshots:
|
||||
tinyglobby: 0.2.15
|
||||
unrs-resolver: 1.11.1
|
||||
optionalDependencies:
|
||||
eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.1(jiti@2.6.1))
|
||||
eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1))
|
||||
transitivePeerDependencies:
|
||||
- supports-color
|
||||
|
||||
@@ -14481,7 +14485,7 @@ snapshots:
|
||||
transitivePeerDependencies:
|
||||
- supports-color
|
||||
|
||||
eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1)(eslint@9.39.1(jiti@2.6.1)):
|
||||
eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.48.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.9.3))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)))(eslint@9.39.1(jiti@2.6.1)):
|
||||
dependencies:
|
||||
'@rtsao/scc': 1.1.0
|
||||
array-includes: 3.1.9
|
||||
|
||||
Reference in New Issue
Block a user