This is a prep commit for #30212. It will allow us to compare the key
and not translated policy name.
Co-authored-by: Tanmay Kumar <tnmdotkr@gmail.com>
This commit refactors user_has_permission_for_group_setting
to accept setting group ID instead of UserGroup object.
We only need ID in checking the permission and this helps in
further commit to avoid prefetching can_access_all_users_group
setting.
UserGroupMembersData is not serializable by orjson. We will be
introducing a TypedDict (which is serializable) in the next commit
called UserGroupMembersDict. This rename will help us distinguish
between the two.
This commit updates the code which computes the dict for
setting groups mapping named user groups to ID and anonymous
groups to UserGroupMembersDict. After the changes, the dict
contains only anonymous groups values and the setting values
for group IDs not present in dict will be computed based on
the fact that those are named user groups.
This is a preparatory refactor for optimizing computing group
setting values for register response by fetching all anonymous
groups membership data just once.
This commit fixes the code to not include deactivated groups in
stream setting values when the setting is set to an anonymous
group. This is consistent with what we do for realm and user
group settings.
As a result, we also deduplicate some code by using existing
function used for realm and group settings.
Since this does impact the ability to access the channel's content, it
makes sense to permit changing subscriptions, just like other
permissions settings on the archived channel.
This was confusingly doing an assertion about the subscription being
active, not the channel. We could rename it to
require_active_subscription. But it was only passed with a non-default
value in b2cb443d24, and that call was
removed in 378062cc83.
We no longer archive private streams when they become vacant,
since user can still have permissions to subscribe to it.
And streams can anyways be archived manually if needed.
Fixes#33689.
This parameter is no longer restricted to realm administrators. Any
user can get the streams they have metadata access to by setting this
parameter to true.
In public_stream_user_ids function, which is used to get users
who can access public streams, there is no need to fetch members
of can_add_subscribers_group as we eventually exclude guests
from them and we have already included all non guest users of
the realm.
We keep around the old `include_all_active` parameter for backwards
compatibility.
Web frontend doesn't use this API and thus there were no changes needed
there.
To get content access streams for mention.py, we will now use
get_content_access_streams and we have done a lot more other refactors
in this commit around filter_stream_authorization. Mainly making that
function only to be used for adding subscribers and naming it
accordingly.
This commit updates code to optimize computing users who have
metadata access via permission groups so that we do not have
to do DB query for each stream to get recursive members for
the groups having permissions.
Previously, when description for a channel -- either during its
creating or when we change its description contained a topic
permalink (through #-mention), then it was not rendered. This
is because of lack of authorization to access the channel.
This is fixed by passing the acting_user through the methods
which update or add the description, so that permissions
of the acting_user could be used to determine whether to
render the #-mention in stream description or not.
Previously, we needed to pass the group to the function, which sometimes
meant having 1 extra query to fetch the user group when we just needed
the group id for this function.
Fixes https://chat.zulip.org/#narrow/channel/101-design/topic/permissions.20for.20admin.20to.20unsubscribe.20others/near/2060197
Non realm admin users were not able to view private channels they were
an administrator of but not subscribed to it. This commit changes that.
We also made changes for those users to be able to see the subscribers
list.
The increase in query count in test_home and test_event_system can be
mitigated by only fetching recursive user group ids when needed within
the `validate_user_access_to_subscribers_helper` function. But that
would require refactoring that function to handle multiple streams and
subscriptions at once, along with changing how that function is used at
different places, which might be an exercise better left as a follow up.
We have optimised the code a little bit by not fetching the group ids in
case the current user is a realm admin.
We are fetching channel_admin_ids and users belonging to
can_add_subscribers_group directly in stream_subscription.py without
using the helper function
`get_user_ids_with_metadata_access_via_permission_groups`. This is due
to a cyclic dependency and we will move `bulk_get_subscriber_peer_info`
to another file in the next commit.
Users in `can_administer_channel_group` and `can_add_subscribers_group`
have access to private channel metadata. They should be notified of
relevant events.
We've only made relevant changes to lib/streams.py in this commit to
make the changes small and reviewable.
Previously, realm and channel admins were not able to change settings
for a private channel they were not subscribed to. This commit changes
that.
We have only added the exception for can_add_subscribers_group
and not privacy settings.
We also need proper functions with proper terminologies for content
and metadata access.
We've also converted the function to check for permission to unsubscribe
others to accept a list of streams instead of checking each stream one
by one.
Earlier, we were passing the whole subscription object to the function
in order to check if the user was subscribed or not. In the future
commits, we want to check that without fetching and passing the complete
subscription object and this commit will help us do that.
We've added a comment highlighting that the function does not check
whether a user has access to the channel or not. Adding `accessible` to
the function name further emphasises that.