From 1045f04c4be080e250364617e226cb714231a01c Mon Sep 17 00:00:00 2001 From: Rhys Bartels-Waller Date: Mon, 3 Aug 2020 23:26:25 +1000 Subject: [PATCH] fix(server): return HTTP 403 errors when rejecting disallowed queries replaces apollo-server-plugin with express middleware since errors throw within are unable to be caught there too. --- packages/server/package.json | 2 + packages/server/src/AllowList.ts | 1 + packages/server/src/CompleteApiServer.ts | 23 ++++ packages/server/src/Server.ts | 105 +++++++++++++++--- .../allow_list_plugin.ts | 19 ---- .../server/src/apollo_server_plugins/index.ts | 1 - packages/server/src/config.ts | 17 +-- .../src/express_middleware/allow_list.ts | 10 ++ .../server/src/express_middleware/index.ts | 1 + packages/server/src/index.ts | 77 ++----------- packages/server/src/util.ts | 9 ++ packages/server/test/Server.test.ts | 52 ++++----- yarn.lock | 4 +- 13 files changed, 170 insertions(+), 151 deletions(-) create mode 100644 packages/server/src/AllowList.ts create mode 100644 packages/server/src/CompleteApiServer.ts delete mode 100644 packages/server/src/apollo_server_plugins/allow_list_plugin.ts create mode 100644 packages/server/src/express_middleware/allow_list.ts create mode 100644 packages/server/src/express_middleware/index.ts create mode 100644 packages/server/src/util.ts diff --git a/packages/server/package.json b/packages/server/package.json index 71c7492e3..9f7c5d372 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -31,8 +31,10 @@ "apollo-server-core": "^2.14.3", "apollo-server-errors": "^2.4.1", "apollo-server-express": "^2.14.3", + "body-parser": "^1.19.0", "cors": "^2.8.5", "express": "^4.17.1", + "fs-extra": "^9.0.1", "graphql": "14.5.8", "graphql-depth-limit": "^1.1.0", "prom-client": "^11.5.3", diff --git a/packages/server/src/AllowList.ts b/packages/server/src/AllowList.ts new file mode 100644 index 000000000..561c3df62 --- /dev/null +++ b/packages/server/src/AllowList.ts @@ -0,0 +1 @@ +export type AllowList = {[key: string]: number } diff --git a/packages/server/src/CompleteApiServer.ts b/packages/server/src/CompleteApiServer.ts new file mode 100644 index 000000000..5c94bf79d --- /dev/null +++ b/packages/server/src/CompleteApiServer.ts @@ -0,0 +1,23 @@ +import { Config } from './config' +import { Server } from './Server' +import { buildSchema as buildCardanoDbHasuraSchema, Db } from '@cardano-graphql/api-cardano-db-hasura' +import { buildSchema as buildGenesisSchema } from '@cardano-graphql/api-genesis' +import { GraphQLSchema } from 'graphql' + +export * from './config' + +export async function CompleteApiServer (config: Config): Promise { + const schemas: GraphQLSchema[] = [] + if (config.genesisFileByron !== undefined || config.genesisFileShelley !== undefined) { + schemas.push(buildGenesisSchema({ + ...config.genesisFileByron !== undefined ? { byron: require(config.genesisFileByron) } : {}, + ...config.genesisFileShelley !== undefined ? { shelley: require(config.genesisFileShelley) } : {} + })) + } + if (config.hasuraUri !== undefined) { + const db = new Db(config.hasuraUri) + await db.init() + schemas.push(await buildCardanoDbHasuraSchema(config.hasuraUri, db)) + } + return new Server(schemas, config) +} diff --git a/packages/server/src/Server.ts b/packages/server/src/Server.ts index 679d8b44b..918df91a1 100644 --- a/packages/server/src/Server.ts +++ b/packages/server/src/Server.ts @@ -1,17 +1,94 @@ -import { ApolloServer, ApolloServerExpressConfig } from 'apollo-server-express' +import { ApolloServer, CorsOptions } from 'apollo-server-express' import express from 'express' -import corsMiddleware from 'cors' +import { GraphQLSchema } from 'graphql' +import { mergeSchemas } from '@graphql-tools/merge' +import { prometheusMetricsPlugin } from './apollo_server_plugins' +import { allowListMiddleware } from './express_middleware' +import depthLimit from 'graphql-depth-limit' +import { PluginDefinition } from 'apollo-server-core' +import { AllowList } from './AllowList' +import { json } from 'body-parser' +import fs from 'fs-extra' +import { listenPromise } from './util' +import http from 'http' -export function Server ( - app: express.Application, - apolloServerExpressConfig: ApolloServerExpressConfig, - cors?: corsMiddleware.CorsOptions -): ApolloServer { - const apolloServer = new ApolloServer(apolloServerExpressConfig) - apolloServer.applyMiddleware({ - app, - cors, - path: '/' - }) - return apolloServer +export type Config = { + allowIntrospection?: boolean + allowListPath?: string + allowedOrigins?: CorsOptions['origin'] + apiPort: number + cacheEnabled?: boolean + prometheusMetrics?: boolean + queryDepthLimit?: number + tracing?: boolean +} + +export class Server { + public app: express.Application + private apolloServer: ApolloServer + private config: Config + private httpServer: http.Server + private schemas: GraphQLSchema[] + + constructor (schemas: GraphQLSchema[], config?: Config) { + this.app = express() + this.config = config + this.schemas = schemas + } + + async init () { + let allowList: AllowList + const plugins: PluginDefinition[] = [] + const validationRules = [] + if (this.config?.allowListPath) { + try { + const file = await fs.readFile(this.config.allowListPath, 'utf8') + allowList = JSON.parse(file) + this.app.use('/', json(), allowListMiddleware(allowList)) + console.log('The server will only allow only operations from the provided list') + } catch (error) { + console.error(`Cannot read or parse allow-list JSON file at ${this.config.allowListPath}`) + throw error + } + } + if (this.config?.prometheusMetrics) { + plugins.push(prometheusMetricsPlugin(this.app)) + console.log('Prometheus metrics will be served at /metrics') + } + if (this.config?.queryDepthLimit) { + validationRules.push(depthLimit(this.config?.queryDepthLimit)) + } + this.apolloServer = new ApolloServer({ + cacheControl: this.config?.cacheEnabled ? { defaultMaxAge: 20 } : undefined, + introspection: !!this.config?.allowIntrospection, + playground: !!this.config?.allowIntrospection, + plugins, + validationRules, + schema: mergeSchemas({ + schemas: this.schemas + }) + }) + this.apolloServer.applyMiddleware({ + app: this.app, + cors: this.config?.allowedOrigins ? { + origin: this.config?.allowedOrigins + } : undefined, + path: '/' + }) + } + + async start () { + this.httpServer = await listenPromise(this.app, { port: this.config.apiPort }) + console.log( + `GraphQL HTTP server at http://localhost:${this.config.apiPort}${this.apolloServer.graphqlPath}` + ) + } + + shutdown () { + this.httpServer.close() + console.log( + `GraphQL HTTP server at http://localhost:${this.config.apiPort}${this.apolloServer.graphqlPath} + shutting down` + ) + } } diff --git a/packages/server/src/apollo_server_plugins/allow_list_plugin.ts b/packages/server/src/apollo_server_plugins/allow_list_plugin.ts deleted file mode 100644 index 205c2961b..000000000 --- a/packages/server/src/apollo_server_plugins/allow_list_plugin.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { ForbiddenError } from 'apollo-server-errors' -import { PluginDefinition } from 'apollo-server-core' - -export function allowListPlugin (allowList: {[key: string]: number }): PluginDefinition { - return { - serverWillStart (_service) { - console.log(`The server is configured to accept ${Object.keys(allowList).length} operations from the provided allow-list`) - }, - requestDidStart () { - return { - parsingDidStart (context) { - if (allowList[context.request.query] === undefined) { - throw new ForbiddenError('Operation is disallowed') - } - } - } - } - } -} diff --git a/packages/server/src/apollo_server_plugins/index.ts b/packages/server/src/apollo_server_plugins/index.ts index 370065767..8a765df63 100644 --- a/packages/server/src/apollo_server_plugins/index.ts +++ b/packages/server/src/apollo_server_plugins/index.ts @@ -1,2 +1 @@ export * from './prometheus_metrics_plugin' -export * from './allow_list_plugin' diff --git a/packages/server/src/config.ts b/packages/server/src/config.ts index 959708296..b75f13c5c 100644 --- a/packages/server/src/config.ts +++ b/packages/server/src/config.ts @@ -1,19 +1,10 @@ -import { CorsOptions } from 'apollo-server-express' import { IntrospectionNotPermitted, MissingConfig, TracingRequired } from './errors' +import { Config as ServerConfig } from './Server' -export type Config = { - allowIntrospection?: boolean - allowedOrigins: CorsOptions['origin'] - allowListPath: string - apiPort: number - cacheEnabled: boolean +export type Config = ServerConfig & { genesisFileByron: string genesisFileShelley: string hasuraUri: string - poolMetadataProxy: string - prometheusMetrics: boolean - queryDepthLimit: number - tracing: boolean } export async function getConfig (): Promise { @@ -26,7 +17,6 @@ export async function getConfig (): Promise { genesisFileByron, genesisFileShelley, hasuraUri, - poolMetadataProxy, prometheusMetrics, queryDepthLimit, tracing @@ -50,7 +40,6 @@ export async function getConfig (): Promise { genesisFileByron, genesisFileShelley, hasuraUri, - poolMetadataProxy, prometheusMetrics, queryDepthLimit: queryDepthLimit || 10, tracing @@ -67,7 +56,6 @@ function filterAndTypecastEnvs (env: any) { GENESIS_FILE_BYRON, GENESIS_FILE_SHELLEY, HASURA_URI, - POOL_METADATA_PROXY, PROMETHEUS_METRICS, QUERY_DEPTH_LIMIT, TRACING, @@ -86,7 +74,6 @@ function filterAndTypecastEnvs (env: any) { genesisFileByron: GENESIS_FILE_BYRON, genesisFileShelley: GENESIS_FILE_SHELLEY, hasuraUri: HASURA_URI, - poolMetadataProxy: POOL_METADATA_PROXY, prometheusMetrics: PROMETHEUS_METRICS === 'true' ? true : undefined, queryDepthLimit: Number(QUERY_DEPTH_LIMIT), tracing: TRACING === 'true' ? true : undefined diff --git a/packages/server/src/express_middleware/allow_list.ts b/packages/server/src/express_middleware/allow_list.ts new file mode 100644 index 000000000..e061584a3 --- /dev/null +++ b/packages/server/src/express_middleware/allow_list.ts @@ -0,0 +1,10 @@ +import express from 'express' + +export function allowListMiddleware (allowList: {[key: string]: number }) { + return (request: express.Request, response: express.Response, next: Function) => { + if (allowList[request.body.query] === undefined) { + response.sendStatus(403) + } + next() + } +} diff --git a/packages/server/src/express_middleware/index.ts b/packages/server/src/express_middleware/index.ts new file mode 100644 index 000000000..29c2ca6dd --- /dev/null +++ b/packages/server/src/express_middleware/index.ts @@ -0,0 +1 @@ +export * from './allow_list' diff --git a/packages/server/src/index.ts b/packages/server/src/index.ts index 223870534..0519ff27f 100644 --- a/packages/server/src/index.ts +++ b/packages/server/src/index.ts @@ -1,76 +1,15 @@ -import * as apolloServerPlugins from './apollo_server_plugins' -import express from 'express' -import fs from 'fs' import { getConfig } from './config' -import { Server } from './Server' -import depthLimit from 'graphql-depth-limit' -import { mergeSchemas } from '@graphql-tools/merge' -import { PluginDefinition } from 'apollo-server-core' -import { buildSchema as buildCardanoDbHasuraSchema, Db } from '@cardano-graphql/api-cardano-db-hasura' -import { buildSchema as buildGenesisSchema } from '@cardano-graphql/api-genesis' -import { GraphQLSchema } from 'graphql' -export * from './config' -export { apolloServerPlugins } - -const { prometheusMetricsPlugin, allowListPlugin } = apolloServerPlugins - -async function boot () { - const config = await getConfig() - const schemas: GraphQLSchema[] = [] - const validationRules = [] - const plugins: PluginDefinition[] = [] - const app = express() - - if (config.genesisFileByron !== undefined || config.genesisFileShelley !== undefined) { - schemas.push(buildGenesisSchema({ - ...config.genesisFileByron !== undefined ? { byron: require(config.genesisFileByron) } : {}, - ...config.genesisFileShelley !== undefined ? { shelley: require(config.genesisFileShelley) } : {} - })) - } - - if (config.hasuraUri !== undefined) { - const db = new Db(config.hasuraUri) - await db.init() - schemas.push(await buildCardanoDbHasuraSchema(config.hasuraUri, db)) - } +import { CompleteApiServer } from './CompleteApiServer' - if (config.prometheusMetrics) { - plugins.push(prometheusMetricsPlugin(app)) - } - if (config.allowListPath) { - const allowList = JSON.parse(fs.readFileSync(config.allowListPath, 'utf8')) - plugins.push(allowListPlugin(allowList)) - } - if (config.queryDepthLimit) { - validationRules.push(depthLimit(config.queryDepthLimit)) - } - const server = Server(app, { - cacheControl: config.cacheEnabled ? { defaultMaxAge: 20 } : undefined, - introspection: config.allowIntrospection, - playground: config.allowIntrospection, - plugins, - validationRules, - schema: mergeSchemas({ - schemas - }) - }, { - origin: config.allowedOrigins - }) +export * from './config' +(async function () { try { - app.listen({ port: config.apiPort }, () => { - const serverUri = `http://localhost:${config.apiPort}` - console.log(`GraphQL HTTP server at ${serverUri}${server.graphqlPath}`) - if (process.env.NODE_ENV !== 'production' && config.allowListPath) { - console.warn('As an allow-list is in effect, the GraphQL Playground is available, but will not allow schema exploration') - } - if (config.prometheusMetrics) { - console.log(`Prometheus metrics at ${serverUri}/metrics`) - } - }) + const server = await CompleteApiServer(await getConfig()) + await server.init() + await server.start() } catch (error) { console.error(error.message) + process.exit(1) } -} - -boot() +})() diff --git a/packages/server/src/util.ts b/packages/server/src/util.ts new file mode 100644 index 000000000..468c5159e --- /dev/null +++ b/packages/server/src/util.ts @@ -0,0 +1,9 @@ +import { Application } from 'express' +import http from 'http' + +export function listenPromise (app: Application, config: { port: number }): Promise { + return new Promise(function (resolve, reject) { + const server: http.Server = app.listen(config.port, () => resolve(server)) + server.on('error', reject) + }) +} diff --git a/packages/server/test/Server.test.ts b/packages/server/test/Server.test.ts index ceeb60ccb..659ad60e0 100644 --- a/packages/server/test/Server.test.ts +++ b/packages/server/test/Server.test.ts @@ -2,34 +2,22 @@ import { ApolloClient, DocumentNode, gql, InMemoryCache } from 'apollo-boost' import { createHttpLink } from 'apollo-link-http' import fetch from 'cross-fetch' import { execSync } from 'child_process' -import express, { Application } from 'express' import { GraphQLSchema } from 'graphql' -import fs from 'fs' -import http from 'http' import path from 'path' import tmp from 'tmp-promise' import util from '@cardano-graphql/util' import { buildSchema as buildGenesisSchema } from '@cardano-graphql/api-genesis' import { Server } from '@src/Server' -import { allowListPlugin } from '@src/apollo_server_plugins' const byronTestnetGenesis = '../../../config/network/testnet/genesis/byron.json' const shelleyTestnetGenesis = '../../../config/network/testnet/genesis/shelley.json' const clientPath = path.resolve(__dirname, 'app_with_graphql_operations') const port = 3301 -function listen (app: Application, port: number): Promise { - return new Promise(function (resolve, reject) { - const server: http.Server = app.listen(port, () => resolve(server)) - server.on('error', reject) - }) -} - describe('Server', () => { let client: ApolloClient let allowedDocumentNode: DocumentNode - let app: express.Application - let httpServer: http.Server + let server: any let genesisSchema: GraphQLSchema beforeAll(async () => { @@ -55,19 +43,17 @@ describe('Server', () => { } }) }) - app = express() }) describe('Allowing specific queries', () => { describe('Booting the server without providing an allow-list', () => { beforeEach(async () => { - Server(app, { - schema: genesisSchema - }) - httpServer = await listen(app, port) + server = new Server([genesisSchema], { apiPort: port }) + await server.init() + await server.start() }) afterEach(() => { - httpServer.close() + server.shutdown() }) it('returns data for all valid queries', async () => { const validQueryResult = await client.query({ @@ -87,17 +73,20 @@ describe('Server', () => { describe('Providing an allow-list produced by persistgraphql, intended to limit the API for specific application requirements', () => { beforeEach(async () => { - const allowlistPath = await tmp.tmpName({ postfix: '.json' }) - execSync(`npx persistgraphql ${clientPath} ${allowlistPath}`) - const allowList = JSON.parse(fs.readFileSync(allowlistPath, 'utf-8')) - Server(app, { - plugins: [allowListPlugin(allowList)], - schema: genesisSchema - }) - httpServer = await listen(app, port) + const allowListPath = await tmp.tmpName({ postfix: '.json' }) + execSync(`npx persistgraphql ${clientPath} ${allowListPath}`) + server = new Server( + [genesisSchema], + { + allowListPath, + apiPort: port + } + ) + await server.init() + await server.start() }) afterEach(() => { - httpServer.close() + server.shutdown() }) it('Accepts allowed queries', async () => { @@ -107,8 +96,8 @@ describe('Server', () => { expect(result.data.genesis.shelley.maxLovelaceSupply).toBeDefined() expect(result.errors).not.toBeDefined() }) - it('Returns a networkError if a valid but disallowed operation is sent', async () => { - expect.assertions(1) + it('Returns a forbidden error and 403 HTTP response error if a valid but disallowed operation is sent', async () => { + expect.assertions(2) try { await client.query({ query: gql`query validButNotWhitelisted { @@ -121,7 +110,8 @@ describe('Server', () => { }` }) } catch (e) { - expect(e.networkError.result.errors[0].message).toBe('Operation is forbidden') + expect(e.networkError.statusCode).toBe(403) + expect(e.networkError.bodyText).toBe('Forbidden') } }) }) diff --git a/yarn.lock b/yarn.lock index 1bbe1247d..426e3988e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2457,7 +2457,7 @@ bluebird@^3.5.1: resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.7.2.tgz#9f229c15be272454ffa973ace0dbee79a1b0c36f" integrity sha512-XpNj6GDQzdfW+r2Wnn7xiSAd7TM3jzkxGXBGTtWKuSXv1xUV+azxAm8jdWZN06QTQk+2N2XB9jRDkvbmQmcRtg== -body-parser@1.19.0, body-parser@^1.18.3: +body-parser@1.19.0, body-parser@^1.18.3, body-parser@^1.19.0: version "1.19.0" resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.19.0.tgz#96b2709e57c9c4e09a6fd66a8fd979844f69f08a" integrity sha512-dhEPs72UPbDnAQJ9ZKMNTP6ptJaionhP5cBb541nXPlW60Jepo9RV/a4fX4XWW9CuFNK22krhrj1+rgzifNCsw== @@ -4086,7 +4086,7 @@ fs-constants@^1.0.0: resolved "https://registry.yarnpkg.com/fs-constants/-/fs-constants-1.0.0.tgz#6be0de9be998ce16af8afc24497b9ee9b7ccd9ad" integrity sha512-y6OAwoSIf7FyjMIv94u+b5rdheZEjzR63GTyZJm5qh4Bi+2YgwLCcI/fPFZkL5PSixOt6ZNKm+w+Hfp/Bciwow== -fs-extra@9.0.1, fs-extra@^9.0.0: +fs-extra@9.0.1, fs-extra@^9.0.0, fs-extra@^9.0.1: version "9.0.1" resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-9.0.1.tgz#910da0062437ba4c39fedd863f1675ccfefcb9fc" integrity sha512-h2iAoN838FqAFJY2/qVpzFXy+EBxfVE220PalAqQLDVsFOHLJrZvut5puAbCdNv6WJk+B8ihI+k0c7JK5erwqQ==