You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ad...@apache.org on 2017/04/24 07:52:33 UTC
[1/2] ambari git commit: AMBARI-20771. BE: Characters used in
usernames should be constrained (Attila Magyar via adoroszlai)
Repository: ambari
Updated Branches:
refs/heads/branch-2.5 39f03e2f8 -> ae30013da
refs/heads/trunk 7f4f421ae -> e8fd10cf6
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/trunk
Commit: e8fd10cf6de7cc20662ab7d609ac8e105326ab6a
Parents: 7f4f421
Author: Attila Magyar <am...@hortonworks.com>
Authored: Mon Apr 24 09:51:19 2017 +0200
Committer: Attila Doroszlai <ad...@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);
[2/2] ambari git commit: AMBARI-20771. BE: Characters used in
usernames should be constrained (Attila Magyar via adoroszlai)
Posted by ad...@apache.org.
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/ae30013d
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/ae30013d
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/ae30013d
Branch: refs/heads/branch-2.5
Commit: ae30013da5ff41fea77b77e7f586b4b26f1000c6
Parents: 39f03e2
Author: Attila Magyar <am...@hortonworks.com>
Authored: Mon Apr 24 09:53:58 2017 +0200
Committer: Attila Doroszlai <ad...@hortonworks.com>
Committed: Mon Apr 24 09:53:58 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 | 4 +-
...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(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ambari/blob/ae30013d/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 5b8360a..a1ce3d2 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
@@ -17,6 +17,7 @@
*/
package org.apache.ambari.server.orm.entities;
+import org.apache.ambari.server.security.authorization.UserName;
import org.apache.ambari.server.security.authorization.UserType;
import javax.persistence.*;
@@ -98,8 +99,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/ae30013d/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/ae30013d/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 78ab64a..83edccc 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/ae30013d/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 ac8c8de..527f031 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/ae30013d/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 0503b94..3f4d063 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;
@@ -116,7 +117,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/ae30013d/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 3442546..73dadda 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
@@ -22,7 +22,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.commons.lang.StringUtils;
+import org.apache.ambari.server.security.authorization.UserName;
import org.junit.Assert;
import org.junit.Test;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
@@ -44,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/ae30013d/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/ae30013d/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 6c25b1d..c52510c 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
@@ -154,7 +154,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/ae30013d/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 80a66fd..3ec6a41 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/ae30013d/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/ae30013d/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 7b79e00..ccea7df 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/ae30013d/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 6c780f4..111dfc0 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
@@ -41,6 +41,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;
@@ -1958,7 +1959,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/ae30013d/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 b9f37d5..8baf496 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
@@ -65,6 +65,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;
@@ -2511,7 +2512,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);