Skip to content

Commit 0193fa3

Browse files
committed
feat(permissions): add RDM_ALLOW_OWNERS_REMOVE_COMMUNITY_FROM_RECORD
* add ``RDM_ALLOW_OWNERS_REMOVE_COMMUNITY_FROM_RECORD`` configuration variable to control whether record owners can remove communities from records. When set to False, only community curators, managers, and owners can remove communities. * closes CERNDocumentServer/cds-rdm#630
1 parent 79afb27 commit 0193fa3

File tree

6 files changed

+88
-14
lines changed

6 files changed

+88
-14
lines changed

invenio_rdm_records/config.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,16 @@ def always_valid(identifier):
273273
"""Enforces at least one community per record."""
274274
RDM_COMMUNITY_INCLUSION_REQUEST_CLS = CommunityInclusion
275275
"""Request type for record inclusion requests."""
276+
RDM_ALLOW_OWNERS_REMOVE_COMMUNITY_FROM_RECORD = True
277+
"""Allow record owners to remove communities from records.
278+
279+
When set to False, only community curators, managers, and owners can remove
280+
communities from records. This applies to published records.
281+
Record owners will still be able to add communities, but removal is restricted
282+
to community curators, managers, and owners.
283+
284+
Default: True (backwards compatible - owners can remove communities)
285+
"""
276286

277287
#
278288
# Search configuration

invenio_rdm_records/services/communities/service.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,10 @@ def _remove(self, identity, community, record, valid_data, errors, uow):
217217

218218
try:
219219
self.require_permission(
220-
identity, "remove_community", record=record, community_id=community_id
220+
identity,
221+
"remove_community_from_record",
222+
record=record,
223+
community_id=community_id,
221224
)
222225
# By default, admin/superuser has permission to do everything,
223226
# so PermissionDeniedError won't be raised for admin in any case
@@ -233,7 +236,7 @@ def _remove(self, identity, community, record, valid_data, errors, uow):
233236

234237
# Run components
235238
self.run_components(
236-
"remove_community",
239+
"remove_community_from_record",
237240
identity,
238241
record=record,
239242
community=community,

invenio_rdm_records/services/permissions.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -251,21 +251,23 @@ class RDMRecordPermissionPolicy(RecordPermissionPolicy):
251251
# Who can add record to a community
252252
can_add_community = can_manage
253253
# Who can remove a community from a record
254-
can_remove_community_ = [
255-
RecordOwners(),
256-
CommunityCurators(),
257-
SystemProcess(),
254+
can_remove_community_from_record_ = [
255+
IfConfig(
256+
"RDM_ALLOW_OWNERS_REMOVE_COMMUNITY_FROM_RECORD",
257+
then_=[RecordOwners(), CommunityCurators(), SystemProcess()],
258+
else_=[CommunityCurators(), SystemProcess()],
259+
),
258260
]
259-
can_remove_community = [
261+
can_remove_community_from_record = [
260262
IfConfig(
261263
"RDM_COMMUNITY_REQUIRED_TO_PUBLISH",
262264
then_=[
263265
IfOneCommunity(
264266
then_=[Administration(), SystemProcess()],
265-
else_=can_remove_community_,
267+
else_=can_remove_community_from_record_,
266268
),
267269
],
268-
else_=can_remove_community_,
270+
else_=can_remove_community_from_record_,
269271
),
270272
]
271273
# Who can remove records from a community

tests/resources/test_resources_communities.py

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ def test_invalid_record_or_draft(
569569
assert response.status_code == 404
570570

571571

572-
def test_remove_community(
572+
def test_remove_community_from_record(
573573
client, uploader, curator, record_community, headers, community
574574
):
575575
"""Test removal of a community from the record."""
@@ -1110,3 +1110,60 @@ def test_remove_record_last_community_with_multiple_communities(
11101110
record_saved = client.get(f"/records/{record_pid}", headers=headers)
11111111
# check that only one community ID is associated with the record
11121112
assert len(record_saved.json["parent"]["communities"]["ids"]) == 1
1113+
1114+
1115+
def test_remove_community_from_record_restricted_to_curators(
1116+
db, client, uploader, record_community, headers, community, monkeypatch
1117+
):
1118+
"""Test that record owners cannot remove communities when config is disabled."""
1119+
from flask import current_app
1120+
1121+
# Create a record with a community as uploader (record owner)
1122+
client_uploader = uploader.login(client)
1123+
record = record_community.create_record()
1124+
1125+
# Verify the community is added
1126+
record_data = client_uploader.get(
1127+
f"/records/{record.pid.pid_value}", headers=headers
1128+
)
1129+
assert community.id in record_data.json["parent"]["communities"]["ids"]
1130+
1131+
# Try to remove community as owner - should succeed with default config
1132+
data = {"communities": [{"id": community.id}]}
1133+
response = client_uploader.delete(
1134+
f"/records/{record.pid.pid_value}/communities",
1135+
headers=headers,
1136+
json=data,
1137+
)
1138+
assert response.status_code == 200
1139+
assert not response.json.get("errors")
1140+
1141+
# Re-add the community for next test
1142+
record = _add_to_community(db, record, community)
1143+
1144+
# Now disable the config - owners should NOT be able to remove
1145+
monkeypatch.setitem(
1146+
current_app.config, "RDM_ALLOW_OWNERS_REMOVE_COMMUNITY_FROM_RECORD", False
1147+
)
1148+
1149+
# Try to remove community as owner - should fail
1150+
response = client_uploader.delete(
1151+
f"/records/{record.pid.pid_value}/communities",
1152+
headers=headers,
1153+
json=data,
1154+
)
1155+
1156+
# Should get errors (permission denied for owner)
1157+
assert response.status_code == 400
1158+
assert len(response.json.get("errors", [])) > 0
1159+
1160+
# Verify community is still there
1161+
record_data = client_uploader.get(
1162+
f"/records/{record.pid.pid_value}", headers=headers
1163+
)
1164+
assert community.id in record_data.json["parent"]["communities"]["ids"]
1165+
1166+
# Reset config
1167+
monkeypatch.setitem(
1168+
current_app.config, "RDM_ALLOW_OWNERS_REMOVE_COMMUNITY_FROM_RECORD", True
1169+
)

tests/resources/test_resources_community_records.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ def service():
1818
return current_community_records_service
1919

2020

21-
def test_remove_community(client, curator, record_community, headers, community):
21+
def test_remove_community_from_record(
22+
client, curator, record_community, headers, community
23+
):
2224
"""Remove a record from the community."""
2325
client = curator.login(client)
2426
record = record_community.create_record()

tests/services/test_service_record_communities.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,14 @@ def add_community(self, identity, record, communities, **kwargs):
9898
assert "dummy_id" not in [c["community_id"] for c in requests]
9999

100100

101-
def test_remove_community_component_called(
101+
def test_remove_community_from_record_component_called(
102102
db, community, community2, uploader, record_factory, set_app_config_fn_scoped
103103
):
104-
"""The component `remove_community` method is called and blocks the removal."""
104+
"""The component `remove_community_from_record` method is called and blocks the removal."""
105105
test_community_id = community.id
106106

107107
class MockRemoveComponent(ServiceComponent):
108-
def remove_community(
108+
def remove_community_from_record(
109109
self, identity, record, community, valid_data, errors, **kwargs
110110
):
111111
if community["id"] == str(community2.id):

0 commit comments

Comments
 (0)