4:32 p.m.
Sheldon,
Thanks for the comments.
On 02/10/2014 12:40 PM, Sheldon wrote:
a minor comment below
On 02/10/2014 10:32 AM, Leonardo Garcia wrote:
> From: Leonardo Garcia <lagarcia(a)br.ibm.com>
>
> When the /login REST API is called with a valid username and password,
> Kimchi will find out the user groups and sudo status, store these values
> in sessions variables, and return them in a JSON message with the
> following format:
>
> {"sudo": true | false, "userid": "<username>",
"groups": [<list of
> groups>]}
>
> For example:
>
> {"sudo": false, "userid": "guest", "groups":
["guest", "foo"]}
>
> This JSON return message can be used by the UI to figure out which
> tabs/options should be shown in the web interface according to the user
> rights.
>
> It is important to notice that sudo configuration for a given user is
> not as simple as an on/off key. Sudo permits various levels of access
> controls. For instance, it is possible to specify which executables a
> user can run with sudo. For the purpose of this implementation, we only
> consider a user to have sudo rights if this user can run any command in
> the system with sudo (which is equivalent to have root rights).
>
> Signed-off-by: Leonardo Garcia <lagarcia(a)br.ibm.com>
> ---
> src/kimchi/auth.py | 60
> ++++++++++++++++++++++++++++++++++++++++++++++++++----
> src/kimchi/root.py | 4 ++--
> 2 files changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py
> index f9873ca..3ffe4b1 100644
> --- a/src/kimchi/auth.py
> +++ b/src/kimchi/auth.py
> @@ -30,9 +30,12 @@ import re
>
> from kimchi import template
> from kimchi.exception import OperationFailed
> +from kimchi.utils import run_command, parse_cmd_output
>
>
> -SESSION_USER = 'userid'
> +USER_ID = 'userid'
> +USER_GROUPS = 'groups'
> +USER_SUDO = 'sudo'
>
>
> def debug(msg):
> @@ -40,6 +43,52 @@ def debug(msg):
> # cherrypy.log.error(msg)
>
>
> +class User(object):
> +
> + def __init__(self, userid):
> + self.user = {}
> + self.user[USER_ID] = userid
should userid -> username?
Well, I am OK with either name. As userid was being
used in the code, I
just kept using it.
> + self.user[USER_GROUPS] = []
> + self.user[USER_SUDO] = False
> +
> + def get_groups(self):
> + out, err, exit = run_command(['groups', self.user[USER_ID]])
> + # the output of the groups command is of the form:
> + # <userid> : <list of groups separated by space>
> + if exit == 0:
> + out = parse_cmd_output(out, ['userid', 'groups'],
':')
> + self.user[USER_GROUPS] = out[0]['groups'].split()
> + debug("Groups for %s: %s" % (self.user[USER_ID],
> + self.user[USER_GROUPS]))
> + return self.user[USER_GROUPS]
> +
How about ?
+import grp
+ def get_groups(self):
+ return [g.gr_name for g in grp.getgrall() if user in g.gr_mem]
Yeah,
definitely! Thanks for the suggestion. I'll send a v2 with this
change. I was not aware of the grp package in python.
>
> + def has_sudo(self):
> + out, err, exit = run_command(['sudo', '-l', '-U',
> self.user[USER_ID],
> + 'sudo'])
> + if exit == 0:
> + debug("User %s is allowed to run sudo" %
> self.user[USER_ID])
> + # sudo allows a wide range of configurations, such as
> controlling
> + # which binaries the user can execute with sudo.
> + # For now, we will just check whether the user is
> allowed to run any
> + # command with sudo.
> + out, err, exit = run_command(['sudo', '-l',
'-U',
> + self.user[USER_ID]])
> + for line in out.split('\n'):
> + if line and re.search("(ALL)", line):
(ALL) is OK for sudo?
sometimes, even a user in sudo list, but it can not run all commands.
Yes, you are
correct. As I stated in the commit message, it is important
to notice that sudo configuration for a given user is not as simple as
an on/off key. As you said above, you might allow a user to run some
commands as sudo and not all commands. For the purpose of this initial
implementation, we are considering someone to have sudo rights only if
they have access to all commands with sudo, which means they will have
(ALL) as the indicator they have access to all system commands through
sudo (which is equivalent to have root rights). All other users, even
the ones that have sudo access to only some commands will be considered
not to have sudo rights under Kimchi --- kimchid runs as root, so it
should only make sense to give total power to a user that have sudo
rights for all commands, just as root have.
Do you agree?
Best regards,
Leonardo Garcia
> + self.user[USER_SUDO] = True
> + debug("User %s can run any command with sudo" %
> + self.user[USER_ID])
> + return self.user[USER_SUDO]
> + debug("User %s can only run some commands with sudo" %
> + self.user[USER_ID])
> + else:
> + debug("User %s is not allowed to run sudo" %
> self.user[USER_ID])
> + return self.user[USER_SUDO]
> +
> + def get_user(self):
> + return self.user
> +
> +
> def authenticate(username, password, service="passwd"):
> '''Returns True if authenticate is OK via PAM.'''
> def _pam_conv(auth, query_list, userData=None):
> @@ -122,13 +171,16 @@ def check_auth_httpba():
> def login(userid, password):
> if not authenticate(userid, password):
> debug("User cannot be verified with the supplied password")
> - return False
> + return None
> + user = User(userid)
> debug("User verified, establishing session")
> cherrypy.session.acquire_lock()
> cherrypy.session.regenerate()
> - cherrypy.session[SESSION_USER] = cherrypy.request.login = userid
> + cherrypy.session[USER_ID] = userid
> + cherrypy.session[USER_GROUPS] = user.get_groups()
> + cherrypy.session[USER_SUDO] = user.has_sudo()
> cherrypy.session.release_lock()
> - return True
> + return user.get_user()
>
>
> def logout():
> diff --git a/src/kimchi/root.py b/src/kimchi/root.py
> index ce4a49c..8e90b7f 100644
> --- a/src/kimchi/root.py
> +++ b/src/kimchi/root.py
> @@ -106,11 +106,11 @@ class KimchiRoot(Root):
> raise cherrypy.HTTPError(400, "Missing parameter:
'%s'"
> % key)
>
> try:
> - auth.login(userid, password)
> + user_info = auth.login(userid, password)
> except OperationFailed:
> raise cherrypy.HTTPError(401)
>
> - return '{}'
> + return json.dumps(user_info)
>
> @cherrypy.expose
> def logout(self):