Skip to content

Commit

Permalink
Fix admins being able to update admin accounts with unauthorized groups.
Browse files Browse the repository at this point in the history
If a non-superuser admin was allowed to update other admin accounts
within their group, and the admin also knew the UUID for another admin
group they were not authorized for, then they could assign admins to
that unauthorized admin group. Since exploiting this hinges upon a
limited admin knowing the UUIDs for other valid admin groups they can't
see, exploiting this should hopefully be difficult, but none the less, a
security issue.
  • Loading branch information
GUI committed Dec 10, 2016
1 parent e9ccb78 commit c5ca3c1
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/api-umbrella/web-app/app/models/admin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def group_names
end

def api_scopes
@api_scopes ||= groups.map { |group| group.api_scopes }.flatten.compact.uniq
groups.map { |group| group.api_scopes }.flatten.compact.uniq
end

def can?(permission)
Expand Down
26 changes: 26 additions & 0 deletions test/apis/v1/admins/test_admin_permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,32 @@ def test_superuser_as_prefix_admin
assert_admin_forbidden(factory, admin)
end

def test_forbids_updating_permitted_admins_with_unpermitted_values
google_admin_group = FactoryGirl.create(:google_admin_group)
yahoo_admin_group = FactoryGirl.create(:yahoo_admin_group)
record = FactoryGirl.create(:limited_admin, :groups => [google_admin_group])
admin = FactoryGirl.create(:google_admin)

attributes = record.serializable_hash
response = Typhoeus.put("https://127.0.0.1:9081/api-umbrella/v1/admins/#{record.id}.json", @@http_options.deep_merge(admin_token(admin)).deep_merge({
:headers => { "Content-Type" => "application/x-www-form-urlencoded" },
:body => { :admin => attributes },
}))
assert_response_code(200, response)

attributes["group_ids"] = [yahoo_admin_group.id]
response = Typhoeus.put("https://127.0.0.1:9081/api-umbrella/v1/admins/#{record.id}.json", @@http_options.deep_merge(admin_token(admin)).deep_merge({
:headers => { "Content-Type" => "application/x-www-form-urlencoded" },
:body => { :admin => attributes },
}))
assert_response_code(403, response)
data = MultiJson.load(response.body)
assert_equal(["errors"], data.keys)

record = Admin.find(record.id)
assert_equal([google_admin_group.id], record.group_ids)
end

def test_forbids_updating_unpermitted_admins_with_permitted_values
google_admin_group = FactoryGirl.create(:google_admin_group)
yahoo_admin_group = FactoryGirl.create(:yahoo_admin_group)
Expand Down

0 comments on commit c5ca3c1

Please sign in to comment.