From 552f2ae60ca97dc2410ca79d11d3074f5b191081 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Mon, 2 Feb 2015 19:56:25 +0530 Subject: [PATCH] CLOUDSTACK-8191: SAML users should have their own accounts (cherry picked from commit 876c78fe1ba6abe132131b3449b21fd09f2c14e1) Signed-off-by: Rohit Yadav --- .../classes/resources/messages.properties | 1 + .../resources/messages_de_DE.properties | 1 + .../classes/resources/messages_es.properties | 1 + .../resources/messages_fr_FR.properties | 1 + .../resources/messages_ja_JP.properties | 3 +- .../resources/messages_ko_KR.properties | 1 + .../resources/messages_nl_NL.properties | 1 + .../classes/resources/messages_pl.properties | 1 + .../resources/messages_pt_BR.properties | 1 + .../resources/messages_ru_RU.properties | 1 + .../resources/messages_zh_CN.properties | 3 +- .../SAML2LoginAPIAuthenticatorCmd.java | 42 +++++++++++-------- .../SAML2LoginAPIAuthenticatorCmdTest.java | 28 ++++++------- .../src/com/cloud/configuration/Config.java | 8 ---- .../com/cloud/user/AccountManagerImpl.java | 2 +- ui/css/cloudstack3.css | 24 ++++++++++- ui/dictionary.jsp | 1 + ui/index.jsp | 1 + ui/scripts/cloudStack.js | 10 +++++ ui/scripts/ui-custom/login.js | 6 +++ 20 files changed, 92 insertions(+), 45 deletions(-) diff --git a/client/WEB-INF/classes/resources/messages.properties b/client/WEB-INF/classes/resources/messages.properties index 990bb317da1..6d47ac00406 100644 --- a/client/WEB-INF/classes/resources/messages.properties +++ b/client/WEB-INF/classes/resources/messages.properties @@ -758,6 +758,7 @@ label.local.storage=Local Storage label.local=Local label.login=Login label.logout=Logout +label.saml.login=SAML Login label.LUN.number=LUN \# label.lun=LUN label.make.project.owner=Make account project owner diff --git a/client/WEB-INF/classes/resources/messages_de_DE.properties b/client/WEB-INF/classes/resources/messages_de_DE.properties index 616cdf9048f..bd360882ef7 100644 --- a/client/WEB-INF/classes/resources/messages_de_DE.properties +++ b/client/WEB-INF/classes/resources/messages_de_DE.properties @@ -572,6 +572,7 @@ label.local=Lokal label.local.storage=Lokaler Speicher label.login=Login label.logout=Abmelden +label.saml.login=SAML Login label.lun=LUN label.LUN.number=LUN \# label.management=Verwaltung diff --git a/client/WEB-INF/classes/resources/messages_es.properties b/client/WEB-INF/classes/resources/messages_es.properties index 6560a2a5513..6b51c38bf38 100644 --- a/client/WEB-INF/classes/resources/messages_es.properties +++ b/client/WEB-INF/classes/resources/messages_es.properties @@ -555,6 +555,7 @@ label.local=local label.local.storage=Almacenamiento Local label.login=Login label.logout=Cerrar sesi\u00c3\u00b3n +label.saml.login=SAML Login label.lun=LUN label.LUN.number=LUN \# label.manage=Administrar diff --git a/client/WEB-INF/classes/resources/messages_fr_FR.properties b/client/WEB-INF/classes/resources/messages_fr_FR.properties index 54dc6215a83..1272442acdb 100644 --- a/client/WEB-INF/classes/resources/messages_fr_FR.properties +++ b/client/WEB-INF/classes/resources/messages_fr_FR.properties @@ -878,6 +878,7 @@ label.local.storage.enabled=Stockage local activ\u00e9 label.local.storage=Stockage local label.login=Connexion label.logout=D\u00e9connexion +label.saml.login=SAML Connexion label.lun=LUN label.LUN.number=N\u00b0 LUN label.lxc.traffic.label=Libell\u00e9 trafic LXC diff --git a/client/WEB-INF/classes/resources/messages_ja_JP.properties b/client/WEB-INF/classes/resources/messages_ja_JP.properties index f9949fe4958..0510f43d568 100644 --- a/client/WEB-INF/classes/resources/messages_ja_JP.properties +++ b/client/WEB-INF/classes/resources/messages_ja_JP.properties @@ -755,6 +755,7 @@ label.local.storage=\u30ed\u30fc\u30ab\u30eb \u30b9\u30c8\u30ec\u30fc\u30b8 label.local=\u30ed\u30fc\u30ab\u30eb label.login=\u30ed\u30b0\u30aa\u30f3 label.logout=\u30ed\u30b0\u30aa\u30d5 +label.saml.login=SAML \u30ed\u30b0\u30aa\u30f3 label.LUN.number=LUN \u756a\u53f7 label.lun=LUN label.make.project.owner=\u30a2\u30ab\u30a6\u30f3\u30c8\u306e\u30d7\u30ed\u30b8\u30a7\u30af\u30c8\u6240\u6709\u8005\u3078\u306e\u5909\u66f4 @@ -2085,4 +2086,4 @@ label.timezone.colon=\u30bf\u30a4\u30e0\u30be\u30fc\u30f3\: label.keep.colon=\u4fdd\u6301\u6570\: label.every=\u6bce\u9031 label.day=\u6bce\u6708 -label.of.month=\u65e5 \ No newline at end of file +label.of.month=\u65e5 diff --git a/client/WEB-INF/classes/resources/messages_ko_KR.properties b/client/WEB-INF/classes/resources/messages_ko_KR.properties index 42ffe6c9760..badb7f3b737 100644 --- a/client/WEB-INF/classes/resources/messages_ko_KR.properties +++ b/client/WEB-INF/classes/resources/messages_ko_KR.properties @@ -651,6 +651,7 @@ label.local.storage=\ub85c\uceec \uc2a4\ud1a0\ub9ac\uc9c0 label.local=\ub85c\uceec label.login=\ub85c\uadf8\uc778 label.logout=\ub85c\uadf8\uc544\uc6c3 +label.saml.login=SAML \ub85c\uadf8\uc778 label.lun=LUN label.LUN.number=LUN \ubc88\ud638 label.make.project.owner=\uacc4\uc815 \uc815\ubcf4 \ud504\ub85c\uc81d\ud2b8 \uc18c\uc720\uc790 diff --git a/client/WEB-INF/classes/resources/messages_nl_NL.properties b/client/WEB-INF/classes/resources/messages_nl_NL.properties index 53c1dc4a1fc..35cd86c92b9 100644 --- a/client/WEB-INF/classes/resources/messages_nl_NL.properties +++ b/client/WEB-INF/classes/resources/messages_nl_NL.properties @@ -826,6 +826,7 @@ label.local.storage.enabled=Lokale opslag ingeschakeld label.local.storage=Lokale Opslag label.login=Login label.logout=Log uit +label.saml.login=SAML Login label.lun=LUN label.LUN.number=LUN \# label.lxc.traffic.label=LXC verkeerslabel diff --git a/client/WEB-INF/classes/resources/messages_pl.properties b/client/WEB-INF/classes/resources/messages_pl.properties index a0d1b354b4d..0388b3ff7e3 100644 --- a/client/WEB-INF/classes/resources/messages_pl.properties +++ b/client/WEB-INF/classes/resources/messages_pl.properties @@ -292,6 +292,7 @@ label.local.storage.enabled=Pami\u0119\u0107 lokalna w\u0142\u0105czona label.local.storage=Pami\u0119\u0107 lokalna label.login=Zaloguj label.logout=Wyloguj +label.saml.login=SAML Zaloguj label.lun=LUN label.LUN.number=LUN \# label.max.guest.limit=Maksymalna liczba go\u015bci diff --git a/client/WEB-INF/classes/resources/messages_pt_BR.properties b/client/WEB-INF/classes/resources/messages_pt_BR.properties index d434fef46ad..c925e6d090b 100644 --- a/client/WEB-INF/classes/resources/messages_pt_BR.properties +++ b/client/WEB-INF/classes/resources/messages_pt_BR.properties @@ -728,6 +728,7 @@ label.local.storage.enabled=Storage local habilitada label.local.storage=Storage Local label.login=Entrar label.logout=Sair +label.saml.login=SAML Entrar label.lun=LUN label.LUN.number=LUN \# label.make.project.owner=Criar propriet\u00e1rio de conta de projeto diff --git a/client/WEB-INF/classes/resources/messages_ru_RU.properties b/client/WEB-INF/classes/resources/messages_ru_RU.properties index 22b6c075adb..16427a1e9b0 100644 --- a/client/WEB-INF/classes/resources/messages_ru_RU.properties +++ b/client/WEB-INF/classes/resources/messages_ru_RU.properties @@ -689,6 +689,7 @@ label.local.storage=\u041b\u043e\u043a\u0430\u043b\u044c\u043d\u043e\u0435 \u044 label.local=\u041b\u043e\u043a\u0430\u043b\u044c\u043d\u044b\u0439 label.login=\u0412\u0445\u043e\u0434 label.logout=\u0412\u044b\u0445\u043e\u0434 +label.saml.login=SAML \u0412\u0445\u043e\u0434 label.lun=LUN label.LUN.number=LUN \# label.make.project.owner=\u0421\u0434\u0435\u043b\u0430\u0442\u044c \u0430\u043a\u043a\u0430\u0443\u043d\u0442 \u0432\u043b\u0430\u0434\u0435\u043b\u044c\u0446\u0435\u043c \u043f\u0440\u043e\u0435\u043a\u0442\u0430 diff --git a/client/WEB-INF/classes/resources/messages_zh_CN.properties b/client/WEB-INF/classes/resources/messages_zh_CN.properties index d02a124d1cc..a527fcbfceb 100644 --- a/client/WEB-INF/classes/resources/messages_zh_CN.properties +++ b/client/WEB-INF/classes/resources/messages_zh_CN.properties @@ -755,6 +755,7 @@ label.local.storage=\u672c\u5730\u5b58\u50a8 label.local=\u672c\u5730 label.login=\u767b\u5f55 label.logout=\u6ce8\u9500 +label.saml.login=SAML \u767b\u5f55 label.LUN.number=LUN \u53f7 label.lun=LUN label.make.project.owner=\u8bbe\u4e3a\u5e10\u6237\u9879\u76ee\u6240\u6709\u8005 @@ -2085,4 +2086,4 @@ label.timezone.colon=\u65f6\u533a\: label.keep.colon=\u4fdd\u7559\u6570\u91cf\: label.every=\u6bcf label.day=\u6bcf\u6708 -label.of.month=\u65e5 \ No newline at end of file +label.of.month=\u65e5 diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java index 6e86d23b42f..1b4cd6aaeed 100644 --- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java @@ -23,7 +23,8 @@ import com.cloud.domain.Domain; import com.cloud.exception.CloudAuthenticationException; import com.cloud.user.Account; import com.cloud.user.DomainManager; -import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.user.dao.UserAccountDao; import com.cloud.utils.HttpUtils; import com.cloud.utils.db.EntityManager; import org.apache.cloudstack.api.APICommand; @@ -73,6 +74,7 @@ import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.util.List; import java.util.Map; +import java.util.UUID; @APICommand(name = "samlSso", description = "SP initiated SAML Single Sign On", requestHasSensitiveInfo = true, responseObject = LoginCmdResponse.class, entityType = {}) public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { @@ -93,6 +95,8 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent ConfigurationDao _configDao; @Inject DomainManager _domainMgr; + @Inject + private UserAccountDao _userAccountDao; SAML2AuthManager _samlAuthManager; @@ -201,11 +205,9 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent } } - String uniqueUserId = null; - String accountName = _configDao.getValue(Config.SAMLUserAccountName.key()); String domainString = _configDao.getValue(Config.SAMLUserDomain.key()); - Long domainId = -1L; + Long domainId = null; Domain domain = _domainMgr.getDomain(domainString); if (domain != null) { domainId = domain.getId(); @@ -215,16 +217,17 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent } catch (NumberFormatException ignore) { } } - if (domainId == -1L) { - s_logger.error("The default domain ID for SAML users is not set correct, it should be a UUID"); + if (domainId == null) { + s_logger.error("The default domain ID for SAML users is not set correct, it should be a UUID. ROOT domain will be used."); } String username = null; String password = SAMLUtils.generateSecureRandomId(); // Random password String firstName = ""; String lastName = ""; - String timeZone = ""; + String timeZone = "GMT"; String email = ""; + short accountType = 0; // User account Assertion assertion = processedSAMLResponse.getAssertions().get(0); NameID nameId = assertion.getSubject().getNameID(); @@ -234,7 +237,6 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent if (nameId.getFormat().equals(NameIDType.PERSISTENT) || nameId.getFormat().equals(NameIDType.EMAIL)) { username = nameId.getValue(); - uniqueUserId = SAMLUtils.createSAMLId(username); if (nameId.getFormat().equals(NameIDType.EMAIL)) { email = username; } @@ -250,9 +252,8 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent for (Attribute attribute: attributeStatement.getAttributes()) { String attributeName = attribute.getName(); String attributeValue = attribute.getAttributeValues().get(0).getDOM().getTextContent(); - if (attributeName.equalsIgnoreCase("uid") && uniqueUserId == null) { + if (attributeName.equalsIgnoreCase("uid") && username == null) { username = attributeValue; - uniqueUserId = SAMLUtils.createSAMLId(username); } else if (attributeName.equalsIgnoreCase("givenName")) { firstName = attributeValue; } else if (attributeName.equalsIgnoreCase(("sn"))) { @@ -264,17 +265,22 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent } } - User user = _entityMgr.findByUuid(User.class, uniqueUserId); - if (user == null && uniqueUserId != null && username != null - && accountName != null && domainId != null) { - CallContext.current().setEventDetails("UserName: " + username + ", FirstName :" + password + ", LastName: " + lastName); - user = _accountService.createUser(username, password, firstName, lastName, email, timeZone, accountName, domainId, uniqueUserId); + if (username == null && email != null) { + username = email; + } + final String uniqueUserId = SAMLUtils.createSAMLId(username); + + UserAccount userAccount = _userAccountDao.getUserAccount(username, domainId); + if (userAccount == null && uniqueUserId != null && username != null) { + CallContext.current().setEventDetails("SAML Account/User with UserName: " + username + ", FirstName :" + password + ", LastName: " + lastName); + _accountService.createUserAccount(username, password, firstName, lastName, email, timeZone, + username, (short) accountType, domainId, null, null, UUID.randomUUID().toString(), uniqueUserId); } - if (user != null) { + if (userAccount != null) { try { - if (_apiServer.verifyUser(user.getId())) { - LoginCmdResponse loginResponse = (LoginCmdResponse) _apiServer.loginUser(session, username, user.getPassword(), domainId, null, remoteAddress, params); + if (_apiServer.verifyUser(userAccount.getId())) { + LoginCmdResponse loginResponse = (LoginCmdResponse) _apiServer.loginUser(session, username, userAccount.getPassword(), domainId, null, remoteAddress, params); 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))); diff --git a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java index f14b02c1028..321b6fa90df 100644 --- a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java +++ b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java @@ -22,10 +22,9 @@ package org.apache.cloudstack.api.command; import com.cloud.domain.Domain; import com.cloud.user.AccountService; import com.cloud.user.DomainManager; -import com.cloud.user.User; -import com.cloud.user.UserVO; +import com.cloud.user.UserAccountVO; +import com.cloud.user.dao.UserAccountDao; import com.cloud.utils.HttpUtils; -import com.cloud.utils.db.EntityManager; import org.apache.cloudstack.api.ApiServerService; import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.ServerApiException; @@ -79,15 +78,15 @@ public class SAML2LoginAPIAuthenticatorCmdTest { @Mock ConfigurationDao configDao; - @Mock - EntityManager entityMgr; - @Mock DomainManager domainMgr; @Mock AccountService accountService; + @Mock + UserAccountDao userAccountDao; + @Mock Domain domain; @@ -139,10 +138,6 @@ public class SAML2LoginAPIAuthenticatorCmdTest { accountServiceField.setAccessible(true); accountServiceField.set(cmd, accountService); - Field entityMgrField = SAML2LoginAPIAuthenticatorCmd.class.getDeclaredField("_entityMgr"); - entityMgrField.setAccessible(true); - entityMgrField.set(cmd, entityMgr); - Field domainMgrField = SAML2LoginAPIAuthenticatorCmd.class.getDeclaredField("_domainMgr"); domainMgrField.setAccessible(true); domainMgrField.set(cmd, domainMgr); @@ -151,6 +146,10 @@ public class SAML2LoginAPIAuthenticatorCmdTest { configDaoField.setAccessible(true); configDaoField.set(cmd, configDao); + Field userAccountDaoField = SAML2LoginAPIAuthenticatorCmd.class.getDeclaredField("_userAccountDao"); + userAccountDaoField.setAccessible(true); + userAccountDaoField.set(cmd, userAccountDao); + String spId = "someSPID"; String url = "someUrl"; X509Certificate cert = SAMLUtils.generateRandomX509Certificate(SAMLUtils.generateRandomKeyPair()); @@ -164,9 +163,10 @@ public class SAML2LoginAPIAuthenticatorCmdTest { Mockito.when(domain.getId()).thenReturn(1L); Mockito.when(domainMgr.getDomain(Mockito.anyString())).thenReturn(domain); - UserVO user = new UserVO(); - user.setUuid(SAMLUtils.createSAMLId("someUID")); - Mockito.when(entityMgr.findByUuid(Mockito.eq(User.class), Mockito.anyString())).thenReturn((User) user); + UserAccountVO user = new UserAccountVO(); + user.setUsername(SAMLUtils.createSAMLId("someUID")); + user.setId(1000L); + Mockito.when(userAccountDao.getUserAccount(Mockito.anyString(), Mockito.anyLong())).thenReturn(user); Mockito.when(apiServer.verifyUser(Mockito.anyLong())).thenReturn(false); Map params = new HashMap(); @@ -184,7 +184,7 @@ public class SAML2LoginAPIAuthenticatorCmdTest { } Mockito.verify(configDao, Mockito.atLeastOnce()).getValue(Mockito.anyString()); Mockito.verify(domainMgr, Mockito.times(1)).getDomain(Mockito.anyString()); - Mockito.verify(entityMgr, Mockito.times(1)).findByUuid(Mockito.eq(User.class), Mockito.anyString()); + Mockito.verify(userAccountDao, Mockito.times(1)).getUserAccount(Mockito.anyString(), Mockito.anyLong()); Mockito.verify(apiServer, Mockito.times(1)).verifyUser(Mockito.anyLong()); } diff --git a/server/src/com/cloud/configuration/Config.java b/server/src/com/cloud/configuration/Config.java index 453a3bc6a1a..dd9bfc003b2 100644 --- a/server/src/com/cloud/configuration/Config.java +++ b/server/src/com/cloud/configuration/Config.java @@ -1379,14 +1379,6 @@ public enum Config { "false", "Set it to true to enable SAML SSO plugin", null), - SAMLUserAccountName( - "Advanced", - ManagementServer.class, - String.class, - "saml2.default.accountname", - "admin", - "The name of the default account to use when creating users from SAML SSO", - null), SAMLUserDomain( "Advanced", ManagementServer.class, diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index a681c902ece..28115a2f8b3 100644 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -1017,7 +1017,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M throw new InvalidParameterValueException("The user " + userName + " already exists in domain " + domainId); } - if (networkDomain != null) { + if (networkDomain != null && networkDomain.length() > 0) { if (!NetUtils.verifyDomainName(networkDomain)) { throw new InvalidParameterValueException( "Invalid network domain. Total length shouldn't exceed 190 chars. Each domain label must be between 1 and 63 characters long, can contain ASCII letters 'a' through 'z', the digits '0' through '9', " diff --git a/ui/css/cloudstack3.css b/ui/css/cloudstack3.css index 15e5d29d7dc..54facdf70cc 100644 --- a/ui/css/cloudstack3.css +++ b/ui/css/cloudstack3.css @@ -440,7 +440,7 @@ body.login { background: transparent url(../images/sprites.png) -563px -747px; cursor: pointer; border: none; - margin: 7px 238px 0 -1px; + margin: 7px 120px 0 -1px; text-align: center; width: 69px; height: 25px; @@ -456,6 +456,27 @@ body.login { text-shadow: 0px 1px 2px #000000; } +.login .fields input[type=samlsubmit] { + background: transparent url(../images/sprites.png) -563px -747px; + cursor: pointer; + border: none; + margin: 7px 120px 0 -1px; + text-align: center; + width: 60px; + height: 15px; + display: block; + color: #FFFFFF; + font-weight: bold; + float: left; + text-indent: -1px; + /*+text-shadow:0px 1px 2px #000000;*/ + -moz-text-shadow: 0px 1px 2px #000000; + -webkit-text-shadow: 0px 1px 2px #000000; + -o-text-shadow: 0px 1px 2px #000000; + text-shadow: 0px 1px 2px #000000; + font-size: 10px; +} + .login .fields input[type=submit]:hover { background-position: -563px -772px; } @@ -13035,4 +13056,3 @@ div.gpugroups div.list-view { background: transparent url("../images/icons.png") no-repeat -626px -209px; padding: 0 0 3px 18px; } - diff --git a/ui/dictionary.jsp b/ui/dictionary.jsp index 4923b81fa26..e283d49dac7 100644 --- a/ui/dictionary.jsp +++ b/ui/dictionary.jsp @@ -762,6 +762,7 @@ dictionary = { 'label.local.storage': '', 'label.login': '', 'label.logout': '', +'label.saml.login': '', 'label.lun': '', 'label.LUN.number': '', 'label.make.project.owner': '', diff --git a/ui/index.jsp b/ui/index.jsp index 68abdf2c774..98dbb272bca 100644 --- a/ui/index.jsp +++ b/ui/index.jsp @@ -67,6 +67,7 @@ " /> + " />