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

[Angular] [Next.js] Support Editing Host protection by handling OPTIONS preflight requests #1976

Merged
merged 2 commits into from
Nov 18, 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
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