Skip to content

Commit c5ca3c1

Browse files
GUIGUI
committed
Fix admins being able to update admin accounts with unauthorized groups.
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.
1 parent e9ccb78 commit c5ca3c1

2 files changed

Lines changed: 27 additions & 1 deletion

File tree

‎src/api-umbrella/web-app/app/models/admin.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def group_names
5858
end
5959

6060
def api_scopes
61-
@api_scopes ||= groups.map { |group| group.api_scopes }.flatten.compact.uniq
61+
groups.map { |group| group.api_scopes }.flatten.compact.uniq
6262
end
6363

6464
def can?(permission)

‎test/apis/v1/admins/test_admin_permissions.rb‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,32 @@ def test_superuser_as_prefix_admin
107107
assert_admin_forbidden(factory, admin)
108108
end
109109

110+
def test_forbids_updating_permitted_admins_with_unpermitted_values
111+
google_admin_group = FactoryGirl.create(:google_admin_group)
112+
yahoo_admin_group = FactoryGirl.create(:yahoo_admin_group)
113+
record = FactoryGirl.create(:limited_admin, :groups => [google_admin_group])
114+
admin = FactoryGirl.create(:google_admin)
115+
116+
attributes = record.serializable_hash
117+
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({
118+
:headers => { "Content-Type" => "application/x-www-form-urlencoded" },
119+
:body => { :admin => attributes },
120+
}))
121+
assert_response_code(200, response)
122+
123+
attributes["group_ids"] = [yahoo_admin_group.id]
124+
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({
125+
:headers => { "Content-Type" => "application/x-www-form-urlencoded" },
126+
:body => { :admin => attributes },
127+
}))
128+
assert_response_code(403, response)
129+
data = MultiJson.load(response.body)
130+
assert_equal(["errors"], data.keys)
131+
132+
record = Admin.find(record.id)
133+
assert_equal([google_admin_group.id], record.group_ids)
134+
end
135+
110136
def test_forbids_updating_unpermitted_admins_with_permitted_values
111137
google_admin_group = FactoryGirl.create(:google_admin_group)
112138
yahoo_admin_group = FactoryGirl.create(:yahoo_admin_group)

0 commit comments

Comments
 (0)