Skip to content

Commit

Permalink
security: Pass all validation rules to the SubscriptionServer.
Browse files Browse the repository at this point in the history
Co-authored-by: Jesse Rosenberger <git@jro.cc>
  • Loading branch information
trevor-scheer and abernix committed May 27, 2020
1 parent 7d6f234 commit 1a0d450
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 1 deletion.
12 changes: 11 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,17 @@ The version headers in this history reflect the versions of Apollo Server itself

> 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.
- _Nothing yet! Stay tuned!_
- **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.

This change means two things, the second of which affects most use cases:

- 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.

- 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.

> 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.)
**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.**

### v2.13.0

Expand Down
1 change: 1 addition & 0 deletions packages/apollo-server-core/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ export class ApolloServerBase {
return { ...connection, context };
},
keepAlive,
validationRules: this.requestOptions.validationRules
},
server instanceof WebSocket.Server
? server
Expand Down
113 changes: 113 additions & 0 deletions packages/apollo-server-integration-testsuite/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
GraphQLError,
ValidationContext,
FieldDefinitionNode,
getIntrospectionQuery,
} from 'graphql';

import { PubSub } from 'graphql-subscriptions';
Expand Down Expand Up @@ -1952,6 +1953,118 @@ export function testApolloServer<AS extends ApolloServerBase>(
})
.catch(done.fail);
});
it('allows introspection when introspection is enabled on ApolloServer', done => {
const typeDefs = gql`
type Query {
hi: String
}
type Subscription {
num: Int
}
`;

const query = getIntrospectionQuery();

const resolvers = {
Query: {
hi: () => 'here to placate graphql-js',
},
Subscription: {
num: {
subscribe: () => {
createEvent(1);
createEvent(2);
createEvent(3);
return pubsub.asyncIterator(SOMETHING_CHANGED_TOPIC);
},
},
},
};

createApolloServer({
typeDefs,
resolvers,
introspection: true,
}).then(({ port, server, httpServer }) => {
server.installSubscriptionHandlers(httpServer);

const client = new SubscriptionClient(
`ws://localhost:${port}${server.subscriptionsPath}`,
{},
WebSocket,
);

const observable = client.request({ query });

subscription = observable.subscribe({
next: ({ data }) => {
try {
expect(data).toMatchObject({ __schema: expect.any(Object) })
} catch (e) {
done.fail(e);
}
done();
}
});
});
});
it('disallows introspection when it\'s disabled on ApolloServer', done => {
const typeDefs = gql`
type Query {
hi: String
}
type Subscription {
num: Int
}
`;

const query = getIntrospectionQuery();

const resolvers = {
Query: {
hi: () => 'here to placate graphql-js',
},
Subscription: {
num: {
subscribe: () => {
createEvent(1);
createEvent(2);
createEvent(3);
return pubsub.asyncIterator(SOMETHING_CHANGED_TOPIC);
},
},
},
};

createApolloServer({
typeDefs,
resolvers,
introspection: false,
}).then(({ port, server, httpServer }) => {
server.installSubscriptionHandlers(httpServer);

const client = new SubscriptionClient(
`ws://localhost:${port}${server.subscriptionsPath}`,
{},
WebSocket,
);

const observable = client.request({ query });

subscription = observable.subscribe({
next: ({ data }) => {
try {
expect(data).toBeUndefined();
} catch (e) {
done.fail(e);
}
done();
}
});
});
});
});

describe('Persisted Queries', () => {
Expand Down

0 comments on commit 1a0d450

Please sign in to comment.