Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sitecore-jss-nextjs][sitecore-jss] multi-origin CORS validation for next editing middlewares #1798

Merged
merged 6 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ Our versioning strategy is as follows:

* `[sitecore-jss-react]`Introduce ErrorBoundary component. All rendered components are wrapped with it and it will catch client or server side errors from any of its children, display appropriate message and prevent the rest of the application from failing. It accepts and can display custom error component and loading message if it is passed as a prop to parent Placeholder. ([#1786](https://github.com/Sitecore/jss/pull/1786) [#1790](https://github.com/Sitecore/jss/pull/1790) [#1793](https://github.com/Sitecore/jss/pull/1793) [#1794](https://github.com/Sitecore/jss/pull/1794))

* `[sitecore-jss-nextjs]` Enforce CORS policy that matches Sitecore Pages domains for editing middleware API endpoints ([#1798](https://github.com/Sitecore/jss/pull/1798))

## 22.0.0

### 🛠 Breaking Changes
Expand Down
1 change: 1 addition & 0 deletions packages/sitecore-jss-nextjs/src/editing/constants.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export const QUERY_PARAM_EDITING_SECRET = 'secret';
export const QUERY_PARAM_PROTECTION_BYPASS_SITECORE = 'x-sitecore-protection-bypass';
export const QUERY_PARAM_PROTECTION_BYPASS_VERCEL = 'x-vercel-protection-bypass';
export const EDITING_ALLOWED_ORIGINS = ['https://pages*.cloud/', 'https://pages.sitecorecloud.io/'];
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@ type Query = {
[key: string]: string;
};

const mockRequest = (method: string, query?: Query) => {
const allowedOrigin = 'https://allowed.com';

const mockRequest = (method: string, query?: Query, headers?: { [key: string]: string }) => {
return {
method,
query: query ?? {},
headers: {
origin: allowedOrigin,
...headers,
},
} as NextApiRequest;
};

Expand All @@ -24,6 +30,9 @@ const mockResponse = () => {
res.json = spy(() => {
return res;
});
res.setHeader = spy(() => {
return res;
});
return res;
};

Expand All @@ -44,10 +53,12 @@ describe('EditingConfigMiddleware', () => {

beforeEach(() => {
process.env.JSS_EDITING_SECRET = secret;
process.env.JSS_ALLOWED_ORIGINS = allowedOrigin;
});

after(() => {
delete process.env.JSS_EDITING_SECRET;
delete process.env.JSS_ALLOWED_ORIGINS;
});

it('should respond with 401 for missing secret', async () => {
Expand All @@ -66,6 +77,20 @@ describe('EditingConfigMiddleware', () => {
expect(res.json).to.have.been.calledWith(expectedResultForbidden);
});

it('should stop request and return 401 when CORS match is not met', async () => {
const req = mockRequest('GET', {}, { origin: 'https://notallowed.com' });
const res = mockResponse();
const middleware = new EditingConfigMiddleware({ components: componentsArray, metadata });
const handler = middleware.getHandler();

await handler(req, res);

expect(res.status).to.have.been.calledOnce;
expect(res.status).to.have.been.calledWith(401);
expect(res.json).to.have.been.calledOnce;
expect(res.json).to.have.been.calledWith({ message: 'Invalid origin' });
});

it('should respond with 401 for invalid secret', async () => {
const key = 'wrongkey';
const query = { key } as Query;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { NextApiRequest, NextApiResponse } from 'next';
import { QUERY_PARAM_EDITING_SECRET } from './constants';
import { EDITING_ALLOWED_ORIGINS, QUERY_PARAM_EDITING_SECRET } from './constants';
import { getJssEditingSecret } from '../utils/utils';
import { debug } from '@sitecore-jss/sitecore-jss';
import { Metadata } from '@sitecore-jss/sitecore-jss-dev-tools';
import { enforceCors } from '@sitecore-jss/sitecore-jss/utils';

export type EditingConfigMiddlewareConfig = {
/**
Expand Down Expand Up @@ -35,6 +36,12 @@ export class EditingConfigMiddleware {

private handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
const secret = _req.query[QUERY_PARAM_EDITING_SECRET];
if (!enforceCors(_req, res, EDITING_ALLOWED_ORIGINS)) {
debug.editing(
'invalid origin host - set allowed origins in JSS_ALLOWED_ORIGINS environment variable'
);
return res.status(401).json({ message: 'Invalid origin' });
}
if (secret !== getJssEditingSecret()) {
debug.editing(
'invalid editing secret - sent "%s" expected "%s"',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,22 @@ type Query = {
[key: string]: string;
};

const mockRequest = (method: string, query?: Query, body?: unknown) => {
const allowedOrigin = 'https://allowed.com';

const mockRequest = (
method: string,
query?: Query,
body?: unknown,
headers?: { [key: string]: string }
) => {
return {
method,
query: query ?? {},
body: body ?? {},
headers: {
origin: allowedOrigin,
...headers,
},
} as NextApiRequest;
};

Expand All @@ -34,7 +45,9 @@ const mockResponse = () => {
res.end = spy(() => {
return res;
});
res.setHeader = spy();
res.setHeader = spy(() => {
return res;
});
return res;
};

Expand All @@ -59,10 +72,12 @@ describe('EditingDataMiddleware', () => {

beforeEach(() => {
process.env.JSS_EDITING_SECRET = secret;
process.env.JSS_ALLOWED_ORIGINS = allowedOrigin;
});

after(() => {
delete process.env.JSS_EDITING_SECRET;
delete process.env.JSS_ALLOWED_ORIGINS;
});

it('should handle PUT request', async () => {
Expand Down Expand Up @@ -132,6 +147,21 @@ describe('EditingDataMiddleware', () => {
expect(res.json).to.have.been.calledWith(mockEditingData);
});

it('should stop request and return 401 when CORS match is not met', async () => {
const req = mockRequest('GET', {}, {}, { origin: 'https://notallowed.com' });
const res = mockResponse();
const cache = mockCache();
const middleware = new EditingDataMiddleware({ editingDataCache: cache });
const handler = middleware.getHandler();

await handler(req, res);

expect(res.status).to.have.been.calledOnce;
expect(res.status).to.have.been.calledWith(401);
expect(res.json).to.have.been.calledOnce;
expect(res.json).to.have.been.calledWith({ message: 'Invalid origin' });
});

it('should respond with 400 for invalid editing data', async () => {
const key = 'key1234';
const query = { key } as Query;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { NextApiRequest, NextApiResponse } from 'next';
import { EditingDataCache, editingDataDiskCache } from './editing-data-cache';
import { EditingData, isEditingData } from './editing-data';
import { QUERY_PARAM_EDITING_SECRET } from './constants';
import { EDITING_ALLOWED_ORIGINS, QUERY_PARAM_EDITING_SECRET } from './constants';
import { getJssEditingSecret } from '../utils/utils';
import { enforceCors } from '@sitecore-jss/sitecore-jss/utils';
import { debug } from '@sitecore-jss/sitecore-jss';

export interface EditingDataMiddlewareConfig {
/**
Expand Down Expand Up @@ -51,6 +53,12 @@ export class EditingDataMiddleware {
const secret = query[QUERY_PARAM_EDITING_SECRET];
const key = query[this.queryParamKey];

if (!enforceCors(req, res, EDITING_ALLOWED_ORIGINS)) {
debug.editing(
'invalid origin host - set allowed origins in JSS_ALLOWED_ORIGINS environment variable'
);
return res.status(401).json({ message: 'Invalid origin' });
}
// Validate secret
if (secret !== getJssEditingSecret()) {
res.status(401).end('Missing or invalid secret');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,23 @@ type Query = {
[key: string]: string;
};

const mockRequest = (body?: any, query?: Query, method?: string, host?: string) => {
const allowedOrigin = 'https://allowed.com';

const mockRequest = (
body?: any,
query?: Query,
method?: string,
headers?: { [key: string]: string }
) => {
return {
body: body ?? {},
method: method ?? 'POST',
query: query ?? {},
headers: { host: host ?? 'localhost:3000' },
headers: {
host: 'localhost:3000',
origin: allowedOrigin,
...headers,
},
} as NextApiRequest;
};

Expand Down Expand Up @@ -83,12 +94,14 @@ describe('EditingRenderMiddleware', () => {

beforeEach(() => {
process.env.JSS_EDITING_SECRET = secret;
process.env.JSS_ALLOWED_ORIGINS = allowedOrigin;
delete process.env.VERCEL;
});

after(() => {
delete process.env.JSS_EDITING_SECRET;
delete process.env.VERCEL;
delete process.env.JSS_ALLOWED_ORIGINS;
});

it('should handle request', async () => {
Expand Down Expand Up @@ -312,6 +325,27 @@ describe('EditingRenderMiddleware', () => {
expect(res.json).to.have.been.calledOnce;
});

it('should stop request and return 401 when CORS match is not met', async () => {
const req = mockRequest({}, {}, 'POST', { origin: 'https://notallowed.com' });
const res = mockResponse();
const fetcher = mockFetcher();
const dataService = mockDataService();
const middleware = new EditingRenderMiddleware({
dataFetcher: fetcher,
editingDataService: dataService,
});
const handler = middleware.getHandler();

await handler(req, res);

expect(res.status).to.have.been.calledOnce;
expect(res.status).to.have.been.calledWith(401);
expect(res.json).to.have.been.calledOnce;
expect(res.json).to.have.been.calledWith({
html: '<html><body>Requests from origin https://notallowed.com not allowed</body></html>',
});
});

it('should respond with 401 for missing secret', async () => {
const fetcher = mockFetcher();
const dataService = mockDataService();
Expand Down Expand Up @@ -381,7 +415,7 @@ describe('EditingRenderMiddleware', () => {
const dataService = mockDataService();
const query = {} as Query;
query[QUERY_PARAM_EDITING_SECRET] = secret;
const req = mockRequest(EE_BODY, query, undefined, 'testhostheader.com');
const req = mockRequest(EE_BODY, query, undefined, { host: 'testhostheader.com' });
const res = mockResponse();

const middleware = new EditingRenderMiddleware({
Expand All @@ -401,7 +435,7 @@ describe('EditingRenderMiddleware', () => {
const dataService = mockDataService();
const query = {} as Query;
query[QUERY_PARAM_EDITING_SECRET] = secret;
const req = mockRequest(EE_BODY, query, undefined, 'vercel.com');
const req = mockRequest(EE_BODY, query, undefined, { host: 'vercel.com' });
const res = mockResponse();
process.env.VERCEL = '1';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import { EDITING_COMPONENT_ID, RenderingType } from '@sitecore-jss/sitecore-jss/
import { parse } from 'node-html-parser';
import { EditingData } from './editing-data';
import { EditingDataService, editingDataService } from './editing-data-service';
import { QUERY_PARAM_EDITING_SECRET } from './constants';
import { EDITING_ALLOWED_ORIGINS, QUERY_PARAM_EDITING_SECRET } from './constants';
import { getJssEditingSecret } from '../utils/utils';
import { RenderMiddlewareBase } from './render-middleware';
import { enforceCors } from '@sitecore-jss/sitecore-jss/utils';

export interface EditingRenderMiddlewareConfig {
/**
Expand Down Expand Up @@ -87,6 +88,15 @@ export class EditingRenderMiddleware extends RenderMiddlewareBase {
body,
});

if (!enforceCors(req, res, EDITING_ALLOWED_ORIGINS)) {
debug.editing(
'invalid origin host - set allowed origins in JSS_ALLOWED_ORIGINS environment variable'
);
return res.status(401).json({
html: `<html><body>Requests from origin ${req.headers?.origin} not allowed</body></html>`,
});
}

if (method !== 'POST') {
debug.editing('invalid method - sent %s expected POST', method);
res.setHeader('Allow', 'POST');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,18 @@ type Query = {
[key: string]: string;
};

const mockRequest = (query?: Query, method?: string, host?: string) => {
const allowedOrigin = 'https://allowed.com';

const mockRequest = (query?: Query, method?: string, headers?: { [key: string]: string }) => {
return {
body: {},
method: method ?? 'GET',
query: query ?? {},
headers: { host: host ?? 'localhost:3000' },
headers: {
host: 'localhost:3000',
origin: allowedOrigin,
...headers,
},
} as NextApiRequest;
};

Expand Down Expand Up @@ -53,10 +59,12 @@ describe('FEAASRenderMiddleware', () => {

beforeEach(() => {
process.env.JSS_EDITING_SECRET = secret;
process.env.JSS_ALLOWED_ORIGINS = allowedOrigin;
});

after(() => {
delete process.env.JSS_EDITING_SECRET;
delete process.env.JSS_ALLOWED_ORIGINS;
});

it('should handle request', async () => {
Expand Down Expand Up @@ -138,6 +146,22 @@ describe('FEAASRenderMiddleware', () => {
);
});

it('should stop request and return 401 when CORS match is not met', async () => {
const req = mockRequest({}, 'POST', { origin: 'https://notallowed.com' });
const res = mockResponse();
const middleware = new FEAASRenderMiddleware();
const handler = middleware.getHandler();

await handler(req, res);

expect(res.status).to.have.been.calledOnce;
expect(res.status).to.have.been.calledWith(401);
expect(res.send).to.have.been.calledOnce;
expect(res.send).to.have.been.calledWith(
'<html><body>Requests from origin https://notallowed.com are not allowed</body></html>'
);
});

it('should respond with 401 for missing secret', async () => {
const query = {} as Query;
const req = mockRequest(query);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { NextApiRequest, NextApiResponse } from 'next';
import { debug } from '@sitecore-jss/sitecore-jss';
import { QUERY_PARAM_EDITING_SECRET } from './constants';
import { EDITING_ALLOWED_ORIGINS, QUERY_PARAM_EDITING_SECRET } from './constants';
import { getJssEditingSecret } from '../utils/utils';
import { RenderMiddlewareBase } from './render-middleware';
import { enforceCors } from '@sitecore-jss/sitecore-jss/utils';

/**
* Configuration for `FEAASRenderMiddleware`.
Expand Down Expand Up @@ -52,6 +53,17 @@ export class FEAASRenderMiddleware extends RenderMiddlewareBase {
headers,
});

if (!enforceCors(req, res, EDITING_ALLOWED_ORIGINS)) {
art-alexeyenko marked this conversation as resolved.
Show resolved Hide resolved
debug.editing(
'invalid origin host - set allowed origins in JSS_ALLOWED_ORIGINS environment variable'
);
return res
.status(401)
.send(
`<html><body>Requests from origin ${req.headers?.origin} are not allowed</body></html>`
);
}

if (method !== 'GET') {
debug.editing('invalid method - sent %s expected GET', method);
res.setHeader('Allow', 'GET');
Expand Down
Loading