Repository: boardgameio/boardgame.io
Pull request #955 (2021-07-13 17:49:21)

feat(server): Use `origins` option to configure Lobby API CORS
#946 added an `origins` option to the server, which is used to enable CORS for the socket.io transport.

The Lobby API server currently uses `@koa/cors` with no configuration to reflect the request origin back in the `Access-Control-Allow-Origin` header, enabling requests from any origin:

https://github.com/boardgameio/boardgame.io/blob/2b1d013a03132d9d40c38c0d34b94e91d0a2509a/src/server/api.ts#L451

Given the server now *requires* setting origins, I thought we should probably also lock down the Lobby REST API to those same origins. This PR adopts the same method used by socket.io to validate request origins against the `origins` option for use with the `@koa/cors` middleware.

What do you reckon @vdfdev? Is there a chance the socket server and REST API need to allow access to different origins, which would require a second `apiOrigins` option?

vdfdev: Oh, good catch, I forgot about the API server! I think having the default of both being the same is great, but also think we should allow overwriting the API server cors. In the case of FBG, we don't want to allow anybody to access our API server but our own server, so this option would be good to restrict that
delucis: OK, added that option, setting it to `origins` by default. PTAL.
vdfdev: Looks great, thank you!

Patch:
diff --git a/docs/documentation/api/Server.md b/docs/documentation/api/Server.md
index bada61c..a5ea53c 100644
--- a/docs/documentation/api/Server.md
+++ b/docs/documentation/api/Server.md
@@ -56,6 +56,9 @@ A config object with the following options:
 
 7. `authenticateCredentials` (_function_): an optional function that tests if a player’s move is made with the correct credentials when using the default socket.io transport implementation.
 
+8. `apiOrigins` (_array_): a list of allowed origins for requests to the Lobby API. Defaults
+   to the value provided as the `origins` option (which also applies to the socket transport).
+
 #### Returns
 
 An object that contains:
diff --git a/package-lock.json b/package-lock.json
index a62892a..d769da3 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -3588,9 +3588,9 @@
       }
     },
     "@koa/cors": {
-      "version": "2.2.3",
-      "resolved": "https://registry.npmjs.org/@koa/cors/-/cors-2.2.3.tgz",
-      "integrity": "sha512-tCVVXa39ETsit5kGBtEWWimjLn1sDaeu8+0phgb8kT3GmBDZOykkI3ZO8nMjV2p3MGkJI4K5P+bxR8Ztq0bwsA==",
+      "version": "3.1.0",
+      "resolved": "https://registry.npmjs.org/@koa/cors/-/cors-3.1.0.tgz",
+      "integrity": "sha512-7ulRC1da/rBa6kj6P4g2aJfnET3z8Uf3SWu60cjbtxTA5g8lxRdX/Bd2P92EagGwwAhANeNw8T8if99rJliR6Q==",
       "requires": {
         "vary": "^1.1.2"
       }
@@ -4210,9 +4210,9 @@
       }
     },
     "@types/koa__cors": {
-      "version": "3.0.1",
-      "resolved": "https://registry.npmjs.org/@types/koa__cors/-/koa__cors-3.0.1.tgz",
-      "integrity": "sha512-loqZNXliley8kncc4wrX9KMqLGN6YfiaO3a3VFX+yVkkXJwOrZU4lipdudNjw5mFyC+5hd7h9075hQWcVVpeOg==",
+      "version": "3.0.3",
+      "resolved": "https://registry.npmjs.org/@types/koa__cors/-/koa__cors-3.0.3.tgz",
+      "integrity": "sha512-74Xb4hJOPGKlrQ4PRBk1A/p0gfLpgbnpT0o67OMVbwyeMXvlBN+ZCRztAAmkKZs+8hKbgMutUlZVbA52Hr/0IA==",
       "dev": true,
       "requires": {
         "@types/koa": "*"
diff --git a/package.json b/package.json
index f84e584..8204d09 100644
--- a/package.json
+++ b/package.json
@@ -93,7 +93,7 @@
     "@types/enzyme": "^3.10.5",
     "@types/jest": "^24.0.0",
     "@types/koa-router": "^7.4.0",
-    "@types/koa__cors": "^3.0.1",
+    "@types/koa__cors": "^3.0.3",
     "@types/node": "^14.0.24",
     "@types/react": "^16.9.36",
     "@types/react-dom": "^16.9.8",
@@ -148,7 +148,7 @@
     "typescript": "^3.8.2"
   },
   "dependencies": {
-    "@koa/cors": "^2.2.1",
+    "@koa/cors": "^3.1.0",
     "@types/koa": "^2.11.3",
     "flatted": "^0.2.3",
     "immer": "^8.0.1",
diff --git a/src/server/api.ts b/src/server/api.ts
index 3559aed..2d435c4 100644
--- a/src/server/api.ts
+++ b/src/server/api.ts
@@ -11,6 +11,7 @@ import Router from 'koa-router';
 import koaBody from 'koa-body';
 import { nanoid } from 'nanoid';
 import cors from '@koa/cors';
+import type IOTypes from 'socket.io';
 import { createMatch } from './util';
 import type { Auth } from './auth';
 import type { Server, LobbyAPI, Game, StorageAPI } from '../types';
@@ -446,9 +447,18 @@ export const createRouter = ({
 
 export const configureApp = (
   app: Server.App,
-  router: Router<any, Server.AppCtx>
+  router: Router<any, Server.AppCtx>,
+  origins: IOTypes.ServerOptions['cors']['origin']
 ): void => {
-  app.use(cors());
+  app.use(
+    cors({
+      // Set Access-Control-Allow-Origin header for allowed origins.
+      origin: (ctx) => {
+        const origin = ctx.get('Origin');
+        return isOriginAllowed(origin, origins) ? origin : '';
+      },
+    })
+  );
 
   // If API_SECRET is set, then require that requests set an
   // api-secret header that is set to the same value.
@@ -465,3 +475,30 @@ export const configureApp = (
 
   app.use(router.routes()).use(router.allowedMethods());
 };
+
+/**
+ * Check if a request’s origin header is allowed for CORS.
+ * Adapted from `cors` package: https://github.com/expressjs/cors
+ * @param origin Request origin to test.
+ * @param allowedOrigin Origin(s) that are allowed to connect via CORS.
+ * @returns `true` if the origin matched at least one of the allowed origins.
+ */
+function isOriginAllowed(
+  origin: string,
+  allowedOrigin: IOTypes.ServerOptions['cors']['origin']
+): boolean {
+  if (Array.isArray(allowedOrigin)) {
+    for (const entry of allowedOrigin) {
+      if (isOriginAllowed(origin, entry)) {
+        return true;
+      }
+    }
+    return false;
+  } else if (typeof allowedOrigin === 'string') {
+    return origin === allowedOrigin;
+  } else if (allowedOrigin instanceof RegExp) {
+    return allowedOrigin.test(origin);
+  } else {
+    return !!allowedOrigin;
+  }
+}
diff --git a/src/server/index.ts b/src/server/index.ts
index 7b385a9..539ac73 100644
--- a/src/server/index.ts
+++ b/src/server/index.ts
@@ -59,6 +59,7 @@ export const getPortFromServer = (
 interface ServerOpts {
   games: Game[];
   origins?: IOTypes.ServerOptions['cors']['origin'];
+  apiOrigins?: IOTypes.ServerOptions['cors']['origin'];
   db?: StorageAPI.Async | StorageAPI.Sync;
   transport?: SocketIO;
   uuid?: () => string;
@@ -74,7 +75,8 @@ interface ServerOpts {
  * @param db - The interface with the database.
  * @param transport - The interface with the clients.
  * @param authenticateCredentials - Function to test player credentials.
- * @param origins - Allowed origins to use this server, i.e. [http://localhost:300]
+ * @param origins - Allowed origins to use this server, e.g. `['http://localhost:3000']`.
+ * @param apiOrigins - Allowed origins to use the Lobby API, defaults to `origins`.
  * @param generateCredentials - Method for API to generate player credentials.
  * @param https - HTTPS configuration options passed through to the TLS module.
  * @param lobbyConfig - Configuration options for the Lobby API server.
@@ -86,6 +88,7 @@ export function Server({
   https,
   uuid,
   origins,
+  apiOrigins = origins,
   generateCredentials = uuid,
   authenticateCredentials,
 }: ServerOpts) {
@@ -133,13 +136,13 @@ export function Server({
       const lobbyConfig = serverRunConfig.lobbyConfig;
       let apiServer: KoaServer | undefined;
       if (!lobbyConfig || !lobbyConfig.apiPort) {
-        configureApp(app, router);
+        configureApp(app, router, apiOrigins);
       } else {
         // Run API in a separate Koa app.
         const api: ServerTypes.App = new Koa();
         api.context.db = db;
         api.context.auth = auth;
-        configureApp(api, router);
+        configureApp(api, router, apiOrigins);
         await new Promise((resolve) => {
           apiServer = api.listen(lobbyConfig.apiPort, resolve);
         });


Test patch:
diff --git a/src/server/api.test.ts b/src/server/api.test.ts
index 5f10e60..ab047df 100644
--- a/src/server/api.test.ts
+++ b/src/server/api.test.ts
@@ -14,6 +14,7 @@ import { createRouter, configureApp } from './api';
 import { ProcessGameConfig } from '../core/game';
 import { Auth } from './auth';
 import * as StorageAPI from './db/base';
+import { Origins } from './cors';
 import type { Game, Server } from '../types';
 
 jest.setTimeout(2000000000);
@@ -72,13 +73,19 @@ class AsyncStorage extends StorageAPI.Async {
 describe('.createRouter', () => {
   function addApiToServer({
     app,
+    origins,
     ...args
-  }: { app: Server.App } & Parameters<typeof createRouter>[0]) {
+  }: {
+    app: Server.App;
+    origins?: Parameters<typeof configureApp>[2];
+  } & Parameters<typeof createRouter>[0]) {
     const router = createRouter(args);
-    configureApp(app, router);
+    configureApp(app, router, origins);
   }
 
-  function createApiServer(args: Parameters<typeof createRouter>[0]) {
+  function createApiServer(
+    args: Omit<Parameters<typeof addApiToServer>[0], 'app'>
+  ) {
     const app: Server.App = new Koa();
     addApiToServer({ app, ...args });
     return app;
@@ -1511,4 +1518,70 @@ describe('.createRouter', () => {
       expect(server.use.mock.calls.length).toBeGreaterThan(1);
     });
   });
+
+  describe('cors', () => {
+    const auth = new Auth();
+    const games: Game[] = [];
+    const db = new AsyncStorage();
+
+    describe('no allowed origins', () => {
+      const app = createApiServer({ auth, games, db, origins: false });
+
+      test('does not allow CORS', async () => {
+        const { res } = await request(app.callback())
+          .get('/games')
+          .set('Origin', 'https://www.example.com')
+          .expect('Vary', 'Origin');
+        expect(res.headers).not.toHaveProperty('access-control-allow-origin');
+        expect(res.headers).not.toHaveProperty('Access-Control-Allow-Origin');
+      });
+    });
+
+    describe('single allowed origin', () => {
+      const origin = 'https://www.example.com';
+      const app = createApiServer({ auth, games, db, origins: origin });
+
+      test('disallows non-matching origin', async () => {
+        const { res } = await request(app.callback())
+          .get('/games')
+          .set('Origin', 'https://www.other.com')
+          .expect('Vary', 'Origin');
+        expect(res.headers).not.toHaveProperty('access-control-allow-origin');
+        expect(res.headers).not.toHaveProperty('Access-Control-Allow-Origin');
+      });
+
+      // eslint-disable-next-line jest/expect-expect
+      test('allows matching origin', async () => {
+        await request(app.callback())
+          .get('/games')
+          .set('Origin', origin)
+          .expect('Vary', 'Origin')
+          .expect('Access-Control-Allow-Origin', origin);
+      });
+    });
+
+    describe('multiple allowed origins', () => {
+      const origins = [Origins.LOCALHOST, 'https://www.example.com'];
+      const app = createApiServer({ auth, games, db, origins });
+
+      test('disallows non-matching origin', async () => {
+        const { res } = await request(app.callback())
+          .get('/games')
+          .set('Origin', 'https://www.other.com')
+          .expect('Vary', 'Origin');
+        expect(res.headers).not.toHaveProperty('access-control-allow-origin');
+        expect(res.headers).not.toHaveProperty('Access-Control-Allow-Origin');
+      });
+
+      // eslint-disable-next-line jest/expect-expect
+      test('allows matching origin', async () => {
+        const origin = 'http://localhost:5000';
+        await request(app.callback())
+          .get('/games')
+          .set('Origin', origin)
+          .expect('Vary', 'Origin')
+          .expect('Access-Control-Allow-Origin', origin);
+      });
+    });
+  });
 });