diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a7e3a69e5a..b03c1aa1a52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ The version headers in this history reflect the versions of Apollo Server itself ## vNEXT +## v3.11.0 +- ⚠️ **SECURITY**: The cache control plugin no longer sets the `cache-control` HTTP response header if the operation is part of a batched HTTP request. Previously, it would set the header to a value describing the cache policy of only one of the operations in the request, which could lead to data being unintentionally cached by proxies or clients. This bug was introduced in v3.0.0 and this fix restores the behavior of Apollo Server 2. (In Apollo Server 4 (specifically, `@apollo/server@4.1.0` or newer), the features work properly together, setting the header based on the combined cache policy of all operations.) This could theoretically have led to data tagged as uncacheable being cached and potentially served to different users. More details are available at the [security advisory](https://github.com/apollographql/apollo-server/security/advisories/GHSA-8r69-3cvp-wxc3). +- `apollo-server-core`: New field `GraphQLRequestContext.requestIsBatched` available to plugins. + ## v3.10.4 - `apollo-server-core`: Manage memory more efficiently in the usage reporting plugin by allowing large objects to be garbage collected more quickly. [PR #7106](https://github.com/apollographql/apollo-server/pull/7106) diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 8e9daefa51b..a8117f1224f 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -1032,6 +1032,7 @@ export class ApolloServerBase< }, debug: options.debug, overallCachePolicy: newCachePolicy(), + requestIsBatched: false, }; return processGraphQLRequest(options, requestCtx); diff --git a/packages/apollo-server-core/src/__tests__/runQuery.test.ts b/packages/apollo-server-core/src/__tests__/runQuery.test.ts index 7336d64fc92..0a2ca892ec3 100644 --- a/packages/apollo-server-core/src/__tests__/runQuery.test.ts +++ b/packages/apollo-server-core/src/__tests__/runQuery.test.ts @@ -59,6 +59,7 @@ function runQuery( debug: options.debug, cache: {} as any, overallCachePolicy: newCachePolicy(), + requestIsBatched: false, ...requestContextExtra, }); } diff --git a/packages/apollo-server-core/src/plugin/cacheControl/index.ts b/packages/apollo-server-core/src/plugin/cacheControl/index.ts index d3a7c933d2a..4ff5f95d0ca 100644 --- a/packages/apollo-server-core/src/plugin/cacheControl/index.ts +++ b/packages/apollo-server-core/src/plugin/cacheControl/index.ts @@ -232,18 +232,21 @@ export function ApolloServerPluginCacheControl( }, async willSendResponse(requestContext) { - const { response, overallCachePolicy } = requestContext; + const { response, overallCachePolicy, requestIsBatched } = + requestContext; const policyIfCacheable = overallCachePolicy.policyIfCacheable(); // If the feature is enabled, there is a non-trivial cache policy, - // there are no errors, and we actually can write headers, write the - // header. + // there are no errors, we actually can write headers, and the request + // is not batched (because we have no way of merging the header across + // operations in AS3), write the header. if ( calculateHttpHeaders && policyIfCacheable && !response.errors && - response.http + response.http && + !requestIsBatched ) { response.http.headers.set( 'Cache-Control', diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index 60f6a34d073..53ebbd59176 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/packages/apollo-server-core/src/runHttpQuery.ts @@ -343,6 +343,7 @@ export async function processHTTPRequest( function buildRequestContext( request: GraphQLRequest, + requestIsBatched: boolean, ): GraphQLRequestContext { // TODO: We currently shallow clone the context for every request, // but that's unlikely to be what people want. @@ -370,6 +371,7 @@ export async function processHTTPRequest( debug: options.debug, metrics: {}, overallCachePolicy: newCachePolicy(), + requestIsBatched, }; } @@ -399,7 +401,7 @@ export async function processHTTPRequest( const responses = await Promise.all( requests.map(async (request) => { try { - const requestContext = buildRequestContext(request); + const requestContext = buildRequestContext(request, true); const response = await processGraphQLRequest( options, requestContext, @@ -429,7 +431,7 @@ export async function processHTTPRequest( // We're processing a normal request const request = parseGraphQLRequest(httpRequest.request, requestPayload); - const requestContext = buildRequestContext(request); + const requestContext = buildRequestContext(request, false); const response = await processGraphQLRequest(options, requestContext); diff --git a/packages/apollo-server-core/src/utils/pluginTestHarness.ts b/packages/apollo-server-core/src/utils/pluginTestHarness.ts index 2eb175b3d9e..72b6c1b8426 100644 --- a/packages/apollo-server-core/src/utils/pluginTestHarness.ts +++ b/packages/apollo-server-core/src/utils/pluginTestHarness.ts @@ -133,6 +133,7 @@ export default async function pluginTestHarness({ cache: new InMemoryLRUCache(), context, overallCachePolicy: newCachePolicy(), + requestIsBatched: false, }; if (requestContext.source === undefined) { diff --git a/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts b/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts index aee3bcfd5ae..2dcb547cb99 100644 --- a/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts +++ b/packages/apollo-server-express/src/__tests__/ApolloServer.test.ts @@ -424,20 +424,37 @@ describe('apollo-server-express', () => { describe('Cache Control Headers', () => { it('applies cacheControl Headers', async () => { - const { url: uri } = await createServer({ typeDefs, resolvers }); + const { server, httpServer } = await createServer({ + typeDefs, + resolvers, + }); - const apolloFetch = createApolloFetch({ uri }).useAfter( - (response, next) => { - expect(response.response.headers.get('cache-control')).toEqual( - 'max-age=200, public', - ); - next(); - }, - ); - const result = await apolloFetch({ - query: `{ cooks { title author } }`, + const res = await request(httpServer) + .post(server.graphqlPath) + .send({ query: `{ cooks { title author } }` }); + expect(res.status).toEqual(200); + expect(res.body.data).toEqual({ cooks: books }); + expect(res.header['cache-control']).toEqual('max-age=200, public'); + }); + + it('does not apply cacheControl Headers for batch', async () => { + const { server, httpServer } = await createServer({ + typeDefs, + resolvers, }); - expect(result.data).toEqual({ cooks: books }); + + const res = await request(httpServer) + .post(server.graphqlPath) + .send([ + { query: `{ cooks { title author } }` }, + { query: `{ cooks { title author } }` }, + ]); + expect(res.status).toEqual(200); + expect(res.body).toEqual([ + { data: { cooks: books } }, + { data: { cooks: books } }, + ]); + expect(res.header['cache-control']).toBeUndefined(); }); it('contains no cacheControl Headers when uncacheable', async () => { diff --git a/packages/apollo-server-integration-testsuite/src/index.ts b/packages/apollo-server-integration-testsuite/src/index.ts index cbd12502f3c..9c0c8284666 100644 --- a/packages/apollo-server-integration-testsuite/src/index.ts +++ b/packages/apollo-server-integration-testsuite/src/index.ts @@ -475,7 +475,19 @@ export default ({ }); it('can handle a basic request', async () => { - app = await createApp(); + let requestIsBatched: boolean | undefined; + app = await createApp({ + graphqlOptions: { + schema, + plugins: [ + { + async requestDidStart(requestContext) { + requestIsBatched = requestContext.requestIsBatched; + }, + }, + ], + }, + }); const expected = { testString: 'it works', }; @@ -485,6 +497,7 @@ export default ({ return req.then((res) => { expect(res.status).toEqual(200); expect(res.body.data).toEqual(expected); + expect(requestIsBatched).toEqual(false); }); }); @@ -793,7 +806,19 @@ export default ({ }); it('can handle batch requests', async () => { - app = await createApp(); + let requestIsBatched: boolean | undefined; + app = await createApp({ + graphqlOptions: { + schema, + plugins: [ + { + async requestDidStart(requestContext) { + requestIsBatched = requestContext.requestIsBatched; + }, + }, + ], + }, + }); const expected = [ { data: { @@ -826,6 +851,7 @@ export default ({ return req.then((res) => { expect(res.status).toEqual(200); expect(res.body).toEqual(expected); + expect(requestIsBatched).toEqual(true); }); }); diff --git a/packages/apollo-server-types/src/index.ts b/packages/apollo-server-types/src/index.ts index cea8ab2d08c..de5c1c1be53 100644 --- a/packages/apollo-server-types/src/index.ts +++ b/packages/apollo-server-types/src/index.ts @@ -172,6 +172,15 @@ export interface GraphQLRequestContext> { debug?: boolean; readonly overallCachePolicy: CachePolicy; + + /** + * True if this request is part of a potentially multi-operation batch. Note + * that if this is true, the headers and status code `response.http` will be + * be merged together; if two operations set the same header one will arbitrarily + * win. (In Apollo Server v4, `response.http` will be shared with the other + * operations in the batch.) + */ + readonly requestIsBatched: boolean; } export type ValidationRule = (context: ValidationContext) => ASTVisitor;