[PATCH V3 0/2] bug fix: redirect to the protected page after login
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> V2 -> V3: rebase V1 -> V2: remove kimchisession hook ShaoHe Feng (2): bug fix: redirect to the protected page after login remove kimchisession hook and add the same logic to root.get src/kimchi/auth.py | 31 ++++++++++++------------------- src/kimchi/config.py.in | 5 ----- src/kimchi/root.py | 40 +++++++++++++++++++++++++--------------- src/kimchi/server.py | 2 -- tests/test_config.py.in | 8 -------- ui/pages/login.html.tmpl | 3 ++- 6 files changed, 39 insertions(+), 50 deletions(-) -- 1.9.3
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> add a 'next' attribute in login html page. if "next" is not None, means there is a html protected page user want to access. after authentication success, redirect to it. if "next" is None, then redirect to the last page recorde in th the cookie. Test this case: 1. input a protected page URL in the browser. such as: https://localhost:8001/spice.html?port=64667&listen=localhost&token=vmname&encrypt=1 2. input a wrong password or username: it should report: "The username or password you entered is incorrect. Please try again." 3. input the right password and username: It should redirect to the former page. https://localhost:8001/spice.html?port=64667&listen=localhost&token=vmname&encrypt=1 Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/auth.py | 18 ++++++++++++------ src/kimchi/root.py | 34 +++++++++++++++++++--------------- ui/pages/login.html.tmpl | 3 ++- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index 59889ed..c7f94a7 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -28,7 +28,6 @@ import re import termios import time -import urllib2 from kimchi import template @@ -43,9 +42,13 @@ def redirect_login(): - next_url = urllib2.quote( - cherrypy.request.path_info.encode('utf-8'), safe="") - raise cherrypy.HTTPRedirect("/login.html?next=%s" % next_url, 303) + url = "/login.html" + if cherrypy.request.path_info.endswith(".html"): + next_url = cherrypy.serving.request.request_line.split()[1] + next_url = base64.urlsafe_b64encode(next_url) + url = "/login.html?next=%s" % next_url + + raise cherrypy.HTTPRedirect(url, 303) def debug(msg): @@ -194,7 +197,7 @@ def check_auth_httpba(): return login(username, password) -def login(username, password): +def login(username, password, **kwargs): try: if not authenticate(username, password): debug("User cannot be verified with the supplied password") @@ -202,7 +205,10 @@ def login(username, password): except PAM.error, (resp, code): if (cherrypy.request.path_info == "/login" and not template.can_accept('application/json')): - raise cherrypy.HTTPRedirect("/login.html?error=userPassWrong", 303) + next_url = kwargs.get("next") + url = "/login.html?error=userPassWrong" + url = url if next_url is None else url + "&next=%s" % next_url + raise cherrypy.HTTPRedirect(url, 303) msg_args = {'username': username, 'code': code} raise OperationFailed("KCHAUTH0001E", msg_args) diff --git a/src/kimchi/root.py b/src/kimchi/root.py index f2b6a53..82bd97b 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.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 base64 import cherrypy import json import os @@ -122,25 +123,28 @@ def __init__(self, model, dev_env): def login(self, *args, **kwargs): username = kwargs.get('username') password = kwargs.get('password') - # forms base authentication + # traditional form base authentication + kwa = {} if username is not None: - next_url = cherrypy.request.cookie.get("lastPage") + # UI can parser the redirect url by "next" query parameter + next_url = kwargs.get('next') + next_url = next_url[0] if(type(next_url) is list) else next_url if next_url is None: - # UI can parser the redirect url by "next" query parameter - next_url = kwargs.get('next', "/") - next_url = next_url[0] if(type(next_url) is list) else next_url + lastPage = cherrypy.request.cookie.get("lastPage") + next_url = lastPage.value if lastPage is not None else "/" else: - next_url = next_url.value - auth.login(username, password) + kwa = {"next": next_url.encode("utf-8")} + next_url = base64.urlsafe_b64decode(next_url.encode("utf-8")) + auth.login(username, password, **kwa) raise cherrypy.HTTPRedirect(next_url, 303) - else: - try: - params = parse_request() - username = params['username'] - password = params['password'] - except KeyError, item: - e = MissingParameter('KCHAUTH0003E', {'item': str(item)}) - raise cherrypy.HTTPError(400, e.message) + + try: + params = parse_request() + 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(username, password) diff --git a/ui/pages/login.html.tmpl b/ui/pages/login.html.tmpl index 0fa7122..f8f683d 100644 --- a/ui/pages/login.html.tmpl +++ b/ui/pages/login.html.tmpl @@ -21,6 +21,7 @@ #silent t = gettext.translation($lang.domain, $lang.localedir, languages=$lang.lang) #silent _ = t.gettext #silent _t = t.gettext +#silent next = "?next=%s" % $getVar('data.next', '') if $getVar('data.next', '') else "" #from kimchi.config import get_version <!DOCTYPE html> <html lang="$lang.lang[0]"> @@ -99,7 +100,7 @@ function init() { <div id="messUserPass" class="err-mess" style="display: none;">$_("The username or password you entered is incorrect. Please try again.")</div> <div id="messSession" class="err-mess" style="display: none;">$_("Session timeout, please re-login.")</div> </div> - <form id="form-login" action="/login" method="POST" class="login-panel" onsubmit="updateBtnLabel();"> + <form id="form-login" action="/login$next" method="POST" class="login-panel" onsubmit="updateBtnLabel();"> <div class="row"> <input type="text" id="username" name="username" required="required" placeholder="$_("User Name")" autofocus/> <div id="username-msg" class="msg-required"></div> -- 1.9.3
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> add a 'next' attribute in login html page. if "next" is not None, means there is a html protected page user want to access. after authentication success, redirect to it. if "next" is None, then redirect to the last page recorde in th the cookie. Test this case: 1. input a protected page URL in the browser. such as: https://localhost:8001/spice.html?port=64667&listen=localhost&token=vmname&encrypt=1 2. input a wrong password or username: it should report: "The username or password you entered is incorrect. Please try again." 3. input the right password and username: It should redirect to the former page. https://localhost:8001/spice.html?port=64667&listen=localhost&token=vmname&encrypt=1 Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/auth.py | 18 ++++++++++++------ src/kimchi/root.py | 34 +++++++++++++++++++--------------- ui/pages/login.html.tmpl | 3 ++- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index 59889ed..c7f94a7 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -28,7 +28,6 @@ import re import termios import time -import urllib2 from kimchi import template @@ -43,9 +42,13 @@ def redirect_login(): - next_url = urllib2.quote( - cherrypy.request.path_info.encode('utf-8'), safe="") - raise cherrypy.HTTPRedirect("/login.html?next=%s" % next_url, 303) + url = "/login.html" + if cherrypy.request.path_info.endswith(".html"): + next_url = cherrypy.serving.request.request_line.split()[1] + next_url = base64.urlsafe_b64encode(next_url) + url = "/login.html?next=%s" % next_url + + raise cherrypy.HTTPRedirect(url, 303) def debug(msg): @@ -194,7 +197,7 @@ def check_auth_httpba(): return login(username, password) -def login(username, password): +def login(username, password, **kwargs): try: if not authenticate(username, password): debug("User cannot be verified with the supplied password") @@ -202,7 +205,10 @@ def login(username, password): except PAM.error, (resp, code): if (cherrypy.request.path_info == "/login" and not template.can_accept('application/json')): - raise cherrypy.HTTPRedirect("/login.html?error=userPassWrong", 303) + next_url = kwargs.get("next") + url = "/login.html?error=userPassWrong" + url = url if next_url is None else url + "&next=%s" % next_url + raise cherrypy.HTTPRedirect(url, 303) msg_args = {'username': username, 'code': code} raise OperationFailed("KCHAUTH0001E", msg_args) diff --git a/src/kimchi/root.py b/src/kimchi/root.py index f2b6a53..82bd97b 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.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 base64 import cherrypy import json import os @@ -122,25 +123,28 @@ def __init__(self, model, dev_env): def login(self, *args, **kwargs): username = kwargs.get('username') password = kwargs.get('password') - # forms base authentication + # traditional form base authentication + kwa = {} if username is not None: - next_url = cherrypy.request.cookie.get("lastPage") + # UI can parser the redirect url by "next" query parameter + next_url = kwargs.get('next') + next_url = next_url[0] if(type(next_url) is list) else next_url if next_url is None: - # UI can parser the redirect url by "next" query parameter - next_url = kwargs.get('next', "/") - next_url = next_url[0] if(type(next_url) is list) else next_url + lastPage = cherrypy.request.cookie.get("lastPage") + next_url = lastPage.value if lastPage is not None else "/" else: - next_url = next_url.value - auth.login(username, password) + kwa = {"next": next_url.encode("utf-8")} + next_url = base64.urlsafe_b64decode(next_url.encode("utf-8")) + auth.login(username, password, **kwa) raise cherrypy.HTTPRedirect(next_url, 303) - else: - try: - params = parse_request() - username = params['username'] - password = params['password'] - except KeyError, item: - e = MissingParameter('KCHAUTH0003E', {'item': str(item)}) - raise cherrypy.HTTPError(400, e.message) + + try: + params = parse_request() + 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(username, password) diff --git a/ui/pages/login.html.tmpl b/ui/pages/login.html.tmpl index 0fa7122..f8f683d 100644 --- a/ui/pages/login.html.tmpl +++ b/ui/pages/login.html.tmpl @@ -21,6 +21,7 @@ #silent t = gettext.translation($lang.domain, $lang.localedir, languages=$lang.lang) #silent _ = t.gettext #silent _t = t.gettext +#silent next = "?next=%s" % $getVar('data.next', '') if $getVar('data.next', '') else "" #from kimchi.config import get_version <!DOCTYPE html> <html lang="$lang.lang[0]"> @@ -99,7 +100,7 @@ function init() { <div id="messUserPass" class="err-mess" style="display: none;">$_("The username or password you entered is incorrect. Please try again.")</div> <div id="messSession" class="err-mess" style="display: none;">$_("Session timeout, please re-login.")</div> </div> - <form id="form-login" action="/login" method="POST" class="login-panel" onsubmit="updateBtnLabel();"> + <form id="form-login" action="/login$next" method="POST" class="login-panel" onsubmit="updateBtnLabel();"> <div class="row"> <input type="text" id="username" name="username" required="required" placeholder="$_("User Name")" autofocus/> <div id="username-msg" class="msg-required"></div> -- 1.9.3
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> kimchisession hook is heavy overhead. root.get just return the kimchi-ui.html, it is not a restful API. consider root.get is the only entry of UI. So move the logic code to root.get. Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/auth.py | 13 ------------- src/kimchi/config.py.in | 5 ----- src/kimchi/root.py | 6 ++++++ src/kimchi/server.py | 2 -- tests/test_config.py.in | 8 -------- 5 files changed, 6 insertions(+), 28 deletions(-) diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index c7f94a7..6a4a610 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -269,16 +269,3 @@ def kimchiauth(admin_methods=None): e = InvalidOperation('KCHAUTH0002E') raise cherrypy.HTTPError(401, e.message.encode('utf-8')) - - -def kimchisession(admin_methods=None): - session = cherrypy.request.cookie.get("kimchi") - last_page = cherrypy.request.cookie.get("lastPage") - headers = cherrypy.request.headers - authheader = headers.get('AUTHORIZATION') - # when client browser first login in, both the session and lastPage cookie - # are None. - # when session timeout, only session cookie is None. - if (session is None and last_page is None and authheader is None and - ("Accept" in headers and not template.can_accept('application/json'))): - redirect_login() diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index 4be767e..3aef8cf 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -154,7 +154,6 @@ class UIConfig(dict): ui_configs['/' + sub_dir] = { 'tools.staticdir.on': True, 'tools.staticdir.dir': os.path.join(paths.ui_dir, sub_dir), - 'tools.kimchisession.on': False, 'tools.nocache.on': False} if sub_dir != 'images': ui_configs['/' + sub_dir].update({ @@ -180,7 +179,6 @@ class KimchiConfig(dict): 'tools.sessions.locking': 'explicit', 'tools.sessions.storage_type': 'ram', 'tools.sessions.timeout': SESSIONSTIMEOUT, - 'tools.kimchisession.on': True, 'tools.kimchiauth.on': False }, '/vnc_auto.html': { @@ -192,9 +190,6 @@ class KimchiConfig(dict): '/kimchi-ui.html': { 'tools.kimchiauth.on': True }, - '/login.html': { - 'tools.kimchisession.on': False, - }, '/data/screenshots': { 'tools.staticdir.on': True, 'tools.staticdir.dir': get_screenshot_path(), diff --git a/src/kimchi/root.py b/src/kimchi/root.py index 82bd97b..2daec64 100644 --- a/src/kimchi/root.py +++ b/src/kimchi/root.py @@ -77,6 +77,12 @@ def error_development_handler(self, status, message, traceback, version): return res def get(self): + last_page = cherrypy.request.cookie.get("lastPage") + # when session timeout, only session cookie is None. + # when first login, both session and lastPage are None. + if cherrypy.session.originalid is None and last_page is None: + raise cherrypy.HTTPRedirect("/login.html", 303) + return self.default(self.default_page) @cherrypy.expose diff --git a/src/kimchi/server.py b/src/kimchi/server.py index 30140ce..7344349 100644 --- a/src/kimchi/server.py +++ b/src/kimchi/server.py @@ -77,8 +77,6 @@ def __init__(self, options): cherrypy.tools.nocache = cherrypy.Tool('on_end_resource', set_no_cache) cherrypy.tools.kimchiauth = cherrypy.Tool('before_handler', auth.kimchiauth) - cherrypy.tools.kimchisession = cherrypy.Tool('before_request_body', - auth.kimchisession) # Setting host to 127.0.0.1. This makes kimchi runs # as a localhost app, inaccessible to the outside # directly. You must go through the proxy. diff --git a/tests/test_config.py.in b/tests/test_config.py.in index 1ca8d9e..f349419 100644 --- a/tests/test_config.py.in +++ b/tests/test_config.py.in @@ -104,7 +104,6 @@ class ConfigTests(unittest.TestCase): 'tools.sessions.locking': 'explicit', 'tools.sessions.storage_type': 'ram', 'tools.sessions.timeout': SESSIONSTIMEOUT, - 'tools.kimchisession.on': True, 'tools.kimchiauth.on': False }, '/vnc_auto.html': { @@ -116,15 +115,11 @@ class ConfigTests(unittest.TestCase): '/kimchi-ui.html': { 'tools.kimchiauth.on': True }, - '/login.html': { - 'tools.kimchisession.on': False, - }, '/css': { 'tools.staticdir.on': True, 'tools.staticdir.dir': '%s/ui/css' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, - 'tools.kimchisession.on': False, 'tools.nocache.on': False }, '/js': { @@ -132,7 +127,6 @@ class ConfigTests(unittest.TestCase): 'tools.staticdir.dir': '%s/ui/js' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, - 'tools.kimchisession.on': False, 'tools.nocache.on': False }, '/libs': { @@ -140,13 +134,11 @@ class ConfigTests(unittest.TestCase): 'tools.staticdir.dir': '%s/ui/libs' % paths.prefix, 'tools.expires.on': True, 'tools.expires.secs': CACHEEXPIRES, - 'tools.kimchisession.on': False, 'tools.nocache.on': False, }, '/images': { 'tools.staticdir.on': True, 'tools.staticdir.dir': '%s/ui/images' % paths.prefix, - 'tools.kimchisession.on': False, 'tools.nocache.on': False }, '/data/screenshots': { -- 1.9.3
On 06/20/2014 06:15 PM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Tested-by: Hongliang Wang <hlwang@linux.vnet.ibm.com>
V2 -> V3: rebase
V1 -> V2: remove kimchisession hook
ShaoHe Feng (2): bug fix: redirect to the protected page after login remove kimchisession hook and add the same logic to root.get
src/kimchi/auth.py | 31 ++++++++++++------------------- src/kimchi/config.py.in | 5 ----- src/kimchi/root.py | 40 +++++++++++++++++++++++++--------------- src/kimchi/server.py | 2 -- tests/test_config.py.in | 8 -------- ui/pages/login.html.tmpl | 3 ++- 6 files changed, 39 insertions(+), 50 deletions(-)
Reviewed and Tested By Yu Xin Huo. On 6/20/2014 6:15 PM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
V2 -> V3: rebase
V1 -> V2: remove kimchisession hook
ShaoHe Feng (2): bug fix: redirect to the protected page after login remove kimchisession hook and add the same logic to root.get
src/kimchi/auth.py | 31 ++++++++++++------------------- src/kimchi/config.py.in | 5 ----- src/kimchi/root.py | 40 +++++++++++++++++++++++++--------------- src/kimchi/server.py | 2 -- tests/test_config.py.in | 8 -------- ui/pages/login.html.tmpl | 3 ++- 6 files changed, 39 insertions(+), 50 deletions(-)
ping. In this patch, we do not make big changes for login before this release. Web storage is a good way to store the URL. But it maybe a big change. So we give up it this release. On 06/20/2014 06:15 PM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
V2 -> V3: rebase
V1 -> V2: remove kimchisession hook
ShaoHe Feng (2): bug fix: redirect to the protected page after login remove kimchisession hook and add the same logic to root.get
src/kimchi/auth.py | 31 ++++++++++++------------------- src/kimchi/config.py.in | 5 ----- src/kimchi/root.py | 40 +++++++++++++++++++++++++--------------- src/kimchi/server.py | 2 -- tests/test_config.py.in | 8 -------- ui/pages/login.html.tmpl | 3 ++- 6 files changed, 39 insertions(+), 50 deletions(-)
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
The tests are failing: ====================================================================== FAIL: test_accepts (test_rest.HttpsRestTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/alinefm/kimchi/tests/test_rest.py", line 134, in test_accepts self.assertTrue('<!doctype html>' in resp.read().lower()) AssertionError: False is not true ====================================================================== FAIL: test_accepts (test_rest.RestTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/alinefm/kimchi/tests/test_rest.py", line 134, in test_accepts self.assertTrue('<!doctype html>' in resp.read().lower()) AssertionError: False is not true ====================================================================== FAIL: test_server_start (test_server.ServerTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/alinefm/kimchi/tests/test_server.py", line 41, in test_server_start self.assertEquals(200, resp.status) AssertionError: 200 != 303 ---------------------------------------------------------------------- Ran 159 tests in 155.488s FAILED (failures=3, skipped=2) [26/Jun/2014:16:19:06] ENGINE Waiting for child threads to terminate... make[3]: *** [check-local] Error 1 make[3]: Leaving directory `/home/alinefm/kimchi/tests' make[2]: *** [check-am] Error 2 make[2]: Leaving directory `/home/alinefm/kimchi/tests' make[1]: *** [check] Error 2 make[1]: Leaving directory `/home/alinefm/kimchi/tests' make: *** [check-recursive] Error 1 On 06/20/2014 07:15 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
V2 -> V3: rebase
V1 -> V2: remove kimchisession hook
ShaoHe Feng (2): bug fix: redirect to the protected page after login remove kimchisession hook and add the same logic to root.get
src/kimchi/auth.py | 31 ++++++++++++------------------- src/kimchi/config.py.in | 5 ----- src/kimchi/root.py | 40 +++++++++++++++++++++++++--------------- src/kimchi/server.py | 2 -- tests/test_config.py.in | 8 -------- ui/pages/login.html.tmpl | 3 ++- 6 files changed, 39 insertions(+), 50 deletions(-)
participants (5)
-
Aline Manera -
Hongliang Wang -
shaohef＠linux.vnet.ibm.com -
Sheldon
-
Yu Xin Huo