SLING-6357 - Allow to extend LoginAdminWhitelist with multiple configurations
- implemented configuration based on WhitelistFragment configurations
- changed configurations in downstream modules and tests
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/bundles/jcr/base@1773046 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/pom.xml b/pom.xml
index b7c0238..9366083 100644
--- a/pom.xml
+++ b/pom.xml
@@ -122,6 +122,11 @@
</dependency>
<dependency>
+ <groupId>org.osgi</groupId>
+ <artifactId>osgi.cmpn</artifactId>
+ </dependency>
+
+ <dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
diff --git a/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepositoryManager.java b/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepositoryManager.java
index eaf443c..7153178 100644
--- a/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepositoryManager.java
+++ b/src/main/java/org/apache/sling/jcr/base/AbstractSlingRepositoryManager.java
@@ -106,8 +106,6 @@
private volatile ServiceTracker<LoginAdminWhitelist, LoginAdminWhitelist> whitelistTracker;
- private volatile LoginAdminWhitelist whitelist;
-
private final Object repoInitLock = new Object();
/**
@@ -159,7 +157,7 @@
* to use {@code loginAdministrative}.
*/
protected boolean allowLoginAdministrativeForBundle(final Bundle bundle) {
- return whitelist.allowLoginAdministrative(bundle);
+ return whitelistTracker.getService().allowLoginAdministrative(bundle);
}
/**
@@ -426,9 +424,11 @@
whitelistTracker = new ServiceTracker<LoginAdminWhitelist, LoginAdminWhitelist>(bundleContext, LoginAdminWhitelist.class, null) {
@Override
public LoginAdminWhitelist addingService(final ServiceReference<LoginAdminWhitelist> reference) {
- whitelist = bundleContext.getService(reference);
- waitForWhitelist.countDown();
- return whitelist;
+ try {
+ return super.addingService(reference);
+ } finally {
+ waitForWhitelist.countDown();
+ }
}
};
whitelistTracker.open();
@@ -588,7 +588,6 @@
}
if (whitelistTracker != null) {
- whitelist = null;
whitelistTracker.close();
whitelistTracker = null;
}
diff --git a/src/main/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelist.java b/src/main/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelist.java
index e41f745..104d13f 100644
--- a/src/main/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelist.java
+++ b/src/main/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelist.java
@@ -18,8 +18,8 @@
*/
package org.apache.sling.jcr.base.internal;
-import java.util.Arrays;
-import java.util.TreeSet;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
import java.util.regex.Pattern;
import org.apache.sling.jcr.api.SlingRepository;
@@ -28,6 +28,10 @@
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Modified;
+import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.component.annotations.ReferenceCardinality;
+import org.osgi.service.component.annotations.ReferencePolicy;
+import org.osgi.service.component.annotations.ReferencePolicyOption;
import org.osgi.service.metatype.annotations.Designate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -55,9 +59,34 @@
private volatile ConfigurationState config;
- @Activate @Modified
+ // for backwards compatibility only
+ private volatile WhitelistFragment defaultFragment;
+
+ // for backwards compatibility only
+ private volatile WhitelistFragment additionalFragment;
+
+ private final List<WhitelistFragment> whitelistFragments = new CopyOnWriteArrayList<>();
+
+ @Reference(
+ cardinality = ReferenceCardinality.MULTIPLE,
+ policy = ReferencePolicy.DYNAMIC,
+ policyOption = ReferencePolicyOption.GREEDY
+ ) @SuppressWarnings("unused")
+ void bindWhitelistFragment(WhitelistFragment fragment) {
+ whitelistFragments.add(fragment);
+ LOG.info("WhitelistFragment added '{}'", fragment);
+ }
+
+ @SuppressWarnings("unused")
+ void unbindWhitelistFragment(WhitelistFragment fragment) {
+ whitelistFragments.remove(fragment);
+ LOG.info("WhitelistFragment removed '{}'", fragment);
+ }
+
+ @Activate @Modified @SuppressWarnings("unused")
void configure(LoginAdminWhitelistConfiguration configuration) {
this.config = new ConfigurationState(configuration);
+ backwardsCompatibility(configuration);
}
public boolean allowLoginAdministrative(Bundle b) {
@@ -72,36 +101,37 @@
}
final String bsn = b.getSymbolicName();
+
if(localConfig.whitelistRegexp != null && localConfig.whitelistRegexp.matcher(bsn).matches()) {
LOG.debug("{} is whitelisted to use loginAdministrative, by regexp", bsn);
return true;
- } else if(localConfig.whitelistedBsn.contains(bsn)) {
- LOG.debug("{} is whitelisted to use loginAdministrative, by explicit whitelist", bsn);
- return true;
}
+
+ for (final WhitelistFragment fragment : whitelistFragments) {
+ if (fragment.allows(bsn)) {
+ LOG.debug("{} is whitelisted to use loginAdministrative, by whitelist fragment '{}'",
+ bsn, fragment);
+ return true;
+ }
+ }
+
LOG.debug("{} is not whitelisted to use loginAdministrative", bsn);
return false;
}
// encapsulate configuration state for atomic configuration updates
private static class ConfigurationState {
- private final TreeSet<String> whitelistedBsn;
- private final Pattern whitelistRegexp;
+
private final boolean bypassWhitelist;
- private ConfigurationState(final LoginAdminWhitelistConfiguration config) {
- whitelistedBsn = new TreeSet<String>();
- if (config.whitelist_bundles_default() != null) { // null check due to FELIX-5404
- whitelistedBsn.addAll(Arrays.asList(config.whitelist_bundles_default()));
- }
- if (config.whitelist_bundles_additional() != null) {
- whitelistedBsn.addAll(Arrays.asList(config.whitelist_bundles_additional()));
- }
+ private final Pattern whitelistRegexp;
+ private ConfigurationState(final LoginAdminWhitelistConfiguration config) {
final String regexp = config.whitelist_bundles_regexp();
if(regexp.trim().length() > 0) {
whitelistRegexp = Pattern.compile(regexp);
- LOG.warn("A whitelist.bundles.regexp is configured, this is NOT RECOMMENDED for production: {}", whitelistRegexp);
+ LOG.warn("A whitelist.bundles.regexp is configured, this is NOT RECOMMENDED for production: {}",
+ whitelistRegexp);
} else {
whitelistRegexp = null;
}
@@ -112,9 +142,30 @@
LOG.warn("All bundles are allowed to use loginAdministrative due to the 'bypass whitelist' " +
"configuration of this service. This is NOT RECOMMENDED, for security reasons."
);
- } else {
- LOG.info("bypassWhitelist=false, whitelisted BSNs({})={}", whitelistedBsn.size(), whitelistedBsn);
}
}
}
+
+ @SuppressWarnings("deprecated")
+ private void backwardsCompatibility(final LoginAdminWhitelistConfiguration configuration) {
+ if (defaultFragment != null) {
+ unbindWhitelistFragment(defaultFragment);
+ }
+ if (additionalFragment != null) {
+ unbindWhitelistFragment(additionalFragment);
+ }
+ final String[] defaultBsns = configuration.whitelist_bundles_default();
+ if (defaultBsns != null && defaultBsns.length != 0) {
+ LOG.warn("Using deprecated configuration property 'whitelist.bundles.default'");
+ defaultFragment = new WhitelistFragment("deprecated-whitelist.bundles.default", defaultBsns);
+ bindWhitelistFragment(defaultFragment);
+ }
+
+ final String[] additionalBsns = configuration.whitelist_bundles_additional();
+ if (additionalBsns != null && additionalBsns.length != 0) {
+ LOG.warn("Using deprecated configuration property 'whitelist.bundles.additional'");
+ additionalFragment = new WhitelistFragment("deprecated-whitelist.bundles.additional", additionalBsns);
+ bindWhitelistFragment(additionalFragment);
+ }
+ }
}
diff --git a/src/main/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelistConfiguration.java b/src/main/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelistConfiguration.java
index f23a135..8c96c3f 100644
--- a/src/main/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelistConfiguration.java
+++ b/src/main/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelistConfiguration.java
@@ -27,10 +27,11 @@
)
@interface LoginAdminWhitelistConfiguration {
- /** Need to allow for bypassing the whitelist, for backwards
- * compatibility with previous Sling versions which didn't
- * implement it. Setting this to true is not recommended
- * and logged as a warning.
+ /**
+ * Need to allow for bypassing the whitelist, for backwards
+ * compatibility with previous Sling versions which didn't
+ * implement it. Setting this to true is not recommended
+ * and logged as a warning.
*/
@AttributeDefinition(
name = "Bypass the whitelist",
@@ -40,41 +41,35 @@
)
boolean whitelist_bypass() default false;
- @AttributeDefinition(
- name = "Whitelist regexp",
- description = "Regular expression for bundle symbolic names for which loginAdministrative() " +
- "is allowed. NOT recommended for production use, but useful for testing with " +
- "generated bundles."
- )
+ /**
+ * Regular expression for bundle symbolic names for which loginAdministrative()
+ * is allowed. NOT recommended for production use, but useful for testing with
+ * generated bundles.
+ * <br>
+ * Note that this property is hidden in order not to advertise its presence,
+ * because it is intended only for testing purposes. Specifically for use-cases
+ * like Pax-Exam, where bundles are generated on the fly and the bundle symbolic
+ * name cannot be predicted, but follows a predictable pattern.
+ *
+ * @return The configured regular exression.
+ */
String whitelist_bundles_regexp() default "";
- @AttributeDefinition(
- name = "Default whitelisted BSNs",
- description = "Default list of bundle symbolic names for which loginAdministrative() is allowed."
- )
- String[] whitelist_bundles_default() default {
- // TODO: remove bundles as their dependency on admin login is fixed, see SLING-5355 for linked issues
- "org.apache.sling.discovery.commons",
- "org.apache.sling.discovery.base",
- "org.apache.sling.discovery.oak",
- "org.apache.sling.extensions.webconsolesecurityprovider",
- "org.apache.sling.i18n",
- "org.apache.sling.jcr.base",
- "org.apache.sling.jcr.contentloader",
- "org.apache.sling.jcr.davex",
- "org.apache.sling.jcr.jackrabbit.usermanager",
- "org.apache.sling.jcr.oak.server",
- "org.apache.sling.jcr.repoinit",
- "org.apache.sling.jcr.resource",
- "org.apache.sling.jcr.webconsole",
- "org.apache.sling.resourceresolver",
- "org.apache.sling.servlets.post", // remove when 2.3.16 is released
- "org.apache.sling.servlets.resolver"
- };
+ /**
+ * Default list of bundle symbolic names for which loginAdministrative() is allowed.
+ *
+ * @return The default whitelisted BSNs
+ * @deprecated use {@link WhitelistFragment} configurations instead
+ */
+ @Deprecated
+ String[] whitelist_bundles_default() default {};
- @AttributeDefinition(
- name = "Additional whitelisted BSNs",
- description = "Additional list of bundle symbolic names for which loginAdministrative() is allowed."
- )
+ /**
+ * Additional list of bundle symbolic names for which loginAdministrative() is allowed.
+ *
+ * @return Additional whitelisted BSNs
+ * @deprecated use {@link WhitelistFragment} configurations instead
+ */
+ @Deprecated
String[] whitelist_bundles_additional() default {};
}
diff --git a/src/main/java/org/apache/sling/jcr/base/internal/WhitelistFragment.java b/src/main/java/org/apache/sling/jcr/base/internal/WhitelistFragment.java
new file mode 100644
index 0000000..6ff49a9
--- /dev/null
+++ b/src/main/java/org/apache/sling/jcr/base/internal/WhitelistFragment.java
@@ -0,0 +1,92 @@
+/*
+ * 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.sling.jcr.base.internal;
+
+import org.osgi.service.component.annotations.Activate;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.ConfigurationPolicy;
+import org.osgi.service.metatype.annotations.AttributeDefinition;
+import org.osgi.service.metatype.annotations.Designate;
+import org.osgi.service.metatype.annotations.ObjectClassDefinition;
+
+import java.util.List;
+
+import static java.util.Arrays.asList;
+
+@ObjectClassDefinition(
+ name = "Apache Sling Login Admin Whitelist Configuration Fragment",
+ description = "Whitelist configuration fragments contribute a list of whitelisted bundle symbolic " +
+ "names to the Login Admin Whitelist. This allows for modularisation of the whitelist."
+)
+@interface Configuration {
+
+ @AttributeDefinition(
+ name = "Name",
+ description = "Optional name to disambiguate configurations."
+ )
+ String whitelist_name() default "[unnamed]";
+
+ @AttributeDefinition(
+ name = "Whitelisted BSNs",
+ description = "A list of bundle symbolic names allowed to use loginAdministrative()."
+ )
+ String[] whitelist_bundles();
+
+ @SuppressWarnings("unused")
+ String webconsole_configurationFactory_nameHint() default "{whitelist.name}: [{whitelist.bundles}]";
+}
+
+@Component(
+ configurationPid = "org.apache.sling.jcr.base.internal.LoginAdminWhitelist.fragment",
+ configurationPolicy = ConfigurationPolicy.REQUIRE,
+ service = WhitelistFragment.class
+)
+@Designate(ocd = Configuration.class, factory = true)
+public class WhitelistFragment {
+
+ private String name;
+
+ private List<String> bundles;
+
+ @SuppressWarnings("unused")
+ public WhitelistFragment() {
+ // default constructor for SCR
+ }
+
+ WhitelistFragment(String name, String[] bundles) {
+ this.name = name;
+ this.bundles = asList(bundles);
+ }
+
+ @Activate
+ @SuppressWarnings("unused")
+ void activate(Configuration config) {
+ name = config.whitelist_name();
+ bundles = asList(config.whitelist_bundles() == null ? new String[0] : config.whitelist_bundles());
+ }
+
+ boolean allows(String bsn) {
+ return bundles.contains(bsn);
+ }
+
+ @Override
+ public String toString() {
+ return name + ": " + bundles + "";
+ }
+}
diff --git a/src/test/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelistTest.java b/src/test/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelistTest.java
index 6a24327..48359a5 100644
--- a/src/test/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelistTest.java
+++ b/src/test/java/org/apache/sling/jcr/base/internal/LoginAdminWhitelistTest.java
@@ -35,8 +35,6 @@
public class LoginAdminWhitelistTest {
- private static final String TYPICAL_DEFAULT_ALLOWED_BSN = "org.apache.sling.jcr.base";
-
private LoginAdminWhitelist whitelist;
@Before
@@ -58,22 +56,6 @@
}
return result;
}
-
- @Test
- public void testDefaultConfig() throws ConfigurationException {
- final LoginAdminWhitelistConfiguration config = config(null, null, null, null);
- whitelist.configure(config);
-
- for(String bsn : config.whitelist_bundles_default()) {
- assertAdminLogin(bsn, true);
- }
-
- assertAdminLogin(TYPICAL_DEFAULT_ALLOWED_BSN, true);
-
- for(String bsn : randomBsn()) {
- assertAdminLogin(bsn, false);
- }
- }
@Test
public void testBypassWhitelist() throws ConfigurationException {
@@ -90,12 +72,13 @@
"bundle1", "bundle2"
};
whitelist.configure(config(null, null, allowed, null));
-
- assertAdminLogin("bundle1", true);
- assertAdminLogin("bundle2", true);
+
assertAdminLogin("foo.1.bar", false);
- assertAdminLogin(TYPICAL_DEFAULT_ALLOWED_BSN, false);
-
+
+ for(String bsn : allowed) {
+ assertAdminLogin(bsn, true);
+ }
+
for(String bsn : randomBsn()) {
assertAdminLogin(bsn, false);
}
@@ -108,16 +91,13 @@
};
final LoginAdminWhitelistConfiguration config = config(null, null, null, allowed);
whitelist.configure(config);
-
- assertAdminLogin("bundle5", true);
- assertAdminLogin("bundle6", true);
- assertAdminLogin("foo.1.bar", false);
- assertAdminLogin(TYPICAL_DEFAULT_ALLOWED_BSN, true);
- for(String bsn : config.whitelist_bundles_default()) {
+ assertAdminLogin("foo.1.bar", false);
+
+ for(String bsn : allowed) {
assertAdminLogin(bsn, true);
}
-
+
for(String bsn : randomBsn()) {
assertAdminLogin(bsn, false);
}
@@ -130,7 +110,6 @@
assertAdminLogin("defB", true);
assertAdminLogin("addB", true);
assertAdminLogin("foo.1.bar", false);
- assertAdminLogin(TYPICAL_DEFAULT_ALLOWED_BSN, false);
for(String bsn : randomBsn()) {
assertAdminLogin(bsn, false);
@@ -143,18 +122,56 @@
"bundle3", "bundle4"
};
whitelist.configure(config(null, "foo.*bar", allowed, null));
-
- assertAdminLogin("bundle3", true);
- assertAdminLogin("bundle4", true);
+
assertAdminLogin("foo.2.bar", true);
assertAdminLogin("foo.somethingElse.bar", true);
- assertAdminLogin(TYPICAL_DEFAULT_ALLOWED_BSN, false);
+
+ for(String bsn : allowed) {
+ assertAdminLogin(bsn, true);
+ }
for(String bsn : randomBsn()) {
assertAdminLogin(bsn, false);
}
}
+
+ @Test
+ public void testWhitelistFragment() throws ConfigurationException {
+ final String [] allowed1 = randomBsn().toArray(new String[0]);
+ final String [] allowed2 = randomBsn().toArray(new String[0]);
+
+ final WhitelistFragment testFragment1 = new WhitelistFragment("test1", allowed1);
+ final WhitelistFragment testFragment2 = new WhitelistFragment("test2", allowed2);
+
+ whitelist.configure(config(null, null, null, null));
+ whitelist.bindWhitelistFragment(testFragment1);
+ whitelist.bindWhitelistFragment(testFragment2);
+
+ for(String bsn : allowed1) {
+ assertAdminLogin(bsn, true);
+ }
+
+ for(String bsn : allowed2) {
+ assertAdminLogin(bsn, true);
+ }
+
+ for(String bsn : randomBsn()) {
+ assertAdminLogin(bsn, false);
+ }
+
+ whitelist.unbindWhitelistFragment(testFragment1);
+
+ for(String bsn : allowed1) {
+ assertAdminLogin(bsn, false);
+ }
+
+ for(String bsn : allowed2) {
+ assertAdminLogin(bsn, true);
+ }
+ }
+
+
private LoginAdminWhitelistConfiguration config(final Boolean bypass, final String regexp, final String[] defaultBSNs, final String[] additionalBSNs) throws ConfigurationException {
final Hashtable<String, Object> props = new Hashtable<>();
if (bypass != null) {