]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commit
Remove get_admin_roles and associated logic
authorSalvatore Orlando <salv.orlando@gmail.com>
Fri, 17 Apr 2015 22:00:20 +0000 (15:00 -0700)
committerIhar Hrachyshka <ihrachys@redhat.com>
Tue, 9 Jun 2015 09:12:47 +0000 (11:12 +0200)
commit734e77365b0f241a3cea0f3c9dfb1d5fcf6eac8c
tree316518a31d68f4acad3506e1315823d7e1390e33
parente95510f8d123e0428baa5d7b043d6542120d89f9
Remove get_admin_roles and associated logic

get_admin_roles was introduced so that contextes generated from
within plugins could be used for policy checks. This was the case
up to the Havana release as several plugins invoked the policy
engine directly to authorize requests.

This was an incorrect behaviour and has now been fixed, meaning
that get_admin_roles is no longer need and can be safely removed.
This will result in a leaner and more reliable codebase. Indeed the
function being removed here was the cause of several bugs where the
policy engine was initialized too early in the server bootstrap
process.
While this patch removes the feature it does not remove the
load_admin_roles parameter from context.get_admin_context. Doing so
will break other projects such as neutron-lbaas. The parameter is
deprecated by this patch and an appropriate warning emitted.

As a consequence neutron's will now no longer perform policy checks
when context.is_admin=True. This flag is instead set either when
a context is explicitly created for granting admin privileges, or
when Neutron is operating in noauth mode. In the latter case every
request is treated by neutron as an admin request, and get_admin_roles
is simply ensuring the appropriate roles get pushed into the context
so that the policy engine will grant admin rights to the request.
This behaviour is probably just a waste of resource; also it is not
adding anything from a security perspective.

On the other hand not performing checks when context.is_admin is
True should not pose a security threat either in noauth mode or
with the keystone middleware. In the former case the software keeps
operating assuming admin rights for every requests, whereas in the
latter case the keystone middleware will always supply a context
with the appropriate roles, and there is no way for an attacker
to trick keystonemiddleware into generating a context for which
is_admin=True.

Finally, this patch also does some non-trivial changes in test_l3.py
as some tests were mocking context.to_dict ignoring the is_admin flag.

Closes-Bug: #1446021

Change-Id: I8a5c02712a0b43f3e36a4f14620ebbd73fbfb03f
neutron/common/rpc.py
neutron/context.py
neutron/policy.py
neutron/tests/functional/db/test_ipam.py
neutron/tests/unit/db/test_db_base_plugin_v2.py
neutron/tests/unit/extensions/test_l3.py
neutron/tests/unit/extensions/test_quotasv2.py
neutron/tests/unit/plugins/opencontrail/test_contrail_plugin.py
neutron/tests/unit/test_context.py
neutron/tests/unit/test_policy.py
requirements.txt