SLING-6707 - LoginAdminWhitelist.fragment metatype descriptor not as intended
- add description for whitelist.bundles.regexp
- remove whitelist.bundles.default and whitelist.bundles.additional from metatype
and handle them separately
- refactor handling of backwards compatibility for above properties
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk@1788436 13f79535-47bb-0310-9956-ffa450edef68
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 104d13f..5bd5d38 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
@@ -19,6 +19,8 @@
package org.apache.sling.jcr.base.internal;
import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.regex.Pattern;
@@ -36,6 +38,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.sling.commons.osgi.PropertiesUtil.toStringArray;
+
/**
* Whitelist that defines which bundles can use the
* {@link SlingRepository#loginAdministrative} method.
@@ -59,13 +63,15 @@
private volatile ConfigurationState config;
- // for backwards compatibility only
- private volatile WhitelistFragment defaultFragment;
+ private final List<WhitelistFragment> whitelistFragments = new CopyOnWriteArrayList<WhitelistFragment>();
- // for backwards compatibility only
- private volatile WhitelistFragment additionalFragment;
+ // for backwards compatibility only (read properties directly to prevent them from appearing in the metatype)
+ private static final String PROP_WHITELIST_BUNDLES_DEFAULT = "whitelist.bundles.default";
- private final List<WhitelistFragment> whitelistFragments = new CopyOnWriteArrayList<>();
+ private static final String PROP_WHITELIST_BUNDLES_ADDITIONAL = "whitelist.bundles.additional";
+
+ private final Map<String, WhitelistFragment> backwardsCompatibleFragments =
+ new ConcurrentHashMap<String, WhitelistFragment>();
@Reference(
cardinality = ReferenceCardinality.MULTIPLE,
@@ -84,9 +90,10 @@
}
@Activate @Modified @SuppressWarnings("unused")
- void configure(LoginAdminWhitelistConfiguration configuration) {
+ void configure(LoginAdminWhitelistConfiguration configuration, Map<String, Object> properties) {
this.config = new ConfigurationState(configuration);
- backwardsCompatibility(configuration);
+ ensureBackwardsCompatibility(properties, PROP_WHITELIST_BUNDLES_DEFAULT);
+ ensureBackwardsCompatibility(properties, PROP_WHITELIST_BUNDLES_ADDITIONAL);
}
public boolean allowLoginAdministrative(Bundle b) {
@@ -130,7 +137,7 @@
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: {}",
+ LOG.warn("A 'whitelist.bundles.regexp' is configured, this is NOT RECOMMENDED for production: {}",
whitelistRegexp);
} else {
whitelistRegexp = null;
@@ -139,7 +146,7 @@
bypassWhitelist = config.whitelist_bypass();
if(bypassWhitelist) {
LOG.info("bypassWhitelist=true, whitelisted BSNs=<ALL>");
- LOG.warn("All bundles are allowed to use loginAdministrative due to the 'bypass whitelist' " +
+ LOG.warn("All bundles are allowed to use loginAdministrative due to the 'whitelist.bypass' " +
"configuration of this service. This is NOT RECOMMENDED, for security reasons."
);
}
@@ -147,25 +154,19 @@
}
@SuppressWarnings("deprecated")
- private void backwardsCompatibility(final LoginAdminWhitelistConfiguration configuration) {
- if (defaultFragment != null) {
- unbindWhitelistFragment(defaultFragment);
+ private void ensureBackwardsCompatibility(final Map<String, Object> properties, final String propertyName) {
+ final WhitelistFragment oldFragment = backwardsCompatibleFragments.remove(propertyName);
+
+ final String[] bsns = toStringArray(properties.get(propertyName), new String[0]);
+ if (bsns.length != 0) {
+ LOG.warn("Using deprecated configuration property '{}'", propertyName);
+ final WhitelistFragment fragment = new WhitelistFragment("deprecated-" + propertyName, bsns);
+ bindWhitelistFragment(fragment);
+ backwardsCompatibleFragments.put(propertyName, fragment);
}
- 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);
+
+ if (oldFragment != null) {
+ unbindWhitelistFragment(oldFragment);
}
}
}
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 8c96c3f..62df245 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
@@ -53,23 +53,11 @@
*
* @return The configured regular exression.
*/
+ @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."
+ )
String whitelist_bundles_regexp() default "";
-
- /**
- * 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 {};
-
- /**
- * 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
index 6ff49a9..81184a4 100644
--- a/src/main/java/org/apache/sling/jcr/base/internal/WhitelistFragment.java
+++ b/src/main/java/org/apache/sling/jcr/base/internal/WhitelistFragment.java
@@ -25,7 +25,9 @@
import org.osgi.service.metatype.annotations.Designate;
import org.osgi.service.metatype.annotations.ObjectClassDefinition;
-import java.util.List;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
import static java.util.Arrays.asList;
@@ -62,23 +64,24 @@
private String name;
- private List<String> bundles;
+ private Set<String> bundles;
@SuppressWarnings("unused")
public WhitelistFragment() {
// default constructor for SCR
}
+ // constructor for tests and for backwards compatible deprecated fragments
WhitelistFragment(String name, String[] bundles) {
this.name = name;
- this.bundles = asList(bundles);
+ this.bundles = asSet(bundles);
}
@Activate
@SuppressWarnings("unused")
void activate(Configuration config) {
name = config.whitelist_name();
- bundles = asList(config.whitelist_bundles() == null ? new String[0] : config.whitelist_bundles());
+ bundles = asSet(config.whitelist_bundles());
}
boolean allows(String bsn) {
@@ -89,4 +92,28 @@
public String toString() {
return name + ": " + bundles + "";
}
+
+ @Override
+ public boolean equals(final Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof WhitelistFragment)) {
+ return false;
+ }
+ final WhitelistFragment that = (WhitelistFragment) o;
+ return name.equals(that.name)
+ && bundles.equals(that.bundles);
+ }
+
+ @Override
+ public int hashCode() {
+ int result = name.hashCode();
+ result = 31 * result + bundles.hashCode();
+ return result;
+ }
+
+ private Set<String> asSet(final String[] values) {
+ return Collections.unmodifiableSet(new HashSet<String>(asList(values)));
+ }
}
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 48359a5..bf08545 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
@@ -59,7 +59,7 @@
@Test
public void testBypassWhitelist() throws ConfigurationException {
- whitelist.configure(config(true, null, null, null));
+ configure(whitelist, true, null, null, null);
for(String bsn : randomBsn()) {
assertAdminLogin(bsn, true);
@@ -71,7 +71,7 @@
final String [] allowed = {
"bundle1", "bundle2"
};
- whitelist.configure(config(null, null, allowed, null));
+ configure(whitelist, null, null, allowed, null);
assertAdminLogin("foo.1.bar", false);
@@ -89,8 +89,7 @@
final String [] allowed = {
"bundle5", "bundle6"
};
- final LoginAdminWhitelistConfiguration config = config(null, null, null, allowed);
- whitelist.configure(config);
+ configure(whitelist, null, null, null, allowed);
assertAdminLogin("foo.1.bar", false);
@@ -105,7 +104,7 @@
@Test
public void testDefaultAndAdditionalConfig() throws ConfigurationException {
- whitelist.configure(config(null, null, new String [] { "defB"}, new String [] { "addB"}));
+ configure(whitelist, null, null, new String [] { "defB"}, new String [] { "addB"});
assertAdminLogin("defB", true);
assertAdminLogin("addB", true);
@@ -121,7 +120,7 @@
final String [] allowed = {
"bundle3", "bundle4"
};
- whitelist.configure(config(null, "foo.*bar", allowed, null));
+ configure(whitelist, null, "foo.*bar", allowed, null);
assertAdminLogin("foo.2.bar", true);
assertAdminLogin("foo.somethingElse.bar", true);
@@ -144,7 +143,7 @@
final WhitelistFragment testFragment1 = new WhitelistFragment("test1", allowed1);
final WhitelistFragment testFragment2 = new WhitelistFragment("test2", allowed2);
- whitelist.configure(config(null, null, null, null));
+ configure(whitelist, null, null, null, null);
whitelist.bindWhitelistFragment(testFragment1);
whitelist.bindWhitelistFragment(testFragment2);
@@ -171,8 +170,7 @@
}
}
-
- private LoginAdminWhitelistConfiguration config(final Boolean bypass, final String regexp, final String[] defaultBSNs, final String[] additionalBSNs) throws ConfigurationException {
+ private void configure(final LoginAdminWhitelist whitelist, final Boolean bypass, final String regexp, final String[] defaultBSNs, final String[] additionalBSNs) throws ConfigurationException {
final Hashtable<String, Object> props = new Hashtable<>();
if (bypass != null) {
props.put("whitelist.bypass", bypass);
@@ -186,6 +184,8 @@
if (additionalBSNs != null) {
props.put("whitelist.bundles.additional", additionalBSNs);
}
- return ConfigAnnotationUtil.fromDictionary(LoginAdminWhitelistConfiguration.class, props);
+ LoginAdminWhitelistConfiguration configuration =
+ ConfigAnnotationUtil.fromDictionary(LoginAdminWhitelistConfiguration.class, props);
+ whitelist.configure(configuration, props);
}
}
\ No newline at end of file
