This is an automated email from the ASF dual-hosted git repository. dahn pushed a commit to branch 4.19 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.19 by this push: new f13cf597a2e 4.19 fix saml account selector (#10311) f13cf597a2e is described below commit f13cf597a2e00054033ab5851db628680a64f8dc Author: Rene Glover <rg9...@att.com> AuthorDate: Mon Apr 14 05:59:43 2025 -0500 4.19 fix saml account selector (#10311) --- .../api/command/ListAndSwitchSAMLAccountCmd.java | 15 +++- .../api/command/SAML2LoginAPIAuthenticatorCmd.java | 2 +- .../apache/cloudstack/saml/SAML2AuthManager.java | 4 + .../cloudstack/saml/SAML2AuthManagerImpl.java | 2 +- .../java/org/apache/cloudstack/saml/SAMLUtils.java | 88 +++++++++++++--------- .../java/org/apache/cloudstack/SAMLUtilsTest.java | 6 +- .../command/ListAndSwitchSAMLAccountCmdTest.java | 2 - server/src/main/java/com/cloud/api/ApiServer.java | 9 ++- .../java/com/cloud/user/AccountManagerImpl.java | 64 ++++++++++++---- .../com/cloud/user/AccountManagerImplTest.java | 71 +++++++++++++++++ ui/src/components/header/SamlDomainSwitcher.vue | 3 + ui/src/store/modules/user.js | 4 +- ui/vue.config.js | 2 +- 13 files changed, 211 insertions(+), 61 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 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<UserAccountVO> 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<SamlUserAccountResponse> response = new ListResponse<SamlUserAccountResponse>(); 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<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); + ConfigKey<Boolean> SAMLRequirePasswordLogin = new ConfigKey<Boolean>("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<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 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,28 +168,21 @@ 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 - AuthnContextClassRefBuilder authnContextClassRefBuilder = new AuthnContextClassRefBuilder(); - AuthnContextClassRef authnContextClassRef = authnContextClassRefBuilder.buildObject( - SAMLConstants.SAML20_NS, - "AuthnContextClassRef", "saml"); - authnContextClassRef.setAuthnContextClassRef(AuthnContext.PPT_AUTHN_CTX); - - // AuthnContext - RequestedAuthnContextBuilder requestedAuthnContextBuilder = new RequestedAuthnContextBuilder(); - RequestedAuthnContext requestedAuthnContext = requestedAuthnContextBuilder.buildObject(); - requestedAuthnContext.setComparison(AuthnContextComparisonTypeEnumeration.EXACT); - requestedAuthnContext.getAuthnContextClassRefs().add(authnContextClassRef); - // 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); @@ -200,11 +193,25 @@ public class SAMLUtils { authnRequest.setAssertionConsumerServiceURL(consumerUrl); authnRequest.setProviderName(spId); authnRequest.setIssuer(issuer); - authnRequest.setRequestedAuthnContext(requestedAuthnContext); return authnRequest; } + public static void setRequestedAuthnContext(AuthnRequest authnRequest, boolean requirePasswordAuthentication) { + AuthnContextClassRefBuilder authnContextClassRefBuilder = new AuthnContextClassRefBuilder(); + AuthnContextClassRef authnContextClassRef = authnContextClassRefBuilder.buildObject( + SAMLConstants.SAML20_NS, + "AuthnContextClassRef", "saml"); + authnContextClassRef.setAuthnContextClassRef(AuthnContext.PPT_AUTHN_CTX); + + // AuthnContext + RequestedAuthnContextBuilder requestedAuthnContextBuilder = new RequestedAuthnContextBuilder(); + RequestedAuthnContext requestedAuthnContext = requestedAuthnContextBuilder.buildObject(); + requestedAuthnContext.setComparison(AuthnContextComparisonTypeEnumeration.EXACT); + requestedAuthnContext.getAuthnContextClassRefs().add(authnContextClassRef); + authnRequest.setRequestedAuthnContext(requestedAuthnContext); + } + public static LogoutRequest buildLogoutRequest(String logoutUrl, String spId, String nameIdString) { Issuer issuer = new IssuerBuilder().buildObject(); issuer.setValue(spId); @@ -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<Boolean> 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<UserVO> 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 * <li> The username must be unique in each domain. Therefore, if there is already another user with the same username, an {@link InvalidParameterValueException} is thrown. * </ul> */ - 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<UserVO> duplicatedUsers = _userDao.findUsersByName(userName); - for (UserVO duplicatedUser : duplicatedUsers) { - if (duplicatedUser.getId() == user.getId()) { + List<UserVO> 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<UserTwoFactorAuthenticator> 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': '/' }