Skip to content

Commit

Permalink
feat(data): remove legacy concept of "public" data in favor of lettin…
Browse files Browse the repository at this point in the history 
…g the policy engine make decisions in all cases, fix tests
  • Loading branch information
@Avantol13-machine-user
Avantol13-machine-user committed Jan 21, 2022
1 parent 0c78840 commit 8d6e69b
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 87 deletions.
37 changes: 7 additions & 30 deletions fence/blueprints/data/indexd.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ def get_signed_url_for_file(
r_pays_project = flask.request.args.get("userProject", None)
db_session = db_session or current_session

# default to signing the url even if it's a public object
# this will work so long as we're provided a user token
# default to signing the url
force_signed_url = True
no_force_sign_param = flask.request.args.get("no_force_sign")
if no_force_sign_param and no_force_sign_param.lower() == "true":
Expand Down Expand Up @@ -526,7 +525,6 @@ def _get_signed_url(
return self.indexed_file_locations[0].get_signed_url(
action,
expires_in,
public_data=self.public,
force_signed_url=force_signed_url,
r_pays_project=r_pays_project,
authorized_user=authorized_user,
Expand All @@ -542,7 +540,6 @@ def _get_signed_url(
return file_location.get_signed_url(
action,
expires_in,
public_data=self.public,
force_signed_url=force_signed_url,
r_pays_project=r_pays_project,
authorized_user=authorized_user,
Expand Down Expand Up @@ -626,21 +623,10 @@ def get_authorized_with_username(self, action, usernames_from_passports=None):
def metadata(self):
return self.index_document.get("metadata", {})

@cached_property
def public(self):
if self.index_document.get("authz", []):
return self.public_authz
else:
return self.public_acl

@cached_property
def public_acl(self):
return "*" in self.set_acls

@cached_property
def public_authz(self):
return "/open" in self.index_document.get("authz", [])

@login_required({"data"})
def check_legacy_authorization(self, action):
# if we have a data file upload without corresponding metadata, the record can
Expand Down Expand Up @@ -759,7 +745,6 @@ def get_signed_url(
self,
action,
expires_in,
public_data=False,
force_signed_url=True,
users_from_passports=None,
**kwargs,
Expand Down Expand Up @@ -960,7 +945,6 @@ def get_signed_url(
self,
action,
expires_in,
public_data=False,
force_signed_url=True,
authorized_user=None,
**kwargs,
Expand Down Expand Up @@ -989,7 +973,7 @@ def get_signed_url(
bucket_name, aws_creds, expires_in
)

# if it's public and we don't need to force the signed url, just return the raw
# if we don't need to force the signed url, just return the raw
# s3 url
aws_access_key_id = get_value(
credential,
Expand All @@ -999,7 +983,7 @@ def get_signed_url(
# `aws_access_key_id == "*"` is a special case to support public buckets
# where we do *not* want to try signing at all. the other case is that the
# data is public and user requested to not sign the url
if aws_access_key_id == "*" or (public_data and not force_signed_url):
if aws_access_key_id == "*" or (not force_signed_url):
return http_url

region = self.get_bucket_region()
Expand Down Expand Up @@ -1128,7 +1112,6 @@ def get_signed_url(
self,
action,
expires_in,
public_data=False,
force_signed_url=True,
r_pays_project=None,
authorized_user=None,
Expand All @@ -1137,9 +1120,9 @@ def get_signed_url(

user_info = _get_user_info_for_id_or_from_request(user=authorized_user)

if public_data and not force_signed_url:
if not force_signed_url:
url = "https://storage.cloud.google.com/" + resource_path
elif public_data and _is_anonymous_user(user_info):
elif _is_anonymous_user(user_info):
url = self._generate_anonymous_google_storage_signed_url(
ACTION_DICT["gs"][action], resource_path, int(expires_in)
)
Expand Down Expand Up @@ -1417,7 +1400,6 @@ def get_signed_url(
self,
action,
expires_in,
public_data=False,
force_signed_url=True,
authorized_user=None,
**kwargs,
Expand All @@ -1439,11 +1421,6 @@ def get_signed_url(
Get a signed url for an action like "upload" or "download".
:param int expires_in:
The SAS token will expire in a given number of seconds from datetime.utcnow()
:param bool public_data:
Indicate if the Azure Blob Storage Account has public access.
If it's public and we don't need to force the signed url, just return the raw
url.
The default for public_data is False.
:param bool force_signed_url:
Enforce signing the URL for the Azure Blob Storage Account using a SAS token.
The default is True.
Expand All @@ -1457,15 +1434,15 @@ def get_signed_url(
container_name, blob_name = self._get_container_and_blob()

user_info = _get_user_info_for_id_or_from_request(user=authorized_user)
if user_info and user_info.get("user_id") == ANONYMOUS_USER_ID:
if _is_anonymous_user(user_info):
logger.info(f"Attempting to get a signed url an anonymous user")

# if it's public and we don't need to force the signed url, just return the raw
# url
# `azure_creds == "*"` is a special case to support public buckets
# where we do *not* want to try signing at all. the other case is that the
# data is public and user requested to not sign the url
if azure_creds == "*" or (public_data and not force_signed_url):
if azure_creds == "*" or (not force_signed_url):
return self._get_converted_url()

url = self._generate_azure_blob_storage_sas(
Expand Down
96 changes: 39 additions & 57 deletions tests/data/test_azure_blob_storage_indexed_file_location.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,64 +16,39 @@
indirect=True,
)
@pytest.mark.parametrize(
"action,expires_in,public_data,force_signed_url,azure_creds,user_id,storage_account_matches,expect_signed",
"action,expires_in,force_signed_url,azure_creds,user_id,storage_account_matches,expect_signed",
[
("download", 5, True, None, "fake conn str", "some user", True, False),
("download", 5, True, None, "fake conn str", "some user", False, False),
("download", 5, True, True, "fake conn str", "some user", True, True),
("download", 5, True, True, "fake conn str", "some user", False, False),
("download", 5, True, False, "fake conn str", "some user", True, False),
("download", 5, True, False, "fake conn str", "some user", False, False),
("download", 5, False, None, "fake conn str", "some user", True, True),
("download", 5, False, None, "fake conn str", "some user", False, False),
("download", 5, False, True, "fake conn str", "some user", True, True),
("download", 5, False, True, "fake conn str", "some user", False, False),
("download", 5, False, None, "fake conn str", "some user", True, True),
("download", 5, False, None, "fake conn str", "some user", False, False),
("download", 5, False, None, "*", "some user", True, True),
("download", 5, False, None, "*", "some user", False, False),
("download", 5, False, None, "*", ANONYMOUS_USER_ID, True, True),
("download", 5, False, None, "*", ANONYMOUS_USER_ID, False, False),
("download", 5, None, None, "fake conn str", "some user", True, True),
("download", 5, None, None, "fake conn str", "some user", False, False),
("download", 5, None, True, "fake conn str", "some user", True, True),
("download", 5, None, True, "fake conn str", "some user", False, False),
("download", 5, None, False, "fake conn str", "some user", True, True),
("download", 5, None, False, "fake conn str", "some user", False, False),
("download", 5, True, None, "fake conn str", ANONYMOUS_USER_ID, True, False),
("download", 5, True, None, "fake conn str", ANONYMOUS_USER_ID, False, False),
("upload", 5, True, None, "fake conn str", "some user", True, False),
("upload", 5, True, None, "fake conn str", "some user", False, False),
("upload", 5, True, True, "fake conn str", "some user", True, True),
("upload", 5, True, True, "fake conn str", "some user", False, False),
("upload", 5, True, False, "fake conn str", "some user", True, False),
("upload", 5, True, False, "fake conn str", "some user", False, False),
("upload", 5, False, None, "fake conn str", "some user", True, True),
("upload", 5, False, None, "fake conn str", "some user", False, False),
("upload", 5, False, True, "fake conn str", "some user", True, True),
("upload", 5, False, True, "fake conn str", "some user", False, False),
("upload", 5, False, None, "fake conn str", "some user", True, True),
("upload", 5, False, None, "fake conn str", "some user", False, False),
("upload", 5, False, None, "*", "some user", True, True),
("upload", 5, False, None, "*", "some user", False, False),
("upload", 5, False, None, "*", ANONYMOUS_USER_ID, True, True),
("upload", 5, False, None, "*", ANONYMOUS_USER_ID, False, False),
("upload", 5, None, None, "fake conn str", "some user", True, True),
("upload", 5, None, None, "fake conn str", "some user", False, False),
("upload", 5, None, True, "fake conn str", "some user", True, True),
("upload", 5, None, True, "fake conn str", "some user", False, False),
("upload", 5, None, False, "fake conn str", "some user", True, True),
("upload", 5, None, False, "fake conn str", "some user", False, False),
("upload", 5, True, None, "fake conn str", ANONYMOUS_USER_ID, True, False),
("upload", 5, True, None, "fake conn str", ANONYMOUS_USER_ID, False, False),
("download", 5, None, "fake conn str", "some user", False, False),
("download", 5, True, "fake conn str", "some user", True, True),
("download", 5, True, "fake conn str", "some user", False, False),
("download", 5, False, "fake conn str", "some user", True, False),
("download", 5, False, "fake conn str", "some user", False, False),
("download", 5, None, "fake conn str", "some user", True, True),
("download", 5, None, "*", "some user", True, True),
("download", 5, None, "*", "some user", False, False),
("download", 5, None, "*", ANONYMOUS_USER_ID, True, True),
("download", 5, None, "*", ANONYMOUS_USER_ID, False, False),
("download", 5, None, "fake conn str", ANONYMOUS_USER_ID, True, True),
("download", 5, None, "fake conn str", ANONYMOUS_USER_ID, False, False),
("upload", 5, None, "fake conn str", "some user", False, False),
("upload", 5, True, "fake conn str", "some user", True, True),
("upload", 5, True, "fake conn str", "some user", False, False),
("upload", 5, False, "fake conn str", "some user", True, False),
("upload", 5, False, "fake conn str", "some user", False, False),
("upload", 5, None, "fake conn str", "some user", True, True),
("upload", 5, None, "*", "some user", True, True),
("upload", 5, None, "*", "some user", False, False),
("upload", 5, None, "*", ANONYMOUS_USER_ID, True, True),
("upload", 5, None, "*", ANONYMOUS_USER_ID, False, False),
("upload", 5, None, "fake conn str", ANONYMOUS_USER_ID, True, True),
("upload", 5, None, "fake conn str", ANONYMOUS_USER_ID, False, False),
],
)
def test_get_signed_url(
app,
indexd_client,
action,
expires_in,
public_data,
force_signed_url,
azure_creds,
user_id,
Expand Down Expand Up @@ -101,14 +76,21 @@ def test_get_signed_url(
azure_blob_storage_indexed_file_location = (
AzureBlobStorageIndexedFileLocation(indexed_file_location_url)
)
return_url = (
azure_blob_storage_indexed_file_location.get_signed_url(
action=action,
expires_in=expires_in,
public_data=public_data,
force_signed_url=force_signed_url,
if force_signed_url == None:
return_url = (
azure_blob_storage_indexed_file_location.get_signed_url(
action=action,
expires_in=expires_in,
)
)
else:
return_url = (
azure_blob_storage_indexed_file_location.get_signed_url(
action=action,
expires_in=expires_in,
force_signed_url=force_signed_url,
)
)
)

if expect_signed:
assert "?" in return_url
Expand Down

0 comments on commit 8d6e69b

Please sign in to comment.