wip

h/routes.py

@@ -159,8 +159,7 @@ def includeme(config):  # pylint: disable=too-many-statements
     config.add_route(
         "api.group_member",
         "/api/groups/{pubid}/members/{userid}",
-        factory="h.traversal.GroupRequiredRoot",
-        traverse="/{pubid}",
+        factory="h.traversal.group_membership_api_factory",
     )
     config.add_route("api.search", "/api/search")
     config.add_route("api.users", "/api/users", factory="h.traversal.UserRoot")

h/security/permission_map.py

@@ -55,6 +55,7 @@
         [p.group_has_user_as_moderator],
     ],
     Permission.Group.MEMBER_ADD: [[p.group_matches_authenticated_client_authority]],
+    Permission.Group.MEMBER_REMOVE: [[p.group_member_remove]],
     Permission.Group.MODERATE: [[p.group_created_by_user]],
     # --------------------------------------------------------------------- #
     # Annotations

h/security/permissions.py

@@ -40,6 +40,9 @@ class Group(Enum):
         MEMBER_ADD = "group:member:add"
         """Add a user other than yourself (that's JOIN) to a group."""
 
+        MEMBER_REMOVE = "group:member:remove"
+        """Remove a member from the group."""
+
     class Annotation(Enum):
         """Permissions relating to annotations."""
 

h/security/predicates.py

@@ -13,6 +13,7 @@
 from itertools import chain
 
 from h.models.group import GroupMembershipRoles, JoinableBy, ReadableBy, WriteableBy
+from h.traversal import GroupMembershipContext
 
 
 def requires(*parent_predicates):
@@ -182,6 +183,42 @@ def group_matches_authenticated_client_authority(identity, context):
     return context.group.authority == identity.auth_client.authority
 
 
+@requires(authenticated_user, group_found)
+def group_member_remove(identity, context: GroupMembershipContext):
+    def get_authenticated_users_membership():
+        """Return the authenticated users membership in the target group."""
+        for membership in identity.user.memberships:
+            if membership.group.id == context.group.id:
+                return membership
+
+        return None
+
+    membership = get_authenticated_users_membership()
+
+    if not membership:
+        # You can't remove anyone from a group you're not a member of.
+        return False
+
+    if identity.user.userid == context.user.userid:
+        # Any member can remove themselves from a group.
+        return True
+
+    if "owner" in context.membership.roles or "admin" in context.membership.roles:
+        # Only owners can remove other owners.
+        return "owner" in membership.roles
+
+    if "moderator" in context.membership.roles:
+        # Owners and admins can remove moderators.
+        return "owner" in membership.roles or "admin" in membership.roles
+
+    # Owners, admins and moderators can remove plain members.
+    return (
+        "owner" in membership.roles
+        or "admin" in membership.roles
+        or "moderator" in membership.roles
+    )
+
+
 def resolve_predicates(mapping):
     """
     Expand predicates with requirements into concrete lists of predicates.

h/services/group_members.py

@@ -24,6 +24,14 @@ def __init__(self, db, user_fetcher, publish):
         self.user_fetcher = user_fetcher
         self.publish = publish
 
+    def get(self, group, user) -> GroupMembership | None:
+        """Return `user`'s existing membership in `group`, if any."""
+        return self.db.scalar(
+            select(GroupMembership)
+            .where(GroupMembership.group == group)
+            .where(GroupMembership.user == user)
+        )
+
     def add_members(self, group, userids):
         """
         Add the users indicated by userids to this group's members.
@@ -63,11 +71,7 @@ def member_join(self, group, userid):
         """Add `userid` to the member list of `group`."""
         user = self.user_fetcher(userid)
 
-        existing_membership = self.db.scalar(
-            select(GroupMembership)
-            .where(GroupMembership.user == user)
-            .where(GroupMembership.group == group)
-        )
+        existing_membership = self.get(group, user)
 
         if existing_membership:
             # The user is already a member of the group.
@@ -86,11 +90,7 @@ def member_leave(self, group, userid):
         """Remove `userid` from the member list of `group`."""
         user = self.user_fetcher(userid)
 
-        membership = self.db.scalars(
-            select(GroupMembership)
-            .where(GroupMembership.group == group)
-            .where(GroupMembership.user == user)
-        ).one_or_none()
+        membership = self.get(group, user)
 
         if not membership:
             return

h/traversal/__init__.py

@@ -62,6 +62,10 @@
 
 from h.traversal.annotation import AnnotationContext, AnnotationRoot
 from h.traversal.group import GroupContext, GroupRequiredRoot, GroupRoot
+from h.traversal.group_membership import (
+    GroupMembershipContext,
+    group_membership_api_factory,
+)
 from h.traversal.organization import OrganizationContext, OrganizationRoot
 from h.traversal.user import UserByIDRoot, UserByNameRoot, UserContext, UserRoot
 
@@ -77,4 +81,6 @@
     "UserByIDRoot",
     "UserRoot",
     "GroupContext",
+    "GroupMembershipContext",
+    "group_membership_api_factory",
 )

h/traversal/group_membership.py

@@ -0,0 +1,51 @@
+from dataclasses import dataclass
+
+from pyramid.httpexceptions import HTTPNotFound
+
+from h.exceptions import InvalidUserId
+from h.models import Group, GroupMembership, User
+
+
+@dataclass
+class GroupMembershipContext:
+    group: Group
+    user: User
+    membership: GroupMembership | None
+
+
+def group_membership_api_factory(request) -> GroupMembershipContext:
+    user_service = request.find_service(name="user")
+    group_service = request.find_service(name="group")
+    group_members_service = request.find_service(name="group_members")
+
+    userid = request.matchdict["userid"]
+    pubid = request.matchdict["pubid"]
+
+    def get_user() -> User | None:
+        if userid == "me":
+            if request.authenticated_userid:
+                return user_service.fetch(request.authenticated_userid)
+
+            return None
+
+        try:
+            return user_service.fetch(userid)
+        except InvalidUserId:
+            return None
+
+    user = get_user()
+
+    if not user:
+        raise HTTPNotFound(f"User not found: {userid}")
+
+    group = group_service.fetch(pubid)
+
+    if not group:
+        raise HTTPNotFound(f"Group not found: {pubid}")
+
+    membership = group_members_service.get(group, user)
+
+    if not membership and request.method != "POST":
+        raise HTTPNotFound(f"Membership not found: ({pubid}, {userid})")
+
+    return GroupMembershipContext(group=group, user=user, membership=membership)

h/views/api/groups.py

@@ -1,16 +1,10 @@
-from pyramid.httpexceptions import (
-    HTTPBadRequest,
-    HTTPConflict,
-    HTTPNoContent,
-    HTTPNotFound,
-)
+from pyramid.httpexceptions import HTTPConflict, HTTPNoContent, HTTPNotFound
 
-from h.exceptions import InvalidUserId
 from h.i18n import TranslationString as _
 from h.presenters import GroupJSONPresenter, GroupsJSONPresenter, UserJSONPresenter
 from h.schemas.api.group import CreateGroupAPISchema, UpdateGroupAPISchema
 from h.security import Permission
-from h.traversal import GroupContext
+from h.traversal import GroupContext, GroupMembershipContext
 from h.views.api.config import api_config
 from h.views.api.exceptions import PayloadError
 
@@ -161,17 +155,13 @@ def read_members(context: GroupContext, _request):
     link_name="group.member.delete",
     description="Remove the current user from a group",
     is_authenticated=True,
+    permission=Permission.Group.MEMBER_REMOVE,
 )
-def remove_member(context: GroupContext, request):
+def remove_member(context: GroupMembershipContext, request):
     """Remove a member from the given group."""
-    # Currently, we only support removing the requesting user
-    if request.matchdict.get("userid") == "me":
-        userid = request.authenticated_userid
-    else:
-        raise HTTPBadRequest('Only the "me" user value is currently supported')
 
     group_members_service = request.find_service(name="group_members")
-    group_members_service.member_leave(context.group, userid)
+    group_members_service.member_leave(context.group, context.user.userid)
 
     return HTTPNoContent()
 
@@ -184,28 +174,19 @@ def remove_member(context: GroupContext, request):
     permission=Permission.Group.MEMBER_ADD,
     description="Add the user in the request params to a group.",
 )
-def add_member(context: GroupContext, request):
+def add_member(context: GroupMembershipContext, request):
     """
     Add a member to a given group.
 
     :raise HTTPNotFound: if the user is not found or if the use and group
       authorities don't match.
     """
-    user_svc = request.find_service(name="user")
     group_members_svc = request.find_service(name="group_members")
 
-    try:
-        user = user_svc.fetch(request.matchdict["userid"])
-    except InvalidUserId as err:
-        raise HTTPNotFound() from err
-
-    if user is None:
-        raise HTTPNotFound()
-
-    if user.authority != context.group.authority:
+    if context.user.authority != context.group.authority:
         raise HTTPNotFound()
 
-    group_members_svc.member_join(context.group, user.userid)
+    group_members_svc.member_join(context.group, context.user.userid)
 
     return HTTPNoContent()