From 9753c0ea74ea5b2373d219029a39929dc89fdda1 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Mon, 19 Jul 2021 11:42:19 +0200 Subject: [PATCH 1/6] =?UTF-8?q?fix:=20Don=E2=80=99t=20leak=20`STRIP=5FTRAN?= =?UTF-8?q?SIENTS`=20action=20(#961)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(master): Don’t crash master if action or payload missing Return error instead of crashing when the master receives an undefined action or an action without a `payload` field. * fix(client): Don’t forward `STRIP_TRANSIENTS` action to transports --- src/client/client.test.ts | 15 ++++++++++++++- src/client/client.ts | 10 ++++++++-- src/master/master.test.ts | 12 ++++++++++++ src/master/master.ts | 4 ++++ src/types.ts | 6 ++++-- 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/client/client.test.ts b/src/client/client.test.ts index 7c5b54e79..62c8125c8 100644 --- a/src/client/client.test.ts +++ b/src/client/client.test.ts @@ -22,6 +22,7 @@ import { gameEvent, patch, } from '../core/action-creators'; +import * as Actions from '../core/action-types'; import Debug from './debug/Debug.svelte'; import { error } from '../core/logger'; import type { LogEntry, State, SyncInfo } from '../types'; @@ -144,7 +145,7 @@ describe('multiplayer', () => { beforeAll(() => { client = Client({ - game: { moves: { A: () => {} } }, + game: { moves: { A: () => {}, Invalid: () => INVALID_MOVE } }, multiplayer: SocketIO({ server: host + ':' + port }), }); client.start(); @@ -173,6 +174,18 @@ describe('multiplayer', () => { expect(client.transport.onAction).toHaveBeenCalled(); }); + test('strip transients action not sent to transport', () => { + jest.spyOn(client.transport, 'onAction'); + const state = { G: {}, ctx: { phase: '' }, plugins: {} }; + const filteredMetadata = []; + client.store.dispatch(sync({ state, filteredMetadata } as SyncInfo)); + client.moves.Invalid(); + expect(client.transport.onAction).not.toHaveBeenCalledWith( + expect.any(Object), + { type: Actions.STRIP_TRANSIENTS } + ); + }); + test('Sends and receives chat messages', () => { jest.spyOn(client.transport, 'onAction'); client.updatePlayerID('0'); diff --git a/src/client/client.ts b/src/client/client.ts index 5f0726544..028ff665d 100644 --- a/src/client/client.ts +++ b/src/client/client.ts @@ -43,7 +43,10 @@ type ClientAction = | ActionShape.Sync | ActionShape.Update | ActionShape.Patch; -type Action = CredentialedActionShape.Any | ClientAction; +type Action = + | CredentialedActionShape.Any + | ActionShape.StripTransients + | ClientAction; export interface DebugOpt { target?: HTMLElement; @@ -287,7 +290,10 @@ export class _ClientImpl { const baseState = store.getState(); const result = next(action); - if (!('clientOnly' in action)) { + if ( + !('clientOnly' in action) && + action.type !== Actions.STRIP_TRANSIENTS + ) { this.transport.onAction(baseState, action); } diff --git a/src/master/master.test.ts b/src/master/master.test.ts index fb2021bdf..95c88e3dd 100644 --- a/src/master/master.test.ts +++ b/src/master/master.test.ts @@ -233,6 +233,18 @@ describe('update', () => { ]); }); + test('missing action', async () => { + const { error } = await master.onUpdate(null, 0, 'matchID', '0'); + expect(sendAll).not.toHaveBeenCalled(); + expect(error).toBe('missing action or action payload'); + }); + + test('missing action payload', async () => { + const { error } = await master.onUpdate({}, 0, 'matchID', '0'); + expect(sendAll).not.toHaveBeenCalled(); + expect(error).toBe('missing action or action payload'); + }); + test('invalid matchID', async () => { await master.onUpdate(action, 0, 'default:unknown', '1'); expect(sendAll).not.toHaveBeenCalled(); diff --git a/src/master/master.ts b/src/master/master.ts index ddb5960f2..c1a692c6a 100644 --- a/src/master/master.ts +++ b/src/master/master.ts @@ -161,6 +161,10 @@ export class Master { matchID: string, playerID: string ): Promise { + if (!credAction || !credAction.payload) { + return { error: 'missing action or action payload' }; + } + let metadata: Server.MatchData | undefined; if (StorageAPI.isSynchronous(this.storageAPI)) { ({ metadata } = this.storageAPI.fetch(matchID, { metadata: true })); diff --git a/src/types.ts b/src/types.ts index 7c0b306e5..85257e213 100644 --- a/src/types.ts +++ b/src/types.ts @@ -431,7 +431,9 @@ export namespace ActionShape { export type Redo = StripCredentials; // Private type used only for internal error processing. // Included here to preserve type-checking of reducer inputs. - type _StripTransients = ReturnType; + export type StripTransients = ReturnType< + typeof ActionCreators.stripTransients + >; export type Any = | MakeMove | GameEvent @@ -443,7 +445,7 @@ export namespace ActionShape { | Undo | Redo | Plugin - | _StripTransients; + | StripTransients; } export namespace ActionPayload { From b52aaea6bf1ff88f0bf2b1883db296725f2d709b Mon Sep 17 00:00:00 2001 From: delucis Date: Mon, 19 Jul 2021 11:51:51 +0200 Subject: [PATCH 2/6] chore(deps): npm audit fix --- package-lock.json | 77 +++++++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 29 deletions(-) diff --git a/package-lock.json b/package-lock.json index b38124c36..1142e47fb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -91,9 +91,9 @@ "optional": true }, "glob-parent": { - "version": "5.1.1", - "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.1.tgz", - "integrity": "sha512-FnI+VGOpnlGHWZxthPGR+QhR78fuiK0sNLkHQv+bL9fQi57lNNdquIbna/WrfROrolq8GK5Ek6BiMwqL/voRYQ==", + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz", + "integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==", "dev": true, "optional": true, "requires": { @@ -5024,12 +5024,6 @@ "integrity": "sha512-z/WhQ5FPySLdvREByI2vZiTWwCnF0moMJ1hK9YQwDTHKh6I7/uSckMetoRGb5UBZPC1z0jlw+n/XCgjeH7y1AQ==", "dev": true }, - "async-limiter": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/async-limiter/-/async-limiter-1.0.1.tgz", - "integrity": "sha512-csOlWGAcRFJaI6m+F2WKdnMKr4HhdhFVBk0H/QbJFMCr+uO2kwohwXQPxw/9OCxp05r5ghVBFSyioixx3gfkNQ==", - "dev": true - }, "asynckit": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", @@ -7575,6 +7569,11 @@ "version": "2.1.2", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" + }, + "ws": { + "version": "7.4.6", + "resolved": "https://registry.npmjs.org/ws/-/ws-7.4.6.tgz", + "integrity": "sha512-YmhHDO4MzaDLB+M9ym/mDA5z0naX8j7SIlT8f8z+I0VtzsRbekxEutHSme7NPS2qE8StCYQNUnfWdXta/Yu85A==" } } }, @@ -7611,6 +7610,11 @@ "version": "2.1.2", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" + }, + "ws": { + "version": "7.4.6", + "resolved": "https://registry.npmjs.org/ws/-/ws-7.4.6.tgz", + "integrity": "sha512-YmhHDO4MzaDLB+M9ym/mDA5z0naX8j7SIlT8f8z+I0VtzsRbekxEutHSme7NPS2qE8StCYQNUnfWdXta/Yu85A==" } } }, @@ -7905,9 +7909,9 @@ "dev": true }, "glob-parent": { - "version": "5.1.1", - "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.1.tgz", - "integrity": "sha512-FnI+VGOpnlGHWZxthPGR+QhR78fuiK0sNLkHQv+bL9fQi57lNNdquIbna/WrfROrolq8GK5Ek6BiMwqL/voRYQ==", + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz", + "integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==", "dev": true, "requires": { "is-glob": "^4.0.1" @@ -8672,9 +8676,9 @@ } }, "glob-parent": { - "version": "5.1.1", - "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.1.tgz", - "integrity": "sha512-FnI+VGOpnlGHWZxthPGR+QhR78fuiK0sNLkHQv+bL9fQi57lNNdquIbna/WrfROrolq8GK5Ek6BiMwqL/voRYQ==", + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz", + "integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==", "dev": true, "requires": { "is-glob": "^4.0.1" @@ -10896,8 +10900,7 @@ }, "yargs-parser": { "version": "13.1.1", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-13.1.1.tgz", - "integrity": "sha512-oVAVsHz6uFrg3XQheFII8ESO2ssAf9luWuAd6Wexsu4F3OtIW0o8IribPXYrD4WC24LWtPrJlGy87y5udK+dxQ==", + "resolved": "", "dev": true, "requires": { "camelcase": "^5.0.0", @@ -11099,9 +11102,9 @@ } }, "ws": { - "version": "7.3.1", - "resolved": "https://registry.npmjs.org/ws/-/ws-7.3.1.tgz", - "integrity": "sha512-D3RuNkynyHmEJIpD2qrgVkc9DQ23OrN/moAwZX4L8DfvszsJxpjQuUq3LMx6HoYji9fbIOBY18XWBsAux1ZZUA==", + "version": "7.5.3", + "resolved": "https://registry.npmjs.org/ws/-/ws-7.5.3.tgz", + "integrity": "sha512-kQ/dHIzuLrS6Je9+uv81ueZomEwH0qVYstcAQ4/Z93K8zeko9gtAbttJWzoC5ukqXY1PpoouV3+VSOqEAFt5wg==", "dev": true } } @@ -11656,12 +11659,20 @@ "dev": true }, "ws": { - "version": "5.2.2", - "resolved": "https://registry.npmjs.org/ws/-/ws-5.2.2.tgz", - "integrity": "sha512-jaHFD6PFv6UgoIVda6qZllptQsMlDEJkTQcybzzXDYM1XO9Y8em691FGMPmM46WGyLU4z9KMgQN+qrux/nhlHA==", + "version": "5.2.3", + "resolved": "https://registry.npmjs.org/ws/-/ws-5.2.3.tgz", + "integrity": "sha512-jZArVERrMsKUatIdnLzqvcfydI85dvd/Fp1u/VOpfdDWQ4c9qWXe+VIeAbQ5FrDwciAkr+lzofXLz3Kuf26AOA==", "dev": true, "requires": { "async-limiter": "~1.0.0" + }, + "dependencies": { + "async-limiter": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/async-limiter/-/async-limiter-1.0.1.tgz", + "integrity": "sha512-csOlWGAcRFJaI6m+F2WKdnMKr4HhdhFVBk0H/QbJFMCr+uO2kwohwXQPxw/9OCxp05r5ghVBFSyioixx3gfkNQ==", + "dev": true + } } } } @@ -15631,6 +15642,13 @@ "debug": "~4.3.1", "engine.io-parser": "~4.0.0", "ws": "~7.4.2" + }, + "dependencies": { + "ws": { + "version": "7.4.6", + "resolved": "https://registry.npmjs.org/ws/-/ws-7.4.6.tgz", + "integrity": "sha512-YmhHDO4MzaDLB+M9ym/mDA5z0naX8j7SIlT8f8z+I0VtzsRbekxEutHSme7NPS2qE8StCYQNUnfWdXta/Yu85A==" + } } }, "ms": { @@ -17406,9 +17424,10 @@ } }, "ws": { - "version": "7.4.6", - "resolved": "https://registry.npmjs.org/ws/-/ws-7.4.6.tgz", - "integrity": "sha512-YmhHDO4MzaDLB+M9ym/mDA5z0naX8j7SIlT8f8z+I0VtzsRbekxEutHSme7NPS2qE8StCYQNUnfWdXta/Yu85A==" + "version": "7.5.3", + "resolved": "https://registry.npmjs.org/ws/-/ws-7.5.3.tgz", + "integrity": "sha512-kQ/dHIzuLrS6Je9+uv81ueZomEwH0qVYstcAQ4/Z93K8zeko9gtAbttJWzoC5ukqXY1PpoouV3+VSOqEAFt5wg==", + "dev": true }, "xdg-basedir": { "version": "3.0.0", @@ -17541,9 +17560,9 @@ } }, "yargs-parser": { - "version": "15.0.1", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-15.0.1.tgz", - "integrity": "sha512-0OAMV2mAZQrs3FkNpDQcBk1x5HXb8X4twADss4S0Iuk+2dGnLOE/fRHrsYm542GduMveyA77OF4wrNJuanRCWw==", + "version": "15.0.3", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-15.0.3.tgz", + "integrity": "sha512-/MVEVjTXy/cGAjdtQf8dW3V9b97bPN7rNn8ETj6BmAQL7ibC7O1Q9SPJbGjgh3SlwoBNXMzj/ZGIj8mBgl12YA==", "dev": true, "requires": { "camelcase": "^5.0.0", From b9cd7287778f564e09161a7df3c449066f126e81 Mon Sep 17 00:00:00 2001 From: delucis Date: Mon, 19 Jul 2021 11:53:11 +0200 Subject: [PATCH 3/6] 0.45.2 --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1142e47fb..4e151cf6b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "boardgame.io", - "version": "0.45.1", + "version": "0.45.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index f1b78c35e..d752e95e2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "boardgame.io", - "version": "0.45.1", + "version": "0.45.2", "description": "library for turn-based games", "repository": "https://github.com/boardgameio/boardgame.io", "scripts": { From bfc8a1f2701fd14627c99f38122abc294aa7c16e Mon Sep 17 00:00:00 2001 From: delucis Date: Mon, 19 Jul 2021 11:55:14 +0200 Subject: [PATCH 4/6] docs(CHANGELOG): 0.45.2 --- docs/documentation/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/documentation/CHANGELOG.md b/docs/documentation/CHANGELOG.md index 3fcecdd2d..0f57b5881 100644 --- a/docs/documentation/CHANGELOG.md +++ b/docs/documentation/CHANGELOG.md @@ -1,3 +1,9 @@ +### v0.45.2 + +#### Bugfixes + +* [[9753c0e](https://github.com/boardgameio/boardgame.io/commit/9753c0e)] fix: Don’t leak `STRIP_TRANSIENTS` action ([#961](https://github.com/boardgameio/boardgame.io/pull/961)) + ### v0.45.1 #### Breaking Changes From b7530948749c4eac2e77374602964b689e3ed0af Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 20 Jul 2021 09:45:13 +0200 Subject: [PATCH 5/6] chore(deps): bump urijs from 1.19.6 to 1.19.7 (#965) Bumps [urijs](https://github.com/medialize/URI.js) from 1.19.6 to 1.19.7. - [Release notes](https://github.com/medialize/URI.js/releases) - [Changelog](https://github.com/medialize/URI.js/blob/gh-pages/CHANGELOG.md) - [Commits](https://github.com/medialize/URI.js/compare/v1.19.6...v1.19.7) --- updated-dependencies: - dependency-name: urijs dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4e151cf6b..f51b6c889 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10900,7 +10900,8 @@ }, "yargs-parser": { "version": "13.1.1", - "resolved": "", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-13.1.1.tgz", + "integrity": "sha512-oVAVsHz6uFrg3XQheFII8ESO2ssAf9luWuAd6Wexsu4F3OtIW0o8IribPXYrD4WC24LWtPrJlGy87y5udK+dxQ==", "dev": true, "requires": { "camelcase": "^5.0.0", @@ -17174,9 +17175,9 @@ } }, "urijs": { - "version": "1.19.6", - "resolved": "https://registry.npmjs.org/urijs/-/urijs-1.19.6.tgz", - "integrity": "sha512-eSXsXZ2jLvGWeLYlQA3Gh36BcjF+0amo92+wHPyN1mdR8Nxf75fuEuYTd9c0a+m/vhCjRK0ESlE9YNLW+E1VEw==" + "version": "1.19.7", + "resolved": "https://registry.npmjs.org/urijs/-/urijs-1.19.7.tgz", + "integrity": "sha512-Id+IKjdU0Hx+7Zx717jwLPsPeUqz7rAtuVBRLLs+qn+J2nf9NGITWVCxcijgYxBqe83C7sqsQPs6H1pyz3x9gA==" }, "urix": { "version": "0.1.0", From afee0b7b1f0d878118c960637fa0e1d90b848ce9 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Wed, 21 Jul 2021 09:39:55 +0200 Subject: [PATCH 6/6] feat: Allow plugins to declare an action invalid (#963) --- docs/documentation/plugins.md | 6 +++ src/core/errors.ts | 2 + src/core/reducer.test.ts | 90 +++++++++++++++++++++++++++++++++++ src/core/reducer.ts | 43 +++++++++++++++-- src/plugins/main.test.ts | 64 +++++++++++++++++++++++++ src/plugins/main.ts | 28 +++++++++++ src/types.ts | 1 + 7 files changed, 230 insertions(+), 4 deletions(-) diff --git a/docs/documentation/plugins.md b/docs/documentation/plugins.md index a45445277..3d03acdb2 100644 --- a/docs/documentation/plugins.md +++ b/docs/documentation/plugins.md @@ -47,6 +47,12 @@ A plugin is an object that contains the following fields. // for the master instead. noClient: ({ G, ctx, game, data, api }) => boolean, + // Function that allows the plugin to indicate that the + // current action should be declared invalid and cancelled. + // If `isInvalid` returns an error message, the whole update + // will be abandoned and an error returned to the client. + isInvalid: ({ G, ctx, game, data, api }) => false | string, + // Function that can filter `data` to hide secret state // before sending it to a specific client. // `playerID` could also be null or undefined for spectators. diff --git a/src/core/errors.ts b/src/core/errors.ts index 267c8621d..ad2fbb900 100644 --- a/src/core/errors.ts +++ b/src/core/errors.ts @@ -30,4 +30,6 @@ export enum ActionErrorType { ActionDisabled = 'action/action_disabled', // The requested action is not currently possible ActionInvalid = 'action/action_invalid', + // The requested action was declared invalid by a plugin + PluginActionInvalid = 'action/plugin_invalid', } diff --git a/src/core/reducer.test.ts b/src/core/reducer.test.ts index 0d2c83917..f6ff82ea4 100644 --- a/src/core/reducer.test.ts +++ b/src/core/reducer.test.ts @@ -299,6 +299,96 @@ describe('Events API', () => { }); }); +describe('Plugin Invalid Action API', () => { + const pluginName = 'validator'; + const message = 'G.value must divide by 5'; + const game: Game<{ value: number }> = { + setup: () => ({ value: 5 }), + plugins: [ + { + name: pluginName, + isInvalid: ({ G }) => { + if (G.value % 5 !== 0) return message; + return false; + }, + }, + ], + moves: { + setValue: (G, _ctx, arg) => { + G.value = arg; + }, + }, + phases: { + unenterable: { + onBegin: () => ({ value: 13 }), + }, + enterable: { + onBegin: () => ({ value: 25 }), + }, + }, + }; + + let state: State; + beforeEach(() => { + state = InitializeGame({ game }); + }); + + describe('multiplayer client', () => { + const reducer = CreateGameReducer({ game }); + + test('move is cancelled if plugin declares it invalid', () => { + state = reducer(state, makeMove('setValue', [6], '0')); + expect(state.G).toMatchObject({ value: 5 }); + expect(state['transients'].error).toEqual({ + type: 'action/plugin_invalid', + payload: { plugin: pluginName, message }, + }); + }); + + test('move is processed if no plugin declares it invalid', () => { + state = reducer(state, makeMove('setValue', [15], '0')); + expect(state.G).toMatchObject({ value: 15 }); + expect(state['transients']).toBeUndefined(); + }); + + test('event is cancelled if plugin declares it invalid', () => { + state = reducer(state, gameEvent('setPhase', 'unenterable', '0')); + expect(state.G).toMatchObject({ value: 5 }); + expect(state.ctx.phase).toBe(null); + expect(state['transients'].error).toEqual({ + type: 'action/plugin_invalid', + payload: { plugin: pluginName, message }, + }); + }); + + test('event is processed if no plugin declares it invalid', () => { + state = reducer(state, gameEvent('setPhase', 'enterable', '0')); + expect(state.G).toMatchObject({ value: 25 }); + expect(state.ctx.phase).toBe('enterable'); + expect(state['transients']).toBeUndefined(); + }); + }); + + describe('local client', () => { + const reducer = CreateGameReducer({ game, isClient: true }); + + test('move is cancelled if plugin declares it invalid', () => { + state = reducer(state, makeMove('setValue', [6], '0')); + expect(state.G).toMatchObject({ value: 5 }); + expect(state['transients'].error).toEqual({ + type: 'action/plugin_invalid', + payload: { plugin: pluginName, message }, + }); + }); + + test('move is processed if no plugin declares it invalid', () => { + state = reducer(state, makeMove('setValue', [15], '0')); + expect(state.G).toMatchObject({ value: 15 }); + expect(state['transients']).toBeUndefined(); + }); + }); +}); + describe('Random inside setup()', () => { const game1: Game = { seed: 'seed1', diff --git a/src/core/reducer.ts b/src/core/reducer.ts index beba12b13..4f5141703 100644 --- a/src/core/reducer.ts +++ b/src/core/reducer.ts @@ -128,6 +128,30 @@ function initializeDeltalog( }; } +/** + * Update plugin state after move/event & check if plugins consider the action to be valid. + * @param newState Latest version of state in the reducer. + * @param oldState Initial value of state when reducer started its work. + * @param pluginOpts Plugin configuration options. + * @returns Tuple of the new state updated after flushing plugins and the old + * state augmented with an error if a plugin declared the action invalid. + */ +function flushAndValidatePlugins( + newState: State, + oldState: State, + pluginOpts: { game: Game; isClient?: boolean } +): [State, TransientState?] { + newState = plugins.Flush(newState, pluginOpts); + const isInvalid = plugins.IsInvalid(newState, pluginOpts); + if (!isInvalid) return [newState]; + const { plugin, message } = isInvalid; + error(`plugin declared action invalid: ${plugin} - ${message}`); + return [ + newState, + WithError(oldState, ActionErrorType.PluginActionInvalid, isInvalid), + ]; +} + /** * ExtractTransientsFromState * @@ -271,7 +295,12 @@ export function CreateGameReducer({ let newState = game.flow.processEvent(state, action); // Execute plugins. - newState = plugins.Flush(newState, { game, isClient: false }); + let stateWithError: TransientState | undefined; + [newState, stateWithError] = flushAndValidatePlugins(newState, state, { + game, + isClient: false, + }); + if (stateWithError) return stateWithError; // Update undo / redo state. newState = updateUndoRedoState(newState, { game, action }); @@ -280,7 +309,7 @@ export function CreateGameReducer({ } case Actions.MAKE_MOVE: { - state = { ...state, deltalog: [] }; + const oldState = (state = { ...state, deltalog: [] }); // Check whether the move is allowed at this time. const move: Move = game.flow.getMove( @@ -348,10 +377,12 @@ export function CreateGameReducer({ // These will be processed on the server, which // will send back a state update. if (isClient) { - state = plugins.Flush(state, { + let stateWithError: TransientState | undefined; + [state, stateWithError] = flushAndValidatePlugins(state, oldState, { game, isClient: true, }); + if (stateWithError) return stateWithError; return { ...state, _stateID: state._stateID + 1, @@ -363,7 +394,11 @@ export function CreateGameReducer({ // Allow the flow reducer to process any triggers that happen after moves. state = game.flow.processMove(state, action.payload); - state = plugins.Flush(state, { game }); + let stateWithError: TransientState | undefined; + [state, stateWithError] = flushAndValidatePlugins(state, oldState, { + game, + }); + if (stateWithError) return stateWithError; // Update undo / redo state. state = updateUndoRedoState(state, { game, action }); diff --git a/src/plugins/main.test.ts b/src/plugins/main.test.ts index 94a706278..bf3ca9fa9 100644 --- a/src/plugins/main.test.ts +++ b/src/plugins/main.test.ts @@ -8,6 +8,7 @@ import { Client } from '../client/client'; import { Local } from '../client/transport/local'; +import type { Game } from '../types'; describe('basic', () => { let client: ReturnType; @@ -134,6 +135,69 @@ describe('default values', () => { }); }); +describe('isInvalid method', () => { + // Silence expected error logging and restore when finished. + const stderr = console.error; + beforeAll(() => (console.error = () => {})); + afterAll(() => (console.error = stderr)); + + test('basic plugin', () => { + const goodG = { good: 'nice' }; + const game: Game = { + plugins: [ + { + name: 'test', + isInvalid: ({ G }) => 'bad' in G && 'not ok', + }, + ], + moves: { + good: () => goodG, + bad: () => ({ bad: 'not ok' }), + }, + }; + + const client = Client({ game, playerID: '0' }); + client.start(); + client.moves.good(); + expect(client.getState().G).toEqual(goodG); + client.moves.bad(); + expect(client.getState().G).toEqual(goodG); + }); + + test('plugin with API and data', () => { + const game: Game = { + plugins: [ + { + name: 'test', + setup: () => ({}), + api: ({ data }) => ({ + set: (key, val) => { + data[key] = val; + }, + }), + isInvalid: ({ data }) => 'bad' in data && 'not ok', + }, + ], + moves: { + good: (_, ctx) => { + ctx.test.set('good', 'nice'); + }, + bad: (_, ctx) => { + ctx.test.set('bad', 'not ok'); + }, + }, + }; + + const client = Client({ game, playerID: '0' }); + client.start(); + expect(client.getState().ctx.numMoves).toBe(0); + client.moves.good(); + expect(client.getState().ctx.numMoves).toBe(1); + client.moves.bad(); + expect(client.getState().ctx.numMoves).toBe(1); + }); +}); + describe('actions', () => { let client; diff --git a/src/plugins/main.ts b/src/plugins/main.ts index 23bcc967f..420eab9bc 100644 --- a/src/plugins/main.ts +++ b/src/plugins/main.ts @@ -243,6 +243,34 @@ export const NoClient = (state: State, opts: PluginOpts): boolean => { .some((value) => value === true); }; +/** + * Allows plugins to indicate if the entire action should be thrown out + * as invalid. This will cancel the entire state update. + */ +export const IsInvalid = ( + state: State, + opts: PluginOpts +): false | { plugin: string; message: string } => { + const firstInvalidReturn = [...DEFAULT_PLUGINS, ...opts.game.plugins] + .filter((plugin) => plugin.isInvalid !== undefined) + .map((plugin) => { + const { name } = plugin; + const pluginState = state.plugins[name]; + + const message = plugin.isInvalid({ + G: state.G, + ctx: state.ctx, + game: opts.game, + api: pluginState && pluginState.api, + data: pluginState && pluginState.data, + }); + + return message ? { plugin: name, message } : false; + }) + .find((value) => value); + return firstInvalidReturn || false; +}; + /** * Allows plugins to customize their data for specific players. * For example, a plugin may want to share no data with the client, or diff --git a/src/types.ts b/src/types.ts index 85257e213..2efb55dd8 100644 --- a/src/types.ts +++ b/src/types.ts @@ -144,6 +144,7 @@ export interface Plugin< > { name: string; noClient?: (context: PluginContext) => boolean; + isInvalid?: (context: PluginContext) => false | string; setup?: (setupCtx: { G: G; ctx: Ctx; game: Game }) => Data; action?: (data: Data, payload: ActionShape.Plugin['payload']) => Data; api?: (context: {