Removing users from channels does not remove documents

[jnordberg]

I'm building a test app to try to get my head around the channels and roles thing.

The app just has a list of documents and the sync gateway is using the default sync function.

Each user has is assigned an admin_channel with the same name as their username. And when a user creates a document it is created with 2 channels, their username and a 'admin' channel. I have also created an admin role with admin_channels: ['admin'].

So far so good, when i create new documents they get synced to all users with the admin role, and when i assign the admin role to a user their documents list get populated with all documents.

The problem is when i remove the admin role from an user, they still have all the documents (or at least they don't get notified that the documents have been removed, i.e. my live query does not get triggered) and they can still modify them and the changes propagate.

Am i missing something or is this a bug?

[snej]

Not a bug, but some interesting behaviors that need better documentation.

they don't get notified that the documents have been removed

The changes feed only notifies clients of new revisions of documents, not of indirect changes like channel access being lost. In most cases if you lose access to a channel it's because of a change to a document that granted you access to it, and you'll see that change. (For example, a "chat room" document that lists the room's members and gives them access to its messages channel; if you get removed from the member list you'll get a change to the room doc telling you it's no longer accessible.)

they can still modify them and the changes propagate

A subtle point: read and write access to documents are independent. In fact write access is entirely governed by your sync function: unless the sync function rejects the revision, a client can modify any document.

This is necessary because there are valid use cases where you should be able to write to a channel you don't have read access to — a "drop box". An example would be an email system where every user has an inbox channel that only they have access to, and you send mail by creating a document targeted at another user's inbox.

But after writing this, I'm thinking that this rationale only applies to creating new documents. It doesn't seem valid to be able to update a document that isn't in any channel you have access to. Hm. I should bring this up with Chris.

[snej]

@jchris, any thoughts on this? Would we break any use cases by automatically rejecting updates of existing documents that the user doesn't have read access to?

[jchris]

My gut says we shouldn't treat modifications differently - all you need is one call to requireAccess(doc.channels)

The reason why is logical consistency in our APIs. Nowhere else (aside from optimizations) do we treat the first rev of a document specially. And it would be a deviation from the wysiwyg sync function API.

But these are all matters of taste.

It sounds like a lot of the issue here is driven by CBL keeping stuff around. Currently CBL doesn't know which channels a doc is on, so we can't drop whole channels on the client.

So we may need a mechanism to send removed marks down to CBL even when it's not the doc that changed it's the access control.

Definitely worth thinking about these.

[jchris]

If there is a reasonable implementation so that CBL can be sent _removed messages for documents when the users access control changes, we should implement it.

At first glance it seems like a hard problem:

• detecting when a user loses access to a channel (or a role, and all it's users...)
• sending removed marks for all the documents that are now no longer visible to the user
  • these are the ones that are now on zero user-visible channels
• what happens if the user gets access again? (I think as long as removes are purges, we're ok. I'm not sure what happens if the client treats a remove as a regular rev.)

I'm just not sure Sync Gateway should be sending messages for each removed document. Maybe it's clearer and easier to reason about if Sync Gateway sends lists of channels the user can (no longer) access, and trusts CBL to remove any docs which no longer match any channels.

There are lots of reasons I can think users might want to drop whole channels from a CBL instance anyway, so we'd get more than just this out of tracking the matching channels in the client.

The only thing this breaks is compatibility with other ecosystem members. (Do we know how CouchDB reacts to the _removed notices we send today?)

[snej]

if Sync Gateway sends lists of channels the user can (no longer) access, and trusts CBL to remove any docs which no longer match any channels

CBL doesn't know what channels docs are in. That information only lives on the server.

[jnordberg]

From a end-user standpoint it makes a lot of sense for the documents to be removed if the user looses access to the channel the document is on.

Without this I can't really see a way to implement access control using roles. You would have to find and update all the documents affected manually, removing the user from the list of users (channels) that has access to that specific document.

[jessliu]

@waynecarter thoughts on this? we should define in spec what agreed behavior should be, and share with @amy-kurtzman

[waynecarter]

@jessicacb Post beta-1 we added Document.isGone. It's scheduled to go into the spec as part of issue couchbaselabs/couchbase-lite-api#39 (which I am very behind on getting done).

@jchris @snej on your comments above, is there an issue w/ this?

[jnordberg]

Any updates regarding this?

[snej]

A lot of the changes-feed code was rewritten over the past month, but I'd have to look at the code to tell whether this behavior changed.

[snej]

This may be working correctly now with the rewritten changes-feed code in beta 3. @jnordberg, could you verify?

[jnordberg]

Nope, same behaviour as before (tested against 79d1f7f)

[snej]

@jnordberg, I don't understand this comment you made:

Without this I can't really see a way to implement access control using roles. You would have to find and update all the documents affected manually, removing the user from the list of users (channels) that has access to that specific document.

Roles do work for access control, without having to update individual documents. If a user only had access to a doc through a role, then removing that role means the user can't access that doc anymore. The only thing missing is that the current version of that document doesn't get removed from a client that's already downloaded it. (It's kind of like the way that when your subscription to a magazine expires, the publisher doesn't come to your house and take away the back issues. :)

Earlier:

The problem is when i remove the admin role from an user, they still have all the documents ... and they can still modify them and the changes propagate.

Sounds like your sync function needs to validate that the user has access to the document they're updating.

[snej]

The problem with propagating doc removals when a user loses access to a channel is that there aren't any actual sequence numbers corresponding to them. The changes feed would need to include an entry for each document in the channel, containing the _removed marker, but all of the logic for generating and processing changes feeds is driven by the sequences. I don't know what to do about that.

[jnordberg]

Sounds like your sync function needs to validate that the user has access to the document they're updating.

Yep, using channels to manage access control like @jchris pointed out earlier works great.

(It's kind of like the way that when your subscription to a magazine expires, the publisher doesn't come to your house and take away the back issues. :)

Wouldn't that make the product a distribution gateway and not a synchronising gateway? I would like it to work more like a bank, it lends documents to you at interest and if you don't pay up it busts in to your house and takes them back along with your furniture. 😀

The problem we are trying to solve is that in our setup users can belong to one or more companies, and when a company removes a user they want the documents to be removed as well.

Do you have some idea of a workaround for that with the way the gateway currently works?

The problem with propagating doc removals when a user loses access to a channel is that there aren't any actual sequence numbers corresponding to them. The changes feed would need to include an entry for each document in the channel, containing the _removed marker, but all of the logic for generating and processing changes feeds is driven by the sequences. I don't know what to do about that.

I'm not familiar with the internals of sync_gateway but couldn't you just maintain a document to channel map and a channel to user map, and when a channel is removed from a user you look up what documents that channel has and emit a event to the user?

[snej]

Well, the innards are a lot more complicated than that, because it has to scale to huge numbers of docs and users and everything's persistent. But the real problem is that the notion of "events" (aka "sequences") is based on changes to documents, and removing user access to a channel doesn't change the documents in the channel. So the changes sent to the user would be a different sort of event that doesn't currently exist in the gateway.

[jchris]

Currently the client doesn't track which channels are the reason it has a
given doc. If we tracked that we could just send a "please drop channel X"
to the device.

On Thursday, April 3, 2014, Jens Alfke notifications@github.com wrote:

Well, the innards are a lot more complicated than that, because it has to
scale to huge numbers of docs and users and everything's persistent. But
the real problem is that the notion of "events" (aka "sequences") is based
on changes to documents, and removing user access to a channel doesn't
change the documents in the channel. So the changes sent to the user would
be a different sort of event that doesn't currently exist in the gateway.

Reply to this email directly or view it on GitHubhttps://github.com//issues/264#issuecomment-39457663
.

Chris Anderson @jchris
http://www.couchbase.com

[snej]

Good idea, Chris. But CBL would have to keep a many-to-many relationship between docs and channels, which would add overhead.

[snej]

Doing this would require architectural changes. It'll take some significant thinking about design, and possibly major implementation work. Tagging as Epic.

[petr-vysotskiy]

Hi! Is there a fix/workaround for that?

[tleyden]

Here are a few workaround scenarios that I put together w/ @adamcfraser. Hopefully at some point a more refined version of these will make it into our official docs.

The examples center around hypothetical Sales Associates, Sales Managers, and Sales Leads, and hopefully map easily to common data models. The formatting is a bit rough since they are just sketches at this point.

@jnordberg @petr-vysotskiy @tommyo @atom992 -- would love to hear feedback on what does and doesn't make sense with these examples!

Scenario + Workaround #1: Sales Associate Promotion + Demotion

[tleyden]

Scenario + Workaround #2: Sales lead assigned to Sales Associate and later un-assigned

[jamesnocentini]

• workaround 1

  It should just work and doesn't require any data modeling changes to make it work which is nice! The use of a 3rd channel acts as the mechanism to send the revision one last time to the user that lost access so it can clean its own database.

• workaround 2

  Not sure I follow all of it. But isn't this approach relying on the fact that the database change listener on CBL would bubble up a revision with "removed": true? I don't think that's the case at the moment so the application doesn't know when that revision is received.

[jnordberg]

AFAIK the solution we ended up using was very similar to workaround 1, I've since left the company but the @SafetyCulture guys might have some input, ping @dancet @tjdavey 👋

[tleyden]

workaround 2

Not sure I follow all of it. But isn't this approach relying on the fact that the database change listener on CBL would bubble up a revision with "removed": true? I don't think that's the case at the moment so the application doesn't know when that revision is received.

The difference is essentially the following:

1. In the problem scenario of "Scenario + Workaround Clarifying the setup instructions a bit with a replication example in the README #2"
   the sales associate is being dynamically added and removed from channels via the admin REST API. This has issues, because when they are removed from a channel, their already sync'd docs remain visible in their local couchbase lite database since there is currently no mechanism to signal this over the _changes feed.

2. In the workaround for "Scenario + Workaround Clarifying the setup instructions a bit with a replication example in the README #2", the modeling is flipped around so that sales associates have much more static assignment to their channels, for example SA1 is assigned to the SA1_Leads channel at the point that the SA1 user is created, and this channel assignment never changes. Docs will then move in and out of these static channels, and so the time sequence of events is that:

   1. A lead is assigned to SA1 (and possible other SA's), and so the sales lead doc is updated with channels: ["SA1_Leads", etc.. ] and so the lead appears on SA1's Couchbase Lite database
   2. That lead is later unassigned from SA1 and all other sales associates, and so the sales lead doc is updated with channels: []. At this point the lead will disappear from SA1's Couchbase Lite instance, due to the fact that it will receive a _removed flag for that sales lead, which tells it to basically locally purge the doc from the couchbase lite database.

[geraldapeoples]

Workaround 1 does not work with the current versions of software - couchbase 5.0.1, sync gateway 1.5.1 and couchbase lite 1.4.1.

• Assign user to channel works perfectly and the user receives the docs from that channel.
• Unassign user from channel works, the user no longer receives changes from documents in the channel.
• The mobile database now contains documents from the channel that they were removed from
• THE BROKEN BIT - deleting the documents that are in the channel the user was removed from on the mobile app deletes the document from the sync gateway

Is this a bug? or has the expected behavior changed?

[geraldapeoples]

Please ignore my previous email ... I updated the sync gateway config to have a sync function which includes a RequireAccess function for the documents current channels and the delete from the mobile device does not delete the doc from the gateway. This in co-ordination with the purge document works a treat :o)

[tleyden]

@geraldapeoples glad to hear!

[adamcfraser]

Moved to https://issues.couchbase.com/browse/CBG-639