From f717ed7bdb712dce38f5dcab4f34576b7e2a5b99 Fri, 21 Jul 2017 17:28:44 +0300 From: Robert Munteanu Date: Fri, 14 Jul 2017 11:08:09 +0300 Subject: [PATCH] OAK-6450 - Stop relying on the service.pid property in SecurityProviderRegistration Use the oak.component.name component property if the service.pid is not available. The SecurityProviderRegistration property name is unchanged, for backwards compatibility reasons. The objectClass property may not be used as it points to the service name(s) under which the component is registered. The component.name property was considered and discarded as it is specific to DS. diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalPrincipalConfiguration.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalPrincipalConfiguration.java index 2cc0a93..c6f8319 100644 --- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalPrincipalConfiguration.java +++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalPrincipalConfiguration.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + import java.security.Principal; import java.security.acl.Group; import java.util.Arrays; @@ -85,7 +87,10 @@ @Property(name = ExternalIdentityConstants.PARAM_PROTECT_EXTERNAL_IDS, label = "External Identity Protection", description = "If disabled rep:externalId properties won't be properly protected (backwards compatible behavior). NOTE: for security reasons it is strongly recommend to keep the protection enabled!", - boolValue = ExternalIdentityConstants.DEFAULT_PROTECT_EXTERNAL_IDS) + boolValue = ExternalIdentityConstants.DEFAULT_PROTECT_EXTERNAL_IDS), + @Property(name = OAK_SECURITY_NAME, + propertyPrivate= true, + value = "org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal.ExternalPrincipalConfiguration") }) public class ExternalPrincipalConfiguration extends ConfigurationBase implements PrincipalConfiguration { diff --git a/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugConfiguration.java b/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugConfiguration.java index 98913bf..4c3fabc 100644 --- a/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugConfiguration.java +++ b/oak-authorization-cug/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/CugConfiguration.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.spi.security.authorization.cug.impl; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + import java.io.IOException; import java.io.InputStream; import java.security.Principal; @@ -89,7 +91,10 @@ @Property(name = CompositeConfiguration.PARAM_RANKING, label = "Ranking", description = "Ranking of this configuration in a setup with multiple authorization configurations.", - intValue = 200) + intValue = 200), + @Property(name = OAK_SECURITY_NAME, + propertyPrivate = true, + value = "org.apache.jackrabbit.oak.spi.security.authorization.cug.impl.CugConfiguration") }) public class CugConfiguration extends ConfigurationBase implements AuthorizationConfiguration, CugConstants { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenConfigurationImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenConfigurationImpl.java index 0dbc5eb..4b58618 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenConfigurationImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenConfigurationImpl.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.security.authentication.token; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + import java.security.Principal; import java.util.List; import java.util.Map; @@ -73,7 +75,10 @@ @Property(name = UserConstants.PARAM_PASSWORD_SALT_SIZE, label = "Hash Salt Size", description = "Size of the salt used to generate the hash.", - intValue = PasswordUtil.DEFAULT_SALT_SIZE) + intValue = PasswordUtil.DEFAULT_SALT_SIZE), + @Property(name = OAK_SECURITY_NAME, + propertyPrivate = true, + value = "org.apache.jackrabbit.oak.security.authentication.token.TokenConfigurationImpl") }) public class TokenConfigurationImpl extends ConfigurationBase implements TokenConfiguration { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java index 8dc7b5a..a0c14fb 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AuthorizationConfigurationImpl.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.security.authorization; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + import java.security.Principal; import java.util.ArrayList; import java.util.List; @@ -108,7 +110,10 @@ @Property(name = CompositeConfiguration.PARAM_RANKING, label = "Ranking", description = "Ranking of this configuration in a setup with multiple authorization configurations.", - intValue = 100) + intValue = 100), + @Property(name = OAK_SECURITY_NAME, + propertyPrivate = true, + value = "org.apache.jackrabbit.oak.security.authorization.AuthorizationConfigurationImpl") }) public class AuthorizationConfigurationImpl extends ConfigurationBase implements AuthorizationConfiguration { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/restriction/RestrictionProviderImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/restriction/RestrictionProviderImpl.java index cad3583..64eb625 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/restriction/RestrictionProviderImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/restriction/RestrictionProviderImpl.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.security.authorization.restriction; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -26,6 +28,7 @@ import com.google.common.collect.ImmutableMap; import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Property; import org.apache.felix.scr.annotations.Service; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Tree; @@ -58,6 +61,8 @@ */ @Component @Service(RestrictionProvider.class) +@Property(name = OAK_SECURITY_NAME, + value = "org.apache.jackrabbit.oak.security.authorization.restriction.RestrictionProviderImpl") public class RestrictionProviderImpl extends AbstractRestrictionProvider { private static final Logger log = LoggerFactory.getLogger(RestrictionProviderImpl.class); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java index c21021b..7d1a9be 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistration.java @@ -40,6 +40,7 @@ import org.apache.jackrabbit.oak.security.user.UserConfigurationImpl; import org.apache.jackrabbit.oak.spi.security.CompositeConfiguration; import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; +import org.apache.jackrabbit.oak.spi.security.RegistrationConstants; 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; @@ -68,6 +69,8 @@ import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Lists.newCopyOnWriteArrayList; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; +import static org.osgi.framework.Constants.OBJECTCLASS; @Component( immediate = true, @@ -78,11 +81,13 @@ @Properties({ @Property( name = "requiredServicePids", - label = "Required Service PIDs", + label = "Required Services", description = "The SecurityProvider will not register itself " + - "unless the services identified by these PIDs are " + - "registered first. Only the PIDs of implementations of " + - "the following interfaces are checked: " + + "unless the services identified by the following service pids " + + "or the oak.security.name properties are registered first. The class name is " + + "identified by checking the service.pid property. If that property " + + "does not exist, the oak.security.name property is used as a fallback." + + "Only implementations of the following interfaces are checked :" + "AuthorizationConfiguration, PrincipalConfiguration, " + "TokenConfiguration, AuthorizableActionProvider, " + "RestrictionProvider and UserAuthenticationFactory.", @@ -567,27 +572,31 @@ } private void addCandidate(Map properties) { - String pid = getServicePid(properties); + String pidOrName = getServicePidOrComponentName(properties); - if (pid == null) { + if (pidOrName == null) { return; } - preconditions.addCandidate(pid); + preconditions.addCandidate(pidOrName); } private void removeCandidate(Map properties) { - String pid = getServicePid(properties); + String pidOrName = getServicePidOrComponentName(properties); - if (pid == null) { + if (pidOrName == null) { return; } - preconditions.removeCandidate(pid); + preconditions.removeCandidate(pidOrName); } - private static String getServicePid(Map properties) { - return PropertiesUtil.toString(properties.get(Constants.SERVICE_PID), null); + private static String getServicePidOrComponentName(Map properties) { + String servicePid = PropertiesUtil.toString(properties.get(Constants.SERVICE_PID), null); + if ( servicePid != null ) { + return servicePid; + } + return PropertiesUtil.toString(properties.get(OAK_SECURITY_NAME), null); } private static String[] getRequiredServicePids(Map configuration) { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalConfigurationImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalConfigurationImpl.java index d2b666f..e553632 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalConfigurationImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalConfigurationImpl.java @@ -16,12 +16,15 @@ */ package org.apache.jackrabbit.oak.security.principal; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + import java.util.Map; import javax.annotation.Nonnull; import org.apache.felix.scr.annotations.Activate; import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Property; import org.apache.felix.scr.annotations.Service; import org.apache.jackrabbit.api.security.principal.PrincipalManager; import org.apache.jackrabbit.oak.api.Root; @@ -40,6 +43,8 @@ */ @Component() @Service({PrincipalConfiguration.class, SecurityConfiguration.class}) +@Property(name = OAK_SECURITY_NAME, + value = "org.apache.jackrabbit.oak.security.principal.PrincipalConfigurationImpl" ) public class PrincipalConfigurationImpl extends ConfigurationBase implements PrincipalConfiguration { @SuppressWarnings("UnusedDeclaration") diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/RandomAuthorizableNodeName.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/RandomAuthorizableNodeName.java index 5991c74..b4cfd90 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/RandomAuthorizableNodeName.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/RandomAuthorizableNodeName.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.security.user; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + import java.security.SecureRandom; import java.util.Map; import java.util.Random; @@ -35,6 +37,9 @@ */ @Component(metatype = true, label = "Apache Jackrabbit Oak Random Authorizable Node Name", description = "Generates a random name for the authorizable node.", policy = ConfigurationPolicy.REQUIRE) @Service(AuthorizableNodeName.class) +@Property(name = OAK_SECURITY_NAME, + propertyPrivate = true, + value = "org.apache.jackrabbit.oak.security.user.RandomAuthorizableNodeName") public class RandomAuthorizableNodeName implements AuthorizableNodeName { /** diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserAuthenticationFactoryImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserAuthenticationFactoryImpl.java index a9f4999..3fb915f 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserAuthenticationFactoryImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserAuthenticationFactoryImpl.java @@ -16,18 +16,23 @@ */ package org.apache.jackrabbit.oak.security.user; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Property; import org.apache.felix.scr.annotations.Service; import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.spi.security.authentication.Authentication; import org.apache.jackrabbit.oak.spi.security.user.UserAuthenticationFactory; import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - @Component @Service +@Property(name = OAK_SECURITY_NAME, + value = "org.apache.jackrabbit.oak.security.user.UserAuthenticationFactoryImpl") public class UserAuthenticationFactoryImpl implements UserAuthenticationFactory { @Nonnull diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/RegistrationConstants.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/RegistrationConstants.java new file mode 100644 index 0000000..2bd40c4 --- /dev/null +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/RegistrationConstants.java @@ -0,0 +1,36 @@ +/* + * 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; + +import org.apache.jackrabbit.oak.security.internal.SecurityProviderRegistration; + +/** + * Holds the names of well-known registration properties for security-related components + * + */ +public abstract class RegistrationConstants { + + /** + * Name to be used when registering components that are required by the {@link SecurityProviderRegistration} + */ + public static final String OAK_SECURITY_NAME = "oak.security.name"; + + private RegistrationConstants() { + + } +} diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java index 4ed997e..fc2fdc0 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/DefaultAuthorizableActionProvider.java @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.spi.security.user.action; +import static org.apache.jackrabbit.oak.spi.security.RegistrationConstants.OAK_SECURITY_NAME; + import java.util.List; import java.util.Map; import javax.annotation.Nonnull; @@ -60,7 +62,10 @@ cardinality = Integer.MAX_VALUE), @Property(name = PasswordValidationAction.CONSTRAINT, label = "Configure PasswordValidationAction: Password Constraint", - description = "A regular expression specifying the pattern that must be matched by a user's password.") + description = "A regular expression specifying the pattern that must be matched by a user's password."), + @Property(name = OAK_SECURITY_NAME, + propertyPrivate = true, + value = "org.apache.jackrabbit.oak.spi.security.user.action.DefaultAuthorizableActionProvider") }) public class DefaultAuthorizableActionProvider implements AuthorizableActionProvider { diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistrationTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistrationTest.java index dad8e97..9f05d73 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistrationTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistrationTest.java @@ -36,6 +36,7 @@ import org.apache.jackrabbit.oak.spi.security.CompositeConfiguration; import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters; import org.apache.jackrabbit.oak.spi.security.Context; +import org.apache.jackrabbit.oak.spi.security.RegistrationConstants; 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; @@ -92,11 +93,11 @@ assertEquals(isDefined, context.definesLocation(TreeLocation.create(tree))); } - private static T mockConfiguration(Class cl) { - SecurityConfiguration sc = Mockito.mock(cl); + private static T mockConfiguration(Class cl) { + T sc = Mockito.mock(cl); when(sc.getContext()).thenReturn(new ContextImpl()); when(sc.getParameters()).thenReturn(ConfigurationParameters.EMPTY); - return (T) sc; + return sc; } private static Map requiredServiceIdMap(@Nonnull String... ids) { @@ -566,6 +567,32 @@ RestrictionProvider rp = service.getConfiguration(AuthorizationConfiguration.class).getRestrictionProvider(); assertTrue(rp instanceof WhiteboardRestrictionProvider); } + + @Test + public void testActivateWithRequiredOakSecurityName() { + registration.activate(context.bundleContext(), requiredServiceIdMap("serviceId")); + + SecurityProvider service = context.getService(SecurityProvider.class); + assertNull(service); + + registration.bindAuthorizableNodeName(Mockito.mock(AuthorizableNodeName.class), ImmutableMap.of(RegistrationConstants.OAK_SECURITY_NAME, "serviceId")); + + service = context.getService(SecurityProvider.class); + assertNotNull(service); + } + + @Test + public void testActivateWithMixedServicePiAnddOakServiceName() { + registration.activate(context.bundleContext(), requiredServiceIdMap("rpId", "authorizationId")); + + RestrictionProvider mockRp = Mockito.mock(RestrictionProvider.class); + registration.bindRestrictionProvider(mockRp, ImmutableMap.of(Constants.SERVICE_PID, "rpId")); + registration.bindAuthorizationConfiguration(new AuthorizationConfigurationImpl(), ImmutableMap.of(RegistrationConstants.OAK_SECURITY_NAME, "authorizationId")); + + SecurityProvider service = context.getService(SecurityProvider.class); + RestrictionProvider rp = service.getConfiguration(AuthorizationConfiguration.class).getRestrictionProvider(); + assertTrue(rp instanceof WhiteboardRestrictionProvider); + } private static class ContextImpl implements Context { diff --git a/oak-doc/src/site/markdown/security/introduction.md b/oak-doc/src/site/markdown/security/introduction.md index 9cb0556..1390b0f 100644 --- a/oak-doc/src/site/markdown/security/introduction.md +++ b/oak-doc/src/site/markdown/security/introduction.md @@ -55,7 +55,8 @@ - [ConfigurationBase]: Abstract base implementation of the `SecurityConfiguration` interface. - [CompositeConfiguration]: Abstract base implementation for all composite configurations that allow for aggregation of multiple modules. - [ConfigurationParameters]: Utility used to pass around parameters and options. -- [Context]: Context information that allows to identify items defined and maintained by a give security module implementation. +- [Context]: Context information that allows to identify items defined and maintained by a give security module implementation. +- [RegistrationConstants]: Utility used to define well-known registration properties #### SecurityProvider @@ -190,7 +191,12 @@ | Parameter | Type | Default | Description | |--------------------------|----------|-----------|------------------------| -| `Required Service PIDs` | String[] | see below | Service references mandatory for the SecurityProvider registration. | +| `Required Services` | String[] | see below | Service references mandatory for the SecurityProvider registration. | + +The value of the individual configuration entries can be one of: + +- the value of the `service.pid` registration property +- the value of the `oak.security.name` registration property By default the `SecurityProviderRegistration` defines the following mandatory services. As long as these required references are not resolved the `SecurityProviderRegistration` @@ -315,6 +321,7 @@ [ConfigurationBase]: /oak/docs/apidocs/org/apache/jackrabbit/oak/spi/security/ConfigurationBase.html [ConfigurationParameters]: /oak/docs/apidocs/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.html [Context]: /oak/docs/apidocs/org/apache/jackrabbit/oak/spi/security/Context.html +[RegistrationConstants]: /oak/docs/apidocs/org/apache/jackrabbit/oak/spi/security/RegistrationConstants.html [AuthenticationConfiguration]: /oak/docs/apidocs/org/apache/jackrabbit/oak/spi/security/authentication/AuthenticationConfiguration.html [TokenConfiguration]: /oak/docs/apidocs/org/apache/jackrabbit/oak/spi/security/authentication/token/TokenConfiguration.html [AuthorizationConfiguration]: /oak/docs/apidocs/org/apache/jackrabbit/oak/spi/security/authorization/AuthorizationConfiguration.html