📝 Document how to stop clients spoofing op src

docs/api/agent.md

@@ -27,6 +27,13 @@ If the `Connection` was created through [`backend.connect()`]({{ site.baseurl }}
 {: .info }
 The `agent.custom` is passed onto the `options` field in [database adapter]({{ site.baseurl }}{% link adapters/database.md %}) calls as `options.agentCustom`. This allows further customisation at the database level e.g. [in `sharedb-mongo` middleware](https://github.com/share/sharedb-mongo#middlewares).
 
+### `clientId` -- string
+
+> The ID that ShareDB generates for the client. The client uses it as the `src` of its ops, unless it is reconnecting and keeps the ID from its previous connection instead.
+
+{: .info }
+The `clientId` can be set in [`'connect'`]({{ site.baseurl }}{% link middleware/actions.md %}#connect) middleware, for example to [restrict op sources]({{ site.baseurl }}{% link middleware/op-submission.md %}#restricting-op-sources). This doesn't work for clients older than ShareDB v1.2, which are sent their ID before `'connect'` runs.
+
 ### `backend` -- [Backend]({{ site.baseurl }}{% link api/backend.md %})
 
 > The [`Backend`]({{ site.baseurl }}{% link api/backend.md %}) instance that created this `Agent`

docs/middleware/op-submission.md

@@ -141,6 +141,57 @@ backend.on('submitRequestEnd', () => {
 {: .info :}
 Since `'submitRequestEnd'` is an event -- not a middleware hook -- it provides no callback, and no way to return an error to the client. It is purely informational.
 
+## Restricting op sources
+
+Every op carries a `src` and a `seq`, which ShareDB uses to recognise an op that a client resends after reconnecting, so that it isn't applied twice. The `src` is chosen by the client: a reconnecting client reclaims its previous [`clientId`]({{ site.baseurl }}{% link api/agent.md %}#clientid--string) in its handshake, and it can also set `src` on an individual op.
+
+{: .warn :}
+Every op is broadcast with its `src` and `seq`, so a client can submit an op with another client's `src` and that client's next `seq`. ShareDB will then mistake that client's real op for a duplicate, and drop it while telling the client it succeeded.
+
+If your users must not be able to do this to each other, make each `src` name its user. The `'connect'` hook below gives each new client a `clientId` that names its user. The [`'receive'`]({{ site.baseurl }}{% link middleware/actions.md %}#receive) hook drops a reclaimed ID that names someone else, so that the client is given a new one, and rejects ops whose `src` names someone else:
+
+```js
+const crypto = require('crypto')
+const ACTIONS = require('sharedb').MESSAGE_ACTIONS
+
+function srcFor(userId) {
+  return userId + ':' + crypto.randomUUID()
+}
+
+function isSrcOf(userId, src) {
+  if (typeof src !== 'string') return false
+  const separator = src.lastIndexOf(':')
+  return separator !== -1 && src.slice(0, separator) === String(userId)
+}
+
+backend.use('connect', (context, next) => {
+  const userId = authenticatedUserId(context.req)
+  context.agent.custom.userId = userId
+  context.agent.clientId = srcFor(userId)
+  next()
+})
+
+backend.use('receive', (context, next) => {
+  const message = context.data
+  const userId = context.agent.custom.userId
+  if (message.a === ACTIONS.handshake && message.id != null && !isSrcOf(userId, message.id)) {
+    delete message.id
+  }
+  if (message.a === ACTIONS.op && message.src != null && !isSrcOf(userId, message.src)) {
+    return next(new Error('Op src belongs to another user'))
+  }
+  next()
+})
+```
+
+The user ID is sent to collaborators and stored with every op as part of its `src`, so use an opaque ID rather than something personal like an email address.
+
+{: .info :}
+A client that connected before you add these hooks, or whose `Connection` is reused after a different user logs in, is given a new ID when it next reconnects. An op it was still sending under its old ID is rejected.
+
+{: .warn :}
+These hooks need clients from ShareDB v1.2 or later. Older clients are sent their ID before `'connect'` runs, so they never learn the new one.
+
 ## Mutating ops
 
 Ops may be amended in the `apply` middleware using the special `request.$fixup()` method:

lib/agent.js

@@ -25,8 +25,7 @@ function Agent(backend, stream) {
   // src is a client-configurable "id" which the client will set in its handshake,
   // and attach to its ops. This should take precedence over clientId if set.
   // Only legacy clients, or new clients connecting for the first time will use the
-  // Agent-provided clientId. Ideally we'll deprecate clientId in favour of src
-  // in the next breaking change.
+  // Agent-provided clientId.
   this.src = null;
   this.connectTime = Date.now();
 

test/middleware.js

@@ -5,6 +5,7 @@ var types = require('../lib/types');
 var errorHandler = util.errorHandler;
 var ShareDBError = require('../lib/error');
 var sinon = require('sinon');
+var crypto = require('crypto');
 var ACTIONS = require('../lib/message-actions').ACTIONS;
 
 var ERROR_CODE = ShareDBError.CODES;
@@ -63,6 +64,101 @@ describe('middleware', function() {
         done();
       });
     });
+
+    it('uses a clientId set on connect as the connection id and op src', function(done) {
+      var backend = this.backend;
+      backend.use('connect', function(request, next) {
+        request.agent.clientId = 'alice:1';
+        next();
+      });
+
+      var srcs = [];
+      backend.use('submit', function(request, next) {
+        srcs.push(request.op.src);
+        next();
+      });
+
+      backend.connect(null, null, function(connection) {
+        expect(connection.id).to.equal('alice:1');
+        connection.get('dogs', 'fido').create({age: 3}, function(error) {
+          if (error) return done(error);
+          expect(srcs).to.eql(['alice:1']);
+          done();
+        });
+      });
+    });
+  });
+
+  describe('restricting op sources', function() {
+    beforeEach(function() {
+      function srcFor(userId) {
+        return userId + ':' + crypto.randomUUID();
+      }
+
+      function isSrcOf(userId, src) {
+        if (typeof src !== 'string') return false;
+        var separator = src.lastIndexOf(':');
+        return separator !== -1 && src.slice(0, separator) === String(userId);
+      }
+
+      this.backend.use('connect', function(context, next) {
+        var userId = context.req.userId;
+        context.agent.custom.userId = userId;
+        context.agent.clientId = srcFor(userId);
+        next();
+      });
+
+      this.backend.use('receive', function(context, next) {
+        var message = context.data;
+        var userId = context.agent.custom.userId;
+        if (message.a === ACTIONS.handshake && message.id != null && !isSrcOf(userId, message.id)) {
+          delete message.id;
+        }
+        if (message.a === ACTIONS.op && message.src != null && !isSrcOf(userId, message.src)) {
+          return next(new Error('Op src belongs to another user'));
+        }
+        next();
+      });
+    });
+
+    it('keeps a reclaimed id that names the same user', function(done) {
+      var backend = this.backend;
+      backend.connect(null, {userId: 42}, function(connection) {
+        var id = connection.id;
+        expect(id).to.match(/^42:/);
+        connection.close();
+        backend.connect(connection, {userId: 42}, function() {
+          expect(connection.id).to.equal(id);
+          done();
+        });
+      });
+    });
+
+    it('replaces a reclaimed id that names another user', function(done) {
+      var backend = this.backend;
+      backend.connect(null, {userId: 42}, function(connection) {
+        connection.close();
+        connection.id = '7:stale';
+        backend.connect(connection, {userId: 42}, function() {
+          expect(connection.id).to.match(/^42:/);
+          expect(connection.id).to.equal(connection.agent.clientId);
+          done();
+        });
+      });
+    });
+
+    it('rejects an op whose src names another user', function(done) {
+      this.backend.connect(null, {userId: 42}, function(connection) {
+        connection.get('dogs', 'fido').on('error', function(error) {
+          expect(error.message).to.equal('Op src belongs to another user');
+          done();
+        });
+        connection.send({
+          a: ACTIONS.op, c: 'dogs', d: 'fido', v: 0, src: '7:1', seq: 1,
+          create: {type: 'json0', data: {}}
+        });
+      });
+    });
   });
 
   describe('readSnapshots', function() {