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/08/04 13:24:55 UTC

svn commit: r1755182 - in /jackrabbit/oak/branches/1.2: oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/ oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/ oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/

Author: angela
Date: Thu Aug  4 13:24:55 2016
New Revision: 1755182

URL: http://svn.apache.org/viewvc?rev=1755182&view=rev
Log:
OAK-4599 : SecurityProviderRegistration fails to update config param of SecurityConfiguration(s)

Added:
    jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializer.java
    jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/
    jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializerTest.java
Modified:
    jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java
    jackrabbit/oak/branches/1.2/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/SecurityProviderRegistrationTest.groovy

Added: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializer.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializer.java?rev=1755182&view=auto
==============================================================================
--- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializer.java (added)
+++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializer.java Thu Aug  4 13:24:55 2016
@@ -0,0 +1,44 @@
+/*
+ * 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.security.internal;
+
+import javax.annotation.Nonnull;
+
+import org.apache.jackrabbit.oak.spi.security.ConfigurationBase;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.SecurityConfiguration;
+import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
+
+class ConfigurationInitializer {
+
+    private ConfigurationInitializer() {}
+
+    @Nonnull
+    static <T extends SecurityConfiguration> T initializeConfiguration(@Nonnull SecurityProvider securityProvider, @Nonnull T configuration) {
+        return initializeConfiguration(securityProvider, configuration, ConfigurationParameters.EMPTY);
+    }
+
+    @Nonnull
+    static <T extends SecurityConfiguration> T initializeConfiguration(@Nonnull SecurityProvider securityProvider, @Nonnull T configuration, @Nonnull ConfigurationParameters parameters) {
+        if (configuration instanceof ConfigurationBase) {
+            ConfigurationBase base = (ConfigurationBase) configuration;
+            base.setSecurityProvider(securityProvider);
+            base.setParameters(ConfigurationParameters.of(base.getParameters(), parameters));
+        }
+        return configuration;
+    }
+}
\ No newline at end of file

Modified: jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java?rev=1755182&r1=1755181&r2=1755182&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java (original)
+++ jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java Thu Aug  4 13:24:55 2016
@@ -16,6 +16,12 @@
  */
 package org.apache.jackrabbit.oak.security.internal;
 
+import java.util.ArrayList;
+import java.util.Dictionary;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.Map;
+
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -30,9 +36,7 @@ import org.apache.felix.scr.annotations.
 import org.apache.jackrabbit.oak.commons.PropertiesUtil;
 import org.apache.jackrabbit.oak.osgi.OsgiWhiteboard;
 import org.apache.jackrabbit.oak.security.user.UserConfigurationImpl;
-import org.apache.jackrabbit.oak.spi.security.ConfigurationBase;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
-import org.apache.jackrabbit.oak.spi.security.SecurityConfiguration;
 import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
 import org.apache.jackrabbit.oak.spi.security.authentication.AuthenticationConfiguration;
 import org.apache.jackrabbit.oak.spi.security.authentication.token.CompositeTokenConfiguration;
@@ -58,12 +62,6 @@ import org.osgi.framework.ServiceRegistr
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.ArrayList;
-import java.util.Dictionary;
-import java.util.Hashtable;
-import java.util.List;
-import java.util.Map;
-
 import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Lists.newCopyOnWriteArrayList;
 
@@ -474,10 +472,10 @@ public class SecurityProviderRegistratio
 
         // Static, mandatory references
 
-        securityProvider.setAuthenticationConfiguration(initializeConfiguration(securityProvider, authenticationConfiguration));
+        securityProvider.setAuthenticationConfiguration(ConfigurationInitializer.initializeConfiguration(securityProvider, authenticationConfiguration));
         securityProvider.setAuthorizationConfiguration(initializeConfiguration(securityProvider, authorizationConfiguration));
         securityProvider.setUserConfiguration(initializeConfiguration(securityProvider, userConfiguration));
-        securityProvider.setPrivilegeConfiguration(initializeConfiguration(securityProvider, privilegeConfiguration));
+        securityProvider.setPrivilegeConfiguration(ConfigurationInitializer.initializeConfiguration(securityProvider, privilegeConfiguration));
 
         // Multiple, dynamic references
 
@@ -499,7 +497,7 @@ public class SecurityProviderRegistratio
                 ArrayList<PrincipalConfiguration> configurations = newArrayList(principalConfigurations);
 
                 for (PrincipalConfiguration configuration : configurations) {
-                    initializeConfiguration(getSecurityProvider(), configuration);
+                    ConfigurationInitializer.initializeConfiguration(getSecurityProvider(), configuration);
                 }
 
                 return configurations;
@@ -516,7 +514,7 @@ public class SecurityProviderRegistratio
                 List<TokenConfiguration> configurations = newArrayList(tokenConfigurations);
 
                 for (TokenConfiguration configuration : configurations) {
-                    initializeConfiguration(getSecurityProvider(), configuration);
+                    ConfigurationInitializer.initializeConfiguration(getSecurityProvider(), configuration);
                 }
 
                 return configurations;
@@ -526,33 +524,19 @@ public class SecurityProviderRegistratio
     }
 
     private AuthorizationConfiguration initializeConfiguration(SecurityProvider securityProvider, AuthorizationConfiguration authorizationConfiguration) {
-        return initializeConfiguration(securityProvider, authorizationConfiguration, ConfigurationParameters.of(
+        return ConfigurationInitializer.initializeConfiguration(securityProvider, authorizationConfiguration, ConfigurationParameters.of(
                 AccessControlConstants.PARAM_RESTRICTION_PROVIDER, createCompositeRestrictionProvider()
         ));
     }
 
     private UserConfiguration initializeConfiguration(SecurityProvider securityProvider, UserConfiguration userConfiguration) {
-        return initializeConfiguration(securityProvider, userConfiguration, ConfigurationParameters.of(
+        return ConfigurationInitializer.initializeConfiguration(securityProvider, userConfiguration, ConfigurationParameters.of(
                 ConfigurationParameters.of(UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER, createCompositeAuthorizableActionProvider()),
                 ConfigurationParameters.of(UserConstants.PARAM_AUTHORIZABLE_NODE_NAME, createCompositeAuthorizableNodeName()),
                 ConfigurationParameters.of(UserConstants.PARAM_USER_AUTHENTICATION_FACTORY, createCompositeUserAuthenticationFactory())
         ));
     }
 
-    private <T extends SecurityConfiguration> T initializeConfiguration(SecurityProvider securityProvider, T configuration) {
-        return initializeConfiguration(securityProvider, configuration, ConfigurationParameters.EMPTY);
-    }
-
-    private <T extends SecurityConfiguration> T initializeConfiguration(SecurityProvider securityProvider, T configuration, ConfigurationParameters parameters) {
-        if (configuration instanceof ConfigurationBase) {
-            ConfigurationBase base = (ConfigurationBase) configuration;
-            base.setSecurityProvider(securityProvider);
-            base.setParameters(ConfigurationParameters.of(parameters, base.getParameters()));
-        }
-
-        return configuration;
-    }
-
     private RestrictionProvider createCompositeRestrictionProvider() {
         return new WhiteboardRestrictionProvider() {
 

Added: jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializerTest.java?rev=1755182&view=auto
==============================================================================
--- jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializerTest.java (added)
+++ jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/ConfigurationInitializerTest.java Thu Aug  4 13:24:55 2016
@@ -0,0 +1,111 @@
+/*
+ * 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.security.internal;
+
+import org.apache.jackrabbit.oak.spi.security.ConfigurationBase;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.SecurityConfiguration;
+import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+public class ConfigurationInitializerTest {
+
+    private final SecurityProvider sp = new InternalSecurityProvider();
+    private final ConfigurationParameters params = ConfigurationParameters.of("key", "value");
+
+    @Test
+    public void testInitConfigurationReturnsSame() {
+        SecurityConfiguration sc = new SecurityConfiguration.Default();
+
+        assertSame(sc, ConfigurationInitializer.initializeConfiguration(sp, sc));
+    }
+
+    @Test
+    public void testInitBaseConfigurationReturnsSame() {
+        SecurityConfiguration sc = new TestConfiguration();
+
+        assertSame(sc, ConfigurationInitializer.initializeConfiguration(sp, sc));
+    }
+
+    @Test
+    public void testInitConfigurationWithParamReturnsSame() {
+        SecurityConfiguration sc = new SecurityConfiguration.Default();
+        assertSame(sc, ConfigurationInitializer.initializeConfiguration(sp, sc, params));
+    }
+
+    @Test
+    public void testInitBaseConfigurationWithParamReturnsSame() {
+        SecurityConfiguration sc = new TestConfiguration();
+        assertSame(sc, ConfigurationInitializer.initializeConfiguration(sp, sc, params));
+    }
+
+    @Test
+    public void testInitNonBaseConfiguration() {
+        SecurityConfiguration sc = new SecurityConfiguration.Default();
+
+        ConfigurationInitializer.initializeConfiguration(sp, sc);
+        assertFalse(sc.getParameters().containsKey("key"));
+    }
+
+    @Test
+    public void testInitBaseConfiguration() {
+        TestConfiguration sc = new TestConfiguration();
+
+        SecurityConfiguration afterInit = ConfigurationInitializer.initializeConfiguration(sp, sc);
+        assertSame(sc, afterInit);
+
+        // verify securityprovider
+        assertSame(sp, sc.getSecurityProvider());
+
+        // verify params
+        ConfigurationParameters parameters = afterInit.getParameters();
+        assertTrue(parameters.containsKey("key"));
+        assertTrue(parameters.containsKey("key2"));
+        assertEquals("initialValue", parameters.get("key"));
+        assertEquals("initialValue", parameters.get("key2"));
+    }
+
+    @Test
+    public void testInitBaseConfigurationWithParam() {
+        TestConfiguration sc = new TestConfiguration();
+
+        SecurityConfiguration afterInit = ConfigurationInitializer.initializeConfiguration(sp, sc, params);
+        assertSame(sc, afterInit);
+
+        // verify securityprovider
+        assertSame(sp, sc.getSecurityProvider());
+
+        // verify params
+        ConfigurationParameters parameters = afterInit.getParameters();
+        assertTrue(parameters.containsKey("key"));
+        assertTrue(parameters.containsKey("key2"));
+        assertEquals("value", parameters.get("key"));
+        assertEquals("initialValue", parameters.get("key2"));
+    }
+
+    private final class TestConfiguration extends ConfigurationBase {
+
+        TestConfiguration() {
+            super(sp, ConfigurationParameters.of("key", "initialValue", "key2", "initialValue"));
+        }
+    }
+}
\ No newline at end of file

Modified: jackrabbit/oak/branches/1.2/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/SecurityProviderRegistrationTest.groovy
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/SecurityProviderRegistrationTest.groovy?rev=1755182&r1=1755181&r2=1755182&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.2/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/SecurityProviderRegistrationTest.groovy (original)
+++ jackrabbit/oak/branches/1.2/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/run/osgi/SecurityProviderRegistrationTest.groovy Thu Aug  4 13:24:55 2016
@@ -17,12 +17,18 @@
 package org.apache.jackrabbit.oak.run.osgi
 
 import org.apache.felix.connect.launch.PojoServiceRegistry
+import org.apache.felix.scr.Component
+import org.apache.felix.scr.ScrService
 import org.apache.jackrabbit.oak.spi.security.SecurityProvider
 import org.apache.jackrabbit.oak.spi.security.authentication.token.TokenConfiguration
+import org.apache.jackrabbit.oak.spi.security.authorization.AuthorizationConfiguration
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider
 import org.apache.jackrabbit.oak.spi.security.principal.PrincipalConfiguration
 import org.apache.jackrabbit.oak.spi.security.user.AuthorizableNodeName
 import org.apache.jackrabbit.oak.spi.security.user.UserAuthenticationFactory
+import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration
+import org.apache.jackrabbit.oak.spi.security.user.action.AccessControlAction
+import org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableAction
 import org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableActionProvider
 import org.junit.Before
 import org.junit.Test
@@ -42,7 +48,11 @@ class SecurityProviderRegistrationTest e
         registry = repositoryFactory.initializeServiceRegistry(config)
     }
 
-    /**
+    @Override
+    protected PojoServiceRegistry getRegistry() {
+        return registry
+    }
+/**
      * Test that, without any additional configuration, a SecurityProvider
      * service is registered by default.
      */
@@ -131,6 +141,93 @@ class SecurityProviderRegistrationTest e
         assert securityProviderServiceReferences != null
     }
 
+    @Test
+    public void testSecurityConfigurations() {
+        //Keep a dummy reference to UserConfiguration such that SCR does not deactivate it
+        //once SecurityProviderRegistration gets deactivated. Otherwise if SecurityProviderRegistration is
+        //the only component that refers it then upon its deactivation UserConfiguration would also be
+        //deactivate and its internal state would be reset
+        UserConfiguration userConfiguration = getServiceWithWait(UserConfiguration.class)
+
+        ScrService scr = getServiceWithWait(ScrService.class)
+        Component[] c = scr.getComponents('org.apache.jackrabbit.oak.security.authentication.AuthenticationConfigurationImpl')
+        assert c
+
+        // 1. Disable AuthenticationConfiguration such that SecurityProvider is unregistered
+        c[0].disable()
+
+        TimeUnit.SECONDS.sleep(1)
+
+        assert securityProviderServiceReferences == null
+
+        // 2. Modify the config for AuthorizableActionProvider. It's expected that this config change is picked up
+        setConfiguration([
+                "org.apache.jackrabbit.oak.spi.security.user.action.DefaultAuthorizableActionProvider": [
+                        "enabledActions":"org.apache.jackrabbit.oak.spi.security.user.action.AccessControlAction",
+                        "groupPrivilegeNames":"jcr:read"
+                ]
+        ])
+
+        TimeUnit.SECONDS.sleep(1)
+
+        // 3. Enable component again such that SecurityProvider gets reactivated
+        c[0].enable()
+
+        SecurityProvider securityProvider = getServiceWithWait(SecurityProvider.class)
+        assertAuthorizationConfig(securityProvider)
+        assertUserConfig(securityProvider, "jcr:read")
+    }
+
+    @Test
+    public void testSecurityConfigurations2() {
+        setConfiguration([
+                "org.apache.jackrabbit.oak.spi.security.user.action.DefaultAuthorizableActionProvider": [
+                        "enabledActions":"org.apache.jackrabbit.oak.spi.security.user.action.AccessControlAction"
+                ]
+        ])
+        SecurityProvider securityProvider = registry.getService(registry.getServiceReference(SecurityProvider.class.name))
+        assertAuthorizationConfig(securityProvider)
+        assertUserConfig(securityProvider)
+
+        //Keep a dummy reference to UserConfiguration such that SCR does not deactivate it
+        //once SecurityProviderRegistration gets deactivated. Otherwise if SecurityProviderRegistration is
+        //the only component that refers it then upon its deactivation UserConfiguration would also be
+        //deactivate and its internal state would be reset
+        UserConfiguration userConfiguration = getServiceWithWait(UserConfiguration.class)
+
+        //1. Modify the config for AuthorizableActionProvider. It's expected that this config change is picked up
+        setConfiguration([
+                "org.apache.jackrabbit.oak.spi.security.user.action.DefaultAuthorizableActionProvider": [
+                        "enabledActions":"org.apache.jackrabbit.oak.spi.security.user.action.AccessControlAction",
+                        "groupPrivilegeNames":"jcr:read"
+                ]
+        ])
+
+        TimeUnit.SECONDS.sleep(1)
+
+        securityProvider = getServiceWithWait(SecurityProvider.class)
+        assertAuthorizationConfig(securityProvider)
+        assertUserConfig(securityProvider, "jcr:read")
+
+        ScrService scr = getServiceWithWait(ScrService.class)
+        Component[] c = scr.getComponents('org.apache.jackrabbit.oak.security.authentication.AuthenticationConfigurationImpl')
+        assert c
+
+        // 2. Disable AuthenticationConfiguration such that SecurityProvider is unregistered
+        c[0].disable()
+
+        TimeUnit.SECONDS.sleep(1)
+
+        assert securityProviderServiceReferences == null
+
+        // 3. Enable component again such that SecurityProvider gets reactivated
+        c[0].enable()
+
+        securityProvider = getServiceWithWait(SecurityProvider.class)
+        assertAuthorizationConfig(securityProvider)
+        assertUserConfig(securityProvider, "jcr:read")
+    }
+
     private <T> void testRequiredService(Class<T> serviceClass, T service) {
 
         // Adding a new precondition on a missing service PID forces the
@@ -187,4 +284,28 @@ class SecurityProviderRegistrationTest e
         return new Hashtable<K, V>(map);
     }
 
+    private static void assertAuthorizationConfig(SecurityProvider securityProvider, String... adminPrincipals) {
+        AuthorizationConfiguration ac = securityProvider.getConfiguration(AuthorizationConfiguration.class)
+
+        assert ac.getParameters().containsKey("restrictionProvider")
+        assert ac.getRestrictionProvider() != null
+        assert ac.getParameters().get("restrictionProvider") == ac.getRestrictionProvider()
+
+        String[] found = ac.getParameters().getConfigValue("administrativePrincipals", new String[0])
+        assert Arrays.equals(adminPrincipals, found)
+    }
+
+    private static void assertUserConfig(SecurityProvider securityProvider, String... groupPrivilegeNames) {
+        UserConfiguration uc = securityProvider.getConfiguration(UserConfiguration.class)
+
+        assert uc.getParameters().containsKey("authorizableActionProvider")
+        AuthorizableActionProvider ap = uc.getParameters().get("authorizableActionProvider")
+        assert ap != null;
+
+        List<AuthorizableAction> actionList = ap.getAuthorizableActions(securityProvider)
+        assert actionList.size() == 1
+        AuthorizableAction action = actionList.get(0)
+        assert Arrays.equals(groupPrivilegeNames, ((AccessControlAction) action).groupPrivilegeNames)
+    }
+
 }