Skip to content

Commit

Permalink
feat: allow deep links in redirectTo validation (#513)
Browse files Browse the repository at this point in the history
  • Loading branch information
@onehassan
onehassan authored May 20, 2024
1 parent dc5d374 commit 6d2fcaf
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 71 deletions.
102 changes: 50 additions & 52 deletions src/validation/fields.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,63 +87,57 @@ export const redirectTo = Joi.string()
}
}

// * We allow any sub-path of the client url
// * With optional hash and query params
if (
new RegExp(`^${ENV.AUTH_CLIENT_URL}(/.*)?([?].*)?([#].*)?$`).test(value)
) {
return value;
}
const regexpContainsPort = new RegExp(`https?://[^/]+(:\\d+)(.*)`);
const regexpAddPort = new RegExp(`(https?://[^/]+)(.*)`);

const matches: string[] = [];

for (let url of [
...ENV.AUTH_ACCESS_CONTROL_ALLOWED_REDIRECT_URLS,
ENV.AUTH_CLIENT_URL,
]) {
switch (true) {
case url.endsWith('/**'):
break;
case url.endsWith('/*'):
url += '*';
break;
case url.endsWith('/'):
url += '**';
break;
default:
url += '/**';
}

// * Check if the value's hostname matches any allowed hostname
// * Required to avoid shadowing domains
const hostnames = ENV.AUTH_ACCESS_CONTROL_ALLOWED_REDIRECT_URLS.map(
(allowed) => {
return new URL(allowed).hostname;
let defaultPort = '80';
if (url.startsWith('https://')) {
defaultPort = '443';
}
);

const valueUrl = new URL(value);
if (!micromatch.isMatch(valueUrl.hostname, hostnames, { nocase: true })) {
return helper.error('redirectTo');
// add default port
if (!regexpContainsPort.test(url)) {
matches.push(url.replace(regexpAddPort, `$1:${defaultPort}$2`));
}

matches.push(url);
}

if (matches.length === 0) {
return value;
}

// * We allow any sub-path of the allowed redirect urls.
// * Allowed redirect urls also accepts wildcards and other micromatch patterns
const expressions = ENV.AUTH_ACCESS_CONTROL_ALLOWED_REDIRECT_URLS.map(
(allowed) => {
// * Replace all the `.` by `/` so micromatch will understand `.` as a path separator
allowed = allowed.replace(/[.]/g, '/');
// * Append `/**` to the end of the allowed URL to allow for any subpath
if (allowed.endsWith('/**')) {
return allowed;
}
if (allowed.endsWith('/*')) {
return `${allowed}*`;
}
if (allowed.endsWith('/')) {
return `${allowed}**`;
}
return `${allowed}/**`;
const redirectToClean = value.split('#')[0].split('?')[0];

for (const match of matches) {
if (
micromatch.isMatch(redirectToClean, match) ||
micromatch.isMatch(redirectToClean + '/', match)
) {
return value;
}
);

try {
// * Don't take the query parameters into account
// * And replace `.` with `/` because of micromatch
const urlWithoutParams = `${valueUrl.origin}${valueUrl.pathname}`.replace(
/[.]/g,
'/'
);
const match = micromatch.isMatch(urlWithoutParams, expressions, {
nocase: true,
});
if (match) return value;
return helper.error('redirectTo');
} catch {
// * value is not a valid URL
return helper.error('redirectTo');
}

return helper.error('redirectTo');
})
.example(`${ENV.AUTH_CLIENT_URL}/catch-redirection`);

Expand Down Expand Up @@ -186,15 +180,19 @@ export const registrationOptions =
.custom((value, helper) => {
const { allowedRoles, defaultRole } = value;
if (!allowedRoles.includes(defaultRole)) {
return helper.message({custom:'Default role must be part of allowed roles'});
return helper.message({
custom: 'Default role must be part of allowed roles',
});
}
// check if allowedRoles is a subset of allowed user roles
if (
!allowedRoles.every((role: string) =>
ENV.AUTH_USER_DEFAULT_ALLOWED_ROLES.includes(role)
)
) {
return helper.message({custom:'Allowed roles must be a subset of allowedRoles'});
return helper.message({
custom: 'Allowed roles must be a subset of allowedRoles',
});
}
return value;
});
Expand Down
71 changes: 52 additions & 19 deletions test/validation.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,24 @@ describe('Unit tests on field validation', () => {

describe('redirections', () => {
const clientUrl = 'https://nhost.io';
const diffDomainUrl = 'https://myotherdomain.com'
const host = 'host.com'
const allowedRedirectUrls = `https://*-nhost.vercel.app,${diffDomainUrl},https://*.${host},https://no-wildcard.io`;
const diffDomainUrl = 'https://myotherdomain.com';
const host = 'host.com';
const allowedRedirectUrls = `https://*-nhost.vercel.app,${diffDomainUrl},https://*.${host},https://no-wildcard.io,myapp://`;

beforeAll(async () => {
process.env.AUTH_CLIENT_URL = clientUrl;
process.env.AUTH_ACCESS_CONTROL_ALLOWED_REDIRECT_URLS = allowedRedirectUrls;
process.env.AUTH_ACCESS_CONTROL_ALLOWED_REDIRECT_URLS =
allowedRedirectUrls;
});

it('should validate any url when the client url is not set', async () => {
process.env.AUTH_CLIENT_URL = ''
process.env.AUTH_CLIENT_URL = '';

expect(
redirectTo.validate('https://www.google.com/path?key=value').value
).toEqual('https://www.google.com/path?key=value');

process.env.AUTH_CLIENT_URL = clientUrl
process.env.AUTH_CLIENT_URL = clientUrl;
});

it('should reject an invalid url', () => {
Expand All @@ -83,24 +84,58 @@ describe('Unit tests on field validation', () => {
it('should validate a value that matches the client url', () => {
expect(redirectTo.validate(clientUrl).error).toBeUndefined();
expect(redirectTo.validate(`${clientUrl}/path`).error).toBeUndefined();
expect(redirectTo.validate(`${clientUrl}?key=value`).error).toBeUndefined();
expect(redirectTo.validate(`${clientUrl}#key=value`).error).toBeUndefined();
expect(
redirectTo.validate(`${clientUrl}?key=value`).error
).toBeUndefined();
expect(
redirectTo.validate(`${clientUrl}#key=value`).error
).toBeUndefined();
});

it('should validate a value that matches an allowed redirect url', () => {
expect(redirectTo.validate(diffDomainUrl).error).toBeUndefined();
expect(redirectTo.validate(`${diffDomainUrl}/path`).error).toBeUndefined();
expect(redirectTo.validate(`${diffDomainUrl}?key=value`).error).toBeUndefined();
expect(redirectTo.validate(`${diffDomainUrl}#key=value`).error).toBeUndefined();
expect(
redirectTo.validate(`${diffDomainUrl}/path`).error
).toBeUndefined();
expect(
redirectTo.validate(`${diffDomainUrl}?key=value`).error
).toBeUndefined();
expect(
redirectTo.validate(`${diffDomainUrl}#key=value`).error
).toBeUndefined();
});

it('should validate a subdomain that matches an allowed redirect url with a wildcard', () => {
expect(redirectTo.validate(`https://subdomain.${host}`).error).toBeUndefined()
expect(redirectTo.validate(`https://subdomain.${host}/path`).error).toBeUndefined()
expect(redirectTo.validate(`https://subdomain.${host}?key=value`).error).toBeUndefined()
expect(redirectTo.validate(`https://subdomain.${host}#key=value`).error).toBeUndefined()
expect(
redirectTo.validate(`https://subdomain.${host}`).error
).toBeUndefined();
expect(
redirectTo.validate(`https://subdomain.${host}/path`).error
).toBeUndefined();
expect(
redirectTo.validate(`https://subdomain.${host}?key=value`).error
).toBeUndefined();
expect(
redirectTo.validate(`https://subdomain.${host}#key=value`).error
).toBeUndefined();

expect(redirectTo.validate(`https://docs-ger4gr-nhost.vercel.app`).error).toBeUndefined()
expect(
redirectTo.validate(`https://docs-ger4gr-nhost.vercel.app`).error
).toBeUndefined();
});

it('should validate a deeplink that matches an allowed redirect url', () => {
expect(redirectTo.validate(`myapp://`).error).toBeUndefined();
expect(redirectTo.validate(`myapp://redirect`).error).toBeUndefined();
expect(redirectTo.validate(`myapp://home/profile`).error).toBeUndefined();
});

it('should reject a deeplink that doesn not match an allowed redirect url', () => {
expect(redirectTo.validate(`myotherapp://`).error).toBeObject();
expect(redirectTo.validate(`myotherapp://redirect`).error).toBeObject();
expect(
redirectTo.validate(`myotherapp://home/profile`).error
).toBeObject();
});

it('should reject a subsubdomain if no wildcard', () => {
Expand All @@ -122,9 +157,7 @@ describe('Unit tests on field validation', () => {
redirectTo.validate(`${diffDomainUrl}.example.com`).error
).toBeObject();

expect(
redirectTo.validate(`https://wwwanhost.com`).error
).toBeObject();
expect(redirectTo.validate(`https://wwwanhost.com`).error).toBeObject();
});
});
});

0 comments on commit 6d2fcaf

Please sign in to comment.