[Kimchi-devel] [PATCH 4/5] Enhance UrlSubNode decorator and kimchiauth tool to check for sudo rights

[Leonardo Augusto Guimarães Garcia]

On 02/11/2014 01:17 AM, Leonardo Augusto Guimarães Garcia wrote:
> On 02/10/2014 05:23 PM, Aline Manera wrote:
>> On 02/10/2014 05:19 PM, Aline Manera wrote:
>>> On 02/10/2014 12:32 AM, Leonardo Garcia wrote:
>>>> From: Leonardo Garcia <lagarcia at br.ibm.com>
>>>>
>>>> kimchiauth tool used to only check if the user was authenticated or
>>>> not.
>>>> Now it also checks whether the REST API being accessed is only allowed
>>>> to users with sudo rights.
>>>>
>>>> The necessity to have sudo rights to access a REST API can be easily
>>>> configured through the UrlSubNode decorator. Similar to the support
>>>> previously implemented for user authentication in UrlSubNode, an
>>>> additional boolean parameter was added to UrlSubNode to indicate
>>>> whether
>>>> the user needs sudo rights in order to access the corresponding REST
>>>> API.
>>>>
>>>> Signed-off-by: Leonardo Garcia <lagarcia at br.ibm.com>
>>>> ---
>>>>   src/kimchi/auth.py          | 10 +++++++---
>>>>   src/kimchi/control/utils.py |  4 +++-
>>>>   src/kimchi/server.py        |  2 ++
>>>>   3 files changed, 12 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
>>>> index 3ffe4b1..b3d1edf 100644
>>>> --- a/src/kimchi/auth.py
>>>> +++ b/src/kimchi/auth.py
>>>> @@ -190,12 +190,16 @@ def logout():
>>>>       cherrypy.lib.sessions.expire()
>>>>
>>>>
>>>> -def kimchiauth(*args, **kwargs):
>>>> +def kimchiauth(needs_admin=False):
>>>>       debug("Entering kimchiauth...")
>>>> -    if check_auth_session():
>>>> +    if check_auth_session() and \
>>>> +       (not needs_admin or (cherrypy.session[USER_SUDO] ==
>>>> needs_admin)):
>>>> +        debug(str(cherrypy.session[USER_SUDO]))
>>>>           return
>>>>
>>>> -    if check_auth_httpba():
>>>> +    if check_auth_httpba() and \
>>>> +       (not needs_admin or (cherrypy.session[USER_SUDO] ==
>>>> needs_admin)):
>>>> +        debug(str(cherrypy.session[USER_SUDO]))
>>>>           return
>>>>
>>>>       if not from_browser():
>>>> diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py
>>>> index 9c6878b..4567af7 100644
>>>> --- a/src/kimchi/control/utils.py
>>>> +++ b/src/kimchi/control/utils.py
>>>> @@ -107,13 +107,15 @@ def validate_params(params, instance, action):
>>>>
>>>>
>>>>   class UrlSubNode(object):
>>>> -    def __init__(self, name, auth=False):
>>>> +    def __init__(self, name, auth=False, needs_admin=False):
>>> We also need to have a list of which methods are exclusive for admin
>>> For example, any kind of user can do GET operations, but POST, PUT
>>> and DELETE are only available for admin
>>>
>>> def __init__(self, name, auth=False, needs_admin=False,
>>> admin_methods=[])
>>>     fun.admin_methods = admin_methods
>>>
>>> And in kimchiauth()
>>>
>>> method = cherrypy.request.method.upper()
>>> if method in [admin_methods]:
>>>     # needs sudo
>>>
>> Or instead of pass admin_methods() we assume in kimchiauth() only GET
>> method does not require admin access.
> Yes, this is a better approach, definitely.
Hmmm... I've been thinking about this and I think that it is wrong to
assume that GET method will not require admin access at all times. I
think there might be some REST APIs for which no even GET should be
permitted for non-admin user. So, I'll send v2 with a patch similar to
what you proposed above, but with only one argument: needs_admin will be
a list of methods that need admin rights to be accessed.

Best regards,

Leonardo Garcia
>
> I'll include this check in v2.
>
> Best regards,
>
> Leonardo Garcia
>>>>           self.name = name
>>>>           self.auth = auth
>>>> +        self.needs_admin = needs_admin
>>>>
>>>>       def __call__(self, fun):
>>>>           fun._url_sub_node_name = {"name": self.name}
>>>>           fun.url_auth = self.auth
>>>> +        fun.needs_admin = self.needs_admin
>>>>           return fun
>>>>
>>>>
>>>> diff --git a/src/kimchi/server.py b/src/kimchi/server.py
>>>> index 1e131b4..469db68 100644
>>>> --- a/src/kimchi/server.py
>>>> +++ b/src/kimchi/server.py
>>>> @@ -191,6 +191,8 @@ class Server(object):
>>>>           for ident, node in sub_nodes.items():
>>>>               if node.url_auth:
>>>>                   self.configObj["/%s" % ident] =
>>>> {'tools.kimchiauth.on': True}
>>>> +                if node.needs_admin:
>>>> +                    self.configObj["/%s" %
>>>> ident]['tools.kimchiauth.needs_admin']  = True
>>>>
>>>>           self.app = cherrypy.tree.mount(KimchiRoot(model_instance,
>>>> dev_env),
>>>>                                          config=self.configObj)
>>> _______________________________________________
>>> Kimchi-devel mailing list
>>> Kimchi-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>