[PATCH v2 0/5] Add users and groups to VMs
Here are the differences between the previous patchset (v1) and this one: - New patch "Use proper term "user name" instead of "user id"" has been added. - Patch "Return users and groups when fetching VM info" has been updated to log the exception message when there is an invalid metadata value in a VM. Notice that an invalid metadata value can only be written to the VM XML by external access; users from within Kimchi will always store correct value through the app. - Rebased to 5d82241. Crístian Viana (5): Override only the updated "User" methods in "patch_auth" Use proper term "user name" instead of "user id" Add functions to check if a user/group exists Return users and groups when fetching VM info Add/remove users and groups to VMs docs/API.md | 6 ++++ po/en_US.po | 2 +- po/kimchi.pot | 2 +- po/pt_BR.po | 2 +- po/zh_CN.po | 2 +- src/kimchi/API.json | 22 ++++++++++++++ src/kimchi/auth.py | 60 ++++++++++++++++++++++++++------------ src/kimchi/control/vms.py | 6 ++-- src/kimchi/i18n.py | 8 +++++- src/kimchi/mockmodel.py | 4 ++- src/kimchi/model/vms.py | 62 ++++++++++++++++++++++++++++++++++++---- src/kimchi/root.py | 4 +-- tests/test_authorization.py | 19 ++++++++++++ tests/test_mockmodel.py | 2 +- tests/test_model.py | 44 +++++++++++++++++++++++++++- tests/test_rest.py | 10 ++++--- tests/utils.py | 26 +++++------------ ui/js/src/kimchi.login_window.js | 16 +++++------ ui/js/src/kimchi.user.js | 14 ++++----- ui/pages/login-window.html.tmpl | 4 +-- 20 files changed, 239 insertions(+), 76 deletions(-) -- 1.8.5.3
The function "patch_auth" creates a fake class to override the existing kimchi.auth.User with a few updated methods. That attribution overrides the entire User class along with all its methods. In order to avoid side effects, override only the methods we need to change in the "User" class. Other methods not related to "patch_auth" will not be affected now. --- tests/utils.py | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 0a1a967..b373f34 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -149,25 +149,12 @@ def patch_auth(sudo=True): Override the authenticate function with a simple test against an internal dict of users and passwords. """ - USER_ID = 'userid' - USER_GROUPS = 'groups' - USER_SUDO = 'sudo' - class _User(object): - def __init__(self, userid): - self.user = {} - self.user[USER_ID] = userid - self.user[USER_GROUPS] = None - self.user[USER_SUDO] = sudo + def _get_groups(self): + return None - def get_groups(self): - return self.user[USER_GROUPS] - - def has_sudo(self): - return self.user[USER_SUDO] - - def get_user(self): - return self.user + def _has_sudo(self): + return sudo def _authenticate(username, password, service="passwd"): try: @@ -178,7 +165,8 @@ def patch_auth(sudo=True): import kimchi.auth kimchi.auth.authenticate = _authenticate - kimchi.auth.User = _User + kimchi.auth.User.get_groups = _get_groups + kimchi.auth.User.has_sudo = _has_sudo def normalize_xml(xml_str): -- 1.8.5.3
In the Linux environment, a user ID is an integer code which identifies a user; a user name is the human friendly text identifier of that user. Kimchi uses both terms interchangeably. Rename all occurrences of "userid" (and its variants) to "username" (and its variants) in external UI messages and internal code. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- po/en_US.po | 2 +- po/kimchi.pot | 2 +- po/pt_BR.po | 2 +- po/zh_CN.po | 2 +- src/kimchi/auth.py | 38 +++++++++++++++++++------------------- src/kimchi/i18n.py | 2 +- src/kimchi/root.py | 4 ++-- tests/test_rest.py | 8 ++++---- tests/utils.py | 2 +- ui/js/src/kimchi.login_window.js | 16 ++++++++-------- ui/js/src/kimchi.user.js | 14 +++++++------- ui/pages/login-window.html.tmpl | 4 ++-- 12 files changed, 48 insertions(+), 48 deletions(-) diff --git a/po/en_US.po b/po/en_US.po index aeff16e..e34bd27 100644 --- a/po/en_US.po +++ b/po/en_US.po @@ -609,7 +609,7 @@ msgid "Datastore is not initiated in the model object." msgstr "" #, python-format -msgid "Authentication failed for user '%(userid)s'. [Error code: %(code)s]" +msgid "Authentication failed for user '%(username)s'. [Error code: %(code)s]" msgstr "" msgid "You are not authorized to access Kimchi" diff --git a/po/kimchi.pot b/po/kimchi.pot index abb7219..91fe9dc 100755 --- a/po/kimchi.pot +++ b/po/kimchi.pot @@ -597,7 +597,7 @@ msgid "Datastore is not initiated in the model object." msgstr "" #, python-format -msgid "Authentication failed for user '%(userid)s'. [Error code: %(code)s]" +msgid "Authentication failed for user '%(username)s'. [Error code: %(code)s]" msgstr "" msgid "You are not authorized to access Kimchi" diff --git a/po/pt_BR.po b/po/pt_BR.po index db7c579..7c0612a 100644 --- a/po/pt_BR.po +++ b/po/pt_BR.po @@ -624,7 +624,7 @@ msgid "Datastore is not initiated in the model object." msgstr "" #, python-format -msgid "Authentication failed for user '%(userid)s'. [Error code: %(code)s]" +msgid "Authentication failed for user '%(username)s'. [Error code: %(code)s]" msgstr "" msgid "You are not authorized to access Kimchi" diff --git a/po/zh_CN.po b/po/zh_CN.po index 0439b04..e8ce600 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -613,7 +613,7 @@ msgid "Datastore is not initiated in the model object." msgstr "" #, python-format -msgid "Authentication failed for user '%(userid)s'. [Error code: %(code)s]" +msgid "Authentication failed for user '%(username)s'. [Error code: %(code)s]" msgstr "" msgid "You are not authorized to access Kimchi" diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index f8ccea1..990fa84 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -29,7 +29,7 @@ from kimchi.exception import InvalidOperation, OperationFailed from kimchi.utils import run_command -USER_ID = 'userid' +USER_NAME = 'username' USER_GROUPS = 'groups' USER_SUDO = 'sudo' @@ -41,38 +41,38 @@ def debug(msg): class User(object): - def __init__(self, userid): + def __init__(self, username): self.user = {} - self.user[USER_ID] = userid + self.user[USER_NAME] = username self.user[USER_GROUPS] = None self.user[USER_SUDO] = False def get_groups(self): self.user[USER_GROUPS] = [g.gr_name for g in grp.getgrall() - if self.user[USER_ID] in g.gr_mem] + if self.user[USER_NAME] in g.gr_mem] return self.user[USER_GROUPS] def has_sudo(self): - out, err, exit = run_command(['sudo', '-l', '-U', self.user[USER_ID], + out, err, exit = run_command(['sudo', '-l', '-U', self.user[USER_NAME], 'sudo']) if exit == 0: - debug("User %s is allowed to run sudo" % self.user[USER_ID]) + debug("User %s is allowed to run sudo" % self.user[USER_NAME]) # 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]]) + self.user[USER_NAME]]) for line in out.split('\n'): if line and re.search("(ALL)", line): self.user[USER_SUDO] = True debug("User %s can run any command with sudo" % - self.user[USER_ID]) + self.user[USER_NAME]) return self.user[USER_SUDO] debug("User %s can only run some commands with sudo" % - self.user[USER_ID]) + self.user[USER_NAME]) else: - debug("User %s is not allowed to run sudo" % self.user[USER_ID]) + debug("User %s is not allowed to run sudo" % self.user[USER_NAME]) return self.user[USER_SUDO] def get_user(self): @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'username': username, 'code': code} raise OperationFailed("KCHAUTH0001E", msg_args) return True @@ -127,7 +127,7 @@ def check_auth_session(): for the user. """ cherrypy.session.acquire_lock() - session = cherrypy.session.get(USER_ID, None) + session = cherrypy.session.get(USER_NAME, None) cherrypy.session.release_lock() if session is not None: debug("Session authenticated for user %s" % session) @@ -156,20 +156,20 @@ def check_auth_httpba(): b64data = re.sub("Basic ", "", authheader) decodeddata = base64.b64decode(b64data.encode("ASCII")) # TODO: test how this handles ':' characters in username/passphrase. - userid, password = decodeddata.decode().split(":", 1) + username, password = decodeddata.decode().split(":", 1) - return login(userid, password) + return login(username, password) -def login(userid, password): - if not authenticate(userid, password): +def login(username, password): + if not authenticate(username, password): debug("User cannot be verified with the supplied password") return None - user = User(userid) + user = User(username) debug("User verified, establishing session") cherrypy.session.acquire_lock() cherrypy.session.regenerate() - cherrypy.session[USER_ID] = userid + cherrypy.session[USER_NAME] = username cherrypy.session[USER_GROUPS] = user.get_groups() cherrypy.session[USER_SUDO] = user.has_sudo() cherrypy.session.release_lock() @@ -178,7 +178,7 @@ def login(userid, password): def logout(): cherrypy.session.acquire_lock() - cherrypy.session[USER_ID] = None + cherrypy.session[USER_NAME] = None cherrypy.session.release_lock() cherrypy.lib.sessions.expire() diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index f3e1803..374bbcd 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -33,7 +33,7 @@ messages = { "KCHASYNC0001E": _("Datastore is not initiated in the model object."), - "KCHAUTH0001E": _("Authentication failed for user '%(userid)s'. [Error code: %(code)s]"), + "KCHAUTH0001E": _("Authentication failed for user '%(username)s'. [Error code: %(code)s]"), "KCHAUTH0002E": _("You are not authorized to access Kimchi"), "KCHAUTH0003E": _("Specify %(item)s to login into Kimchi"), "KCHAUTH0004E": _("This operation is not allowed as you have restricted access to Kimchi."), diff --git a/src/kimchi/root.py b/src/kimchi/root.py index 3956ea6..1b2a651 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.py @@ -99,14 +99,14 @@ class KimchiRoot(Root): def login(self, *args): params = parse_request() try: - userid = params['userid'] + username = params['username'] password = params['password'] except KeyError, item: e = MissingParameter('KCHAUTH0003E', {'item': str(item)}) raise cherrypy.HTTPError(400, e.message) try: - user_info = auth.login(userid, password) + user_info = auth.login(username, password) except OperationFailed: raise cherrypy.HTTPError(401) diff --git a/tests/test_rest.py b/tests/test_rest.py index 54530f3..8cfa2a2 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -105,7 +105,7 @@ class RestTests(unittest.TestCase): # HTTP:401. Since HTTP Simple Auth is not allowed for text/html, we # need to use the login API and establish a session. user, pw = fake_user.items()[0] - req = json.dumps({'userid': user, 'password': pw}) + req = json.dumps({'username': user, 'password': pw}) resp = self.request('/login', req, 'POST') self.assertEquals(200, resp.status) cookie = resp.getheader('set-cookie') @@ -1336,7 +1336,7 @@ class RestTests(unittest.TestCase): self.assertEquals(200, resp.status) user, pw = fake_user.items()[0] - req = json.dumps({'userid': user, 'password': pw}) + req = json.dumps({'username': user, 'password': pw}) resp = self.request('/login', req, 'POST', hdrs) self.assertEquals(200, resp.status) @@ -1357,7 +1357,7 @@ class RestTests(unittest.TestCase): # Test REST API hdrs = {'AUTHORIZATION': ''} - req = json.dumps({'userid': 'nouser', 'password': 'badpass'}) + req = json.dumps({'username': 'nouser', 'password': 'badpass'}) resp = self.request('/login', req, 'POST', hdrs) self.assertEquals(401, resp.status) @@ -1382,7 +1382,7 @@ class RestTests(unittest.TestCase): # Execute a login call user, pw = fake_user.items()[0] - req = json.dumps({'userid': user, 'password': pw}) + req = json.dumps({'username': user, 'password': pw}) resp = self.request('/login', req, 'POST', hdrs) self.assertEquals(200, resp.status) cookie = resp.getheader('set-cookie') diff --git a/tests/utils.py b/tests/utils.py index b373f34..fe03a1a 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -160,7 +160,7 @@ def patch_auth(sudo=True): try: return fake_user[username] == password except KeyError, e: - raise OperationFailed("KCHAUTH0001E", {'userid': 'username', + raise OperationFailed("KCHAUTH0001E", {'username': 'username', 'code': e.message}) import kimchi.auth diff --git a/ui/js/src/kimchi.login_window.js b/ui/js/src/kimchi.login_window.js index 22d74e0..9af3805 100644 --- a/ui/js/src/kimchi.login_window.js +++ b/ui/js/src/kimchi.login_window.js @@ -56,10 +56,10 @@ kimchi.login_main = function() { return; } - var userName = kimchi.user.getUserID(); - userName && $('#user-id').val(userName); + var userName = kimchi.user.getUserName(); + userName && $('#username').val(userName); - var nodeToFocus = ! $('#user-id').val() ? $('#user-id') : + var nodeToFocus = ! $('#username').val() ? $('#username') : (! $('#password').val() ? $('#password') : $('#btn-login')); $(nodeToFocus).focus(); @@ -67,16 +67,16 @@ kimchi.login_main = function() { var login = function(event) { - if (!validateNonEmpty(['user-id', 'password'])) { + if (!validateNonEmpty(['username', 'password'])) { return false; } $('#btn-login').text(i18n['KCHAUTH6002M']).prop('disabled', true); - var userID = $('#user-id').val(); - userID && kimchi.user.setUserID(userID); + var userName = $('#username').val(); + userName && kimchi.user.setUserName(userName); var settings = { - userid: userID, + username: userName, password: $("#password").val() }; @@ -93,7 +93,7 @@ kimchi.login_main = function() { }, function() { $('#message-container').text(i18n['KCHAUTH6001E']); $('#btn-login').prop('disabled', false).text(i18n['KCHAUTH6001M']); - placeCursor('user-id'); + placeCursor('username'); }); return false; diff --git a/ui/js/src/kimchi.user.js b/ui/js/src/kimchi.user.js index bd7d20b..9134849 100644 --- a/ui/js/src/kimchi.user.js +++ b/ui/js/src/kimchi.user.js @@ -16,17 +16,17 @@ * limitations under the License. */ kimchi.user = (function() { - var getUserID = function() { - return kimchi.cookie.get('userid'); + var getUserName = function() { + return kimchi.cookie.get('username'); }; - var setUserID = function(userID) { - kimchi.cookie.set('userid', userID, 365); + var setUserName = function(userName) { + kimchi.cookie.set('username', userName, 365); }; var showUser = function(toShow) { if (toShow) { - var userName = getUserID(); + var userName = getUserName(); userName && $('#user-name').text(userName); $('#user').removeClass('not-logged-in'); return; @@ -36,8 +36,8 @@ kimchi.user = (function() { }; return { - getUserID: getUserID, - setUserID: setUserID, + getUserName: getUserName, + setUserName: setUserName, showUser: showUser }; })(); diff --git a/ui/pages/login-window.html.tmpl b/ui/pages/login-window.html.tmpl index dfcb6b0..b600377 100644 --- a/ui/pages/login-window.html.tmpl +++ b/ui/pages/login-window.html.tmpl @@ -35,8 +35,8 @@ <form id="form-login" action="/login" method="POST"> <div id="message-container" class="row error-message"></div> <div class="row"> - <input type="text" id="user-id" name="userid" required="required" placeholder="$_("User Name")" /> - <div id="user-id-msg" class="msg-required"></div> + <input type="text" id="username" name="username" required="required" placeholder="$_("User Name")" /> + <div id="username-msg" class="msg-required"></div> </div> <div class="row"> <input type="password" id="password" name="password" required="required" placeholder="$_("Password")" /> -- 1.8.5.3
Reviewed-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> That's great. I have give comments when the auth patch. Why use "user id" instead of "user name". But no response. On 03/01/2014 02:40 AM, Crístian Viana wrote:
In the Linux environment, a user ID is an integer code which identifies a user; a user name is the human friendly text identifier of that user. Kimchi uses both terms interchangeably.
Rename all occurrences of "userid" (and its variants) to "username" (and its variants) in external UI messages and internal code.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- po/en_US.po | 2 +- po/kimchi.pot | 2 +- po/pt_BR.po | 2 +- po/zh_CN.po | 2 +- src/kimchi/auth.py | 38 +++++++++++++++++++------------------- src/kimchi/i18n.py | 2 +- src/kimchi/root.py | 4 ++-- tests/test_rest.py | 8 ++++---- tests/utils.py | 2 +- ui/js/src/kimchi.login_window.js | 16 ++++++++-------- ui/js/src/kimchi.user.js | 14 +++++++------- ui/pages/login-window.html.tmpl | 4 ++-- 12 files changed, 48 insertions(+), 48 deletions(-)
diff --git a/po/en_US.po b/po/en_US.po index aeff16e..e34bd27 100644 --- a/po/en_US.po +++ b/po/en_US.po @@ -609,7 +609,7 @@ msgid "Datastore is not initiated in the model object." msgstr ""
#, python-format -msgid "Authentication failed for user '%(userid)s'. [Error code: %(code)s]" +msgid "Authentication failed for user '%(username)s'. [Error code: %(code)s]" msgstr ""
msgid "You are not authorized to access Kimchi" diff --git a/po/kimchi.pot b/po/kimchi.pot index abb7219..91fe9dc 100755 --- a/po/kimchi.pot +++ b/po/kimchi.pot @@ -597,7 +597,7 @@ msgid "Datastore is not initiated in the model object." msgstr ""
#, python-format -msgid "Authentication failed for user '%(userid)s'. [Error code: %(code)s]" +msgid "Authentication failed for user '%(username)s'. [Error code: %(code)s]" msgstr ""
msgid "You are not authorized to access Kimchi" diff --git a/po/pt_BR.po b/po/pt_BR.po index db7c579..7c0612a 100644 --- a/po/pt_BR.po +++ b/po/pt_BR.po @@ -624,7 +624,7 @@ msgid "Datastore is not initiated in the model object." msgstr ""
#, python-format -msgid "Authentication failed for user '%(userid)s'. [Error code: %(code)s]" +msgid "Authentication failed for user '%(username)s'. [Error code: %(code)s]" msgstr ""
msgid "You are not authorized to access Kimchi" diff --git a/po/zh_CN.po b/po/zh_CN.po index 0439b04..e8ce600 100644 --- a/po/zh_CN.po +++ b/po/zh_CN.po @@ -613,7 +613,7 @@ msgid "Datastore is not initiated in the model object." msgstr ""
#, python-format -msgid "Authentication failed for user '%(userid)s'. [Error code: %(code)s]" +msgid "Authentication failed for user '%(username)s'. [Error code: %(code)s]" msgstr ""
msgid "You are not authorized to access Kimchi" diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index f8ccea1..990fa84 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -29,7 +29,7 @@ from kimchi.exception import InvalidOperation, OperationFailed from kimchi.utils import run_command
-USER_ID = 'userid' +USER_NAME = 'username' USER_GROUPS = 'groups' USER_SUDO = 'sudo'
@@ -41,38 +41,38 @@ def debug(msg):
class User(object):
- def __init__(self, userid): + def __init__(self, username): self.user = {} - self.user[USER_ID] = userid + self.user[USER_NAME] = username self.user[USER_GROUPS] = None self.user[USER_SUDO] = False
def get_groups(self): self.user[USER_GROUPS] = [g.gr_name for g in grp.getgrall() - if self.user[USER_ID] in g.gr_mem] + if self.user[USER_NAME] in g.gr_mem] return self.user[USER_GROUPS]
def has_sudo(self): - out, err, exit = run_command(['sudo', '-l', '-U', self.user[USER_ID], + out, err, exit = run_command(['sudo', '-l', '-U', self.user[USER_NAME], 'sudo']) if exit == 0: - debug("User %s is allowed to run sudo" % self.user[USER_ID]) + debug("User %s is allowed to run sudo" % self.user[USER_NAME]) # 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]]) + self.user[USER_NAME]]) for line in out.split('\n'): if line and re.search("(ALL)", line): self.user[USER_SUDO] = True debug("User %s can run any command with sudo" % - self.user[USER_ID]) + self.user[USER_NAME]) return self.user[USER_SUDO] debug("User %s can only run some commands with sudo" % - self.user[USER_ID]) + self.user[USER_NAME]) else: - debug("User %s is not allowed to run sudo" % self.user[USER_ID]) + debug("User %s is not allowed to run sudo" % self.user[USER_NAME]) return self.user[USER_SUDO]
def get_user(self): @@ -107,7 +107,7 @@ def authenticate(username, password, service="passwd"): try: auth.authenticate() except PAM.error, (resp, code): - msg_args = {'userid': username, 'code': code} + msg_args = {'username': username, 'code': code} raise OperationFailed("KCHAUTH0001E", msg_args)
return True @@ -127,7 +127,7 @@ def check_auth_session(): for the user. """ cherrypy.session.acquire_lock() - session = cherrypy.session.get(USER_ID, None) + session = cherrypy.session.get(USER_NAME, None) cherrypy.session.release_lock() if session is not None: debug("Session authenticated for user %s" % session) @@ -156,20 +156,20 @@ def check_auth_httpba(): b64data = re.sub("Basic ", "", authheader) decodeddata = base64.b64decode(b64data.encode("ASCII")) # TODO: test how this handles ':' characters in username/passphrase. - userid, password = decodeddata.decode().split(":", 1) + username, password = decodeddata.decode().split(":", 1)
- return login(userid, password) + return login(username, password)
-def login(userid, password): - if not authenticate(userid, password): +def login(username, password): + if not authenticate(username, password): debug("User cannot be verified with the supplied password") return None - user = User(userid) + user = User(username) debug("User verified, establishing session") cherrypy.session.acquire_lock() cherrypy.session.regenerate() - cherrypy.session[USER_ID] = userid + cherrypy.session[USER_NAME] = username cherrypy.session[USER_GROUPS] = user.get_groups() cherrypy.session[USER_SUDO] = user.has_sudo() cherrypy.session.release_lock() @@ -178,7 +178,7 @@ def login(userid, password):
def logout(): cherrypy.session.acquire_lock() - cherrypy.session[USER_ID] = None + cherrypy.session[USER_NAME] = None cherrypy.session.release_lock() cherrypy.lib.sessions.expire()
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index f3e1803..374bbcd 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -33,7 +33,7 @@ messages = {
"KCHASYNC0001E": _("Datastore is not initiated in the model object."),
- "KCHAUTH0001E": _("Authentication failed for user '%(userid)s'. [Error code: %(code)s]"), + "KCHAUTH0001E": _("Authentication failed for user '%(username)s'. [Error code: %(code)s]"), "KCHAUTH0002E": _("You are not authorized to access Kimchi"), "KCHAUTH0003E": _("Specify %(item)s to login into Kimchi"), "KCHAUTH0004E": _("This operation is not allowed as you have restricted access to Kimchi."), diff --git a/src/kimchi/root.py b/src/kimchi/root.py index 3956ea6..1b2a651 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.py @@ -99,14 +99,14 @@ class KimchiRoot(Root): def login(self, *args): params = parse_request() try: - userid = params['userid'] + username = params['username'] password = params['password'] except KeyError, item: e = MissingParameter('KCHAUTH0003E', {'item': str(item)}) raise cherrypy.HTTPError(400, e.message)
try: - user_info = auth.login(userid, password) + user_info = auth.login(username, password) except OperationFailed: raise cherrypy.HTTPError(401)
diff --git a/tests/test_rest.py b/tests/test_rest.py index 54530f3..8cfa2a2 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -105,7 +105,7 @@ class RestTests(unittest.TestCase): # HTTP:401. Since HTTP Simple Auth is not allowed for text/html, we # need to use the login API and establish a session. user, pw = fake_user.items()[0] - req = json.dumps({'userid': user, 'password': pw}) + req = json.dumps({'username': user, 'password': pw}) resp = self.request('/login', req, 'POST') self.assertEquals(200, resp.status) cookie = resp.getheader('set-cookie') @@ -1336,7 +1336,7 @@ class RestTests(unittest.TestCase): self.assertEquals(200, resp.status)
user, pw = fake_user.items()[0] - req = json.dumps({'userid': user, 'password': pw}) + req = json.dumps({'username': user, 'password': pw}) resp = self.request('/login', req, 'POST', hdrs) self.assertEquals(200, resp.status)
@@ -1357,7 +1357,7 @@ class RestTests(unittest.TestCase):
# Test REST API hdrs = {'AUTHORIZATION': ''} - req = json.dumps({'userid': 'nouser', 'password': 'badpass'}) + req = json.dumps({'username': 'nouser', 'password': 'badpass'}) resp = self.request('/login', req, 'POST', hdrs) self.assertEquals(401, resp.status)
@@ -1382,7 +1382,7 @@ class RestTests(unittest.TestCase):
# Execute a login call user, pw = fake_user.items()[0] - req = json.dumps({'userid': user, 'password': pw}) + req = json.dumps({'username': user, 'password': pw}) resp = self.request('/login', req, 'POST', hdrs) self.assertEquals(200, resp.status) cookie = resp.getheader('set-cookie') diff --git a/tests/utils.py b/tests/utils.py index b373f34..fe03a1a 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -160,7 +160,7 @@ def patch_auth(sudo=True): try: return fake_user[username] == password except KeyError, e: - raise OperationFailed("KCHAUTH0001E", {'userid': 'username', + raise OperationFailed("KCHAUTH0001E", {'username': 'username', 'code': e.message})
import kimchi.auth diff --git a/ui/js/src/kimchi.login_window.js b/ui/js/src/kimchi.login_window.js index 22d74e0..9af3805 100644 --- a/ui/js/src/kimchi.login_window.js +++ b/ui/js/src/kimchi.login_window.js @@ -56,10 +56,10 @@ kimchi.login_main = function() { return; }
- var userName = kimchi.user.getUserID(); - userName && $('#user-id').val(userName); + var userName = kimchi.user.getUserName(); + userName && $('#username').val(userName);
- var nodeToFocus = ! $('#user-id').val() ? $('#user-id') : + var nodeToFocus = ! $('#username').val() ? $('#username') : (! $('#password').val() ? $('#password') : $('#btn-login'));
$(nodeToFocus).focus(); @@ -67,16 +67,16 @@ kimchi.login_main = function() {
var login = function(event) {
- if (!validateNonEmpty(['user-id', 'password'])) { + if (!validateNonEmpty(['username', 'password'])) { return false; }
$('#btn-login').text(i18n['KCHAUTH6002M']).prop('disabled', true);
- var userID = $('#user-id').val(); - userID && kimchi.user.setUserID(userID); + var userName = $('#username').val(); + userName && kimchi.user.setUserName(userName); var settings = { - userid: userID, + username: userName, password: $("#password").val() };
@@ -93,7 +93,7 @@ kimchi.login_main = function() { }, function() { $('#message-container').text(i18n['KCHAUTH6001E']); $('#btn-login').prop('disabled', false).text(i18n['KCHAUTH6001M']); - placeCursor('user-id'); + placeCursor('username'); });
return false; diff --git a/ui/js/src/kimchi.user.js b/ui/js/src/kimchi.user.js index bd7d20b..9134849 100644 --- a/ui/js/src/kimchi.user.js +++ b/ui/js/src/kimchi.user.js @@ -16,17 +16,17 @@ * limitations under the License. */ kimchi.user = (function() { - var getUserID = function() { - return kimchi.cookie.get('userid'); + var getUserName = function() { + return kimchi.cookie.get('username'); };
- var setUserID = function(userID) { - kimchi.cookie.set('userid', userID, 365); + var setUserName = function(userName) { + kimchi.cookie.set('username', userName, 365); };
var showUser = function(toShow) { if (toShow) { - var userName = getUserID(); + var userName = getUserName(); userName && $('#user-name').text(userName); $('#user').removeClass('not-logged-in'); return; @@ -36,8 +36,8 @@ kimchi.user = (function() { };
return { - getUserID: getUserID, - setUserID: setUserID, + getUserName: getUserName, + setUserName: setUserName, showUser: showUser }; })(); diff --git a/ui/pages/login-window.html.tmpl b/ui/pages/login-window.html.tmpl index dfcb6b0..b600377 100644 --- a/ui/pages/login-window.html.tmpl +++ b/ui/pages/login-window.html.tmpl @@ -35,8 +35,8 @@ <form id="form-login" action="/login" method="POST"> <div id="message-container" class="row error-message"></div> <div class="row"> - <input type="text" id="user-id" name="userid" required="required" placeholder="$_("User Name")" /> - <div id="user-id-msg" class="msg-required"></div> + <input type="text" id="username" name="username" required="required" placeholder="$_("User Name")" /> + <div id="username-msg" class="msg-required"></div> </div> <div class="row"> <input type="password" id="password" name="password" required="required" placeholder="$_("Password")" />
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
The user/group validation is done on the current system. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- src/kimchi/auth.py | 22 ++++++++++++++++++++++ tests/test_authorization.py | 19 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index 990fa84..b783401 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -21,6 +21,7 @@ import base64 import cherrypy import grp import PAM +import pwd import re @@ -78,6 +79,27 @@ class User(object): def get_user(self): return self.user + def exists(self): + try: + pwd.getpwnam(self.user[USER_NAME]) + except KeyError: + return False + else: + return True + + +class Group(object): + def __init__(self, groupname): + self.groupname = groupname + + def exists(self): + try: + grp.getgrnam(self.groupname) + except KeyError: + return False + else: + return True + def authenticate(username, password, service="passwd"): '''Returns True if authenticate is OK via PAM.''' diff --git a/tests/test_authorization.py b/tests/test_authorization.py index b211e06..ab98987 100644 --- a/tests/test_authorization.py +++ b/tests/test_authorization.py @@ -17,14 +17,17 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import grp import json import os +import pwd import unittest from functools import partial +import kimchi.auth import kimchi.mockmodel from utils import get_free_port, patch_auth, request from utils import run_server @@ -119,3 +122,19 @@ class AuthorizationTests(unittest.TestCase): self.assertEquals(403, resp.status) resp = self.request('/vms', '{}', 'DELETE') self.assertEquals(403, resp.status) + + +class CurrentUserGroupTests(unittest.TestCase): + def test_current_user(self): + current_user = pwd.getpwuid(os.getuid()).pw_name + self.assertTrue(kimchi.auth.User(current_user).exists()) + + invalid_user = "userdoesnotexist" + self.assertFalse(kimchi.auth.User(invalid_user).exists()) + + def test_current_group(self): + current_group = grp.getgrgid(os.getgid()).gr_name + self.assertTrue(kimchi.auth.Group(current_group).exists()) + + invalid_group = "groupdoesnotexist" + self.assertFalse(kimchi.auth.Group(invalid_group).exists()) -- 1.8.5.3
In order to have more control over what users can use a virtual machine under Kimchi, a VM should have users and groups associated with it. Kimchi uses metadata information (http://libvirt.org/formatdomain.html#elementsMetadata) to store users and groups to the virtual machines. The data is stored as string representations of Python lists. Keep in mind that these security restrictions will not apply when using other virtual machine managers (e.g. virsh, virt-manager, etc). Users and groups stored in the VM are now returned when the user queries the VM data. Currently, there is no way to associate users and groups to a VM. The REST command below now returns the users and groups of every virtual machine: /vms/:name GET: It now lists the users and groups associated with that VM. { "users":[ "user1", "user2" ], "groups":[ "group1", "group2" ], ... } If there is no metadata information about users and groups, empty lists will be displayed Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/API.md | 4 ++++ src/kimchi/API.json | 22 ++++++++++++++++++++++ src/kimchi/control/vms.py | 4 +++- src/kimchi/i18n.py | 4 ++++ src/kimchi/mockmodel.py | 4 +++- src/kimchi/model/vms.py | 33 +++++++++++++++++++++++++++++++-- tests/test_mockmodel.py | 2 +- tests/test_model.py | 4 +++- tests/test_rest.py | 2 ++ 9 files changed, 73 insertions(+), 6 deletions(-) diff --git a/docs/API.md b/docs/API.md index fc39c65..8520abf 100644 --- a/docs/API.md +++ b/docs/API.md @@ -96,6 +96,10 @@ the following general conventions: * port: The real port number of the graphics, vnc or spice. Users can use this port to connect to the vm with general vnc/spice clients. + * users: A list of system users who have permission to access the VM. + Default is: empty. + * groups: A list of system groups whose users have permission to access + the VM. Default is: empty. * **DELETE**: Remove the Virtual Machine * **PUT**: update the parameters of existed VM * name: New name for this VM (only applied for shutoff VM) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index f595bbf..e7a0aff 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -183,6 +183,28 @@ "type": "string", "minLength": 1, "error": "KCHVM0011E" + }, + "users": { + "description": "Array of users who have permission to the VM", + "type": "array", + "uniqueItems": true, + "error": "KCHVM0022E", + "items": { + "description": "User name", + "type": "string", + "error": "KCHVM0023E" + } + }, + "groups": { + "description": "Array of groups who have permission to the VM", + "type": "array", + "uniqueItems": true, + "error": "KCHVM0024E", + "items": { + "description": "Group name", + "type": "string", + "error": "KCHVM0025E" + } } } }, diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index 81ad3a6..c2a96c0 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -53,7 +53,9 @@ class VM(Resource): 'icon': self.info['icon'], 'graphics': {'type': self.info['graphics']['type'], 'listen': self.info['graphics']['listen'], - 'port': self.info['graphics']['port']} + 'port': self.info['graphics']['port']}, + 'users': self.info['users'], + 'groups': self.info['groups'] } diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 374bbcd..f6bce8c 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -76,6 +76,10 @@ messages = { "KCHVM0019E": _("Unable to start virtual machine %(name)s. Details: %(err)s"), "KCHVM0020E": _("Unable to stop virtual machine %(name)s. Details: %(err)s"), "KCHVM0021E": _("Unable to delete virtual machine %(name)s. Details: %(err)s"), + "KCHVM0022E": _("User names list must be an array"), + "KCHVM0023E": _("User name must be a string"), + "KCHVM0024E": _("Group names list must be an array"), + "KCHVM0025E": _("Group name must be a string"), "KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"), "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"), diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 50b5f0e..5934ea6 100644 --- a/src/kimchi/mockmodel.py +++ b/src/kimchi/mockmodel.py @@ -934,7 +934,9 @@ class MockVM(object): 'cpus': template_info['cpus'], 'icon': None, 'graphics': {'type': 'vnc', 'listen': '0.0.0.0', - 'port': None} + 'port': None}, + 'users': ['user1', 'user2', 'root'], + 'groups': ['group1', 'group2', 'admin'] } self.info['graphics'].update(template_info['graphics']) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 3ae2048..bedf660 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -17,6 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import ast import os import time import uuid @@ -24,6 +25,7 @@ from xml.etree import ElementTree import libvirt from cherrypy.process.plugins import BackgroundTask +from lxml import etree from kimchi import vnc from kimchi import xmlutils @@ -33,7 +35,8 @@ from kimchi.model.config import CapabilitiesModel from kimchi.model.templates import TemplateModel from kimchi.model.utils import get_vm_name from kimchi.screenshot import VMScreenshot -from kimchi.utils import run_setfacl_set_attr, template_name_from_uri +from kimchi.utils import kimchi_log, run_setfacl_set_attr +from kimchi.utils import template_name_from_uri DOM_STATE_MAP = {0: 'nostate', @@ -50,6 +53,17 @@ VM_LIVE_UPDATE_PARAMS = {} stats = {} +KIMCHI_NS_TAG = 'kimchi' +KIMCHI_NS_URI = 'http://github.com/kimchi-project/kimchi' + + +def _get_vm_metadata(xml, tag): + et = etree.fromstring(xml) + val = et.xpath("/domain/metadata/%s:access/%s:%s" + % (KIMCHI_NS_TAG, KIMCHI_NS_TAG, tag), + namespaces={KIMCHI_NS_TAG: KIMCHI_NS_URI}) + return ast.literal_eval(val[0].text) if val else [] + class VMsModel(object): def __init__(self, **kargs): @@ -302,6 +316,19 @@ class VMModel(object): res['io_throughput'] = vm_stats.get('disk_io', 0) res['io_throughput_peak'] = vm_stats.get('max_disk_io', 100) + xml = dom.XMLDesc(0) + try: + users = _get_vm_metadata(xml, "users") + except Exception as e: + kimchi_log.error("Invalid users metadata on VM '%s': %s", name, e) + users = [] + + try: + groups = _get_vm_metadata(xml, "groups") + except Exception as e: + kimchi_log.error("Invalid groups metadata on VM '%s': %s", name, e) + groups = [] + return {'state': state, 'stats': res, 'uuid': dom.UUIDString(), @@ -311,7 +338,9 @@ class VMModel(object): 'icon': icon, 'graphics': {"type": graphics_type, "listen": graphics_listen, - "port": graphics_port} + "port": graphics_port}, + 'users': users, + 'groups': groups } def _vm_get_disk_paths(self, dom): diff --git a/tests/test_mockmodel.py b/tests/test_mockmodel.py index 0798701..9e258ce 100644 --- a/tests/test_mockmodel.py +++ b/tests/test_mockmodel.py @@ -135,7 +135,7 @@ class MockModelTests(unittest.TestCase): self.assertEquals(u'test', vms[0]) keys = set(('state', 'stats', 'uuid', 'memory', 'cpus', 'screenshot', - 'icon', 'graphics')) + 'icon', 'graphics', 'users', 'groups')) stats_keys = set(('cpu_utilization', 'net_throughput', 'net_throughput_peak', 'io_throughput', 'io_throughput_peak')) diff --git a/tests/test_model.py b/tests/test_model.py index a08b3d9..6d2927a 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -55,7 +55,7 @@ class ModelTests(unittest.TestCase): self.assertEquals('test', vms[0]) keys = set(('state', 'stats', 'uuid', 'memory', 'cpus', 'screenshot', - 'icon', 'graphics')) + 'icon', 'graphics', 'users', 'groups')) stats_keys = set(('cpu_utilization', 'net_throughput', 'net_throughput_peak', 'io_throughput', 'io_throughput_peak')) @@ -67,6 +67,8 @@ class ModelTests(unittest.TestCase): self.assertEquals(None, info['icon']) self.assertEquals(stats_keys, set(info['stats'].keys())) self.assertRaises(NotFoundError, inst.vm_lookup, 'nosuchvm') + self.assertEquals([], info['users']) + self.assertEquals([], info['groups']) @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_lifecycle(self): diff --git a/tests/test_rest.py b/tests/test_rest.py index 8cfa2a2..a421263 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -180,6 +180,8 @@ class RestTests(unittest.TestCase): vm = json.loads(self.request('/vms/vm-1').read()) self.assertEquals('vm-1', vm['name']) self.assertEquals('shutoff', vm['state']) + self.assertEquals(['user1', 'user2', 'root'], vm['users']) + self.assertEquals(['group1', 'group2', 'admin'], vm['groups']) def test_edit_vm(self): req = json.dumps({'name': 'test', 'cdrom': '/nonexistent.iso'}) -- 1.8.5.3
To update (add/remove) the users and groups of a virtual machine, you should use the REST command: /vms/<vm_name> PUT "{ 'users': [ 'user1', 'user2' ], 'groups': [ 'group1', 'group2' ] }" Users and groups associated with a virtual machine must be valid when they are added to the VM. Such verification is not done when they are removed from the system. This means that virtual machines may contain outdated information about users and groups. Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/API.md | 2 ++ src/kimchi/control/vms.py | 2 +- src/kimchi/i18n.py | 2 ++ src/kimchi/model/vms.py | 29 ++++++++++++++++++++++++++--- tests/test_model.py | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 4 deletions(-) diff --git a/docs/API.md b/docs/API.md index 8520abf..a6c2b65 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,8 @@ the following general conventions: * **DELETE**: Remove the Virtual Machine * **PUT**: update the parameters of existed VM * name: New name for this VM (only applied for shutoff VM) + * users: New list of system users. + * groups: New list of system groups. * **POST**: *See Virtual Machine Actions* **Actions (POST):** diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index c2a96c0..b0cf2e0 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -32,7 +32,7 @@ class VMs(Collection): class VM(Resource): def __init__(self, model, ident): super(VM, self).__init__(model, ident) - self.update_params = ["name"] + self.update_params = ["name", "users", "groups"] self.screenshot = VMScreenShot(model, ident) self.uri_fmt = '/vms/%s' for ident, node in sub_nodes.items(): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index f6bce8c..94a02f5 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -80,6 +80,8 @@ messages = { "KCHVM0023E": _("User name must be a string"), "KCHVM0024E": _("Group names list must be an array"), "KCHVM0025E": _("Group name must be a string"), + "KCHVM0026E": _("User %(user)s does not exist"), + "KCHVM0027E": _("Group %(group)s does not exist"), "KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"), "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index bedf660..4f4f572 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -29,6 +29,7 @@ from lxml import etree from kimchi import vnc from kimchi import xmlutils +from kimchi.auth import Group, User from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel @@ -260,10 +261,24 @@ class VMModel(object): old_xml = new_xml = dom.XMLDesc(0) + users = _get_vm_metadata(old_xml, "users") + groups = _get_vm_metadata(old_xml, "groups") + for key, val in params.items(): - if key in VM_STATIC_UPDATE_PARAMS: - xpath = VM_STATIC_UPDATE_PARAMS[key] - new_xml = xmlutils.xml_item_update(new_xml, xpath, val) + if key == 'users': + for user in val: + if not User(user).exists(): + raise OperationFailed("KCHVM0026E", {'user': user}) + users = val + elif key == 'groups': + for group in val: + if not Group(group).exists(): + raise OperationFailed("KCHVM0027E", {'group': group}) + groups = val + else: + if key in VM_STATIC_UPDATE_PARAMS: + xpath = VM_STATIC_UPDATE_PARAMS[key] + new_xml = xmlutils.xml_item_update(new_xml, xpath, val) try: if 'name' in params: @@ -274,6 +289,14 @@ class VMModel(object): dom.undefine() conn = self.conn.get() dom = conn.defineXML(new_xml) + metadata_xml = """ + <access> + <users>%s</users> + <groups>%s</groups> + </access> + """ % (str(users), str(groups)) + dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, metadata_xml, + KIMCHI_NS_TAG, KIMCHI_NS_URI) except libvirt.libvirtError as e: dom = conn.defineXML(old_xml) raise OperationFailed("KCHVM0008E", {'name': dom.name(), diff --git a/tests/test_model.py b/tests/test_model.py index 6d2927a..af52eed 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -18,9 +18,11 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import grp import os import platform import psutil +import pwd import shutil import tempfile import threading @@ -577,6 +579,44 @@ class ModelTests(unittest.TestCase): rollback.prependDefer(self._rollback_wrapper, inst.vm_delete, u'пeω-∨м') + # change only VM users - groups are not changed (default is empty) + users = ['root'] + inst.vm_update(u'пeω-∨м', {'users': users}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + + # change only VM groups - users are not changed (default is empty) + groups = ['wheel'] + inst.vm_update(u'пeω-∨м', {'groups': groups}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups by adding a new element to each one + users.append(pwd.getpwuid(os.getuid()).pw_name) + groups.append(grp.getgrgid(os.getgid()).gr_name) + inst.vm_update(u'пeω-∨м', {'users': users, 'groups': groups}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users (wrong value) and groups + # when an error occurs, everything fails and nothing is changed + self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м', + {'users': ['userdoesnotexist'], 'groups': []}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups (wrong value) + # when an error occurs, everything fails and nothing is changed + self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м', + {'users': [], 'groups': ['groupdoesnotexist']}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups by removing all elements + inst.vm_update(u'пeω-∨м', {'users': [], 'groups': []}) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_network(self): inst = model.Model('qemu:///system', self.tmp_store) -- 1.8.5.3
On 02/28/2014 03:40 PM, Crístian Viana wrote:
To update (add/remove) the users and groups of a virtual machine, you should use the REST command:
/vms/<vm_name> PUT "{ 'users': [ 'user1', 'user2' ], 'groups': [ 'group1', 'group2' ] }"
Users and groups associated with a virtual machine must be valid when they are added to the VM. Such verification is not done when they are removed from the system. This means that virtual machines may contain outdated information about users and groups.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/API.md | 2 ++ src/kimchi/control/vms.py | 2 +- src/kimchi/i18n.py | 2 ++ src/kimchi/model/vms.py | 29 ++++++++++++++++++++++++++--- tests/test_model.py | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/docs/API.md b/docs/API.md index 8520abf..a6c2b65 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,8 @@ the following general conventions: * **DELETE**: Remove the Virtual Machine * **PUT**: update the parameters of existed VM * name: New name for this VM (only applied for shutoff VM) + * users: New list of system users. + * groups: New list of system groups. * **POST**: *See Virtual Machine Actions*
**Actions (POST):** diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index c2a96c0..b0cf2e0 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -32,7 +32,7 @@ class VMs(Collection): class VM(Resource): def __init__(self, model, ident): super(VM, self).__init__(model, ident) - self.update_params = ["name"] + self.update_params = ["name", "users", "groups"] self.screenshot = VMScreenShot(model, ident) self.uri_fmt = '/vms/%s' for ident, node in sub_nodes.items(): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index f6bce8c..94a02f5 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -80,6 +80,8 @@ messages = { "KCHVM0023E": _("User name must be a string"), "KCHVM0024E": _("Group names list must be an array"), "KCHVM0025E": _("Group name must be a string"), + "KCHVM0026E": _("User %(user)s does not exist"), + "KCHVM0027E": _("Group %(group)s does not exist"),
"KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"), "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index bedf660..4f4f572 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -29,6 +29,7 @@ from lxml import etree
from kimchi import vnc from kimchi import xmlutils +from kimchi.auth import Group, User from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel @@ -260,10 +261,24 @@ class VMModel(object):
old_xml = new_xml = dom.XMLDesc(0)
+ users = _get_vm_metadata(old_xml, "users") + groups = _get_vm_metadata(old_xml, "groups") + for key, val in params.items(): - if key in VM_STATIC_UPDATE_PARAMS: - xpath = VM_STATIC_UPDATE_PARAMS[key] - new_xml = xmlutils.xml_item_update(new_xml, xpath, val) + if key == 'users': + for user in val: + if not User(user).exists(): + raise OperationFailed("KCHVM0026E", {'user': user}) + users = val + elif key == 'groups': + for group in val: + if not Group(group).exists(): + raise OperationFailed("KCHVM0027E", {'group': group}) + groups = val + else: + if key in VM_STATIC_UPDATE_PARAMS: + xpath = VM_STATIC_UPDATE_PARAMS[key] + new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
try: if 'name' in params: @@ -274,6 +289,14 @@ class VMModel(object): dom.undefine() conn = self.conn.get() dom = conn.defineXML(new_xml)
Don't you need to undefine the vm first and then create the new one with the updated xml? From the code, the vm will be undefine only when update its name
+ metadata_xml = """ + <access> + <users>%s</users> + <groups>%s</groups> + </access> + """ % (str(users), str(groups)) + dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, metadata_xml, + KIMCHI_NS_TAG, KIMCHI_NS_URI) except libvirt.libvirtError as e: dom = conn.defineXML(old_xml)
Same problem I mentioned above. As you don't undefine the original VM, when entering here we will get an error: libvirtError: operation failed: domain 'пeω-∨м' is already defined with uuid cf2043b8-0234-4ded-896d-26ebbaed5be0
raise OperationFailed("KCHVM0008E", {'name': dom.name(), diff --git a/tests/test_model.py b/tests/test_model.py index 6d2927a..af52eed 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -18,9 +18,11 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import grp import os import platform import psutil +import pwd import shutil import tempfile import threading @@ -577,6 +579,44 @@ class ModelTests(unittest.TestCase): rollback.prependDefer(self._rollback_wrapper, inst.vm_delete, u'пeω-∨м')
+ # change only VM users - groups are not changed (default is empty) + users = ['root'] + inst.vm_update(u'пeω-∨м', {'users': users}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + + # change only VM groups - users are not changed (default is empty) + groups = ['wheel'] + inst.vm_update(u'пeω-∨м', {'groups': groups}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups by adding a new element to each one + users.append(pwd.getpwuid(os.getuid()).pw_name) + groups.append(grp.getgrgid(os.getgid()).gr_name) + inst.vm_update(u'пeω-∨м', {'users': users, 'groups': groups}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users (wrong value) and groups + # when an error occurs, everything fails and nothing is changed + self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м', + {'users': ['userdoesnotexist'], 'groups': []}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups (wrong value) + # when an error occurs, everything fails and nothing is changed + self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м', + {'users': [], 'groups': ['groupdoesnotexist']}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups by removing all elements + inst.vm_update(u'пeω-∨м', {'users': [], 'groups': []}) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_network(self): inst = model.Model('qemu:///system', self.tmp_store)
On 02/28/2014 03:40 PM, Crístian Viana wrote:
To update (add/remove) the users and groups of a virtual machine, you should use the REST command:
/vms/<vm_name> PUT "{ 'users': [ 'user1', 'user2' ], 'groups': [ 'group1', 'group2' ] }"
Users and groups associated with a virtual machine must be valid when they are added to the VM. Such verification is not done when they are removed from the system. This means that virtual machines may contain outdated information about users and groups.
Signed-off-by: Crístian Viana <vianac@linux.vnet.ibm.com> --- docs/API.md | 2 ++ src/kimchi/control/vms.py | 2 +- src/kimchi/i18n.py | 2 ++ src/kimchi/model/vms.py | 29 ++++++++++++++++++++++++++--- tests/test_model.py | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/docs/API.md b/docs/API.md index 8520abf..a6c2b65 100644 --- a/docs/API.md +++ b/docs/API.md @@ -103,6 +103,8 @@ the following general conventions: * **DELETE**: Remove the Virtual Machine * **PUT**: update the parameters of existed VM * name: New name for this VM (only applied for shutoff VM) + * users: New list of system users. + * groups: New list of system groups. * **POST**: *See Virtual Machine Actions*
**Actions (POST):** diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index c2a96c0..b0cf2e0 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -32,7 +32,7 @@ class VMs(Collection): class VM(Resource): def __init__(self, model, ident): super(VM, self).__init__(model, ident) - self.update_params = ["name"] + self.update_params = ["name", "users", "groups"] self.screenshot = VMScreenShot(model, ident) self.uri_fmt = '/vms/%s' for ident, node in sub_nodes.items(): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index f6bce8c..94a02f5 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -80,6 +80,8 @@ messages = { "KCHVM0023E": _("User name must be a string"), "KCHVM0024E": _("Group names list must be an array"), "KCHVM0025E": _("Group name must be a string"), + "KCHVM0026E": _("User %(user)s does not exist"), + "KCHVM0027E": _("Group %(group)s does not exist"),
"KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"), "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"), diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index bedf660..4f4f572 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -29,6 +29,7 @@ from lxml import etree
from kimchi import vnc from kimchi import xmlutils +from kimchi.auth import Group, User from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import MissingParameter, NotFoundError, OperationFailed from kimchi.model.config import CapabilitiesModel @@ -260,10 +261,24 @@ class VMModel(object):
old_xml = new_xml = dom.XMLDesc(0)
+ users = _get_vm_metadata(old_xml, "users") + groups = _get_vm_metadata(old_xml, "groups") + for key, val in params.items(): - if key in VM_STATIC_UPDATE_PARAMS: - xpath = VM_STATIC_UPDATE_PARAMS[key] - new_xml = xmlutils.xml_item_update(new_xml, xpath, val) + if key == 'users': + for user in val: + if not User(user).exists(): + raise OperationFailed("KCHVM0026E", {'user': user}) + users = val + elif key == 'groups': + for group in val: + if not Group(group).exists(): + raise OperationFailed("KCHVM0027E", {'group': group}) + groups = val + else: + if key in VM_STATIC_UPDATE_PARAMS: + xpath = VM_STATIC_UPDATE_PARAMS[key] + new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
try: if 'name' in params: @@ -274,6 +289,14 @@ class VMModel(object): dom.undefine() conn = self.conn.get() dom = conn.defineXML(new_xml) + metadata_xml = """ + <access> + <users>%s</users> + <groups>%s</groups> + </access> + """ % (str(users), str(groups)) + dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, metadata_xml, + KIMCHI_NS_TAG, KIMCHI_NS_URI)
I also noticed some libvirt version requires the "flags" parameter libvirt.virDomain.setMetadata = setMetadata(self, type, metadata, key, uri, flags) In other ones, it already has a default value: libvirt.virDomain.setMetadata = setMetadata(self, type, metadata, key, uri, *flags=0*)
except libvirt.libvirtError as e: dom = conn.defineXML(old_xml) raise OperationFailed("KCHVM0008E", {'name': dom.name(), diff --git a/tests/test_model.py b/tests/test_model.py index 6d2927a..af52eed 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -18,9 +18,11 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import grp import os import platform import psutil +import pwd import shutil import tempfile import threading @@ -577,6 +579,44 @@ class ModelTests(unittest.TestCase): rollback.prependDefer(self._rollback_wrapper, inst.vm_delete, u'пeω-∨м')
+ # change only VM users - groups are not changed (default is empty) + users = ['root'] + inst.vm_update(u'пeω-∨м', {'users': users}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + + # change only VM groups - users are not changed (default is empty) + groups = ['wheel'] + inst.vm_update(u'пeω-∨м', {'groups': groups}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups by adding a new element to each one + users.append(pwd.getpwuid(os.getuid()).pw_name) + groups.append(grp.getgrgid(os.getgid()).gr_name) + inst.vm_update(u'пeω-∨м', {'users': users, 'groups': groups}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users (wrong value) and groups + # when an error occurs, everything fails and nothing is changed + self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м', + {'users': ['userdoesnotexist'], 'groups': []}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups (wrong value) + # when an error occurs, everything fails and nothing is changed + self.assertRaises(OperationFailed, inst.vm_update, u'пeω-∨м', + {'users': [], 'groups': ['groupdoesnotexist']}) + self.assertEquals(users, inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals(groups, inst.vm_lookup(u'пeω-∨м')['groups']) + + # change VM users and groups by removing all elements + inst.vm_update(u'пeω-∨м', {'users': [], 'groups': []}) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['users']) + self.assertEquals([], inst.vm_lookup(u'пeω-∨м')['groups']) + @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_network(self): inst = model.Model('qemu:///system', self.tmp_store)
While trying it on my system I got: libvir: QEMU Driver error : argument unsupported: QEMU driver does not support<metadata> element We need to verify if all the supported distros has this QEMU support. Otherwise, we need to think how to handle this difference. On 02/28/2014 03:40 PM, Crístian Viana wrote:
Here are the differences between the previous patchset (v1) and this one:
- New patch "Use proper term "user name" instead of "user id"" has been added. - Patch "Return users and groups when fetching VM info" has been updated to log the exception message when there is an invalid metadata value in a VM. Notice that an invalid metadata value can only be written to the VM XML by external access; users from within Kimchi will always store correct value through the app. - Rebased to 5d82241.
Crístian Viana (5): Override only the updated "User" methods in "patch_auth" Use proper term "user name" instead of "user id" Add functions to check if a user/group exists Return users and groups when fetching VM info Add/remove users and groups to VMs
docs/API.md | 6 ++++ po/en_US.po | 2 +- po/kimchi.pot | 2 +- po/pt_BR.po | 2 +- po/zh_CN.po | 2 +- src/kimchi/API.json | 22 ++++++++++++++ src/kimchi/auth.py | 60 ++++++++++++++++++++++++++------------ src/kimchi/control/vms.py | 6 ++-- src/kimchi/i18n.py | 8 +++++- src/kimchi/mockmodel.py | 4 ++- src/kimchi/model/vms.py | 62 ++++++++++++++++++++++++++++++++++++---- src/kimchi/root.py | 4 +-- tests/test_authorization.py | 19 ++++++++++++ tests/test_mockmodel.py | 2 +- tests/test_model.py | 44 +++++++++++++++++++++++++++- tests/test_rest.py | 10 ++++--- tests/utils.py | 26 +++++------------ ui/js/src/kimchi.login_window.js | 16 +++++------ ui/js/src/kimchi.user.js | 14 ++++----- ui/pages/login-window.html.tmpl | 4 +-- 20 files changed, 239 insertions(+), 76 deletions(-)
participants (4)
-
Aline Manera -
Crístian Viana -
Ramon Medeiros -
Sheldon