<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 20, 2013 at 10:24 AM, Yuriy Taraday <span dir="ltr"><<a href="mailto:yorik.sar@gmail.com" target="_blank">yorik.sar@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><div class="im">On Wed, Nov 20, 2013 at 3:21 PM, Sylvain Bauza <span dir="ltr"><<a href="mailto:sylvain.bauza@bull.net" target="_blank">sylvain.bauza@bull.net</a>></span> wrote: </div>
<div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"><div>Yes indeed, that's something coming into my mind. Looking at Nova, I
found a "context_is_admin" policy in policy.json allowing you to say
which role is admin or not [1] and is matched in policy.py [2],
which itself is called when creating a context [3].<br></div>
<br>
I'm OK copying that, any objections to it ?</div></blockquote><div><br></div></div><div>I would suggest not to copy this stuff from Nova. There's a lot of legacy there and it's based on old openstack.common.policy version. We should rely on openstack.common.policy alone, no need to add more checks here. </div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF">
[1]
<a href="https://github.com/openstack/nova/blob/master/etc/nova/policy.json#L2" target="_blank">https://github.com/openstack/nova/blob/master/etc/nova/policy.json#L2</a></div></blockquote><div><br></div><div>This rule is here just to support</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF">
[2]
<a href="https://github.com/openstack/nova/blob/master/nova/policy.py#L116" target="_blank">https://github.com/openstack/nova/blob/master/nova/policy.py#L116</a></div></blockquote><div><br></div><div>this, which is used only</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF">
[3]
<a href="https://github.com/openstack/nova/blob/master/nova/context.py#L102" target="_blank">https://github.com/openstack/nova/blob/master/nova/context.py#L102</a></div></blockquote><div><br></div><div>here. This is not what I would call a consistent usage of policies. </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF">
</div>
</blockquote></div><br>If we need to check access rights to some method, we should use an appropriate decorator or helper method and let it check appropriate policy rule that would contain "rule:admin_required", just like in Keystone: <a href="https://github.com/openstack/keystone/blob/master/etc/policy.json" target="_blank">https://github.com/openstack/keystone/blob/master/etc/policy.json</a>.</div>
</div></blockquote><div><br></div><div>++ Define actual policy rules with a suggested policy.json file, but do NOT hardcode a definition of "admin". Allow the deployer to define more granular policy. oslo.policy makes this pretty easy. If you're looking at keystone, be sure to look at how we protect v3 controller methods (which ask the policy engine, "does the requestor have authorization to perform this operation?"), NOT how we protect v2 controller methods (which ask the policy engine, "does the requestor have a magical pre-defined role?" regardless of what operation is actually being performed).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div class="gmail_extra"><br></div><div class="gmail_extra">context.is_admin should not be checked directly from code, only through policy rules. It should be set only if we need to elevate privileges from code. That should be the meaning of it.</div>
</div></blockquote><div><br></div><div>is_admin is a short sighted and not at all granular -- it needs to die, so avoid imitating it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><span class="HOEnZb"><font color="#888888"><br clear="all">
<div><br></div>-- <br><br><div>Kind regards, Yuriy.</div>
</font></span></div></div>
<br>_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><br></div>-Dolph
</div></div>