You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Angela Schreiber <an...@adobe.com> on 2015/09/18 16:48:17 UTC
Re: svn commit: r1703758 - in /jackrabbit/oak/trunk:
oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/
oak-core/src/main/java/org/apache/jackrabbit/oak/security/
oak-core/src/main/java/org/apache/jackrab...
hi francesco
thanks a lot for taking care of this issue! very much appreciated.
just a few comments:
as just discussed in private required configurationin
RandomAuthorizableNodeName
is mandatory. please don't remove it... just because CQ has this
configured by default doesn't mean that it's the default in oak
(it isn't for backwards compatibility as you can see in the
documentation).
second, i feel a bit uneasy about the compability break in
SecurityProviderImpl
which is a public class and exported... let's discuss that again
next week.
and finally... the SecurityProviderRegistration should have annotations
for the public methods that are nowhere used inside oak (SuppressWarning).
can you add those?
thanks a lot and have a nice weekend
angela
On 18/09/15 09:34, "frm@apache.org" <fr...@apache.org> wrote:
>Author: frm
>Date: Fri Sep 18 07:34:53 2015
>New Revision: 1703758
>
>URL: http://svn.apache.org/viewvc?rev=1703758&view=rev
>Log:
>OAK-3201 - Prevent SecurityProviderImpl to register unless its required
>dependencies are accessible
>
>Added:
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/Preconditions.java (with props)
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderRegistration.java (with props)
>
>jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/
>run/osgi/SecurityProviderRegistrationTest.groovy
>Modified:
>
>jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackra
>bbit/oak/spi/security/authorization/cug/impl/CugSecurityProvider.java
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderImpl.java
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/package-info.java
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/user/RandomAuthorizableNodeName.java
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/authentication/token/CompositeTokenConfiguration.java
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/authentication/token/package-info.java
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/principal/CompositePrincipalConfiguration.java
>
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/principal/package-info.java
> jackrabbit/oak/trunk/oak-pojosr/pom.xml
>
>Modified:
>jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackra
>bbit/oak/spi/security/authorization/cug/impl/CugSecurityProvider.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-authorization-cug/sr
>c/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/
>CugSecurityProvider.java?rev=1703758&r1=1703757&r2=1703758&view=diff
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackra
>bbit/oak/spi/security/authorization/cug/impl/CugSecurityProvider.java
>(original)
>+++
>jackrabbit/oak/trunk/oak-authorization-cug/src/test/java/org/apache/jackra
>bbit/oak/spi/security/authorization/cug/impl/CugSecurityProvider.java Fri
>Sep 18 07:34:53 2015
>@@ -33,12 +33,8 @@ final class CugSecurityProvider extends
> composite.setDefaultConfig(authorizationConfiguration);
> composite.addConfiguration(new CugConfiguration(this));
> composite.addConfiguration(authorizationConfiguration);
>- ((CugSecurityProvider)
>this).bindAuthorizationConfiguration(composite);
>+ setAuthorizationConfiguration(composite);
> }
> }
>
>- @Override
>- protected void bindAuthorizationConfiguration(@Nonnull
>AuthorizationConfiguration reference) {
>- super.bindAuthorizationConfiguration(reference);
>- }
> }
>\ No newline at end of file
>
>Added:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/Preconditions.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/security/Preconditions.java?rev=1703758&view=auto
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/Preconditions.java (added)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/Preconditions.java Fri Sep 18 07:34:53 2015
>@@ -0,0 +1,115 @@
>+/*
>+ * 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;
>+
>+import java.util.Set;
>+
>+import static com.google.common.collect.Sets.newHashSet;
>+
>+/**
>+ * Represents a preconditions set that may be satisfied by adding the
>right
>+ * candidates.
>+ * <p/>
>+ * Initially, a set of preconditions is empty. An empty set of
>preconditions is
>+ * always satisfied. If candidates are added, but the precondition set
>is empty,
>+ * the preconditions are considered satisfied.
>+ * <p/>
>+ * When some preconditions are added, the preconditions set may enter
>into the
>+ * unsatisfied state. In this case, the preconditions set may be come
>satisfied
>+ * again only with the addition of the right candidates.
>+ * <p/>
>+ * This class doesn't admit duplicates for preconditions or candidates.
>Adding
>+ * the same precondition (or candidate) twice doesn't have any effect on
>the
>+ * state of the preconditions set.
>+ */
>+class Preconditions {
>+
>+ private final Set<String> preconditions = newHashSet();
>+
>+ private final Set<String> candidates = newHashSet();
>+
>+ private boolean dirty = false;
>+
>+ private boolean satisfied = true;
>+
>+ /**
>+ * Add a precondition to this preconditions set. If the precondition
>already
>+ * belongs to this set, this operation has no effect.
>+ *
>+ * @param precondition The precondition to be added.
>+ */
>+ public void addPrecondition(String precondition) {
>+ if (preconditions.add(precondition)) {
>+ dirty = true;
>+ }
>+ }
>+
>+ /**
>+ * Remove all the preconditions to this set. This makes the set of
>+ * preconditions empty and, as such, satisfied.
>+ */
>+ public void clearPreconditions() {
>+ preconditions.clear();
>+ dirty = false;
>+ satisfied = true;
>+ }
>+
>+ /**
>+ * Add a candidate to this preconditions set. If the candidate
>already
>+ * belongs to this set, this operation has no effect.
>+ *
>+ * @param candidate The candidate to be added.
>+ */
>+ public void addCandidate(String candidate) {
>+ if (candidates.add(candidate)) {
>+ dirty = true;
>+ }
>+ }
>+
>+ /**
>+ * Remove a candidate from this preconditions set. If the candidate
>doesn't
>+ * belong to this set, this operation has no effect.
>+ *
>+ * @param candidate The candidate to be removed.
>+ */
>+ public void removeCandidate(String candidate) {
>+ if (candidates.remove(candidate)) {
>+ dirty = true;
>+ }
>+ }
>+
>+ /**
>+ * Check if the preconditions set are satisfied.
>+ *
>+ * @return {@code true} if the preconditions set is satisfied,
>{@code false}
>+ * otherwise.
>+ */
>+ public boolean areSatisfied() {
>+ if (dirty) {
>+ satisfied = candidates.containsAll(preconditions);
>+ dirty = false;
>+ }
>+
>+ return satisfied;
>+ }
>+
>+ @Override
>+ public String toString() {
>+ return String.format("Preconditions(preconditions = %s,
>candidates = %s)", preconditions, candidates);
>+ }
>+
>+}
>
>Propchange:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/Preconditions.java
>--------------------------------------------------------------------------
>----
> svn:eol-style = native
>
>Modified:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderImpl.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/security/SecurityProviderImpl.java?rev=1703758&r1
>=1703757&r2=1703758&view=diff
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderImpl.java (original)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderImpl.java Fri Sep 18 07:34:53 2015
>@@ -16,116 +16,94 @@
> */
> package org.apache.jackrabbit.oak.security;
>
>-import java.util.HashSet;
>-import java.util.Map;
>-import java.util.Set;
>-import javax.annotation.Nonnull;
>-import javax.annotation.Nullable;
>-
>-import com.google.common.collect.ImmutableMap;
>-import org.apache.felix.scr.annotations.Activate;
>-import org.apache.felix.scr.annotations.Component;
>-import org.apache.felix.scr.annotations.Deactivate;
>-import org.apache.felix.scr.annotations.Reference;
>-import org.apache.felix.scr.annotations.ReferenceCardinality;
>-import org.apache.felix.scr.annotations.ReferencePolicy;
>-import org.apache.felix.scr.annotations.Service;
>-import org.apache.jackrabbit.oak.osgi.OsgiWhiteboard;
> import
>org.apache.jackrabbit.oak.security.authentication.AuthenticationConfigurat
>ionImpl;
> import
>org.apache.jackrabbit.oak.security.authentication.token.TokenConfiguration
>Impl;
> import
>org.apache.jackrabbit.oak.security.authorization.AuthorizationConfiguratio
>nImpl;
> import
>org.apache.jackrabbit.oak.security.principal.PrincipalConfigurationImpl;
> import
>org.apache.jackrabbit.oak.security.privilege.PrivilegeConfigurationImpl;
> 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.AuthenticationConfig
>uration;
>-import
>org.apache.jackrabbit.oak.spi.security.authentication.token.CompositeToken
>Configuration;
> import
>org.apache.jackrabbit.oak.spi.security.authentication.token.TokenConfigura
>tion;
> import
>org.apache.jackrabbit.oak.spi.security.authorization.AuthorizationConfigur
>ation;
>-import
>org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessC
>ontrolConstants;
>-import
>org.apache.jackrabbit.oak.spi.security.principal.CompositePrincipalConfigu
>ration;
> import
>org.apache.jackrabbit.oak.spi.security.principal.PrincipalConfiguration;
> import
>org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConfiguration;
> import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
>-import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
> import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
>-import
>org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardAuthorizableActionProvi
>der;
>-import
>org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardAuthorizableNodeName;
> import org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardAware;
>-import
>org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardRestrictionProvider;
>-import
>org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUserAuthenticationFacto
>ry;
>-import org.osgi.framework.BundleContext;
>+
>+import javax.annotation.Nonnull;
>+import javax.annotation.Nullable;
>
> import static com.google.common.base.Preconditions.checkNotNull;
>+import static com.google.common.collect.Sets.newHashSet;
>
>-@Component
>-@Service(value = {SecurityProvider.class})
> public class SecurityProviderImpl implements SecurityProvider,
>WhiteboardAware {
>
>- @Reference
>- private volatile AuthorizationConfiguration
>authorizationConfiguration;
>+ private AuthenticationConfiguration authenticationConfiguration;
>
>- @Reference
>- private volatile AuthenticationConfiguration
>authenticationConfiguration;
>+ private AuthorizationConfiguration authorizationConfiguration;
>
>- @Reference
>- private volatile PrivilegeConfiguration privilegeConfiguration;
>+ private UserConfiguration userConfiguration;
>
>- @Reference
>- private volatile UserConfiguration userConfiguration;
>-
>- @Reference(referenceInterface = PrincipalConfiguration.class,
>- name = "principalConfiguration",
>- bind = "bindPrincipalConfiguration",
>- unbind = "unbindPrincipalConfiguration",
>- policy = ReferencePolicy.DYNAMIC,
>- cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE)
>- private final CompositePrincipalConfiguration principalConfiguration
>= new CompositePrincipalConfiguration(this);
>-
>- @Reference(referenceInterface = TokenConfiguration.class,
>- name = "tokenConfiguration",
>- bind = "bindTokenConfiguration",
>- unbind = "unbindTokenConfiguration",
>- policy = ReferencePolicy.DYNAMIC,
>- cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE)
>- private final CompositeTokenConfiguration tokenConfiguration = new
>CompositeTokenConfiguration(this);
>-
>- private final WhiteboardAuthorizableNodeName authorizableNodeName =
>new WhiteboardAuthorizableNodeName();
>- private final WhiteboardAuthorizableActionProvider
>authorizableActionProvider = new WhiteboardAuthorizableActionProvider();
>- private final WhiteboardRestrictionProvider restrictionProvider =
>new WhiteboardRestrictionProvider();
>- private final WhiteboardUserAuthenticationFactory
>userAuthenticationFactory = new
>WhiteboardUserAuthenticationFactory(UserConfigurationImpl.getDefaultAuthen
>ticationFactory());
>+ private PrincipalConfiguration principalConfiguration;
>+
>+ private PrivilegeConfiguration privilegeConfiguration;
>+
>+ private TokenConfiguration tokenConfiguration;
>
> private ConfigurationParameters configuration;
>
> private Whiteboard whiteboard;
>
> /**
>- * Default constructor used in OSGi environments.
>+ * Default constructor using an empty configuration and default
>+ * implementations for the security configurations.
> */
> public SecurityProviderImpl() {
> this(ConfigurationParameters.EMPTY);
> }
>
> /**
>- * Create a new {@code SecurityProvider} instance with the given
>configuration
>- * parameters.
>+ * Create a new {@code SecurityProvider} instance with the given
>+ * configuration parameters.
> *
> * @param configuration security configuration
> */
> public SecurityProviderImpl(@Nonnull ConfigurationParameters
>configuration) {
>- checkNotNull(configuration);
>- this.configuration = configuration;
>+ this.configuration = checkNotNull(configuration);
>+ this.authenticationConfiguration = new
>AuthenticationConfigurationImpl(this);
>+ this.authorizationConfiguration = new
>AuthorizationConfigurationImpl(this);
>+ this.userConfiguration = new UserConfigurationImpl(this);
>+ this.principalConfiguration = new
>PrincipalConfigurationImpl(this);
>+ this.privilegeConfiguration = new PrivilegeConfigurationImpl();
>+ this.tokenConfiguration = new TokenConfigurationImpl(this);
>+ }
>+
>+ protected void
>setAuthenticationConfiguration(AuthenticationConfiguration
>authenticationConfiguration) {
>+ this.authenticationConfiguration =
>checkNotNull(authenticationConfiguration);
>+ }
>+
>+ protected void
>setAuthorizationConfiguration(AuthorizationConfiguration
>authorizationConfiguration) {
>+ this.authorizationConfiguration = authorizationConfiguration;
>+ }
>+
>+ protected void setUserConfiguration(UserConfiguration
>userConfiguration) {
>+ this.userConfiguration = userConfiguration;
>+ }
>
>- authenticationConfiguration = new
>AuthenticationConfigurationImpl(this);
>- authorizationConfiguration = new
>AuthorizationConfigurationImpl(this);
>- userConfiguration = new UserConfigurationImpl(this);
>- privilegeConfiguration = new PrivilegeConfigurationImpl();
>+ protected void setPrincipalConfiguration(PrincipalConfiguration
>principalConfiguration) {
>+ this.principalConfiguration = principalConfiguration;
>+ }
>+
>+ protected void setPrivilegeConfiguration(PrivilegeConfiguration
>privilegeConfiguration) {
>+ this.privilegeConfiguration = privilegeConfiguration;
>+ }
>
>- principalConfiguration.setDefaultConfig(new
>PrincipalConfigurationImpl(this));
>- tokenConfiguration.setDefaultConfig(new
>TokenConfigurationImpl(this));
>+ protected void setTokenConfiguration(TokenConfiguration
>tokenConfiguration) {
>+ this.tokenConfiguration = tokenConfiguration;
> }
>
> @Override
>@@ -144,7 +122,9 @@ public class SecurityProviderImpl implem
> if (name == null) {
> return configuration;
> }
>+
> ConfigurationParameters params =
>configuration.getConfigValue(name, ConfigurationParameters.EMPTY);
>+
> for (SecurityConfiguration sc : getConfigurations()) {
> if (sc != null && sc.getName().equals(name)) {
> return ConfigurationParameters.of(params,
>sc.getParameters());
>@@ -156,14 +136,14 @@ public class SecurityProviderImpl implem
> @Nonnull
> @Override
> public Iterable<? extends SecurityConfiguration> getConfigurations()
>{
>- Set<SecurityConfiguration> scs = new
>HashSet<SecurityConfiguration>();
>- scs.add(authenticationConfiguration);
>- scs.add(authorizationConfiguration);
>- scs.add(userConfiguration);
>- scs.add(principalConfiguration);
>- scs.add(privilegeConfiguration);
>- scs.add(tokenConfiguration);
>- return scs;
>+ return newHashSet(
>+ authenticationConfiguration,
>+ authorizationConfiguration,
>+ userConfiguration,
>+ principalConfiguration,
>+ privilegeConfiguration,
>+ tokenConfiguration
>+ );
> }
>
> @SuppressWarnings("unchecked")
>@@ -187,89 +167,4 @@ public class SecurityProviderImpl implem
> }
> }
>
>- //----------------------------------------------------------------<
>SCR >---
>- @Activate
>- protected void activate(BundleContext context) {
>- whiteboard = new OsgiWhiteboard(context);
>- authorizableActionProvider.start(whiteboard);
>- authorizableNodeName.start(whiteboard);
>- restrictionProvider.start(whiteboard);
>- userAuthenticationFactory.start(whiteboard);
>-
>- initializeConfigurations();
>- }
>-
>- @Deactivate
>- protected void deactivate() {
>- authorizableActionProvider.stop();
>- authorizableNodeName.stop();
>- restrictionProvider.stop();
>- userAuthenticationFactory.stop();
>- }
>-
>- @SuppressWarnings("UnusedDeclaration")
>- protected void bindPrincipalConfiguration(@Nonnull
>PrincipalConfiguration reference) {
>-
>principalConfiguration.addConfiguration(initConfiguration(reference));
>- }
>-
>- @SuppressWarnings("UnusedDeclaration")
>- protected void unbindPrincipalConfiguration(@Nonnull
>PrincipalConfiguration reference) {
>- principalConfiguration.removeConfiguration(reference);
>- }
>-
>- @SuppressWarnings("UnusedDeclaration")
>- protected void bindTokenConfiguration(@Nonnull TokenConfiguration
>reference) {
>-
>tokenConfiguration.addConfiguration(initConfiguration(reference));
>- }
>-
>- @SuppressWarnings("UnusedDeclaration")
>- protected void unbindTokenConfiguration(@Nonnull TokenConfiguration
>reference) {
>- tokenConfiguration.removeConfiguration(reference);
>- }
>-
>- @SuppressWarnings("UnusedDeclaration")
>- protected void bindAuthorizationConfiguration(@Nonnull
>AuthorizationConfiguration reference) {
>- authorizationConfiguration = initConfiguration(reference);
>- // TODO (OAK-1268):
>authorizationConfiguration.addConfiguration(initConfiguration(reference));
>- }
>-
>- @SuppressWarnings("UnusedDeclaration")
>- protected void unbindAuthorizationConfiguration(@Nonnull
>AuthorizationConfiguration reference) {
>- authorizationConfiguration = new
>AuthorizationConfigurationImpl(this);
>- // TODO (OAK-1268):
>authorizationConfiguration.removeConfiguration(reference);
>- }
>-
>- //------------------------------------------------------------<
>private >---
>- private void initializeConfigurations() {
>- initConfiguration(authorizationConfiguration,
>ConfigurationParameters.of(
>- AccessControlConstants.PARAM_RESTRICTION_PROVIDER,
>restrictionProvider)
>- );
>-
>- Map<String, Object> userMap = ImmutableMap.<String,Object>of(
>- UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER,
>authorizableActionProvider,
>- UserConstants.PARAM_AUTHORIZABLE_NODE_NAME,
>authorizableNodeName,
>- UserConstants.PARAM_USER_AUTHENTICATION_FACTORY,
>userAuthenticationFactory);
>- initConfiguration(userConfiguration,
>ConfigurationParameters.of(userMap));
>-
>- initConfiguration(authenticationConfiguration);
>- initConfiguration(privilegeConfiguration);
>- }
>-
>- private <T extends SecurityConfiguration> T
>initConfiguration(@Nonnull T config) {
>- if (config instanceof ConfigurationBase) {
>- ConfigurationBase cfg = (ConfigurationBase) config;
>- cfg.setSecurityProvider(this);
>-
>cfg.setParameters(ConfigurationParameters.of(ConfigurationParameters.EMPTY
>, cfg.getParameters()));
>- }
>- return config;
>- }
>-
>- private <T extends SecurityConfiguration> T
>initConfiguration(@Nonnull T config, @Nonnull ConfigurationParameters
>params) {
>- if (config instanceof ConfigurationBase) {
>- ConfigurationBase cfg = (ConfigurationBase) config;
>- cfg.setSecurityProvider(this);
>- cfg.setParameters(ConfigurationParameters.of(params,
>cfg.getParameters()));
>- }
>- return config;
>- }
> }
>
>Added:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderRegistration.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/security/SecurityProviderRegistration.java?rev=17
>03758&view=auto
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderRegistration.java (added)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderRegistration.java Fri Sep 18 07:34:53 2015
>@@ -0,0 +1,622 @@
>+/*
>+ * 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;
>+
>+import org.apache.felix.scr.annotations.Activate;
>+import org.apache.felix.scr.annotations.Component;
>+import org.apache.felix.scr.annotations.Deactivate;
>+import org.apache.felix.scr.annotations.Modified;
>+import org.apache.felix.scr.annotations.Properties;
>+import org.apache.felix.scr.annotations.Property;
>+import org.apache.felix.scr.annotations.PropertyUnbounded;
>+import org.apache.felix.scr.annotations.Reference;
>+import org.apache.felix.scr.annotations.ReferenceCardinality;
>+import org.apache.felix.scr.annotations.ReferencePolicy;
>+import org.apache.felix.scr.annotations.References;
>+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.AuthenticationConfig
>uration;
>+import
>org.apache.jackrabbit.oak.spi.security.authentication.token.CompositeToken
>Configuration;
>+import
>org.apache.jackrabbit.oak.spi.security.authentication.token.TokenConfigura
>tion;
>+import
>org.apache.jackrabbit.oak.spi.security.authorization.AuthorizationConfigur
>ation;
>+import
>org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessC
>ontrolConstants;
>+import
>org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restricti
>onProvider;
>+import
>org.apache.jackrabbit.oak.spi.security.principal.CompositePrincipalConfigu
>ration;
>+import
>org.apache.jackrabbit.oak.spi.security.principal.PrincipalConfiguration;
>+import
>org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConfiguration;
>+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.UserConstants;
>+import
>org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableActionProvi
>der;
>+import
>org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardAuthorizableActionProvi
>der;
>+import
>org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardAuthorizableNodeName;
>+import
>org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardRestrictionProvider;
>+import
>org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUserAuthenticationFacto
>ry;
>+import org.osgi.framework.BundleContext;
>+import org.osgi.framework.Constants;
>+import org.osgi.framework.ServiceRegistration;
>+import org.slf4j.Logger;
>+import org.slf4j.LoggerFactory;
>+
>+import java.util.ArrayList;
>+import java.util.List;
>+import java.util.Map;
>+
>+import static com.google.common.collect.Lists.newArrayList;
>+import static com.google.common.collect.Lists.newCopyOnWriteArrayList;
>+
>+@Component(
>+ immediate = true,
>+ metatype = true,
>+ label = "Apache Jackrabbit Oak SecurityProvider",
>+ description = "The default SecurityProvider embedded in Apache
>Jackrabbit Oak"
>+)
>+@Properties({
>+ @Property(
>+ name = "requiredServicePids",
>+ label = "Required service PIDs",
>+ 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: " +
>+ "PrincipalConfiguration, TokenConfiguration, " +
>+ "AuthorizableNodeName,
>AuthorizableActionProvider, " +
>+ "RestrictionProvider and
>UserAuthenticationFactory.",
>+ value = {
>+
>"org.apache.jackrabbit.oak.security.principal.PrincipalConfigurationImpl",
>+
>"org.apache.jackrabbit.oak.security.authentication.token.TokenConfiguratio
>nImpl",
>+
>"org.apache.jackrabbit.oak.security.user.RandomAuthorizableNodeName",
>+
>"org.apache.jackrabbit.oak.spi.security.user.action.DefaultAuthorizableAct
>ionProvider",
>+
>"org.apache.jackrabbit.oak.security.authorization.restriction.RestrictionP
>roviderImpl",
>+
>"org.apache.jackrabbit.oak.security.user.UserAuthenticationFactoryImpl"
>+ },
>+ unbounded = PropertyUnbounded.ARRAY
>+ )
>+})
>+@References({
>+ @Reference(
>+ name = "principalConfiguration",
>+ referenceInterface = PrincipalConfiguration.class,
>+ cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE,
>+ policy = ReferencePolicy.DYNAMIC
>+ ),
>+ @Reference(
>+ name = "tokenConfiguration",
>+ referenceInterface = TokenConfiguration.class,
>+ cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE,
>+ policy = ReferencePolicy.DYNAMIC
>+ ),
>+ @Reference(
>+ name = "authorizableNodeName",
>+ referenceInterface = AuthorizableNodeName.class,
>+ cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE,
>+ policy = ReferencePolicy.DYNAMIC
>+ ),
>+ @Reference(
>+ name = "authorizableActionProvider",
>+ referenceInterface = AuthorizableActionProvider.class,
>+ cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE,
>+ policy = ReferencePolicy.DYNAMIC
>+ ),
>+ @Reference(
>+ name = "restrictionProvider",
>+ referenceInterface = RestrictionProvider.class,
>+ cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE,
>+ policy = ReferencePolicy.DYNAMIC
>+ ),
>+ @Reference(
>+ name = "userAuthenticationFactory",
>+ referenceInterface = UserAuthenticationFactory.class,
>+ cardinality = ReferenceCardinality.OPTIONAL_MULTIPLE,
>+ policy = ReferencePolicy.DYNAMIC
>+ )
>+})
>+public class SecurityProviderRegistration {
>+
>+ private static final Logger log =
>LoggerFactory.getLogger(SecurityProviderRegistration.class);
>+
>+ @Reference
>+ private AuthorizationConfiguration authorizationConfiguration;
>+
>+ @Reference
>+ private AuthenticationConfiguration authenticationConfiguration;
>+
>+ @Reference
>+ private PrivilegeConfiguration privilegeConfiguration;
>+
>+ @Reference
>+ private UserConfiguration userConfiguration;
>+
>+ private BundleContext context;
>+
>+ private ServiceRegistration registration;
>+
>+ private boolean registering;
>+
>+ private final Preconditions preconditions = new Preconditions();
>+
>+ private final List<PrincipalConfiguration> principalConfigurations =
>newCopyOnWriteArrayList();
>+
>+ private final List<TokenConfiguration> tokenConfigurations =
>newCopyOnWriteArrayList();
>+
>+ private final List<AuthorizableNodeName> authorizableNodeNames =
>newCopyOnWriteArrayList();
>+
>+ private final List<AuthorizableActionProvider>
>authorizableActionProviders = newCopyOnWriteArrayList();
>+
>+ private final List<RestrictionProvider> restrictionProviders =
>newCopyOnWriteArrayList();
>+
>+ private final List<UserAuthenticationFactory>
>userAuthenticationFactories = newCopyOnWriteArrayList();
>+
>+ @Activate
>+ public void activate(BundleContext context, Map<String, Object>
>configuration) {
>+ String[] requiredServicePids =
>getRequiredServicePids(configuration);
>+
>+ synchronized (this) {
>+ for (String pid : requiredServicePids) {
>+ preconditions.addPrecondition(pid);
>+ }
>+
>+ this.context = context;
>+ }
>+
>+ maybeRegister();
>+ }
>+
>+ @Modified
>+ public void modified(Map<String, Object> configuration) {
>+ String[] requiredServicePids =
>getRequiredServicePids(configuration);
>+
>+ synchronized (this) {
>+ preconditions.clearPreconditions();
>+
>+ for (String pid : requiredServicePids) {
>+ preconditions.addPrecondition(pid);
>+ }
>+ }
>+
>+ maybeUnregister();
>+ maybeRegister();
>+ }
>+
>+ @Deactivate
>+ public void deactivate() {
>+ ServiceRegistration registration;
>+
>+ synchronized (this) {
>+ registration = this.registration;
>+
>+ this.registration = null;
>+ this.registering = false;
>+ this.context = null;
>+
>+ this.preconditions.clearPreconditions();
>+ }
>+
>+ if (registration != null) {
>+ registration.unregister();
>+ }
>+ }
>+
>+ public void
>bindAuthorizationConfiguration(AuthorizationConfiguration
>authorizationConfiguration) {
>+ this.authorizationConfiguration = authorizationConfiguration;
>+ }
>+
>+ public void
>unbindAuthorizationConfiguration(AuthorizationConfiguration
>authorizationConfiguration) {
>+ this.authorizationConfiguration = null;
>+ }
>+
>+ public void
>bindAuthenticationConfiguration(AuthenticationConfiguration
>authenticationConfiguration) {
>+ this.authenticationConfiguration = authenticationConfiguration;
>+ }
>+
>+ public void
>unbindAuthenticationConfiguration(AuthenticationConfiguration
>authenticationConfiguration) {
>+ this.authenticationConfiguration = null;
>+ }
>+
>+ public void bindPrivilegeConfiguration(PrivilegeConfiguration
>privilegeConfiguration) {
>+ this.privilegeConfiguration = privilegeConfiguration;
>+ }
>+
>+ public void unbindPrivilegeConfiguration(PrivilegeConfiguration
>privilegeConfiguration) {
>+ this.privilegeConfiguration = null;
>+ }
>+
>+ public void bindUserConfiguration(UserConfiguration
>userConfiguration) {
>+ this.userConfiguration = userConfiguration;
>+ }
>+
>+ public void unbindUserConfiguration(UserConfiguration
>userConfiguration) {
>+ this.userConfiguration = null;
>+ }
>+
>+ public void bindPrincipalConfiguration(PrincipalConfiguration
>principalConfiguration, Map<String, Object> properties) {
>+ synchronized (this) {
>+ principalConfigurations.add(principalConfiguration);
>+ addCandidate(properties);
>+ }
>+
>+ maybeRegister();
>+ }
>+
>+ public void unbindPrincipalConfiguration(PrincipalConfiguration
>principalConfiguration, Map<String, Object> properties) {
>+ synchronized (this) {
>+ principalConfigurations.remove(principalConfiguration);
>+ removeCandidate(properties);
>+ }
>+
>+ maybeUnregister();
>+ }
>+
>+ public void bindTokenConfiguration(TokenConfiguration
>tokenConfiguration, Map<String, Object> properties) {
>+ synchronized (this) {
>+ tokenConfigurations.add(tokenConfiguration);
>+ addCandidate(properties);
>+ }
>+
>+ maybeRegister();
>+ }
>+
>+ public void unbindTokenConfiguration(TokenConfiguration
>tokenConfiguration, Map<String, Object> properties) {
>+ synchronized (this) {
>+ tokenConfigurations.remove(tokenConfiguration);
>+ removeCandidate(properties);
>+ }
>+
>+ maybeUnregister();
>+ }
>+
>+ public void bindAuthorizableNodeName(AuthorizableNodeName
>authorizableNodeName, Map<String, Object> properties) {
>+ synchronized (this) {
>+ authorizableNodeNames.add(authorizableNodeName);
>+ addCandidate(properties);
>+ }
>+
>+ maybeRegister();
>+ }
>+
>+ public void unbindAuthorizableNodeName(AuthorizableNodeName
>authorizableNodeName, Map<String, Object> properties) {
>+ synchronized (this) {
>+ authorizableNodeNames.remove(authorizableNodeName);
>+ removeCandidate(properties);
>+ }
>+
>+ maybeUnregister();
>+ }
>+
>+ public void
>bindAuthorizableActionProvider(AuthorizableActionProvider
>authorizableActionProvider, Map<String, Object> properties) {
>+ synchronized (this) {
>+ authorizableActionProviders.add(authorizableActionProvider);
>+ addCandidate(properties);
>+ }
>+
>+ maybeRegister();
>+ }
>+
>+ public void
>unbindAuthorizableActionProvider(AuthorizableActionProvider
>authorizableActionProvider, Map<String, Object> properties) {
>+ synchronized (this) {
>+
>authorizableActionProviders.remove(authorizableActionProvider);
>+ removeCandidate(properties);
>+ }
>+
>+ maybeUnregister();
>+ }
>+
>+ public void bindRestrictionProvider(RestrictionProvider
>restrictionProvider, Map<String, Object> properties) {
>+ synchronized (this) {
>+ restrictionProviders.add(restrictionProvider);
>+ addCandidate(properties);
>+ }
>+
>+ maybeRegister();
>+ }
>+
>+ public void unbindRestrictionProvider(RestrictionProvider
>restrictionProvider, Map<String, Object> properties) {
>+ synchronized (this) {
>+ restrictionProviders.remove(restrictionProvider);
>+ removeCandidate(properties);
>+ }
>+
>+ maybeUnregister();
>+ }
>+
>+ public void bindUserAuthenticationFactory(UserAuthenticationFactory
>userAuthenticationFactory, Map<String, Object> properties) {
>+ synchronized (this) {
>+ userAuthenticationFactories.add(userAuthenticationFactory);
>+ addCandidate(properties);
>+ }
>+
>+ maybeRegister();
>+ }
>+
>+ public void
>unbindUserAuthenticationFactory(UserAuthenticationFactory
>userAuthenticationFactory, Map<String, Object> properties) {
>+ synchronized (this) {
>+
>userAuthenticationFactories.remove(userAuthenticationFactory);
>+ removeCandidate(properties);
>+ }
>+
>+ maybeUnregister();
>+ }
>+
>+ private void maybeRegister() {
>+ BundleContext context;
>+
>+ log.info("Trying to register a SecurityProvider...");
>+
>+ synchronized (this) {
>+
>+ // The component is not activated, yet. We have no means of
>registering
>+ // the SecurityProvider. This method will be called again
>after
>+ // activation completes.
>+
>+ if (this.context == null) {
>+ log.info("Aborting: no BundleContext is available");
>+ return;
>+ }
>+
>+ // The preconditions are not satisifed. This may happen when
>this
>+ // component is activated but not enough mandatory services
>are bound
>+ // to it.
>+
>+ if (!preconditions.areSatisfied()) {
>+ log.info("Aborting: preconditions are not satisfied:
>{}", preconditions);
>+ return;
>+ }
>+
>+ // The SecurityProvider is already registered. This may
>happen when a
>+ // new dependency is added to this component, but the
>requirements are
>+ // already satisfied.
>+
>+ if (registration != null) {
>+ log.info("Aborting: a SecurityProvider is already
>registered");
>+ return;
>+ }
>+
>+ // If the component is in the process of registering an
>instance of
>+ // SecurityProvider, return. This check is necessary because
>we don't
>+ // want to call createSecurityProvider() more than once.
>That method,
>+ // in fact, changes the state of the bound dependencies (it
>sets a
>+ // back-reference from the security configurations to the new
>+ // SecurityProvider). We want those dependencies to change
>state only
>+ // when we are sure that we will register the
>SecurityProvider we
>+ // are creating.
>+
>+ if (registering) {
>+ log.info("Aborting: a SecurityProvider is already being
>registered");
>+ return;
>+ }
>+
>+ // Mark the start of a registration process.
>+
>+ registering = true;
>+
>+ // Save the BundleContext for local usage.
>+
>+ context = this.context;
>+ }
>+
>+ // Register the SecurityProvider.
>+
>+ ServiceRegistration registration = context.registerService(
>+ SecurityProvider.class.getName(),
>+ createSecurityProvider(context),
>+ null
>+ );
>+
>+ synchronized (this) {
>+ this.registration = registration;
>+ this.registering = false;
>+ }
>+
>+ log.info("SecurityProvider instance registered");
>+ }
>+
>+ private void maybeUnregister() {
>+ ServiceRegistration registration;
>+
>+ log.info("Trying to unregister the SecurityProvider...");
>+
>+ synchronized (this) {
>+
>+ // If there is nothing to register, we obviously have
>nothing to do.
>+
>+ if (this.registration == null) {
>+ log.info("Aborting: no SecurityProvider is registered");
>+ return;
>+ }
>+
>+ // The preconditions are not satisfied. This may happen when
>a
>+ // dependency is unbound from the current component.
>+
>+ if (preconditions.areSatisfied()) {
>+ log.info("Aborting: preconditions are satisfied");
>+ return;
>+ }
>+
>+ // Save the ServiceRegistration for local use.
>+
>+ registration = this.registration;
>+ this.registration = null;
>+ }
>+
>+ registration.unregister();
>+
>+ log.info("SecurityProvider instance unregistered");
>+ }
>+
>+ private SecurityProvider createSecurityProvider(BundleContext
>context) {
>+ SecurityProviderImpl securityProvider = new
>SecurityProviderImpl();
>+
>+ // Static, mandatory references
>+
>+
>securityProvider.setAuthenticationConfiguration(initializeConfiguration(se
>curityProvider, authenticationConfiguration));
>+
>securityProvider.setAuthorizationConfiguration(initializeConfiguration(sec
>urityProvider, authorizationConfiguration));
>+
>securityProvider.setUserConfiguration(initializeConfiguration(securityProv
>ider, userConfiguration));
>+
>securityProvider.setPrivilegeConfiguration(initializeConfiguration(securit
>yProvider, privilegeConfiguration));
>+
>+ // Multiple, dynamic references
>+
>+
>securityProvider.setPrincipalConfiguration(createCompositePrincipalConfigu
>ration(securityProvider));
>+
>securityProvider.setTokenConfiguration(createCompositeTokenConfiguration(s
>ecurityProvider));
>+
>+ // Whiteboard
>+
>+ securityProvider.setWhiteboard(new OsgiWhiteboard(context));
>+
>+ return securityProvider;
>+ }
>+
>+ private PrincipalConfiguration
>createCompositePrincipalConfiguration(SecurityProvider securityProvider) {
>+ return new CompositePrincipalConfiguration(securityProvider) {
>+
>+ @Override
>+ protected List<PrincipalConfiguration> getConfigurations() {
>+ ArrayList<PrincipalConfiguration> configurations =
>newArrayList(newArrayList(principalConfigurations));
>+
>+ for (PrincipalConfiguration configuration :
>configurations) {
>+ initializeConfiguration(getSecurityProvider(),
>configuration);
>+ }
>+
>+ return configurations;
>+ }
>+
>+ };
>+ }
>+
>+ private TokenConfiguration
>createCompositeTokenConfiguration(SecurityProvider securityProvider) {
>+ return new CompositeTokenConfiguration(securityProvider) {
>+
>+ @Override
>+ protected List<TokenConfiguration> getConfigurations() {
>+ List<TokenConfiguration> configurations =
>newArrayList(tokenConfigurations);
>+
>+ for (TokenConfiguration configuration : configurations) {
>+ initializeConfiguration(getSecurityProvider(),
>configuration);
>+ }
>+
>+ return configurations;
>+ }
>+
>+ };
>+ }
>+
>+ private AuthorizationConfiguration
>initializeConfiguration(SecurityProvider securityProvider,
>AuthorizationConfiguration authorizationConfiguration) {
>+ return initializeConfiguration(securityProvider,
>authorizationConfiguration, ConfigurationParameters.of(
>+ AccessControlConstants.PARAM_RESTRICTION_PROVIDER,
>createCompositeRestrictionProvider()
>+ ));
>+ }
>+
>+ private UserConfiguration initializeConfiguration(SecurityProvider
>securityProvider, UserConfiguration userConfiguration) {
>+ return initializeConfiguration(securityProvider,
>userConfiguration, ConfigurationParameters.of(
>+
>ConfigurationParameters.of(UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDE
>R, 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() {
>+
>+ @Override
>+ protected List<RestrictionProvider> getServices() {
>+ return newArrayList(restrictionProviders);
>+ }
>+
>+ };
>+ }
>+
>+ private AuthorizableActionProvider
>createCompositeAuthorizableActionProvider() {
>+ return new WhiteboardAuthorizableActionProvider() {
>+
>+ @Override
>+ protected List<AuthorizableActionProvider> getServices() {
>+ return newArrayList(authorizableActionProviders);
>+ }
>+
>+ };
>+ }
>+
>+ private AuthorizableNodeName createCompositeAuthorizableNodeName() {
>+ return new WhiteboardAuthorizableNodeName() {
>+
>+ @Override
>+ protected List<AuthorizableNodeName> getServices() {
>+ return newArrayList(authorizableNodeNames);
>+ }
>+
>+ };
>+ }
>+
>+ private UserAuthenticationFactory
>createCompositeUserAuthenticationFactory() {
>+ return new
>WhiteboardUserAuthenticationFactory(UserConfigurationImpl.getDefaultAuthen
>ticationFactory()) {
>+
>+ @Override
>+ protected List<UserAuthenticationFactory> getServices() {
>+ return newArrayList(userAuthenticationFactories);
>+ }
>+
>+ };
>+ }
>+
>+ private void addCandidate(Map<String, Object> properties) {
>+ String pid = getServicePid(properties);
>+
>+ if (pid == null) {
>+ return;
>+ }
>+
>+ preconditions.addCandidate(pid);
>+ }
>+
>+ private void removeCandidate(Map<String, Object> properties) {
>+ String pid = getServicePid(properties);
>+
>+ if (pid == null) {
>+ return;
>+ }
>+
>+ preconditions.removeCandidate(pid);
>+ }
>+
>+ private String getServicePid(Map<String, Object> properties) {
>+ return
>PropertiesUtil.toString(properties.get(Constants.SERVICE_PID), null);
>+ }
>+
>+ private String[] getRequiredServicePids(Map<String, Object>
>configuration) {
>+ return
>PropertiesUtil.toStringArray(configuration.get("requiredServicePids"),
>new String[]{});
>+ }
>+
>+}
>
>Propchange:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/SecurityProviderRegistration.java
>--------------------------------------------------------------------------
>----
> svn:eol-style = native
>
>Modified:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/package-info.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/security/package-info.java?rev=1703758&r1=1703757
>&r2=1703758&view=diff
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/package-info.java (original)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/package-info.java Fri Sep 18 07:34:53 2015
>@@ -20,7 +20,7 @@
> *
> * See <a href="README.md">README.md</a> for more details.
> */
>-@Version("1.0.1")
>+@Version("2.0.0")
> @Export(optional = "provide:=true")
> package org.apache.jackrabbit.oak.security;
>
>
>Modified:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/user/RandomAuthorizableNodeName.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/security/user/RandomAuthorizableNodeName.java?rev
>=1703758&r1=1703757&r2=1703758&view=diff
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/user/RandomAuthorizableNodeName.java (original)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/user/RandomAuthorizableNodeName.java Fri Sep 18 07:34:53 2015
>@@ -33,7 +33,7 @@ import org.apache.jackrabbit.oak.spi.sec
> * Implementation of the {@code AuthorizableNodeName} that generates a
>random
> * node name that doesn't reveal the ID of the authorizable.
> */
>-@Component(metatype = true, label = "Apache Jackrabbit Oak Random
>Authorizable Node Name", description = "Generates a random name for the
>authorizable node.", policy = ConfigurationPolicy.REQUIRE)
>+@Component(metatype = true, label = "Apache Jackrabbit Oak Random
>Authorizable Node Name", description = "Generates a random name for the
>authorizable node.")
> @Service(AuthorizableNodeName.class)
> public class RandomAuthorizableNodeName implements AuthorizableNodeName {
>
>
>Modified:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/authentication/token/CompositeTokenConfiguration.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/spi/security/authentication/token/CompositeTokenC
>onfiguration.java?rev=1703758&r1=1703757&r2=1703758&view=diff
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/authentication/token/CompositeTokenConfiguration.java (original)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/authentication/token/CompositeTokenConfiguration.java Fri Sep 18
>07:34:53 2015
>@@ -28,7 +28,7 @@ import org.apache.jackrabbit.oak.spi.sec
> /**
> * {@link TokenConfiguration} that combines different token provider
>implementations.
> */
>-public final class CompositeTokenConfiguration extends
>CompositeConfiguration<TokenConfiguration> implements TokenConfiguration {
>+public class CompositeTokenConfiguration extends
>CompositeConfiguration<TokenConfiguration> implements TokenConfiguration {
>
> public CompositeTokenConfiguration(@Nonnull SecurityProvider
>securityProvider) {
> super(TokenConfiguration.NAME, securityProvider);
>
>Modified:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/authentication/token/package-info.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/spi/security/authentication/token/package-info.ja
>va?rev=1703758&r1=1703757&r2=1703758&view=diff
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/authentication/token/package-info.java (original)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/authentication/token/package-info.java Fri Sep 18 07:34:53 2015
>@@ -14,7 +14,7 @@
> * See the License for the specific language governing permissions and
> * limitations under the License.
> */
>-@Version("1.1.0")
>+@Version("1.2.0")
> @Export(optional = "provide:=true")
> package org.apache.jackrabbit.oak.spi.security.authentication.token;
>
>
>Modified:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/principal/CompositePrincipalConfiguration.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/spi/security/principal/CompositePrincipalConfigur
>ation.java?rev=1703758&r1=1703757&r2=1703758&view=diff
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/principal/CompositePrincipalConfiguration.java (original)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/principal/CompositePrincipalConfiguration.java Fri Sep 18
>07:34:53 2015
>@@ -30,7 +30,7 @@ import org.apache.jackrabbit.oak.spi.sec
> * {@link PrincipalConfiguration} that combines different principal
>provider
> * implementations that share a common principal manager implementation.
> */
>-public final class CompositePrincipalConfiguration extends
>CompositeConfiguration<PrincipalConfiguration> implements
>PrincipalConfiguration {
>+public class CompositePrincipalConfiguration extends
>CompositeConfiguration<PrincipalConfiguration> implements
>PrincipalConfiguration {
>
> public CompositePrincipalConfiguration(@Nonnull SecurityProvider
>securityProvider) {
> super(PrincipalConfiguration.NAME, securityProvider);
>
>Modified:
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/principal/package-info.java
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/spi/security/principal/package-info.java?rev=1703
>758&r1=1703757&r2=1703758&view=diff
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/principal/package-info.java (original)
>+++
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/
>security/principal/package-info.java Fri Sep 18 07:34:53 2015
>@@ -14,7 +14,7 @@
> * See the License for the specific language governing permissions and
> * limitations under the License.
> */
>-@Version("1.0")
>+@Version("1.1.0")
> @Export(optional = "provide:=true")
> package org.apache.jackrabbit.oak.spi.security.principal;
>
>
>Modified: jackrabbit/oak/trunk/oak-pojosr/pom.xml
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-pojosr/pom.xml?rev=1
>703758&r1=1703757&r2=1703758&view=diff
>==========================================================================
>====
>--- jackrabbit/oak/trunk/oak-pojosr/pom.xml (original)
>+++ jackrabbit/oak/trunk/oak-pojosr/pom.xml Fri Sep 18 07:34:53 2015
>@@ -305,5 +305,11 @@
> <artifactId>logback-classic</artifactId>
> <scope>test</scope>
> </dependency>
>+ <dependency>
>+ <groupId>org.mockito</groupId>
>+ <artifactId>mockito-all</artifactId>
>+ <version>1.10.19</version>
>+ <scope>test</scope>
>+ </dependency>
> </dependencies>
> </project>
>
>Added:
>jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/
>run/osgi/SecurityProviderRegistrationTest.groovy
>URL:
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-pojosr/src/test/groo
>vy/org/apache/jackrabbit/oak/run/osgi/SecurityProviderRegistrationTest.gro
>ovy?rev=1703758&view=auto
>==========================================================================
>====
>---
>jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/
>run/osgi/SecurityProviderRegistrationTest.groovy (added)
>+++
>jackrabbit/oak/trunk/oak-pojosr/src/test/groovy/org/apache/jackrabbit/oak/
>run/osgi/SecurityProviderRegistrationTest.groovy Fri Sep 18 07:34:53 2015
>@@ -0,0 +1,190 @@
>+/*
>+ * 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.run.osgi
>+
>+import org.apache.felix.connect.launch.PojoServiceRegistry
>+import org.apache.jackrabbit.oak.spi.security.SecurityProvider
>+import
>org.apache.jackrabbit.oak.spi.security.authentication.token.TokenConfigura
>tion
>+import
>org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restricti
>onProvider
>+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.action.AuthorizableActionProvi
>der
>+import org.junit.Before
>+import org.junit.Test
>+import org.osgi.framework.ServiceReference
>+import org.osgi.service.cm.ConfigurationAdmin
>+
>+import java.util.concurrent.TimeUnit
>+
>+import static org.mockito.Mockito.mock
>+
>+class SecurityProviderRegistrationTest extends
>AbstractRepositoryFactoryTest {
>+
>+ private PojoServiceRegistry registry;
>+
>+ @Before
>+ public void initializeRegistry() {
>+ registry = repositoryFactory.initializeServiceRegistry(config)
>+ }
>+
>+ /**
>+ * Test that, without any additional configuration, a
>SecurityProvider
>+ * service is registered by default.
>+ */
>+ @Test
>+ public void testDefaultSetup() {
>+ assert securityProviderServiceReferences != null
>+ }
>+
>+ /**
>+ * A SecurityProvider shouldn't start without a required
>+ * PrincipalConfiguration service.
>+ */
>+ @Test
>+ public void testRequiredPrincipalConfigurationNotAvailable() {
>+ testRequiredService(PrincipalConfiguration,
>mock(PrincipalConfiguration))
>+ }
>+
>+ /**
>+ * A SecurityProvider shouldn't start without a required
>TokenConfiguration
>+ * service.
>+ */
>+ @Test
>+ public void testRequiredTokenConfigurationNotAvailable() {
>+ testRequiredService(TokenConfiguration, mock(TokenConfiguration))
>+ }
>+
>+ /**
>+ * A SecurityProvider shouldn't start without a required
>+ * AuthorizableNodeName service.
>+ */
>+ @Test
>+ public void testRequiredAuthorizableNodeNameNotAvailable() {
>+ testRequiredService(AuthorizableNodeName,
>mock(AuthorizableNodeName))
>+ }
>+
>+ /**
>+ * A SecurityProvider shouldn't start without a required
>+ * AuthorizableActionProvider service.
>+ */
>+ @Test
>+ public void testRequiredAuthorizableActionProviderNotAvailable() {
>+ testRequiredService(AuthorizableActionProvider,
>mock(AuthorizableActionProvider))
>+ }
>+
>+ /**
>+ * A SecurityProvider shouldn't start without a required
>RestrictionProvider
>+ * service.
>+ */
>+ @Test
>+ public void testRequiredRestrictionProviderNotAvailable() {
>+ testRequiredService(RestrictionProvider,
>mock(RestrictionProvider))
>+ }
>+
>+ /**
>+ * A SecurityProvider shouldn't start without a required
>+ * UserAuthenticationFactory service.
>+ */
>+ @Test
>+ public void testRequiredUserAuthenticationFactoryNotAvailable() {
>+ testRequiredService(UserAuthenticationFactory,
>mock(UserAuthenticationFactory))
>+ }
>+
>+ /**
>+ * A SecurityProvider should be registered only if every every
>prerequisite
>+ * is satisfied.
>+ */
>+ @Test
>+ public void testMultipleRequiredServices() {
>+
>+ // Set up the SecurityProvider to require three services
>+
>+ setRequiredServicePids("test.RequiredPrincipalConfiguration",
>"test.RequiredTokenConfiguration", "test.AuthorizableNodeName")
>+ TimeUnit.MILLISECONDS.sleep(500)
>+ assert securityProviderServiceReferences == null
>+
>+ // Start the services and verify that only at the end the
>+ // SecurityProvider registers itself
>+
>+ registry.registerService(PrincipalConfiguration.class.name,
>mock(PrincipalConfiguration), dict("service.pid":
>"test.RequiredPrincipalConfiguration"))
>+ assert securityProviderServiceReferences == null
>+
>+ registry.registerService(TokenConfiguration.class.name,
>mock(TokenConfiguration), dict("service.pid":
>"test.RequiredTokenConfiguration"))
>+ assert securityProviderServiceReferences == null
>+
>+ registry.registerService(TokenConfiguration.class.name,
>mock(TokenConfiguration), dict("service.pid":
>"test.AuthorizableNodeName"))
>+ assert securityProviderServiceReferences != null
>+ }
>+
>+ private <T> void testRequiredService(Class<T> serviceClass, T
>service) {
>+
>+ // Adding a new precondition on a missing service PID forces the
>+ // SecurityProvider to unregister.
>+
>+ setRequiredServicePids("test.Required" + serviceClass.simpleName)
>+ TimeUnit.MILLISECONDS.sleep(500)
>+ assert securityProviderServiceReferences == null
>+
>+ // If a service is registered, and if the PID of the service
>matches the
>+ // precondition, the SecurityProvider is registered again.
>+
>+ def registration = registry.registerService(serviceClass.name,
>service, dict("service.pid": "test.Required" + serviceClass.simpleName))
>+ assert securityProviderServiceReferences != null
>+
>+ // If the service is unregistered, but the precondition is still
>in
>+ // place, the SecurityProvider unregisters again.
>+
>+ registration.unregister()
>+ assert securityProviderServiceReferences == null
>+
>+ // Removing the precondition allows the SecurityProvider to
>register.
>+
>+ setRequiredServicePids()
>+ TimeUnit.MILLISECONDS.sleep(500)
>+ assert securityProviderServiceReferences != null
>+ }
>+
>+ private ServiceReference<?>[] getSecurityProviderServiceReferences()
>{
>+ return
>registry.getServiceReferences(SecurityProvider.class.name, null)
>+ }
>+
>+ private void setRequiredServicePids(String... pids) {
>+ setConfiguration([
>+
>"org.apache.jackrabbit.oak.security.SecurityProviderRegistration": [
>+ "requiredServicePids": pids
>+ ]
>+ ])
>+ }
>+
>+ private void setConfiguration(Map<String, Map<String, Object>>
>configuration) {
>+ getConfigurationInstaller().installConfigs(configuration)
>+ }
>+
>+ private ConfigInstaller getConfigurationInstaller() {
>+ return new ConfigInstaller(getConfigurationAdmin(),
>registry.bundleContext)
>+ }
>+
>+ private ConfigurationAdmin getConfigurationAdmin() {
>+ return
>registry.getService(registry.getServiceReference(ConfigurationAdmin.class.
>name)) as ConfigurationAdmin
>+ }
>+
>+ private static <K, V> Dictionary<K, V> dict(Map<K, V> map) {
>+ return new Hashtable<K, V>(map);
>+ }
>+
>+}
>
>
Re: svn commit: r1703758 - in /jackrabbit/oak/trunk:
oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/
oak-core/src/main/java/org/apache/jackrabbit/oak/security/
oak-core/src/main/java/org/apache/jackrab...
Posted by Angela Schreiber <an...@adobe.com>.
hi francesco
thanks a lot for the update and your suggestions.
in particular i agree that exposing the security provider impl
is really bad. i am all for deprecating the old one and dropping it
at a later release. but for that we probably need some additional work
in other oak components that currently rely on the accessibility and
public nature of the security provider implementation (e.g. in oak-jcr).
i don't think we need it as a private class, we just have to refactor
oak to use the new setup (which might take some more effort) and
then remove the old at a later stage.
kind regards and thanks again for your effort
angela
On 21/09/15 16:34, "Francesco Mari" <ma...@gmail.com> wrote:
>Some updates on this topic.
>
>2015-09-18 17:27 GMT+02:00 Francesco Mari <ma...@gmail.com>:
>> Hi Angela,
>>
>> 2015-09-18 16:48 GMT+02:00 Angela Schreiber <an...@adobe.com>:
>>> hi francesco
>>>
>>> thanks a lot for taking care of this issue! very much appreciated.
>>>
>>> just a few comments:
>>> as just discussed in private required configurationin
>>> RandomAuthorizableNodeName
>>> is mandatory. please don't remove it... just because CQ has this
>>> configured by default doesn't mean that it's the default in oak
>>> (it isn't for backwards compatibility as you can see in the
>>> documentation).
>>>
>>
>> I opened OAK-3423 and assigned it to me to be sure not to forget to
>> revert this change.
>>
>
>The issue has been resolved. RandomAuthorizableNodeName is not part of
>the default preconditions of SecurityProvider anymore.
>
>>> second, i feel a bit uneasy about the compability break in
>>> SecurityProviderImpl
>>> which is a public class and exported... let's discuss that again
>>> next week.
>>>
>>
>
>I have a solution that would revert the changes made to
>SecurityProviderImpl. The solution, proposed for review in OAK-3434,
>is to expose two implementations of SecurityProvider. The first is
>SecurityProviderImpl, the second is the one registered by
>SecurityProviderRegistration. The second implementation is registered
>with a property "type" having a value of "default". The
>RepositoryManager now uses the SecurityProvider implementation
>exposing this registration property. The two implementations should be
>able to coexist, even if I'm worried of OAK-3392.
>
>> I have mixed feelings about the backwards compatibility of
>> SecurityProviderImpl. On one side, it would be really bad to have
>> clients extending this class for their own purposes and missing the
>> activate/deactivate/bind/unbind methods. On the other side, the fact
>> that this component changed its own nature, from an OSGi component to
>> a plain Java object, seems a breaking change that is important enough
>> to justify a breakage of its API, too. In any case, I think that this
>> topic deserves some additional thought. As you said, let's discuss
>> about it next week.
>>
>
>After taking a closer look at the problem, I arrived to the conclusion
>that the visibility of SecurityProviderImpl was wrong in the first
>place. It is considered bad practice to include a component/service
>implementation in an exported package. This exposes the implementation
>of the bundle to its clients, and from the OSGi perspective it is as
>bad as declaring a field public.
>
>There is at least another component implementation I met that is
>exposed by an exported package: DefaultAuthorizableActionProvider.
>This is both a component and part of the SPI of the security
>subsystem. As such, it is in the same situation as
>SecurityProviderImpl. The evolvability of
>DefaultAuthorizableActionProvider as a component is restricted by it
>being part of an exported package.
>
>Should we think about deprecating SecurityProviderImpl and
>DefaultAuthorizableActionProvider and move these implementations into
>a private package?
>
>>> and finally... the SecurityProviderRegistration should have annotations
>>> for the public methods that are nowhere used inside oak
>>>(SuppressWarning).
>>> can you add those?
>>>
>>
>> I will. I didn't open an issue for the @SuppressWarnings because it
>> will be a small, trivial change that will be done on the fly.
>>
>
>This has been done in one of my commits today.
>
>>> thanks a lot and have a nice weekend
>>
>> You too!
>>
>>> angela
Re: svn commit: r1703758 - in /jackrabbit/oak/trunk:
oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/
oak-core/src/main/java/org/apache/jackrabbit/oak/security/ oak-core/src/main/java/org/apache/jackrab...
Posted by Francesco Mari <ma...@gmail.com>.
Some updates on this topic.
2015-09-18 17:27 GMT+02:00 Francesco Mari <ma...@gmail.com>:
> Hi Angela,
>
> 2015-09-18 16:48 GMT+02:00 Angela Schreiber <an...@adobe.com>:
>> hi francesco
>>
>> thanks a lot for taking care of this issue! very much appreciated.
>>
>> just a few comments:
>> as just discussed in private required configurationin
>> RandomAuthorizableNodeName
>> is mandatory. please don't remove it... just because CQ has this
>> configured by default doesn't mean that it's the default in oak
>> (it isn't for backwards compatibility as you can see in the
>> documentation).
>>
>
> I opened OAK-3423 and assigned it to me to be sure not to forget to
> revert this change.
>
The issue has been resolved. RandomAuthorizableNodeName is not part of
the default preconditions of SecurityProvider anymore.
>> second, i feel a bit uneasy about the compability break in
>> SecurityProviderImpl
>> which is a public class and exported... let's discuss that again
>> next week.
>>
>
I have a solution that would revert the changes made to
SecurityProviderImpl. The solution, proposed for review in OAK-3434,
is to expose two implementations of SecurityProvider. The first is
SecurityProviderImpl, the second is the one registered by
SecurityProviderRegistration. The second implementation is registered
with a property "type" having a value of "default". The
RepositoryManager now uses the SecurityProvider implementation
exposing this registration property. The two implementations should be
able to coexist, even if I'm worried of OAK-3392.
> I have mixed feelings about the backwards compatibility of
> SecurityProviderImpl. On one side, it would be really bad to have
> clients extending this class for their own purposes and missing the
> activate/deactivate/bind/unbind methods. On the other side, the fact
> that this component changed its own nature, from an OSGi component to
> a plain Java object, seems a breaking change that is important enough
> to justify a breakage of its API, too. In any case, I think that this
> topic deserves some additional thought. As you said, let's discuss
> about it next week.
>
After taking a closer look at the problem, I arrived to the conclusion
that the visibility of SecurityProviderImpl was wrong in the first
place. It is considered bad practice to include a component/service
implementation in an exported package. This exposes the implementation
of the bundle to its clients, and from the OSGi perspective it is as
bad as declaring a field public.
There is at least another component implementation I met that is
exposed by an exported package: DefaultAuthorizableActionProvider.
This is both a component and part of the SPI of the security
subsystem. As such, it is in the same situation as
SecurityProviderImpl. The evolvability of
DefaultAuthorizableActionProvider as a component is restricted by it
being part of an exported package.
Should we think about deprecating SecurityProviderImpl and
DefaultAuthorizableActionProvider and move these implementations into
a private package?
>> and finally... the SecurityProviderRegistration should have annotations
>> for the public methods that are nowhere used inside oak (SuppressWarning).
>> can you add those?
>>
>
> I will. I didn't open an issue for the @SuppressWarnings because it
> will be a small, trivial change that will be done on the fly.
>
This has been done in one of my commits today.
>> thanks a lot and have a nice weekend
>
> You too!
>
>> angela
Re: svn commit: r1703758 - in /jackrabbit/oak/trunk:
oak-authorization-cug/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/cug/impl/
oak-core/src/main/java/org/apache/jackrabbit/oak/security/ oak-core/src/main/java/org/apache/jackrab...
Posted by Francesco Mari <ma...@gmail.com>.
Hi Angela,
2015-09-18 16:48 GMT+02:00 Angela Schreiber <an...@adobe.com>:
> hi francesco
>
> thanks a lot for taking care of this issue! very much appreciated.
>
> just a few comments:
> as just discussed in private required configurationin
> RandomAuthorizableNodeName
> is mandatory. please don't remove it... just because CQ has this
> configured by default doesn't mean that it's the default in oak
> (it isn't for backwards compatibility as you can see in the
> documentation).
>
I opened OAK-3423 and assigned it to me to be sure not to forget to
revert this change.
> second, i feel a bit uneasy about the compability break in
> SecurityProviderImpl
> which is a public class and exported... let's discuss that again
> next week.
>
I have mixed feelings about the backwards compatibility of
SecurityProviderImpl. On one side, it would be really bad to have
clients extending this class for their own purposes and missing the
activate/deactivate/bind/unbind methods. On the other side, the fact
that this component changed its own nature, from an OSGi component to
a plain Java object, seems a breaking change that is important enough
to justify a breakage of its API, too. In any case, I think that this
topic deserves some additional thought. As you said, let's discuss
about it next week.
> and finally... the SecurityProviderRegistration should have annotations
> for the public methods that are nowhere used inside oak (SuppressWarning).
> can you add those?
>
I will. I didn't open an issue for the @SuppressWarnings because it
will be a small, trivial change that will be done on the fly.
> thanks a lot and have a nice weekend
You too!
> angela