From cd4979c3bef5e173d91059727aeccbdfbe04d967 Mon Sep 17 00:00:00 2001 From: Jerko Steiner Date: Fri, 13 Mar 2020 13:33:51 +0100 Subject: [PATCH] Generate userIDs on server-side We don't want to depend on: 1) socket.io generated IDs because they change on server reconnect 2) simple-peer generated IDs because they change for every peer connection We generate a single ID when the call web page is refreshed and use that throughout the session (until page refresh). We keep relations of user-id to socket-id on the server side in memory and use that to get to the right socket. In the future this might be replaced with Redis to allow multiple nodes. If the server is restarted, but people have active calls, we want them to keep using the active peer connections and only connect to new peers. Ideally, we do not want to disturb the active peer connections, but peer connections might be restarted because the in-memory store will not have the information on for any peers in the room upon restart. --- src/client/__mocks__/window.ts | 2 + src/client/actions/CallActions.test.ts | 3 +- src/client/actions/CallActions.ts | 3 +- src/client/actions/PeerActions.test.ts | 10 ++-- src/client/actions/PeerActions.ts | 2 +- src/client/actions/SocketActions.test.ts | 60 ++++++++++++++-------- src/client/actions/SocketActions.ts | 39 +++++++------- src/client/components/App.tsx | 6 --- src/client/components/Video.test.tsx | 4 -- src/client/components/Video.tsx | 5 +- src/client/window.ts | 1 + src/server/app.test.ts | 7 ++- src/server/app.ts | 4 +- src/server/routes/call.ts | 1 + src/server/socket.test.ts | 65 ++++++++++++++++++------ src/server/socket.ts | 59 ++++++++++++++------- src/server/store/index.ts | 2 + src/server/store/memory.ts | 17 +++++++ src/server/store/store.ts | 5 ++ src/shared/SocketEvent.ts | 12 +++-- views/call.html | 1 + 21 files changed, 210 insertions(+), 98 deletions(-) create mode 100644 src/server/store/index.ts create mode 100644 src/server/store/memory.ts create mode 100644 src/server/store/store.ts diff --git a/src/client/__mocks__/window.ts b/src/client/__mocks__/window.ts index a17aaaa..7dc2d65 100644 --- a/src/client/__mocks__/window.ts +++ b/src/client/__mocks__/window.ts @@ -42,4 +42,6 @@ export const valueOf = jest.fn() export const callId = 'call1234' +export const userId = 'user1234' + export const iceServers = [] diff --git a/src/client/actions/CallActions.test.ts b/src/client/actions/CallActions.test.ts index de6aeb6..f4b72f4 100644 --- a/src/client/actions/CallActions.test.ts +++ b/src/client/actions/CallActions.test.ts @@ -6,7 +6,7 @@ import * as CallActions from './CallActions' import * as SocketActions from './SocketActions' import * as constants from '../constants' import socket from '../socket' -import { callId } from '../window' +import { callId, userId } from '../window' import { bindActionCreators, createStore, AnyAction, combineReducers, applyMiddleware } from 'redux' import reducers from '../reducers' import { middlewares } from '../middlewares' @@ -60,6 +60,7 @@ describe('CallActions', () => { expect((SocketActions.handshake as jest.Mock).mock.calls).toEqual([[{ socket, roomName: callId, + userId: userId, }]]) }) diff --git a/src/client/actions/CallActions.ts b/src/client/actions/CallActions.ts index 6fd0da0..7ab6e06 100644 --- a/src/client/actions/CallActions.ts +++ b/src/client/actions/CallActions.ts @@ -1,6 +1,6 @@ import socket from '../socket' import { ThunkResult } from '../store' -import { callId } from '../window' +import { callId, userId } from '../window' import * as NotifyActions from './NotifyActions' import * as SocketActions from './SocketActions' @@ -25,6 +25,7 @@ async (dispatch, getState) => { dispatch(SocketActions.handshake({ socket, roomName: callId, + userId, })) dispatch(initialize()) resolve() diff --git a/src/client/actions/PeerActions.test.ts b/src/client/actions/PeerActions.test.ts index cc370bc..6ddaee4 100644 --- a/src/client/actions/PeerActions.test.ts +++ b/src/client/actions/PeerActions.test.ts @@ -12,7 +12,7 @@ import { PEERCALLS, PEER_EVENT_DATA, ME } from '../constants' describe('PeerActions', () => { function createSocket () { const socket = new EventEmitter() as unknown as ClientSocket - socket.id = 'user1' + socket.id = 'socket-id-user-1' return socket } @@ -30,7 +30,7 @@ describe('PeerActions', () => { dispatch = store.dispatch getState = store.getState - user = { id: 'user2' } + user = { id: 'user1' } socket = createSocket() instances = (Peer as any).instances = []; (Peer as unknown as jest.Mock).mockClear() @@ -40,7 +40,7 @@ describe('PeerActions', () => { describe('create', () => { it('creates a new peer', () => { - PeerActions.createPeer({ socket, user, initiator: 'user2', stream })( + PeerActions.createPeer({ socket, user, initiator: 'other-user', stream })( dispatch, getState) expect(instances.length).toBe(1) @@ -52,7 +52,7 @@ describe('PeerActions', () => { it('sets initiator correctly', () => { PeerActions .createPeer({ - socket, user, initiator: 'user1', stream, + socket, user, initiator: user.id, stream, })(dispatch, getState) expect(instances.length).toBe(1) @@ -124,7 +124,7 @@ describe('PeerActions', () => { const { list } = store.getState().messages expect(list.length).toBeGreaterThan(0) expect(list[list.length - 1]).toEqual({ - userId: 'user2', + userId: user.id, timestamp: jasmine.any(String), image: undefined, message: 'test', diff --git a/src/client/actions/PeerActions.ts b/src/client/actions/PeerActions.ts index 36d379c..dbfbd38 100644 --- a/src/client/actions/PeerActions.ts +++ b/src/client/actions/PeerActions.ts @@ -173,7 +173,7 @@ export function createPeer (options: CreatePeerOptions) { } const peer = new Peer({ - initiator: socket.id === initiator, + initiator: userId === initiator, config: { iceServers }, // Allow the peer to receive video, even if it's not sending stream: // https://github.com/feross/simple-peer/issues/95 diff --git a/src/client/actions/SocketActions.test.ts b/src/client/actions/SocketActions.test.ts index 6c588be..25b9f6b 100644 --- a/src/client/actions/SocketActions.test.ts +++ b/src/client/actions/SocketActions.test.ts @@ -9,6 +9,7 @@ import { createStore, Store, GetState } from '../store' import { ClientSocket } from '../socket' import { Dispatch } from 'redux' import { MediaStream } from '../window' +import { SocketEvent } from '../../shared' describe('SocketActions', () => { const roomName = 'bla' @@ -29,22 +30,39 @@ describe('SocketActions', () => { instances = (Peer as any).instances = [] }) + const userA = { + socketId: 'socket-a', + userId: 'user-a', + } + const userId = userA.userId + + const userB = { + socketId: 'socket-b', + userId: 'user-b', + } + + const userC = { + socketId: 'socket-c', + userId: 'user-c', + } + describe('handshake', () => { describe('users', () => { beforeEach(() => { - SocketActions.handshake({ socket, roomName })(dispatch, getState) + SocketActions + .handshake({ socket, roomName, userId })(dispatch, getState) const payload = { - users: [{ id: 'a' }, { id: 'b' }], - initiator: 'a', + users: [userA, userB], + initiator: userA.userId, } socket.emit('users', payload) expect(instances.length).toBe(1) }) - it('adds a peer for each new user and destroys peers for missing', () => { + it('adds a peer for each new user and destroys missing peers', () => { const payload = { - users: [{ id: 'a' }, { id: 'c' }], - initiator: 'c', + users: [userA, userC], + initiator: userC.userId, } socket.emit(constants.SOCKET_EVENT_USERS, payload) @@ -59,16 +77,17 @@ describe('SocketActions', () => { let data: Peer.SignalData beforeEach(() => { data = {} as any - SocketActions.handshake({ socket, roomName })(dispatch, getState) + SocketActions + .handshake({ socket, roomName, userId })(dispatch, getState) socket.emit('users', { - initiator: 'a', - users: [{ id: 'a' }, { id: 'b' }], + initiator: userA.userId, + users: [userA, userB], }) }) it('should forward signal to peer', () => { socket.emit('signal', { - userId: 'b', + userId: userB.userId, signal: data, }) @@ -94,11 +113,12 @@ describe('SocketActions', () => { let ready = false socket.once('ready', () => { ready = true }) - SocketActions.handshake({ socket, roomName })(dispatch, getState) + SocketActions + .handshake({ socket, roomName, userId })(dispatch, getState) socket.emit('users', { - initiator: 'a', - users: [{ id: 'a' }, { id: 'b' }], + initiator: userA.userId, + users: [userA, userB], }) expect(instances.length).toBe(1) peer = instances[0] @@ -117,8 +137,8 @@ describe('SocketActions', () => { it('emits socket signal with user id', done => { const signal = { bla: 'bla' } - socket.once('signal', (payload: SocketActions.SignalOptions) => { - expect(payload.userId).toEqual('b') + socket.once('signal', (payload: SocketEvent['signal']) => { + expect(payload.userId).toEqual(userB.userId) expect(payload.signal).toBe(signal) done() }) @@ -139,8 +159,8 @@ describe('SocketActions', () => { peer.emit(constants.PEER_EVENT_TRACK, stream.getTracks()[0], stream) expect(store.getState().streams).toEqual({ - b: { - userId: 'b', + [userB.userId]: { + userId: userB.userId, streams: [{ stream, type: undefined, @@ -159,8 +179,8 @@ describe('SocketActions', () => { // test stream with two tracks peer.emit(constants.PEER_EVENT_TRACK, track, stream) expect(store.getState().streams).toEqual({ - b: { - userId: 'b', + [userB.userId]: { + userId: userB.userId, streams: [{ stream, type: undefined, @@ -171,7 +191,7 @@ describe('SocketActions', () => { }) it('removes stream & peer from store', () => { - expect(store.getState().peers).toEqual({ b: peer }) + expect(store.getState().peers).toEqual({ [userB.userId]: peer }) peer.emit('close') expect(store.getState().streams).toEqual({}) expect(store.getState().peers).toEqual({}) diff --git a/src/client/actions/SocketActions.ts b/src/client/actions/SocketActions.ts index 5c93943..8d0c5a2 100644 --- a/src/client/actions/SocketActions.ts +++ b/src/client/actions/SocketActions.ts @@ -3,9 +3,9 @@ import * as PeerActions from '../actions/PeerActions' import * as constants from '../constants' import keyBy from 'lodash/keyBy' import _debug from 'debug' -import { SignalData } from 'simple-peer' import { Dispatch, GetState } from '../store' import { ClientSocket } from '../socket' +import { SocketEvent } from '../../shared' const debug = _debug('peercalls') @@ -15,24 +15,16 @@ export interface SocketHandlerOptions { stream?: MediaStream dispatch: Dispatch getState: GetState -} - -export interface SignalOptions { - signal: SignalData userId: string } -export interface UsersOptions { - initiator: string - users: Array<{ id: string }> -} - class SocketHandler { socket: ClientSocket roomName: string stream?: MediaStream dispatch: Dispatch getState: GetState + userId: string constructor (options: SocketHandlerOptions) { this.socket = options.socket @@ -40,30 +32,36 @@ class SocketHandler { this.stream = options.stream this.dispatch = options.dispatch this.getState = options.getState + this.userId = options.userId } - handleSignal = ({ userId, signal }: SignalOptions) => { + handleSignal = ({ userId, signal }: SocketEvent['signal']) => { const { getState } = this const peer = getState().peers[userId] // debug('socket signal, userId: %s, signal: %o', userId, signal); if (!peer) return debug('user: %s, no peer found', userId) peer.signal(signal) } - handleUsers = ({ initiator, users }: UsersOptions) => { + handleUsers = ({ initiator, users }: SocketEvent['users']) => { const { socket, stream, dispatch, getState } = this debug('socket users: %o', users) this.dispatch(NotifyActions.info('Connected users: {0}', users.length)) const { peers } = this.getState() users - .filter(user => !peers[user.id] && user.id !== socket.id) + .filter( + user => + user.userId && !peers[user.userId] && user.userId !== this.userId) .forEach(user => PeerActions.createPeer({ socket, - user, + user: { + // users without id should be filtered out + id: user.userId!, + }, initiator, stream, })(dispatch, getState)) - const newUsersMap = keyBy(users, 'id') + const newUsersMap = keyBy(users, 'userId') Object.keys(peers) .filter(id => !newUsersMap[id]) .forEach(id => peers[id].destroy()) @@ -73,11 +71,12 @@ class SocketHandler { export interface HandshakeOptions { socket: ClientSocket roomName: string + userId: string stream?: MediaStream } export function handshake (options: HandshakeOptions) { - const { socket, roomName, stream } = options + const { socket, roomName, stream, userId } = options return (dispatch: Dispatch, getState: GetState) => { const handler = new SocketHandler({ @@ -86,6 +85,7 @@ export function handshake (options: HandshakeOptions) { stream, dispatch, getState, + userId, }) // remove listeneres to make seocket reusable @@ -95,9 +95,12 @@ export function handshake (options: HandshakeOptions) { socket.on(constants.SOCKET_EVENT_SIGNAL, handler.handleSignal) socket.on(constants.SOCKET_EVENT_USERS, handler.handleUsers) - debug('socket.id: %s', socket.id) + debug('userId: %s', userId) debug('emit ready for room: %s', roomName) dispatch(NotifyActions.info('Ready for connections')) - socket.emit('ready', roomName) + socket.emit('ready', { + room: roomName, + userId, + }) } } diff --git a/src/client/components/App.tsx b/src/client/components/App.tsx index 7202c45..dcb2900 100644 --- a/src/client/components/App.tsx +++ b/src/client/components/App.tsx @@ -36,13 +36,11 @@ export interface AppProps { } export interface AppState { - videos: Record chatVisible: boolean } export default class App extends React.PureComponent { state: AppState = { - videos: {}, chatVisible: false, } handleShowChat = () => { @@ -92,8 +90,6 @@ export default class App extends React.PureComponent { streams, } = this.props - const { videos } = this.state - const chatVisibleClassName = classnames({ 'chat-visible': this.state.chatVisible, }) @@ -140,7 +136,6 @@ export default class App extends React.PureComponent { const key = localStreams.userId + '_' + i return (