diff --git a/CHANGELOG.md b/CHANGELOG.md index 63a9b97c32..15e00d6755 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) ### Changed - #3267 - Remove JSR305 dependency +- #3262 - Allow to configure Component/BundleDisabler via Configuration Factories - #3296 - Add image cropping customisation ### Fixed @@ -20,6 +21,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) - #3270 - Re-enable accidentally disabled JUnit3/4 tests - #3200 - Remove useless public interface in Cloud Bundle to get javadocs to be built - #3295 - Updated the annotations in QueryReportConfig fixing the query manager issue due to empty query language +- #3284 - Allow anonymous to read redirect caconfig options ## 6.4.0 - 2024-02-22 diff --git a/all/src/main/content/jcr_root/apps/acs-commons/config/org.apache.sling.jcr.repoinit.RepositoryInitializer-acs-commons-all.config b/all/src/main/content/jcr_root/apps/acs-commons/config/org.apache.sling.jcr.repoinit.RepositoryInitializer-acs-commons-all.config index 1dec5e6dc5..1d9d7c13d1 100644 --- a/all/src/main/content/jcr_root/apps/acs-commons/config/org.apache.sling.jcr.repoinit.RepositoryInitializer-acs-commons-all.config +++ b/all/src/main/content/jcr_root/apps/acs-commons/config/org.apache.sling.jcr.repoinit.RepositoryInitializer-acs-commons-all.config @@ -44,6 +44,7 @@ end # web requests need read access to redirect configurations, e.g. /conf/global/settings/redirects set ACL for anonymous allow jcr:read on /conf restriction(rep:glob,/*/settings/redirects) + allow jcr:read on /conf restriction(rep:glob,/*/settings/redirects/*) end create service user acs-commons-automatic-package-replicator-service with path system/acs-commons diff --git a/bundle/pom.xml b/bundle/pom.xml index e14b71c2a4..17a0453e45 100644 --- a/bundle/pom.xml +++ b/bundle/pom.xml @@ -349,6 +349,11 @@ org.osgi.service.cm provided + + org.osgi + org.osgi.service.event + provided + org.slf4j slf4j-api diff --git a/bundle/src/main/java/com/adobe/acs/commons/util/impl/BundleDisabler.java b/bundle/src/main/java/com/adobe/acs/commons/util/impl/BundleDisabler.java index e89475bb7d..dd9d874c88 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/util/impl/BundleDisabler.java +++ b/bundle/src/main/java/com/adobe/acs/commons/util/impl/BundleDisabler.java @@ -17,53 +17,73 @@ */ package com.adobe.acs.commons.util.impl; -import org.apache.felix.scr.annotations.Activate; -import org.apache.felix.scr.annotations.ConfigurationPolicy; -import org.apache.felix.scr.annotations.Property; -import org.apache.felix.scr.annotations.Service; -import org.apache.sling.commons.osgi.PropertiesUtil; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; -import org.osgi.service.component.ComponentContext; +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.component.annotations.Reference; +import org.osgi.service.component.annotations.ReferencePolicyOption; import org.osgi.service.event.Event; import org.osgi.service.event.EventHandler; +import org.osgi.service.event.propertytypes.EventTopics; +import org.osgi.service.metatype.annotations.AttributeDefinition; +import org.osgi.service.metatype.annotations.Designate; +import org.osgi.service.metatype.annotations.ObjectClassDefinition; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Map; - /** - * Component disabler service + * OSGi bundle disabler service *

- * In the AEM security checklist, there are some bundles which should be disabled in production environents. + * In the AEM security checklist, there are some bundles which should be disabled in production environments. * Whilst these bundles can be manually stopped, this component will do that as part of a deployment. It will also * ensure that if they are manually started once more, then they will be immediately stopped. */ -@org.apache.felix.scr.annotations.Component(immediate = true, metatype = true, - label = "ACS AEM Commons - OSGI Bundle Disabler", description = "Disables bundles by configuration", - policy = ConfigurationPolicy.REQUIRE) -@Service() -@Property(name = "event.topics", value = { "org/osgi/framework/BundleEvent/STARTED" }, propertyPrivate = true) +@Component(immediate = true, configurationPolicy = ConfigurationPolicy.REQUIRE) +@EventTopics(value = { "org/osgi/framework/BundleEvent/STARTED"} ) +@Designate(ocd = BundleDisabler.Config.class) public class BundleDisabler implements EventHandler { - private static final Logger log = LoggerFactory.getLogger(BundleDisabler.class); + @ObjectClassDefinition(name = "ACS AEM Commons - OSGI Bundle Disabler", description = "Disables OSGi bundles by configuration") + static @interface Config { + @AttributeDefinition(name = "Disabled bundles", description = "The symbolic names of the bundles you want to disable", cardinality = Integer.MAX_VALUE) + String[] bundles(); + } - @Property(label = "Disabled bundles", description = "The symbolic names of the bundles you want to disable", - cardinality = Integer.MAX_VALUE) - private static final String DISABLED_BUNDLES = "bundles"; + @Component(service = ConfigAmendment.class, property = "webconsole.configurationFactory.nameHint={bundles}") + @Designate(factory = true, ocd = BundleDisabler.Config.class) + public static final class ConfigAmendment { + private final Config config; - private BundleContext bundleContext; + @Activate + public ConfigAmendment(Config config) { + this.config = config; + } - private List disabledBundles = Collections.emptyList(); + public Config getConfig() { + return config; + } + } + + private static final Logger log = LoggerFactory.getLogger(BundleDisabler.class); + + private final BundleContext bundleContext; + + private final Set disabledBundles; @Activate - protected void activate(ComponentContext componentContext, Map properties) { - this.bundleContext = componentContext.getBundleContext(); - this.disabledBundles = getDisabledBundles(properties); + public BundleDisabler(BundleContext bundleContext, Config config, @Reference(policyOption = ReferencePolicyOption.GREEDY) List configAmendments) { + this.bundleContext = bundleContext; + this.disabledBundles = new HashSet<>(); + this.disabledBundles.addAll(Arrays.asList(config.bundles())); + configAmendments.stream().forEach(amendment -> disabledBundles.addAll(Arrays.asList(amendment.getConfig().bundles()))); disableBundles(); } @@ -93,17 +113,12 @@ private void disableBundles() { private void disableBundle(final Bundle bundle) throws BundleException { if (isBundleStoppable(bundle) && isNotOwnBundle(bundle)) { - log.info("Bundle {} disabled by configuration (name={}) ", + log.info("Bundle {} disabled by configuration (id={}) ", bundle.getSymbolicName(), bundle.getBundleId()); bundle.stop(); } } - private List getDisabledBundles(final Map properties) { - final String[] bundlesProperty = PropertiesUtil.toStringArray(properties.get(DISABLED_BUNDLES), new String[0]); - return Arrays.asList(PropertiesUtil.toStringArray(bundlesProperty, new String[0])); - } - private boolean isOnBundleStopList(final Bundle bundle) { return disabledBundles.contains(bundle.getSymbolicName()); } diff --git a/bundle/src/main/java/com/adobe/acs/commons/util/impl/ComponentDisabler.java b/bundle/src/main/java/com/adobe/acs/commons/util/impl/ComponentDisabler.java index 1289d55587..49cc9111eb 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/util/impl/ComponentDisabler.java +++ b/bundle/src/main/java/com/adobe/acs/commons/util/impl/ComponentDisabler.java @@ -18,68 +18,91 @@ package com.adobe.acs.commons.util.impl; import java.util.Arrays; -import java.util.Map; +import java.util.HashSet; +import java.util.List; +import java.util.Set; -import org.apache.felix.scr.annotations.Activate; -import org.apache.felix.scr.annotations.ConfigurationPolicy; -import org.apache.felix.scr.annotations.Property; -import org.apache.felix.scr.annotations.Reference; -import org.apache.felix.scr.annotations.ReferencePolicyOption; -import org.apache.felix.scr.annotations.Service; -import org.apache.sling.commons.osgi.PropertiesUtil; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; +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.component.annotations.Reference; +import org.osgi.service.component.annotations.ReferencePolicyOption; import org.osgi.service.component.runtime.ServiceComponentRuntime; import org.osgi.service.component.runtime.dto.ComponentDescriptionDTO; import org.osgi.service.event.Event; import org.osgi.service.event.EventHandler; +import org.osgi.service.event.propertytypes.EventTopics; +import org.osgi.service.metatype.annotations.AttributeDefinition; +import org.osgi.service.metatype.annotations.Designate; +import org.osgi.service.metatype.annotations.ObjectClassDefinition; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Component disabler service + * OSGi DS Component disabler service. * - * In Apache Felix the state of components and services is not persisted across restarts of its containing bundle. - * For example, when you have a Bundle S containing a service S, and you manually stop the service S; after a - * deactivate and activate of the bundle the service S is up again. - * - * This service allows you to specify the names of components, which shouldn't be running. Whenever an OSGI service event is + * In Apache Felix the state of DS components is not persisted across restarts of its containing bundle. + * For example, when you have a Bundle S containing a component S, and you manually stop the component S; after a + * deactivate and activate of the bundle the component S is up again. + *

+ * This service allows you to specify the names of DS components, which shouldn't be running. Whenever an OSGi service event is * fired, which services checks the status of this components and stops them if required. - * + *

* Note 1: The component is always started, but this service takes care, that it is stopped immediately after. So if a behaviour * you don't like already happens during the activation of this service, you cannot prevent it using the mechanism here. - * + * Particularly components not implementing an OSGi service may be running for a long time until a service registered or bundle + * started event finally stops them. + *

* Note 2: Using this service should always be considered as a workaround. The primary focus should be to fix the component - * you want to disable, so it's no longer required to disable it. If this component is part of Adobe AEM please raise a Daycare + * you want to disable, so it's no longer required to disable it. If this component is part of Adobe AEM please raise a Support * ticket for it. - * - * + * @see OSGi Declarative Services 1.4, Service Component Runtime, Introspection + * @see ServiceComponentRuntime#disableComponent(ComponentDescriptionDTO) */ -@org.apache.felix.scr.annotations.Component(immediate = true, metatype = true, - label = "ACS AEM Commons - OSGI Component Disabler", description = "Disables components by configuration", - policy = ConfigurationPolicy.REQUIRE) -@Service() -@Property(name = "event.topics", value = { "org/osgi/framework/BundleEvent/STARTED", - "org/osgi/framework/ServiceEvent/REGISTERED" }, propertyPrivate = true) +@Component(immediate = true, configurationPolicy = ConfigurationPolicy.REQUIRE) +@EventTopics(value = { "org/osgi/framework/BundleEvent/STARTED", "org/osgi/framework/ServiceEvent/REGISTERED"} ) +@Designate(ocd = ComponentDisabler.Config.class) public class ComponentDisabler implements EventHandler { - private static final Logger log = LoggerFactory.getLogger(ComponentDisabler.class); + @ObjectClassDefinition(name = "ACS AEM Commons - OSGi DS Component Disabler", description = "Disables OSGi DS components by configuration") + static @interface Config { + @AttributeDefinition(name = "Disabled components", description = "The names of the components you want to disable (usually their fully class name)", cardinality = Integer.MAX_VALUE) + String[] components(); + } - @Property(label = "Disabled components", description = "The names of the components/services you want to disable", - cardinality = Integer.MAX_VALUE) - private static final String DISABLED_COMPONENTS = "components"; + @Component(service = ConfigAmendment.class, property = "webconsole.configurationFactory.nameHint={components}") + @Designate(factory = true, ocd = ComponentDisabler.Config.class) + public static final class ConfigAmendment { + private final Config config; - private String[] disabledComponents; + @Activate + public ConfigAmendment(Config config) { + this.config = config; + } + + public Config getConfig() { + return config; + } + } + + private static final Logger log = LoggerFactory.getLogger(ComponentDisabler.class); - @Reference - private ServiceComponentRuntime scr; + private final BundleContext bundleContext; - private BundleContext bundleContext; + private final ServiceComponentRuntime scr; + + private final Set disabledComponents; @Activate - protected void activate(BundleContext bundleContext, Map properties) { - disabledComponents = PropertiesUtil.toStringArray(properties.get(DISABLED_COMPONENTS), new String[0]); + public ComponentDisabler(BundleContext bundleContext, Config config, @Reference ServiceComponentRuntime scr, @Reference(policyOption = ReferencePolicyOption.GREEDY) List configAmendments) { this.bundleContext = bundleContext; + // merge amendments + disabledComponents = new HashSet<>(); + disabledComponents.addAll(Arrays.asList(config.components())); + configAmendments.stream().forEach(amendment -> disabledComponents.addAll(Arrays.asList(amendment.getConfig().components()))); + this.scr = scr; handleEvent(null); } @@ -87,7 +110,7 @@ protected void activate(BundleContext bundleContext, Map propert public void handleEvent(Event event) { // We don't care about the event, we just need iterate all configured // components and try to disable them - log.trace("Disabling components and services {}", Arrays.toString(disabledComponents)); + log.trace("Disabling OSGi DS components {}", String.join(",", disabledComponents)); for (String component : disabledComponents) { disable(component); @@ -98,7 +121,7 @@ public void disable(String componentName) { for (Bundle bundle : bundleContext.getBundles()) { ComponentDescriptionDTO dto = scr.getComponentDescriptionDTO(bundle, componentName); if (dto != null && scr.isComponentEnabled(dto)) { - log.info("Component {} disabled by configuration.", dto.implementationClass); + log.info("Disabling OSGi DS Component {}.", dto.implementationClass); scr.disableComponent(dto); } } diff --git a/bundle/src/test/java/com/adobe/acs/commons/it/build/ScrMetadataIT.java b/bundle/src/test/java/com/adobe/acs/commons/it/build/ScrMetadataIT.java index 14668c4549..f4f325a85b 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/it/build/ScrMetadataIT.java +++ b/bundle/src/test/java/com/adobe/acs/commons/it/build/ScrMetadataIT.java @@ -31,6 +31,7 @@ import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -273,7 +274,7 @@ private DescriptorList parseJar(InputStream is, boolean checkNs) throws Exceptio while (entry != null) { if (!entry.isDirectory() && entry.getName().endsWith(".xml")) { if (entry.getName().startsWith("OSGI-INF/metatype")) { - metatypeDescriptors.add(parseMetatype(new InputStreamFacade(zis), entry.getName())); + metatypeDescriptors.addAll(parseMetatype(new InputStreamFacade(zis), entry.getName())); } else if (entry.getName().startsWith("OSGI-INF/")) { result.merge(parseScr(new InputStreamFacade(zis), entry.getName(), checkNs)); } @@ -308,7 +309,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO } } else if ("metatype".equals(parentDirectoryName)) { try (InputStream input = Files.newInputStream(file)) { - metatypeDescriptors.add(parseMetatype(input, file.getFileName().toString())); + metatypeDescriptors.addAll(parseMetatype(input, file.getFileName().toString())); } } } @@ -360,8 +361,9 @@ private Descriptor parseScr(InputStream is, String name, boolean checkNs) throws return result; } - private Descriptor parseMetatype(InputStream is, String name) throws IOException { - Descriptor result = new Descriptor(); + private Collection parseMetatype(InputStream is, String name) throws IOException { + List descriptors = new ArrayList<>(); + List properties = new ArrayList<>(); try { XMLEventReader reader = xmlInputFactory.createXMLEventReader(is); while (reader.hasNext()) { @@ -370,25 +372,32 @@ private Descriptor parseMetatype(InputStream is, String name) throws IOException StartElement start = event.asStartElement(); String elementName = start.getName().getLocalPart(); if (elementName.equals("Designate")) { + // each designate defines a mapping between SCR component and metatype, the same metatype may be bound to multiple components Attribute pidAttribute = start.getAttributeByName(new QName("pid")); + Descriptor descriptor = new Descriptor(); + descriptor.properties = properties; if (pidAttribute != null) { - result.name = pidAttribute.getValue(); + descriptor.name = pidAttribute.getValue(); } else { pidAttribute = start.getAttributeByName(new QName("factoryPid")); if (pidAttribute != null) { - result.name = pidAttribute.getValue(); + descriptor.name = pidAttribute.getValue(); } - result.factory = true; + descriptor.factory = true; + } + if (descriptor.name == null) { + throw new IllegalArgumentException("Could not identify (factory)pid for " + name); } + descriptors.add(descriptor); } else if (elementName.equals("AD")) { String propName = start.getAttributeByName(new QName("id")).getValue(); Attribute value = start.getAttributeByName(new QName("default")); Attribute typeAttr = start.getAttributeByName(new QName("type")); String type = typeAttr == null ? "String" : typeAttr.getValue(); if (value == null) { - result.properties.add(new Property(propName, "(metatype)", type)); + properties.add(new Property(propName, "(metatype)", type)); } else { - result.properties.add(new Property(propName, "(metatype)" + value.getValue(), type)); + properties.add(new Property(propName, "(metatype)" + value.getValue(), type)); } } } @@ -396,10 +405,7 @@ private Descriptor parseMetatype(InputStream is, String name) throws IOException } catch (XMLStreamException e) { throw new IOException("Error parsing XML", e); } - if (result.name == null) { - throw new IllegalArgumentException("Could not identify pid for " + name); - } - return result; + return descriptors; } private String cleanText(String input) { diff --git a/bundle/src/test/java/com/adobe/acs/commons/util/impl/BundleDisablerTest.java b/bundle/src/test/java/com/adobe/acs/commons/util/impl/BundleDisablerTest.java index f1e237ed9e..01b60d5f97 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/util/impl/BundleDisablerTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/util/impl/BundleDisablerTest.java @@ -17,53 +17,53 @@ */ package com.adobe.acs.commons.util.impl; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import org.jetbrains.annotations.NotNull; import org.junit.Before; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; -import org.osgi.framework.Bundle; -import org.osgi.framework.BundleContext; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; -import org.osgi.service.component.ComponentContext; +import org.osgi.util.converter.Converters; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; - -import static org.mockito.Mockito.*; +import com.adobe.acs.commons.util.impl.BundleDisabler.Config; @RunWith(MockitoJUnitRunner.class) public class BundleDisablerTest { - @Mock - private ComponentContext componentContext; - @Mock private BundleContext bundleContext; @Mock private Bundle ownBundle; - @InjectMocks - private BundleDisabler disabler; - private final List bundles = new ArrayList(); @Test public void testNullProperties() { - disabler.activate(componentContext, Collections.emptyMap()); + Converters.standardConverter().convert(bundleContext).to(BundleDisabler.Config.class); + new BundleDisabler(bundleContext, configuration(), Collections.emptyList() ); verifyNoMoreInteractions(bundleContext); } @Before public void setUp() { bundles.clear(); - when(componentContext.getBundleContext()).thenReturn(bundleContext); when(bundleContext.getBundles()).then(new Answer() { @Override public Bundle[] answer(final InvocationOnMock invocationOnMock) throws Throwable { @@ -74,7 +74,7 @@ public Bundle[] answer(final InvocationOnMock invocationOnMock) throws Throwable @Test public void shouldNotDisableOwnBundle() { - disabler.activate(componentContext, bundleProperties("my.own.bundle")); + new BundleDisabler(bundleContext, configuration("my.own.bundle"), Collections.emptyList()); } @Test @@ -84,7 +84,7 @@ public void shouldStopBundle() { when(targetBundle.getSymbolicName()).thenReturn("to.stop.bundle"); - disabler.activate(componentContext, bundleProperties("to.stop.bundle")); + new BundleDisabler(bundleContext, configuration("to.stop.bundle"), Collections.emptyList()); try { verify(targetBundle).stop(); @@ -101,7 +101,7 @@ public void shouldNotStopUninstalledBundle() { when(targetBundle.getState()).thenReturn(Bundle.UNINSTALLED); when(targetBundle.getSymbolicName()).thenReturn("to.stop.bundle"); - disabler.activate(componentContext, bundleProperties("to.stop.bundle")); + new BundleDisabler(bundleContext, configuration("to.stop.bundle"), Collections.emptyList()); try { verify(targetBundle, never()).stop(); @@ -110,8 +110,10 @@ public void shouldNotStopUninstalledBundle() { } } - private Map bundleProperties(String... bundles) { - return Collections.singletonMap("bundles", bundles); + + private @NotNull Config configuration(String... bundles) { + Map propertiesMap = Collections.singletonMap("bundles", bundles); + return Converters.standardConverter().convert(propertiesMap).to(BundleDisabler.Config.class); } }