Skip to content

Commit

Permalink
[Next.js] Support Editing Host protection by handling OPTIONS preflig…
Browse files Browse the repository at this point in the history
…ht requests (#1986)
  • Loading branch information
illiakovalenko authored Nov 28, 2024
1 parent f48cfbd commit 3eff42e
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 10 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ Our versioning strategy is as follows:
- Minor: may include breaking changes in framework packages (e.g. framework upgrades, new features, improvements)
- Major: may include breaking changes in core packages (e.g. major architectural changes, major features)

## 22.2.2

### 🐛 Bug Fixes

* `[sitecore-jss-nextjs]` Support Editing Host protection by handling OPTIONS preflight requests ([#1986](https://github.com/Sitecore/jss/pull/1986))

## 22.2.1

### 🐛 Bug Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ const mockResponse = () => {
res.status = spy(() => {
return res;
});
res.send = spy(() => {
return res;
});
res.json = spy(() => {
return res;
});
Expand Down Expand Up @@ -120,6 +123,33 @@ describe('EditingConfigMiddleware', () => {
expect(res.json).to.have.been.calledWith(expectedResultForbidden);
});

it('should respond with 204 for preflight OPTIONS request', async () => {
const query = {} as Query;
query[QUERY_PARAM_EDITING_SECRET] = secret;
const req = mockRequest('OPTIONS', query);
const res = mockResponse();

const middleware = new EditingConfigMiddleware({ components: componentsArray, metadata });
const handler = middleware.getHandler();

await handler(req, res);

expect(res.setHeader.getCall(0).args).to.deep.equal([
'Access-Control-Allow-Origin',
allowedOrigin,
]);
expect(res.setHeader.getCall(1).args).to.deep.equal([
'Access-Control-Allow-Methods',
'GET, POST, OPTIONS, DELETE, PUT, PATCH',
]);
expect(res.setHeader.getCall(2).args).to.deep.equal([
'Access-Control-Allow-Headers',
'Content-Type, Authorization',
]);
expect(res.status).to.have.been.calledWith(204);
expect(res.send).to.have.been.calledOnceWith(null);
});

const testEditingConfig = async (
components: string[] | Map<string, unknown>,
expectedResult,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ export class EditingConfigMiddleware {
return res.status(401).json({ message: 'Missing or invalid editing secret' });
}

// Handle preflight request
if (_req.method === 'OPTIONS') {
debug.editing('preflight request');

// CORS headers are set by enforceCors
return res.status(204).send(null);
}

const components = Array.isArray(this.config.components)
? this.config.components
: Array.from(this.config.components.keys());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ const mockResponse = () => {
res.status = spy(() => {
return res;
});
res.send = spy(() => {
return res;
});
res.json = spy(() => {
return res;
});
Expand Down Expand Up @@ -105,7 +108,7 @@ describe('EditingRenderMiddleware', () => {
delete process.env.JSS_ALLOWED_ORIGINS;
});

it('should respondWith 405 for unsupported method', async () => {
it('should respond with 405 for unsupported method', async () => {
const query = {} as Query;
query[QUERY_PARAM_EDITING_SECRET] = secret;
const req = mockRequest(EE_BODY, query, 'PUT');
Expand All @@ -122,6 +125,33 @@ describe('EditingRenderMiddleware', () => {
expect(res.json).to.have.been.calledOnce;
});

it('should respond with 204 for OPTIONS method', async () => {
const query = {} as Query;
query[QUERY_PARAM_EDITING_SECRET] = secret;
const req = mockRequest(EE_BODY, query, 'OPTIONS');
const res = mockResponse();

const middleware = new EditingRenderMiddleware();
const handler = middleware.getHandler();

await handler(req, res);

expect(res.status).to.have.been.calledOnceWith(204);
expect(res.setHeader.getCall(0).args).to.deep.equal([
'Access-Control-Allow-Origin',
allowedOrigin,
]);
expect(res.setHeader.getCall(1).args).to.deep.equal([
'Access-Control-Allow-Methods',
'GET, POST, OPTIONS, DELETE, PUT, PATCH',
]);
expect(res.setHeader.getCall(2).args).to.deep.equal([
'Access-Control-Allow-Headers',
'Content-Type, Authorization',
]);
expect(res.send).to.have.been.calledOnceWith(null);
});

it('should respond with 401 for invalid secret', async () => {
const query = {} as Query;
query[QUERY_PARAM_EDITING_SECRET] = 'nope';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,12 @@ export class EditingRenderMiddleware extends RenderMiddlewareBase {
await handler.render(req, res);
break;
}
case 'OPTIONS': {
debug.editing('preflight request');

// CORS headers are set by enforceCors
return res.status(204).send(null);
}
default:
debug.editing('invalid method - sent %s expected GET/POST', req.method);
res.setHeader('Allow', 'GET, POST');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,34 @@ describe('FEAASRenderMiddleware', () => {
);
});

it('should respond with 204 for preflight OPTIONS request', async () => {
const query = {} as Query;
query[QUERY_PARAM_EDITING_SECRET] = secret;

const req = mockRequest(query, 'OPTIONS');
const res = mockResponse();

const middleware = new FEAASRenderMiddleware();
const handler = middleware.getHandler();

await handler(req, res);

expect(res.setHeader.getCall(0).args).to.deep.equal([
'Access-Control-Allow-Origin',
allowedOrigin,
]);
expect(res.setHeader.getCall(1).args).to.deep.equal([
'Access-Control-Allow-Methods',
'GET, POST, OPTIONS, DELETE, PUT, PATCH',
]);
expect(res.setHeader.getCall(2).args).to.deep.equal([
'Access-Control-Allow-Headers',
'Content-Type, Authorization',
]);
expect(res.status).to.have.been.calledOnceWith(204);
expect(res.send).to.have.been.calledOnceWith(null);
});

it('should throw error', async () => {
const query = {} as Query;
query[QUERY_PARAM_EDITING_SECRET] = secret;
Expand Down Expand Up @@ -140,7 +168,7 @@ describe('FEAASRenderMiddleware', () => {

await handler(req, res);

expect(res.setHeader).to.have.been.calledWithExactly('Allow', 'GET');
expect(res.setHeader).to.have.been.calledWithExactly('Allow', 'GET, OPTIONS');
expect(res.status).to.have.been.calledOnce;
expect(res.status).to.have.been.calledWith(405);
expect(res.send).to.have.been.calledOnce;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ export class FEAASRenderMiddleware extends RenderMiddlewareBase {
);
}

if (method !== 'GET') {
debug.editing('invalid method - sent %s expected GET', method);
res.setHeader('Allow', 'GET');
if (!method || !['GET', 'OPTIONS'].includes(method)) {
debug.editing('invalid method - sent %s expected GET,OPTIONS', method);
res.setHeader('Allow', 'GET, OPTIONS');
return res.status(405).send(`<html><body>Invalid request method '${method}'</body></html>`);
}

Expand All @@ -81,6 +81,14 @@ export class FEAASRenderMiddleware extends RenderMiddlewareBase {
return res.status(401).send('<html><body>Missing or invalid secret</body></html>');
}

// Handle preflight request
if (method === 'OPTIONS') {
debug.editing('preflight request');

// CORS headers are set by enforceCors
return res.status(204).send(null);
}

try {
// Get query string parameters to propagate on subsequent requests (e.g. for deployment protection bypass)
const params = this.getQueryParamsForPropagation(query);
Expand Down
27 changes: 22 additions & 5 deletions packages/sitecore-jss/src/utils/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ describe('utils', () => {

describe('enforceCors', () => {
const mockOrigin = 'https://maybeallowed.com';
const mockRequest = (origin?: string) => {
const mockRequest = ({ origin, method }: { origin?: string; method?: string } = {}) => {
return {
method: method || 'GET',
headers: {
origin: origin || mockOrigin,
},
Expand Down Expand Up @@ -155,22 +156,22 @@ describe('utils', () => {
});

it('should return true if origin is found in allowedOrigins passed as argument', () => {
const req = mockRequest('http://allowed.com');
const req = mockRequest({ origin: 'http://allowed.com' });
const res = mockResponse();

expect(enforceCors(req, res, ['http://allowed.com'])).to.be.equal(true);
});

it('should return false if origin matches neither allowedOrigins from JSS_ALLOWED_ORIGINS env variable nor argument', () => {
const req = mockRequest('https://notallowed.com');
const req = mockRequest({ origin: 'https://notallowed.com' });
const res = mockResponse();
process.env.JSS_ALLOWED_ORIGINS = 'https://strictallowed.com, https://alsoallowed.com';
expect(enforceCors(req, res, ['https://paramallowed.com'])).to.be.equal(false);
delete process.env.JSS_ALLOWED_ORIGINS;
});

it('should return true when origin matches a wildcard value from allowedOrigins', () => {
const req = mockRequest('https://allowed.dev.com');
const req = mockRequest({ origin: 'https://allowed.dev.com' });
const res = mockResponse();
expect(enforceCors(req, res, ['https://allowed.*.com'])).to.be.equal(true);
});
Expand All @@ -187,8 +188,24 @@ describe('utils', () => {
);
});

it('should set CORS headers for preflight OPTIONS request', () => {
const req = mockRequest({ method: 'OPTIONS' });
const res = mockResponse();
const allowedMethods = 'GET, POST, OPTIONS, DELETE, PUT, PATCH';
enforceCors(req, res, [mockOrigin]);
expect(res.setHeader).to.have.been.called.with('Access-Control-Allow-Origin', mockOrigin);
expect(res.setHeader).to.have.been.called.with(
'Access-Control-Allow-Methods',
allowedMethods
);
expect(res.setHeader).to.have.been.called.with(
'Access-Control-Allow-Headers',
'Content-Type, Authorization'
);
});

it('should consider existing CORS header when present', () => {
const req = mockRequest('https://preallowed.com');
const req = mockRequest({ origin: 'https://preallowed.com' });
const res = mockResponse('https://preallowed.com');
expect(enforceCors(req, res)).to.be.equal(true);
});
Expand Down
7 changes: 7 additions & 0 deletions packages/sitecore-jss/src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export const getAllowedOriginsFromEnv = () =>
* set in JSS's JSS_ALLOWED_ORIGINS env variable, passed via allowedOrigins param and/or
* be already set in Access-Control-Allow-Origin by other logic.
* Applies Access-Control-Allow-Origin and Access-Control-Allow-Methods on match
* Also applies Access-Control-Allow-Headers for preflight requests
* @param {IncomingMessage} req incoming request
* @param {OutgoingMessage} res response to set CORS headers for
* @param {string[]} [allowedOrigins] additional list of origins to test against
Expand Down Expand Up @@ -149,6 +150,12 @@ export const enforceCors = (
) {
res.setHeader('Access-Control-Allow-Origin', origin);
res.setHeader('Access-Control-Allow-Methods', 'GET, POST, OPTIONS, DELETE, PUT, PATCH');

// set the allowed headers for preflight requests
if (req.method === 'OPTIONS') {
res.setHeader('Access-Control-Allow-Headers', 'Content-Type, Authorization');
}

return true;
}
return false;
Expand Down

0 comments on commit 3eff42e

Please sign in to comment.