Skip to content

Commit 1a0d450

Browse files
security: Pass all validation rules to the SubscriptionServer.
Co-authored-by: Jesse Rosenberger <[email protected]>
1 parent 7d6f234 commit 1a0d450

File tree

3 files changed

+125
-1
lines changed

3 files changed

+125
-1
lines changed

CHANGELOG.md

+11-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,17 @@ The version headers in this history reflect the versions of Apollo Server itself
99

1010
> The changes noted within this `vNEXT` section have not been released yet. New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development. When a release is being prepared, a new header will be (manually) created below and the appropriate changes within that release will be moved into the new section.
1111
12-
- _Nothing yet! Stay tuned!_
12+
- **SECURITY:** If subscriptions were disabled with `subscriptions: false`, there is not a possible security risk. When subscriptions are enabled (**the default, when `subscriptions: false` is not explicitly set, regardless of whether there is a `Subscription` type in the schema**), ALL `validationRules` (including those that prevent introspection) will now passed be through to the underlying `SubscriptionServer` which is implemented by the [`subscriptions-transport-ws` ](https://github.com/apollographql/subscriptions-transport-ws) package. The previous behavior of not passing `validationRules` was a bug.
13+
14+
This change means two things, the second of which affects most use cases:
15+
16+
- User-provided validation rules (those provided by implementors to the `validationRules` option during `ApolloServer` construction) will now be passed to and enforced by the subscriptions server.
17+
18+
- Internal validation rules, like the [`NoIntrospection`](https://github.com/apollographql/apollo-server/blob/7d6f23443/packages/apollo-server-core/src/ApolloServer.ts#L77-L88) validation rule, will also be passed to - and enforced by - the subscriptions server.
19+
20+
> The `NoIntrospection` validation rule is used by Apollo Server to disable introspection when `introspection: true` is set explicitly, or when it is disabled implicitly when the `NODE_ENV` environment variable is set to `production`. (The former, automatic disabling of introspection in production can be disabled by explicitly setting `introspection: true`. If this is set on a server, then there is no change in behavior by this commit.)
21+
22+
**To be clear, if subscriptions were disabled with `subscriptions: false`, the server is unaffected. In all other cases, introspection was unexpectedly enabled on the WebSocket endpoint provided by `SubscriptionServer` when it was meant to be disabled, either with `introspection: false` or when deployed to production. The risk is largely dependent on the data exposed in the schema itself.**
1323

1424
### v2.13.0
1525

packages/apollo-server-core/src/ApolloServer.ts

+1
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,7 @@ export class ApolloServerBase {
716716
return { ...connection, context };
717717
},
718718
keepAlive,
719+
validationRules: this.requestOptions.validationRules
719720
},
720721
server instanceof WebSocket.Server
721722
? server

packages/apollo-server-integration-testsuite/src/ApolloServer.ts

+113
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
GraphQLError,
1414
ValidationContext,
1515
FieldDefinitionNode,
16+
getIntrospectionQuery,
1617
} from 'graphql';
1718

1819
import { PubSub } from 'graphql-subscriptions';
@@ -1952,6 +1953,118 @@ export function testApolloServer<AS extends ApolloServerBase>(
19521953
})
19531954
.catch(done.fail);
19541955
});
1956+
it('allows introspection when introspection is enabled on ApolloServer', done => {
1957+
const typeDefs = gql`
1958+
type Query {
1959+
hi: String
1960+
}
1961+
1962+
type Subscription {
1963+
num: Int
1964+
}
1965+
`;
1966+
1967+
const query = getIntrospectionQuery();
1968+
1969+
const resolvers = {
1970+
Query: {
1971+
hi: () => 'here to placate graphql-js',
1972+
},
1973+
Subscription: {
1974+
num: {
1975+
subscribe: () => {
1976+
createEvent(1);
1977+
createEvent(2);
1978+
createEvent(3);
1979+
return pubsub.asyncIterator(SOMETHING_CHANGED_TOPIC);
1980+
},
1981+
},
1982+
},
1983+
};
1984+
1985+
createApolloServer({
1986+
typeDefs,
1987+
resolvers,
1988+
introspection: true,
1989+
}).then(({ port, server, httpServer }) => {
1990+
server.installSubscriptionHandlers(httpServer);
1991+
1992+
const client = new SubscriptionClient(
1993+
`ws://localhost:${port}${server.subscriptionsPath}`,
1994+
{},
1995+
WebSocket,
1996+
);
1997+
1998+
const observable = client.request({ query });
1999+
2000+
subscription = observable.subscribe({
2001+
next: ({ data }) => {
2002+
try {
2003+
expect(data).toMatchObject({ __schema: expect.any(Object) })
2004+
} catch (e) {
2005+
done.fail(e);
2006+
}
2007+
done();
2008+
}
2009+
});
2010+
});
2011+
});
2012+
it('disallows introspection when it\'s disabled on ApolloServer', done => {
2013+
const typeDefs = gql`
2014+
type Query {
2015+
hi: String
2016+
}
2017+
2018+
type Subscription {
2019+
num: Int
2020+
}
2021+
`;
2022+
2023+
const query = getIntrospectionQuery();
2024+
2025+
const resolvers = {
2026+
Query: {
2027+
hi: () => 'here to placate graphql-js',
2028+
},
2029+
Subscription: {
2030+
num: {
2031+
subscribe: () => {
2032+
createEvent(1);
2033+
createEvent(2);
2034+
createEvent(3);
2035+
return pubsub.asyncIterator(SOMETHING_CHANGED_TOPIC);
2036+
},
2037+
},
2038+
},
2039+
};
2040+
2041+
createApolloServer({
2042+
typeDefs,
2043+
resolvers,
2044+
introspection: false,
2045+
}).then(({ port, server, httpServer }) => {
2046+
server.installSubscriptionHandlers(httpServer);
2047+
2048+
const client = new SubscriptionClient(
2049+
`ws://localhost:${port}${server.subscriptionsPath}`,
2050+
{},
2051+
WebSocket,
2052+
);
2053+
2054+
const observable = client.request({ query });
2055+
2056+
subscription = observable.subscribe({
2057+
next: ({ data }) => {
2058+
try {
2059+
expect(data).toBeUndefined();
2060+
} catch (e) {
2061+
done.fail(e);
2062+
}
2063+
done();
2064+
}
2065+
});
2066+
});
2067+
});
19552068
});
19562069

19572070
describe('Persisted Queries', () => {

0 commit comments

Comments
 (0)