diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 472fe148a5d..f3b3b31bd71 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -5240,10 +5240,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac workJob = newVmWorkJobAndInfo.first(); VmWorkMigrateAway workInfo = new VmWorkMigrateAway(newVmWorkJobAndInfo.second(), srcHostId); - workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo)); + setCmdInfoAndSubmitAsyncJob(workJob, workInfo, vmId); } - _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vmId); AsyncJobExecutionContext.getCurrentExecutionContext().joinJob(workJob.getId()); diff --git a/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java b/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java index a1d9f4a8089..9d5e1b0ff50 100644 --- a/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java +++ b/engine/schema/src/main/java/com/cloud/vm/VMInstanceVO.java @@ -503,7 +503,7 @@ public class VMInstanceVO implements VirtualMachine, FiniteStateObject switchableAccounts = _userAccountDao.getAllUsersByNameAndEntity(currentUserAccount.getUsername(), currentUserAccount.getExternalEntity()); - if (switchableAccounts != null && switchableAccounts.size() > 0 && currentUserId != User.UID_SYSTEM) { - List accountResponses = new ArrayList(); + if (switchableAccounts != null && !switchableAccounts.isEmpty() && currentUserId != User.UID_SYSTEM) { + List accountResponses = new ArrayList<>(); for (UserAccountVO userAccount: switchableAccounts) { User user = _userDao.getUser(userAccount.getId()); Domain domain = _domainService.getDomain(userAccount.getDomainId()); @@ -176,8 +184,9 @@ public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthentic accountResponse.setAccountName(userAccount.getAccountName()); accountResponse.setIdpId(user.getExternalEntity()); accountResponses.add(accountResponse); + logger.debug("Returning available useraccount for [{}]: UserUUID: [{}], DomainUUID: [{}], Account: [{}]", currentUserAccount.getUsername(), user.getUuid(), domain.getUuid(), userAccount.getAccountName()); } - ListResponse response = new ListResponse(); + ListResponse response = new ListResponse<>(); response.setResponses(accountResponses); response.setResponseName(getCommandName()); return ApiResponseSerializer.toSerializedString(response, responseType); @@ -196,7 +205,7 @@ public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthentic @Override public void setAuthenticators(List authenticators) { for (PluggableAPIAuthenticator authManager: authenticators) { - if (authManager != null && authManager instanceof SAML2AuthManager) { + if (authManager instanceof SAML2AuthManager) { _samlAuthManager = (SAML2AuthManager) authManager; } } 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 742680fdcc5..bfd47922142 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 @@ -78,7 +78,7 @@ import com.cloud.user.UserAccountVO; import com.cloud.user.dao.UserAccountDao; import com.cloud.utils.db.EntityManager; -@APICommand(name = "samlSso", description = "SP initiated SAML Single Sign On", requestHasSensitiveInfo = true, responseObject = LoginCmdResponse.class, entityType = {}) +@APICommand(name = "samlSso", description = "SP initiated SAML Single Sign On", responseObject = LoginCmdResponse.class) public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator, Configurable { private static final String s_name = "loginresponse"; @@ -97,7 +97,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent @Inject private UserAccountDao userAccountDao; - protected static ConfigKey saml2FailedLoginRedirectUrl = new ConfigKey("Advanced", String.class, "saml2.failed.login.redirect.url", "", + protected static ConfigKey saml2FailedLoginRedirectUrl = new ConfigKey<>("Advanced", String.class, "saml2.failed.login.redirect.url", "", "The URL to redirect the SAML2 login failed message (the default vaulue is empty).", true); SAML2AuthManager samlAuthManager; @@ -190,7 +190,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent String authnId = SAMLUtils.generateSecureRandomId(); samlAuthManager.saveToken(authnId, domainPath, idpMetadata.getEntityId()); 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")) { @@ -207,7 +207,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent params, responseType)); } - String username = null; + String username; Issuer issuer = processedSAMLResponse.getIssuer(); SAMLProviderMetadata spMetadata = samlAuthManager.getSPMetadata(); SAMLProviderMetadata idpMetadata = samlAuthManager.getIdPMetadata(issuer.getValue()); @@ -273,7 +273,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent try { assertion = decrypter.decrypt(encryptedAssertion); } catch (DecryptionException e) { - logger.warn("SAML EncryptedAssertion error: " + e.toString()); + logger.warn("SAML EncryptedAssertion error: " + e); } if (assertion == null) { continue; @@ -310,7 +310,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent UserAccount userAccount = null; List possibleUserAccounts = userAccountDao.getAllUsersByNameAndEntity(username, issuer.getValue()); - if (possibleUserAccounts != null && possibleUserAccounts.size() > 0) { + if (possibleUserAccounts != null && !possibleUserAccounts.isEmpty()) { // Log into the first enabled user account // Users can switch to other allowed accounts later for (UserAccountVO possibleUserAccount : possibleUserAccounts) { @@ -370,7 +370,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent @Override public void setAuthenticators(List authenticators) { for (PluggableAPIAuthenticator authManager: authenticators) { - if (authManager != null && authManager instanceof SAML2AuthManager) { + if (authManager instanceof SAML2AuthManager) { samlAuthManager = (SAML2AuthManager) authManager; } } 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 545b01a4a31..93b7bc5be93 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 @@ -541,6 +541,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 364a43e93c0..1a9d677d43a 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 @@ -152,11 +152,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(); @@ -169,13 +169,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, @@ -187,23 +210,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) { @@ -285,23 +292,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; @@ -317,6 +307,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); LOGGER.debug("Adding sessionkey cookie to response: " + sessionKeyCookie); @@ -324,6 +326,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 9225574ff60..afc2f82b206 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 de875f67944..5e962cdb382 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -1186,7 +1186,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/ha/HighAvailabilityManagerImpl.java b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java index 2ce803756fe..1522402ae32 100644 --- a/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java +++ b/server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java @@ -384,11 +384,10 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements Configur } @Override - public boolean scheduleMigration(final VMInstanceVO vm, ReasonType reasonType) { + public boolean scheduleMigration(final VMInstanceVO vm, HighAvailabilityManager.ReasonType reasonType) { if (vm.getHostId() == null) { return false; } - if (!VmHaEnabled.valueIn(vm.getDataCenterId())) { String message = String.format("Unable to schedule migration for the VM %s on host %s, VM high availability manager is disabled.", vm, _hostDao.findById(vm.getHostId())); if (logger.isDebugEnabled()) { @@ -398,6 +397,7 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements Configur return false; } + Long hostId = VirtualMachine.State.Migrating.equals(vm.getState()) ? vm.getLastHostId() : vm.getHostId(); final HaWorkVO work = new HaWorkVO(vm.getId(), vm.getType(), WorkType.Migration, Step.Scheduled, vm.getHostId(), vm.getState(), 0, vm.getUpdated(), reasonType); _haDao.persist(work); logger.info("Scheduled migration work of VM {} from host {} with HAWork {}", vm, _hostDao.findById(vm.getHostId()), work); @@ -813,6 +813,18 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements Configur return null; } logger.info("Migration attempt: for VM {}from host {}. Starting attempt: {}/{} times.", vm, srcHost, 1 + work.getTimesTried(), _maxRetries); + + if (VirtualMachine.State.Stopped.equals(vm.getState())) { + logger.info(String.format("vm %s is Stopped, skipping migrate.", vm)); + return null; + } + if (VirtualMachine.State.Running.equals(vm.getState()) && srcHostId != vm.getHostId()) { + logger.info(String.format("VM %s is running on a different host %s, skipping migration", vm, vm.getHostId())); + return null; + } + logger.info("Migration attempt: for VM " + vm.getUuid() + "from host id " + srcHostId + + ". Starting attempt: " + (1 + work.getTimesTried()) + "/" + _maxRetries + " times."); + try { work.setStep(Step.Migrating); _haDao.update(work.getId(), work); @@ -1148,6 +1160,15 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements Configur @Override public void run() { logger.info("Starting work"); + try { + synchronized (this) { + wait(_timeToSleep); + } + } catch (final InterruptedException e) { + logger.info("Interrupted"); + } + logger.info("Starting work"); + while (!_stopped) { _managedContext.runWithContext(new Runnable() { @Override diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index faff178345a..4fbb501e8e4 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -1774,10 +1774,19 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C throwInvalidIdException("Network offering with specified id doesn't support adding multiple ip ranges", ntwkOff.getUuid(), NETWORK_OFFERING_ID); } - if (GuestType.Shared == ntwkOff.getGuestType() && !ntwkOff.isSpecifyVlan() && Objects.isNull(associatedNetworkId)) { - throw new CloudRuntimeException("Associated network must be provided when creating Shared networks when specifyVlan is false"); - } + + if (GuestType.Shared == ntwkOff.getGuestType()) { + if (!ntwkOff.isSpecifyIpRanges()) { + throw new CloudRuntimeException("The 'specifyipranges' parameter should be true for Shared Networks"); + } + if (ipv4 && Objects.isNull(startIP)) { + throw new CloudRuntimeException("IPv4 address range needs to be provided"); + } + if (ipv6 && Objects.isNull(startIPv6)) { + throw new CloudRuntimeException("IPv6 address range needs to be provided"); + } + } Pair interfaceMTUs = validateMtuConfig(publicMtu, privateMtu, zone.getId()); mtuCheckForVpcNetwork(vpcId, interfaceMTUs, publicMtu); diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index fbf70a8eaad..f7eb654141d 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -293,7 +293,7 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { } /** - * For each zone ID in {@link TemplateProfile#zoneIdList}, verifies if there is active heuristic rules for allocating template and returns the + * For each zone ID in {@link TemplateProfile#getZoneIdList()}, verifies if there is active heuristic rules for allocating template and returns the * {@link DataStore} returned by the heuristic rule. If there is not an active heuristic rule, then allocate it to a random {@link DataStore}, if the ISO/template is private * or allocate it to all {@link DataStore} in the zone, if it is public. * @param profile @@ -453,10 +453,10 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { /** * If the template/ISO is marked as private, then it is allocated to a random secondary storage; otherwise, allocates to every storage pool in every zone given by the - * {@link TemplateProfile#zoneIdList}. + * {@link TemplateProfile#getZoneIdList()}. */ private void postUploadAllocation(List imageStores, VMTemplateVO template, List payloads) { - Set zoneSet = new HashSet(); + Set zoneSet = new HashSet<>(); Collections.shuffle(imageStores); for (DataStore imageStore : imageStores) { Long zoneId_is = imageStore.getScope().getScopeId(); @@ -697,8 +697,8 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { } // delete all cache entries for this template - List cacheTmpls = imageFactory.listTemplateOnCache(template.getId()); - for (TemplateInfo tmplOnCache : cacheTmpls) { + List cachedTemplates = imageFactory.listTemplateOnCache(template.getId()); + for (TemplateInfo tmplOnCache : cachedTemplates) { logger.info("Delete template: {} from image cache store: {}", tmplOnCache, tmplOnCache.getDataStore()); tmplOnCache.delete(); } @@ -727,27 +727,32 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { } // remove its related ACL permission - Pair, Long> tmplt = new Pair, Long>(VirtualMachineTemplate.class, template.getId()); - _messageBus.publish(_name, EntityManager.MESSAGE_REMOVE_ENTITY_EVENT, PublishScope.LOCAL, tmplt); - - checkAndRemoveTemplateDetails(template); - - // Remove comments (if any) - AnnotationService.EntityType entityType = template.getFormat().equals(ImageFormat.ISO) ? - AnnotationService.EntityType.ISO : AnnotationService.EntityType.TEMPLATE; - annotationDao.removeByEntityType(entityType.name(), template.getUuid()); + Pair, Long> templateClassForId = new Pair<>(VirtualMachineTemplate.class, template.getId()); + _messageBus.publish(_name, EntityManager.MESSAGE_REMOVE_ENTITY_EVENT, PublishScope.LOCAL, templateClassForId); + List zoneRegistrations = templateZoneDao.listByTemplateId(template.getId()); + if (zoneRegistrations.isEmpty()) { + removeTemplateDetails(template); + removeTemplateAnnotations(template); + } } return success; } + private void removeTemplateAnnotations(VMTemplateVO template) { + // Remove comments (if any) + AnnotationService.EntityType entityType = template.getFormat().equals(ImageFormat.ISO) ? + AnnotationService.EntityType.ISO : AnnotationService.EntityType.TEMPLATE; + annotationDao.removeByEntityType(entityType.name(), template.getUuid()); + } + /** * removes details of the template and * if the template is registered as deploy as is, * then it also deletes the details related to deploy as is only if there are no VMs using the template * @param template */ - void checkAndRemoveTemplateDetails(VMTemplateVO template) { + private void removeTemplateDetails(VMTemplateVO template) { templateDetailsDao.removeDetails(template.getId()); if (template.isDeployAsIs()) { diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index a3ae3fff01f..4913b9a56b5 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -386,6 +386,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M 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(); } @@ -1289,7 +1297,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M // Check permissions checkAccess(getCurrentCallingAccount(), domain); - if (!_userAccountDao.validateUsernameInDomain(userName, domainId)) { + if (!userAllowMultipleAccounts.valueInDomain(domainId) && !_userAccountDao.validateUsernameInDomain(userName, domainId)) { throw new InvalidParameterValueException(String.format("The user %s already exists in domain %s", userName, domain)); } @@ -1477,9 +1485,15 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M throw new PermissionDeniedException(String.format("Account: %s is a system account, can't add a user to it", account)); } - if (!_userAccountDao.validateUsernameInDomain(userName, domainId)) { - throw new CloudRuntimeException(String.format("The user %s already exists in domain %s", userName, domain)); + 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; user = createUser(account.getId(), userName, password, firstName, lastName, email, timeZone, userUUID, source); return user; @@ -1607,7 +1621,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; @@ -1615,18 +1629,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 (%s)", duplicatedUser, domain)); + + // 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); } /** @@ -1895,7 +1912,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; if (user.getState().equals(State.LOCKED)) { // already locked...no-op @@ -3408,7 +3425,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] {UseSecretKeyInResponse, enableUserTwoFactorAuthentication, - userTwoFactorAuthenticationDefaultProvider, mandateUserTwoFactorAuthentication, userTwoFactorAuthenticationIssuer, apiKeyAccess}; + userTwoFactorAuthenticationDefaultProvider, mandateUserTwoFactorAuthentication, userTwoFactorAuthenticationIssuer, apiKeyAccess, + userAllowMultipleAccounts}; } public List getUserTwoFactorAuthenticationProviders() { @@ -3593,4 +3611,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 61a273c8946..eb401efa341 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -1406,4 +1406,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/components/view/InfoCard.vue b/ui/src/components/view/InfoCard.vue index d8b37e3c6e4..2f8e54a2010 100644 --- a/ui/src/components/view/InfoCard.vue +++ b/ui/src/components/view/InfoCard.vue @@ -646,7 +646,7 @@ {{ resource.podname || resource.pod || resource.podid }} -
    +
    {{ $t('label.zone') }}
    @@ -733,7 +733,7 @@ {{ resource.managementserver || resource.managementserverid }}
    -
    +
    {{ $t('label.created') }}
    {{ $toLocaleDate(resource.created) }} diff --git a/ui/src/store/modules/user.js b/ui/src/store/modules/user.js index bd0afaa17bf..6113f3ed940 100644 --- a/ui/src/store/modules/user.js +++ b/ui/src/store/modules/user.js @@ -313,7 +313,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) @@ -386,7 +386,7 @@ const user = { }).catch(ignored => {}) } - 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/src/views/image/IsoZones.vue b/ui/src/views/image/IsoZones.vue index daf1e7e21e0..75eac8fd97f 100644 --- a/ui/src/views/image/IsoZones.vue +++ b/ui/src/views/image/IsoZones.vue @@ -48,6 +48,9 @@ {{ $t('label.yes') }} {{ $t('label.no') }} + +