Index: ../oak/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../oak/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java (revision 1695001) +++ ../oak/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java (revision ) @@ -16,8 +16,6 @@ */ package org.apache.jackrabbit.oak.security.user; -import static com.google.common.base.Preconditions.checkNotNull; - import java.io.UnsupportedEncodingException; import java.security.NoSuchAlgorithmException; import java.security.Principal; @@ -79,7 +77,8 @@ private UserQueryManager queryManager; private ReadOnlyNodeTypeManager ntMgr; - public UserManagerImpl(Root root, NamePathMapper namePathMapper, SecurityProvider securityProvider) { + public UserManagerImpl(@Nonnull Root root, @Nonnull NamePathMapper namePathMapper, + @Nonnull SecurityProvider securityProvider) { this.root = root; this.namePathMapper = namePathMapper; this.securityProvider = securityProvider; @@ -92,7 +91,7 @@ } @Nonnull - private static AuthorizableActionProvider getActionProvider(ConfigurationParameters config) { + private static AuthorizableActionProvider getActionProvider(@Nonnull ConfigurationParameters config) { AuthorizableActionProvider actionProvider = config.getConfigValue(UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER, null, AuthorizableActionProvider.class); if (actionProvider == null) { actionProvider = new DefaultAuthorizableActionProvider(config); @@ -104,9 +103,9 @@ @Override public Authorizable getAuthorizable(String id) throws RepositoryException { Authorizable authorizable = null; - Tree tree = userProvider.getAuthorizable(id); + Tree tree = (Strings.isNullOrEmpty(id)) ? null : userProvider.getAuthorizable(id); if (tree != null) { - authorizable = getAuthorizable(id, tree); + authorizable = getAuthorizable(UserUtil.getAuthorizableId(tree), tree); } return authorizable; } @@ -118,7 +117,7 @@ @Override public Authorizable getAuthorizable(Principal principal) throws RepositoryException { - return getAuthorizable(userProvider.getAuthorizableByPrincipal(principal)); + return (principal == null) ? null : getAuthorizable(userProvider.getAuthorizableByPrincipal(principal)); } @Override @@ -154,7 +153,7 @@ @Override public User createUser(String userID, String password, Principal principal, @Nullable String intermediatePath) throws RepositoryException { - checkValidID(userID); + checkValidId(userID); checkValidPrincipal(principal, false); if (intermediatePath != null) { @@ -175,7 +174,7 @@ @Override public User createSystemUser(String userID, String intermediatePath) throws RepositoryException { - checkValidID(userID); + checkValidId(userID); Principal principal = new PrincipalImpl(userID); checkValidPrincipal(principal, false); @@ -206,7 +205,7 @@ @Override public Group createGroup(String groupID, Principal principal, @Nullable String intermediatePath) throws RepositoryException { - checkValidID(groupID); + checkValidId(groupID); checkValidPrincipal(principal, true); if (intermediatePath != null) { @@ -257,7 +256,7 @@ * @param password The password. * @throws RepositoryException If an exception occurs. */ - void onCreate(User user, String password) throws RepositoryException { + void onCreate(@Nonnull User user, @CheckForNull String password) throws RepositoryException { for (AuthorizableAction action : actionProvider.getAuthorizableActions(securityProvider)) { action.onCreate(user, password, root, namePathMapper); } @@ -271,7 +270,7 @@ * @param group The new group. * @throws RepositoryException If an exception occurs. */ - void onCreate(Group group) throws RepositoryException { + void onCreate(@Nonnull Group group) throws RepositoryException { for (AuthorizableAction action : actionProvider.getAuthorizableActions(securityProvider)) { action.onCreate(group, root, namePathMapper); } @@ -285,7 +284,7 @@ * @param authorizable The authorizable to be removed. * @throws RepositoryException If an exception occurs. */ - void onRemove(Authorizable authorizable) throws RepositoryException { + void onRemove(@Nonnull Authorizable authorizable) throws RepositoryException { for (AuthorizableAction action : actionProvider.getAuthorizableActions(securityProvider)) { action.onRemove(authorizable, root, namePathMapper); } @@ -300,7 +299,7 @@ * @param password The new password. * @throws RepositoryException If an exception occurs. */ - void onPasswordChange(User user, String password) throws RepositoryException { + void onPasswordChange(@Nonnull User user, @Nonnull String password) throws RepositoryException { for (AuthorizableAction action : actionProvider.getAuthorizableActions(securityProvider)) { action.onPasswordChange(user, password, root, namePathMapper); } @@ -308,7 +307,7 @@ //-------------------------------------------------------------------------- @CheckForNull - Authorizable getAuthorizable(Tree tree) throws RepositoryException { + Authorizable getAuthorizable(@CheckForNull Tree tree) throws RepositoryException { if (tree == null || !tree.exists()) { return null; } @@ -344,32 +343,33 @@ } @CheckForNull - private Authorizable getAuthorizable(String id, Tree tree) throws RepositoryException { + private Authorizable getAuthorizable(@CheckForNull String id, @CheckForNull Tree tree) throws RepositoryException { if (id == null || tree == null) { return null; } if (UserUtil.isType(tree, AuthorizableType.USER)) { if (UserUtil.isSystemUser(tree)) { - return new SystemUserImpl(UserUtil.getAuthorizableId(tree), tree, this); + return new SystemUserImpl(id, tree, this); } else { - return new UserImpl(UserUtil.getAuthorizableId(tree), tree, this); + return new UserImpl(id, tree, this); } } else if (UserUtil.isType(tree, AuthorizableType.GROUP)) { - return new GroupImpl(UserUtil.getAuthorizableId(tree), tree, this); + return new GroupImpl(id, tree, this); } else { throw new RepositoryException("Not a user or group tree " + tree.getPath() + '.'); } } - private void checkValidID(String id) throws RepositoryException { + private void checkValidId(@CheckForNull String id) throws RepositoryException { if (id == null || id.isEmpty()) { throw new IllegalArgumentException("Invalid ID " + id); - } else if (getAuthorizable(id) != null) { + } + if (getAuthorizable(id) != null) { throw new AuthorizableExistsException("Authorizable with ID " + id + " already exists"); } } - void checkValidPrincipal(Principal principal, boolean isGroup) throws RepositoryException { + void checkValidPrincipal(@CheckForNull Principal principal, boolean isGroup) throws RepositoryException { if (principal == null || Strings.isNullOrEmpty(principal.getName())) { throw new IllegalArgumentException("Principal may not be null and must have a valid name."); } @@ -381,12 +381,11 @@ } } - void setPrincipal(Tree authorizableTree, Principal principal) { - checkNotNull(principal); + void setPrincipal(@Nonnull Tree authorizableTree, @Nonnull Principal principal) { authorizableTree.setProperty(UserConstants.REP_PRINCIPAL_NAME, principal.getName()); } - void setPassword(Tree userTree, String userId, String password, boolean forceHash) throws RepositoryException { + void setPassword(@Nonnull Tree userTree, @Nonnull String userId, @Nonnull String password, boolean forceHash) throws RepositoryException { String pwHash; if (forceHash || PasswordUtil.isPlainTextPassword(password)) { try { @@ -429,6 +428,7 @@ return config.getConfigValue(UserConstants.PARAM_PASSWORD_INITIAL_CHANGE, UserConstants.DEFAULT_PASSWORD_INITIAL_CHANGE); } + @Nonnull private UserQueryManager getQueryManager() { if (queryManager == null) { queryManager = new UserQueryManager(this, namePathMapper, config, root); \ No newline at end of file Index: ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java (revision 1695001) +++ ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java (revision ) @@ -90,6 +90,46 @@ super.after(); } + /** + * @see OAK-3191 + */ + @Test + public void testGetAuthorizableByEmptyId() throws Exception { + assertNull(userMgr.getAuthorizable("")); + } + + /** + * @see OAK-3191 + */ + @Test + public void testGetTypedAuthorizableByEmptyId() throws Exception { + assertNull(userMgr.getAuthorizable("", User.class)); + } + + /** + * @see OAK-3191 + */ + @Test + public void testGetAuthorizableByNullId() throws Exception { + assertNull(userMgr.getAuthorizable((String) null)); + } + + /** + * @see OAK-3191 + */ + @Test + public void testGetTypedAuthorizableByNullId() throws Exception { + assertNull(userMgr.getAuthorizable(null, User.class)); + } + + /** + * @see OAK-3191 + */ + @Test + public void testGetTypedAuthorizableByNullPrincipal() throws Exception { + assertNull(userMgr.getAuthorizable((Principal) null)); + } + @Test public void testSetPassword() throws Exception { User user = userMgr.createUser(testUserId, "pw"); \ No newline at end of file