Merge e4b4788 into d577541

b2share/modules/communities/models.py

@@ -91,6 +91,8 @@ def receive_before_insert(mapper, connection, target):
     """Create community admin and member roles and add their permissions."""
     from b2share.modules.deposit.permissions import (
         create_deposit_need_factory, read_deposit_need_factory,
+        update_deposit_metadata_need_factory,
+        update_deposit_publication_state_need_factory,
     )
     from b2share.modules.deposit.api import PublicationStates
 
@@ -117,6 +119,22 @@ def receive_before_insert(mapper, connection, target):
             community=str(target.id),
             publication_state=PublicationStates.published.name
         ),
+        update_deposit_metadata_need_factory(
+            community=str(target.id),
+            publication_state=PublicationStates.submitted.name
+        ),
+        # permission to publish a submission
+        update_deposit_publication_state_need_factory(
+            community=str(target.id),
+            old_state=PublicationStates.submitted.name,
+            new_state=PublicationStates.published.name
+        ),
+        # permission to ask the owners to fix a submission before resubmitting
+        update_deposit_publication_state_need_factory(
+            community=str(target.id),
+            old_state=PublicationStates.submitted.name,
+            new_state=PublicationStates.draft.name
+        )
     ]
     for need in member_needs:
         db.session.add(ActionRoles.allow(need, role=member_role))

b2share/modules/deposit/permissions.py

@@ -45,6 +45,8 @@
 from b2share.modules.communities.api import Community
 from invenio_db import db
 
+from .api import PublicationStates
+
 
 def _deposit_need_factory(name, **kwargs):
     if kwargs:
@@ -66,13 +68,29 @@ def create_deposit_need_factory(community=None, publication_state='draft'):
                                  publication_state=publication_state)
 
 
-def read_deposit_need_factory(community=None, publication_state='draft'):
+def read_deposit_need_factory(community, publication_state):
     # FIXME: check that the community_id and publication_state exist
     return _deposit_need_factory('read-deposit',
                                  community=community,
                                  publication_state=publication_state)
 
 
+def update_deposit_publication_state_need_factory(community, old_state,
+                                                  new_state):
+    # FIXME: check that the community_id and publication states exist
+    return _deposit_need_factory('update-deposit-publication-state',
+                                 community=community,
+                                 old_state=old_state,
+                                 new_state=new_state)
+
+
+def update_deposit_metadata_need_factory(community, publication_state):
+    # FIXME: check that the community_id and publication state exist
+    return _deposit_need_factory('update-deposit-metadata',
+                                 community=community,
+                                 publication_state=publication_state)
+
+
 ReadableCommunities = namedtuple('ReadableCommunities', ['all', 'communities'])
 
 
@@ -134,7 +152,6 @@ def __init__(self, record=None):
 
             self.permissions.add(StrictDynamicPermission(*needs))
 
-
     def allows(self, *args, **kwargs):
         # allowed if the data is not loaded yet
         if self.record is None:
@@ -185,6 +202,41 @@ def _load_additional_permissions(self):
         self.permissions.add(permission)
 
 
+class UpdateDepositMetadataPermission(StrictDynamicPermission):
+    """Permissio to update a deposit's metadata fields."""
+
+    def __init__(self, deposit, new_state=None):
+        """Constructor
+
+        Args:
+            deposit (Deposit): deposit which is modified.
+            new_state (str): new publication state of the deposit
+                if applicable.
+        """
+        super(UpdateDepositMetadataPermission, self).__init__()
+        # Owners are allowed to update
+        for owner_id in deposit['_deposit']['owners']:
+            self.explicit_needs.add(UserNeed(owner_id))
+
+        # authorize if the user can modify metadata in the old
+        # publication state
+        self.explicit_needs.add(
+            update_deposit_metadata_need_factory(
+                community=deposit['community'],
+                publication_state=deposit['publication_state']
+            )
+        )
+        # authorize if the user can modify metadata in the new
+        # publication state
+        if new_state != deposit['publication_state']:
+            self.explicit_needs.add(
+                update_deposit_metadata_need_factory(
+                    community=deposit['community'],
+                    publication_state=new_state
+                )
+            )
+
+
 class UpdateDepositPermission(DepositPermission):
     """Deposit update permission."""
 
@@ -216,36 +268,41 @@ def _load_additional_permissions(self):
         if (new_deposit is not None and new_deposit['publication_state']
                 != self.deposit['publication_state']):
             state_permission = StrictDynamicPermission()
-            # state_permission.needs.add(ParameterizedActionNeed(
-            #     'deposit-publication-state-transition-{0}-{1}'.format(
-            #         self.deposit['publication_state'],
-            #         new_deposit['publication_state'])
-            #     , self.deposit['community'])
-            # )
-            # User can change the state
-            # FIXME: Disable later user pulication when it is not allowed
-            for owner_id in self.deposit['_deposit']['owners']:
-                state_permission.needs.add(UserNeed(owner_id))
+            state_permission.explicit_needs.add(
+                update_deposit_publication_state_need_factory(
+                    community=self.deposit['community'],
+                    old_state=self.deposit['publication_state'],
+                    new_state=new_deposit['publication_state']
+                )
+            )
+            # Owners of a record can always "submit" it.
+            if (self.deposit['publication_state'] ==
+                PublicationStates.draft.name and
+                new_deposit['publication_state'] ==
+                PublicationStates.submitted.name or
+                # Owners have also the right to move the record from submitted
+                # to draft again.
+                self.deposit['publication_state'] ==
+                PublicationStates.submitted.name and
+                new_deposit['publication_state'] ==
+                PublicationStates.draft.name):
+                # Owners are allowed to update
+                for owner_id in self.deposit['_deposit']['owners']:
+                    state_permission.explicit_needs.add(UserNeed(owner_id))
             permissions.append(state_permission)
 
         # Create permission for updating generic metadata fields.
         # Only superadmin can modify published draft.
         if self.deposit['publication_state'] != 'published':
+            new_state = new_deposit['publication_state']
             # Check if any metadata has been changed
             del new_deposit['publication_state']
             original_metadata = deepcopy(self.deposit)
             del original_metadata['publication_state']
             if original_metadata != new_deposit:
-                metadata_permission = StrictDynamicPermission()
-                # Owners are allowed to update
-                for owner_id in self.deposit['_deposit']['owners']:
-                    metadata_permission.needs.add(UserNeed(owner_id))
-                # metadata_permission.needs.add(ParameterizedActionNeed(
-                #     'update-deposit-{}'.format(
-                #         self.deposit['publication_state'])
-                #     , self.deposit['community'])
-                # )
-                permissions.append(metadata_permission)
+                permissions.append(
+                    UpdateDepositMetadataPermission(self.deposit, new_state)
+                )
 
         if len(permissions) > 1:
             self.permissions.add(AndPermissions(*permissions))
@@ -257,13 +314,9 @@ class DeleteDepositPermission(DepositPermission):
     """Deposit delete permission."""
 
     def _load_additional_permissions(self):
-        permission = DynamicPermission()
+        permission = StrictDynamicPermission()
         # owners can delete the deposit if it is not published
         if not 'pid' in self.deposit['_deposit']:
             for owner_id in self.deposit['_deposit']['owners']:
                 permission.needs.add(UserNeed(owner_id))
-        permission.needs.add(ParameterizedActionNeed(
-            'delete-deposit-{}'.format(self.deposit['publication_state']),
-            self.deposit['community'])
-        )
         self.permissions.add(permission)

b2share/modules/files/permissions.py

@@ -126,9 +126,20 @@ def _load_additional_permissions(self):
             self.permissions.clear()
             self.permissions.add(DenyAllPermission)
         else:
+            from b2share.modules.deposit.permissions import (
+                ReadDepositPermission, UpdateDepositMetadataPermission
+            )
             # all actions are granted to the owner
-            for owner_id in self.record.get('_deposit', {}).get('owners', []):
-                self.permissions.add(Permission(UserNeed(owner_id)))
+            if self.action in {'bucket-read', 'object-read'}:
+                # A user can read the files if he can read the deposit
+                self.permissions.add(
+                    ReadDepositPermission(self.record)
+                )
+            if self.action in {'bucket-update', 'object-delete'}:
+                # A user can modify the files if he can modify the metadata
+                self.permissions.add(
+                    UpdateDepositMetadataPermission(self.record)
+                )
 
 
 class PublicationFilesPermission(RecordFilesPermission):

tests/b2share_functional_tests/test_search.py

@@ -31,18 +31,26 @@
 from invenio_search import current_search
 from flask import url_for as flask_url_for
 from invenio_db import db
-from b2share_unit_tests.helpers import create_user
+from b2share.modules.communities.api import Community
+from b2share_unit_tests.helpers import (
+    create_user, generate_record_data
+)
 
 
 def test_make_record_with_no_file_and_search(app, test_communities,
-                                             login_user, test_records_data):
+                                             login_user):
     '''Test for issue https://github.com/EUDAT-B2SHARE/b2share/issues/1073'''
     def url_for(*args, **kwargs):
         with app.app_context():
             return flask_url_for(*args, **kwargs)
 
     with app.app_context():
+        community_name = 'MyTestCommunity1'
+        record_data = generate_record_data(community=community_name)
+
         allowed_user = create_user('allowed')
+        community = Community.get(name=community_name)
+        com_admin = create_user('com_admin', roles=[community.admin_role])
         db.session.commit()
 
     with app.test_client() as client:
@@ -51,7 +59,6 @@ def url_for(*args, **kwargs):
                    ('Accept', 'application/json')]
         patch_headers = [('Content-Type', 'application/json-patch+json'),
                          ('Accept', 'application/json')]
-        record_data = test_records_data[0]
 
         # create record without files
         draft_create_res = client.post(
@@ -72,6 +79,8 @@ def url_for(*args, **kwargs):
             headers=patch_headers)
         assert draft_submit_res.status_code == 200
 
+    with app.test_client() as client:
+        login_user(com_admin, client)
         # publish record
         draft_publish_res = client.patch(
             url_for('b2share_deposit_rest.b2share_deposit_item',

tests/b2share_functional_tests/test_submission.py

@@ -29,7 +29,8 @@
 from io import BytesIO
 
 import pytest
-from b2share_unit_tests.helpers import create_user
+from b2share.modules.communities.api import Community
+from b2share_unit_tests.helpers import create_user, generate_record_data
 from b2share.modules.deposit.api import PublicationStates
 from invenio_search import current_search
 from flask import url_for as flask_url_for
@@ -39,8 +40,7 @@
 
 
 @pytest.mark.parametrize('authentication', [('user/password'), ('oauth')])
-def test_deposit(app, test_communities, login_user,
-                 test_records_data, authentication):
+def test_deposit(app, test_communities, login_user, authentication):
     """Test record submission with classic login and access token."""
     with app.app_context():
         allowed_user = create_user('allowed')
@@ -63,20 +63,34 @@ def test_deposit(app, test_communities, login_user,
         other_headers = [('Authorization',
                           'Bearer {}'.format(other_token.access_token))]
 
+        community_name = 'MyTestCommunity1'
+        community = Community.get(name=community_name)
+        com_admin = create_user('com_admin', roles=[community.admin_role])
+        com_admin_token = Token.create_personal(
+            'com_admin_token', com_admin.id,
+            scopes=[s[0] for s in scopes]
+        )
+        # application authentication token header
+        com_admin_headers = [('Authorization',
+                              'Bearer {}'.format(com_admin_token.access_token))]
+
+        test_records_data = [generate_record_data(community=community_name)
+                             for idx in range(1,3)]
+
         db.session.commit()
 
     if authentication == 'user/password':
         subtest_deposit(app, test_communities, allowed_user, other_user,
-                        [], [], login_user, test_records_data)
+                        com_admin, [], [], [], login_user, test_records_data)
     else:
         subtest_deposit(app, test_communities, allowed_user, other_user,
-                        allowed_headers, other_headers, lambda u, c: 42,
-                        test_records_data)
+                        com_admin, allowed_headers, other_headers,
+                        com_admin_headers, lambda u, c: 42, test_records_data)
 
 
 def subtest_deposit(app, test_communities, allowed_user, other_user,
-                    allowed_headers, other_headers, login_user,
-                    test_records_data):
+                    com_admin, allowed_headers, other_headers,
+                    com_admin_headers, login_user, test_records_data):
     def url_for(*args, **kwargs):
         """Generate url using flask.url_for and the current app ctx.
 
@@ -100,15 +114,17 @@ def test_files(client, bucket_link, uploaded_files):
             uploaded_content = uploaded_files[draft_file['key']]
             assert draft_file['size'] == len(uploaded_content)
 
-    with app.test_client() as client:
-        login_user(allowed_user, client)
-        headers = [('Content-Type', 'application/json'),
-                   ('Accept', 'application/json')] + allowed_headers
-        patch_headers = [('Content-Type', 'application/json-patch+json'),
-                         ('Accept', 'application/json')] + allowed_headers
-
-        created_records = {}  # dict of created records
-        for record_data in test_records_data:
+    headers = [('Content-Type', 'application/json'),
+                ('Accept', 'application/json')] + allowed_headers
+    patch_headers = [('Content-Type', 'application/json-patch+json'),
+                        ('Accept', 'application/json')] + allowed_headers
+    publish_headers = [('Content-Type', 'application/json-patch+json'),
+                       ('Accept', 'application/json')] + com_admin_headers
+
+    created_records = {}  # dict of created records
+    for record_data in test_records_data:
+        with app.test_client() as client:
+            login_user(allowed_user, client)
             record_list_url = (
                 lambda **kwargs:
                 url_for('b2share_records_rest.b2share_record_list',
@@ -179,6 +195,8 @@ def test_files(client, bucket_link, uploaded_files):
                 headers=patch_headers)
             assert draft_submit_res.status_code == 200
 
+        with app.test_client() as client:
+            login_user(com_admin, client)
             # test draft publish
             draft_publish_res = client.patch(
                 url_for('b2share_deposit_rest.b2share_deposit_item',
@@ -187,7 +205,7 @@ def test_files(client, bucket_link, uploaded_files):
                     "op": "replace", "path": "/publication_state",
                     "value": PublicationStates.published.name
                 }]),
-                headers=patch_headers)
+                headers=publish_headers)
 
             assert draft_publish_res.status_code == 200
             draft_publish_data = json.loads(