Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

permissions: records always in community feature #1719

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,6 @@ Pipfile.lock
# pip wheel
pip-wheel-metadata/

# node_modules
node_modules
# node modules
**/*/node_modules
node_modules
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This file is part of Invenio-RDM-Records
// Copyright (C) 2020-2023 CERN.
// Copyright (C) 2020-2022 Northwestern University.
// Copyright (C) 2023 KTH Royal Institute of Technology.
//
// Invenio-RDM-Records is free software; you can redistribute it and/or modify it
// under the terms of the MIT License; see LICENSE file for more details.
Expand Down Expand Up @@ -40,10 +41,13 @@ class PublishButtonComponent extends Component {
this.closeConfirmModal();
};

isDisabled = (values, isSubmitting, numberOfFiles) => {
isDisabled = (values, isSubmitting, numberOfFiles, permissions) => {
const canPublish = permissions?.can_publish ?? false;
const publishWithCommunity = permissions?.can_publish_always_in_community ?? false;
const hasNoPermission = !canPublish && publishWithCommunity;
const filesEnabled = _get(values, "files.enabled", false);
const filesMissing = filesEnabled && !numberOfFiles;
return isSubmitting || filesMissing;
return isSubmitting || filesMissing || hasNoPermission;
};

render() {
Expand All @@ -54,6 +58,7 @@ class PublishButtonComponent extends Component {
publishWithoutCommunity,
formik,
publishModalExtraContent,
permissions,
...ui
} = this.props;
const { isConfirmModalOpen } = this.state;
Expand All @@ -64,7 +69,7 @@ class PublishButtonComponent extends Component {
return (
<>
<Button
disabled={this.isDisabled(values, isSubmitting, numberOfFiles)}
disabled={this.isDisabled(values, isSubmitting, numberOfFiles, permissions)}
name="publish"
onClick={this.openConfirmModal}
positive
Expand Down Expand Up @@ -126,19 +131,22 @@ PublishButtonComponent.propTypes = {
numberOfFiles: PropTypes.number.isRequired,
formik: PropTypes.object.isRequired,
publishModalExtraContent: PropTypes.string,
permissions: PropTypes.object,
};

PublishButtonComponent.defaultProps = {
buttonLabel: i18next.t("Publish"),
publishWithoutCommunity: false,
actionState: undefined,
publishModalExtraContent: undefined,
permissions: {},
};

const mapStateToProps = (state) => ({
actionState: state.deposit.actionState,
numberOfFiles: Object.values(state.files.entries).length,
publishModalExtraContent: state.deposit.config.publish_modal_extra,
permissions: state.deposit.permissions,
});

export const PublishButton = connect(
Expand Down
7 changes: 7 additions & 0 deletions invenio_rdm_records/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# Copyright (C) 2019 Northwestern University.
# Copyright (C) 2021-2023 Graz University of Technology.
# Copyright (C) 2023 TU Wien.
# Copyright (C) 2023 KTH Royal Institute of Technology.
#
# Invenio-RDM-Records is free software; you can redistribute it and/or modify
# it under the terms of the MIT License; see LICENSE file for more details.
Expand Down Expand Up @@ -131,6 +132,12 @@ def always_valid(identifier):
RDM_ALLOW_RESTRICTED_RECORDS = True
"""Allow users to set restricted/private records."""

#
# Record communities
#
RDM_RECORD_ALWAYS_IN_COMMUNITY = False
"""Enforces at least one community per record on remove community function."""

#
# Search configuration
#
Expand Down
12 changes: 12 additions & 0 deletions invenio_rdm_records/services/generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# Copyright (C) 2021 Graz University of Technology.
# Copyright (C) 2021 CERN.
# Copyright (C) 2021 TU Wien.
# Copyright (C) 2023 KTH Royal Institute of Technology.
#
# Invenio-RDM-Records is free software; you can redistribute it and/or modify
# it under the terms of the MIT License; see LICENSE file for more details.
Expand Down Expand Up @@ -127,6 +128,17 @@ def _condition(self, record=None, **kwargs):
return record is None


class IfOneCommunity(ConditionalGenerator):
"""Conditional generator for records always in communities case."""

def _condition(self, record=None, **kwargs):
"""Check if the record is associated with more than one community."""
if record is None:
return True
rec_communities = record.parent.communities.ids
return len(rec_communities) == 1


class IfExternalDOIRecord(ConditionalGenerator):
"""Conditional generator for external DOI records."""

Expand Down
29 changes: 27 additions & 2 deletions invenio_rdm_records/services/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
# Copyright (C) 2019-2024 CERN.
# Copyright (C) 2019 Northwestern University.
# Copyright (C) 2023 TU Wien.
# Copyright (C) 2023 KTH Royal Institute of Technology.
#
# Invenio-RDM-Records is free software; you can redistribute it and/or modify
# it under the terms of the MIT License; see LICENSE file for more details.

"""Permissions for Invenio RDM Records."""
from invenio_administration.generators import Administration
from invenio_communities.generators import CommunityCurators
from invenio_communities.permissions import CommunityMembers
from invenio_records_permissions.generators import (
AnyUser,
AuthenticatedUser,
Expand All @@ -34,6 +36,7 @@
IfExternalDOIRecord,
IfFileIsLocal,
IfNewRecord,
IfOneCommunity,
IfRecordDeleted,
IfRequestType,
IfRestricted,
Expand Down Expand Up @@ -197,8 +200,21 @@ class RDMRecordPermissionPolicy(RecordPermissionPolicy):
else_=[IfExternalDOIRecord(then_=[Disable()], else_=can_curate)],
),
]
can_publish_via_community = (
[RecordCommunitiesAction("curate")]
+ [SystemProcess()]
+ [CommunityMembers()]
+ [SecretLinks("edit")]
+ [SubmissionReviewer()]
)
# Allow publishing a new record or changes to an existing record.
can_publish = can_review
can_publish = [
IfConfig(
"RDM_RECORD_ALWAYS_IN_COMMUNITY",
then_=can_publish_via_community,
else_=can_review,
),
]
# Allow lifting a record or draft.
can_lift_embargo = can_manage

Expand All @@ -208,11 +224,20 @@ class RDMRecordPermissionPolicy(RecordPermissionPolicy):
# Who can add record to a community
can_add_community = can_manage
# Who can remove a community from a record
can_remove_community = [
can_remove_community_ = [
RecordOwners(),
CommunityCurators(),
SystemProcess(),
]
can_remove_community = [
IfConfig(
"RDM_RECORD_ALWAYS_IN_COMMUNITY",
then_=[
IfOneCommunity(then_=[SystemProcess()], else_=can_remove_community_)
],
else_=can_remove_community_,
)
]
# Who can remove records from a community
can_remove_record = [CommunityCurators()]
# Who can add records to a community in bulk
Expand Down
182 changes: 182 additions & 0 deletions tests/resources/test_resources_communities.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
# -*- coding: utf-8 -*-
#
# Copyright (C) 2023-2024 CERN.
# Copyright (C) 2023 KTH Royal Institute of Technology.
#
# Invenio-RDM-Records is free software; you can redistribute it and/or modify
# it under the terms of the MIT License; see LICENSE file for more details.

"""Tests record's communities resources."""

from contextlib import contextmanager
from copy import deepcopy

import pytest
Expand Down Expand Up @@ -747,3 +749,183 @@ def test_search_communities(
headers=headers,
)
assert response.status_code == 403


# Assure Records community exists tests
# -------------------------------------
from invenio_records_resources.services.errors import PermissionDeniedError


@contextmanager
def ensure_record_community_exists_config(app):
"""Context manager to ensure record community exists config
is set to True during a specific code block.

Parameters:
app: app fixture

Usage:
with ensure_record_community_exists_config(app):
# code block that requires the flag to be set"""
try:
app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"] = True
yield
finally:
app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"] = False


def test_restricted_record_creation(
app, record_community, uploader, curator, community_owner, test_user, superuser
):
"""Verify PermissionDeniedError is raised when direct publish a record"""
# You can directly publish a record when the config is disabled
rec = record_community.create_record()
assert rec.id
with ensure_record_community_exists_config(app):
# You can't directly publish
users = [
curator,
test_user,
uploader,
community_owner,
]
for user in users:
with pytest.raises(PermissionDeniedError):
record_community.create_record(uploader=user)

# Super user can!
super_user_rec = record_community.create_record(uploader=superuser)
assert super_user_rec.id


def test_remove_last_existing_non_existing_community(
app, client, uploader, record_community, headers, community
):
"""Test removal of an existing and non-existing community from the record,
while ensuring at least one community exists."""
data = {
"communities": [
{"id": "wrong-id"},
{"id": community.id},
{"id": "wrong-id2"},
]
}

client = uploader.login(client)
record = record_community.create_record()
with ensure_record_community_exists_config(app):
response = client.delete(
f"/records/{record.pid.pid_value}/communities",
headers=headers,
json=data,
)
assert response.status_code == 400
# Should get 3 errors: Can't remove community, 2 bad IDs
assert len(response.json["errors"]) == 3
record_saved = client.get(f"/records/{record.pid.pid_value}", headers=headers)
assert record_saved.json["parent"]["communities"]


def test_remove_last_community_api_error_handling(
record_community,
community,
uploader,
headers,
curator,
client,
app,
):
"""Testing error message when trying to remove last community."""
record = record_community.create_record()
data = {"communities": [{"id": community.id}]}
for user in [uploader, curator]:
client = user.login(client)
response = client.get(
f"/communities/{community.id}/records",
headers=headers,
json=data,
)
assert (
len(response.json["hits"]["hits"][0]["parent"]["communities"]["ids"]) == 1
)
with ensure_record_community_exists_config(app):
response = client.delete(
f"/records/{record.pid.pid_value}/communities",
headers=headers,
json=data,
)
assert response.is_json
assert response.status_code == 400
res_data = response.get_json()
assert len(res_data["errors"]) == 1
assert len(res_data["errors"][0]["community"]) > 1
assert len(res_data["errors"][0]["message"]) > 1
record_saved = client.get(
f"/records/{record.pid.pid_value}", headers=headers
)
assert record_saved.json["parent"]["communities"]

client = user.logout(client)
# check communities number
response = client.get(
f"/communities/{community.id}/records",
headers=headers,
json=data,
)
assert (
len(response.json["hits"]["hits"][0]["parent"]["communities"]["ids"])
== 1
)


def test_remove_record_last_community_with_multiple_communities(
closed_review_community,
open_review_community,
record_community,
community2,
uploader,
headers,
client,
app,
db,
):
"""Testing correct removal of multiple communities"""
client = uploader.login(client)

record = record_community.create_record()
comm = [
community2,
open_review_community,
closed_review_community,
] # one more in the rec fixuture so it's 4
for com in comm:
_add_to_community(db, record, com)
assert len(record.parent.communities.ids) == 4

with ensure_record_community_exists_config(app):
data = {"communities": [{"id": x} for x in record.parent.communities.ids]}

response = client.delete(
f"/records/{record.pid.pid_value}/communities",
headers=headers,
json=data,
)
# You get res 200 with error msg if all communities you are deleting
assert response.status_code == 200
assert "error" in str(response.data)

rec_com_left = client.get(f"/records/{record.pid.pid_value}", headers=headers)
assert len(rec_com_left.json["parent"]["communities"]["ids"]) == 1

# You get res 400 with error msg if you Delete the last one only.
response = client.delete(
f"/records/{record.pid.pid_value}/communities",
headers=headers,
json={"communities": [{"id": str(record.parent.communities.ids[0])}]},
)
assert response.status_code == 400
assert "error" in str(response.data)

record_saved = client.get(f"/records/{record.pid.pid_value}", headers=headers)
# check that only one community ID is associated with the record
assert len(record_saved.json["parent"]["communities"]["ids"]) == 1