feat: Add authentication to chat messages (#879)

* Add authentication to chat messages

* refactor: Improve typing of chat message methods in transports

* feat: Only allow players to post chat messages

* refactor: Always await metadata in `Master#onChatMessage`

Metadata is only used for authentication, which always runs 
asynchronously in any case. If the storage API actually is synchronous, 
`await` will have no effect.

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

Author: tomasvidhall

src/client/transport/local.test.ts

@@ -15,7 +15,7 @@ import { InitializeGame } from '../../core/initialize';
 import { Client } from '../client';
 import { RandomBot } from '../../ai/random-bot';
 import { Stage } from '../../core/turn-order';
-import type { State, Store, SyncInfo } from '../../types';
+import type { ChatMessage, State, Store, SyncInfo } from '../../types';
 
 jest.useFakeTimers();
 
@@ -361,10 +361,13 @@ describe('LocalTransport', () => {
     });
 
     test('send chat-message', () => {
-      m.onChatMessage('matchID', { message: 'foo' });
-      expect(m.master.onChatMessage).lastCalledWith('matchID', {
-        message: 'foo',
-      });
+      const msg: ChatMessage = {
+        id: '0',
+        sender: '0',
+        payload: { message: 'foo' },
+      };
+      m.onChatMessage('matchID', msg);
+      expect(m.master.onChatMessage).lastCalledWith('matchID', msg, undefined);
     });
   });
 });

src/client/transport/local.ts

@@ -14,6 +14,7 @@ import type { TransportAPI, TransportData } from '../../master/master';
 import { Transport } from './transport';
 import type { TransportOpts, ChatCallback } from './transport';
 import type {
+  ChatMessage,
   CredentialedActionShape,
   Game,
   LogEntry,
@@ -161,8 +162,18 @@ export class LocalTransport extends Transport {
     this.isConnected = true;
   }
 
-  onChatMessage(matchID, chatMessage) {
-    this.master.onChatMessage(matchID, chatMessage);
+  /**
+   * Called when any player sends a chat message and the
+   * master broadcasts the update to other clients (including
+   * this one).
+   */
+  onChatMessage(matchID: string, chatMessage: ChatMessage) {
+    const args: Parameters<Master['onChatMessage']> = [
+      matchID,
+      chatMessage,
+      this.credentials,
+    ];
+    this.master.onChatMessage(...args);
   }
 
   /**

src/client/transport/socketio.test.ts

@@ -13,7 +13,7 @@ import { CreateGameReducer } from '../../core/reducer';
 import { InitializeGame } from '../../core/initialize';
 import * as Actions from '../../core/action-types';
 import type { Master } from '../../master/master';
-import type { State, Store } from '../../types';
+import type { ChatMessage, State, Store } from '../../types';
 
 type UpdateArgs = Parameters<Master['onUpdate']>;
 type SyncArgs = Parameters<Master['onSync']>;
@@ -204,10 +204,18 @@ describe('multiplayer', () => {
   });
 
   test('send chat-message', () => {
-    m.onChatMessage('matchID', { message: 'foo' });
-    expect(mockSocket.emit).lastCalledWith('chat', 'matchID', {
-      message: 'foo',
-    });
+    const message: ChatMessage = {
+      id: '0',
+      sender: '0',
+      payload: { message: 'foo' },
+    };
+    m.onChatMessage('matchID', message);
+    expect(mockSocket.emit).lastCalledWith(
+      'chat',
+      'matchID',
+      message,
+      m.getCredentials()
+    );
   });
 });
 

src/client/transport/socketio.ts

@@ -96,8 +96,13 @@ export class SocketIOTransport extends Transport {
     this.socket.emit('update', ...args);
   }
 
-  onChatMessage(matchID, chatMessage) {
-    this.socket.emit('chat', matchID, chatMessage);
+  onChatMessage(matchID: string, chatMessage: ChatMessage) {
+    const args: Parameters<Master['onChatMessage']> = [
+      matchID,
+      chatMessage,
+      this.credentials,
+    ];
+    this.socket.emit('chat', ...args);
   }
 
   /**

src/master/master.test.ts

@@ -808,6 +808,54 @@ describe('authentication', () => {
       expect(sendAll).not.toHaveBeenCalled();
     });
   });
+
+  describe('onChatMessage', () => {
+    const chatMessage = {
+      id: 'uuid',
+      payload: { message: 'foo' },
+      sender: '0',
+    };
+
+    beforeEach(resetTestEnvironment);
+
+    test('auth failure', async () => {
+      const authenticateCredentials = () => false;
+      const master = new Master(
+        game,
+        storage,
+        TransportAPI(send, sendAll),
+        new Auth({ authenticateCredentials })
+      );
+      const ret = await master.onChatMessage(matchID, chatMessage, undefined);
+      expect(ret && ret.error).toBe('unauthorized');
+      expect(sendAll).not.toHaveBeenCalled();
+    });
+
+    test('auth success', async () => {
+      const authenticateCredentials = () => true;
+      const master = new Master(
+        game,
+        storage,
+        TransportAPI(send, sendAll),
+        new Auth({ authenticateCredentials })
+      );
+      const ret = await master.onChatMessage(matchID, chatMessage, undefined);
+      expect(ret).toBeUndefined();
+      expect(sendAll).toHaveBeenCalled();
+    });
+
+    test('default', async () => {
+      const master = new Master(
+        game,
+        storage,
+        TransportAPI(send, sendAll),
+        new Auth()
+      );
+      const ret = await master.onChatMessage(matchID, chatMessage, undefined);
+      expect(ret).toBeUndefined();
+      expect(sendAll).toHaveBeenCalled();
+    });
+  });
 });
 
 describe('redactLog', () => {
@@ -982,10 +1030,17 @@ describe('chat', () => {
   });
 
   test('Sends chat messages to all', async () => {
-    master.onChatMessage('matchID', { message: 'foo' });
+    master.onChatMessage(
+      'matchID',
+      { id: 'uuid', sender: '0', payload: { message: 'foo' } },
+      undefined
+    );
     expect(sendAllReturn('0')).toEqual({
       type: 'chat',
-      args: ['matchID', { message: 'foo' }],
+      args: [
+        'matchID',
+        { id: 'uuid', sender: '0', payload: { message: 'foo' } },
+      ],
     });
   });
 });

src/master/master.ts

@@ -459,7 +459,30 @@ export class Master {
     }
   }
 
-  async onChatMessage(matchID, chatMessage) {
+  async onChatMessage(
+    matchID: string,
+    chatMessage: ChatMessage,
+    credentials: string | undefined
+  ): Promise<void | { error: string }> {
+    const key = matchID;
+
+    if (this.auth) {
+      const { metadata } = await (this.storageAPI as StorageAPI.Async).fetch(
+        key,
+        {
+          metadata: true,
+        }
+      );
+      const isAuthentic = await this.auth.authenticateCredentials({
+        playerID: chatMessage.sender,
+        credentials,
+        metadata,
+      });
+      if (!isAuthentic) {
+        return { error: 'unauthorized' };
+      }
+    }
+
     this.transportAPI.sendAll(() => ({
       type: 'chat',
       args: [matchID, chatMessage],

src/server/transport/socketio.ts

@@ -185,15 +185,19 @@ export class SocketIO {
             );
           }
         });
-        socket.on('chat', async (matchID, chatMessage) => {
-          const master = new Master(
-            game,
-            app.context.db,
-            TransportAPI(matchID, socket, this.clientInfo, this.roomInfo),
-            app.context.auth
-          );
-          master.onChatMessage(matchID, chatMessage);
-        });
+        socket.on(
+          'chat',
+          async (...args: Parameters<Master['onChatMessage']>) => {
+            const [matchID] = args;
+            const master = new Master(
+              game,
+              app.context.db,
+              TransportAPI(matchID, socket, this.clientInfo, this.roomInfo),
+              app.context.auth
+            );
+            master.onChatMessage(...args);
+          }
+        );
       });
     }
   }