Add update acls granularly to core code

lib/SampleService/core/samples.py

@@ -11,7 +11,7 @@
 from SampleService.core.arg_checkers import not_falsy as _not_falsy
 from SampleService.core.arg_checkers import check_timestamp as _check_timestamp
 from SampleService.core.acls import SampleAccessType as _SampleAccessType
-from SampleService.core.acls import SampleACL, SampleACLOwnerless
+from SampleService.core.acls import SampleACL, SampleACLOwnerless, SampleACLDelta
 from SampleService.core.core_types import PrimitiveType
 from SampleService.core.data_link import DataLink
 from SampleService.core.errors import (
@@ -240,6 +240,48 @@ def replace_sample_acls(
         if self._kafka:
             self._kafka.notify_sample_acl_change(id_)
 
+    def update_sample_acls(
+            self,
+            id_: UUID,
+            user: UserID,
+            update: SampleACLDelta,
+            as_admin: bool = False) -> None:
+        '''
+        Completely replace a sample's ACLs.
+
+        :param id_: the sample's ID.
+        :param user: the user changing the ACLs.
+        :param update: the ACL update. Note the update time is ignored. If the sample owner is
+            in any of the lists in the update, the update will fail.
+        :param as_admin: Skip ACL checks.
+        :raises NoSuchUserError: if any of the users in the ACLs do not exist.
+        :raises NoSuchSampleError: if the sample does not exist.
+        :raises UnauthorizedError: if the user does not have admin permission for the sample or
+            the request attempts to alter the owner.
+        :raises SampleStorageError: if the sample could not be retrieved.
+        '''
+        # could make yet another ACL class that's a delta w/o an update time - probably not
+        # worth it. If people get confused do it.
+        _not_falsy(id_, 'id_')
+        _not_falsy(user, 'user')
+        _not_falsy(update, 'update')
+        self._check_for_bad_users(_cast(List[UserID], []) + list(update.admin) +
+                                  list(update.write) + list(update.read) + list(update.remove))
+
+        self._check_perms(id_, user, _SampleAccessType.ADMIN, as_admin=as_admin)
+
+        update = SampleACLDelta(
+            self._now(),
+            update.admin,
+            update.write,
+            update.read,
+            update.remove,
+            update.public_read
+        )
+        self._storage.update_sample_acls(id_, update)
+        if self._kafka:
+            self._kafka.notify_sample_acl_change(id_)
+
     # TODO change owner. Probably needs a request/accept flow.
 
     def _check_for_bad_users(self, users: List[UserID]):

test/core/samples_test.py

@@ -6,7 +6,7 @@
 from unittest.mock import create_autospec
 
 from SampleService.core.storage.arango_sample_storage import ArangoSampleStorage
-from SampleService.core.acls import SampleACL, SampleACLOwnerless
+from SampleService.core.acls import SampleACL, SampleACLOwnerless, SampleACLDelta
 from SampleService.core.data_link import DataLink
 from SampleService.core.errors import (
     IllegalParameterError,
@@ -843,6 +843,250 @@ def _replace_sample_acls_fail(samples, id_, user, acls: SampleACLOwnerless, expe
     assert_exception_correct(got.value, expected)
 
 
+def test_update_sample_acls():
+    _update_sample_acls(UserID('someuser'), True)
+    _update_sample_acls(UserID('otheruser'), False)
+
+
+def _update_sample_acls(user: UserID, public_read):
+    storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
+    lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)
+    meta = create_autospec(MetadataValidatorSet, spec_set=True, instance=True)
+    ws = create_autospec(WS, spec_set=True, instance=True)
+    kafka = create_autospec(KafkaNotifier, spec_set=True, instance=True)
+    samples = Samples(storage, lu, meta, ws, kafka, now=nw,
+                      uuid_gen=lambda: UUID('1234567890abcdef1234567890abcdef'))
+    id_ = UUID('1234567890abcdef1234567890abcde0')
+
+    lu.invalid_users.return_value = []
+
+    storage.get_sample_acls.return_value = SampleACL(
+        u('someuser'),
+        dt(1),
+        [u('otheruser'), u('y')],
+        [u('anotheruser'), u('ur mum')],
+        [u('Fungus J. Pustule Jr.'), u('x')])
+
+    samples.update_sample_acls(id_, user, SampleACLDelta(
+        dt(-100), [u('x'), u('y')], [u('z'), u('a')], [u('b'), u('c')], [u('r'), u('q')],
+        public_read))
+
+    lu.invalid_users.assert_called_once_with(
+        [u(x) for x in ['x', 'y', 'a', 'z', 'b', 'c', 'q', 'r']])
+
+    storage.get_sample_acls.assert_called_once_with(UUID('1234567890abcdef1234567890abcde0'))
+
+    storage.update_sample_acls.assert_called_once_with(
+        UUID('1234567890abcdef1234567890abcde0'),
+        SampleACLDelta(
+            dt(6),
+            [u('x'), u('y')],
+            [u('z'), u('a')],
+            [u('b'), u('c')],
+            [u('r'), u('q')],
+            public_read))
+
+    kafka.notify_sample_acl_change.assert_called_once_with(
+        UUID('1234567890abcdef1234567890abcde0'))
+
+
+def test_update_sample_acls_as_admin_without_notifier():
+    '''
+    Also use None for public read.
+    '''
+    storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
+    lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)
+    meta = create_autospec(MetadataValidatorSet, spec_set=True, instance=True)
+    ws = create_autospec(WS, spec_set=True, instance=True)
+    samples = Samples(storage, lu, meta, ws, now=nw,
+                      uuid_gen=lambda: UUID('1234567890abcdef1234567890abcdef'))
+    id_ = UUID('1234567890abcdef1234567890abcde0')
+
+    lu.invalid_users.return_value = []
+
+    storage.get_sample_acls.return_value = SampleACL(
+        u('someuser'),
+        dt(1),
+        [u('otheruser'), u('y')],
+        [u('anotheruser'), u('ur mum')],
+        [u('Fungus J. Pustule Jr.'), u('x')])
+
+    samples.update_sample_acls(id_, UserID('someguy'), SampleACLDelta(
+        dt(-100), [u('x'), u('y')], [u('z'), u('a')], [u('b'), u('c')], [u('r'), u('q')],
+        None),
+        as_admin=True)
+
+    lu.invalid_users.assert_called_once_with(
+        [u(x) for x in ['x', 'y', 'a', 'z', 'b', 'c', 'q', 'r']])
+
+    storage.update_sample_acls.assert_called_once_with(
+        UUID('1234567890abcdef1234567890abcde0'),
+        SampleACLDelta(
+            dt(6),
+            [u('x'), u('y')],
+            [u('z'), u('a')],
+            [u('b'), u('c')],
+            [u('r'), u('q')],
+            None))
+
+
+def test_update_sample_acls_fail_bad_input():
+    storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
+    lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)
+    meta = create_autospec(MetadataValidatorSet, spec_set=True, instance=True)
+    ws = create_autospec(WS, spec_set=True, instance=True)
+    samples = Samples(
+        storage, lu, meta, ws, now=nw, uuid_gen=lambda: UUID('1234567890abcdef1234567890abcdef'))
+    id_ = UUID('1234567890abcdef1234567890abcde0')
+    u = UserID('u')
+
+    _update_sample_acls_fail(samples, None, u, SampleACLDelta(dt(1)), ValueError(
+        'id_ cannot be a value that evaluates to false'))
+    _update_sample_acls_fail(samples, id_, None, SampleACLDelta(dt(1)), ValueError(
+        'user cannot be a value that evaluates to false'))
+    _update_sample_acls_fail(samples, id_, u, None, ValueError(
+        'update cannot be a value that evaluates to false'))
+
+
+def test_update_sample_acls_fail_nonexistent_user_5_users():
+    storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
+    lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)
+    meta = create_autospec(MetadataValidatorSet, spec_set=True, instance=True)
+    ws = create_autospec(WS, spec_set=True, instance=True)
+    samples = Samples(
+        storage, lu, meta, ws, now=nw, uuid_gen=lambda: UUID('1234567890abcdef1234567890abcdef'))
+    id_ = UUID('1234567890abcdef1234567890abcde0')
+
+    lu.invalid_users.return_value = [u('whoo'), u('yay'), u('bugga'), u('w'), u('c')]
+
+    acls = SampleACLDelta(
+        dt(1),
+        [u('x'), u('whoo')],
+        [u('yay'), u('fwew')],
+        [u('y'), u('bugga'), u('z'), u('w'), u('c')],
+        [u('rem')])
+
+    _update_sample_acls_fail(
+        samples, id_, UserID('foo'), acls, NoSuchUserError('whoo, yay, bugga, w, c'))
+
+    lu.invalid_users.assert_called_once_with(
+        [u('whoo'), u('x'), u('fwew'), u('yay'), u('bugga'), u('c'), u('w'), u('y'), u('z'),
+         u('rem')])
+
+
+def test_update_sample_acls_fail_nonexistent_user_6_users():
+    storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
+    lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)
+    meta = create_autospec(MetadataValidatorSet, spec_set=True, instance=True)
+    ws = create_autospec(WS, spec_set=True, instance=True)
+    samples = Samples(
+        storage, lu, meta, ws, now=nw, uuid_gen=lambda: UUID('1234567890abcdef1234567890abcdef'))
+    id_ = UUID('1234567890abcdef1234567890abcde0')
+
+    lu.invalid_users.return_value = [u('whoo'), u('yay'), u('bugga'), u('w'), u('c'), u('whee')]
+
+    acls = SampleACLDelta(
+        dt(6),
+        [u('x'), u('whoo')],
+        [u('yay'), u('fwew')],
+        [u('y'), u('bugga'), u('z'), u('w'), u('c'), u('whee')],
+        [u('rem')])
+
+    _update_sample_acls_fail(
+        samples, id_, UserID('foo'), acls, NoSuchUserError('whoo, yay, bugga, w, c'))
+
+    lu.invalid_users.assert_called_once_with(
+        [u('whoo'), u('x'), u('fwew'), u('yay'), u('bugga'), u('c'), u('w'), u('whee'), u('y'),
+         u('z'), u('rem')])
+
+
+def test_update_sample_acls_fail_invalid_user():
+    storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
+    lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)
+    meta = create_autospec(MetadataValidatorSet, spec_set=True, instance=True)
+    ws = create_autospec(WS, spec_set=True, instance=True)
+    samples = Samples(
+        storage, lu, meta, ws, now=nw, uuid_gen=lambda: UUID('1234567890abcdef1234567890abcdef'))
+    id_ = UUID('1234567890abcdef1234567890abcde0')
+
+    lu.invalid_users.side_effect = user_lookup.InvalidUserError('o shit waddup')
+
+    acls = SampleACLDelta(
+        dt(67),
+        [u('o shit waddup'), u('whoo')],
+        [u('yay'), u('fwew')],
+        [u('y'), u('bugga'), u('z')])
+
+    _update_sample_acls_fail(samples, id_, UserID('foo'), acls, NoSuchUserError('o shit waddup'))
+
+    assert lu.invalid_users.call_args_list == [
+        (([u('o shit waddup'), u('whoo'), u('fwew'), u('yay'), u('bugga'), u('y'), u('z')],), {})]
+
+
+def test_update_sample_acls_fail_invalid_token():
+    storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
+    lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)
+    meta = create_autospec(MetadataValidatorSet, spec_set=True, instance=True)
+    ws = create_autospec(WS, spec_set=True, instance=True)
+    samples = Samples(
+        storage, lu, meta, ws, now=nw, uuid_gen=lambda: UUID('1234567890abcdef1234567890abcdef'))
+    id_ = UUID('1234567890abcdef1234567890abcde0')
+
+    lu.invalid_users.side_effect = user_lookup.InvalidTokenError('you big dummy')
+
+    acls = SampleACLDelta(
+        dt(1),
+        [u('x'), u('whoo')],
+        [u('yay'), u('fwew')],
+        [u('y'), u('bugga'), u('z')])
+
+    _update_sample_acls_fail(samples, id_, UserID('foo'), acls, ValueError(
+        'user lookup token for KBase auth server is invalid, cannot continue'))
+
+    assert lu.invalid_users.call_args_list == [
+        (([u('whoo'), u('x'), u('fwew'), u('yay'), u('bugga'), u('y'), u('z')],), {})]
+
+
+def test_update_sample_acls_fail_unauthorized():
+    _update_sample_acls_fail_unauthorized(UserID('anotheruser'))
+    _update_sample_acls_fail_unauthorized(UserID('x'))
+    _update_sample_acls_fail_unauthorized(UserID('MrsEntity'))
+
+
+def _update_sample_acls_fail_unauthorized(user: UserID):
+    storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
+    lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)
+    meta = create_autospec(MetadataValidatorSet, spec_set=True, instance=True)
+    ws = create_autospec(WS, spec_set=True, instance=True)
+    samples = Samples(
+        storage, lu, meta, ws, now=nw, uuid_gen=lambda: UUID('1234567890abcdef1234567890abcdef'))
+    id_ = UUID('1234567890abcdef1234567890abcde0')
+
+    lu.invalid_users.return_value = []
+
+    storage.get_sample_acls.return_value = SampleACL(
+        u('someuser'),
+        dt(1),
+        [u('otheruser')],
+        [u('anotheruser'), u('ur mum')],
+        [u('Fungus J. Pustule Jr.'), u('x')],
+        public_read=True)  # public read shouldn't grant privs.
+
+    _update_sample_acls_fail(samples, id_, user, SampleACLDelta(dt(1)), UnauthorizedError(
+        f'User {user} cannot administrate sample 12345678-90ab-cdef-1234-567890abcde0'))
+
+    assert lu.invalid_users.call_args_list == [(([],), {})]
+
+    assert storage.get_sample_acls.call_args_list == [
+        ((UUID('1234567890abcdef1234567890abcde0'),), {})]
+
+
+def _update_sample_acls_fail(samples, id_, user, update, expected):
+    with raises(Exception) as got:
+        samples.update_sample_acls(id_, user, update)
+    assert_exception_correct(got.value, expected)
+
+
 def test_get_key_metadata():
     storage = create_autospec(ArangoSampleStorage, spec_set=True, instance=True)
     lu = create_autospec(KBaseUserLookup, spec_set=True, instance=True)