require_admin_context() does not account for policy.json rulesets

[Sylvain Bauza]

The API RBAC is done using a policy.json file which allows fine-grained control over each API endpoint by setting specific rules.
Consequently, some defaulted admin-only endpoints can be opened by modifying their corresponding policy rules to be for anyone.

Unfortunately, in many places (in the DB and at the API level following the blueprint api-policy-v3 ), there is a call to context.require_admin_context() which is just checking if the user is admin or no but doesn't match with the policy rules.

As we all agreed with api-policy-v3 that RBAC should be done at the API level, there is no reason to keep that call to context.require_admin_context() and we should assume that policy.json is the single source of truth for knowing the access rights.

[jichenjc (jichenjc)]

Only wondering whether some operators utilize the admin check before so
the not allowed operations will became allowed now

the api-policy-v3 aims to backward compatible but this need Doc update and operator need to be awared?

[Matt Riedemann (mriedem)]

I don't disagree, see bug 1168488 from way back in grizzly.

The only thing would be we'd have to make sure the default rule is admin for any v2 extensions which are enforcing an admin context today.

[Diana Clarke (diana-clarke)]

There are currently 41 calls to require_admin_context:

    nova_context.require_admin_context(context)

All of them are in:

    nova/api/openstack/compute/legacy_v2/contrib/

Each one of those calls is prefixed by a similar comment:

        # NOTE(alex_xu): back-compatible with db layer hard-code admin
        # permission checks.
        nova_context.require_admin_context(context)

And this comment addresses the question this bug raises:

        # NOTE(alex_xu): back-compatible with db layer hard-code admin
        # permission checks. This has to be left only for API v2.0 because
        # this version has to be stable even if it means that only admins
        # can call this method while the policy could be changed.
        nova_context.require_admin_context(context)

I suspect (but I could be wrong) that there was consensus to stop here with these API policy/permission check changes.

Can someone please confirm that the direction this bug proposes is still desired? Thanks!

If not... then we can close this bug.

[Alex Xu (xuhj)]

Actually in the original spec, we said keep the v2 API behavior as the same https://github.com/openstack/nova-specs/blob/master/specs/kilo/approved/nova-api-policy.rst

So we add back-compatible check to the legacy v2 API. And this legacy v2 API already deprecated, so I think it's worth to do it.

I prefer to close the bug...

[Diana Clarke (diana-clarke)]

Thanks for the additional context, Alex. I'll close this bug (mark it as invalid).