Skip to content

Commit 98709a9

Browse files
fix: [UIE-10136] - Fix Open Re-direction vulnerability in Account Cancel flow. (#13400)
* fix: [UIE-10136] - Fix open redirection vuln in Account Closure flow. * Added changeset: Fix Open Re-direction vulnerability in Account Cancel flow * Address review comments from Banks.
1 parent 5fd0f8a commit 98709a9

File tree

10 files changed

+82
-16
lines changed

10 files changed

+82
-16
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@linode/manager": Fixed
3+
---
4+
5+
Fix Open Re-direction vulnerability in Account Cancel flow ([#13400](https://github.com/linode/manager/pull/13400))

packages/manager/src/features/Account/CloseAccountDialog.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,10 @@ const CloseAccountDialog = ({ closeDialog, open }: Props) => {
5959
})
6060
.then((response) => {
6161
setIsClosingAccount(false);
62-
/** shoot the user off to survey monkey to answer some questions */
62+
/** Store survey link as state param for security so that it is not exposed in URL */
6363
navigate({
6464
to: '/cancel',
65-
search: { survey_link: response.survey_link },
65+
state: (prev) => ({ ...prev, surveyLink: response.survey_link }),
6666
});
6767
})
6868
.catch((e: APIError[]) => {

packages/manager/src/features/CancelLanding/CancelLanding.test.tsx

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,27 @@ import { CancelLanding } from './CancelLanding';
77

88
const realLocation = window.location;
99

10-
beforeAll(() => {
11-
window.location = realLocation;
10+
const queryMocks = vi.hoisted(() => ({
11+
useLocation: vi.fn(),
12+
}));
13+
14+
vi.mock('@tanstack/react-router', async () => {
15+
const actual = await vi.importActual('@tanstack/react-router');
16+
return {
17+
...actual,
18+
useLocation: queryMocks.useLocation,
19+
};
1220
});
1321

1422
afterEach(() => {
15-
window.location = realLocation;
23+
(window as Partial<Window>).location = realLocation;
1624
});
1725

1826
describe('CancelLanding', () => {
1927
it('does not render the body when there is no survey_link in the state', () => {
28+
queryMocks.useLocation.mockReturnValue({
29+
state: {},
30+
});
2031
const { queryByTestId } = renderWithTheme(<CancelLanding />, {
2132
initialEntries: ['/cancel'],
2233
initialRoute: '/cancel',
@@ -25,8 +36,11 @@ describe('CancelLanding', () => {
2536
});
2637

2738
it('renders the body when there is a survey_link in the state', () => {
39+
queryMocks.useLocation.mockReturnValue({
40+
state: { surveyLink: 'https://linode.com' },
41+
});
2842
const { queryByTestId } = renderWithTheme(<CancelLanding />, {
29-
initialEntries: ['/cancel?survey_link=https://linode.com'],
43+
initialEntries: ['/cancel'],
3044
initialRoute: '/cancel',
3145
});
3246
expect(queryByTestId('body')).toBeInTheDocument();
@@ -38,11 +52,17 @@ describe('CancelLanding', () => {
3852
const mockAssign = vi.fn();
3953
delete (window as Partial<Window>).location;
4054

41-
window.location = { ...realLocation, assign: mockAssign };
55+
(window as Partial<Window>).location = {
56+
...realLocation,
57+
assign: mockAssign,
58+
};
4259

4360
const surveyLink = 'https://linode.com';
61+
queryMocks.useLocation.mockReturnValue({
62+
state: { surveyLink },
63+
});
4464
const { getByTestId } = renderWithTheme(<CancelLanding />, {
45-
initialEntries: ['/cancel?survey_link=' + encodeURIComponent(surveyLink)],
65+
initialEntries: ['/cancel'],
4666
initialRoute: '/cancel',
4767
});
4868
const button = getByTestId('survey-button');

packages/manager/src/features/CancelLanding/CancelLanding.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Button, H1Header, Typography } from '@linode/ui';
22
import { useTheme } from '@mui/material/styles';
3-
import { redirect, useSearch } from '@tanstack/react-router';
3+
import { redirect, useLocation } from '@tanstack/react-router';
44
import * as React from 'react';
55
import { makeStyles } from 'tss-react/mui';
66

@@ -37,10 +37,11 @@ const useStyles = makeStyles()((theme: Theme) => ({
3737

3838
export const CancelLanding = React.memo(() => {
3939
const { classes } = useStyles();
40-
const search = useSearch({ from: '/cancel' });
40+
const location = useLocation();
41+
const locationState = location.state;
4142
const theme = useTheme();
4243

43-
const surveyLink = search.survey_link;
44+
const surveyLink = locationState.surveyLink;
4445

4546
if (!surveyLink) {
4647
throw redirect({ to: '/' });

packages/manager/src/mocks/presets/baseline/crud.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
} from 'src/mocks/presets/crud/handlers/events';
66
import { linodeCrudPreset } from 'src/mocks/presets/crud/linodes';
77

8+
import { accountCrudPreset } from '../crud/account';
89
import { cloudNATCrudPreset } from '../crud/cloudnats';
910
import { childAccountsCrudPreset } from '../crud/delegation';
1011
import { domainCrudPreset } from '../crud/domains';
@@ -27,6 +28,7 @@ import type { MockPresetBaseline } from 'src/mocks/types';
2728
export const baselineCrudPreset: MockPresetBaseline = {
2829
group: { id: 'General' },
2930
handlers: [
31+
...accountCrudPreset.handlers,
3032
...childAccountsCrudPreset.handlers,
3133
...cloudNATCrudPreset.handlers,
3234
...domainCrudPreset.handlers,
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { cancelAccount } from 'src/mocks/presets/crud/handlers/account';
2+
3+
import type { MockPresetCrud } from 'src/mocks/types';
4+
5+
export const accountCrudPreset: MockPresetCrud = {
6+
group: { id: 'Account' },
7+
handlers: [cancelAccount],
8+
id: 'account:crud',
9+
label: 'Account CRUD',
10+
};
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { http } from 'msw';
2+
3+
import { makeResponse } from 'src/mocks/utilities/response';
4+
5+
import type { CancelAccount } from '@linode/api-v4';
6+
import type { StrictResponse } from 'msw';
7+
import type { MockState } from 'src/mocks/types';
8+
9+
/**
10+
* MSW Handlers for Account operations
11+
*
12+
* This module provides mock handlers for the Account API endpoints.
13+
*/
14+
15+
export const cancelAccount = (mockState: MockState) => [
16+
http.post(
17+
'*/account/cancel',
18+
async ({ request }): Promise<StrictResponse<CancelAccount>> => {
19+
// The payload contains { comments: string } but we don't need to do anything with it for mocking
20+
const response: CancelAccount = {
21+
survey_link:
22+
'https://akamaisurveys.eu.qualtrics.com/jfe/form/abcd?SURVEY_KEY=12345',
23+
};
24+
25+
return makeResponse(response);
26+
}
27+
),
28+
];

packages/manager/src/mocks/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ export interface MockPresetExtra extends MockPresetBase {
132132
*/
133133
export type MockPresetCrudGroup = {
134134
id:
135+
| 'Account'
135136
| 'Child Accounts'
136137
| 'CloudNATs'
137138
| 'Delivery'
@@ -153,6 +154,7 @@ export type MockPresetCrudGroup = {
153154
| 'VPCs';
154155
};
155156
export type MockPresetCrudId =
157+
| 'account:crud'
156158
| 'child-accounts-for-user:crud'
157159
| 'child-accounts:crud'
158160
| 'cloudnats:crud'

packages/manager/src/routes/auth/index.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,10 @@ interface OAuthCallbackSearch {
1313
state?: string;
1414
}
1515

16-
interface CancelLandingSearch {
17-
survey_link?: string;
18-
}
19-
2016
const cancelLandingRoute = createRoute({
2117
getParentRoute: () => rootRoute,
2218
path: 'cancel',
2319
component: CancelLanding,
24-
validateSearch: (search: CancelLandingSearch) => search,
2520
});
2621

2722
const logoutRoute = createRoute({

packages/manager/src/routes/index.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,7 @@ declare module '@tanstack/react-router' {
122122
// This infers the type of our router and registers it across the entire project
123123
router: typeof router;
124124
}
125+
interface HistoryState {
126+
surveyLink?: string;
127+
}
125128
}

0 commit comments

Comments
 (0)