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
  • Loading branch information
illiakovalenko committed Nov 18, 2024
1 parent e3e3221 commit 81e27d4
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 10 deletions.
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 81e27d4

Please sign in to comment.