From b82e7fea10e43e6532e5da12850d8c70d58c2630 Mon Sep 17 00:00:00 2001 From: Michael Hamann Date: Tue, 19 Nov 2024 16:09:08 +0100 Subject: [PATCH] XWIKI-22662: Impossible to access any editor if the content contains a call to a macro named "default" (#3650) * Don't try loading the "default" RequiredRightAnalyzer for a macro named "default". * Add a test case for the default macro. (cherry picked from commit 471381d892d1e5f7f64823b6e28a6bb9f0fd2a23) --- ...efaultMacroBlockRequiredRightAnalyzer.java | 44 +++++++++++++++---- ...ltMacroBlockRequiredRightAnalyzerTest.java | 31 +++++++++++++ 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-requiredrights/xwiki-platform-security-requiredrights-default/src/main/java/org/xwiki/platform/security/requiredrights/internal/analyzer/DefaultMacroBlockRequiredRightAnalyzer.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-requiredrights/xwiki-platform-security-requiredrights-default/src/main/java/org/xwiki/platform/security/requiredrights/internal/analyzer/DefaultMacroBlockRequiredRightAnalyzer.java index 5edf39027aad..f7a51bd7ff9d 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-requiredrights/xwiki-platform-security-requiredrights-default/src/main/java/org/xwiki/platform/security/requiredrights/internal/analyzer/DefaultMacroBlockRequiredRightAnalyzer.java +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-requiredrights/xwiki-platform-security-requiredrights-default/src/main/java/org/xwiki/platform/security/requiredrights/internal/analyzer/DefaultMacroBlockRequiredRightAnalyzer.java @@ -27,6 +27,8 @@ import javax.inject.Provider; import javax.inject.Singleton; +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.slf4j.Logger; import org.xwiki.component.annotation.Component; import org.xwiki.component.manager.ComponentLookupException; import org.xwiki.component.manager.ComponentManager; @@ -60,6 +62,9 @@ public class DefaultMacroBlockRequiredRightAnalyzer extends AbstractMacroBlockRe @Named(ScriptMacroAnalyzer.ID) private RequiredRightAnalyzer scriptMacroAnalyzer; + @Inject + private Logger logger; + @Inject private Provider macroRequiredRightReporterProvider; @@ -82,15 +87,12 @@ private List analyzeWithExceptions(MacroBlock macro { List result; - try { - // Check if there is an analyzer specifically for this macro, if yes, use it. - RequiredRightAnalyzer specificAnalyzer = - this.componentManagerProvider.get().getInstance( - new DefaultParameterizedType(null, RequiredRightAnalyzer.class, MacroBlock.class), - macroBlock.getId() - ); - result = specificAnalyzer.analyze(macroBlock); - } catch (ComponentLookupException e) { + Optional> + specificAnalyzer = getMacroBlockRequiredRightAnalyzer(macroBlock); + + if (specificAnalyzer.isPresent()) { + result = specificAnalyzer.get().analyze(macroBlock); + } else { // Check if there is a macro analyzer for this macro, if yes, use it. Optional macroAnalyzer = getMacroAnalyzer(macroBlock.getId()); @@ -115,6 +117,30 @@ private List analyzeWithExceptions(MacroBlock macro return result; } + private Optional> getMacroBlockRequiredRightAnalyzer(MacroBlock macroBlock) + { + String macroId = macroBlock.getId(); + DefaultParameterizedType roleType = + new DefaultParameterizedType(null, RequiredRightAnalyzer.class, MacroBlock.class); + ComponentManager componentManager = this.componentManagerProvider.get(); + + try { + // Check if there is an analyzer specifically for this macro, if yes, use it. + // Don't try loading the "default" analyzer as this would cause an endless recursion. + // For the "default" macro, a MacroRequiredRightsAnalyzer component should be created if needed. + if (!"default".equals(macroId) && componentManager.hasComponent(roleType, macroId)) { + return Optional.of(componentManager.getInstance(roleType, macroId)); + } + } catch (ComponentLookupException e) { + this.logger.warn( + "The macro block required rights analyzer for macro [{}] failed to be initialized, root cause: [{}]", + macroId, ExceptionUtils.getRootCauseMessage(e)); + this.logger.debug("Full exception trace: ", e); + } + + return Optional.empty(); + } + private Optional getMacroAnalyzer(String id) { try { diff --git a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-requiredrights/xwiki-platform-security-requiredrights-default/src/test/java/org/xwiki/platform/security/requiredrights/internal/analyzer/DefaultMacroBlockRequiredRightAnalyzerTest.java b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-requiredrights/xwiki-platform-security-requiredrights-default/src/test/java/org/xwiki/platform/security/requiredrights/internal/analyzer/DefaultMacroBlockRequiredRightAnalyzerTest.java index cda0f1907b70..87635456b5df 100644 --- a/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-requiredrights/xwiki-platform-security-requiredrights-default/src/test/java/org/xwiki/platform/security/requiredrights/internal/analyzer/DefaultMacroBlockRequiredRightAnalyzerTest.java +++ b/xwiki-platform-core/xwiki-platform-security/xwiki-platform-security-requiredrights/xwiki-platform-security-requiredrights-default/src/test/java/org/xwiki/platform/security/requiredrights/internal/analyzer/DefaultMacroBlockRequiredRightAnalyzerTest.java @@ -28,6 +28,10 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.xwiki.platform.security.requiredrights.MacroRequiredRight; +import org.xwiki.platform.security.requiredrights.MacroRequiredRightReporter; +import org.xwiki.platform.security.requiredrights.MacroRequiredRightsAnalyzer; +import org.xwiki.platform.security.requiredrights.RequiredRight; import org.xwiki.platform.security.requiredrights.RequiredRightAnalysisResult; import org.xwiki.platform.security.requiredrights.RequiredRightAnalyzer; import org.xwiki.rendering.block.Block; @@ -42,6 +46,7 @@ import org.xwiki.rendering.macro.descriptor.MacroDescriptor; import org.xwiki.rendering.macro.script.ScriptMacro; import org.xwiki.rendering.syntax.Syntax; +import org.xwiki.test.annotation.ComponentList; import org.xwiki.test.junit5.mockito.InjectMockComponents; import org.xwiki.test.junit5.mockito.MockComponent; @@ -53,6 +58,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -67,6 +73,7 @@ */ @OldcoreTest @ReferenceComponentList +@ComponentList(DefaultMacroRequiredRightReporter.class) class DefaultMacroBlockRequiredRightAnalyzerTest { @InjectMockComponents @@ -80,6 +87,9 @@ class DefaultMacroBlockRequiredRightAnalyzerTest @Named(ScriptMacroAnalyzer.ID) private RequiredRightAnalyzer scriptMacroAnalyzer; + @MockComponent + private MacroRequiredRightsAnalyzer defaultMacroRequiredRightsAnalyzer; + @MockComponent private MacroManager macroManager; @@ -181,4 +191,25 @@ void analyzeContentRecursively(boolean isWikiContent) throws Exception assertEquals(List.of(), result); } } + + @Test + void analyzeDefaultMacro() + { + String macroName = "default"; + + MacroBlock block = mock(); + when(block.getId()).thenReturn(macroName); + + doAnswer(invocationOnMock -> { + MacroBlock macroBlock = invocationOnMock.getArgument(0); + MacroRequiredRightReporter reporter = invocationOnMock.getArgument(1); + reporter.report(macroBlock, List.of(MacroRequiredRight.PROGRAM), "test.summary"); + return null; + }).when(this.defaultMacroRequiredRightsAnalyzer).analyze(eq(block), any()); + + List results = this.analyzer.analyze(block); + assertEquals(1, results.size()); + RequiredRightAnalysisResult result = results.get(0); + assertEquals(List.of(RequiredRight.PROGRAM), result.getRequiredRights()); + } }