From 374d2a66df0d9c392256e58b2dca1111af2ccd59 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Fri, 18 Mar 2022 19:08:11 +0530 Subject: [PATCH] user_groups: Add endpoint to check whether a user is member of a group. This commit adds 'GET /user_groups/{id}/members/{id}' endpoint to check whether a user is member of a group. This commit also adds for_read parameter to access_user_group_by_id, which if passed as True will provide access to read user group even if it a system group or if non-admin acting user is not part of the group. --- templates/zerver/api/changelog.md | 3 + .../zerver/help/include/rest-endpoints.md | 1 + zerver/lib/user_groups.py | 11 +++- zerver/openapi/zulip.yaml | 48 ++++++++++++++++ zerver/tests/test_user_groups.py | 57 +++++++++++++++++++ zerver/views/user_groups.py | 27 ++++++++- zproject/urls.py | 4 ++ 7 files changed, 148 insertions(+), 3 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index d254108769..7b50f01dfa 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -31,6 +31,9 @@ format used by the Zulip server that they are interacting with. `remove_subgroups`). * [`PATCH /user_groups/{user_group_id}/subgroups`](/api/update-user-group-subgroups): Added new endpoint for updating subgroups of a user group. +* [`GET /user_groups/{user_group_id}/members/{user_id}`](/api/get-is-user-group-member): + Added new endpoint for checking whether a given user is member of a + given user group. **Feature level 126** diff --git a/templates/zerver/help/include/rest-endpoints.md b/templates/zerver/help/include/rest-endpoints.md index ee9f265405..7bf8e32b69 100644 --- a/templates/zerver/help/include/rest-endpoints.md +++ b/templates/zerver/help/include/rest-endpoints.md @@ -62,6 +62,7 @@ * [Delete a user group](/api/remove-user-group) * [Update user group members](/api/update-user-group-members) * [Update user group subgroups](/api/update-user-group-subgroups) +* [Get user group membership status](/api/get-is-user-group-member) * [Mute a user](/api/mute-user) * [Unmute a user](/api/unmute-user) diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py index d5de2b60ec..6e971db81c 100644 --- a/zerver/lib/user_groups.py +++ b/zerver/lib/user_groups.py @@ -9,9 +9,18 @@ from zerver.lib.exceptions import JsonableError from zerver.models import GroupGroupMembership, Realm, UserGroup, UserGroupMembership, UserProfile -def access_user_group_by_id(user_group_id: int, user_profile: UserProfile) -> UserGroup: +def access_user_group_by_id( + user_group_id: int, user_profile: UserProfile, *, for_read: bool = False +) -> UserGroup: try: user_group = UserGroup.objects.get(id=user_group_id, realm=user_profile.realm) + if for_read and not user_profile.is_guest: + # Everyone is allowed to read a user group and check who + # are its members. Guests should be unable to reach this + # code path, since they can't access user groups API + # endpoints, but we check for guests here for defense in + # depth. + return user_group if user_group.is_system_group: raise JsonableError(_("Insufficient permission")) group_member_ids = get_user_group_direct_members(user_group) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index a746ed652f..b9e976fdcb 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -13975,6 +13975,44 @@ paths: responses: "200": $ref: "#/components/responses/SimpleSuccess" + /user_groups/{user_group_id}/members/{user_id}: + get: + operationId: get-is-user-group-member + summary: Get user group membership status + tags: ["users"] + description: | + Check whether a user is member of user group. + + `GET {{ api_url }}/v1/user_groups/{user_group_id}/members/{user_id}` + + **Changes**: New in Zulip 6.0 (feature level 127). + parameters: + - $ref: "#/components/parameters/UserGroupId" + - $ref: "#/components/parameters/UserId" + - $ref: "#/components/parameters/DirectMemberOnly" + responses: + "200": + description: Success + content: + application/json: + schema: + allOf: + - $ref: "#/components/schemas/JsonSuccessBase" + - $ref: "#/components/schemas/SuccessDescription" + - additionalProperties: false + properties: + result: {} + msg: {} + is_user_group_member: + type: boolean + description: | + Whether the user is member of user group. + example: + { + "msg": "", + "result": "success", + "is_user_group_member": false, + } /real-time: # This entry is a hack; it exists to give us a place to put the text # documenting the parameters for call_on_each_event and friends. @@ -16320,3 +16358,13 @@ components: type: string example: https://github.com/zulip/zulip/issues/%(id)s required: true + DirectMemberOnly: + name: direct_member_only + in: query + description: | + Whether to consider only the direct members of user group and not members + of its subgroups. Default is `False`. + schema: + type: boolean + example: false + required: false diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index e4185d20f1..97f0a9e91d 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -884,3 +884,60 @@ class UserGroupAPITestCase(UserGroupTestCase): # Test when nothing is provided result = self.client_post(f"/json/user_groups/{support_group.id}/subgroups", info={}) self.assert_json_error(result, 'Nothing to do. Specify at least one of "add" or "delete".') + + def test_get_is_user_group_member_status(self) -> None: + self.login("iago") + realm = get_realm("zulip") + desdemona = self.example_user("desdemona") + iago = self.example_user("iago") + othello = self.example_user("othello") + admins_group = UserGroup.objects.get( + realm=realm, name="@role:administrators", is_system_group=True + ) + + # Invalid user ID. + result = self.client_get(f"/json/user_groups/{admins_group.id}/members/25") + self.assert_json_error(result, "No such user") + + # Invalid user group ID. + result = self.client_get(f"/json/user_groups/25/members/{iago.id}") + self.assert_json_error(result, "Invalid user group") + + result_dict = orjson.loads( + self.client_get(f"/json/user_groups/{admins_group.id}/members/{othello.id}").content + ) + self.assertFalse(result_dict["is_user_group_member"]) + + result_dict = orjson.loads( + self.client_get(f"/json/user_groups/{admins_group.id}/members/{iago.id}").content + ) + self.assertTrue(result_dict["is_user_group_member"]) + + # Checking membership of not a direct member but member of a subgroup. + result_dict = orjson.loads( + self.client_get(f"/json/user_groups/{admins_group.id}/members/{desdemona.id}").content + ) + self.assertTrue(result_dict["is_user_group_member"]) + + # Checking membership of not a direct member but member of a subgroup when passing + # recursive parameter as False. + params = {"direct_member_only": orjson.dumps(True).decode()} + result_dict = orjson.loads( + self.client_get( + f"/json/user_groups/{admins_group.id}/members/{desdemona.id}", info=params + ).content + ) + self.assertFalse(result_dict["is_user_group_member"]) + + # Logging in with a user not part of the group. + self.login("hamlet") + + result_dict = orjson.loads( + self.client_get(f"/json/user_groups/{admins_group.id}/members/{iago.id}").content + ) + self.assertTrue(result_dict["is_user_group_member"]) + + result_dict = orjson.loads( + self.client_get(f"/json/user_groups/{admins_group.id}/members/{othello.id}").content + ) + self.assertFalse(result_dict["is_user_group_member"]) diff --git a/zerver/views/user_groups.py b/zerver/views/user_groups.py index 4d4ec88475..c3f89e2022 100644 --- a/zerver/views/user_groups.py +++ b/zerver/views/user_groups.py @@ -22,10 +22,11 @@ from zerver.lib.user_groups import ( access_user_groups_as_potential_subgroups, get_direct_memberships_of_users, get_user_group_direct_members, + is_user_in_group, user_groups_in_realm_serialized, ) -from zerver.lib.users import user_ids_to_users -from zerver.lib.validator import check_int, check_list +from zerver.lib.users import access_user_by_id, user_ids_to_users +from zerver.lib.validator import check_bool, check_int, check_list from zerver.models import UserProfile from zerver.views.streams import compose_views @@ -217,3 +218,25 @@ def update_subgroups_of_user_group( data = compose_views(thunks) return json_success(request, data) + + +@require_member_or_admin +@has_request_variables +def get_is_user_group_member( + request: HttpRequest, + user_profile: UserProfile, + user_group_id: int = REQ(json_validator=check_int, path_only=True), + user_id: int = REQ(json_validator=check_int, path_only=True), + direct_member_only: bool = REQ(json_validator=check_bool, default=False), +) -> HttpResponse: + user_group = access_user_group_by_id(user_group_id, user_profile, for_read=True) + target_user = access_user_by_id(user_profile, user_id, for_admin=False) + + return json_success( + request, + data={ + "is_user_group_member": is_user_in_group( + user_group, target_user, direct_member_only=direct_member_only + ) + }, + ) diff --git a/zproject/urls.py b/zproject/urls.py index 2f97ca0361..84f6327c98 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -176,6 +176,7 @@ from zerver.views.user_groups import ( add_user_group, delete_user_group, edit_user_group, + get_is_user_group_member, get_user_group, update_subgroups_of_user_group, update_user_group_backend, @@ -374,6 +375,9 @@ v1_api_and_json_patterns = [ rest_path("user_groups/", PATCH=edit_user_group, DELETE=delete_user_group), rest_path("user_groups//members", POST=update_user_group_backend), rest_path("user_groups//subgroups", POST=update_subgroups_of_user_group), + rest_path( + "user_groups//members/", GET=get_is_user_group_member + ), # users/me -> zerver.views.user_settings rest_path("users/me/avatar", POST=set_avatar_backend, DELETE=delete_avatar_backend), # users/me/hotspots -> zerver.views.hotspots