"legacy" admin rule does not work and is not needed anymore

[Salvatore Orlando]

in neutron/policy.py:

def check_is_admin(context):
    """Verify context has admin rights according to policy settings."""
    init()
    # the target is user-self
    credentials = context.to_dict()
    target = credentials
    # Backward compatibility: if ADMIN_CTX_POLICY is not
    # found, default to validating role:admin
    admin_policy = (ADMIN_CTX_POLICY if ADMIN_CTX_POLICY in _ENFORCER.rules
                    else 'role:admin')
    return _ENFORCER.enforce(admin_policy, target, credentials)

if ADMIN_CTX_POLICY is not specified the enforcer checks role:admin, which since it does not exist among rules loaded from file, defaults to TrueCheck. This is wrong, and to an extent even dangerous because if ADMIN_CTX_POLICY is missing, then every context would be regarded as an admin context. Thankfully this was only for backward compatibility and is not necessary anymore.

A similar mistake is done for ADVSVC_CTX_POLICY. This is even more puzzling because there was no backward compatibility requirmeent there,

Obviously the unit tests supposed to ensure the correct behaviour of the backward compatibility tweak are validating something completely different.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/175078

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/175078
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=4625c45a30ffe09fbd29c16337e64e264de75bd8
Submitter: Jenkins
Branch: master

commit 4625c45a30ffe09fbd29c16337e64e264de75bd8
Author: Salvatore Orlando <email address hidden>
Date: Fri Apr 17 16:59:42 2015 -0700

    Remove backward compatibility for check_is_admin

    This routine in policy.py used to have a backward compatibility
    check to ensure proper behaviour even when the policy.json file
    did not have a specific 'context_is_admin' policy.
    However, this backward compatibility check does not work. It
    appears indeed that it has been broken for several release cycles;
    it is also possible that actually it never worked.
    When the 'context_is_admin' policy is not in the policy.json file
    the enforcer simply ends up evaluating whatever is the default
    policy configured there.

    Therefore this patch:
    - Removes the backward compatibility check, since it does not work
    - Fails, for safety, check_is_admin if 'context_is_admin' policy is
      not specified
    - Fixeds check_is_advsvc in the same way (the backward compatibility
      check never made any sense for this function)
    - Fixes unit tests adding appropriate tests for check_is_admin and
      check_is_advsvc

    Change-Id: Ia47e5781d86a3f21b9d837c9ac70a62ac435d20b
    Closes-Bug: #1445690

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: neutron-pecan
Review: https://review.openstack.org/185072

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/216405

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/216405
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=2228f6d16accc012eadff59d44b7fd475256c36a
Submitter: Jenkins
Branch: master

commit 2228f6d16accc012eadff59d44b7fd475256c36a
Author: Salvatore Orlando <email address hidden>
Date: Mon Aug 24 02:31:45 2015 -0700

    Remove _extract_roles method from neutron.policy

    This method is not used anymore as it was called only from
    the get_admin_roles method. This method has been removed
    by commit 734e77365b0f241a3cea0f3c9dfb1d5fcf6eac8c; therefore
    the _extract_roles method is now zombie code and should be
    removed before it starts feeding on Neutron's brain.

    Related-Bug: #1445690

    Change-Id: I0306bcea813031897e319af9693e18d03848e378