Skip to content

Commit

Permalink
Add actions to group membership APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
seanh committed Nov 23, 2024
1 parent 702c89e commit 728cd4e
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 13 deletions.
33 changes: 31 additions & 2 deletions h/presenters/group_membership_json.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,41 @@
from h.models import GroupMembershipRoles
from h.security import Permission
from h.traversal.group_membership import (
EditGroupMembershipContext,
GroupMembershipContext,
)


class GroupMembershipJSONPresenter:
def __init__(self, membership):
def __init__(self, request, membership):
self.request = request
self.membership = membership

def asdict(self):
return {
membership_dict = {
"authority": self.membership.group.authority,
"userid": self.membership.user.userid,
"username": self.membership.user.username,
"display_name": self.membership.user.display_name,
"roles": self.membership.roles,
"actions": [],
}

if self.request.has_permission(
Permission.Group.MEMBER_REMOVE,
GroupMembershipContext(
self.membership.group, self.membership.user, self.membership
),
):
membership_dict["actions"].append("delete")

for role in GroupMembershipRoles:
if self.request.has_permission(
Permission.Group.MEMBER_EDIT,
EditGroupMembershipContext(
self.membership.group, self.membership.user, self.membership, role
),
):
membership_dict["actions"].append(f"updates.roles.{role}")

return membership_dict
14 changes: 11 additions & 3 deletions h/views/api/group_members.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
description="Fetch a list of all members of a group",
permission=Permission.Group.READ,
)
def list_members(context: GroupContext, _request):
def list_members(context: GroupContext, request):
return [
GroupMembershipJSONPresenter(membership).asdict()
GroupMembershipJSONPresenter(request, membership).asdict()
for membership in context.group.memberships
]

Expand Down Expand Up @@ -90,4 +90,12 @@ def edit_member(context: GroupMembershipContext, request):
old_roles,
)

return GroupMembershipJSONPresenter(context.membership).asdict()
if context.user == request.user:
# Update request.identity.user.memberships so permissions checks done
# by GroupMembershipJSONPresenter below return the right results.
# Otherwise permissions checks will be based on the old roles.
for membership in request.identity.user.memberships:
if membership.group.id == context.group.id:
membership.roles = new_roles

return GroupMembershipJSONPresenter(request, context.membership).asdict()
45 changes: 40 additions & 5 deletions tests/unit/h/presenters/group_membership_json_test.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,54 @@
import pytest

from h.models import GroupMembership
from h.presenters.group_membership_json import GroupMembershipJSONPresenter


class TestGroupMembershipJSONPresenter:
def test_it(self, factories):
user = factories.User.build()
group = factories.Group.build()
membership = GroupMembership(user=user, group=group)
def test_it(self, user, group, membership, pyramid_request, pyramid_config):
pyramid_config.testing_securitypolicy(permissive=False)

json = GroupMembershipJSONPresenter(pyramid_request, membership).asdict()

assert json == {
"authority": group.authority,
"userid": user.userid,
"username": user.username,
"display_name": user.display_name,
"roles": membership.roles,
"actions": [],
}

def test_it_with_permissive_securitypolicy(
self, user, group, membership, pyramid_request, pyramid_config
):
pyramid_config.testing_securitypolicy(permissive=True)

json = GroupMembershipJSONPresenter(membership).asdict()
json = GroupMembershipJSONPresenter(pyramid_request, membership).asdict()

assert json == {
"authority": group.authority,
"userid": user.userid,
"username": user.username,
"display_name": user.display_name,
"roles": membership.roles,
"actions": [
"delete",
"updates.roles.member",
"updates.roles.moderator",
"updates.roles.admin",
"updates.roles.owner",
],
}

@pytest.fixture
def user(self, factories):
return factories.User.build()

@pytest.fixture
def group(self, factories):
return factories.Group.build()

@pytest.fixture
def membership(self, user, group):
return GroupMembership(user=user, group=group)
32 changes: 29 additions & 3 deletions tests/unit/h/views/api/group_members_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from h import presenters
from h.models import GroupMembership
from h.schemas.base import ValidationError
from h.security.identity import Identity, LongLivedGroup, LongLivedMembership
from h.traversal import GroupContext, GroupMembershipContext
from h.views.api.exceptions import PayloadError

Expand All @@ -27,8 +28,8 @@ def test_it(self, context, pyramid_request, GroupMembershipJSONPresenter):
response = views.list_members(context, pyramid_request)

assert GroupMembershipJSONPresenter.call_args_list == [
call(sentinel.membership_1),
call(sentinel.membership_2),
call(pyramid_request, sentinel.membership_1),
call(pyramid_request, sentinel.membership_2),
]
presenter_instances[0].asdict.assert_called_once_with()
presenter_instances[1].asdict.assert_called_once_with()
Expand Down Expand Up @@ -99,7 +100,9 @@ def test_it(
sentinel.json_body
)
assert context.membership.roles == sentinel.new_roles
GroupMembershipJSONPresenter.assert_called_once_with(context.membership)
GroupMembershipJSONPresenter.assert_called_once_with(
pyramid_request, context.membership
)
assert response == GroupMembershipJSONPresenter.return_value.asdict.return_value
assert caplog.messages == [
f"Changed group membership roles: {context.membership!r} (previous roles were: {sentinel.old_roles!r})",
Expand All @@ -114,6 +117,29 @@ def test_noop(self, context, pyramid_request, EditGroupMembershipAPISchema, capl

assert not caplog.messages

def test_user_changing_own_role(self, context, pyramid_request, pyramid_config):
context.user = pyramid_request.user
identity = create_autospec(Identity, instance=True, spec_set=True)
identity.user.memberships = [
create_autospec(
LongLivedMembership,
instance=True,
group=create_autospec(LongLivedGroup, instance=True, id="other"),
),
create_autospec(
LongLivedMembership,
instance=True,
group=create_autospec(
LongLivedGroup, instance=True, id=context.group.id
),
),
]
pyramid_config.testing_securitypolicy(permissive=True, identity=identity)

views.edit_member(context, pyramid_request)

assert identity.user.memberships[1].roles == sentinel.new_roles

def test_it_errors_if_the_user_doesnt_have_permission(
self, context, pyramid_request, pyramid_config
):
Expand Down

0 comments on commit 728cd4e

Please sign in to comment.