cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bhais...@apache.org
Subject git commit: updated refs/heads/CLOUDSTACK-8622 to 88f63d5
Date Thu, 09 Jul 2015 21:11:36 GMT
Repository: cloudstack
Updated Branches:
  refs/heads/CLOUDSTACK-8622 [created] 88f63d516


CLOUDSTACK-8622:  Reinstate working sessions in browser

- Login is based on sessionkey HttpOnly Cookie
- ApiServlet does login verification using sessionKey from both the request cookies
  and the API parameters. In both cases, if either or both are passed they should
  match the sessionKey stored in the current session of the HttpRequest
- UI: it no longer needs to read or set sessionkey cookie
- UI: it no longer needs to return g_sessionKey value in the API requests, though
  to support a sso mechanism it is not removed for that specific case
- UI: on logout, all cookies are removed (though this won't remove httponly ones)
- Secure jsessionid cookie is set to be HttpOnly and Secure
- SAML login should also set HttpOnly cookie before redirecting to UI
- SAML: ListIdps API is made a readonly API that won't destroy an existing session

Performed tests (login, saml login if applicable, page refreshes, opening
multiple tabs, logout) with following combinations:
- SAML disabled, normal auth as admin, domain-admin and user
- SAML enabled, normal auth as admin, domain-admin and user; and saml sso as
  admin, domain-admin and user


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/88f63d51
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/88f63d51
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/88f63d51

Branch: refs/heads/CLOUDSTACK-8622
Commit: 88f63d516860d348b818d1c9149829f3469cd00a
Parents: 4de4a0f
Author: Rohit Yadav <rohit.yadav@shapeblue.com>
Authored: Fri Jul 10 02:28:30 2015 +0530
Committer: Rohit Yadav <rohit.yadav@shapeblue.com>
Committed: Fri Jul 10 02:37:10 2015 +0530

----------------------------------------------------------------------
 .../api/auth/APIAuthenticationType.java         |  2 +-
 .../cloudstack/api/command/ListIdpsCmd.java     |  2 +-
 .../command/SAML2LoginAPIAuthenticatorCmd.java  |  2 +-
 server/src/com/cloud/api/ApiServlet.java        | 60 ++++++++------
 ui/scripts/cloudStack.js                        | 84 ++++++++++----------
 ui/scripts/sharedFunctions.js                   |  5 +-
 ui/scripts/ui-custom/login.js                   | 69 ++++++----------
 utils/src/com/cloud/utils/HttpUtils.java        |  1 +
 8 files changed, 109 insertions(+), 116 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/88f63d51/api/src/org/apache/cloudstack/api/auth/APIAuthenticationType.java
----------------------------------------------------------------------
diff --git a/api/src/org/apache/cloudstack/api/auth/APIAuthenticationType.java b/api/src/org/apache/cloudstack/api/auth/APIAuthenticationType.java
index e8c7f0f..fac969d 100644
--- a/api/src/org/apache/cloudstack/api/auth/APIAuthenticationType.java
+++ b/api/src/org/apache/cloudstack/api/auth/APIAuthenticationType.java
@@ -17,5 +17,5 @@
 package org.apache.cloudstack.api.auth;
 
 public enum APIAuthenticationType {
-    LOGIN_API, LOGOUT_API
+    LOGIN_API, LOGOUT_API, READONLY_API
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/88f63d51/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListIdpsCmd.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListIdpsCmd.java
b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListIdpsCmd.java
index 7d7c95e..43ecfc5 100644
--- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListIdpsCmd.java
+++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListIdpsCmd.java
@@ -97,7 +97,7 @@ public class ListIdpsCmd extends BaseCmd implements APIAuthenticator {
 
     @Override
     public APIAuthenticationType getAPIType() {
-        return APIAuthenticationType.LOGIN_API;
+        return APIAuthenticationType.READONLY_API;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/88f63d51/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java
----------------------------------------------------------------------
diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java
b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java
index b05ebf6..d4940ec 100644
--- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java
+++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java
@@ -326,13 +326,13 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements
APIAuthent
                             resp.addCookie(new Cookie("domainid", URLEncoder.encode(loginResponse.getDomainId(),
HttpUtils.UTF_8)));
                             resp.addCookie(new Cookie("role", URLEncoder.encode(loginResponse.getType(),
HttpUtils.UTF_8)));
                             resp.addCookie(new Cookie("username", URLEncoder.encode(loginResponse.getUsername(),
HttpUtils.UTF_8)));
-                            resp.addCookie(new Cookie(ApiConstants.SESSIONKEY, URLEncoder.encode(loginResponse.getSessionKey(),
HttpUtils.UTF_8)));
                             resp.addCookie(new Cookie("account", URLEncoder.encode(loginResponse.getAccount(),
HttpUtils.UTF_8)));
                             String timezone = loginResponse.getTimeZone();
                             if (timezone != null) {
                                 resp.addCookie(new Cookie("timezone", URLEncoder.encode(timezone,
HttpUtils.UTF_8)));
                             }
                             resp.addCookie(new Cookie("userfullname", URLEncoder.encode(loginResponse.getFirstName()
+ " " + loginResponse.getLastName(), HttpUtils.UTF_8).replace("+", "%20")));
+                            resp.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly",
ApiConstants.SESSIONKEY, loginResponse.getSessionKey()));
                             resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value());
                             return ApiResponseSerializer.toSerializedString(loginResponse,
responseType);
                         }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/88f63d51/server/src/com/cloud/api/ApiServlet.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiServlet.java b/server/src/com/cloud/api/ApiServlet.java
index 2bffc77..d9e77bd 100644
--- a/server/src/com/cloud/api/ApiServlet.java
+++ b/server/src/com/cloud/api/ApiServlet.java
@@ -16,19 +16,13 @@
 // under the License.
 package com.cloud.api;
 
-import java.io.UnsupportedEncodingException;
-import java.net.URLDecoder;
-import java.util.HashMap;
-import java.util.Map;
-
-import javax.inject.Inject;
-import javax.servlet.ServletConfig;
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import javax.servlet.http.HttpSession;
-
+import com.cloud.user.Account;
+import com.cloud.user.AccountService;
+import com.cloud.user.User;
+import com.cloud.utils.HttpUtils;
+import com.cloud.utils.StringUtils;
+import com.cloud.utils.db.EntityManager;
+import com.cloud.utils.net.NetUtils;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiServerService;
 import org.apache.cloudstack.api.ServerApiException;
@@ -41,13 +35,18 @@ import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 import org.springframework.web.context.support.SpringBeanAutowiringSupport;
 
-import com.cloud.user.Account;
-import com.cloud.user.AccountService;
-import com.cloud.user.User;
-import com.cloud.utils.HttpUtils;
-import com.cloud.utils.StringUtils;
-import com.cloud.utils.db.EntityManager;
-import com.cloud.utils.net.NetUtils;
+import javax.inject.Inject;
+import javax.servlet.ServletConfig;
+import javax.servlet.ServletException;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
+import java.util.HashMap;
+import java.util.Map;
 
 @Component("apiServlet")
 @SuppressWarnings("serial")
@@ -180,7 +179,7 @@ public class ApiServlet extends HttpServlet {
                         }
                         session = req.getSession(true);
                         if (ApiServer.isSecureSessionCookieEnabled()) {
-                            resp.setHeader("SET-COOKIE", "JSESSIONID=" + session.getId()
+ ";Secure;Path=/client");
+                            resp.setHeader("SET-COOKIE", String.format("JSESSIONID=%s;Secure;HttpOnly;Path=/client",
session.getId()));
                             if (s_logger.isDebugEnabled()) {
                                 if (s_logger.isDebugEnabled()) {
                                     s_logger.debug("Session cookie is marked secure!");
@@ -191,11 +190,15 @@ public class ApiServlet extends HttpServlet {
 
                     try {
                         responseString = apiAuthenticator.authenticate(command, params, session,
remoteAddress, responseType, auditTrailSb, req, resp);
+                        if (session != null && session.getAttribute(ApiConstants.SESSIONKEY)
!= null) {
+                            resp.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly",
ApiConstants.SESSIONKEY, session.getAttribute(ApiConstants.SESSIONKEY)));
+                        }
                     } catch (ServerApiException e) {
                         httpResponseCode = e.getErrorCode().getHttpCode();
                         responseString = e.getMessage();
                         s_logger.debug("Authentication failure: " + e.getMessage());
                     }
+
                     if (apiAuthenticator.getAPIType() == APIAuthenticationType.LOGOUT_API)
{
                         if (session != null) {
                             final Long userId = (Long) session.getAttribute("userid");
@@ -213,6 +216,9 @@ public class ApiServlet extends HttpServlet {
                             } catch (final IllegalStateException ignored) {
                             }
                         }
+                        Cookie sessionKeyCookie = new Cookie(ApiConstants.SESSIONKEY, "");
+                        sessionKeyCookie.setMaxAge(0);
+                        resp.addCookie(sessionKeyCookie);
                     }
                     HttpUtils.writeHttpResponse(resp, responseString, httpResponseCode, responseType,
ApiServer.getJSONContentType());
                     return;
@@ -229,11 +235,15 @@ public class ApiServlet extends HttpServlet {
 
             if (!isNew) {
                 userId = (Long)session.getAttribute("userid");
-                final String account = (String)session.getAttribute("account");
+                final String account = (String) session.getAttribute("account");
                 final Object accountObj = session.getAttribute("accountobj");
-                final String sessionKey = (String)session.getAttribute(ApiConstants.SESSIONKEY);
-                final String[] sessionKeyParam = (String[])params.get(ApiConstants.SESSIONKEY);
-                if ((sessionKeyParam == null) || (sessionKey == null) || !sessionKey.equals(sessionKeyParam[0]))
{
+                final String sessionKey = (String) session.getAttribute(ApiConstants.SESSIONKEY);
+                final String sessionKeyFromCookie = HttpUtils.findCookie(req.getCookies(),
ApiConstants.SESSIONKEY);
+                final String[] sessionKeyFromParams = (String[]) params.get(ApiConstants.SESSIONKEY);
+                if ((sessionKey == null)
+                        || (sessionKeyFromParams == null && sessionKeyFromCookie
== null)
+                        || (sessionKeyFromParams != null && !sessionKey.equals(sessionKeyFromParams[0]))
+                        || (sessionKeyFromCookie != null && !sessionKey.equals(sessionKeyFromCookie)))
{
                     try {
                         session.invalidate();
                     } catch (final IllegalStateException ise) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/88f63d51/ui/scripts/cloudStack.js
----------------------------------------------------------------------
diff --git a/ui/scripts/cloudStack.js b/ui/scripts/cloudStack.js
index feb6e26..d0ef11b 100644
--- a/ui/scripts/cloudStack.js
+++ b/ui/scripts/cloudStack.js
@@ -107,28 +107,11 @@
             // Use this for checking the session, to bypass login screen
             bypassLoginCheck: function(args) { //determine to show or bypass login screen
                 if (g_loginResponse == null) { //show login screen
-                    /*
-                     * Since we no longer store sessionKey in cookie, opening the
-                     * 2nd browser window (of the same domain) will show login screen (i.e.
user has to
-                     * enter credentials again) and will cause the 1st browser window session
timeout.
-                     */
                     var unBoxCookieValue = function (cookieName) {
-                        var cookieValue = $.cookie(cookieName);
-                        if (cookieValue && cookieValue.length > 2 && cookieValue[0]
=== '"' && cookieValue[cookieValue.length-1] === '"') {
-                            cookieValue = cookieValue.slice(1, cookieValue.length-1);
-                            $.cookie(cookieName, cookieValue, { expires: 1 });
-                        }
-                        return decodeURIComponent(cookieValue);
+                        return decodeURIComponent($.cookie(cookieName)).replace(/"([^"]+(?="))"/g,
'$1');
                     };
-                    unBoxCookieValue('sessionkey');
-                    // if sessionkey cookie exists use this to set g_sessionKey
-                    // and destroy sessionkey cookie
-                    if ($.cookie('sessionkey')) {
-                        g_sessionKey = $.cookie('sessionkey');
-                        $.cookie('sessionkey', null);
-                    } else {
-                        g_sessionKey = unBoxCookieValue('JSESSIONID');
-                    }
+                    // sessionkey is a HttpOnly cookie now, no need to pass as API param
+                    g_sessionKey = null;
                     g_role = unBoxCookieValue('role');
                     g_userid = unBoxCookieValue('userid');
                     g_domainid = unBoxCookieValue('domainid');
@@ -136,7 +119,7 @@
                     g_username = unBoxCookieValue('username');
                     g_userfullname = unBoxCookieValue('userfullname');
                     g_timezone = unBoxCookieValue('timezone');
-                } else { //single-sign-on	(bypass login screen)
+                } else { //single-sign-on (bypass login screen)
                     g_sessionKey = encodeURIComponent(g_loginResponse.sessionkey);
                     g_role = g_loginResponse.type;
                     g_username = g_loginResponse.username;
@@ -144,7 +127,7 @@
                     g_account = g_loginResponse.account;
                     g_domainid = g_loginResponse.domainid;
                     g_userfullname = g_loginResponse.firstname + ' ' + g_loginResponse.lastname;
-                    g_timezone = g_loginResponse.timezone;                    
+                    g_timezone = g_loginResponse.timezone;
                 }
 
                 var userValid = false;
@@ -180,7 +163,25 @@
                         return true;
                     }
                 });
-               
+
+                // Populate IDP list
+                $.ajax({
+                    type: 'GET',
+                    url: createURL('listIdps'),
+                    dataType: 'json',
+                    async: false,
+                    success: function(data, textStatus, xhr) {
+                        if (data && data.listidpsresponse && data.listidpsresponse.idp)
{
+                            var idpList = data.listidpsresponse.idp.sort(function (a, b)
{
+                                return a.orgName.localeCompare(b.orgName);
+                            });
+                            g_idpList = idpList;
+                        }
+                    },
+                    error: function(xhr) {
+                    },
+                });
+
                 return userValid ? {
                     user: {
                         userid: g_userid,
@@ -227,14 +228,16 @@
                     async: false,
                     success: function(json) {
                         var loginresponse = json.loginresponse;
-                        
-                        g_sessionKey = encodeURIComponent(loginresponse.sessionkey);
+                        // sessionkey is recevied as a HttpOnly cookie
+                        // therefore reset any g_sessionKey value, an explicit
+                        // param in the API call is no longer needed
+                        g_sessionKey = null;
                         g_role = loginresponse.type;
                         g_username = loginresponse.username;
                         g_userid = loginresponse.userid;
                         g_account = loginresponse.account;
                         g_domainid = loginresponse.domainid;
-                        g_timezone = loginresponse.timezone;                        
+                        g_timezone = loginresponse.timezone;
                         g_userfullname = loginresponse.firstname + ' ' + loginresponse.lastname;
 
                         $.cookie('username', g_username, {
@@ -331,15 +334,13 @@
                         g_kvmsnapshotenabled = null;
                         g_regionsecondaryenabled = null;
                         g_loginCmdText = null;
-                        
-                        $.cookie('JSESSIONID', null);
-                        $.cookie('sessionkey', null);
-                        $.cookie('username', null);
-                        $.cookie('account', null);
-                        $.cookie('domainid', null);
-                        $.cookie('role', null);  
-                        $.cookie('timezone', null);
-                        
+
+                        // Remove any cookies
+                        var cookies = document.cookie.split(";");
+                        for (var i = 0; i < cookies.length; i++) {
+                            $.cookie($.trim(cookies[i].split("=")[0]), null);
+                        }
+
                         if (onLogoutCallback()) { //onLogoutCallback() will set g_loginResponse(single-sign-on
variable) to null, then bypassLoginCheck() will show login screen.
                             document.location.reload(); //when onLogoutCallback() returns
true, reload the current document.
                         }
@@ -367,13 +368,12 @@
                 g_regionsecondaryenabled = null;
                 g_loginCmdText = null;
 
-                $.cookie('JSESSIONID', null);
-                $.cookie('sessionkey', null);
-                $.cookie('username', null);
-                $.cookie('account', null);
-                $.cookie('domainid', null);
-                $.cookie('role', null);
-                $.cookie('timezone', null);
+                // Remove any cookies
+                var cookies = document.cookie.split(";");
+                for (var i = 0; i < cookies.length; i++) {
+                    $.cookie($.trim(cookies[i].split("=")[0]), null);
+                }
+
                 var url = 'samlSso';
                 if (args.data.idpid) {
                     url = url + '&idpid=' + args.data.idpid;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/88f63d51/ui/scripts/sharedFunctions.js
----------------------------------------------------------------------
diff --git a/ui/scripts/sharedFunctions.js b/ui/scripts/sharedFunctions.js
index 75860dc..11c87a0 100644
--- a/ui/scripts/sharedFunctions.js
+++ b/ui/scripts/sharedFunctions.js
@@ -170,7 +170,10 @@ var pollAsyncJobResult = function(args) {
 
     function createURL(apiName, options) {
         if (!options) options = {};
-        var urlString = clientApiUrl + "?" + "command=" + apiName + "&response=json&sessionkey="
+ g_sessionKey;
+        var urlString = clientApiUrl + "?" + "command=" + apiName + "&response=json";
+        if (g_sessionKey) {
+            urlString += "&sessionkey=" + g_sessionKey;
+        }
 
         if (cloudStack.context && cloudStack.context.projects && !options.ignoreProject)
{
             urlString = urlString + '&projectid=' + cloudStack.context.projects[0].id;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/88f63d51/ui/scripts/ui-custom/login.js
----------------------------------------------------------------------
diff --git a/ui/scripts/ui-custom/login.js b/ui/scripts/ui-custom/login.js
index bad40d2..8896dde 100644
--- a/ui/scripts/ui-custom/login.js
+++ b/ui/scripts/ui-custom/login.js
@@ -129,7 +129,7 @@
             return false;
         });
 
-        // Show SAML button if only SP is configured
+        // By Default hide login option dropdown
         $login.find('#login-dropdown').hide();
         $login.find('#saml-login').hide();
         $login.find('#cloudstack-login').hide();
@@ -160,55 +160,34 @@
             }
         });
 
-        $.ajax({
-            type: 'GET',
-            url: createURL('listIdps'),
-            dataType: 'json',
-            async: false,
-            success: function(data, textStatus, xhr) {
-                if (xhr.status === 200) {
-                    $login.find('#login-dropdown').show();
-                    $login.find('#login-submit').hide();
-                } else {
-                    $login.find('#cloudstack-login').show();
-                    $login.find('#login-submit').show();
-                    return;
-                }
-
+        // If any IdP servers were set, SAML is enabled
+        if (g_idpList && g_idpList.length > 0) {
+            $login.find('#login-dropdown').show();
+            $login.find('#login-submit').hide();
+            $login.find('#login-options')
+                .append($('<option>', {
+                    value: '',
+                    text: '--- Select Identity Provider -- ',
+                    selected: true
+                }));
+
+            $.each(g_idpList, function(index, idp) {
                 $login.find('#login-options')
                     .append($('<option>', {
-                        value: '',
-                        text: '--- Select Identity Provider -- ',
-                        selected: true
+                        value: idp.id,
+                        text: idp.orgName
                     }));
+            });
 
-                if (data.listidpsresponse && data.listidpsresponse.idp) {
-                    var idpList = data.listidpsresponse.idp.sort(function (a, b) {
-                        return a.orgName.localeCompare(b.orgName);
-                    });
-                    g_idpList = idpList;
-                    $.each(idpList, function(index, idp) {
-                        $login.find('#login-options')
-                            .append($('<option>', {
-                                value: idp.id,
-                                text: idp.orgName
-                            }));
-                    });
-                }
-
-                var loginOption = $.cookie('login-option');
-                if (loginOption) {
-                    var option = $login.find('#login-options option[value="' + loginOption
+ '"]');
-                    if (option.length > 0) {
-                        option.prop('selected', true);
-                        toggleLoginView(loginOption);
-                    }
+            var loginOption = $.cookie('login-option');
+            if (loginOption) {
+                var option = $login.find('#login-options option[value="' + loginOption +
'"]');
+                if (option.length > 0) {
+                    option.prop('selected', true);
+                    toggleLoginView(loginOption);
                 }
-            },
-            error: function(xhr) {
-                $login.find('#cloudstack-login').show();
-            },
-        });
+            }
+        }
 
         // Select language
         var $languageSelect = $login.find('select[name=language]');

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/88f63d51/utils/src/com/cloud/utils/HttpUtils.java
----------------------------------------------------------------------
diff --git a/utils/src/com/cloud/utils/HttpUtils.java b/utils/src/com/cloud/utils/HttpUtils.java
index 2940985..38447ff 100644
--- a/utils/src/com/cloud/utils/HttpUtils.java
+++ b/utils/src/com/cloud/utils/HttpUtils.java
@@ -51,6 +51,7 @@ public class HttpUtils {
     }
 
     public static String findCookie(final Cookie[] cookies, final String key) {
+        if (cookies == null || key == null) return null;
         for (Cookie cookie: cookies) {
             if (cookie != null && cookie.getName().equals(key)) {
                 return cookie.getValue();


Mime
View raw message