Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
Hi Yuriy, Dolph et al. I'm implementing a climate.policy.check_is_admin(ctx) which will look at policy.json entry 'context_is_admin' for knowing which roles do have elevated rights for Climate. This check must be called when creating a context for knowing if we can allow extra rights. The is_admin flag is pretty handsome because it can be triggered upon that check. If we say that one is bad, how should we manage that ? -Sylvain Le 21/11/2013 06:18, Yuriy Taraday a écrit : On Wed, Nov 20, 2013 at 9:57 PM, Dolph Mathews dolph.math...@gmail.com mailto:dolph.math...@gmail.com wrote: On Wed, Nov 20, 2013 at 10:52 AM, Yuriy Taraday yorik@gmail.com mailto:yorik@gmail.com wrote: On Wed, Nov 20, 2013 at 8:42 PM, Dolph Mathews dolph.math...@gmail.com mailto:dolph.math...@gmail.com wrote: is_admin is a short sighted and not at all granular -- it needs to die, so avoid imitating it. I suggest keeping it in case we need to elevate privileges from code. Can you expand on this point? It sounds like you want to ignore the deployer-specified authorization configuration... No, we're not ignoring it. In Keystone we have two options to become an admin: either have 'admin'-like role (set in policy.json by deployer) or have 'is_admin' set (the only way in Keystone is to pass configured admin_token). We don't have bootstrap problem in any other services, so we don't need any admin_token. But we might need to run code that requires admin privileges for user that don't have them. Other projects use get_admin_context() or smth like that for this. I suggest we keep the option to have such 'in-code sudo' using is_admin that will be mentioned in policy.json, but limit is_admin usage to just that. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
On Thu, Nov 21, 2013 at 12:37 PM, Sylvain Bauza sylvain.ba...@bull.netwrote: Hi Yuriy, Dolph et al. I'm implementing a climate.policy.check_is_admin(ctx) which will look at policy.json entry 'context_is_admin' for knowing which roles do have elevated rights for Climate. This check must be called when creating a context for knowing if we can allow extra rights. The is_admin flag is pretty handsome because it can be triggered upon that check. If we say that one is bad, how should we manage that ? -Sylvain There should be no need for is_admin and some special policy rule like context_is_admin. Every action that might require granular access control (for controllers it should be every action at all, I guess) should call enforce() from openstack.common.policy to check appropriate rule in policy.json. Rules for actions that require user to be admin should contain a reference to some basic rule like admin_required in Keystone (see https://github.com/openstack/keystone/blob/master/etc/policy.json). We should not check from code if the user is an admin. We should always ask openstack.common.policy if the user have access to the action. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
Le 21/11/2013 10:04, Yuriy Taraday a écrit : On Thu, Nov 21, 2013 at 12:37 PM, Sylvain Bauza sylvain.ba...@bull.net mailto:sylvain.ba...@bull.net wrote: Hi Yuriy, Dolph et al. I'm implementing a climate.policy.check_is_admin(ctx) which will look at policy.json entry 'context_is_admin' for knowing which roles do have elevated rights for Climate. This check must be called when creating a context for knowing if we can allow extra rights. The is_admin flag is pretty handsome because it can be triggered upon that check. If we say that one is bad, how should we manage that ? -Sylvain There should be no need for is_admin and some special policy rule like context_is_admin. Every action that might require granular access control (for controllers it should be every action at all, I guess) should call enforce() from openstack.common.policy to check appropriate rule in policy.json. Rules for actions that require user to be admin should contain a reference to some basic rule like admin_required in Keystone (see https://github.com/openstack/keystone/blob/master/etc/policy.json). We should not check from code if the user is an admin. We should always ask openstack.common.policy if the user have access to the action. -- Kind regards, Yuriy. Thanks for all your thoughts, really appreciated. OK, I will discuss with Swann and see what needs to be modified accordingly. I'll deliver a new patchset for https://review.openstack.org/#/c/57200/ (policies) based on Context patch from Swann and having is_admin, and then I'll iterate removing the necessary parts. -Sylvain (Btw, that's bad I spent a few days implementing policies without clear guidelines and copying Nova stuff with latest Oslo policies, we definitely need developer documentation for that...) ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
Hi, When reviewing https://review.openstack.org/#/c/54539/, it appeared to me that we need to make consensus on the way to know that a request is having admin creds. Currently, for implementing policies check in Climate, I'm looking at context.roles dict, which contains the unicode string 'admin' if so. I don't need to have an extra param. That said, having a method check_is_admin on the context module would be great, and I fully reckon it would be helpful. What do you think all folks ? -Sylvain ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
On Wed, Nov 20 2013, Sylvain Bauza wrote: When reviewing https://review.openstack.org/#/c/54539/, it appeared to me that we need to make consensus on the way to know that a request is having admin creds. Currently, for implementing policies check in Climate, I'm looking at context.roles dict, which contains the unicode string 'admin' if so. I don't need to have an extra param. That said, having a method check_is_admin on the context module would be great, and I fully reckon it would be helpful. What do you think all folks ? It depends on how fine grained you want your ACL to be, but anyway setting a user as admin depends on the 'admin' role being present in the context.roles dict or not, so I don't see the point of having an extra param; a helper method, at best. -- Julien Danjou /* Free Software hacker * independent consultant http://julien.danjou.info */ signature.asc Description: PGP signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
Le 20/11/2013 11:18, Julien Danjou a écrit : It depends on how fine grained you want your ACL to be, Then, that's policy matter to let you know if you can trust the user or not. I'm digging into http://adam.younglogic.com/2013/11/policy-enforcement-in-openstack/,great value for knowing how managing fine grained ACLs. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
Looking at implementations in Keystone and Nova, I found the only use for is_admin but it is essential. Whenever in code you need to run a piece of code with admin privileges, you can create a new context with is_admin=True keeping all other parameters as is, run code requiring admin access and then revert context back. My first though was: Hey, why don't they just add 'admin' role then?. But what if in current deployment admin role is named like 'TheVerySpecialAdmin'? What if user has tweaked policy.json to better suite one's needs? So my current understanding is (and I suggest to follow this logic): - 'admin' role in context.roles can vary, it's up to cloud admin to set necessary value in policy.json; - 'is_admin' flag is used to elevate privileges from code and it's name is fixed; - policy check should assume that user is admin if either special role is present or is_admin flag is set. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
Hi Yuriy, Le 20/11/2013 11:56, Yuriy Taraday a écrit : Looking at implementations in Keystone and Nova, I found the only use for is_admin but it is essential. Whenever in code you need to run a piece of code with admin privileges, you can create a new context with is_admin=True keeping all other parameters as is, run code requiring admin access and then revert context back. My first though was: Hey, why don't they just add 'admin' role then?. But what if in current deployment admin role is named like 'TheVerySpecialAdmin'? What if user has tweaked policy.json to better suite one's needs? So my current understanding is (and I suggest to follow this logic): - 'admin' role in context.roles can vary, it's up to cloud admin to set necessary value in policy.json; - 'is_admin' flag is used to elevate privileges from code and it's name is fixed; - policy check should assume that user is admin if either special role is present or is_admin flag is set. 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]. I'm OK copying that, any objections to it ? [1] https://github.com/openstack/nova/blob/master/etc/nova/policy.json#L2 [2] https://github.com/openstack/nova/blob/master/nova/policy.py#L116 [3] https://github.com/openstack/nova/blob/master/nova/context.py#L102 ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
I suppose it's ok - just rebase from Swann's commit to have is_admin param to use. On Wed, Nov 20, 2013 at 3:21 PM, Sylvain Bauza sylvain.ba...@bull.netwrote: Hi Yuriy, Le 20/11/2013 11:56, Yuriy Taraday a écrit : Looking at implementations in Keystone and Nova, I found the only use for is_admin but it is essential. Whenever in code you need to run a piece of code with admin privileges, you can create a new context with is_admin=True keeping all other parameters as is, run code requiring admin access and then revert context back. My first though was: Hey, why don't they just add 'admin' role then?. But what if in current deployment admin role is named like 'TheVerySpecialAdmin'? What if user has tweaked policy.json to better suite one's needs? So my current understanding is (and I suggest to follow this logic): - 'admin' role in context.roles can vary, it's up to cloud admin to set necessary value in policy.json; - 'is_admin' flag is used to elevate privileges from code and it's name is fixed; - policy check should assume that user is admin if either special role is present or is_admin flag is set. 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]. I'm OK copying that, any objections to it ? [1] https://github.com/openstack/nova/blob/master/etc/nova/policy.json#L2 [2] https://github.com/openstack/nova/blob/master/nova/policy.py#L116 [3] https://github.com/openstack/nova/blob/master/nova/context.py#L102 ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Best regards, Dina Belova Software Engineer Mirantis Inc. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
Well, I'm guessing the best way is the contrary, Swann needing to rebase from the change I proposed about policies. The latter is still as draft, committing myself to finish it by today. -Sylvain Le 20/11/2013 12:42, Dina Belova a écrit : I suppose it's ok - just rebase from Swann's commit to have is_admin param to use. On Wed, Nov 20, 2013 at 3:21 PM, Sylvain Bauza sylvain.ba...@bull.net mailto:sylvain.ba...@bull.net wrote: Hi Yuriy, Le 20/11/2013 11:56, Yuriy Taraday a écrit : Looking at implementations in Keystone and Nova, I found the only use for is_admin but it is essential. Whenever in code you need to run a piece of code with admin privileges, you can create a new context with is_admin=True keeping all other parameters as is, run code requiring admin access and then revert context back. My first though was: Hey, why don't they just add 'admin' role then?. But what if in current deployment admin role is named like 'TheVerySpecialAdmin'? What if user has tweaked policy.json to better suite one's needs? So my current understanding is (and I suggest to follow this logic): - 'admin' role in context.roles can vary, it's up to cloud admin to set necessary value in policy.json; - 'is_admin' flag is used to elevate privileges from code and it's name is fixed; - policy check should assume that user is admin if either special role is present or is_admin flag is set. 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]. I'm OK copying that, any objections to it ? [1] https://github.com/openstack/nova/blob/master/etc/nova/policy.json#L2 [2] https://github.com/openstack/nova/blob/master/nova/policy.py#L116 [3] https://github.com/openstack/nova/blob/master/nova/context.py#L102 ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org mailto:OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- Best regards, Dina Belova Software Engineer Mirantis Inc. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
On Wed, Nov 20, 2013 at 3:21 PM, Sylvain Bauza sylvain.ba...@bull.netwrote: 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]. I'm OK copying that, any objections to it ? 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. [1] https://github.com/openstack/nova/blob/master/etc/nova/policy.json#L2 This rule is here just to support [2] https://github.com/openstack/nova/blob/master/nova/policy.py#L116 this, which is used only [3] https://github.com/openstack/nova/blob/master/nova/context.py#L102 here. This is not what I would call a consistent usage of policies. 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: https://github.com/openstack/keystone/blob/master/etc/policy.json. 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. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
On Wed, Nov 20, 2013 at 10:24 AM, Yuriy Taraday yorik@gmail.com wrote: On Wed, Nov 20, 2013 at 3:21 PM, Sylvain Bauza sylvain.ba...@bull.netwrote: 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]. I'm OK copying that, any objections to it ? 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. [1] https://github.com/openstack/nova/blob/master/etc/nova/policy.json#L2 This rule is here just to support [2] https://github.com/openstack/nova/blob/master/nova/policy.py#L116 this, which is used only [3] https://github.com/openstack/nova/blob/master/nova/context.py#L102 here. This is not what I would call a consistent usage of policies. 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: https://github.com/openstack/keystone/blob/master/etc/policy.json. ++ 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). 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. is_admin is a short sighted and not at all granular -- it needs to die, so avoid imitating it. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- -Dolph ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
Hello, Dolph. On Wed, Nov 20, 2013 at 8:42 PM, Dolph Mathews dolph.math...@gmail.comwrote: On Wed, Nov 20, 2013 at 10:24 AM, Yuriy Taraday yorik@gmail.comwrote: 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. is_admin is a short sighted and not at all granular -- it needs to die, so avoid imitating it. I suggest keeping it in case we need to elevate privileges from code. In this case we can't rely on roles so just one flag should work fine. As I said before, we should avoid setting or reading is_admin directly from code. It should be set only in context.elevated and read only by admin_required policy rule. Does this sound reasonable? -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
On Wed, Nov 20, 2013 at 10:52 AM, Yuriy Taraday yorik@gmail.com wrote: Hello, Dolph. On Wed, Nov 20, 2013 at 8:42 PM, Dolph Mathews dolph.math...@gmail.comwrote: On Wed, Nov 20, 2013 at 10:24 AM, Yuriy Taraday yorik@gmail.comwrote: 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. is_admin is a short sighted and not at all granular -- it needs to die, so avoid imitating it. I suggest keeping it in case we need to elevate privileges from code. Can you expand on this point? It sounds like you want to ignore the deployer-specified authorization configuration... In this case we can't rely on roles so just one flag should work fine. As I said before, we should avoid setting or reading is_admin directly from code. It should be set only in context.elevated and read only by admin_required policy rule. Does this sound reasonable? -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev -- -Dolph ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Climate] How we agree to determine that an user has admin rights ?
On Wed, Nov 20, 2013 at 9:57 PM, Dolph Mathews dolph.math...@gmail.comwrote: On Wed, Nov 20, 2013 at 10:52 AM, Yuriy Taraday yorik@gmail.comwrote: On Wed, Nov 20, 2013 at 8:42 PM, Dolph Mathews dolph.math...@gmail.comwrote: is_admin is a short sighted and not at all granular -- it needs to die, so avoid imitating it. I suggest keeping it in case we need to elevate privileges from code. Can you expand on this point? It sounds like you want to ignore the deployer-specified authorization configuration... No, we're not ignoring it. In Keystone we have two options to become an admin: either have 'admin'-like role (set in policy.json by deployer) or have 'is_admin' set (the only way in Keystone is to pass configured admin_token). We don't have bootstrap problem in any other services, so we don't need any admin_token. But we might need to run code that requires admin privileges for user that don't have them. Other projects use get_admin_context() or smth like that for this. I suggest we keep the option to have such 'in-code sudo' using is_admin that will be mentioned in policy.json, but limit is_admin usage to just that. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
