Skip to content

Commit 854dde8

Browse files
authored
feat(clerk-js): Link to external App page in OAuth Consent (#6447)
In OAuthConsent wire the ApplicationLogo.href up to OAuth Application URL so that users can click the Logo of the OAuth App to navigate to the App's homepage. Set isExternal to open the link in a new window and apply attribute rel="noopener noreferrer" to to avoid (in legacy browsers) passing an opener object to the target page, and to avoid sharing sensitive query string parameters. Harden <Link> to not set href to dangerous URLs (e.g. from javascript: or data: protocols). Test in sandbox http://localhost:4000/oauth-consent?app-url=https%3A%2F%2Fclerk.com&logo-url=http%3A%2F%2Flocalhost%3A3000%2Flogo Part of USER-2011
1 parent 21c2956 commit 854dde8

File tree

9 files changed

+207
-13
lines changed

9 files changed

+207
-13
lines changed

.changeset/sad-turkeys-rhyme.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
'@clerk/types': patch
4+
---
5+
6+
Add optional `isExternal` to `ApplicationLogo`
7+
8+
Add optional `oAuthApplicationUrl` parameter to OAuth Consent mounting (which is used to provide a link to the OAuth App homepage).
9+
10+
Harden `Link` component so it sanitizes the given `href` to avoid dangerous protocols.

.vscode/tasks.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"version": "2.0.0",
3+
"tasks": [
4+
{
5+
"type": "npm",
6+
"script": "dev:sandbox",
7+
"path": "packages/clerk-js",
8+
"problemMatcher": [],
9+
"label": "Dev: Sandbox",
10+
"detail": "npm: dev:sandbox - packages/clerk-js"
11+
}
12+
]
13+
}

packages/clerk-js/sandbox/app.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,8 @@ void (async () => {
334334
scopes,
335335
oAuthApplicationName: searchParams.get('oauth-application-name'),
336336
redirectUrl: searchParams.get('redirect_uri'),
337+
oAuthApplicationLogoUrl: searchParams.get('logo-url'),
338+
oAuthApplicationUrl: searchParams.get('app-url'),
337339
},
338340
);
339341
},

packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { common } from '@/ui/styledSystem';
1717
import { colors } from '@/ui/utils/colors';
1818

1919
export function OAuthConsentInternal() {
20-
const { scopes, oAuthApplicationName, oAuthApplicationLogoUrl, redirectUrl, onDeny, onAllow } =
20+
const { scopes, oAuthApplicationName, oAuthApplicationLogoUrl, oAuthApplicationUrl, redirectUrl, onDeny, onAllow } =
2121
useOAuthConsentContext();
2222
const { user } = useUser();
2323
const { applicationName, logoImageUrl } = useEnvironment().displayConfig;
@@ -46,6 +46,8 @@ export function OAuthConsentInternal() {
4646
<ApplicationLogo
4747
src={oAuthApplicationLogoUrl}
4848
alt={oAuthApplicationName}
49+
href={oAuthApplicationUrl}
50+
isExternal
4951
/>
5052
</ConnectionItem>
5153
<ConnectionSeparator />
@@ -65,6 +67,8 @@ export function OAuthConsentInternal() {
6567
<ApplicationLogo
6668
src={oAuthApplicationLogoUrl}
6769
alt={oAuthApplicationName}
70+
href={oAuthApplicationUrl}
71+
isExternal
6872
/>
6973
<ConnectionIcon
7074
size='sm'

packages/clerk-js/src/ui/elements/ApplicationLogo.tsx

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import React from 'react';
22

33
import { useEnvironment } from '../contexts';
44
import { descriptors, Flex, Image, useAppearance } from '../customizables';
5+
import { Link } from '../primitives';
56
import type { PropsOfComponent } from '../styledSystem';
67
import { RouterLink } from './RouterLink';
78

@@ -34,10 +35,16 @@ export type ApplicationLogoProps = PropsOfComponent<typeof Flex> & {
3435
* The URL to navigate to when the logo is clicked.
3536
*/
3637
href?: string;
38+
/**
39+
* Whether the href should be treated as an external link.
40+
* When true, uses a Link component with target="_blank" and proper security attributes.
41+
* When false or undefined, uses RouterLink for internal navigation.
42+
*/
43+
isExternal?: boolean;
3744
};
3845

3946
export const ApplicationLogo: React.FC<ApplicationLogoProps> = (props: ApplicationLogoProps): JSX.Element | null => {
40-
const { src, alt, href, sx, ...rest } = props;
47+
const { src, alt, href, isExternal, sx, ...rest } = props;
4148
const imageRef = React.useRef<HTMLImageElement>(null);
4249
const [loaded, setLoaded] = React.useState(false);
4350
const { logoImageUrl, applicationName, homeUrl } = useEnvironment().displayConfig;
@@ -80,12 +87,22 @@ export const ApplicationLogo: React.FC<ApplicationLogoProps> = (props: Applicati
8087
]}
8188
>
8289
{logoUrl ? (
83-
<RouterLink
84-
focusRing
85-
to={logoUrl}
86-
>
87-
{image}
88-
</RouterLink>
90+
isExternal ? (
91+
<Link
92+
focusRing
93+
href={logoUrl}
94+
isExternal
95+
>
96+
{image}
97+
</Link>
98+
) : (
99+
<RouterLink
100+
focusRing
101+
to={logoUrl}
102+
>
103+
{image}
104+
</RouterLink>
105+
)
89106
) : (
90107
image
91108
)}

packages/clerk-js/src/ui/primitives/Link.tsx

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React from 'react';
22

3+
import { sanitizeHref } from '../../utils/url';
34
import type { PrimitiveProps, StyleVariants } from '../styledSystem';
45
import { common, createVariants } from '../styledSystem';
56
import { applyDataStateProps } from './applyDataStateProps';
@@ -57,9 +58,12 @@ export type LinkProps = PrimitiveProps<'a'> & OwnProps & StyleVariants<typeof ap
5758
export const Link = (props: LinkProps): JSX.Element => {
5859
const { isExternal, children, href, onClick, ...rest } = props;
5960

61+
// Sanitize href to prevent dangerous protocols
62+
const sanitizedHref = sanitizeHref(href);
63+
6064
const onClickHandler = onClick
6165
? (e: React.MouseEvent<HTMLAnchorElement, MouseEvent>) => {
62-
if (!href) {
66+
if (!sanitizedHref) {
6367
e.preventDefault();
6468
}
6569
onClick(e);
@@ -70,9 +74,9 @@ export const Link = (props: LinkProps): JSX.Element => {
7074
<a
7175
{...applyDataStateProps(filterProps(rest))}
7276
onClick={onClickHandler}
73-
href={href || ''}
74-
target={href && isExternal ? '_blank' : undefined}
75-
rel={href && isExternal ? 'noopener' : undefined}
77+
href={sanitizedHref || ''}
78+
target={sanitizedHref && isExternal ? '_blank' : undefined}
79+
rel={sanitizedHref && isExternal ? 'noopener noreferrer' : undefined}
7680
css={applyVariants(props) as any}
7781
>
7882
{children}

packages/clerk-js/src/utils/__tests__/url.spec.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
createAllowedRedirectOrigins,
88
getETLDPlusOneFromFrontendApi,
99
getSearchParameterFromHash,
10+
hasBannedHrefProtocol,
1011
hasBannedProtocol,
1112
hasExternalAccountSignUpError,
1213
isAllowedRedirect,
@@ -18,6 +19,7 @@ import {
1819
mergeFragmentIntoUrl,
1920
relativeToAbsoluteUrl,
2021
requiresUserInput,
22+
sanitizeHref,
2123
trimLeadingSlash,
2224
trimTrailingSlash,
2325
} from '../url';
@@ -36,7 +38,7 @@ describe('isDevAccountPortalOrigin(url)', () => {
3638
];
3739

3840
test.each(goodUrls)('.isDevAccountPortalOrigin(%s)', (a, expected) => {
39-
// @ts-ignore
41+
// @ts-ignore - Type assertion for test parameter
4042
expect(isDevAccountPortalOrigin(a)).toBe(expected);
4143
});
4244
});
@@ -147,6 +149,79 @@ describe('hasBannedProtocol(url)', () => {
147149
});
148150
});
149151

152+
describe('hasBannedHrefProtocol(url)', () => {
153+
const cases: Array<[string, boolean]> = [
154+
['https://www.clerk.com/', false],
155+
['http://www.clerk.com/', false],
156+
['/sign-in', false],
157+
['/sign-in?test=1', false],
158+
['/?test', false],
159+
['javascript:console.log(document.cookies)', true],
160+
['data:image/png;base64,iVBORw0KGgoAAA5ErkJggg==', true],
161+
['vbscript:alert("xss")', true],
162+
['blob:https://example.com/12345678-1234-1234-1234-123456789012', true],
163+
['ftp://files.example.com/file.txt', false],
164+
['mailto:[email protected]', false],
165+
];
166+
167+
test.each(cases)('.hasBannedHrefProtocol(%s)', (a, expected) => {
168+
expect(hasBannedHrefProtocol(a)).toBe(expected);
169+
});
170+
});
171+
172+
describe('sanitizeHref(href)', () => {
173+
const cases: Array<[string | undefined | null, string | null]> = [
174+
// Null/undefined/empty cases
175+
[null, null],
176+
[undefined, null],
177+
['', null],
178+
[' ', null],
179+
180+
// Safe relative URLs
181+
['/path/to/page', '/path/to/page'],
182+
['#anchor', '#anchor'],
183+
['?query=param', '?query=param'],
184+
['../relative/path', '../relative/path'],
185+
['relative/path', 'relative/path'],
186+
['path/page#anchor', 'path/page#anchor'],
187+
188+
// Safe absolute URLs
189+
['https://www.clerk.com/', 'https://www.clerk.com/'],
190+
['http://localhost:3000/path', 'http://localhost:3000/path'],
191+
['ftp://files.example.com/file.txt', 'ftp://files.example.com/file.txt'],
192+
['mailto:[email protected]', 'mailto:[email protected]'],
193+
194+
// Dangerous protocols - should return null
195+
['javascript:alert("xss")', null],
196+
['javascript:console.log(document.cookies)', null],
197+
['data:text/html,<script>alert("xss")</script>', null],
198+
['data:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTIGZyb20gZGF0YSBVUkknKTwvc2NyaXB0Pg==', null],
199+
['data:image/png;base64,iVBORw0KGgoAAA5ErkJggg==', null],
200+
['vbscript:alert("xss")', null],
201+
['blob:https://example.com/12345678-1234-1234-1234-123456789012', null],
202+
203+
// Sneaky cases with dangerous protocols
204+
['JAVASCRIPT:alert("xss")', null], // All caps protocol
205+
['JavaScript:alert("xss")', null], // Mixed case
206+
[' javascript:alert("xss") ', null], // Whitespace
207+
['javascript: alert("xss") ', null], // Whitespace
208+
209+
// Malformed URLs that might be relative paths
210+
['not-a-url', 'not-a-url'],
211+
['path:with:colons', 'path:with:colons'],
212+
];
213+
214+
test.each(cases)('.sanitizeHref(%s)', (href, expected) => {
215+
expect(sanitizeHref(href)).toBe(expected);
216+
});
217+
218+
it('handles malformed URLs gracefully', () => {
219+
// These should not throw errors and should be allowed as potential relative URLs
220+
expect(sanitizeHref(':::invalid:::')).toBe(':::invalid:::');
221+
expect(sanitizeHref('malformed:url:here')).toBe('malformed:url:here');
222+
});
223+
});
224+
150225
describe('buildURL(options: URLParams, skipOrigin)', () => {
151226
it('builds a URL()', () => {
152227
expect(buildURL({}, { stringify: true })).toBe('http://localhost:3000/');

packages/clerk-js/src/utils/url.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ const DUMMY_URL_BASE = 'http://clerk-dummy';
2121

2222
const BANNED_URI_PROTOCOLS = ['javascript:'] as const;
2323

24+
// Protocols that are dangerous specifically for href attributes in links
25+
const BANNED_HREF_PROTOCOLS = ['javascript:', 'data:', 'vbscript:', 'blob:'] as const;
26+
2427
const { isDevOrStagingUrl } = createDevOrStagingUrlCache();
2528
export { isDevOrStagingUrl };
2629
const accountPortalCache = new Map<string, boolean>();
@@ -276,6 +279,16 @@ export function isDataUri(val?: string): val is string {
276279
return new URL(val).protocol === 'data:';
277280
}
278281

282+
/**
283+
* Checks if a URL uses javascript: protocol.
284+
* This prevents some XSS attacks through javascript: URLs.
285+
*
286+
* IMPORTANT: This does not check for `data:` or other protocols which
287+
* are dangerous if used for links or setting the window location.
288+
*
289+
* @param val - The URL to check
290+
* @returns True if the URL contains a banned protocol, false otherwise
291+
*/
279292
export function hasBannedProtocol(val: string | URL) {
280293
if (!isValidUrl(val)) {
281294
return false;
@@ -284,6 +297,58 @@ export function hasBannedProtocol(val: string | URL) {
284297
return BANNED_URI_PROTOCOLS.some(bp => bp === protocol);
285298
}
286299

300+
/**
301+
* Checks if a URL contains a banned protocol for href attributes in links.
302+
* This prevents some XSS attacks through javascript:, data:, vbscript:, and blob: URLs.
303+
*
304+
* @param val - The URL to check
305+
* @returns True if the URL contains a banned protocol, false otherwise
306+
*/
307+
export function hasBannedHrefProtocol(val: string | URL): boolean {
308+
if (!isValidUrl(val)) {
309+
return false;
310+
}
311+
const protocol = new URL(val).protocol;
312+
return BANNED_HREF_PROTOCOLS.some(bp => bp === protocol);
313+
}
314+
315+
/**
316+
* Sanitizes an href value by checking for dangerous protocols.
317+
* Returns null if the href contains a dangerous protocol, otherwise returns the original href.
318+
* This prevents some XSS attacks through javascript:, data:, vbscript:, and blob: URLs.
319+
*
320+
* @param href - The href value to sanitize
321+
* @returns The sanitized href or null if dangerous
322+
*/
323+
export function sanitizeHref(href: string | undefined | null): string | null {
324+
if (!href || href.trim() === '') {
325+
return null;
326+
}
327+
328+
// For relative URLs (starting with / or # or ?), allow them through
329+
if (href.startsWith('/') || href.startsWith('#') || href.startsWith('?')) {
330+
return href;
331+
}
332+
333+
// For relative URLs without leading slash, allow them through
334+
if (!href.includes(':')) {
335+
return href;
336+
}
337+
338+
// Check if it's a valid URL with a dangerous protocol
339+
try {
340+
const url = new URL(href);
341+
if (hasBannedHrefProtocol(url)) {
342+
return null;
343+
}
344+
return href;
345+
} catch {
346+
// If URL parsing fails, it's likely a relative URL or malformed
347+
// Allow relative URLs through, but be cautious with malformed ones
348+
return href;
349+
}
350+
}
351+
287352
export const hasUrlInFragment = (_url: URL | string) => {
288353
return new URL(_url, DUMMY_URL_BASE).hash.startsWith('#/');
289354
};

packages/types/src/clerk.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2002,6 +2002,10 @@ export type __internal_OAuthConsentProps = {
20022002
* Logo URL of the OAuth application.
20032003
*/
20042004
oAuthApplicationLogoUrl?: string;
2005+
/**
2006+
* URL of the OAuth application.
2007+
*/
2008+
oAuthApplicationUrl?: string;
20052009
/**
20062010
* Scopes requested by the OAuth application.
20072011
*/

0 commit comments

Comments
 (0)