diff --git a/lib/SampleService/core/acls.py b/lib/SampleService/core/acls.py index 5ad4af1c..909c9bcc 100644 --- a/lib/SampleService/core/acls.py +++ b/lib/SampleService/core/acls.py @@ -18,6 +18,8 @@ ) from SampleService.core.user import UserID +# may need to add sane limits on ACL sizes if people act like jerks + class SampleAccessType(IntEnum): ''' diff --git a/lib/SampleService/core/storage/arango_sample_storage.py b/lib/SampleService/core/storage/arango_sample_storage.py index d9f06337..31a3de59 100644 --- a/lib/SampleService/core/storage/arango_sample_storage.py +++ b/lib/SampleService/core/storage/arango_sample_storage.py @@ -832,15 +832,21 @@ def _update_sample_acls_pt2(self, id_, update, owner, update_time): # some other thread of operation changed the ACLs to the exact state that application of # our delta update would result in. The only issue here is that the update time stamp will # be a few milliseconds later than it should be, so don't worry about it. + a = [u.id for u in update.admin] + w = [u.id for u in update.write] + r = [u.id for u in update.read] + rem = [u.id for u in update.remove] bind_vars = {'@col': self._col_sample.name, 'id': str(id_), 'owner': owner.id, 'ts': update_time.timestamp(), - 'admin': [u.id for u in update.admin], - 'write': [u.id for u in update.write], - 'read': [u.id for u in update.read], - 'remove': [u.id for u in update.remove], + 'admin': a, + 'admin_remove': w + r + rem, + 'write': w, + 'write_remove': a + r + rem, + 'read': r, + 'read_remove': a + w + rem, } # Could return a subset of s to save bandwith # ensures the owner hasn't changed since we pulled the acls above. @@ -852,11 +858,11 @@ def _update_sample_acls_pt2(self, id_, update, owner, update_time): {_FLD_ACL_UPDATE_TIME}: @ts, {_FLD_ACLS}: {{ {_FLD_ADMIN}: REMOVE_VALUES(UNION_DISTINCT( - s.{_FLD_ACLS}.{_FLD_ADMIN}, @admin), @remove), + s.{_FLD_ACLS}.{_FLD_ADMIN}, @admin), @admin_remove), {_FLD_WRITE}: REMOVE_VALUES(UNION_DISTINCT( - s.{_FLD_ACLS}.{_FLD_WRITE}, @write), @remove), + s.{_FLD_ACLS}.{_FLD_WRITE}, @write), @write_remove), {_FLD_READ}: REMOVE_VALUES(UNION_DISTINCT( - s.{_FLD_ACLS}.{_FLD_READ}, @read), @remove) + s.{_FLD_ACLS}.{_FLD_READ}, @read), @read_remove) ''' if update.public_read is not None: aql += f''', diff --git a/test/core/storage/arango_sample_storage_test.py b/test/core/storage/arango_sample_storage_test.py index 79b51a18..34347506 100644 --- a/test/core/storage/arango_sample_storage_test.py +++ b/test/core/storage/arango_sample_storage_test.py @@ -1248,6 +1248,92 @@ def test_update_sample_acls_with_false_public(samplestorage): False) +def test_update_sample_acls_with_existing_users(samplestorage): + ''' + Tests that when a user is added to an acl it's removed from any other acls. + ''' + id_ = uuid.UUID('1234567890abcdef1234567890abcdef') + assert samplestorage.save_sample( + SavedSample(id_, UserID('user'), [TEST_NODE], dt(1), 'foo')) is True + + samplestorage.replace_sample_acls(id_, SampleACL( + UserID('user'), + dt(56), + admin=[UserID('a1'), UserID('a2'), UserID('arem')], + write=[UserID('w1'), UserID('w2'), UserID('wrem')], + read=[UserID('r1'), UserID('r2'), UserID('rrem')])) + + samplestorage.update_sample_acls(id_, SampleACLDelta( + admin=[UserID('w1')], remove=[UserID('arem')]), dt(89)) + + res = samplestorage.get_sample_acls(id_) + assert res == SampleACL( + UserID('user'), + dt(89), + [UserID('a1'), UserID('a2'), UserID('w1')], + [UserID('w2'), UserID('wrem')], + [UserID('r1'), UserID('r2'), UserID('rrem')], + False) + + samplestorage.update_sample_acls(id_, SampleACLDelta(admin=[UserID('r1')]), dt(90)) + + res = samplestorage.get_sample_acls(id_) + assert res == SampleACL( + UserID('user'), + dt(90), + [UserID('a1'), UserID('a2'), UserID('w1'), UserID('r1')], + [UserID('w2'), UserID('wrem')], + [UserID('r2'), UserID('rrem')], + False) + + samplestorage.update_sample_acls(id_, SampleACLDelta( + read=[UserID('w1')], remove=[UserID('wrem')]), dt(91)) + + res = samplestorage.get_sample_acls(id_) + assert res == SampleACL( + UserID('user'), + dt(91), + [UserID('a1'), UserID('a2'), UserID('r1')], + [UserID('w2')], + [UserID('r2'), UserID('w1'), UserID('rrem')], + False) + + samplestorage.update_sample_acls(id_, SampleACLDelta(read=[UserID('a1')]), dt(92)) + + res = samplestorage.get_sample_acls(id_) + assert res == SampleACL( + UserID('user'), + dt(92), + [UserID('a2'), UserID('r1')], + [UserID('w2')], + [UserID('r2'), UserID('w1'), UserID('a1'), UserID('rrem')], + False) + + samplestorage.update_sample_acls(id_, SampleACLDelta( + write=[UserID('a2')], remove=[UserID('rrem')]), dt(93)) + + res = samplestorage.get_sample_acls(id_) + assert res == SampleACL( + UserID('user'), + dt(93), + [UserID('r1')], + [UserID('w2'), UserID('a2')], + [UserID('r2'), UserID('w1'), UserID('a1')], + False) + + samplestorage.update_sample_acls(id_, SampleACLDelta( + write=[UserID('r2')], read=[UserID('a2'), UserID('a1')]), dt(94)) + + res = samplestorage.get_sample_acls(id_) + assert res == SampleACL( + UserID('user'), + dt(94), + [UserID('r1')], + [UserID('w2'), UserID('r2')], + [UserID('w1'), UserID('a2'), UserID('a1')], + False) + + def test_update_sample_acls_fail_bad_args(samplestorage): id_ = uuid.uuid4() s = SampleACLDelta()