Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for denying guest users access to all other users in the organization #10970

Closed
4 tasks
timabbott opened this issue Dec 6, 2018 · 41 comments
Closed
4 tasks
Labels
area: settings (admin/org) difficult Issues which we expect to be quite difficult new feature A proposed new feature for the product priority: high

Comments

@timabbott
Copy link
Sponsor Member

Zulip's current "guest users" feature is designed to handle well the case of contractors, where you want someone to be part of your organization and able to send PMs to anyone, but limited to only a few streams.

For some use cases (e.g. customer support), folks often want to have a more limited guest user feature, where the guests are unable to send PMs to or otherwise interact with users not subscribed to streams they are a member of. This issue is to track for what would be required to support that.

  • The set of "users" data delivered by the /register endpoint (and also any GET /users type endpoints) will need to be restricted to the set of users who have ever been subscribed to a stream the guest user has ever been on (i.e. we'll be using a Subscription() model lookup, but without limiting for active=True, both on the guest side and the other user side. We need to do this because the guest user's clients need to be able to understand any messages that they might have access to.
  • We need to restrict which users the guests can send PMs to, and possibly also which users can PM guest users in a corresponding fashion.
  • The places where we have realm.presence_disabled checks need to be updated to support sending limited presence data to guest users. Probably for this, we should just pull the general presence cache and then strip data from it, because that's what makes sense from a cache performance perspective (each guest will have access to a different set of users)
  • (Probably a task for me) We need to look at the denormalized message objects that clients send to ensure that they have all the data a guest user client might need to render them. I think this is OK already.
@timabbott timabbott added area: settings (admin/org) priority: high difficult Issues which we expect to be quite difficult labels Dec 6, 2018
@zulipbot
Copy link
Member

zulipbot commented Dec 6, 2018

Hello @zulip/server-settings members, this issue was labeled with the "area: settings (admin/org)" label, so you may want to check it out!

@mateuszmandera
Copy link
Contributor

Could this be done as a special case in a more general "user groups" framework, thus a part of #10688 ?

@2208Abhinav
Copy link
Collaborator

@timabbott I will try to solve this ssue. I will try my best :) . I haven't contributed to backend till now.

@2208Abhinav
Copy link
Collaborator

@zulipbot claim

@timabbott
Copy link
Sponsor Member Author

@2208Abhinav I don't think this is a good first backend issue, which is why I tagged it as "difficult".

@2208Abhinav
Copy link
Collaborator

@timabbott I will not take much time if I am not able to solve it and will abandon it.

@2208Abhinav
Copy link
Collaborator

@zulipbot abandon

@donShakespeare
Copy link

I had assumed that guests could see nothing but their fellow users in a stream, but boy was I mistaken. Having rival clients in the same org is no bueno, especially when their usernames are attractively spelled out in the list (usually with company name), and their emails are accessible with a simple click.

While we wait for an implementation, I suggest to introduce a warning asap - something like what Matrix/riot does when inviting a user to a Community.

Cheers fellows!

image

@nanathlor
Copy link

If possible, having a "Contractor" account with current Guest rights and a new Guest account who can't see other guests (optionally can/can't in the subscribed stream) would be very cool.

@timabbott
Copy link
Sponsor Member Author

Yeah, we're planning to build something of that form. There's a reasonable amount of complexity to making that change, because our frontends currently assume that they know about users who might send a message, so don't expect it soon.

In the meantime, @rishig we should probably add one of our nice ? help links here pointed at our docs on how guests work (https://zulipchat.com/help/roles-and-permissions); that'll be useful regardless of how the feature works in the future:
image

@happyalu
Copy link

Is there an update on the ETA of this feature?

@ghost
Copy link

ghost commented Feb 26, 2020

i'm not liking the fact that ALL users even though people separated into different streams can see everyone. Can you hide all online users i.e. if I am a guest in stream A, I don't want that person seeing people in stream B, C, D etc. I've turned off PM even though I would prefer that be available, I expected each invited guest to only be able to see people in their own stream.....

@timabbott
Copy link
Sponsor Member Author

We don't have an ETA for this, as it isn't currently being worked on.

@jamesmunns
Copy link

Hey there @timabbott, our company would like to use guests in this kind of way (they can only see folks that share streams with them). Is there anything as a customer/business we can do to help prioritize this feature?

Alternatively, if you know any way of making it easier to exist in multiple zulip organizations at once (some kind of multi-organization web interface), that could be another mitigation for us.

We tend to have customer interactions, and having them be part of our chat would be a huge workflow win for us, but due to NDAs and such, we can't generally have them be visible to other guests.

@jamesmunns
Copy link

Speaking specifically, depending on the cost, we could be interested in sponsoring the implementation of this feature.

We really love using Zulip, and would prefer not to move to another platform for comms outside of our organization, or to have to juggle 5-10 Zulip organizations at once.

@timabbott
Copy link
Sponsor Member Author

Chat with us via support; it's something that we'd definitely be open to prioritizing in response to funding.

@Gittenburg Gittenburg added the new feature A proposed new feature for the product label Jul 9, 2020
@yassinetouati
Copy link

Hi
i need to add a guest to my organization and also to a special stream, i did it 👍
the problem is : the guest cant send messages to all users(members) of my organization !
can you please share if there is a special configuration to do for stopping permission to chat with other users, but only those in the same stream
Thank you so much 👍

@cgb
Copy link

cgb commented Aug 12, 2020

Hi there, I'm also eager to see this being implemented, Covid has really accelerated my organisations online interactions with customers and using zulip for this would be a great win compared to having to use Teams... :) Thanks!

@timabbott
Copy link
Sponsor Member Author

There's currently no way to do this that's effective (even if we hid the UI widget, data on other guest users would be send via the API). But it's something on the longer-term roadmap, and if anyone's interested in helping fund this happen, email Zulip support.

@sahil87
Copy link

sahil87 commented Oct 3, 2020

We are currently using Zulip for both inter-company and partner communication. Having this would allow us to keep the two realms shielded from others. Really looking forward to this. Thanks!

@bmalynovytch
Copy link

Hi there !

This issue has been there for nearly 5 years.
Before reworking the whole stuff the hard way, taking literally years, a quick (and maybe a little dirty) fix would've been better for a better adoption of Zulip in companies providing support to multiple customers.

Could we have an ETA on this ? (are we still talking in tens of months ?)

Thx !
cc @alya @timabbott

sahil839 added a commit to sahil839/zulip that referenced this issue Oct 30, 2023
We now send "realm_user/update" (and "realm_bot/update" for bots)
events with "is_active" field when deactivating and reactivating
users, including bots.

We would want to use "remove" event for a user losing access
to another user for zulip#10970, so it is better to use "update"
event for deactivation as we only update "is_active" field
in the user objects and the clients still have the data for
deactivated users.

Previously, we used to send "add" event for reactivation along
with complete user objects, but clients should have the data
for deactivated users as well, so an "update" event is enough
like we do when deactivating users.
sahil839 added a commit to sahil839/zulip that referenced this issue Oct 31, 2023
We now send "realm_user/update" (and "realm_bot/update" for bots)
events with "is_active" field when deactivating and reactivating
users, including bots.

We would want to use "remove" event for a user losing access
to another user for zulip#10970, so it is better to use "update"
event for deactivation as we only update "is_active" field
in the user objects and the clients still have the data for
deactivated users.

Previously, we used to send "add" event for reactivation along
with complete user objects, but clients should have the data
for deactivated users as well, so an "update" event is enough
like we do when deactivating users.
sahil839 added a commit to sahil839/zulip that referenced this issue Nov 1, 2023
We now send "realm_user/update" (and "realm_bot/update" for bots)
events with "is_active" field when deactivating and reactivating
users, including bots.

We would want to use "remove" event for a user losing access
to another user for zulip#10970, so it is better to use "update"
event for deactivation as we only update "is_active" field
in the user objects and the clients still have the data for
deactivated users.

Previously, we used to send "add" event for reactivation along
with complete user objects, but clients should have the data
for deactivated users as well, so an "update" event is enough
like we do when deactivating users.
sahil839 added a commit to sahil839/zulip that referenced this issue Nov 1, 2023
We now send "realm_user/update" (and "realm_bot/update" for bots)
events with "is_active" field when deactivating and reactivating
users, including bots.

We would want to use "remove" event for a user losing access
to another user for zulip#10970, so it is better to use "update"
event for deactivation as we only update "is_active" field
in the user objects and the clients still have the data for
deactivated users.

Previously, we used to send "add" event for reactivation along
with complete user objects, but clients should have the data
for deactivated users as well, so an "update" event is enough
like we do when deactivating users.
timabbott pushed a commit to sahil839/zulip that referenced this issue Nov 2, 2023
We now send "realm_user/update" (and "realm_bot/update" for bots)
events with "is_active" field when deactivating and reactivating
users, including bots.

We would want to use "remove" event for a user losing access
to another user for zulip#10970, so it is better to use "update"
event for deactivation as we only update "is_active" field
in the user objects and the clients still have the data for
deactivated users.

Previously, we used to send "add" event for reactivation along
with complete user objects, but clients should have the data
for deactivated users as well, so an "update" event is enough
like we do when deactivating users.
timabbott pushed a commit that referenced this issue Nov 2, 2023
We now send "realm_user/update" (and "realm_bot/update" for bots)
events with "is_active" field when deactivating and reactivating
users, including bots.

We would want to use "remove" event for a user losing access
to another user for #10970, so it is better to use "update"
event for deactivation as we only update "is_active" field
in the user objects and the clients still have the data for
deactivated users.

Previously, we used to send "add" event for reactivation along
with complete user objects, but clients should have the data
for deactivated users as well, so an "update" event is enough
like we do when deactivating users.
sahil839 added a commit to sahil839/zulip that referenced this issue Nov 4, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of zulip#10970.
sahil839 added a commit to sahil839/zulip that referenced this issue Nov 4, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of zulip#10970.
sahil839 added a commit to sahil839/zulip that referenced this issue Nov 5, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of zulip#10970.
emmawitt pushed a commit to emmawitt/zulip that referenced this issue Nov 7, 2023
We now send "realm_user/update" (and "realm_bot/update" for bots)
events with "is_active" field when deactivating and reactivating
users, including bots.

We would want to use "remove" event for a user losing access
to another user for zulip#10970, so it is better to use "update"
event for deactivation as we only update "is_active" field
in the user objects and the clients still have the data for
deactivated users.

Previously, we used to send "add" event for reactivation along
with complete user objects, but clients should have the data
for deactivated users as well, so an "update" event is enough
like we do when deactivating users.
sahil839 added a commit to sahil839/zulip that referenced this issue Nov 8, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of zulip#10970.
sahil839 added a commit to sahil839/zulip that referenced this issue Nov 9, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of zulip#10970.
sahil839 added a commit to sahil839/zulip that referenced this issue Nov 10, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of zulip#10970.
timabbott pushed a commit to sahil839/zulip that referenced this issue Nov 11, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of zulip#10970.
timabbott pushed a commit to timabbott/zulip that referenced this issue Nov 11, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of zulip#10970.
timabbott pushed a commit that referenced this issue Nov 13, 2023
This commit adds new setting for controlling who can access
all users in the realm which would have "Everyone" and
"Members only" option.

Fixes part of #10970.
@bmalynovytch
Copy link

@sahil839 @alya @timabbott Any update / ETA for this issue ?

@timabbott
Copy link
Sponsor Member Author

The main PR for this with ~85% of the complexity merged today; so I expect it to be in 8.0 (due in ~2 weeks).

timabbott pushed a commit to sahil839/zulip that referenced this issue Dec 10, 2023
This commit adds code to not include original details of senders like
name, email and avatar url in the message objects sent through events
and in the response of endpoint used to fetch messages.

This is the last major commit for the project to add support for
limiting guest access to an entire organization.

Fixes zulip#10970.
afeefuddin pushed a commit to afeefuddin/zulip that referenced this issue Dec 12, 2023
This commit adds code to not include original details of senders like
name, email and avatar url in the message objects sent through events
and in the response of endpoint used to fetch messages.

This is the last major commit for the project to add support for
limiting guest access to an entire organization.

Fixes zulip#10970.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: settings (admin/org) difficult Issues which we expect to be quite difficult new feature A proposed new feature for the product priority: high
Projects
Status: Done
Development

No branches or pull requests