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

fix: permissions inconsistency in local mode #348

Merged
merged 2 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- BREAKING: rename Algo to Function ([#341](https://github.com/Substra/substra/pull/341))
- Rename fields in Performances model (#344)
- Rename fields in Performances model ([#344](https://github.com/Substra/substra/pull/344))

### Fixed

- Permissions inconsistency in local mode ([#348](https://github.com/Substra/substra/pull/348))

## [0.42.0](https://github.com/Substra/substra/releases/tag/0.42.0) - 2023-02-20

Expand Down
11 changes: 6 additions & 5 deletions substra/sdk/backends/local/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,13 @@ def __compute_permissions(self, permissions):
If the permissions are private, the active organization is
in the authorized ids.
"""
updated_permissions = copy.deepcopy(permissions)
owner = self._org_id
if permissions.public:
permissions.authorized_ids = list()
elif not permissions.public and owner not in permissions.authorized_ids:
permissions.authorized_ids.append(owner)
return permissions
if updated_permissions.public:
updated_permissions.authorized_ids = list()
elif not updated_permissions.public and owner not in updated_permissions.authorized_ids:
updated_permissions.authorized_ids.append(owner)
return updated_permissions

def __add_compute_plan(
self,
Expand Down
30 changes: 25 additions & 5 deletions tests/sdk/local/test_debug.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import copy
import uuid

import docker
Expand Down Expand Up @@ -43,7 +44,6 @@ def test_client_tmp_dir(self, clients):

@pytest.mark.parametrize("dockerfile_type", ("BAD_ENTRYPOINT", "NO_ENTRYPOINT", "NO_FUNCTION_NAME"))
def test_client_bad_dockerfile(self, asset_factory, dockerfile_type, clients):

client = clients[0]

dataset_query = asset_factory.create_dataset()
Expand Down Expand Up @@ -287,6 +287,30 @@ def test_add_compute_plan_with_wrong_metadata(self, clients):
)
)

def test_permissions_consistency_add_dataset(self, clients, asset_factory):
client = clients[0]

permissions = substra.sdk.schemas.Permissions(public=False, authorized_ids=["foo"])
copy_permissions = copy.deepcopy(permissions)

dataset_query = asset_factory.create_dataset(permissions=permissions)

client.add_dataset(dataset_query)

assert str(copy_permissions) == str(permissions)

def test_permissions_consistency_add_function(self, clients, asset_factory):
client = clients[0]

permissions = substra.sdk.schemas.Permissions(public=False, authorized_ids=["foo"])
copy_permissions = copy.deepcopy(permissions)

function_query = asset_factory.create_function(permissions=permissions)

client.add_function(function_query)

assert str(copy_permissions) == str(permissions)

def test_live_performances_json_file_exist(self, asset_factory, clients):
"""Assert the performances file is well created."""
client = clients[0]
Expand Down Expand Up @@ -612,14 +636,12 @@ def test_task_different_owner_dataset(asset_factory, clients):

class TestMultipleOrgLocalClient:
def test_local_org_info(self, docker_clients):

org1info = docker_clients[0].organization_info()
org2info = docker_clients[1].organization_info()

assert org1info.organization_id != org2info.organization_id

def test_list_org(self, docker_clients):

org1_id = docker_clients[0].organization_info().organization_id
org2_id = docker_clients[1].organization_info().organization_id

Expand All @@ -634,7 +656,6 @@ def test_list_org(self, docker_clients):
assert not org2[0].is_current

def test_local_org_owner(self, asset_factory, docker_clients):

client = docker_clients[0]
dataset_query = asset_factory.create_dataset()
dataset_key = client.add_dataset(dataset_query)
Expand All @@ -643,7 +664,6 @@ def test_local_org_owner(self, asset_factory, docker_clients):
assert dataset.owner == client.organization_info().organization_id

def test_local_org_shared_db(self, asset_factory, docker_clients):

dataset_query = asset_factory.create_dataset()
dataset_key = docker_clients[0].add_dataset(dataset_query)

Expand Down