Skip to content

Commit

Permalink
subdivisions: fix moderator access bug
Browse files Browse the repository at this point in the history
* Solves a problem that prevented a moderator to see the deposits
  of his/her subdivision in the list of deposits.
* Closes rero#631.

Co-Authored-by: Miguel Moreira <miguel.moreira@rero.ch>
  • Loading branch information
mmo committed Nov 3, 2021
1 parent a043432 commit 1ed909d
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 5 deletions.
9 changes: 7 additions & 2 deletions sonar/modules/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,11 @@ def has_organisation(self, organisation_pid):

return False

@property
def subdivisions(self):
"""Getter for subdivisions."""
return self.get('subdivisions', [])

def has_subdivision(self, subdivision_pid):
"""Check if record belongs to the subdivision.
Expand All @@ -336,10 +341,10 @@ def has_subdivision(self, subdivision_pid):
return True

# No subdivision in record, the document is accessible.
if not self.get('subdivisions'):
if not self.subdivisions:
return False

for subdivision in self['subdivisions']:
for subdivision in self.subdivisions:
if subdivision_pid == subdivision['pid']:
return True

Expand Down
5 changes: 5 additions & 0 deletions sonar/modules/deposits/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ class DepositRecord(SonarRecord):
provider = DepositProvider
schema = 'deposits/deposit-v1.0.0.json'

@property
def subdivisions(self):
"""Getter for subdivisions."""
return self.get('diffusion', {}).get('subdivisions', [])

@classmethod
def create(cls, data, id_=None, dbcommit=False, with_bucket=True,
**kwargs):
Expand Down
1 change: 0 additions & 1 deletion sonar/modules/deposits/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ def read(cls, user, record):

# Special rules for moderators
if user.is_moderator:
user = user.replace_refs()

# Deposit does not belong to user's organisation
if current_organisation['pid'] != deposit['user']['organisation'][
Expand Down
16 changes: 14 additions & 2 deletions tests/api/deposits/test_deposits_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def test_create(client, deposit_json, bucket_location, superuser, admin,


def test_read(client, db, make_deposit, make_user, superuser, admin, moderator,
submitter, user, subdivision):
submitter, user, subdivision, subdivision2):
"""Test read deposits permissions."""
deposit1 = make_deposit('submitter', 'org')
deposit2 = make_deposit('submitter', 'org2')
Expand Down Expand Up @@ -171,10 +171,22 @@ def test_read(client, db, make_deposit, make_user, superuser, admin, moderator,
url_for('invenio_records_rest.depo_item', pid_value=deposit1['pid']))
assert res.status_code == 200

# Moderator has subdivision, I can read a deposit of my subdivision
sub_pid = deposit1["diffusion"]["subdivisions"][0]["$ref"].split('/')[-1]
moderator['subdivision'] = {
'$ref': f'https://sonar.ch/api/subdivisions/{sub_pid}'
}
moderator.commit()
moderator.reindex()
db.session.commit()
res = client.get(
url_for('invenio_records_rest.depo_item', pid_value=deposit1['pid']))
assert res.status_code == 200

# Moderator has subdivision, I cannot read deposit outside of his
# subdivision
moderator['subdivision'] = {
'$ref': f'https://sonar.ch/api/subdivisions/{subdivision["pid"]}'
'$ref': f'https://sonar.ch/api/subdivisions/{subdivision2["pid"]}'
}
moderator.commit()
moderator.reindex()
Expand Down
17 changes: 17 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,23 @@ def subdivision(app, db, es, admin, organisation, subdivision_json):
return subdivision


@pytest.fixture()
def subdivision2(app, db, es, admin, organisation, subdivision_json):
"""Second subdivision fixture."""
json = copy.deepcopy(subdivision_json)
json['organisation'] = {
'$ref':
'https://sonar.ch/api/organisations/{pid}'.format(
pid=organisation['pid'])
}

subdivision = SubdivisionRecord.create(json, dbcommit=True)
subdivision.commit()
subdivision.reindex()
db.session.commit()
return subdivision


@pytest.fixture()
def bucket_location(app, db):
"""Create a default location for managing files."""
Expand Down

0 comments on commit 1ed909d

Please # to comment.