AMBARI-20771. BE: Characters used in usernames should be constrained (Attila Magyar via adoroszlai)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/e8fd10cf Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/e8fd10cf Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/e8fd10cf Branch: refs/heads/branch-feature-AMBARI-12556 Commit: e8fd10cf6de7cc20662ab7d609ac8e105326ab6a Parents: 7f4f421 Author: Attila Magyar <amag...@hortonworks.com> Authored: Mon Apr 24 09:51:19 2017 +0200 Committer: Attila Doroszlai <adorosz...@hortonworks.com> Committed: Mon Apr 24 09:51:19 2017 +0200 ---------------------------------------------------------------------- .../ambari/server/orm/entities/UserEntity.java | 5 +- .../server/security/authorization/UserName.java | 76 ++++++++++++++++++++ .../server/security/authorization/Users.java | 4 +- .../apache/ambari/server/orm/OrmTestHelper.java | 5 +- .../ambari/server/orm/dao/UserDAOTest.java | 3 +- .../server/security/SecurityHelperImplTest.java | 3 +- ...ariAuthorizationProviderDisableUserTest.java | 2 +- .../AmbariLocalUserProviderTest.java | 2 +- .../AmbariUserAuthenticationFilterTest.java | 2 +- .../security/authorization/UserNameTest.java | 70 ++++++++++++++++++ .../security/authorization/UsersTest.java | 2 +- .../ldap/AmbariLdapDataPopulatorTest.java | 3 +- .../server/upgrade/UpgradeCatalog240Test.java | 3 +- 13 files changed, 166 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java index 576ca97..432191e 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UserEntity.java @@ -42,6 +42,7 @@ import javax.persistence.Temporal; import javax.persistence.TemporalType; import javax.persistence.UniqueConstraint; +import org.apache.ambari.server.security.authorization.UserName; import org.apache.ambari.server.security.authorization.UserType; @Table(name = "users", uniqueConstraints = {@UniqueConstraint(columnNames = {"user_name", "user_type"})}) @@ -117,8 +118,8 @@ public class UserEntity { return userName; } - public void setUserName(String userName) { - this.userName = userName; + public void setUserName(UserName userName) { + this.userName = userName.toString(); } public Boolean getLdapUser() { http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/UserName.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/UserName.java b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/UserName.java new file mode 100644 index 0000000..605183c --- /dev/null +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/UserName.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ambari.server.security.authorization; + +import java.util.Arrays; + +import org.apache.commons.lang.StringUtils; + +/** + * Represents an Ambari user name + */ +public class UserName { + private static final char[] FORBIDDEN_CHARS = {'<', '>', '&', '|', '\\', '`'}; + private final String userName; + + /** + * Creates a UserName from the given string + */ + public static UserName fromString(String userName) { + return new UserName(validated(userName)); + } + + private static String validated(String userName) { + if (StringUtils.isBlank(userName)) { + throw new IllegalArgumentException("Username cannot be empty"); + } + rejectIfContainsAnyOf(userName, FORBIDDEN_CHARS); + return userName; + } + + private static void rejectIfContainsAnyOf(String name, char[] forbiddenChars) { + for (char each : forbiddenChars) { + if (name.contains(Character.toString(each))) { + throw new IllegalArgumentException("Invalid username: " + name + " Avoid characters " + Arrays.toString(forbiddenChars)); + } + } + } + + private UserName(String userName) { + this.userName = userName; + } + + @Override + public String toString() { + return userName; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + UserName userName1 = (UserName) o; + return userName.equals(userName1.userName); + } + + @Override + public int hashCode() { + return userName.hashCode(); + } +} http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java index 4ed777b..9cdde8f 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java @@ -321,7 +321,7 @@ public class Users { principalDAO.create(principalEntity); UserEntity userEntity = new UserEntity(); - userEntity.setUserName(userName); + userEntity.setUserName(UserName.fromString(userName)); if (userType == UserType.LOCAL) { //passwords should be stored for local users only userEntity.setUserPassword(passwordEncoder.encode(password)); @@ -709,7 +709,7 @@ public class Users { principalsToCreate.add(principalEntity); final UserEntity userEntity = new UserEntity(); - userEntity.setUserName(userName); + userEntity.setUserName(UserName.fromString(userName)); userEntity.setUserPassword(""); userEntity.setPrincipal(principalEntity); userEntity.setLdapUser(true); http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java index fdc19d1..574ffa4 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/OrmTestHelper.java @@ -75,6 +75,7 @@ import org.apache.ambari.server.orm.entities.StackEntity; import org.apache.ambari.server.orm.entities.StageEntity; import org.apache.ambari.server.orm.entities.UserEntity; import org.apache.ambari.server.security.authorization.ResourceType; +import org.apache.ambari.server.security.authorization.UserName; import org.apache.ambari.server.state.Cluster; import org.apache.ambari.server.state.Clusters; import org.apache.ambari.server.state.Host; @@ -227,7 +228,7 @@ public class OrmTestHelper { PasswordEncoder encoder = injector.getInstance(PasswordEncoder.class); UserEntity admin = new UserEntity(); - admin.setUserName("administrator"); + admin.setUserName(UserName.fromString("administrator")); admin.setUserPassword(encoder.encode("admin")); admin.setPrincipal(principalEntity); @@ -242,7 +243,7 @@ public class OrmTestHelper { getEntityManager().persist(principalEntity); UserEntity userWithoutRoles = new UserEntity(); - userWithoutRoles.setUserName("userWithoutRoles"); + userWithoutRoles.setUserName(UserName.fromString("userWithoutRoles")); userWithoutRoles.setUserPassword(encoder.encode("test")); userWithoutRoles.setPrincipal(principalEntity); userDAO.create(userWithoutRoles); http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java b/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java index bb0b0cf..800bef1 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UserDAOTest.java @@ -34,6 +34,7 @@ import javax.persistence.TypedQuery; import org.apache.ambari.server.orm.DBAccessor; import org.apache.ambari.server.orm.entities.UserEntity; +import org.apache.ambari.server.security.authorization.UserName; import org.apache.ambari.server.security.authorization.UserType; import org.junit.Test; @@ -114,7 +115,7 @@ public class UserDAOTest { private static final UserEntity user(String name, UserType type) { UserEntity userEntity = new UserEntity(); - userEntity.setUserName(name); + userEntity.setUserName(UserName.fromString(name)); userEntity.setUserType(type); return userEntity; } http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/security/SecurityHelperImplTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/SecurityHelperImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/SecurityHelperImplTest.java index d69d49a..43d2ed2 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/SecurityHelperImplTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/SecurityHelperImplTest.java @@ -24,6 +24,7 @@ import org.apache.ambari.server.orm.entities.PrincipalEntity; import org.apache.ambari.server.orm.entities.UserEntity; import org.apache.ambari.server.security.authorization.AmbariUserAuthentication; import org.apache.ambari.server.security.authorization.User; +import org.apache.ambari.server.security.authorization.UserName; import org.junit.Assert; import org.junit.Test; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; @@ -43,7 +44,7 @@ public class SecurityHelperImplTest { SecurityContext ctx = SecurityContextHolder.getContext(); UserEntity userEntity = new UserEntity(); userEntity.setPrincipal(new PrincipalEntity()); - userEntity.setUserName("userName"); + userEntity.setUserName(UserName.fromString("userName")); userEntity.setUserId(1); User user = new User(userEntity); Authentication auth = new AmbariUserAuthentication(null, user, null); http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationProviderDisableUserTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationProviderDisableUserTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationProviderDisableUserTest.java index 6b98a5b..891ab38 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationProviderDisableUserTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationProviderDisableUserTest.java @@ -90,7 +90,7 @@ public class AmbariAuthorizationProviderDisableUserTest { UserEntity activeUser = new UserEntity(); activeUser.setUserId(1); activeUser.setActive(isActive); - activeUser.setUserName(login); + activeUser.setUserName(UserName.fromString(login)); activeUser.setUserPassword(encoder.encode("pwd")); activeUser.setPrincipal(principalEntity); Mockito.when(userDAO.findLocalUserByName(login)).thenReturn(activeUser); http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java index 9ff381f..f43ca90 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariLocalUserProviderTest.java @@ -155,7 +155,7 @@ public class AmbariLocalUserProviderTest { PrincipalEntity principalEntity = new PrincipalEntity(); UserEntity userEntity = new UserEntity(); userEntity.setUserId(1); - userEntity.setUserName(TEST_USER_NAME); + userEntity.setUserName(UserName.fromString(TEST_USER_NAME)); userEntity.setUserPassword(passwordEncoder.encode(TEST_USER_PASS)); userEntity.setUserType(UserType.LOCAL); userEntity.setPrincipal(principalEntity); http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariUserAuthenticationFilterTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariUserAuthenticationFilterTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariUserAuthenticationFilterTest.java index 6541a59..629be46 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariUserAuthenticationFilterTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariUserAuthenticationFilterTest.java @@ -208,7 +208,7 @@ public class AmbariUserAuthenticationFilterTest { PrincipalEntity principalEntity = new PrincipalEntity(); UserEntity userEntity = new UserEntity(); userEntity.setUserId(TEST_USER_ID); - userEntity.setUserName(TEST_USER_NAME); + userEntity.setUserName(UserName.fromString(TEST_USER_NAME)); userEntity.setUserType(UserType.LOCAL); userEntity.setPrincipal(principalEntity); User user = new User(userEntity); http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UserNameTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UserNameTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UserNameTest.java new file mode 100644 index 0000000..578acdb --- /dev/null +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UserNameTest.java @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ambari.server.security.authorization; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.util.Arrays; +import java.util.Collection; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(value = Parameterized.class) +public class UserNameTest { + private final String name; + private final boolean valid; + + @Parameterized.Parameters + public static Collection userNames() { + return Arrays.asList(new Object[][] { + {"", false}, + {null, false}, + {"invalid<", false}, + {">invalid", false}, + {"inv&lid", false}, + {"i\\nvalid", false}, + {"inva`lid", false}, + {"inval|d", false}, + {"user01", true}, + {"user_name", true}, + }); + } + + public UserNameTest(String name, boolean valid) { + this.name = name; + this.valid = valid; + } + + @Test + public void testRejectsForbiddenCharacters() throws Exception { + try { + assertEquals(name, UserName.fromString(name).toString()); + if (!valid) { + fail("Expected user " + name + " to be invalid."); + } + } catch (IllegalArgumentException e) { + if (valid) { + fail("Expected user " + name + " to be valid. But was: " + e.getMessage()); + } + } + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java index f426c85..ac91c90 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java @@ -161,7 +161,7 @@ public class UsersTest extends EasyMockSupport { @Test(expected = AmbariException.class) public void testCreateUser_Duplicate() throws Exception { UserEntity existing = new UserEntity(); - existing.setUserName(SERVICEOP_USER_NAME); + existing.setUserName(UserName.fromString(SERVICEOP_USER_NAME)); existing.setUserType(UserType.LDAP); existing.setUserId(1); existing.setMemberEntities(Collections.<MemberEntity>emptySet()); http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java index e5e36f3..d4213a6 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java @@ -59,6 +59,7 @@ import org.apache.ambari.server.security.authorization.Group; import org.apache.ambari.server.security.authorization.GroupType; import org.apache.ambari.server.security.authorization.LdapServerProperties; import org.apache.ambari.server.security.authorization.User; +import org.apache.ambari.server.security.authorization.UserName; import org.apache.ambari.server.security.authorization.Users; import org.easymock.Capture; import org.easymock.EasyMock; @@ -1970,7 +1971,7 @@ public class AmbariLdapDataPopulatorTest { private User createUser(String name, boolean ldapUser, GroupEntity group) { final UserEntity userEntity = new UserEntity(); userEntity.setUserId(userIdCounter++); - userEntity.setUserName(name); + userEntity.setUserName(UserName.fromString(name)); userEntity.setCreateTime(new Date()); userEntity.setLdapUser(ldapUser); userEntity.setActive(true); http://git-wip-us.apache.org/repos/asf/ambari/blob/e8fd10cf/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java index e31a428..70673f8 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java @@ -93,6 +93,7 @@ import org.apache.ambari.server.orm.entities.ViewInstanceEntity; import org.apache.ambari.server.orm.entities.WidgetEntity; import org.apache.ambari.server.security.authorization.ResourceType; import org.apache.ambari.server.security.authorization.User; +import org.apache.ambari.server.security.authorization.UserName; import org.apache.ambari.server.security.authorization.Users; import org.apache.ambari.server.stack.StackManagerFactory; import org.apache.ambari.server.state.AlertFirmness; @@ -2579,7 +2580,7 @@ public class UpgradeCatalog240Test { expect(requestScheduleDAO.findAll()).andReturn(Collections.singletonList(requestScheduleEntity)).once(); UserEntity userEntity = new UserEntity(); - userEntity.setUserName("createdUser"); + userEntity.setUserName(UserName.fromString("createdUser")); userEntity.setUserId(1); userEntity.setPrincipal(new PrincipalEntity()); User user = new User(userEntity);