Author: angela
Date: Thu May 11 15:20:20 2017
New Revision: 1794845
URL: http://svn.apache.org/viewvc?rev=1794845&view=rev
Log:
OAK-6212 : AccessControlAction: minor improvement when user or group privileges
are empty
OAK-6038 : Drop dependency of spi.security.* tests from AbstractSecurityTest
(wip)
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/AccessControlActionTest.java
- copied, changed from r1794697,
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlAction.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlAction.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlAction.java?rev=1794845&r1=1794844&r2=1794845&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlAction.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlAction.java
Thu May 11 15:20:20 2017
@@ -149,14 +149,22 @@ public class AccessControlAction extends
if (securityProvider == null) {
throw new IllegalStateException("Not initialized");
}
- if (isBuiltInUser(authorizable)) {
- log.debug("System user: " + authorizable.getID() + "; omit ac
setup.");
- return;
- }
- if (groupPrivilegeNames.length == 0 && userPrivilegeNames.length == 0)
{
- log.debug("No privileges configured for groups and users; omit ac
setup.");
- return;
+ if (authorizable.isGroup()) {
+ if (groupPrivilegeNames.length == 0) {
+ log.debug("No privileges configured for groups; omit ac
setup.");
+ return;
+ }
+ } else {
+ if (userPrivilegeNames.length == 0) {
+ log.debug("No privileges configured for users; omit ac
setup.");
+ return;
+ }
+ if (isBuiltInUser(authorizable)) {
+ log.debug("System user: " + authorizable.getID() + "; omit ac
setup.");
+ return;
+ }
}
+
Principal principal = authorizable.getPrincipal();
if (administrativePrincipals.contains(principal.getName())) {
log.debug("Administrative principal: " + principal.getName() + ";
omit ac setup.");
@@ -180,17 +188,8 @@ public class AccessControlAction extends
} else {
// setup acl according to configuration.
boolean modified = false;
- if (authorizable.isGroup()) {
- // new authorizable is a Group
- if (groupPrivilegeNames.length > 0) {
- modified = acl.addAccessControlEntry(principal,
getPrivileges(groupPrivilegeNames, acMgr));
- }
- } else {
- // new authorizable is a User
- if (userPrivilegeNames.length > 0) {
- modified = acl.addAccessControlEntry(principal,
getPrivileges(userPrivilegeNames, acMgr));
- }
- }
+ String[] privNames = (authorizable.isGroup()) ?
groupPrivilegeNames : userPrivilegeNames;
+ modified = acl.addAccessControlEntry(principal,
getPrivileges(privNames, acMgr));
if (modified) {
acMgr.setPolicy(path, acl);
}
@@ -198,9 +197,6 @@ public class AccessControlAction extends
}
private boolean isBuiltInUser(@Nonnull Authorizable authorizable) throws
RepositoryException {
- if (authorizable.isGroup()) {
- return false;
- }
ConfigurationParameters userConfig =
securityProvider.getConfiguration(UserConfiguration.class).getParameters();
String userId = authorizable.getID();
return UserUtil.getAdminId(userConfig).equals(userId) ||
UserUtil.getAnonymousId(userConfig).equals(userId);
Copied:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/AccessControlActionTest.java
(from r1794697,
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java)
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/AccessControlActionTest.java?p2=jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/AccessControlActionTest.java&p1=jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java&r1=1794697&r2=1794845&rev=1794845&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/AccessControlActionTest.java
Thu May 11 15:20:20 2017
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.jackrabbit.oak.spi.security.user.action;
+package org.apache.jackrabbit.oak.security.user.action;
import javax.jcr.security.AccessControlList;
import javax.jcr.security.AccessControlManager;
@@ -31,6 +31,7 @@ import org.apache.jackrabbit.oak.spi.sec
import
org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionConstants;
import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
+import org.apache.jackrabbit.oak.spi.security.user.action.AccessControlAction;
import org.junit.Test;
import static org.junit.Assert.assertArrayEquals;
@@ -38,7 +39,7 @@ import static org.junit.Assert.assertEqu
import static org.junit.Assert.assertTrue;
/**
- * Testing the {@code AccessControlAction}
+ * Integration tests for the {@code AccessControlAction} with complete
security setup
*/
public class AccessControlActionTest extends AbstractSecurityTest {
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java?rev=1794845&r1=1794844&r2=1794845&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AccessControlActionTest.java
Thu May 11 15:20:20 2017
@@ -16,108 +16,193 @@
*/
package org.apache.jackrabbit.oak.spi.security.user.action;
-import javax.jcr.security.AccessControlList;
-import javax.jcr.security.AccessControlManager;
-import javax.jcr.security.AccessControlPolicy;
-import javax.jcr.security.Privilege;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import javax.jcr.RepositoryException;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.User;
-import org.apache.jackrabbit.api.security.user.UserManager;
-import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
import
org.apache.jackrabbit.oak.spi.security.authorization.AuthorizationConfiguration;
import
org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionConstants;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
+import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.junit.Test;
+import org.mockito.Mockito;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
/**
- * Testing the {@code AccessControlAction}
+ * Unit tests for {@code AccessControlAction}.
+ *
+ * @see {@link
org.apache.jackrabbit.oak.security.user.action.AccessControlActionTest} for
integration tests include complete setup.
*/
-public class AccessControlActionTest extends AbstractSecurityTest {
+public class AccessControlActionTest implements UserConstants {
+
+ private final Root root = Mockito.mock(Root.class);
+ private final SecurityProvider securityProvider =
Mockito.mock(SecurityProvider.class);
+ private final UserConfiguration userConfiguration =
Mockito.mock(UserConfiguration.class);
+ private final AuthorizationConfiguration authorizationConfiguration =
Mockito.mock(AuthorizationConfiguration.class);
+
+ private void initSecurityProvider(@Nonnull String adminId, @Nonnull String
anonymousId, @Nonnull String... adminPrincipalNames) {
+
when(userConfiguration.getParameters()).thenReturn(ConfigurationParameters.of(
+ PARAM_ADMIN_ID, adminId,
+ PARAM_ANONYMOUS_ID, anonymousId));
+
+
when(authorizationConfiguration.getParameters()).thenReturn(ConfigurationParameters.of(PermissionConstants.PARAM_ADMINISTRATIVE_PRINCIPALS,
adminPrincipalNames));
+
+
when(securityProvider.getConfiguration(UserConfiguration.class)).thenReturn(userConfiguration);
+
when(securityProvider.getConfiguration(AuthorizationConfiguration.class)).thenReturn(authorizationConfiguration);
- @Override
- protected ConfigurationParameters getSecurityConfigParameters() {
- ConfigurationParameters userConfig = ConfigurationParameters.of(
- AccessControlAction.GROUP_PRIVILEGE_NAMES, new String[]
{PrivilegeConstants.JCR_READ},
- AccessControlAction.USER_PRIVILEGE_NAMES, new String[]
{PrivilegeConstants.JCR_ALL}
- );
- ConfigurationParameters authorizationConfig =
ConfigurationParameters.of(
- PermissionConstants.PARAM_ADMINISTRATIVE_PRINCIPALS, new
String[] {"administrativePrincipalName"}
- );
- return ConfigurationParameters.of(UserConfiguration.NAME, userConfig,
AuthorizationConfiguration.NAME, authorizationConfig);
}
- @Test
- public void testAccessControlActionForUser() throws Exception {
- UserManager userMgr = getUserManager(root);
- User u = null;
- try {
- String uid = "actionTestUser";
- u = userMgr.createUser(uid, uid);
- root.commit();
-
- assertAcAction(u, PrivilegeConstants.JCR_ALL);
- } finally {
- root.refresh();
- if (u != null) {
- u.remove();
- }
- root.commit();
+ private AccessControlAction createAction(@Nonnull String... privNames) {
+ AccessControlAction action = new AccessControlAction();
+ action.init(securityProvider, ConfigurationParameters.of(
+ AccessControlAction.USER_PRIVILEGE_NAMES, privNames,
+ AccessControlAction.GROUP_PRIVILEGE_NAMES, privNames));
+ return action;
+ }
+
+ private AccessControlAction createAction(@Nonnull String[] userPrivNames,
@Nonnull String[] groupPrivNames) {
+ AccessControlAction action = new AccessControlAction();
+ action.init(securityProvider, ConfigurationParameters.of(
+ AccessControlAction.USER_PRIVILEGE_NAMES, userPrivNames,
+ AccessControlAction.GROUP_PRIVILEGE_NAMES, groupPrivNames));
+ return action;
+ }
+
+ private static void mockAuthorizable(@Nonnull Authorizable a, @Nonnull
String id, @CheckForNull String principalName, @CheckForNull String path)
throws RepositoryException {
+ when(a.getID()).thenReturn(id);
+ if (principalName != null) {
+ when(a.getPrincipal()).thenReturn(new
PrincipalImpl(principalName));
+ } else {
+ when(a.getPrincipal()).thenThrow(new RepositoryException());
+ }
+ if (path != null) {
+ when(a.getPath()).thenReturn(path);
+ } else {
+ when(a.getPath()).thenThrow(new RepositoryException());
}
}
+ private static User mockUser(@Nonnull String id, @CheckForNull String
principalName, @CheckForNull String path) throws RepositoryException {
+ User user = Mockito.mock(User.class);
+ when(user.isGroup()).thenReturn(false);
+ mockAuthorizable(user, id, principalName, path);
+ return user;
+ }
+
+ private static Group mockGroup(@Nonnull String id, @CheckForNull String
principalName, @CheckForNull String path) throws RepositoryException {
+ Group gr = Mockito.mock(Group.class);
+ when(gr.isGroup()).thenReturn(true);
+ mockAuthorizable(gr, id, principalName, path);
+ return gr;
+ }
+
@Test
- public void testAccessControlAction() throws Exception {
- UserManager userMgr = getUserManager(root);
- Group gr = null;
- try {
- gr = userMgr.createGroup("actionTestGroup");
- root.commit();
-
- assertAcAction(gr, PrivilegeConstants.JCR_READ);
- } finally {
- root.refresh();
- if (gr != null) {
- gr.remove();
- }
- root.commit();
- }
+ public void testInit() {
+ // TODO
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testOnCreateUserMissingSecurityProvider() throws Exception {
+ new AccessControlAction().onCreate(Mockito.mock(User.class), null,
root, NamePathMapper.DEFAULT);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testOnCreateGroupMissingSecurityProvider() throws Exception {
+ new AccessControlAction().onCreate(Mockito.mock(Group.class), root,
NamePathMapper.DEFAULT);
}
@Test
- public void testAdministrativePrincipals() throws Exception {
- UserManager userMgr = getUserManager(root);
- Group gr = null;
- try {
- gr = userMgr.createGroup("administrativePrincipalName");
- root.commit();
-
- AccessControlManager acMgr = getAccessControlManager(root);
- AccessControlPolicy[] policies = acMgr.getPolicies(gr.getPath());
- assertEquals(0, policies.length);
- } finally {
- root.refresh();
- if (gr != null) {
- gr.remove();
- }
- root.commit();
+ public void testOnCreateBuiltinUser() throws Exception {
+ initSecurityProvider("adminId", "anonymousId");
+ AccessControlAction action = createAction(PrivilegeConstants.JCR_READ);
+
+ String[] buildinIds = new String[] {"adminId", "anonymousId"};
+ for (String id : buildinIds) {
+ // throw upon getPrincipal as onCreate for builtin users must not
reach that statement
+ User user = mockUser(id, null, null);
+ action.onCreate(user, null, root, NamePathMapper.DEFAULT);
}
}
- private void assertAcAction(Authorizable a, String expectedPrivName)
throws Exception {
- AccessControlManager acMgr = getAccessControlManager(root);
- AccessControlPolicy[] policies = acMgr.getPolicies(a.getPath());
- assertEquals(1, policies.length);
- assertTrue(policies[0] instanceof AccessControlList);
- AccessControlList acl = (AccessControlList) policies[0];
- assertEquals(1, acl.getAccessControlEntries().length);
- assertArrayEquals(new
Privilege[]{getPrivilegeManager(root).getPrivilege(expectedPrivName)},
acl.getAccessControlEntries()[0].getPrivileges());
+ @Test(expected = RepositoryException.class)
+ public void testOnCreateBuiltinIsGroup() throws Exception {
+ initSecurityProvider("adminIdIsUsedByGroup", "anonymousId");
+ AccessControlAction action = createAction(PrivilegeConstants.JCR_READ);
+
+ // the check for built-in user must ignore groups
+ Group gr = mockGroup("adminIdIsUsedByGroup", null, null);
+ action.onCreate(gr, root, NamePathMapper.DEFAULT);
}
+
+ @Test
+ public void testOnCreateUserEmptyPrivs() throws Exception {
+ initSecurityProvider(DEFAULT_ADMIN_ID, DEFAULT_ANONYMOUS_ID);
+ AccessControlAction action = createAction(new String[0], new String[]
{PrivilegeConstants.JCR_READ});
+
+ // throw upon getPrincipal as onCreate without configured privileges
call must not reach that statement
+ User user = mockUser("id", null, null);
+ action.onCreate(user, null, root, NamePathMapper.DEFAULT);
+ }
+
+ @Test
+ public void testOnCreateGroupEmptyPrivs() throws Exception {
+ initSecurityProvider(DEFAULT_ADMIN_ID, DEFAULT_ANONYMOUS_ID);
+ AccessControlAction action = createAction(new String[]
{PrivilegeConstants.JCR_READ}, new String[0]);
+
+ // throw upon getPrincipal as onCreate without configured privileges
call must not reach that statement
+ Group gr = mockGroup("id", null, null);
+ action.onCreate(gr, root, NamePathMapper.DEFAULT);
+ }
+
+ @Test
+ public void testOnCreateAdminUser() throws Exception {
+ initSecurityProvider(DEFAULT_ADMIN_ID, DEFAULT_ANONYMOUS_ID,
"administrativePrincipal");
+ AccessControlAction action = createAction(PrivilegeConstants.JCR_READ);
+
+ // throw upon getPath as onCreate for administrative principal call
must not reach that statement
+ User user = mockUser("id", "administrativePrincipal", null);
+ action.onCreate(user, null, root, NamePathMapper.DEFAULT);
+ }
+
+ @Test
+ public void testOnCreateAdminGroup() throws Exception {
+ initSecurityProvider(DEFAULT_ADMIN_ID, DEFAULT_ANONYMOUS_ID,
"administrativePrincipal");
+ AccessControlAction action = createAction(PrivilegeConstants.JCR_READ);
+
+ // throw upon getPath as onCreate for administrative principal call
must not reach that statement
+ Group gr = mockGroup("id", "administrativePrincipal", null);
+ action.onCreate(gr, root, NamePathMapper.DEFAULT);
+ }
+
+
+ @Test(expected = RepositoryException.class)
+ public void testOnCreateUserWithoutPath() throws Exception {
+ initSecurityProvider(DEFAULT_ADMIN_ID, DEFAULT_ANONYMOUS_ID);
+ AccessControlAction action = createAction(PrivilegeConstants.JCR_READ);
+
+ // throw upon getPath
+ User user = mockUser("id", "principalName", null);
+ action.onCreate(user, null, root, NamePathMapper.DEFAULT);
+ }
+
+ @Test(expected = RepositoryException.class)
+ public void testOnCreateGroupWithoutPath() throws Exception {
+ initSecurityProvider(DEFAULT_ADMIN_ID, DEFAULT_ANONYMOUS_ID);
+ AccessControlAction action = createAction(PrivilegeConstants.JCR_READ);
+
+ // throw upon getPath as onCreate for administrative principal call
must not reach that statement
+ Group gr = mockGroup("id", "principal", null);
+ action.onCreate(gr, root, NamePathMapper.DEFAULT);
+ }
+
}
\ No newline at end of file
