util: check JSESSIONID in cookies if user is passed

Author: weizhouapache

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java

@@ -48,6 +48,7 @@
 import org.apache.cloudstack.saml.SAMLUtils;
 import org.apache.log4j.Logger;
 
+import com.cloud.api.ApiServer;
 import com.cloud.api.response.ApiResponseSerializer;
 import com.cloud.domain.Domain;
 import com.cloud.domain.dao.DomainDao;
@@ -60,6 +61,8 @@
 import com.cloud.user.dao.UserDao;
 import com.cloud.utils.HttpUtils;
 
+import org.apache.commons.lang3.EnumUtils;
+
 @APICommand(name = "listAndSwitchSamlAccount", description = "Lists and switches to other SAML accounts owned by the SAML user", responseObject = SuccessResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthenticator {
     public static final Logger s_logger = Logger.getLogger(ListAndSwitchSAMLAccountCmd.class.getName());
@@ -104,7 +107,9 @@ public String authenticate(final String command, final Map<String, Object[]> par
                     params, responseType));
         }
 
-        if (!HttpUtils.validateSessionKey(session, params, req.getCookies(), ApiConstants.SESSIONKEY)) {
+        HttpUtils.ApiSessionKeyCheckOption sessionKeyCheckOption = EnumUtils.getEnumIgnoreCase(HttpUtils.ApiSessionKeyCheckOption.class,
+                ApiServer.ApiSessionKeyCheckLocations.value(), HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter);
+        if (!HttpUtils.validateSessionKey(session, params, req.getCookies(), ApiConstants.SESSIONKEY, sessionKeyCheckOption)) {
             throw new ServerApiException(ApiErrorCode.UNAUTHORIZED, _apiServer.getSerializedApiError(ApiErrorCode.UNAUTHORIZED.getHttpCode(),
                     "Unauthorized session, please re-login",
                     params, responseType));

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java

@@ -73,6 +73,9 @@ public interface SAML2AuthManager extends PluggableAPIAuthenticator, PluggableSe
     ConfigKey<Boolean> SAMLCheckSignature = new ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.check.signature", "true",
             "When enabled (default and recommended), SAML2 signature checks are enforced and lack of signature in the SAML SSO response will cause login exception. Disabling this is not advisable but provided for backward compatibility for users who are able to accept the risks.", false);
 
+    ConfigKey<String> SAMLUserSessionKeyPathAttribute = new ConfigKey<String>("Advanced", String.class, "saml2.user.sessionkey.path", "",
+            "The Path attribute of sessionkey cookie when SAML users have logged in. If not set, it will be set to the path of SAML redirection URL (saml2.redirect.url).", true);
+
     SAMLProviderMetadata getSPMetadata();
     SAMLProviderMetadata getIdPMetadata(String entityId);
     Collection<SAMLProviderMetadata> getAllIdPMetadata();

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java

@@ -542,6 +542,7 @@ public ConfigKey<?>[] getConfigKeys() {
                 SAMLServiceProviderSingleSignOnURL, SAMLServiceProviderSingleLogOutURL,
                 SAMLCloudStackRedirectionUrl, SAMLUserAttributeName,
                 SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId,
-                SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature};
+                SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature,
+                SAMLUserSessionKeyPathAttribute};
     }
 }

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java

@@ -25,6 +25,8 @@
 import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
 import java.math.BigInteger;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URLEncoder;
 import java.nio.charset.Charset;
 import java.security.InvalidKeyException;
@@ -101,7 +103,9 @@
 import org.w3c.dom.Element;
 import org.xml.sax.SAXException;
 
+import com.cloud.api.ApiServlet;
 import com.cloud.utils.HttpUtils;
+import com.cloud.utils.exception.CloudRuntimeException;
 
 public class SAMLUtils {
     public static final Logger s_logger = Logger.getLogger(SAMLUtils.class);
@@ -296,7 +300,26 @@ public static void setupSamlUserCookies(final LoginCmdResponse loginResponse, fi
             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;Path=/client/api", ApiConstants.SESSIONKEY, loginResponse.getSessionKey()));
+
+        String redirectUrl = SAML2AuthManager.SAMLCloudStackRedirectionUrl.value();
+        String path = SAML2AuthManager.SAMLUserSessionKeyPathAttribute.value();
+        String domain = null;
+        try {
+            URI redirectUri = new URI(redirectUrl);
+            domain = redirectUri.getHost();
+            if (StringUtils.isBlank(path)) {
+                path = redirectUri.getPath();
+            }
+            if (StringUtils.isBlank(path)) {
+                path = "/";
+            }
+        } catch (URISyntaxException ex) {
+            throw new CloudRuntimeException("Invalid URI: " + redirectUrl);
+        }
+        String sameSite = ApiServlet.getApiSessionKeySameSite();
+        String sessionKeyCookie = String.format("%s=%s;Domain=%s;Path=%s;%s", ApiConstants.SESSIONKEY, loginResponse.getSessionKey(), domain, path, sameSite);
+        s_logger.debug("Adding sessionkey cookie to response: " + sessionKeyCookie);
+        resp.addHeader("SET-COOKIE", sessionKeyCookie);
     }
 
     /**

plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java

@@ -27,6 +27,7 @@
 import java.util.HashMap;
 import java.util.Map;
 
+import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
@@ -88,13 +89,17 @@ public class ListAndSwitchSAMLAccountCmdTest extends TestCase {
     @Mock
     HttpServletRequest req;
 
+    final String sessionId = "node0xxxxxxxxxxxxx";
+    Cookie[] cookies;
+
     @Test
     public void testListAndSwitchSAMLAccountCmd() throws Exception {
         // Setup
         final Map<String, Object[]> params = new HashMap<String, Object[]>();
         final String sessionKeyValue = "someSessionIDValue";
         Mockito.when(session.getAttribute(ApiConstants.SESSIONKEY)).thenReturn(sessionKeyValue);
         Mockito.when(session.getAttribute("userid")).thenReturn(2L);
+        Mockito.when(session.getId()).thenReturn(sessionId);
         params.put(ApiConstants.USER_ID, new String[]{"2"});
         params.put(ApiConstants.DOMAIN_ID, new String[]{"1"});
         Mockito.when(userDao.findByUuid(anyString())).thenReturn(new UserVO(2L));
@@ -146,7 +151,25 @@ public void testListAndSwitchSAMLAccountCmd() throws Exception {
             Mockito.verify(accountService, Mockito.times(0)).getUserAccountById(Mockito.anyLong());
         }
 
-        // valid sessionkey value test
+        // valid sessionkey value and invalid JSESSIONID test
+        cookies = new Cookie[2];
+        cookies[0] = new Cookie(ApiConstants.SESSIONKEY, sessionKeyValue);
+        cookies[1] = new Cookie("JSESSIONID", "invalid-JSESSIONID");
+        Mockito.when(req.getCookies()).thenReturn(cookies);
+        params.put(ApiConstants.SESSIONKEY, new String[]{sessionKeyValue});
+        try {
+            cmd.authenticate("command", params, session, null, HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
+        } catch (ServerApiException exception) {
+            assertEquals(exception.getErrorCode(), ApiErrorCode.UNAUTHORIZED);
+        } finally {
+            Mockito.verify(accountService, Mockito.times(0)).getUserAccountById(Mockito.anyLong());
+        }
+
+        // valid sessionkey value and valid JSESSIONID test
+        cookies = new Cookie[2];
+        cookies[0] = new Cookie(ApiConstants.SESSIONKEY, sessionKeyValue);
+        cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0");
+        Mockito.when(req.getCookies()).thenReturn(cookies);
         params.put(ApiConstants.SESSIONKEY, new String[]{sessionKeyValue});
         try {
             cmd.authenticate("command", params, session, null, HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);

server/src/main/java/com/cloud/api/ApiServer.java

@@ -33,6 +33,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
+import java.util.EnumSet;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -47,6 +48,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import javax.crypto.Mac;
 import javax.crypto.spec.SecretKeySpec;
@@ -168,6 +170,8 @@
 import com.cloud.utils.ConstantTimeComparator;
 import com.cloud.utils.DateUtil;
 import com.cloud.utils.HttpUtils;
+import com.cloud.utils.HttpUtils.ApiSessionKeySameSite;
+import com.cloud.utils.HttpUtils.ApiSessionKeyCheckOption;
 import com.cloud.utils.Pair;
 import com.cloud.utils.ReflectUtil;
 import com.cloud.utils.StringUtils;
@@ -306,6 +310,24 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
             , true
             , ConfigKey.Scope.Global);
 
+    static final ConfigKey<String> ApiSessionKeyCookieSameSiteSetting = new ConfigKey<>(String.class
+            , "api.sessionkey.cookie.samesite"
+            , ConfigKey.CATEGORY_ADVANCED
+            , ApiSessionKeySameSite.Lax.name()
+            , "The SameSite attribute of cookie 'sessionkey'. Valid options are: Lax (default), Strict, NoneAndSecure and Null."
+            , true
+            , ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.Select,
+            EnumSet.allOf(ApiSessionKeySameSite.class).stream().map(Enum::toString).collect(Collectors.joining(", ")));
+
+    public static final ConfigKey<String> ApiSessionKeyCheckLocations = new ConfigKey<>(String.class
+            , "api.sessionkey.check.locations"
+            , ConfigKey.CATEGORY_ADVANCED
+            , ApiSessionKeyCheckOption.CookieAndParameter.name()
+            , "The locations of 'sessionkey' during the validation of the API requests. Valid options are: CookieOrParameter, ParameterOnly, CookieAndParameter (default)."
+            , true
+            , ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.Select,
+            EnumSet.allOf(ApiSessionKeyCheckOption.class).stream().map(Enum::toString).collect(Collectors.joining(", ")));
+
     @Override
     public boolean configure(final String name, final Map<String, Object> params) throws ConfigurationException {
         messageBus.subscribe(AsyncJob.Topics.JOB_EVENT_PUBLISH, MessageDispatcher.getDispatcher(this));
@@ -1531,7 +1553,9 @@ public ConfigKey<?>[] getConfigKeys() {
                 JSONDefaultContentType,
                 proxyForwardList,
                 useForwardHeader,
-                listOfForwardHeaders
+                listOfForwardHeaders,
+                ApiSessionKeyCookieSameSiteSetting,
+                ApiSessionKeyCheckLocations
         };
     }
 }