jsedding commented on code in PR #11:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-base/pull/11#discussion_r1341023515
##########
src/main/java/org/apache/sling/jcr/base/internal/LoginAdminAllowList.java:
##########
@@ -133,24 +124,47 @@ public boolean allowLoginAdministrative(Bundle b) {
}
// encapsulate configuration state for atomic configuration updates
- private static class ConfigurationState {
+ static class ConfigurationState {
+
+ public final boolean bypassAllowList;
- private final boolean bypassAllowList;
+ public final Pattern allowListRegexp;
- private final Pattern allowListRegexp;
+ ConfigurationState(final LoginAdminAllowListConfiguration config,
final Map<String, Object> properties) {
+ // first check for legacy properties
+ boolean bypass = config.allowlist_bypass();
+ final Object legacyBypassObject =
properties.get(LEGACY_BYPASS_PROPERTY);
+ if (legacyBypassObject != null) {
+ LOG.warn("Using deprecated configuration property '{}' from
configuration '{}'. " +
+ "Update your configuration to use configuration '{}' and
property '{}' instead.",
+ LEGACY_BYPASS_PROPERTY, LEGACY_PID, PID,
"allowlist.bypass");
+ bypass =
Converters.standardConverter().convert(legacyBypassObject).defaultValue(false).to(Boolean.class);
+ }
+ String legacyRegexp = null;
+ final Object legacyBundlesObject =
properties.get(LEGACY_BUNDLES_PROPERTY);
+ if (legacyBypassObject != null) {
+ LOG.warn("Using deprecated configuration property '{}' from
configuration '{}'. " +
+ "Update your configuration to use configuration '{}' and
property '{}' instead.",
+ LEGACY_BUNDLES_PROPERTY, LEGACY_PID, PID,
"allowlist.bundles.regexp");
+ legacyRegexp =
Converters.standardConverter().convert(legacyBundlesObject).to(String.class);
+ }
- private ConfigurationState(final LoginAdminAllowListConfiguration
config) {
final String regexp = config.allowlist_bundles_regexp();
- if(regexp.trim().length() > 0) {
- allowListRegexp = Pattern.compile(regexp);
- LOG.warn("A 'allowlist.bundles.regexp' is configured, this is
NOT RECOMMENDED for production: {}",
- allowListRegexp);
+ if (regexp.trim().length() > 0) {
+ if (legacyRegexp != null) {
+ LOG.warn("Both deprecated configuration property '{}' and
configuration property '{}' are set. " +
+ "The deprecated property '{}' is ignored.",
+ LEGACY_BUNDLES_PROPERTY, "allowlist.bundles.regexp",
LEGACY_BUNDLES_PROPERTY);
+ }
+ this.allowListRegexp = Pattern.compile(regexp);
} else {
- allowListRegexp = null;
+ this.allowListRegexp = legacyRegexp != null ?
Pattern.compile(legacyRegexp) : null;
}
-
- bypassAllowList = config.allowlist_bypass();
- if(bypassAllowList) {
+ if (this.allowListRegexp != null) {
+ LOG.warn("A 'allowlist.bundles.regexp' is configured, this is
NOT RECOMMENDED for production: {}", allowListRegexp);
Review Comment:
Fine with me.
##########
src/main/java/org/apache/sling/jcr/base/internal/LoginAdminAllowList.java:
##########
@@ -133,24 +124,47 @@ public boolean allowLoginAdministrative(Bundle b) {
}
// encapsulate configuration state for atomic configuration updates
- private static class ConfigurationState {
+ static class ConfigurationState {
+
+ public final boolean bypassAllowList;
- private final boolean bypassAllowList;
+ public final Pattern allowListRegexp;
- private final Pattern allowListRegexp;
+ ConfigurationState(final LoginAdminAllowListConfiguration config,
final Map<String, Object> properties) {
+ // first check for legacy properties
+ boolean bypass = config.allowlist_bypass();
+ final Object legacyBypassObject =
properties.get(LEGACY_BYPASS_PROPERTY);
+ if (legacyBypassObject != null) {
+ LOG.warn("Using deprecated configuration property '{}' from
configuration '{}'. " +
+ "Update your configuration to use configuration '{}' and
property '{}' instead.",
+ LEGACY_BYPASS_PROPERTY, LEGACY_PID, PID,
"allowlist.bypass");
+ bypass =
Converters.standardConverter().convert(legacyBypassObject).defaultValue(false).to(Boolean.class);
+ }
+ String legacyRegexp = null;
+ final Object legacyBundlesObject =
properties.get(LEGACY_BUNDLES_PROPERTY);
+ if (legacyBypassObject != null) {
+ LOG.warn("Using deprecated configuration property '{}' from
configuration '{}'. " +
+ "Update your configuration to use configuration '{}' and
property '{}' instead.",
+ LEGACY_BUNDLES_PROPERTY, LEGACY_PID, PID,
"allowlist.bundles.regexp");
+ legacyRegexp =
Converters.standardConverter().convert(legacyBundlesObject).to(String.class);
+ }
- private ConfigurationState(final LoginAdminAllowListConfiguration
config) {
final String regexp = config.allowlist_bundles_regexp();
- if(regexp.trim().length() > 0) {
- allowListRegexp = Pattern.compile(regexp);
- LOG.warn("A 'allowlist.bundles.regexp' is configured, this is
NOT RECOMMENDED for production: {}",
- allowListRegexp);
+ if (regexp.trim().length() > 0) {
+ if (legacyRegexp != null) {
+ LOG.warn("Both deprecated configuration property '{}' and
configuration property '{}' are set. " +
+ "The deprecated property '{}' is ignored.",
+ LEGACY_BUNDLES_PROPERTY, "allowlist.bundles.regexp",
LEGACY_BUNDLES_PROPERTY);
+ }
+ this.allowListRegexp = Pattern.compile(regexp);
} else {
- allowListRegexp = null;
+ this.allowListRegexp = legacyRegexp != null ?
Pattern.compile(legacyRegexp) : null;
}
-
- bypassAllowList = config.allowlist_bypass();
- if(bypassAllowList) {
+ if (this.allowListRegexp != null) {
+ LOG.warn("A 'allowlist.bundles.regexp' is configured, this is
NOT RECOMMENDED for production: {}", allowListRegexp);
+ }
+ this.bypassAllowList = bypass;
+ if (this.bypassAllowList) {
LOG.info("bypassAllowlist=true, allowlisted BSNs=<ALL>");
LOG.warn("All bundles are allowed to use loginAdministrative
due to the 'allowlist.bypass' " +
Review Comment:
Fine with me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
