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

store ACEs by client/user #240

Merged
merged 1 commit into from
Sep 1, 2016
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
4 changes: 2 additions & 2 deletions lib/chef_zero/chef_data/data_normalizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ def self.normalize_acls(acls)
# is the final list of actors that a subsequent GET will
# provide. Each list is guaranteed to be unique, but the
# combined list is not.
acls[perm]["actors"] = acls[perm].delete("users").uniq +
acls[perm].delete("clients").uniq
acls[perm]["actors"] = acls[perm]["clients"].uniq +
acls[perm]["users"].uniq
else
# this gets doubled sometimes, for reasons.
(acls[perm]["actors"] ||= []).uniq!
Expand Down
51 changes: 39 additions & 12 deletions lib/chef_zero/chef_data/default_creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,7 @@ def get_org_acl_default(path)
end

def get_owners(acl_path)
owners = []

unknown_owners = []
path = AclPath.get_object_path(acl_path)
if path

Expand All @@ -356,10 +355,10 @@ def get_owners(acl_path)
begin
client = FFI_Yajl::Parser.parse(data.get(path), :create_additions => false)
if !client["validator"]
owners |= [ path[3] ]
unknown_owners |= [ path[3] ]
end
rescue
owners |= [ path[3] ]
unknown_owners |= [ path[3] ]
end

# Add creators as owners (except any validator clients).
Expand All @@ -370,33 +369,60 @@ def get_owners(acl_path)
next if client["validator"]
rescue
end
owners |= [ creator ]
unknown_owners |= [ creator ]
end
end
else
owners |= @creators[path] if @creators[path]
unknown_owners |= @creators[path] if @creators[path]
end
owners = filter_owners(path, unknown_owners)

#ANGRY
# Non-default containers do not get superusers added to them,
# because reasons.
unless path.size == 4 && path[0] == "organizations" && path[2] == "containers" && !exists?(path)
owners += superusers
owners[:users] += superusers
end
else
owners = { clients: [], users: [] }
end

# we don't de-dup this list, because pedant expects to see ["pivotal", "pivotal"] in some cases.
owners[:users].uniq!
owners[:clients].uniq!
owners
end

# Figures out if an object was created by a user or client.
# If the object does not exist in the context
# of an organization, it can only be a user
#
# This isn't perfect, because we are never explicitly told
# if a requestor creating an object is a user or client -
# but it gets us reasonably close
def filter_owners(path, unknown_owners)
owners = { clients: [], users: [] }
unknown_owners.each do |entity|
if path[0] == "organizations" && path.length > 2
begin
data.get(["organizations", path[1], "clients", entity])
owners[:clients] |= [ entity ]
rescue
owners[:users] |= [ entity ]
end
else
owners[:users] |= [ entity ]
end
end
owners
end

def default_acl(acl_path, acl = {})
owners = nil
owners = get_owners(acl_path)
container_acl = nil
PERMISSIONS.each do |perm|
acl[perm] ||= {}
acl[perm]["actors"] ||= begin
owners ||= get_owners(acl_path)
end
acl[perm]["users"] = owners[:users]
acl[perm]["clients"] = owners[:clients]
acl[perm]["groups"] ||= begin
# When we create containers, we don't merge groups (not sure why).
if acl_path[0] == "organizations" && acl_path[3] == "containers"
Expand All @@ -406,6 +432,7 @@ def default_acl(acl_path, acl = {})
(container_acl[perm] ? container_acl[perm]["groups"] : []) || []
end
end
acl[perm]["actors"] = acl[perm]["clients"] + acl[perm]["users"]
end
acl
end
Expand Down
11 changes: 11 additions & 0 deletions lib/chef_zero/endpoints/acls_endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@ def get(request)
end
acls = FFI_Yajl::Parser.parse(get_data(request, acl_path), :create_additions => false)
acls = ChefData::DataNormalizer.normalize_acls(acls)
if request.query_params["detail"] == "granular"
acls.each do |perm, ace|
acls[perm]["actors"] = []
Copy link

@ksubrama ksubrama Sep 1, 2016

Choose a reason for hiding this comment

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

Out of curiosity - does it make sense to leave all the actors in here even if granular is requested? I don't know who uses this field.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is mirroring the behavior that we added to the Chef Server, where if you request granular actors will be empty but users and clients will not.

This prevents someone from using a modified version of the GET response to PUT an acl update, and getting an error because Chef Server won't except a PUT body with both actors and users/clients.

It's a bit awkward, but is the path least likely to break existing tooling outside of what we maintain - the most common method of modifying ACLs is to request the ACLs, modify a single ACE, and PUT it back to the server.

end
else
acls.each do |perm, ace|
acls[perm].delete("clients")
acls[perm].delete("users")
end
end

json_response(200, acls)
end
end
Expand Down