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 c2f81cd3356..040d5414f26 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 @@ -133,10 +133,12 @@ public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthentic } if (userUuid != null && domainUuid != null) { + s_logger.debug("User [" + currentUserAccount.getUsername() + "] is requesting to switch from user profile [" + currentUserAccount.getId() + "] to useraccount [" + userUuid + "] in domain [" + domainUuid + "]"); final User user = _userDao.findByUuid(userUuid); final Domain domain = _domainDao.findByUuid(domainUuid); final UserAccount nextUserAccount = _accountService.getUserAccountById(user.getId()); if (nextUserAccount != null && !nextUserAccount.getAccountState().equals(Account.State.ENABLED.toString())) { + s_logger.warn("User [" + currentUserAccount.getUsername() + "] is requesting to switch from user profile [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] but the associated target account [" + nextUserAccount.getAccountName() + "] is not enabled"); throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, _apiServer.getSerializedApiError(ApiErrorCode.PARAM_ERROR.getHttpCode(), "The requested user account is locked and cannot be switched to, please contact your administrator.", params, responseType)); @@ -147,20 +149,26 @@ public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthentic || !nextUserAccount.getExternalEntity().equals(currentUserAccount.getExternalEntity()) || (nextUserAccount.getDomainId() != domain.getId()) || (nextUserAccount.getSource() != User.Source.SAML2)) { + s_logger.warn("User [" + currentUserAccount.getUsername() + "] is requesting to switch from user profile [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] but the associated target account is not found or invalid"); throw new ServerApiException(ApiErrorCode.PARAM_ERROR, _apiServer.getSerializedApiError(ApiErrorCode.PARAM_ERROR.getHttpCode(), "User account is not allowed to switch to the requested account", params, responseType)); } try { if (_apiServer.verifyUser(nextUserAccount.getId())) { + s_logger.info("User [" + currentUserAccount.getUsername() + "] user profile switch is accepted: from [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" + nextUserAccount.getAccountName() + "]"); + // need to set a sessoin variable to inform the login function of the specific user to login as, rather than using email only (which could have multiple matches) + session.setAttribute("nextUserId", user.getId()); final LoginCmdResponse loginResponse = (LoginCmdResponse) _apiServer.loginUser(session, nextUserAccount.getUsername(), nextUserAccount.getUsername() + nextUserAccount.getSource().toString(), nextUserAccount.getDomainId(), null, remoteAddress, params); SAMLUtils.setupSamlUserCookies(loginResponse, resp); - resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value()); + session.removeAttribute("nextUserId"); + s_logger.debug("User [" + currentUserAccount.getUsername() + "] user profile switch cookies set: from [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" + nextUserAccount.getAccountName() + "]"); + //resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value()); return ApiResponseSerializer.toSerializedString(loginResponse, responseType); } } catch (CloudAuthenticationException | IOException exception) { - s_logger.debug("Failed to switch to request SAML user account due to: " + exception.getMessage()); + s_logger.debug("User [" + currentUserAccount.getUsername() + "] user profile switch cookies set FAILED: from [" + currentUserId + "] to user profile [" + userUuid + "] in domain [" + domainUuid + "] with account [" + nextUserAccount.getAccountName() + "]", exception); } } else { List switchableAccounts = _userAccountDao.getAllUsersByNameAndEntity(currentUserAccount.getUsername(), currentUserAccount.getExternalEntity()); @@ -178,6 +186,9 @@ public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthentic accountResponse.setAccountName(userAccount.getAccountName()); accountResponse.setIdpId(user.getExternalEntity()); accountResponses.add(accountResponse); + if (s_logger.isDebugEnabled()) { + s_logger.debug("Returning available useraccount for [" + currentUserAccount.getUsername() + "]: UserUUID: [" + user.getUuid() + "], DomainUUID: [" + domain.getUuid() + "], Account: [" + userAccount.getAccountName() + "]"); + } } ListResponse response = new ListResponse(); response.setResponses(accountResponses); diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java index 332e0602784..0f25123ff88 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java @@ -192,7 +192,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent String authnId = SAMLUtils.generateSecureRandomId(); samlAuthManager.saveToken(authnId, domainPath, idpMetadata.getEntityId()); s_logger.debug("Sending SAMLRequest id=" + authnId); - String redirectUrl = SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value()); + String redirectUrl = SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value(), SAML2AuthManager.SAMLRequirePasswordLogin.value()); resp.sendRedirect(redirectUrl); return ""; } if (params.containsKey("SAMLart")) { 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 4e8ba16c739..523f694d80b 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 @@ -79,6 +79,10 @@ public interface SAML2AuthManager extends PluggableAPIAuthenticator, PluggableSe ConfigKey SAMLUserSessionKeyPathAttribute = new ConfigKey("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); + ConfigKey SAMLRequirePasswordLogin = new ConfigKey("Advanced", Boolean.class, "saml2.require.password", "true", + "When enabled SAML2 will validate that the SAML login was performed with a password. If disabled, other forms of authentication are allowed (two-factor, certificate, etc) on the SAML Authentication Provider", true); + + SAMLProviderMetadata getSPMetadata(); SAMLProviderMetadata getIdPMetadata(String entityId); Collection 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 a7524ec63a7..92408141ef2 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 @@ -543,6 +543,6 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage SAMLCloudStackRedirectionUrl, SAMLUserAttributeName, SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId, SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature, - SAMLForceAuthn, SAMLUserSessionKeyPathAttribute}; + SAMLForceAuthn, SAMLUserSessionKeyPathAttribute, SAMLRequirePasswordLogin}; } } 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 fd68e2be1ae..2460e3826c6 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 @@ -151,11 +151,11 @@ public class SAMLUtils { return null; } - public static String buildAuthnRequestUrl(final String authnId, final SAMLProviderMetadata spMetadata, final SAMLProviderMetadata idpMetadata, final String signatureAlgorithm) { + public static String buildAuthnRequestUrl(final String authnId, final SAMLProviderMetadata spMetadata, final SAMLProviderMetadata idpMetadata, final String signatureAlgorithm, boolean requirePasswordAuthentication) { String redirectUrl = ""; try { DefaultBootstrap.bootstrap(); - AuthnRequest authnRequest = SAMLUtils.buildAuthnRequestObject(authnId, spMetadata.getEntityId(), idpMetadata.getSsoUrl(), spMetadata.getSsoUrl()); + AuthnRequest authnRequest = SAMLUtils.buildAuthnRequestObject(authnId, spMetadata.getEntityId(), idpMetadata.getSsoUrl(), spMetadata.getSsoUrl(), requirePasswordAuthentication); PrivateKey privateKey = null; if (spMetadata.getKeyPair() != null) { privateKey = spMetadata.getKeyPair().getPrivate(); @@ -168,13 +168,36 @@ public class SAMLUtils { return redirectUrl; } - public static AuthnRequest buildAuthnRequestObject(final String authnId, final String spId, final String idpUrl, final String consumerUrl) { + public static AuthnRequest buildAuthnRequestObject(final String authnId, final String spId, final String idpUrl, final String consumerUrl, boolean requirePasswordAuthentication) { // Issuer object IssuerBuilder issuerBuilder = new IssuerBuilder(); Issuer issuer = issuerBuilder.buildObject(); issuer.setValue(spId); - // AuthnContextClass + // Creation of AuthRequestObject + AuthnRequestBuilder authRequestBuilder = new AuthnRequestBuilder(); + AuthnRequest authnRequest = authRequestBuilder.buildObject(); + + // AuthnContextClass. When this is false, the authentication requirements are defered to the SAML IDP and its default or configured workflow + if (requirePasswordAuthentication) { + setRequestedAuthnContext(authnRequest, requirePasswordAuthentication); + } + + authnRequest.setID(authnId); + authnRequest.setDestination(idpUrl); + authnRequest.setVersion(SAMLVersion.VERSION_20); + authnRequest.setForceAuthn(SAML2AuthManager.SAMLForceAuthn.value()); + authnRequest.setIsPassive(false); + authnRequest.setIssueInstant(new DateTime()); + authnRequest.setProtocolBinding(SAMLConstants.SAML2_POST_BINDING_URI); + authnRequest.setAssertionConsumerServiceURL(consumerUrl); + authnRequest.setProviderName(spId); + authnRequest.setIssuer(issuer); + + return authnRequest; + } + + public static void setRequestedAuthnContext(AuthnRequest authnRequest, boolean requirePasswordAuthentication) { AuthnContextClassRefBuilder authnContextClassRefBuilder = new AuthnContextClassRefBuilder(); AuthnContextClassRef authnContextClassRef = authnContextClassRefBuilder.buildObject( SAMLConstants.SAML20_NS, @@ -186,23 +209,7 @@ public class SAMLUtils { RequestedAuthnContext requestedAuthnContext = requestedAuthnContextBuilder.buildObject(); requestedAuthnContext.setComparison(AuthnContextComparisonTypeEnumeration.EXACT); requestedAuthnContext.getAuthnContextClassRefs().add(authnContextClassRef); - - // Creation of AuthRequestObject - AuthnRequestBuilder authRequestBuilder = new AuthnRequestBuilder(); - AuthnRequest authnRequest = authRequestBuilder.buildObject(); - authnRequest.setID(authnId); - authnRequest.setDestination(idpUrl); - authnRequest.setVersion(SAMLVersion.VERSION_20); - authnRequest.setForceAuthn(SAML2AuthManager.SAMLForceAuthn.value()); - authnRequest.setIsPassive(false); - authnRequest.setIssueInstant(new DateTime()); - authnRequest.setProtocolBinding(SAMLConstants.SAML2_POST_BINDING_URI); - authnRequest.setAssertionConsumerServiceURL(consumerUrl); - authnRequest.setProviderName(spId); - authnRequest.setIssuer(issuer); authnRequest.setRequestedAuthnContext(requestedAuthnContext); - - return authnRequest; } public static LogoutRequest buildLogoutRequest(String logoutUrl, String spId, String nameIdString) { @@ -284,23 +291,6 @@ public class SAMLUtils { } public static void setupSamlUserCookies(final LoginCmdResponse loginResponse, final HttpServletResponse resp) throws IOException { - resp.addCookie(new Cookie("userid", URLEncoder.encode(loginResponse.getUserId(), HttpUtils.UTF_8))); - 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("account", URLEncoder.encode(loginResponse.getAccount(), HttpUtils.UTF_8))); - resp.addCookie(new Cookie("isSAML", URLEncoder.encode("true", HttpUtils.UTF_8))); - resp.addCookie(new Cookie("twoFaEnabled", URLEncoder.encode(loginResponse.is2FAenabled(), HttpUtils.UTF_8))); - String providerFor2FA = loginResponse.getProviderFor2FA(); - if (StringUtils.isNotEmpty(providerFor2FA)) { - resp.addCookie(new Cookie("twoFaProvider", URLEncoder.encode(loginResponse.getProviderFor2FA(), 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"))); - String redirectUrl = SAML2AuthManager.SAMLCloudStackRedirectionUrl.value(); String path = SAML2AuthManager.SAMLUserSessionKeyPathAttribute.value(); String domain = null; @@ -316,6 +306,18 @@ public class SAMLUtils { } catch (URISyntaxException ex) { throw new CloudRuntimeException("Invalid URI: " + redirectUrl); } + + addBaseCookies(loginResponse, resp, domain, path); + + String providerFor2FA = loginResponse.getProviderFor2FA(); + if (StringUtils.isNotEmpty(providerFor2FA)) { + resp.addCookie(newCookie(domain, path,"twoFaProvider", URLEncoder.encode(loginResponse.getProviderFor2FA(), HttpUtils.UTF_8))); + } + String timezone = loginResponse.getTimeZone(); + if (timezone != null) { + resp.addCookie(newCookie(domain, path,"timezone", URLEncoder.encode(timezone, HttpUtils.UTF_8))); + } + 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); @@ -323,6 +325,24 @@ public class SAMLUtils { resp.addHeader("SET-COOKIE", String.format("%s=%s;HttpOnly;Path=/client/api;%s", ApiConstants.SESSIONKEY, loginResponse.getSessionKey(), sameSite)); } + private static void addBaseCookies(final LoginCmdResponse loginResponse, final HttpServletResponse resp, String domain, String path) throws IOException { + resp.addCookie(newCookie(domain, path, "userid", URLEncoder.encode(loginResponse.getUserId(), HttpUtils.UTF_8))); + resp.addCookie(newCookie(domain, path,"domainid", URLEncoder.encode(loginResponse.getDomainId(), HttpUtils.UTF_8))); + resp.addCookie(newCookie(domain, path,"role", URLEncoder.encode(loginResponse.getType(), HttpUtils.UTF_8))); + resp.addCookie(newCookie(domain, path,"username", URLEncoder.encode(loginResponse.getUsername(), HttpUtils.UTF_8))); + resp.addCookie(newCookie(domain, path,"account", URLEncoder.encode(loginResponse.getAccount(), HttpUtils.UTF_8))); + resp.addCookie(newCookie(domain, path,"isSAML", URLEncoder.encode("true", HttpUtils.UTF_8))); + resp.addCookie(newCookie(domain, path,"twoFaEnabled", URLEncoder.encode(loginResponse.is2FAenabled(), HttpUtils.UTF_8))); + resp.addCookie(newCookie(domain, path,"userfullname", URLEncoder.encode(loginResponse.getFirstName() + " " + loginResponse.getLastName(), HttpUtils.UTF_8).replace("+", "%20"))); + } + + private static Cookie newCookie(final String domain, final String path, final String name, final String value) { + Cookie cookie = new Cookie(name, value); + cookie.setDomain(domain); + cookie.setPath(path); + return cookie; + } + /** * Returns base64 encoded PublicKey * @param key PublicKey diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java index 752845edb64..891d028aebf 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAMLUtilsTest.java @@ -58,7 +58,7 @@ public class SAMLUtilsTest extends TestCase { String idpUrl = "http://idp.domain.example"; String spId = "cloudstack"; String authnId = SAMLUtils.generateSecureRandomId(); - AuthnRequest req = SAMLUtils.buildAuthnRequestObject(authnId, spId, idpUrl, consumerUrl); + AuthnRequest req = SAMLUtils.buildAuthnRequestObject(authnId, spId, idpUrl, consumerUrl, true); assertEquals(req.getAssertionConsumerServiceURL(), consumerUrl); assertEquals(req.getDestination(), idpUrl); assertEquals(req.getIssuer().getValue(), spId); @@ -86,7 +86,7 @@ public class SAMLUtilsTest extends TestCase { idpMetadata.setSsoUrl(idpUrl); idpMetadata.setEntityId(idpId); - URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value())); + URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value(), true)); assertThat(redirectUrl).hasScheme(urlScheme).hasHost(idpDomain).hasParameter("SAMLRequest"); assertEquals(urlScheme, redirectUrl.getScheme()); assertEquals(idpDomain, redirectUrl.getHost()); @@ -115,7 +115,7 @@ public class SAMLUtilsTest extends TestCase { idpMetadata.setSsoUrl(idpUrl); idpMetadata.setEntityId(idpId); - URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value())); + URI redirectUrl = new URI(SAMLUtils.buildAuthnRequestUrl(authnId, spMetadata, idpMetadata, SAML2AuthManager.SAMLSignatureAlgorithm.value(), true)); assertThat(redirectUrl).hasScheme(urlScheme).hasHost(idpDomain).hasParameter("idpid").hasParameter("SAMLRequest"); assertEquals(urlScheme, redirectUrl.getScheme()); assertEquals(idpDomain, redirectUrl.getHost()); 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 729334d22ce..bfee28a7e3b 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 @@ -213,7 +213,6 @@ public class ListAndSwitchSAMLAccountCmdTest extends TestCase { loginCmdResponse.set2FAenabled("false"); Mockito.when(apiServer.loginUser(nullable(HttpSession.class), nullable(String.class), nullable(String.class), nullable(Long.class), nullable(String.class), nullable(InetAddress.class), nullable(Map.class))).thenReturn(loginCmdResponse); - Mockito.doNothing().when(resp).sendRedirect(nullable(String.class)); try { cmd.authenticate("command", params, session, null, HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp); } catch (ServerApiException exception) { @@ -221,7 +220,6 @@ public class ListAndSwitchSAMLAccountCmdTest extends TestCase { } finally { // accountService should have been called 4 times by now, for this case twice and 2 for cases above Mockito.verify(accountService, Mockito.times(4)).getUserAccountById(Mockito.anyLong()); - Mockito.verify(resp, Mockito.times(1)).sendRedirect(anyString()); } } diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index c78f8e68c2b..afa5f07c826 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -1159,7 +1159,14 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer domainId = userDomain.getId(); } - UserAccount userAcct = accountMgr.authenticateUser(username, password, domainId, loginIpAddress, requestParameters); + Long userId = (Long)session.getAttribute("nextUserId"); + UserAccount userAcct = null; + if (userId != null) { + userAcct = accountMgr.getUserAccountById(userId); + } else { + userAcct = accountMgr.authenticateUser(username, password, domainId, loginIpAddress, requestParameters); + } + if (userAcct != null) { final String timezone = userAcct.getTimezone(); float offsetInHrs = 0f; diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 8e06c576881..4e3a2e98564 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -372,6 +372,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M "totp", "The default user two factor authentication provider. Eg. totp, staticpin", true, ConfigKey.Scope.Domain); + static ConfigKey userAllowMultipleAccounts = new ConfigKey<>("Advanced", + Boolean.class, + "user.allow.multiple.accounts", + "false", + "Determines if the same username can be added to more than one account in the same domain (SAML-only).", + true, + ConfigKey.Scope.Domain); + protected AccountManagerImpl() { super(); } @@ -1252,8 +1260,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M // Check permissions checkAccess(getCurrentCallingAccount(), domain); - if (!_userAccountDao.validateUsernameInDomain(userName, domainId)) { - throw new InvalidParameterValueException("The user " + userName + " already exists in domain " + domainId); + if (!userAllowMultipleAccounts.valueInDomain(domainId) && !_userAccountDao.validateUsernameInDomain(userName, domainId)) { + throw new CloudRuntimeException("The user " + userName + " already exists in domain " + domainId); } if (networkDomain != null && networkDomain.length() > 0) { @@ -1436,9 +1444,16 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M throw new PermissionDeniedException("Account id : " + account.getId() + " is a system account, can't add a user to it"); } - if (!_userAccountDao.validateUsernameInDomain(userName, domainId)) { + if (!userAllowMultipleAccounts.valueInDomain(domainId) && !_userAccountDao.validateUsernameInDomain(userName, domainId)) { throw new CloudRuntimeException("The user " + userName + " already exists in domain " + domainId); } + + List duplicatedUsers = _userDao.findUsersByName(userName); + for (UserVO duplicatedUser : duplicatedUsers) { + // users can't exist in same account + assertUserNotAlreadyInAccount(duplicatedUser, account); + } + UserVO user = null; user = createUser(account.getId(), userName, password, firstName, lastName, email, timeZone, userUUID, source); return user; @@ -1564,7 +1579,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M *
  • The username must be unique in each domain. Therefore, if there is already another user with the same username, an {@link InvalidParameterValueException} is thrown. * */ - protected void validateAndUpdateUsernameIfNeeded(UpdateUserCmd updateUserCmd, UserVO user, Account account) { + protected void validateAndUpdateUsernameIfNeeded(UpdateUserCmd updateUserCmd, UserVO newUser, Account newAccount) { String userName = updateUserCmd.getUsername(); if (userName == null) { return; @@ -1572,18 +1587,21 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M if (StringUtils.isBlank(userName)) { throw new InvalidParameterValueException("Username cannot be empty."); } - List duplicatedUsers = _userDao.findUsersByName(userName); - for (UserVO duplicatedUser : duplicatedUsers) { - if (duplicatedUser.getId() == user.getId()) { + List existingUsers = _userDao.findUsersByName(userName); + for (UserVO existingUser : existingUsers) { + if (existingUser.getId() == newUser.getId()) { continue; } - Account duplicatedUserAccountWithUserThatHasTheSameUserName = _accountDao.findById(duplicatedUser.getAccountId()); - if (duplicatedUserAccountWithUserThatHasTheSameUserName.getDomainId() == account.getDomainId()) { - DomainVO domain = _domainDao.findById(duplicatedUserAccountWithUserThatHasTheSameUserName.getDomainId()); - throw new InvalidParameterValueException(String.format("Username [%s] already exists in domain [id=%s,name=%s]", duplicatedUser.getUsername(), domain.getUuid(), domain.getName())); + + // duplicate usernames cannot exist in same domain unless explicitly configured + if (!userAllowMultipleAccounts.valueInDomain(newAccount.getDomainId())) { + assertUserNotAlreadyInDomain(existingUser, newAccount); } + + // can't rename a username to an existing one in the same account + assertUserNotAlreadyInAccount(existingUser, newAccount); } - user.setUsername(userName); + newUser.setUsername(userName); } /** @@ -1820,7 +1838,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M // make sure the account is enabled too // if the user is either locked already or disabled already, don't change state...only lock currently enabled -// users + // users boolean success = true; if (user.getState().equals(State.LOCKED)) { // already locked...no-op @@ -3317,7 +3335,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] {UseSecretKeyInResponse, enableUserTwoFactorAuthentication, - userTwoFactorAuthenticationDefaultProvider, mandateUserTwoFactorAuthentication, userTwoFactorAuthenticationIssuer}; + userTwoFactorAuthenticationDefaultProvider, mandateUserTwoFactorAuthentication, userTwoFactorAuthenticationIssuer, + userAllowMultipleAccounts}; } public List getUserTwoFactorAuthenticationProviders() { @@ -3502,4 +3521,21 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M return userAccountVO; }); } + + void assertUserNotAlreadyInAccount(User existingUser, Account newAccount) { + System.out.println(existingUser.getAccountId()); + System.out.println(newAccount.getId()); + if (existingUser.getAccountId() == newAccount.getId()) { + AccountVO existingAccount = _accountDao.findById(newAccount.getId()); + throw new InvalidParameterValueException(String.format("Username [%s] already exists in account [id=%s,name=%s]", existingUser.getUsername(), existingAccount.getUuid(), existingAccount.getAccountName())); + } + } + + void assertUserNotAlreadyInDomain(User existingUser, Account originalAccount) { + Account existingAccount = _accountDao.findById(existingUser.getAccountId()); + if (existingAccount.getDomainId() == originalAccount.getDomainId()) { + DomainVO existingDomain = _domainDao.findById(existingAccount.getDomainId()); + throw new InvalidParameterValueException(String.format("Username [%s] already exists in domain [id=%s,name=%s] user account [id=%s,name=%s]", existingUser.getUsername(), existingDomain.getUuid(), existingDomain.getName(), existingAccount.getUuid(), existingAccount.getAccountName())); + } + } } diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index e68de194f01..f1cf0008676 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -1270,4 +1270,75 @@ public class AccountManagerImplTest extends AccountManagetImplTestBase { Assert.assertNull(updatedUser.getUser2faProvider()); Assert.assertNull(updatedUser.getKeyFor2fa()); } + + @Test(expected = InvalidParameterValueException.class) + public void testAssertUserNotAlreadyInAccount_UserExistsInAccount() { + User existingUser = new UserVO(); + existingUser.setUsername("testuser"); + existingUser.setAccountId(1L); + + Account newAccount = Mockito.mock(Account.class); + Mockito.when(newAccount.getId()).thenReturn(1L); + + AccountVO existingAccount = Mockito.mock(AccountVO.class); + Mockito.when(existingAccount.getUuid()).thenReturn("existing-account-uuid"); + Mockito.when(existingAccount.getAccountName()).thenReturn("existing-account"); + + Mockito.when(_accountDao.findById(1L)).thenReturn(existingAccount); + + accountManagerImpl.assertUserNotAlreadyInAccount(existingUser, newAccount); + } + + @Test + public void testAssertUserNotAlreadyInAccount_UserExistsInDiffAccount() { + User existingUser = new UserVO(); + existingUser.setUsername("testuser"); + existingUser.setAccountId(2L); + + Account newAccount = Mockito.mock(Account.class); + Mockito.when(newAccount.getId()).thenReturn(1L); + + accountManagerImpl.assertUserNotAlreadyInAccount(existingUser, newAccount); + } + + @Test(expected = InvalidParameterValueException.class) + public void testAssertUserNotAlreadyInDomain_UserExistsInDomain() { + User existingUser = new UserVO(); + existingUser.setUsername("testuser"); + existingUser.setAccountId(1L); + + Account originalAccount = Mockito.mock(Account.class); + Mockito.when(originalAccount.getDomainId()).thenReturn(1L); + + AccountVO existingAccount = Mockito.mock(AccountVO.class); + Mockito.when(existingAccount.getDomainId()).thenReturn(1L); + Mockito.when(existingAccount.getUuid()).thenReturn("existing-account-uuid"); + Mockito.when(existingAccount.getAccountName()).thenReturn("existing-account"); + + DomainVO existingDomain = Mockito.mock(DomainVO.class); + Mockito.when(existingDomain.getUuid()).thenReturn("existing-domain-uuid"); + Mockito.when(existingDomain.getName()).thenReturn("existing-domain"); + + Mockito.when(_accountDao.findById(1L)).thenReturn(existingAccount); + Mockito.when(_domainDao.findById(1L)).thenReturn(existingDomain); + + accountManagerImpl.assertUserNotAlreadyInDomain(existingUser, originalAccount); + } + + @Test + public void testAssertUserNotAlreadyInDomain_UserExistsInDiffDomain() { + User existingUser = new UserVO(); + existingUser.setUsername("testuser"); + existingUser.setAccountId(1L); + + Account originalAccount = Mockito.mock(Account.class); + Mockito.when(originalAccount.getDomainId()).thenReturn(1L); + + AccountVO existingAccount = Mockito.mock(AccountVO.class); + Mockito.when(existingAccount.getDomainId()).thenReturn(2L); + + Mockito.when(_accountDao.findById(1L)).thenReturn(existingAccount); + + accountManagerImpl.assertUserNotAlreadyInDomain(existingUser, originalAccount); + } } diff --git a/ui/src/components/header/SamlDomainSwitcher.vue b/ui/src/components/header/SamlDomainSwitcher.vue index 1d820dcbcff..082bab7bf13 100644 --- a/ui/src/components/header/SamlDomainSwitcher.vue +++ b/ui/src/components/header/SamlDomainSwitcher.vue @@ -88,6 +88,7 @@ export default { this.showSwitcher = false return } + this.samlAccounts = samlAccounts this.samlAccounts = _.orderBy(samlAccounts, ['domainPath'], ['asc']) const currentAccount = this.samlAccounts.filter(x => { return x.userId === store.getters.userInfo.id @@ -109,6 +110,8 @@ export default { this.$message.success(`Switched to "${account.accountName} (${account.domainPath})"`) this.$router.go() }) + }).else(error => { + console.log('error refreshing with new user context: ' + error) }) } } diff --git a/ui/src/store/modules/user.js b/ui/src/store/modules/user.js index 46a1905619f..c0a45259a53 100644 --- a/ui/src/store/modules/user.js +++ b/ui/src/store/modules/user.js @@ -290,7 +290,7 @@ const user = { commit('SET_CUSTOM_COLUMNS', cachedCustomColumns) // Ensuring we get the user info so that store.getters.user is never empty when the page is freshly loaded - api('listUsers', { username: Cookies.get('username'), listall: true }).then(response => { + api('listUsers', { id: Cookies.get('userid'), listall: true }).then(response => { const result = response.listusersresponse.user[0] commit('SET_INFO', result) commit('SET_NAME', result.firstname + ' ' + result.lastname) @@ -331,7 +331,7 @@ const user = { }) } - api('listUsers', { username: Cookies.get('username') }).then(response => { + api('listUsers', { id: Cookies.get('userid') }).then(response => { const result = response.listusersresponse.user[0] commit('SET_INFO', result) commit('SET_NAME', result.firstname + ' ' + result.lastname) diff --git a/ui/vue.config.js b/ui/vue.config.js index 9cae2ff66fb..a0e795531fb 100644 --- a/ui/vue.config.js +++ b/ui/vue.config.js @@ -143,7 +143,7 @@ const vueConfig = { ws: false, changeOrigin: true, proxyTimeout: 10 * 60 * 1000, // 10 minutes - cookieDomainRewrite: '*', + cookieDomainRewrite: process.env.CS_COOKIE_HOST || 'localhost', cookiePathRewrite: { '/client': '/' }