This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to tag 4.19.1.2
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 13dd6983428a535a5d401bce106dd5da9a1b802a
Author: Wei Zhou <weiz...@apache.org>
AuthorDate: Tue Sep 10 18:25:36 2024 +0200
util: check JSESSIONID in cookies if user is passed
---
.../api/command/ListAndSwitchSAMLAccountCmd.java | 7 +-
.../apache/cloudstack/saml/SAML2AuthManager.java | 3 +
.../cloudstack/saml/SAML2AuthManagerImpl.java | 3 +-
.../java/org/apache/cloudstack/saml/SAMLUtils.java | 25 ++++-
.../command/ListAndSwitchSAMLAccountCmdTest.java | 25 ++++-
server/src/main/java/com/cloud/api/ApiServer.java | 26 +++++-
server/src/main/java/com/cloud/api/ApiServlet.java | 102 +++++++++++++--------
test/integration/smoke/test_login.py | 1 +
ui/src/api/index.js | 10 ++
ui/src/store/modules/user.js | 1 -
utils/src/main/java/com/cloud/utils/HttpUtils.java | 42 +++++++--
.../test/java/com/cloud/utils/HttpUtilsTest.java | 86 ++++++++++++++---
12 files changed, 266 insertions(+), 65 deletions(-)
diff --git
a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java
b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java
index 25f056adf68..c2f81cd3356 100644
---
a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java
+++
b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java
@@ -48,6 +48,7 @@ import org.apache.cloudstack.saml.SAML2AuthManager;
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.UserAccountDao;
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 class ListAndSwitchSAMLAccountCmd extends BaseCmd
implements APIAuthentic
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));
diff --git
a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java
b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java
index 3a4030f9c0d..e10ea08012f 100644
---
a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java
+++
b/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();
diff --git
a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java
b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java
index dfa76414fb7..aa1e0be91c7 100644
---
a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java
+++
b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java
@@ -542,6 +542,7 @@ public class SAML2AuthManagerImpl extends AdapterBase
implements SAML2AuthManage
SAMLServiceProviderSingleSignOnURL,
SAMLServiceProviderSingleLogOutURL,
SAMLCloudStackRedirectionUrl, SAMLUserAttributeName,
SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId,
- SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout,
SAMLCheckSignature};
+ SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout,
SAMLCheckSignature,
+ SAMLUserSessionKeyPathAttribute};
}
}
diff --git
a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java
b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java
index f10bc891368..bb94c8af4c2 100644
---
a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java
+++
b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java
@@ -25,6 +25,8 @@ import java.io.IOException;
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.Document;
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 class SAMLUtils {
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);
}
/**
diff --git
a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java
b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java
index b4d230e3cd6..729334d22ce 100644
---
a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java
+++
b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java
@@ -27,6 +27,7 @@ import java.net.InetAddress;
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,6 +89,9 @@ public class ListAndSwitchSAMLAccountCmdTest extends TestCase
{
@Mock
HttpServletRequest req;
+ final String sessionId = "node0xxxxxxxxxxxxx";
+ Cookie[] cookies;
+
@Test
public void testListAndSwitchSAMLAccountCmd() throws Exception {
// Setup
@@ -95,6 +99,7 @@ public class ListAndSwitchSAMLAccountCmdTest extends TestCase
{
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 class ListAndSwitchSAMLAccountCmdTest extends
TestCase {
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);
diff --git a/server/src/main/java/com/cloud/api/ApiServer.java
b/server/src/main/java/com/cloud/api/ApiServer.java
index b1a04932cf9..d68f42470d5 100644
--- a/server/src/main/java/com/cloud/api/ApiServer.java
+++ b/server/src/main/java/com/cloud/api/ApiServer.java
@@ -33,6 +33,7 @@ import java.text.ParseException;
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.ThreadPoolExecutor;
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.user.UserVO;
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 class ApiServer extends ManagerBase implements
HttpRequestHandler, ApiSer
JSONDefaultContentType,
proxyForwardList,
useForwardHeader,
- listOfForwardHeaders
+ listOfForwardHeaders,
+ ApiSessionKeyCookieSameSiteSetting,
+ ApiSessionKeyCheckLocations
};
}
}
diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java
b/server/src/main/java/com/cloud/api/ApiServlet.java
index e719238afef..d9654f03916 100644
--- a/server/src/main/java/com/cloud/api/ApiServlet.java
+++ b/server/src/main/java/com/cloud/api/ApiServlet.java
@@ -47,6 +47,7 @@ import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.managed.context.ManagedContext;
import org.apache.cloudstack.utils.consoleproxy.ConsoleAccessUtils;
import org.apache.log4j.Logger;
+import org.apache.commons.lang3.EnumUtils;
import org.jetbrains.annotations.Nullable;
import org.springframework.stereotype.Component;
import org.springframework.web.context.support.SpringBeanAutowiringSupport;
@@ -63,13 +64,15 @@ import com.cloud.user.User;
import com.cloud.user.UserAccount;
import com.cloud.utils.HttpUtils;
+import com.cloud.utils.HttpUtils.ApiSessionKeySameSite;
+import com.cloud.utils.HttpUtils.ApiSessionKeyCheckOption;
import com.cloud.utils.StringUtils;
import com.cloud.utils.db.EntityManager;
import com.cloud.utils.net.NetUtils;
@Component("apiServlet")
public class ApiServlet extends HttpServlet {
- public static final Logger s_logger =
Logger.getLogger(ApiServlet.class.getName());
+ public static final Logger LOGGER =
Logger.getLogger(ApiServlet.class.getName());
private static final Logger s_accessLogger = Logger.getLogger("apiserver."
+ ApiServlet.class.getName());
private static final String REPLACEMENT = "_";
private static final String LOG_REPLACEMENTS = "[\n\r\t]";
@@ -127,7 +130,7 @@ public class ApiServlet extends HttpServlet {
String value = decodeUtf8(paramTokens[1]);
params.put(name, new String[] {value});
} else {
- s_logger.debug("Invalid parameter in URL found. param: " +
param);
+ LOGGER.debug("Invalid parameter in URL found. param: " +
param);
}
}
}
@@ -156,7 +159,7 @@ public class ApiServlet extends HttpServlet {
if (v.length > 1) {
String message = String.format("Query parameter '%s' has
multiple values %s. Only the last value will be respected." +
"It is advised to pass only a single parameter", k,
Arrays.toString(v));
- s_logger.warn(message);
+ LOGGER.warn(message);
}
});
@@ -167,7 +170,7 @@ public class ApiServlet extends HttpServlet {
try {
remoteAddress = getClientAddress(req);
} catch (UnknownHostException e) {
- s_logger.warn("UnknownHostException when trying to lookup remote
IP-Address. This should never happen. Blocking request.", e);
+ LOGGER.warn("UnknownHostException when trying to lookup remote
IP-Address. This should never happen. Blocking request.", e);
final String response =
apiServer.getSerializedApiError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
"UnknownHostException when trying to lookup remote
IP-Address", null,
HttpUtils.RESPONSE_TYPE_XML);
@@ -191,17 +194,17 @@ public class ApiServlet extends HttpServlet {
// logging the request start and end in management log for easy
debugging
String reqStr = "";
String cleanQueryString =
StringUtils.cleanString(req.getQueryString());
- if (s_logger.isDebugEnabled()) {
+ if (LOGGER.isDebugEnabled()) {
reqStr = auditTrailSb.toString() + " " + cleanQueryString;
- s_logger.debug("===START=== " + reqStr);
+ LOGGER.debug("===START=== " + reqStr);
}
try {
resp.setContentType(HttpUtils.XML_CONTENT_TYPE);
HttpSession session = req.getSession(false);
- if (s_logger.isTraceEnabled()) {
- s_logger.trace(String.format("session found: %s", session));
+ if (LOGGER.isTraceEnabled()) {
+ LOGGER.trace(String.format("session found: %s", session));
}
final Object[] responseTypeParam =
params.get(ApiConstants.RESPONSE);
if (responseTypeParam != null) {
@@ -212,10 +215,10 @@ public class ApiServlet extends HttpServlet {
final String command = commandObj == null ? null : (String)
commandObj[0];
final Object[] userObj = params.get(ApiConstants.USERNAME);
String username = userObj == null ? null : (String)userObj[0];
- if (s_logger.isTraceEnabled()) {
+ if (LOGGER.isTraceEnabled()) {
String logCommand = saveLogString(command);
String logName = saveLogString(username);
- s_logger.trace(String.format("command %s processing for user
\"%s\"",
+ LOGGER.trace(String.format("command %s processing for user
\"%s\"",
logCommand,
logName));
}
@@ -238,25 +241,26 @@ public class ApiServlet extends HttpServlet {
if (ApiServer.EnableSecureSessionCookie.value()) {
resp.setHeader("SET-COOKIE",
String.format("JSESSIONID=%s;Secure;HttpOnly;Path=/client", session.getId()));
- if (s_logger.isDebugEnabled()) {
- s_logger.debug("Session cookie is marked
secure!");
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("Session cookie is marked
secure!");
}
}
}
try {
- if (s_logger.isTraceEnabled()) {
-
s_logger.trace(String.format("apiAuthenticator.authenticate(%s, params[%d], %s,
%s, %s, %s, %s,%s)",
+ if (LOGGER.isTraceEnabled()) {
+
LOGGER.trace(String.format("apiAuthenticator.authenticate(%s, params[%d], %s,
%s, %s, %s, %s,%s)",
saveLogString(command), params.size(),
session.getId(), remoteAddress.getHostAddress(), saveLogString(responseType),
"auditTrailSb", "req", "resp"));
}
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)));
+ String sameSite = getApiSessionKeySameSite();
+ resp.addHeader("SET-COOKIE",
String.format("%s=%s;HttpOnly;%s", ApiConstants.SESSIONKEY,
session.getAttribute(ApiConstants.SESSIONKEY), sameSite));
}
} catch (ServerApiException e) {
httpResponseCode = e.getErrorCode().getHttpCode();
responseString = e.getMessage();
- s_logger.debug("Authentication failure: " +
e.getMessage());
+ LOGGER.debug("Authentication failure: " +
e.getMessage());
}
if (apiAuthenticator.getAPIType() ==
APIAuthenticationType.LOGOUT_API) {
@@ -289,7 +293,7 @@ public class ApiServlet extends HttpServlet {
return;
}
} else {
- s_logger.trace("no command available");
+ LOGGER.trace("no command available");
}
auditTrailSb.append(cleanQueryString);
final boolean isNew = ((session == null) ? true : session.isNew());
@@ -298,15 +302,15 @@ public class ApiServlet extends HttpServlet {
// we no longer rely on web-session here, verifyRequest will
populate user/account information
// if a API key exists
- if (isNew && s_logger.isTraceEnabled()) {
- s_logger.trace(String.format("new session: %s", session));
+ if (isNew && LOGGER.isTraceEnabled()) {
+ LOGGER.trace(String.format("new session: %s", session));
}
if (!isNew &&
(command.equalsIgnoreCase(ValidateUserTwoFactorAuthenticationCodeCmd.APINAME)
|| (!skip2FAcheckForAPIs(command) && !skip2FAcheckForUser(session)))) {
- s_logger.debug("Verifying two factor authentication");
+ LOGGER.debug("Verifying two factor authentication");
boolean success = verify2FA(session, command, auditTrailSb,
params, remoteAddress, responseType, req, resp);
if (!success) {
- s_logger.debug("Verification of two factor authentication
failed");
+ LOGGER.debug("Verification of two factor authentication
failed");
return;
}
}
@@ -319,8 +323,8 @@ public class ApiServlet extends HttpServlet {
if (account != null) {
if (invalidateHttpSessionIfNeeded(req, resp, auditTrailSb,
responseType, params, session, account)) return;
} else {
- if (s_logger.isDebugEnabled()) {
- s_logger.debug("no account, this request will be
validated through apikey(%s)/signature");
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("no account, this request will be
validated through apikey(%s)/signature");
}
}
@@ -330,8 +334,8 @@ public class ApiServlet extends HttpServlet {
CallContext.register(accountMgr.getSystemUser(),
accountMgr.getSystemAccount());
}
setProjectContext(params);
- if (s_logger.isTraceEnabled()) {
- s_logger.trace(String.format("verifying request for user %s
from %s with %d parameters",
+ if (LOGGER.isTraceEnabled()) {
+ LOGGER.trace(String.format("verifying request for user %s from
%s with %d parameters",
userId, remoteAddress.getHostAddress(),
params.size()));
}
if (apiServer.verifyRequest(params, userId, remoteAddress)) {
@@ -362,18 +366,34 @@ public class ApiServlet extends HttpServlet {
HttpUtils.writeHttpResponse(resp, serializedResponseText,
se.getErrorCode().getHttpCode(), responseType,
ApiServer.JSONcontentType.value());
auditTrailSb.append(" " + se.getErrorCode() + " " +
se.getDescription());
} catch (final Exception ex) {
- s_logger.error("unknown exception writing api response", ex);
+ LOGGER.error("unknown exception writing api response", ex);
auditTrailSb.append(" unknown exception writing api response");
} finally {
s_accessLogger.info(auditTrailSb.toString());
- if (s_logger.isDebugEnabled()) {
- s_logger.debug("===END=== " + reqStr);
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("===END=== " + reqStr);
}
// cleanup user context to prevent from being peeked in other
request context
CallContext.unregister();
}
}
+ public static String getApiSessionKeySameSite() {
+ ApiSessionKeySameSite sameSite =
EnumUtils.getEnumIgnoreCase(ApiSessionKeySameSite.class,
+ ApiServer.ApiSessionKeyCookieSameSiteSetting.value(),
ApiSessionKeySameSite.Lax);
+ switch (sameSite) {
+ case Strict:
+ return "SameSite=Strict";
+ case NoneAndSecure:
+ return "SameSite=None;Secure";
+ case Null:
+ return "";
+ case Lax:
+ default:
+ return "SameSite=Lax";
+ }
+ }
+
private boolean checkIfAuthenticatorIsOf2FA(String command) {
boolean verify2FA = false;
APIAuthenticator apiAuthenticator =
authManager.getAPIAuthenticator(command);
@@ -402,7 +422,7 @@ public class ApiServlet extends HttpServlet {
Long userId = (Long) session.getAttribute("userid");
boolean is2FAverified = (boolean)
session.getAttribute(ApiConstants.IS_2FA_VERIFIED);
if (is2FAverified) {
- s_logger.debug(String.format("Two factor authentication is already
verified for the user %d, so skipping", userId));
+ LOGGER.debug(String.format("Two factor authentication is already
verified for the user %d, so skipping", userId));
skip2FAcheck = true;
} else {
UserAccount userAccount = accountMgr.getUserAccountById(userId);
@@ -433,7 +453,7 @@ public class ApiServlet extends HttpServlet {
HttpUtils.writeHttpResponse(resp, responseString,
HttpServletResponse.SC_OK, responseType, ApiServer.JSONcontentType.value());
verify2FA = true;
} else {
- s_logger.error("Cannot find API authenticator while verifying
2FA");
+ LOGGER.error("Cannot find API authenticator while verifying
2FA");
auditTrailSb.append(" Cannot find API authenticator while
verifying 2FA");
verify2FA = false;
}
@@ -457,7 +477,7 @@ public class ApiServlet extends HttpServlet {
errorMsg = "Two factor authentication is mandated by admin,
user needs to setup 2FA using setupUserTwoFactorAuthentication API and" +
" then verify 2FA using
validateUserTwoFactorAuthenticationCode API before calling other APIs. Existing
session is invalidated.";
}
- s_logger.error(errorMsg);
+ LOGGER.error(errorMsg);
invalidateHttpSession(session, String.format("Unable to process
the API request for %s from %s due to %s", userId,
remoteAddress.getHostAddress(), errorMsg));
auditTrailSb.append(" " + ApiErrorCode.UNAUTHORIZED2FA + " " +
errorMsg);
@@ -488,7 +508,7 @@ public class ApiServlet extends HttpServlet {
private boolean requestChecksoutAsSane(HttpServletResponse resp,
StringBuilder auditTrailSb, String responseType, Map<String, Object[]> params,
HttpSession session, String command, Long userId, String account, Object
accountObj) {
if ((userId != null) && (account != null) && (accountObj != null) &&
apiServer.verifyUser(userId)) {
if (command == null) {
- s_logger.info("missing command, ignoring request...");
+ LOGGER.info("missing command, ignoring request...");
auditTrailSb.append(" " + HttpServletResponse.SC_BAD_REQUEST +
" " + "no command specified");
final String serializedResponse =
apiServer.getSerializedApiError(HttpServletResponse.SC_BAD_REQUEST, "no command
specified", params, responseType);
HttpUtils.writeHttpResponse(resp, serializedResponse,
HttpServletResponse.SC_BAD_REQUEST, responseType,
ApiServer.JSONcontentType.value());
@@ -509,7 +529,9 @@ public class ApiServlet extends HttpServlet {
}
private boolean invalidateHttpSessionIfNeeded(HttpServletRequest req,
HttpServletResponse resp, StringBuilder auditTrailSb, String responseType,
Map<String, Object[]> params, HttpSession session, String account) {
- if (!HttpUtils.validateSessionKey(session, params, req.getCookies(),
ApiConstants.SESSIONKEY)) {
+ ApiSessionKeyCheckOption sessionKeyCheckOption =
EnumUtils.getEnumIgnoreCase(ApiSessionKeyCheckOption.class,
+ ApiServer.ApiSessionKeyCheckLocations.value(),
ApiSessionKeyCheckOption.CookieAndParameter);
+ if (!HttpUtils.validateSessionKey(session, params, req.getCookies(),
ApiConstants.SESSIONKEY, sessionKeyCheckOption)) {
String msg = String.format("invalidating session %s for account
%s", session.getId(), account);
invalidateHttpSession(session, msg);
auditTrailSb.append(" " + HttpServletResponse.SC_UNAUTHORIZED + "
" + "unable to verify user credentials");
@@ -523,13 +545,13 @@ public class ApiServlet extends HttpServlet {
public static void invalidateHttpSession(HttpSession session, String msg) {
try {
- if (s_logger.isTraceEnabled()) {
- s_logger.trace(msg);
+ if (LOGGER.isTraceEnabled()) {
+ LOGGER.trace(msg);
}
session.invalidate();
} catch (final IllegalStateException ise) {
- if (s_logger.isTraceEnabled()) {
- s_logger.trace(String.format("failed to invalidate session
%s", session.getId()));
+ if (LOGGER.isTraceEnabled()) {
+ LOGGER.trace(String.format("failed to invalidate session %s",
session.getId()));
}
}
}
@@ -537,7 +559,7 @@ public class ApiServlet extends HttpServlet {
private void setProjectContext(Map<String, Object[]> requestParameters) {
final String[] command =
(String[])requestParameters.get(ApiConstants.COMMAND);
if (command == null) {
- s_logger.info("missing command, ignoring request...");
+ LOGGER.info("missing command, ignoring request...");
return;
}
@@ -585,14 +607,14 @@ public class ApiServlet extends HttpServlet {
header = header.trim();
ip = getCorrectIPAddress(request.getHeader(header));
if (StringUtils.isNotBlank(ip)) {
- s_logger.debug(String.format("found ip %s in header %s
", ip, header));
+ LOGGER.debug(String.format("found ip %s in header %s
", ip, header));
break;
}
} // no address found in header so ip is blank and use remote
addr
} // else not an allowed proxy address, ip is blank and use remote
addr
}
if (StringUtils.isBlank(ip)) {
- s_logger.trace(String.format("no ip found in headers, returning
remote address %s.", pretender.getHostAddress()));
+ LOGGER.trace(String.format("no ip found in headers, returning
remote address %s.", pretender.getHostAddress()));
return pretender;
}
diff --git a/test/integration/smoke/test_login.py
b/test/integration/smoke/test_login.py
index 40d8349a13d..acd6c4334ac 100644
--- a/test/integration/smoke/test_login.py
+++ b/test/integration/smoke/test_login.py
@@ -133,6 +133,7 @@ class TestLogin(cloudstackTestCase):
args["command"] = 'listUsers'
args["listall"] = 'true'
args["response"] = "json"
+ args["sessionkey"] = response.json()['loginresponse']['sessionkey']
response = session.get(self.server_url, params=args)
self.assertEqual(
response.status_code,
diff --git a/ui/src/api/index.js b/ui/src/api/index.js
index 14432010738..7ab87780a9d 100644
--- a/ui/src/api/index.js
+++ b/ui/src/api/index.js
@@ -15,8 +15,13 @@
// specific language governing permissions and limitations
// under the License.
+import Cookies from 'js-cookie'
import { axios, sourceToken } from '@/utils/request'
import { message, notification } from 'ant-design-vue'
+import { vueProps } from '@/vue-app'
+import {
+ ACCESS_TOKEN
+} from '@/store/mutation-types'
export function api (command, args = {}, method = 'GET', data = {}) {
let params = {}
@@ -30,6 +35,11 @@ export function api (command, args = {}, method = 'GET',
data = {}) {
})
}
+ const sessionkey = vueProps.$localStorage.get(ACCESS_TOKEN) ||
Cookies.get('sessionkey')
+ if (sessionkey) {
+ args.sessionkey = sessionkey
+ }
+
return axios({
params: {
...args
diff --git a/ui/src/store/modules/user.js b/ui/src/store/modules/user.js
index 08a0c340c64..46a1905619f 100644
--- a/ui/src/store/modules/user.js
+++ b/ui/src/store/modules/user.js
@@ -24,7 +24,6 @@ import router from '@/router'
import store from '@/store'
import { oauthlogin, login, logout, api } from '@/api'
import { i18n } from '@/locales'
-import { sourceToken } from '@/utils/request'
import {
ACCESS_TOKEN,
diff --git a/utils/src/main/java/com/cloud/utils/HttpUtils.java
b/utils/src/main/java/com/cloud/utils/HttpUtils.java
index a5d9f6a16b6..cc97bf4ba15 100644
--- a/utils/src/main/java/com/cloud/utils/HttpUtils.java
+++ b/utils/src/main/java/com/cloud/utils/HttpUtils.java
@@ -37,6 +37,14 @@ public class HttpUtils {
public static final String JSON_CONTENT_TYPE = "application/json;
charset=UTF-8";
public static final String XML_CONTENT_TYPE = "text/xml; charset=UTF-8";
+ public enum ApiSessionKeySameSite {
+ Lax, Strict, NoneAndSecure, Null
+ }
+
+ public enum ApiSessionKeyCheckOption {
+ CookieOrParameter, ParameterOnly, CookieAndParameter
+ }
+
public static void addSecurityHeaders(final HttpServletResponse resp) {
if (resp.containsHeader("X-Content-Type-Options")) {
resp.setHeader("X-Content-Type-Options", "nosniff");
@@ -103,23 +111,43 @@ public class HttpUtils {
return null;
}
- public static boolean validateSessionKey(final HttpSession session, final
Map<String, Object[]> params, final Cookie[] cookies, final String
sessionKeyString) {
+ public static boolean validateSessionKey(final HttpSession session, final
Map<String, Object[]> params, final Cookie[] cookies, final String
sessionKeyString, final ApiSessionKeyCheckOption apiSessionKeyCheckLocations) {
if (session == null || sessionKeyString == null) {
return false;
}
+ final String jsessionidFromCookie = HttpUtils.findCookie(cookies,
"JSESSIONID");
+ if (jsessionidFromCookie == null
+ || !(jsessionidFromCookie.startsWith(session.getId() + '.'))) {
+ s_logger.error("JSESSIONID from cookie is invalid.");
+ return false;
+ }
final String sessionKey = (String)
session.getAttribute(sessionKeyString);
+ if (sessionKey == null) {
+ s_logger.error("sessionkey attribute of the session is null.");
+ return false;
+ }
final String sessionKeyFromCookie = HttpUtils.findCookie(cookies,
sessionKeyString);
+ boolean isSessionKeyFromCookieValid = sessionKeyFromCookie != null &&
sessionKey.equals(sessionKeyFromCookie);
+
String[] sessionKeyFromParams = null;
if (params != null) {
sessionKeyFromParams = (String[]) params.get(sessionKeyString);
}
- if ((sessionKey == null)
- || (sessionKeyFromParams == null && sessionKeyFromCookie ==
null)
- || (sessionKeyFromParams != null &&
!sessionKey.equals(sessionKeyFromParams[0]))
- || (sessionKeyFromCookie != null &&
!sessionKey.equals(sessionKeyFromCookie))) {
- return false;
+ boolean isSessionKeyFromParamsValid = sessionKeyFromParams != null &&
sessionKey.equals(sessionKeyFromParams[0]);
+
+ switch (apiSessionKeyCheckLocations) {
+ case CookieOrParameter:
+ return (sessionKeyFromCookie != null || sessionKeyFromParams
!= null)
+ && (sessionKeyFromCookie == null ||
isSessionKeyFromCookieValid)
+ && (sessionKeyFromParams == null ||
isSessionKeyFromParamsValid);
+ case ParameterOnly:
+ return sessionKeyFromParams != null &&
isSessionKeyFromParamsValid
+ && (sessionKeyFromCookie == null ||
isSessionKeyFromCookieValid);
+ case CookieAndParameter:
+ default:
+ return sessionKeyFromCookie != null &&
isSessionKeyFromCookieValid
+ && sessionKeyFromParams != null &&
isSessionKeyFromParamsValid;
}
- return true;
}
}
diff --git a/utils/src/test/java/com/cloud/utils/HttpUtilsTest.java
b/utils/src/test/java/com/cloud/utils/HttpUtilsTest.java
index e10a5a36b27..e94724ce3d6 100644
--- a/utils/src/test/java/com/cloud/utils/HttpUtilsTest.java
+++ b/utils/src/test/java/com/cloud/utils/HttpUtilsTest.java
@@ -60,35 +60,97 @@ public class HttpUtilsTest {
final String sessionKeyValue = "randomUniqueSessionID";
// session and sessionKeyString null test
- assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
sessionKeyString = "sessionkey";
- assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
// param and cookie null test
session = new MockHttpSession();
+ final String sessionId = session.getId();
session.setAttribute(sessionKeyString, sessionKeyValue);
- assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
- // param null, cookies not null test
+ // param null, cookies not null test (JSESSIONID is null)
params = null;
cookies = new Cookie[]{new Cookie(sessionKeyString, sessionKeyValue)};
- assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
"randomString"));
- assertTrue(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
"randomString", HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+
+ // param null, cookies not null test (JSESSIONID is not null and
matches)
+ cookies = new Cookie[2];
+ cookies[0] = new Cookie(sessionKeyString, sessionKeyValue);
+ cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0");
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
"randomString", HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+
+ // param null, cookies not null test (JSESSIONID is not null but
mismatches)
+ cookies = new Cookie[2];
+ cookies[0] = new Cookie(sessionKeyString, sessionKeyValue);
+ cookies[1] = new Cookie("JSESSIONID", "node0xxxxxxxxxxxxx.node0");
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
"randomString", HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
// param not null, cookies null test
params = new HashMap<String, Object[]>();
params.put(sessionKeyString, new String[]{"randomString"});
cookies = null;
- assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
params.put(sessionKeyString, new String[]{sessionKeyValue});
- assertTrue(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
- // both param and cookies not null test
+ // both param and cookies not null test (JSESSIONID is null)
params = new HashMap<String, Object[]>();
- cookies = new Cookie[]{new Cookie(sessionKeyString, sessionKeyValue)};
+ cookies = new Cookie[2];
+ cookies[0] = new Cookie(sessionKeyString, sessionKeyValue);
params.put(sessionKeyString, new String[]{"incorrectValue"});
- assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
params.put(sessionKeyString, new String[]{sessionKeyValue});
- assertTrue(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+
+ // both param and cookies not null test (JSESSIONID is not null but
mismatches)
+ params = new HashMap<String, Object[]>();
+ cookies = new Cookie[2];
+ cookies[0] = new Cookie(sessionKeyString, sessionKeyValue);
+ cookies[1] = new Cookie("JSESSIONID", "node0xxxxxxxxxxxxx.node0");
+ params.put(sessionKeyString, new String[]{"incorrectValue"});
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+ params.put(sessionKeyString, new String[]{sessionKeyValue});
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+
+ // both param and cookies not null test (JSESSIONID is not null amd
matches)
+ params = new HashMap<String, Object[]>();
+ cookies = new Cookie[2];
+ cookies[0] = new Cookie(sessionKeyString, sessionKeyValue);
+ cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0");
+ params.put(sessionKeyString, new String[]{"incorrectValue"});
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+ params.put(sessionKeyString, new String[]{sessionKeyValue});
+ assertTrue(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+
+ // param not null, cookies null test (JSESSIONID is not null amd
matches)
+ params = new HashMap<String, Object[]>();
+ cookies = new Cookie[1];
+ cookies[0] = new Cookie("JSESSIONID", sessionId + ".node0");
+ params.put(sessionKeyString, new String[]{"incorrectValue"});
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.ParameterOnly));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter));
+ params.put(sessionKeyString, new String[]{sessionKeyValue});
+ assertTrue(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.ParameterOnly));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter));
+
+ // param not null (correct), cookies not null test (correct)
+ cookies = new Cookie[2];
+ cookies[0] = new Cookie(sessionKeyString, sessionKeyValue);
+ cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0");
+ assertTrue(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+ assertTrue(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.ParameterOnly));
+ assertTrue(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter));
+
+ // param not null (correct), cookies not null test (wrong)
+ cookies = new Cookie[2];
+ cookies[0] = new Cookie(sessionKeyString, "incorrectValue");
+ cookies[1] = new Cookie("JSESSIONID", sessionId + ".node0");
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieOrParameter));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.ParameterOnly));
+ assertFalse(HttpUtils.validateSessionKey(session, params, cookies,
sessionKeyString, HttpUtils.ApiSessionKeyCheckOption.CookieAndParameter));
}
}
