🔒️ Only accept presence for the doc on its channel

docs/api/backend.md

@@ -113,6 +113,17 @@ Optional
 
 {: .d-inline-block }
 
+`doNotForwardSendPresenceErrorsToClient` -- boolean
+
+Optional
+{: .label .label-grey }
+
+> Default: `false`
+
+> If set to `true`, errors from the [`'sendPresence'`]({{ site.baseurl }}{% link middleware/actions.md %}#sendpresence) middleware will be passed to the `errorHandler` instead of being sent to the client. Sending these errors to the client is deprecated
+
+{: .d-inline-block }
+
 `doNotCommitNoOps` -- boolean
 
 Optional

docs/api/presence.md

@@ -92,6 +92,8 @@ Optional
 
 {: .warn }
 > Depending on use-case, the same client may have **multiple** presences, so a user or client ID may not be appropriate to use as a presence ID.
+>
+> Presence IDs are sent to every subscriber, so the server should check that a client only uses its own. See [Access control]({{ site.baseurl }}{% link presence.md %}#presence-ids).
 
 Return value
 

docs/middleware/actions.md

@@ -69,6 +69,9 @@ This action has these additional `context` properties:
 
 Presence information has just been received from a client. The presence has not yet been transformed against any ops it has missed.
 
+{: .warn }
+Presence is not covered by document permissions. See [Access control]({{ site.baseurl }}{% link presence.md %}#access-control).
+
 This action has these additional `context` properties:
 
 `collection` -- string
@@ -83,6 +86,9 @@ This action has these additional `context` properties:
 
 Presence information is about to be sent to a client.
 
+{: .warn }
+Presence is not covered by document permissions. See [Access control]({{ site.baseurl }}{% link presence.md %}#access-control).
+
 This action has these additional `context` properties:
 
 `collection` -- string

docs/presence.md

@@ -76,3 +76,86 @@ const localPresence = presence.create()
 // The presence value depends on the type
 localPresence.submit(value)
 ```
+
+## Access control
+
+{: .warn }
+Presence is **not** covered by document permissions: being able to read or write a document does not control who can see or send presence on it. Use [middleware]({{ site.baseurl }}{% link middleware/index.md %}) to restrict presence.
+
+### Who can see presence
+
+Subscribing to presence doesn't trigger any presence middleware, so any client can subscribe to any presence channel -- including the presence of a document it can't read.
+
+To stop a client receiving presence, reject the update in the [`'sendPresence'`]({{ site.baseurl }}{% link middleware/actions.md %}#sendpresence) middleware. For typed presence, ShareDB makes sure that `presence.c` and `presence.d` match the presence channel, so you can check read access with them. This middleware runs for every update about to be sent to every client, so keep the check cheap -- for example, by caching permissions on [`agent.custom`]({{ site.baseurl }}{% link api/agent.md %}#custom--object).
+
+{: .warn }
+Rejecting updates in `'sendPresence'` only stops them reaching the client if you set the [`doNotForwardSendPresenceErrorsToClient`]({{ site.baseurl }}{% link api/backend.md %}#options) option. Otherwise, the client is sent an error for each rejected update, including its presence ID. With the option set, each rejected update is passed to the `errorHandler` instead, which logs it by default.
+
+### Who can send presence
+
+ShareDB rejects typed presence whose collection and document ID don't match its channel. It also only broadcasts typed presence if the client can read the document, as determined by the [`'readSnapshots'`]({{ site.baseurl }}{% link middleware/actions.md %}#readsnapshots) middleware. This is checked when the client starts sending presence on the document, not on every update.
+
+ShareDB doesn't check:
+
+ - whether the client can write to the document
+ - `null` presence, which is sent when a client leaves
+ - untyped presence
+
+Check these in the [`'receivePresence'`]({{ site.baseurl }}{% link middleware/actions.md %}#receivepresence) middleware if you need to.
+
+If you only use typed presence, reject untyped presence in `'receivePresence'`: older clients still apply untyped presence sent on a document's channel.
+
+Older clients also apply presence for another document whose collection and ID join to the same channel, which can happen when collection names contain `.` -- for example, `docs.a`/`b` and `docs`/`a.b` both use the `docs.a.b` channel. The server can't tell these apart, so avoid `.` in the names of collections that use presence until all clients are upgraded.
+
+### Presence IDs
+
+Presence IDs are chosen by the client, and sent to every subscriber, so a client could use another client's presence ID to overwrite or clear its presence, or to stop its later updates being applied. Older clients also fire their pending submit callbacks early when they receive presence with their own presence ID.
+
+To prevent this, tie presence IDs to the user in `'receivePresence'` -- for example by requiring them to start with the user ID and a separator that can't appear in user IDs:
+
+```js
+const localPresence = presence.create(`${userId}:${randomId}`)
+```
+
+{: .info }
+Keep a random suffix, so that the same user can have more than one presence -- for example, in multiple browser tabs.
+
+### Example
+
+```js
+const backend = new Backend({
+  presence: true,
+  doNotForwardSendPresenceErrorsToClient: true,
+  errorHandler: (error) => {
+    if (error.code !== 'PRESENCE_FORBIDDEN') console.error(error)
+  }
+})
+
+backend.use('receivePresence', (context, next) => {
+  // agent.custom is usually set in the 'connect' hook
+  const userId = context.agent.custom.userId
+  const presence = context.presence
+  if (!userId || !presence.id.startsWith(`${userId}:`)) {
+    return next(new Error('Unauthorized'))
+  }
+  // This app only uses typed presence
+  if (!presence.c) {
+    return next(new Error('Unauthorized'))
+  }
+  // Let users clear their own presence, even if they've lost access
+  if (presence.p === null) return next()
+  if (!userCanChangeDoc(userId, presence.c, presence.d)) {
+    return next(new Error('Unauthorized'))
+  }
+  next()
+})
+
+backend.use('sendPresence', (context, next) => {
+  const userId = context.agent.custom.userId
+  const presence = context.presence
+  if (!userCanReadDoc(userId, presence.c, presence.d)) {
+    return next({code: 'PRESENCE_FORBIDDEN', message: 'Forbidden'})
+  }
+  next()
+})
+```

lib/agent.js

@@ -5,6 +5,7 @@ var ACTIONS = require('./message-actions').ACTIONS;
 var types = require('./types');
 var util = require('./util');
 var protocol = require('./protocol');
+var DocPresence = require('./client/presence/doc-presence');
 
 var ERROR_CODE = ShareDBError.CODES;
 
@@ -52,6 +53,9 @@ function Agent(backend, stream) {
   // ops to transform presence if not needed
   this.latestDocVersionStreams = Object.create(null);
   this.latestDocVersions = Object.create(null);
+  // Presence requests whose transform is still in flight, each with the
+  // request it restores to presenceRequests if it's rejected
+  this._pendingPresenceTransforms = [];
 
   // We need to track this manually to make sure we don't reply to messages
   // after the stream was closed.
@@ -448,9 +452,14 @@ Agent.prototype._checkRequest = function(request) {
     if (isInvalidVersion(request.v)) return 'Invalid version';
     if (request.ts != null && typeof request.ts !== 'number') return 'Invalid timestamp';
 
+    if (request.a === ACTIONS.op || request.a === ACTIONS.presence) {
+      if (request.v != null && !util.isInteger(request.v)) return 'Invalid version';
+    }
+
     if (request.a === ACTIONS.presence) {
       if (isInvalidName(request.id)) return 'Invalid presence ID';
       if (isInvalidVersion(request.pv)) return 'Invalid presence version';
+      if (isDocPresence(request) && request.v == null) return 'Invalid version';
     }
   } else if (
     request.a === ACTIONS.bulkFetch ||
@@ -466,8 +475,19 @@ Agent.prototype._checkRequest = function(request) {
   if (request.c != null && isInvalidName(request.c)) return 'Invalid collection';
   if (request.ch != null && isInvalidName(request.ch)) return 'Invalid presence channel';
   if (request.seq != null && typeof request.seq !== 'number') return 'Invalid seq';
+  if (request.a === ACTIONS.presence && isDocPresence(request) && !matchesDocPresenceChannel(request)) {
+    return 'Presence collection and doc ID do not match channel';
+  }
 };
 
+function isDocPresence(request) {
+  return request.c != null || request.d != null;
+}
+
+function matchesDocPresenceChannel(request) {
+  return !!request.c && !!request.d && request.ch === DocPresence.channel(request.c, request.d);
+}
+
 // Handle an incoming message from the client
 Agent.prototype._handleMessage = function(request, callback) {
   try {
@@ -852,6 +872,7 @@ Agent.prototype._broadcastPresence = function(presence, callback) {
     collection: presence.c
   };
   var start = Date.now();
+  var docVersionBeforeUpdate = util.dig(this.latestDocVersions, presence.c, presence.d);
 
   var subscriptionUpdater = presence.p === null ?
     this._unsubscribeDocVersion.bind(this) :
@@ -863,33 +884,68 @@ Agent.prototype._broadcastPresence = function(presence, callback) {
       if (error) return callback(error);
       var requests = presenceRequests[presence.ch] || (presenceRequests[presence.ch] = Object.create(null));
       var previousRequest = requests[presence.id];
-      if (!previousRequest || previousRequest.pv < presence.pv) {
-        presenceRequests[presence.ch][presence.id] = presence;
-      }
-
-      var transformer = function(agent, presence, callback) {
-        callback(null, presence);
-      };
+      cacheNewerPresenceRequest(requests, presence);
+      var transform = {request: presence, fallbackRequest: previousRequest};
+      agent._pendingPresenceTransforms.push(transform);
 
       var latestDocVersion = util.dig(agent.latestDocVersions, presence.c, presence.d);
-      var presenceIsUpToDate = presence.v === latestDocVersion;
-      if (!presenceIsUpToDate) {
-        transformer = backend.transformPresenceToLatestVersion.bind(backend);
-      }
+      if (latestDocVersion == null) latestDocVersion = docVersionBeforeUpdate;
 
-      transformer(agent, presence, function(error, presence) {
-        if (error) return callback(error);
-        var channel = agent._getPresenceChannel(presence.ch);
-        agent.backend.pubsub.publish([channel], presence, function(error) {
+      backend._transformPresenceToLatestVersion(agent, presence, latestDocVersion, function(error, latestPresence) {
+        agent._removePendingPresenceTransform(transform);
+        if (error) {
+          agent._rejectPresenceTransform(requests, transform);
+          return callback(error);
+        }
+        cacheNewerPresenceRequest(requests, presence);
+        agent._raisePendingPresenceFallbacks(presence);
+        var channel = agent._getPresenceChannel(latestPresence.ch);
+        agent.backend.pubsub.publish([channel], latestPresence, function(error) {
           if (error) return callback(error);
           backend.emit('timing', 'presence.broadcast', Date.now() - start, context);
-          callback(null, presence);
+          callback(null, latestPresence);
         });
       });
     });
   });
 };
 
+Agent.prototype._removePendingPresenceTransform = function(transform) {
+  var transforms = this._pendingPresenceTransforms;
+  var index = transforms.indexOf(transform);
+  if (index !== -1) transforms.splice(index, 1);
+};
+
+Agent.prototype._raisePendingPresenceFallbacks = function(acceptedRequest) {
+  var transforms = this._pendingPresenceTransforms;
+  for (var i = 0; i < transforms.length; i++) {
+    var request = transforms[i].request;
+    var fallbackRequest = transforms[i].fallbackRequest;
+    var isSamePresence = request.ch === acceptedRequest.ch && request.id === acceptedRequest.id;
+    var fallbackIsOlder = !fallbackRequest || fallbackRequest.pv < acceptedRequest.pv;
+    if (isSamePresence && request.pv > acceptedRequest.pv && fallbackIsOlder) {
+      transforms[i].fallbackRequest = acceptedRequest;
+    }
+  }
+};
+
+Agent.prototype._rejectPresenceTransform = function(requests, rejectedTransform) {
+  var rejectedRequest = rejectedTransform.request;
+  var fallbackRequest = rejectedTransform.fallbackRequest;
+  var transforms = this._pendingPresenceTransforms;
+  for (var i = 0; i < transforms.length; i++) {
+    if (transforms[i].fallbackRequest === rejectedRequest) transforms[i].fallbackRequest = fallbackRequest;
+  }
+  if (requests[rejectedRequest.id] !== rejectedRequest) return;
+  if (fallbackRequest) requests[rejectedRequest.id] = fallbackRequest;
+  else delete requests[rejectedRequest.id];
+};
+
+function cacheNewerPresenceRequest(requests, request) {
+  var cachedRequest = requests[request.id];
+  if (!cachedRequest || cachedRequest.pv < request.pv) requests[request.id] = request;
+}
+
 Agent.prototype._subscribeDocVersion = function(collection, id, callback) {
   if (!collection || !id) return callback();
 

lib/backend.js

@@ -936,8 +936,12 @@ Backend.prototype._buildSnapshotFromOps = function(id, startingSnapshot, ops, ca
 };
 
 Backend.prototype.transformPresenceToLatestVersion = function(agent, presence, callback) {
+  this._transformPresenceToLatestVersion(agent, presence, null, callback);
+};
+
+Backend.prototype._transformPresenceToLatestVersion = function(agent, presence, trackedVersion, callback) {
   if (!presence.c || !presence.d) return callback(null, presence);
-  this.getOps(agent, presence.c, presence.d, presence.v, null, function(error, ops) {
+  this._getOpsSincePresence(agent, presence, trackedVersion, function(error, ops) {
     if (error) return callback(error);
     for (var i = 0; i < ops.length; i++) {
       var op = ops[i];
@@ -951,6 +955,47 @@ Backend.prototype.transformPresenceToLatestVersion = function(agent, presence, c
   });
 };
 
+Backend.prototype._getOpsSincePresence = function(agent, presence, trackedVersion, callback) {
+  if (trackedVersion == null) return this._getOpsSincePresenceFromSnapshotVersion(agent, presence, callback);
+  var isLeave = presence.p === null;
+  if (!isLeave && presence.v === trackedVersion) return callback(null, []);
+  var backend = this;
+  this._getOpsSincePresenceFromVersion(agent, presence, trackedVersion, function(error, ops) {
+    if (error && presence.v > trackedVersion) {
+      return backend._getOpsSincePresenceFromSnapshotVersion(agent, presence, callback);
+    }
+    callback(error, ops);
+  });
+};
+
+Backend.prototype._getOpsSincePresenceFromSnapshotVersion = function(agent, presence, callback) {
+  var backend = this;
+  var projection = this.projections[presence.c];
+  var collection = (projection) ? projection.target : presence.c;
+  this.db.getSnapshot(collection, presence.d, {}, {agentCustom: agent.custom}, function(error, snapshot) {
+    if (error) return callback(error);
+    if (presence.v === snapshot.v) return callback(null, []);
+    backend._getOpsSincePresenceFromVersion(agent, presence, snapshot.v, callback);
+  });
+};
+
+Backend.prototype._getOpsSincePresenceFromVersion = function(agent, presence, knownVersion, callback) {
+  var from = Math.min(presence.v, knownVersion);
+  this.getOps(agent, presence.c, presence.d, from, null, function(error, ops) {
+    if (error) return callback(error);
+    var versionAfterOps = ops.length ? ops[ops.length - 1].v + 1 : from;
+    if (presence.v > versionAfterOps) {
+      return callback(new ShareDBError(
+        ERROR_CODE.ERR_OP_VERSION_NEWER_THAN_CURRENT_SNAPSHOT,
+        'Presence version newer than current snapshot'
+      ));
+    }
+    callback(null, ops.filter(function(op) {
+      return op.v >= presence.v;
+    }));
+  });
+};
+
 function pluckIds(snapshots) {
   var ids = [];
   for (var i = 0; i < snapshots.length; i++) {

lib/client/presence/doc-presence.js

@@ -17,6 +17,12 @@ DocPresence.channel = function(collection, id) {
   return collection + '.' + id;
 };
 
+DocPresence.prototype._receiveUpdate = function(error, message) {
+  var isForThisDoc = message.c === this.collection && message.d === this.id;
+  if (!error && !isForThisDoc) return;
+  Presence.prototype._receiveUpdate.call(this, error, message);
+};
+
 DocPresence.prototype._createLocalPresence = function(id) {
   return new LocalDocPresence(this, id);
 };

lib/client/presence/presence.js

@@ -120,10 +120,13 @@ Presence.prototype._handleUnsubscribe = function(error, seq) {
 };
 
 Presence.prototype._receiveUpdate = function(error, message) {
+  var isErrorForOwnRequest = !!error && message.pv != null;
+  var isFromThisConnection = message.src === this.connection.id;
   var localPresence = util.dig(this.localPresences, message.id);
-  if (localPresence) return localPresence._ack(error, message.pv);
+  if (localPresence && (isErrorForOwnRequest || isFromThisConnection)) return localPresence._ack(error, message.pv);
 
   if (error) return this.emit('error', error);
+  if (localPresence) return;
   var presence = this;
   var remotePresence = util.digOrCreate(this._remotePresenceInstances, message.id, function() {
     return presence._createRemotePresence(message.id);

test/agent.js

@@ -144,6 +144,25 @@ describe('Agent', function() {
       });
     });
 
+    it('rejects an op with a non-integer version', function(done) {
+      var message = {a: ACTIONS.op, c: 'dogs', d: 'fido', v: 1.5, seq: 1, op: [{p: ['age'], na: 1}]};
+      expectRejected(message, 'Invalid version', done);
+    });
+
+    [unconvertible, ['dogs'], '__proto__'].forEach(function(badCollection) {
+      it('rejects doc presence with collection ' + JSON.stringify(badCollection), function(done) {
+        var message = {a: ACTIONS.presence, ch: 'dogs.fido', c: badCollection, d: 'fido', id: 'rex', pv: 0, v: 0};
+        expectRejected(message, 'Invalid collection', done);
+      });
+    });
+
+    [unconvertible, '__proto__'].forEach(function(badChannel) {
+      it('rejects doc presence with channel ' + JSON.stringify(badChannel), function(done) {
+        var message = {a: ACTIONS.presence, ch: badChannel, c: 'dogs', d: 'fido', id: 'rex', pv: 0, v: 0};
+        expectRejected(message, 'Invalid presence channel', done);
+      });
+    });
+
     [unconvertible, true].forEach(function(badProtocol) {
       it('rejects a handshake with protocol ' + JSON.stringify(badProtocol), function(done) {
         expectRejected({a: ACTIONS.handshake, protocol: badProtocol, protocolMinor: 2}, 'Invalid protocol', done);