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.

Sign up for GitHub

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

Jump to bottom

feat(public-files): check authz/acl field, then hit arborist as anon … #653

Merged
merged 8 commits into from
Jun 28, 2019
Merged

feat(public-files): check authz/acl field, then hit arborist as anon … #653

Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
081254b
feat(public-files): check authz/acl field, then hit arborist as anon …
Avantol13 Jun 27, 2019
ca23962
fix(public-files): make it a class method to access authorization check
Avantol13 Jun 27, 2019
e8d3a58
chore(revert): just check for /open
Avantol13 Jun 27, 2019
880e6c2
fix(revert): undo changes
Avantol13 Jun 27, 2019
de85db0
chore(inline): inline function only called one place
Avantol13 Jun 28, 2019
1283bbb
fix(indexd): only check authz for if its public, don't include in pre…
Avantol13 Jun 28, 2019
55547ff
fix(acls): extend acls by anything in authz field
Avantol13 Jun 28, 2019
e0af39e
fix(acls): extend correctly
Avantol13 Jun 28, 2019
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
14 changes: 8 additions & 6 deletions fence/blueprints/data/indexd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,10 @@ def _get_signed_url(self, protocol, action, expires_in, force_signed_url):
)

@cached_property
def set_acls(self):
if "acl" in self.index_document:
def authz(self):
if "authz" in self.index_document:
return set(self.index_document["authz"])
elif "acl" in self.index_document:
return set(self.index_document["acl"])
elif "acls" in self.metadata:
return set(self.metadata["acls"].split(","))
Expand All @@ -365,7 +367,7 @@ def metadata(self):

@cached_property
def public(self):
return check_public(self.set_acls)
return check_public(self.authz)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple thoughts:

  • check_public only used here so this would be a good opportunity to inline
  • should keep the ACLs check here too I think—we can use authz if exists or default to acl

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, this is perhaps misleading, the cached property isn't just authz, it'll get acl field if authz doesn't exist. I will inline check_public tho


@login_required({"data"})
def check_authorization(self, action):
Expand Down Expand Up @@ -399,7 +401,7 @@ def check_authorization(self, action):
given_acls = set(
filter_auth_ids(action, flask.g.token["context"]["user"]["projects"])
)
return len(self.set_acls & given_acls) > 0
return len(self.authz & given_acls) > 0

@login_required({"data"})
def delete_files(self, urls=None, delete_all=True):
Expand Down Expand Up @@ -829,6 +831,6 @@ def filter_auth_ids(action, list_auth_ids):
return authorized_dbgaps


def check_public(set_acls):
if "*" in set_acls:
def check_public(authz):
if "*" in authz or "/open" in authz:
return True