Skip to content

Commit

Permalink
[Angular] [Next.js] Support Editing Host protection by handling OPTIO…
Browse files Browse the repository at this point in the history
…NS preflight requests (#1976)

* [Angular] [Next.js] Support Editing Host Protection by Handling OPTIONS preflight requests

* Updated CHANGELOG
  • Loading branch information
illiakovalenko authored Nov 18, 2024
1 parent e3e3221 commit 02c4c7d
Show file tree
Hide file tree
Showing 11 changed files with 172 additions and 10 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Our versioning strategy is as follows:
* `[sitecore-jss-angular]` Fix nested dynamic placeholders not being displayed in Pages ([#1947](https://github.com/Sitecore/jss/pull/1947))
* `[sitecore-jss-dev-tools]` getMetadata() now uses `npm query` command to get the list and exact versions of packages. this solution works for monorepo setups ([#1949](https://github.com/Sitecore/jss/pull/1949))
* `[templates/nextjs-sxa]` Fix an alignment issue where components using both `me-auto` and `ms-md-auto` classes resulted in inconsistent alignment of elements. ([#1946](https://github.com/Sitecore/jss/pull/1946)) ([#1950](https://github.com/Sitecore/jss/pull/1950)) ([#1955](https://github.com/Sitecore/jss/pull/1955))
* `[sitecore-jss-proxy]` Support Editing Host protection by handling OPTIONS preflight requests ([#1976](https://github.com/Sitecore/jss/pull/1976))

### 🎉 New Features & Improvements

Expand Down Expand Up @@ -93,6 +94,12 @@ Our versioning strategy is as follows:
* New Angular add-on's are not scaffolded within build pipeline ([#1962](https://github.com/Sitecore/jss/pull/1962))
* `[template/angular-xmcloud]``[template/xmcloud-proxy]` Add README.md ([#1965](https://github.com/Sitecore/jss/pull/1965))

## 22.2.2

### 🐛 Bug Fixes

* `[sitecore-jss-nextjs]` Support Editing Host protection by handling OPTIONS preflight requests (#[TBD](TBD))

## 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 @@ -62,6 +62,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 @@ -59,6 +59,9 @@ const mockResponse = () => {
res.status = spy(() => {
return res;
});
res.send = spy(() => {
return res;
});
res.json = spy(() => {
return res;
});
Expand Down Expand Up @@ -107,7 +110,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 @@ -124,6 +127,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 @@ -476,6 +476,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 @@ -67,9 +67,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 @@ -84,6 +84,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
14 changes: 14 additions & 0 deletions packages/sitecore-jss-proxy/src/middleware/editing/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('editingRouter', () => {
app = express();

process.env.JSS_EDITING_SECRET = 'correct';
process.env.JSS_ALLOWED_ORIGINS = 'http://allowed.com';
});

afterEach(() => {
Expand Down Expand Up @@ -92,4 +93,17 @@ describe('editingRouter', () => {
done
);
});

it('should response 204 when preflight OPTIONS request is sent', (done) => {
app.use('/api/editing', editingRouter(defaultOptions));

request(app)
.options('/api/editing/config')
.query({ secret: 'correct' })
.set('origin', 'http://allowed.com')
.expect(204, '', done)
.expect('Access-Control-Allow-Origin', 'http://allowed.com')
.expect('Access-Control-Allow-Methods', 'GET, POST, OPTIONS, DELETE, PUT, PATCH')
.expect('Access-Control-Allow-Headers', 'Content-Type, Authorization');
});
});
7 changes: 7 additions & 0 deletions packages/sitecore-jss-proxy/src/middleware/editing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ export const editingMiddleware = async (
return res.status(401).send('Missing or invalid secret');
}

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

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

return next();
};

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 02c4c7d

Please sign in to comment.