Skip to content

Commit

Permalink
Merge branch 'test/usersync' of https://github.com/uc-cdis/fence into…
Browse files Browse the repository at this point in the history
… test/usersync
  • Loading branch information
BinamB committed Jan 19, 2023
2 parents 84f37bf + 92b8e3f commit 13accc6
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 146 deletions.
12 changes: 3 additions & 9 deletions fence/sync/sync_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,11 +498,8 @@ def _parse_csv(self, file_dict, sess, dbgap_config={}, encrypted=True):
# parse dbGaP sftp server information
dbgap_key = dbgap_config.get("decrypt_key", None)

self.id_patterns += (
dbgap_config.get("allowed_whitelist_patterns", [])
if dbgap_config.get("allow_non_dbGaP_whitelist", False)
else []
)
self.id_patterns += dbgap_config.get("allowed_whitelist_patterns", [])

enable_common_exchange_area_access = dbgap_config.get(
"enable_common_exchange_area_access", False
)
Expand Down Expand Up @@ -552,10 +549,7 @@ def _parse_csv(self, file_dict, sess, dbgap_config={}, encrypted=True):
continue

phsid_privileges = {}
if dbgap_config.get("allow_non_dbGaP_whitelist", False):
phsid = row.get("phsid", row.get("project_id", "")).split(".")
else:
phsid = row.get("phsid", "").split(".")
phsid = row.get("phsid", row.get("project_id", "")).split(".")

dbgap_project = phsid[0]
# There are issues where dbgap has a wrong entry in their whitelist. Since we do a bulk arborist request, there are wrong entries in it that invalidates the whole request causing other correct entries not to be added
Expand Down
223 changes: 87 additions & 136 deletions tests/dbgap_sync/test_user_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,11 @@ def test_sync_incorrect_user_yaml_file(syncer, monkeypatch, db_session):
assert syncer.arborist_client.create_policy.not_called()


@pytest.mark.parametrize("allow_non_dbgap_whitelist", [False, True])
@pytest.mark.parametrize("syncer", ["google", "cleversafe"], indirect=True)
@pytest.mark.parametrize("parse_consent_code_config", [False, True])
def test_sync(
syncer,
db_session,
allow_non_dbgap_whitelist,
storage_client,
parse_consent_code_config,
monkeypatch,
Expand All @@ -90,9 +88,6 @@ def test_sync(
syncer.dbGaP[0], "parse_consent_code", parse_consent_code_config
)
monkeypatch.setattr(syncer, "parse_consent_code", parse_consent_code_config)
monkeypatch.setitem(
syncer.dbGaP[2], "allow_non_dbGaP_whitelist", allow_non_dbgap_whitelist
)

syncer.sync()

Expand All @@ -102,139 +97,79 @@ def test_sync(
assert len(users) == 9

if parse_consent_code_config:
if allow_non_dbgap_whitelist:
user = models.query_for_user(session=db_session, username="TESTUSERD")
assert equal_project_access(
user.project_access,
{
"phs000179.c1": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="TESTUSERB")
assert equal_project_access(
user.project_access,
{
"phs000178.c1": ["read", "read-storage"],
"phs000179.c1": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="USERC")
assert equal_project_access(
user.project_access,
{
"phs000178.c1": ["read", "read-storage"],
"phs000178.c2": ["read", "read-storage"],
"phs000178.c999": ["read", "read-storage"],
"phs000179.c1": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)
# Assertion to ensure an invalid project id is not used
user = models.query_for_user(session=db_session, username="USERF")
assert "(888)-888-8888" not in user.project_access
assert equal_project_access(
user.project_access,
{
"phs000178.c1": ["read", "read-storage"],
"phs000178.c2": ["read", "read-storage"],
},
)
else:
user = models.query_for_user(session=db_session, username="USERC")
assert equal_project_access(
user.project_access,
{
"phs000178.c1": ["read", "read-storage"],
"phs000178.c2": ["read", "read-storage"],
"phs000178.c999": ["read", "read-storage"],
"phs000179.c1": ["read", "read-storage"],
},
)
user = models.query_for_user(session=db_session, username="TESTUSERD")
assert equal_project_access(
user.project_access,
{
"phs000179.c1": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="USERF")
assert equal_project_access(
user.project_access,
{
"phs000178.c1": ["read", "read-storage"],
"phs000178.c2": ["read", "read-storage"],
},
)
user = models.query_for_user(session=db_session, username="TESTUSERB")
assert equal_project_access(
user.project_access,
{
"phs000178.c1": ["read", "read-storage"],
"phs000179.c1": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="TESTUSERB")
assert equal_project_access(
user.project_access,
{
"phs000179.c1": ["read", "read-storage"],
"phs000178.c1": ["read", "read-storage"],
},
)
user = models.query_for_user(session=db_session, username="USERC")
assert equal_project_access(
user.project_access,
{
"phs000178.c1": ["read", "read-storage"],
"phs000178.c2": ["read", "read-storage"],
"phs000178.c999": ["read", "read-storage"],
"phs000179.c1": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)
# Assertion to ensure an invalid project id is not used
user = models.query_for_user(session=db_session, username="USERF")
assert "(888)-888-8888" not in user.project_access
assert equal_project_access(
user.project_access,
{
"phs000178.c1": ["read", "read-storage"],
"phs000178.c2": ["read", "read-storage"],
},
)
else:
if allow_non_dbgap_whitelist:
user = models.query_for_user(session=db_session, username="TESTUSERD")
assert equal_project_access(
user.project_access,
{
"phs000179": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="TESTUSERB")
assert equal_project_access(
user.project_access,
{
"phs000178": ["read", "read-storage"],
"phs000179": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
"TCGA-PCAWG": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="USERC")
assert equal_project_access(
user.project_access,
{
"phs000178": ["read", "read-storage"],
"phs000178": ["read", "read-storage"],
"phs000178": ["read", "read-storage"],
"phs000179": ["read", "read-storage"],
"TCGA-PCAWG": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)
else:
user = models.query_for_user(session=db_session, username="USERC")
assert equal_project_access(
user.project_access,
{
"phs000178": ["read", "read-storage"],
"TCGA-PCAWG": ["read", "read-storage"],
"phs000179": ["read", "read-storage"],
},
)
user = models.query_for_user(session=db_session, username="TESTUSERD")
assert equal_project_access(
user.project_access,
{
"phs000179": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="USERF")
assert equal_project_access(
user.project_access,
{
"phs000178": ["read", "read-storage"],
"TCGA-PCAWG": ["read", "read-storage"],
},
)
user = models.query_for_user(session=db_session, username="TESTUSERB")
assert equal_project_access(
user.project_access,
{
"phs000178": ["read", "read-storage"],
"phs000179": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
"TCGA-PCAWG": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="TESTUSERB")
assert equal_project_access(
user.project_access,
{
"phs000178": ["read", "read-storage"],
"TCGA-PCAWG": ["read", "read-storage"],
"phs000179": ["read", "read-storage"],
},
)
user = models.query_for_user(session=db_session, username="USERC")
assert equal_project_access(
user.project_access,
{
"phs000178": ["read", "read-storage"],
"phs000178": ["read", "read-storage"],
"phs000178": ["read", "read-storage"],
"phs000179": ["read", "read-storage"],
"TCGA-PCAWG": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="TESTUSERD")
assert user.display_name == "USER D"
Expand Down Expand Up @@ -301,6 +236,7 @@ def test_dbgap_consent_codes(
# should additionally include the study-specific exchange area access and
# access to the common exchange area
"test_common_exchange_area": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)
else:
Expand All @@ -314,6 +250,7 @@ def test_dbgap_consent_codes(
"phs000178.c1": ["read", "read-storage"],
"phs000178.c2": ["read", "read-storage"],
"phs000178.c999": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)
else:
Expand All @@ -323,6 +260,7 @@ def test_dbgap_consent_codes(
{
"phs000178": ["read", "read-storage"],
"phs000179": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

Expand All @@ -337,7 +275,10 @@ def test_dbgap_consent_codes(
)
else:
assert equal_project_access(
user.project_access, {"phs000178": ["read", "read-storage"]}
user.project_access,
{
"phs000178": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="TESTUSERB")
Expand All @@ -347,6 +288,7 @@ def test_dbgap_consent_codes(
{
"phs000178.c1": ["read", "read-storage"],
"phs000179.c1": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)
else:
Expand All @@ -355,17 +297,26 @@ def test_dbgap_consent_codes(
{
"phs000178": ["read", "read-storage"],
"phs000179": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

user = models.query_for_user(session=db_session, username="TESTUSERD")
if parse_consent_code_config:
assert equal_project_access(
user.project_access, {"phs000179.c1": ["read", "read-storage"]}
user.project_access,
{
"phs000179.c1": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)
else:
assert equal_project_access(
user.project_access, {"phs000179": ["read", "read-storage"]}
user.project_access,
{
"phs000179": ["read", "read-storage"],
"PROJECT-12345": ["read", "read-storage"],
},
)

resource_to_parent_paths = collections.defaultdict(list)
Expand Down
1 change: 0 additions & 1 deletion tests/test-fence-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ dbGaP:
proxy_user: ''
protocol: 'sftp'
decrypt_key: ''
allow_non_dbGaP_whitelist: false
allowed_whitelist_patterns: ['authentication_file_PROJECT-(\d*).(csv|txt)']
# Additional allowed patterns for project_ids. The default value in usersync is 'phs(\d{6}) for dbgap projects'
additional_allowed_project_id_patterns: ['PROJECT-(\d*)']
Expand Down

0 comments on commit 13accc6

Please sign in to comment.