You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2015/08/11 10:54:11 UTC
svn commit: r1695231 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/
oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/
oak-doc/src/site/markdown/security/user/
Author: angela
Date: Tue Aug 11 08:54:11 2015
New Revision: 1695231
URL: http://svn.apache.org/r1695231
Log:
OAK-3191 : Oak UserManager#getAuthorizable handles null and empty string differently than Jackrabbit (credits to timothee maret for spotting and providing an initial patch)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java
jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/user/differences.md
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java?rev=1695231&r1=1695230&r2=1695231&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java Tue Aug 11 08:54:11 2015
@@ -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 @@ public class UserManagerImpl implements
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 @@ public class UserManagerImpl implements
}
@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 @@ public class UserManagerImpl implements
@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 @@ public class UserManagerImpl implements
@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 @@ public class UserManagerImpl implements
@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 @@ public class UserManagerImpl implements
@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 @@ public class UserManagerImpl implements
@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 @@ public class UserManagerImpl implements
* @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 @@ public class UserManagerImpl implements
* @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 @@ public class UserManagerImpl implements
* @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 @@ public class UserManagerImpl implements
* @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 @@ public class UserManagerImpl implements
//--------------------------------------------------------------------------
@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 @@ public class UserManagerImpl implements
}
@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 @@ public class UserManagerImpl implements
}
}
- 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 @@ public class UserManagerImpl implements
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);
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java?rev=1695231&r1=1695230&r2=1695231&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java Tue Aug 11 08:54:11 2015
@@ -90,6 +90,46 @@ public class UserManagerImplTest extends
super.after();
}
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-3191">OAK-3191</a>
+ */
+ @Test
+ public void testGetAuthorizableByEmptyId() throws Exception {
+ assertNull(userMgr.getAuthorizable(""));
+ }
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-3191">OAK-3191</a>
+ */
+ @Test
+ public void testGetTypedAuthorizableByEmptyId() throws Exception {
+ assertNull(userMgr.getAuthorizable("", User.class));
+ }
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-3191">OAK-3191</a>
+ */
+ @Test
+ public void testGetAuthorizableByNullId() throws Exception {
+ assertNull(userMgr.getAuthorizable((String) null));
+ }
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-3191">OAK-3191</a>
+ */
+ @Test
+ public void testGetTypedAuthorizableByNullId() throws Exception {
+ assertNull(userMgr.getAuthorizable(null, User.class));
+ }
+
+ /**
+ * @see <a href="https://issues.apache.org/jira/browse/OAK-3191">OAK-3191</a>
+ */
+ @Test
+ public void testGetTypedAuthorizableByNullPrincipal() throws Exception {
+ assertNull(userMgr.getAuthorizable((Principal) null));
+ }
+
@Test
public void testSetPassword() throws Exception {
User user = userMgr.createUser(testUserId, "pw");
Modified: jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/user/differences.md
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/user/differences.md?rev=1695231&r1=1695230&r2=1695231&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/user/differences.md (original)
+++ jackrabbit/oak/trunk/oak-doc/src/site/markdown/security/user/differences.md Tue Aug 11 08:54:11 2015
@@ -31,7 +31,9 @@ invalid transient modifications.
- stores user/group information in the workspace associated with the editing Session
- the autosave feature is no longer supported by default; configuration option
- `PARAM_SUPPORT_AUTOSAVE` can be used to obtain backwards compatible behavior.
+ `PARAM_SUPPORT_AUTOSAVE` can be used to obtain backwards compatible behavior
+- calling `getAuthorizable` with empty id or `null` id/principal will not throw
+ a runtime exception but silently returns `null`
#### Authorizable
@@ -43,6 +45,7 @@ invalid transient modifications.
it falls back on the node name in case the ID property is missing.
* Node Name: The name of the authorizable node is generated based on a configurable implementation
of the `AuthorizableNodeName` interface. Default: ID as name hint.
+ See section [Authorizable Node Name Generation](authorizablenodename.html) for details.
#### User