From 1344016a1489e544f8093c45d13fbab7cc1c0080 Mon Sep 17 00:00:00 2001 From: Illia Kovalenko <23364749+illiakovalenko@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:15:19 +0200 Subject: [PATCH 1/2] [Next.js] Support Editing Host protection by handling OPTIONS preflight requests (#1976) --- CHANGELOG.md | 6 ++++ .../editing/editing-config-middleware.test.ts | 30 +++++++++++++++++ .../src/editing/editing-config-middleware.ts | 8 +++++ .../editing/editing-render-middleware.test.ts | 32 ++++++++++++++++++- .../src/editing/editing-render-middleware.ts | 6 ++++ .../editing/feaas-render-middleware.test.ts | 30 ++++++++++++++++- .../src/editing/feaas-render-middleware.ts | 14 ++++++-- packages/sitecore-jss/src/utils/utils.test.ts | 27 +++++++++++++--- packages/sitecore-jss/src/utils/utils.ts | 7 ++++ 9 files changed, 150 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0df4b33ef..a8f6eb6ac1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 (#[TBD](TBD)) + ## 22.2.1 ### 🐛 Bug Fixes diff --git a/packages/sitecore-jss-nextjs/src/editing/editing-config-middleware.test.ts b/packages/sitecore-jss-nextjs/src/editing/editing-config-middleware.test.ts index acd8a8446c..189e60969e 100644 --- a/packages/sitecore-jss-nextjs/src/editing/editing-config-middleware.test.ts +++ b/packages/sitecore-jss-nextjs/src/editing/editing-config-middleware.test.ts @@ -28,6 +28,9 @@ const mockResponse = () => { res.status = spy(() => { return res; }); + res.send = spy(() => { + return res; + }); res.json = spy(() => { return res; }); @@ -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, expectedResult, diff --git a/packages/sitecore-jss-nextjs/src/editing/editing-config-middleware.ts b/packages/sitecore-jss-nextjs/src/editing/editing-config-middleware.ts index 7be123f5e9..fee23b3600 100644 --- a/packages/sitecore-jss-nextjs/src/editing/editing-config-middleware.ts +++ b/packages/sitecore-jss-nextjs/src/editing/editing-config-middleware.ts @@ -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()); diff --git a/packages/sitecore-jss-nextjs/src/editing/editing-render-middleware.test.ts b/packages/sitecore-jss-nextjs/src/editing/editing-render-middleware.test.ts index 66d50e35b2..97bb1ceac8 100644 --- a/packages/sitecore-jss-nextjs/src/editing/editing-render-middleware.test.ts +++ b/packages/sitecore-jss-nextjs/src/editing/editing-render-middleware.test.ts @@ -57,6 +57,9 @@ const mockResponse = () => { res.status = spy(() => { return res; }); + res.send = spy(() => { + return res; + }); res.json = spy(() => { return res; }); @@ -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'); @@ -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'; diff --git a/packages/sitecore-jss-nextjs/src/editing/editing-render-middleware.ts b/packages/sitecore-jss-nextjs/src/editing/editing-render-middleware.ts index 4d554d90f3..bc7bee0465 100644 --- a/packages/sitecore-jss-nextjs/src/editing/editing-render-middleware.ts +++ b/packages/sitecore-jss-nextjs/src/editing/editing-render-middleware.ts @@ -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'); diff --git a/packages/sitecore-jss-nextjs/src/editing/feaas-render-middleware.test.ts b/packages/sitecore-jss-nextjs/src/editing/feaas-render-middleware.test.ts index 165d285ee5..c7a72ce7e9 100644 --- a/packages/sitecore-jss-nextjs/src/editing/feaas-render-middleware.test.ts +++ b/packages/sitecore-jss-nextjs/src/editing/feaas-render-middleware.test.ts @@ -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; @@ -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; diff --git a/packages/sitecore-jss-nextjs/src/editing/feaas-render-middleware.ts b/packages/sitecore-jss-nextjs/src/editing/feaas-render-middleware.ts index 3e7772ba50..78a57e780d 100644 --- a/packages/sitecore-jss-nextjs/src/editing/feaas-render-middleware.ts +++ b/packages/sitecore-jss-nextjs/src/editing/feaas-render-middleware.ts @@ -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(`Invalid request method '${method}'`); } @@ -81,6 +81,14 @@ export class FEAASRenderMiddleware extends RenderMiddlewareBase { return res.status(401).send('Missing or invalid secret'); } + // 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); diff --git a/packages/sitecore-jss/src/utils/utils.test.ts b/packages/sitecore-jss/src/utils/utils.test.ts index 02579a48ae..34cea19881 100644 --- a/packages/sitecore-jss/src/utils/utils.test.ts +++ b/packages/sitecore-jss/src/utils/utils.test.ts @@ -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, }, @@ -155,14 +156,14 @@ 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); @@ -170,7 +171,7 @@ describe('utils', () => { }); 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); }); @@ -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); }); diff --git a/packages/sitecore-jss/src/utils/utils.ts b/packages/sitecore-jss/src/utils/utils.ts index dc057957e5..56f4f56f6c 100644 --- a/packages/sitecore-jss/src/utils/utils.ts +++ b/packages/sitecore-jss/src/utils/utils.ts @@ -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 @@ -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; From 135225ec0b756780048b7bda78e44fa4ef379e08 Mon Sep 17 00:00:00 2001 From: illiakovalenko Date: Thu, 28 Nov 2024 05:47:00 +0200 Subject: [PATCH 2/2] Updated CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8f6eb6ac1..06d8aecb76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ Our versioning strategy is as follows: ### 🐛 Bug Fixes -* `[sitecore-jss-nextjs]` Support Editing Host protection by handling OPTIONS preflight requests (#[TBD](TBD)) +* `[sitecore-jss-nextjs]` Support Editing Host protection by handling OPTIONS preflight requests ([#1986](https://github.com/Sitecore/jss/pull/1986)) ## 22.2.1