Skip to content

Commit

Permalink
XWIKI-22662: Impossible to access any editor if the content contains …
Browse files Browse the repository at this point in the history
…a call to a macro named "default" (#3650)

* Don't try loading the "default" RequiredRightAnalyzer<MacroBlock> for
  a macro named "default".
* Add a test case for the default macro.
  • Loading branch information
michitux authored Nov 19, 2024
1 parent 4390526 commit 471381d
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -60,6 +62,9 @@ public class DefaultMacroBlockRequiredRightAnalyzer extends AbstractMacroBlockRe
@Named(ScriptMacroAnalyzer.ID)
private RequiredRightAnalyzer<MacroBlock> scriptMacroAnalyzer;

@Inject
private Logger logger;

@Inject
private Provider<DefaultMacroRequiredRightReporter> macroRequiredRightReporterProvider;

Expand All @@ -82,15 +87,12 @@ private List<RequiredRightAnalysisResult> analyzeWithExceptions(MacroBlock macro
{
List<RequiredRightAnalysisResult> result;

try {
// Check if there is an analyzer specifically for this macro, if yes, use it.
RequiredRightAnalyzer<MacroBlock> specificAnalyzer =
this.componentManagerProvider.get().getInstance(
new DefaultParameterizedType(null, RequiredRightAnalyzer.class, MacroBlock.class),
macroBlock.getId()
);
result = specificAnalyzer.analyze(macroBlock);
} catch (ComponentLookupException e) {
Optional<RequiredRightAnalyzer<MacroBlock>>
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<MacroRequiredRightsAnalyzer> macroAnalyzer = getMacroAnalyzer(macroBlock.getId());

Expand All @@ -115,6 +117,30 @@ private List<RequiredRightAnalysisResult> analyzeWithExceptions(MacroBlock macro
return result;
}

private Optional<RequiredRightAnalyzer<MacroBlock>> 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<MacroRequiredRightsAnalyzer> getMacroAnalyzer(String id)
{
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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;
Expand All @@ -67,6 +73,7 @@
*/
@OldcoreTest
@ReferenceComponentList
@ComponentList(DefaultMacroRequiredRightReporter.class)
class DefaultMacroBlockRequiredRightAnalyzerTest
{
@InjectMockComponents
Expand All @@ -80,6 +87,9 @@ class DefaultMacroBlockRequiredRightAnalyzerTest
@Named(ScriptMacroAnalyzer.ID)
private RequiredRightAnalyzer<MacroBlock> scriptMacroAnalyzer;

@MockComponent
private MacroRequiredRightsAnalyzer defaultMacroRequiredRightsAnalyzer;

@MockComponent
private MacroManager macroManager;

Expand Down Expand Up @@ -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<RequiredRightAnalysisResult> results = this.analyzer.analyze(block);
assertEquals(1, results.size());
RequiredRightAnalysisResult result = results.get(0);
assertEquals(List.of(RequiredRight.PROGRAM), result.getRequiredRights());
}
}

0 comments on commit 471381d

Please sign in to comment.