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 2016/05/13 11:12:57 UTC
svn commit: r1743653 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/spi/security/
main/java/org/apache/jackrabbit/oak/spi/security/user/action/
test/java/org/apache/jackrabbit/oak/spi/security/
test/java/org/apache/jackrab...
Author: angela
Date: Fri May 13 11:12:57 2016
New Revision: 1743653
URL: http://svn.apache.org/viewvc?rev=1743653&view=rev
Log:
OAK-4365 : Redundant Action Class Lookup in DefaultAuthorizableActionProvider
- minor improvment: simplify setup of AuthorizationAction-tests without custom sec-provider
- minor improvement: simplify ConfigurationParameters.of(Map) if map represents an instanceof ConfigurationParameters
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProviderTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/GroupActionTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordValidationActionTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java?rev=1743653&r1=1743652&r2=1743653&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java Fri May 13 11:12:57 2016
@@ -138,6 +138,9 @@ public final class ConfigurationParamete
if (map.isEmpty()) {
return EMPTY;
}
+ if (map instanceof ConfigurationParameters) {
+ return (ConfigurationParameters) map;
+ }
Map<String, Object> options = new HashMap<String, Object>(map.size());
for (Map.Entry<?,?> e : map.entrySet()) {
options.put(String.valueOf(e.getKey()), e.getValue());
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java?rev=1743653&r1=1743652&r2=1743653&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java Fri May 13 11:12:57 2016
@@ -18,9 +18,9 @@ package org.apache.jackrabbit.oak.spi.se
import java.util.List;
import java.util.Map;
-
import javax.annotation.Nonnull;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
@@ -67,16 +67,28 @@ public class DefaultAuthorizableActionPr
private static final Logger log = LoggerFactory.getLogger(DefaultAuthorizableActionProvider.class);
+ private static final Map<String, Class<? extends AuthorizableAction>> SUPPORTED_ACTIONS = ImmutableMap.<String, Class<? extends AuthorizableAction>>of(
+ AccessControlAction.class.getName(), AccessControlAction.class,
+ PasswordValidationAction.class.getName(), PasswordValidationAction.class,
+ PasswordChangeAction.class.getName(), PasswordChangeAction.class,
+ ClearMembershipAction.class.getName(), ClearMembershipAction.class
+ );
+
+ private static final String[] DEFAULT_ACTIONS = new String[] {AccessControlAction.class.getName()};
+
static final String ENABLED_ACTIONS = "enabledActions";
- private String[] enabledActions = new String[] {AccessControlAction.class.getName()};
+ private String[] enabledActions = DEFAULT_ACTIONS;
private ConfigurationParameters config = ConfigurationParameters.EMPTY;
+ @SuppressWarnings("UnusedDeclaration")
public DefaultAuthorizableActionProvider() {}
public DefaultAuthorizableActionProvider(ConfigurationParameters config) {
- this.config = config;
+ if (config != null) {
+ activate(config);
+ }
}
//-----------------------------------------< AuthorizableActionProvider >---
@@ -86,19 +98,16 @@ public class DefaultAuthorizableActionPr
List<AuthorizableAction> actions = Lists.newArrayList();
for (String className : enabledActions) {
try {
- Object o = Class.forName(className).newInstance();
- if (o instanceof AuthorizableAction) {
- actions.add((AuthorizableAction) o);
+ Class<? extends AuthorizableAction> cl = SUPPORTED_ACTIONS.get(className);
+ if (cl != null) {
+ AuthorizableAction action = cl.newInstance();
+ action.init(securityProvider, config);
+ actions.add(action);
}
} catch (Exception e) {
log.debug("Unable to create authorizable action", e);
}
}
-
- // merge configurations that may contain action configuration information
- for (AuthorizableAction action : actions) {
- action.init(securityProvider, config);
- }
return actions;
}
@@ -107,6 +116,6 @@ public class DefaultAuthorizableActionPr
@Activate
private void activate(Map<String, Object> properties) {
config = ConfigurationParameters.of(properties);
- enabledActions = PropertiesUtil.toStringArray(properties.get(ENABLED_ACTIONS), new String[0]);
+ enabledActions = config.getConfigValue(ENABLED_ACTIONS, DEFAULT_ACTIONS);
}
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java?rev=1743653&r1=1743652&r2=1743653&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java Fri May 13 11:12:57 2016
@@ -25,6 +25,7 @@ import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import org.junit.After;
import org.junit.Before;
@@ -66,6 +67,17 @@ public class ConfigurationParametersTest
}
@Test
+ public void testCreationFromMap() {
+ Map<String, String> m = ImmutableMap.of("a", "b");
+ ConfigurationParameters cp = ConfigurationParameters.of(m);
+ assertEquals(m.size(), cp.size());
+
+ // verify shortcut if the passed map is already an instanceof ConfigurationParameters
+ ConfigurationParameters cp2 = ConfigurationParameters.of(cp);
+ assertSame(cp, cp2);
+ }
+
+ @Test
public void testContains() {
ConfigurationParameters params = ConfigurationParameters.EMPTY;
assertFalse(params.contains("some"));
Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProviderTest.java?rev=1743653&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProviderTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProviderTest.java Fri May 13 11:12:57 2016
@@ -0,0 +1,106 @@
+/*
+ * 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.jackrabbit.oak.spi.security.user.action;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+public class DefaultAuthorizableActionProviderTest extends AbstractSecurityTest {
+
+ @Test
+ public void testNoConfig() {
+ AuthorizableActionProvider[] providers = new AuthorizableActionProvider[] {
+ new DefaultAuthorizableActionProvider(),
+ new DefaultAuthorizableActionProvider(null)
+ };
+
+ for (AuthorizableActionProvider actionProvider : providers) {
+ List<? extends AuthorizableAction> actions = actionProvider.getAuthorizableActions(getSecurityProvider());
+ assertNotNull(actions);
+ assertEquals(1, actions.size());
+ assertTrue(actions.get(0) instanceof AccessControlAction);
+ }
+ }
+
+ @Test
+ public void testEmptyConfig() {
+ AuthorizableActionProvider actionProvider = new DefaultAuthorizableActionProvider(ConfigurationParameters.EMPTY);
+
+ List<? extends AuthorizableAction> actions = actionProvider.getAuthorizableActions(getSecurityProvider());
+ assertEquals(1, actions.size());
+ assertTrue(actions.get(0) instanceof AccessControlAction);
+ }
+
+ @Test
+ public void testNullActionConfig() {
+ Map<String, String[]> m = new HashMap();
+ m.put(DefaultAuthorizableActionProvider.ENABLED_ACTIONS, null);
+
+ AuthorizableActionProvider actionProvider = new DefaultAuthorizableActionProvider(ConfigurationParameters.of(m));
+ List<? extends AuthorizableAction> actions = actionProvider.getAuthorizableActions(getSecurityProvider());
+ assertEquals(1, actions.size());
+ assertTrue(actions.get(0) instanceof AccessControlAction);
+ }
+
+ @Test
+ public void testEmtpyActionConfig() {
+ AuthorizableActionProvider actionProvider = new DefaultAuthorizableActionProvider(
+ ConfigurationParameters.of(DefaultAuthorizableActionProvider.ENABLED_ACTIONS, new String[0]));
+ List<? extends AuthorizableAction> actions = actionProvider.getAuthorizableActions(getSecurityProvider());
+ assertNotNull(actions);
+ assertEquals(0, actions.size());
+ }
+
+ @Test
+ public void testNonExistingClassName() {
+ String[] classNames = new String[] {
+ "org.apache.jackrabbit.oak.spi.security.user.action.NonExistingAction",
+ ""
+ };
+ AuthorizableActionProvider actionProvider = new DefaultAuthorizableActionProvider(
+ ConfigurationParameters.of(DefaultAuthorizableActionProvider.ENABLED_ACTIONS, classNames));
+
+ List<? extends AuthorizableAction> actions = actionProvider.getAuthorizableActions(getSecurityProvider());
+ assertNotNull(actions);
+ assertEquals(0, actions.size());
+ }
+
+ @Test
+ public void testValidConfig() {
+ String[] classNames = new String[] {
+ PasswordChangeAction.class.getName(),
+ PasswordValidationAction.class.getName()
+ };
+ AuthorizableActionProvider actionProvider = new DefaultAuthorizableActionProvider(
+ ConfigurationParameters.of(DefaultAuthorizableActionProvider.ENABLED_ACTIONS, classNames));
+
+ List<? extends AuthorizableAction> actions = actionProvider.getAuthorizableActions(getSecurityProvider());
+ assertNotNull(actions);
+ assertEquals(2, actions.size());
+ assertTrue(actions.get(0) instanceof PasswordChangeAction);
+ assertTrue(actions.get(1) instanceof PasswordValidationAction);
+ }
+}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/GroupActionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/GroupActionTest.java?rev=1743653&r1=1743652&r2=1743653&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/GroupActionTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/GroupActionTest.java Fri May 13 11:12:57 2016
@@ -16,6 +16,11 @@
*/
package org.apache.jackrabbit.oak.spi.security.user.action;
+import java.util.List;
+import java.util.Set;
+import javax.annotation.Nonnull;
+import javax.jcr.RepositoryException;
+
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -25,11 +30,8 @@ import org.apache.jackrabbit.api.securit
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.security.SecurityProviderImpl;
-import org.apache.jackrabbit.oak.security.user.UserConfigurationImpl;
import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
-import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider;
import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
@@ -38,12 +40,6 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-import javax.jcr.RepositoryException;
-import java.util.List;
-import java.util.Set;
-
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -53,11 +49,20 @@ public class GroupActionTest extends Abs
private static final String TEST_GROUP_ID = "testGroup";
private static final String TEST_USER_PREFIX = "testUser";
- TestGroupAction groupAction = new TestGroupAction();
- Group testGroup;
+ final TestGroupAction groupAction = new TestGroupAction();
+ private final AuthorizableActionProvider actionProvider = new AuthorizableActionProvider() {
+ @Nonnull
+ @Override
+ public List<? extends AuthorizableAction> getAuthorizableActions(@Nonnull SecurityProvider securityProvider) {
+ return ImmutableList.of(groupAction);
+ }
+ };
+
private User testUser01;
private User testUser02;
+ Group testGroup;
+
@Before
public void before() throws Exception {
super.before();
@@ -87,6 +92,19 @@ public class GroupActionTest extends Abs
super.after();
}
+ @Override
+ protected ConfigurationParameters getSecurityConfigParameters() {
+ ConfigurationParameters userParams = ConfigurationParameters.of(
+ UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER, actionProvider,
+ ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, getImportBehavior()
+ );
+ return ConfigurationParameters.of(UserConfiguration.NAME, userParams);
+ }
+
+ String getImportBehavior() {
+ return ImportBehavior.NAME_IGNORE;
+ }
+
@Test
public void testMemberAdded() throws Exception {
testUser01 = getUserManager(root).createUser(TEST_USER_PREFIX + "01", "");
@@ -161,14 +179,6 @@ public class GroupActionTest extends Abs
assertTrue(Iterables.elementsEqual(nonExisting, groupAction.failedIds));
}
- @Override
- protected SecurityProvider getSecurityProvider() {
- if (securityProvider == null) {
- securityProvider = new TestSecurityProvider();
- }
- return securityProvider;
- }
-
class TestGroupAction extends AbstractGroupAction {
boolean onMemberAddedCalled = false;
@@ -211,47 +221,4 @@ public class GroupActionTest extends Abs
onMembersRemovedCalled = true;
}
}
-
- private class TestSecurityProvider extends SecurityProviderImpl {
-
- private final AuthorizableActionProvider actionProvider;
-
- private TestSecurityProvider() {
- actionProvider = new AuthorizableActionProvider() {
- @Nonnull
- @Override
- public List<? extends AuthorizableAction> getAuthorizableActions(@Nonnull SecurityProvider securityProvider) {
- return ImmutableList.of(groupAction);
- }
- };
- }
-
- @Nonnull
- public <T> T getConfiguration(@Nonnull Class<T> configClass) {
- if (UserConfiguration.class == configClass) {
- return (T) new UserConfigurationImpl(this) {
- @Nonnull
- @Override
- public ConfigurationParameters getParameters() {
- return ConfigurationParameters.of(super.getParameters(),
- ConfigurationParameters.of(UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER, actionProvider),
- ConfigurationParameters.of(ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, getImportBehavior())
- );
- }
-
- @Nullable
- @Override
- public PrincipalProvider getUserPrincipalProvider(@Nonnull Root root, @Nonnull NamePathMapper namePathMapper) {
- return null;
- }
- };
- } else {
- return super.getConfiguration(configClass);
- }
- }
- }
-
- String getImportBehavior() {
- return ImportBehavior.NAME_IGNORE;
- }
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordValidationActionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordValidationActionTest.java?rev=1743653&r1=1743652&r2=1743653&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordValidationActionTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordValidationActionTest.java Fri May 13 11:12:57 2016
@@ -29,11 +29,8 @@ import org.apache.jackrabbit.oak.Abstrac
import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
-import org.apache.jackrabbit.oak.security.SecurityProviderImpl;
-import org.apache.jackrabbit.oak.security.user.UserConfigurationImpl;
import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
-import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider;
import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil;
@@ -48,8 +45,15 @@ import static org.junit.Assert.fail;
public class PasswordValidationActionTest extends AbstractSecurityTest {
- private PasswordValidationAction pwAction = new PasswordValidationAction();
- private TestAction testAction = new TestAction();
+ private final PasswordValidationAction pwAction = new PasswordValidationAction();
+ private final TestAction testAction = new TestAction();
+ private final AuthorizableActionProvider actionProvider = new AuthorizableActionProvider() {
+ @Nonnull
+ @Override
+ public List<? extends AuthorizableAction> getAuthorizableActions(@Nonnull SecurityProvider securityProvider) {
+ return ImmutableList.of(pwAction, testAction);
+ }
+ };
private User user;
private User testUser;
@@ -77,11 +81,11 @@ public class PasswordValidationActionTes
}
@Override
- protected SecurityProvider getSecurityProvider() {
- if (securityProvider == null) {
- securityProvider = new TestSecurityProvider();
- }
- return securityProvider;
+ protected ConfigurationParameters getSecurityConfigParameters() {
+ ConfigurationParameters userParams = ConfigurationParameters.of(
+ UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER, actionProvider
+ );
+ return ConfigurationParameters.of(UserConfiguration.NAME, userParams);
}
@Test
@@ -176,41 +180,4 @@ public class PasswordValidationActionTes
onPasswordChangeCalled++;
}
}
-
- private class TestSecurityProvider extends SecurityProviderImpl {
-
- private final AuthorizableActionProvider actionProvider;
-
- private TestSecurityProvider() {
- actionProvider = new AuthorizableActionProvider() {
- @Nonnull
- @Override
- public List<? extends AuthorizableAction> getAuthorizableActions(@Nonnull SecurityProvider securityProvider) {
- return ImmutableList.of(pwAction, testAction);
- }
- };
- }
-
- @Nonnull
- public <T> T getConfiguration(@Nonnull Class<T> configClass) {
- if (UserConfiguration.class == configClass) {
- return (T) new UserConfigurationImpl(this) {
- @Nonnull
- @Override
- public ConfigurationParameters getParameters() {
- return ConfigurationParameters.of(super.getParameters(),
- ConfigurationParameters.of(UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER, actionProvider));
- }
-
- @Nullable
- @Override
- public PrincipalProvider getUserPrincipalProvider(@Nonnull Root root, @Nonnull NamePathMapper namePathMapper) {
- return null;
- }
- };
- } else {
- return super.getConfiguration(configClass);
- }
- }
- }
}