Skip to content

Commit 6149e3a

Browse files
committed
Show hidden annotations to group moderators
1 parent 179c7b6 commit 6149e3a

File tree

7 files changed

+267
-146
lines changed

7 files changed

+267
-146
lines changed

h/search/core.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,9 @@ def __init__(
5050
query.Limiter(),
5151
query.DeletedFilter(),
5252
query.AuthFilter(request),
53-
query.GroupFilter(request),
54-
query.UserFilter(),
5553
query.NipsaFilter(request),
56-
query.HiddenFilter(request),
54+
query.GroupAndModerationFilter(request),
55+
query.UserFilter(),
5756
query.AnyMatcher(),
5857
query.TagsMatcher(),
5958
query.UriCombinedWildcardFilter(

h/search/query.py

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,22 @@
1-
from datetime import datetime as dt
1+
from datetime import UTC, datetime
22

3-
from dateutil import tz
43
from dateutil.parser import parse
54
from elasticsearch_dsl import Q
65
from elasticsearch_dsl.query import SimpleQueryString
76

87
from h import storage
8+
from h.models import Group, User
99
from h.search.util import add_default_scheme, wildcard_uri_is_valid
10+
from h.security.identity import Identity
11+
from h.security.permissions import Permission
12+
from h.security.permits import identity_permits
1013
from h.util import uri
1114

1215
LIMIT_DEFAULT = 20
1316
# Elasticsearch requires offset + limit must be <= 10,000.
1417
LIMIT_MAX = 200
1518
OFFSET_MAX = 9800
16-
DEFAULT_DATE = dt(1970, 1, 1, 0, 0, 0, 0).replace(tzinfo=tz.tzutc()) # noqa: DTZ001
19+
DEFAULT_DATE = datetime(1970, 1, 1, 0, 0, 0, 0, tzinfo=UTC)
1720

1821

1922
def popall(multidict, key):
@@ -142,7 +145,7 @@ def _parse_date(str_value):
142145
except ValueError:
143146
try:
144147
date = parse(str_value, default=DEFAULT_DATE)
145-
return dt.timestamp(date) * 1000
148+
return datetime.timestamp(date) * 1000
146149

147150
except ValueError:
148151
pass
@@ -207,23 +210,57 @@ def __call__(self, search, params): # noqa: ARG002
207210
return search.filter("term", shared=True)
208211

209212

210-
class GroupFilter:
211-
"""
212-
Filter that limits which groups annotations are returned from.
213-
214-
This excludes annotations from groups that the user is not authorized to
215-
read or which are explicitly excluded by the search query.
216-
"""
217-
213+
class GroupAndModerationFilter:
218214
def __init__(self, request):
219215
self.user = request.user
220216
self.group_service = request.find_service(name="group")
221217

218+
@staticmethod
219+
def _is_moderator(user: User, group: Group) -> bool:
220+
from h.traversal import GroupContext
221+
222+
return identity_permits(
223+
identity=Identity.from_models(user=user),
224+
context=GroupContext(group),
225+
permission=Permission.Group.MODERATE,
226+
)
227+
222228
def __call__(self, search, params):
223-
# Remove parameter if passed, preventing it being passed to default query
224229
group_ids = popall(params, "group") or None
225-
groups = self.group_service.groupids_readable_by(self.user, group_ids)
226-
return search.filter("terms", group=groups)
230+
groups = self.group_service.groups_readable_by(self.user, group_ids)
231+
232+
if not groups:
233+
# We are assuming that there's always some group readable by the user (eg. __world__)
234+
# if for some reason that was not the case, we would return all annotations
235+
# let's explicitly raise an error in that case
236+
msg = "No groups readable by the user"
237+
raise ValueError(msg)
238+
239+
not_hidden = ~Q("term", hidden=True)
240+
241+
if not self.user:
242+
return search.filter(
243+
not_hidden & Q("terms", group=[g.pubid for g in groups])
244+
)
245+
246+
user_annotations = Q("term", user=self.user.userid.lower())
247+
query_clauses = []
248+
# If t he user is logged in and we are filtering by groups
249+
# we'll check for each group if we are a moderator
250+
for group in groups:
251+
group_annotations = Q("term", group=group.pubid)
252+
if self._is_moderator(self.user, group):
253+
# We don't filter out hidden annotations for moderators
254+
query_clauses.append(group_annotations)
255+
256+
else:
257+
# For non moderators we hide moderated annotations except the ones authored
258+
# by the user
259+
query_clauses.append(
260+
group_annotations & (not_hidden | user_annotations)
261+
)
262+
263+
return search.filter(Q("bool", should=query_clauses))
227264

228265

229266
class UriCombinedWildcardFilter:

h/security/permits.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
from pyramid.security import Allowed, Denied
22

33
from h.security.identity import Identity
4-
from h.security.permission_map import PERMISSION_MAP
54

65

76
def identity_permits(
@@ -17,6 +16,8 @@ def identity_permits(
1716
:param context: Context object representing the objects acted upon
1817
:param permission: Permission requested
1918
"""
19+
from h.security.permission_map import PERMISSION_MAP
20+
2021
if clauses := PERMISSION_MAP.get(permission): # noqa: SIM102
2122
# Grant the permissions if for *any* single clause...
2223
if any(

h/services/group.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import sqlalchemy as sa
22
from sqlalchemy import literal_column, select, union
3+
from sqlalchemy.orm import aliased
34

4-
from h.models import Group, GroupMembership
5+
from h.models import Group, GroupMembership, User
56
from h.models.group import ReadableBy
67
from h.util import group as group_util
78

@@ -76,28 +77,26 @@ def filter_by_name(self, name=None):
7677
.order_by(Group.created.desc())
7778
)
7879

79-
def groupids_readable_by(self, user, group_ids=None):
80+
def groups_readable_by(self, user: User | None, group_ids=None) -> list[Group]:
8081
"""
81-
Return a list of pubids for which the user has read access.
82+
Return a list of groups for which the user has read access.
8283
8384
If the passed-in user is ``None``, this returns the list of
8485
world-readable groups.
8586
8687
If `group_ids` is specified, only the subset of groups from that list is
8788
returned. This is more efficient if the caller wants to know which
8889
groups from a specific list are readable by the user.
89-
90-
:type user: `h.models.user.User`
9190
"""
9291
# Very few groups are readable by world, query those separately
93-
readable_by_world_query = select(Group.pubid).where(
92+
readable_by_world_query = select(Group).where(
9493
Group.readable_by == ReadableBy.world
9594
)
9695
query = readable_by_world_query
9796

9897
if user is not None:
9998
user_is_member_query = (
100-
select(Group.pubid)
99+
select(Group)
101100
.join(GroupMembership)
102101
.where(
103102
GroupMembership.user_id == user.id,
@@ -106,14 +105,22 @@ def groupids_readable_by(self, user, group_ids=None):
106105
)
107106
# Union these with the world readable ones
108107
# We wrap a subquery around in case we need to apply the group_ids filter below
109-
query = select(
110-
union(readable_by_world_query, user_is_member_query).subquery()
111-
)
108+
union_groups = union(
109+
readable_by_world_query, user_is_member_query
110+
).subquery()
111+
# We explicitly alias the union to hint SQLAlchemy to fetch Group objects
112+
group_alias = aliased(Group, union_groups)
113+
query = select(group_alias)
112114

113115
if group_ids:
114116
query = query.where(literal_column("pubid").in_(group_ids))
115117

116-
return self.session.scalars(query).all()
118+
return self.session.scalars(query)
119+
120+
def groupids_readable_by(self, user, group_ids=None):
121+
"""Return a list of groupids for which the user has read access."""
122+
123+
return [g.pubid for g in self.groups_readable_by(user, group_ids)]
117124

118125
def groupids_created_by(self, user):
119126
"""

tests/unit/h/search/conftest.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,21 @@
22

33
import pytest
44

5+
from h.models import Group
56
from h.services.group import GroupService
67
from h.services.search_index import SearchIndexService
78

89

910
@pytest.fixture
10-
def group_service(pyramid_config):
11+
def world_group(db_session):
12+
return db_session.query(Group).filter_by(pubid="__world__").one()
13+
14+
15+
@pytest.fixture
16+
def group_service(pyramid_config, world_group):
1117
group_service = mock.create_autospec(GroupService, instance=True, spec_set=True)
1218
group_service.groupids_readable_by.return_value = ["__world__"]
19+
group_service.groups_readable_by.return_value = [world_group]
1320
pyramid_config.register_service(group_service, name="group")
1421
return group_service
1522

0 commit comments

Comments
 (0)