Skip to content

Commit 0a705e8

Browse files
committed
XWIKI-22759: The content macro is missing a required rights analyzer
* Add a required rights analyzer for the content macro. * Add tests.
1 parent cc74dc8 commit 0a705e8

File tree

4 files changed

+295
-0
lines changed

4 files changed

+295
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* See the NOTICE file distributed with this work for additional
3+
* information regarding copyright ownership.
4+
*
5+
* This is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU Lesser General Public License as
7+
* published by the Free Software Foundation; either version 2.1 of
8+
* the License, or (at your option) any later version.
9+
*
10+
* This software is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
* Lesser General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Lesser General Public
16+
* License along with this software; if not, write to the Free
17+
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
18+
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
19+
*/
20+
package org.xwiki.rendering.internal.macro;
21+
22+
import java.util.List;
23+
import java.util.Objects;
24+
25+
import javax.inject.Inject;
26+
import javax.inject.Named;
27+
import javax.inject.Singleton;
28+
29+
import org.xwiki.component.annotation.Component;
30+
import org.xwiki.platform.security.requiredrights.MacroRequiredRight;
31+
import org.xwiki.platform.security.requiredrights.MacroRequiredRightReporter;
32+
import org.xwiki.platform.security.requiredrights.MacroRequiredRightsAnalyzer;
33+
import org.xwiki.properties.ConverterManager;
34+
import org.xwiki.rendering.block.MacroBlock;
35+
import org.xwiki.rendering.macro.source.MacroContentSourceReference;
36+
import org.xwiki.rendering.syntax.Syntax;
37+
38+
/**
39+
* Required rights analyzer for content macro.
40+
*
41+
* @version $Id$
42+
* @since 16.4.7
43+
* @since 16.10.3
44+
* @since 17.0.0
45+
*/
46+
@Component
47+
@Singleton
48+
@Named("content")
49+
public class ContentMacroRequiredRightsAnalyzer implements MacroRequiredRightsAnalyzer
50+
{
51+
@Inject
52+
private ConverterManager converterManager;
53+
54+
@Override
55+
public void analyze(MacroBlock macroBlock, MacroRequiredRightReporter reporter)
56+
{
57+
List<Syntax> contentSyntaxes = getParameterValues(macroBlock, Syntax.class, "syntax");
58+
List<MacroContentSourceReference> sources = getParameterValues(macroBlock, MacroContentSourceReference.class,
59+
"source");
60+
61+
if (!sources.isEmpty()) {
62+
// If there are several sources, we don't know which one will win - just analyze all, having more than
63+
// one source isn't a real use case.
64+
for (MacroContentSourceReference source : sources) {
65+
if (MacroContentSourceReference.TYPE_SCRIPT.equals(source.getType())) {
66+
reporter.report(macroBlock, List.of(MacroRequiredRight.SCRIPT, MacroRequiredRight.MAYBE_PROGRAM),
67+
"rendering.macro.content.requiredRights.scriptSource");
68+
} else if (MacroContentSourceReference.TYPE_STRING.equals(source.getType())) {
69+
analyzeContentWithSyntaxes(macroBlock, reporter, contentSyntaxes, source.getReference());
70+
}
71+
}
72+
} else {
73+
analyzeContentWithSyntaxes(macroBlock, reporter, contentSyntaxes, macroBlock.getContent());
74+
}
75+
}
76+
77+
private static void analyzeContentWithSyntaxes(MacroBlock macroBlock, MacroRequiredRightReporter reporter,
78+
List<Syntax> contentSyntaxes, String content)
79+
{
80+
if (contentSyntaxes.isEmpty()) {
81+
reporter.analyzeContent(macroBlock, content);
82+
} else {
83+
// If there are several syntax parameters, we don't know which one will really be used, so analyze
84+
// with all to catch dangerous content in all syntaxes. In practice, there should be at most a single syntax
85+
// parameter.
86+
contentSyntaxes.forEach(syntax -> reporter.analyzeContent(macroBlock, content, syntax));
87+
}
88+
}
89+
90+
private <T> List<T> getParameterValues(MacroBlock macroBlock, Class<T> tClass, String parameterName)
91+
{
92+
return macroBlock.getParameters().entrySet().stream()
93+
.filter(entry -> parameterName.equalsIgnoreCase(entry.getKey()))
94+
.map(entry -> {
95+
try {
96+
return this.converterManager.<T>convert(tClass, entry.getValue());
97+
} catch (Exception e) {
98+
// Ignore invalid values.
99+
return null;
100+
}
101+
})
102+
.filter(Objects::nonNull)
103+
.toList();
104+
}
105+
}

xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/resources/ApplicationResources.properties

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,7 @@ rendering.macro.htmlRequiredRights.wikiContent=An HTML macro with wiki content c
5151
on the content, please review the content carefully.
5252
rendering.macro.htmlRequiredRights.dangerousContent=An HTML macro that contains content that would be removed by \
5353
cleaning in restricted mode requires script right to avoid restricted cleaning.
54+
55+
rendering.macro.content.requiredRights.scriptSource=Referencing a script variable in the source parameter of the \
56+
content macro requires script right. Additionally, the script variable could contain arbitrary wiki syntax that \
57+
could require any right including programming right.

xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/resources/META-INF/components.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
500:org.xwiki.rendering.internal.util.XWikiErrorBlockGenerator
66
500:org.xwiki.rendering.internal.wiki.XWikiWikiModel
77
500:org.xwiki.rendering.internal.macro.XWikiHTMLRawBlockFilter
8+
org.xwiki.rendering.internal.macro.ContentMacroRequiredRightsAnalyzer
89
org.xwiki.rendering.internal.macro.HTMLMacroRequiredRightsAnalyzer
910
org.xwiki.rendering.internal.macro.RawMacroRequiredRightsAnalyzer
1011
org.xwiki.rendering.internal.resolver.AttachmentResourceReferenceEntityReferenceResolver
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
/*
2+
* See the NOTICE file distributed with this work for additional
3+
* information regarding copyright ownership.
4+
*
5+
* This is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU Lesser General Public License as
7+
* published by the Free Software Foundation; either version 2.1 of
8+
* the License, or (at your option) any later version.
9+
*
10+
* This software is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
* Lesser General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU Lesser General Public
16+
* License along with this software; if not, write to the Free
17+
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
18+
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
19+
*/
20+
package org.xwiki.rendering.internal.macro;
21+
22+
import java.util.List;
23+
import java.util.Map;
24+
import java.util.stream.Stream;
25+
26+
import javax.inject.Inject;
27+
import javax.inject.Named;
28+
import javax.inject.Provider;
29+
30+
import org.junit.jupiter.api.BeforeEach;
31+
import org.junit.jupiter.api.Test;
32+
import org.junit.jupiter.params.ParameterizedTest;
33+
import org.junit.jupiter.params.provider.Arguments;
34+
import org.junit.jupiter.params.provider.MethodSource;
35+
import org.xwiki.component.manager.ComponentManager;
36+
import org.xwiki.platform.security.requiredrights.MacroRequiredRight;
37+
import org.xwiki.platform.security.requiredrights.MacroRequiredRightReporter;
38+
import org.xwiki.properties.internal.DefaultConverterManager;
39+
import org.xwiki.properties.internal.converter.ConvertUtilsConverter;
40+
import org.xwiki.properties.internal.converter.EnumConverter;
41+
import org.xwiki.rendering.block.MacroBlock;
42+
import org.xwiki.rendering.internal.macro.source.MacroContentSourceReferenceConverter;
43+
import org.xwiki.rendering.internal.syntax.DefaultSyntaxRegistry;
44+
import org.xwiki.rendering.internal.syntax.SyntaxConverter;
45+
import org.xwiki.rendering.syntax.Syntax;
46+
import org.xwiki.rendering.syntax.SyntaxRegistry;
47+
import org.xwiki.test.annotation.ComponentList;
48+
import org.xwiki.test.junit5.mockito.ComponentTest;
49+
import org.xwiki.test.junit5.mockito.InjectComponentManager;
50+
import org.xwiki.test.junit5.mockito.InjectMockComponents;
51+
import org.xwiki.test.junit5.mockito.MockComponent;
52+
import org.xwiki.test.mockito.MockitoComponentManager;
53+
54+
import static org.mockito.Mockito.mock;
55+
import static org.mockito.Mockito.times;
56+
import static org.mockito.Mockito.verify;
57+
import static org.mockito.Mockito.verifyNoMoreInteractions;
58+
import static org.mockito.Mockito.when;
59+
60+
@ComponentTest
61+
@ComponentList({ DefaultConverterManager.class, EnumConverter.class, ConvertUtilsConverter.class,
62+
SyntaxConverter.class, MacroContentSourceReferenceConverter.class, DefaultSyntaxRegistry.class })
63+
// Adding constants for string literals wouldn't really help the readability of the test.
64+
@SuppressWarnings("checkstyle:MultipleStringLiterals")
65+
class ContentMacroRequiredRightsAnalyzerTest
66+
{
67+
protected static final String XWIKI_2_0 = "xwiki/2.0";
68+
69+
protected static final String XWIKI_2_1 = "xwiki/2.1";
70+
71+
protected static final String INVALID = "invalid";
72+
73+
@InjectMockComponents
74+
private ContentMacroRequiredRightsAnalyzer analyzer;
75+
76+
@InjectComponentManager
77+
private MockitoComponentManager componentManager;
78+
79+
@Inject
80+
private SyntaxRegistry syntaxRegistry;
81+
82+
@MockComponent
83+
@Named("context")
84+
private Provider<ComponentManager> contextComponentManager;
85+
86+
@BeforeEach
87+
void beforeEach()
88+
{
89+
when(this.contextComponentManager.get()).thenReturn(this.componentManager);
90+
this.syntaxRegistry.registerSyntaxes(Syntax.XWIKI_2_0, Syntax.XWIKI_2_1);
91+
}
92+
93+
@ParameterizedTest
94+
@MethodSource("provideDifferentContentSyntaxes")
95+
void analyzeDifferentContentSyntaxes(Map<String, String> parameters, List<Syntax> syntaxes)
96+
{
97+
String content = "Content";
98+
MacroBlock macroBlock = new MacroBlock("content", parameters, content, false);
99+
100+
MacroRequiredRightReporter reporter = mock();
101+
102+
this.analyzer.analyze(macroBlock, reporter);
103+
104+
// Verify that the content is analyzed
105+
if (syntaxes.isEmpty()) {
106+
verify(reporter).analyzeContent(macroBlock, content);
107+
} else {
108+
for (Syntax syntax : syntaxes) {
109+
verify(reporter).analyzeContent(macroBlock, content, syntax);
110+
}
111+
}
112+
}
113+
114+
@Test
115+
void analyzeWithScriptSource()
116+
{
117+
String content = "Content";
118+
MacroBlock macroBlock = new MacroBlock("content", Map.of("source", "script:variable"), content, false);
119+
120+
MacroRequiredRightReporter reporter = mock();
121+
122+
this.analyzer.analyze(macroBlock, reporter);
123+
124+
verify(reporter).report(macroBlock, List.of(MacroRequiredRight.SCRIPT, MacroRequiredRight.MAYBE_PROGRAM),
125+
"rendering.macro.content.requiredRights.scriptSource");
126+
}
127+
128+
@Test
129+
void analyzeWithStringSource()
130+
{
131+
String stringContent1 = "Content1";
132+
String stringContent2 = "Content2";
133+
MacroBlock macroBlock = new MacroBlock("content",
134+
Map.of("source", "string:" + stringContent1, "Source", "string:" + stringContent2,
135+
"SyntaX", XWIKI_2_0, "synTaX", XWIKI_2_1),
136+
"Content", false);
137+
138+
MacroRequiredRightReporter reporter = mock();
139+
140+
this.analyzer.analyze(macroBlock, reporter);
141+
142+
for (Syntax syntax : List.of(Syntax.XWIKI_2_1, Syntax.XWIKI_2_0)) {
143+
for (String content : List.of(stringContent1, stringContent2)) {
144+
verify(reporter).analyzeContent(macroBlock, content, syntax);
145+
}
146+
}
147+
148+
verifyNoMoreInteractions(reporter);
149+
}
150+
151+
@Test
152+
void analyzeWithDifferentSourcesNoSyntax()
153+
{
154+
MacroBlock macroBlock = new MacroBlock("content", Map.of(
155+
"Source", "script:variable", "sourcE", "script:variable2",
156+
"souRCE", "string:1", "SOURCE", "string:2",
157+
"soUrce", "invalid:value"),
158+
false);
159+
160+
MacroRequiredRightReporter reporter = mock();
161+
162+
this.analyzer.analyze(macroBlock, reporter);
163+
164+
verify(reporter, times(2))
165+
.report(macroBlock, List.of(MacroRequiredRight.SCRIPT, MacroRequiredRight.MAYBE_PROGRAM),
166+
"rendering.macro.content.requiredRights.scriptSource");
167+
168+
verify(reporter).analyzeContent(macroBlock, "1");
169+
verify(reporter).analyzeContent(macroBlock, "2");
170+
verifyNoMoreInteractions(reporter);
171+
}
172+
173+
private static Stream<Arguments> provideDifferentContentSyntaxes()
174+
{
175+
return Stream.of(
176+
Arguments.of(Map.of("syntax", XWIKI_2_0), List.of(Syntax.XWIKI_2_0)),
177+
Arguments.of(Map.of("Syntax", XWIKI_2_1), List.of(Syntax.XWIKI_2_1)),
178+
Arguments.of(Map.of("sYnTax", INVALID, "syntaX", XWIKI_2_1), List.of(Syntax.XWIKI_2_1)),
179+
Arguments.of(Map.of("syNtax", XWIKI_2_0, "synTax", XWIKI_2_1), List.of(Syntax.XWIKI_2_0,
180+
Syntax.XWIKI_2_1)),
181+
Arguments.of(Map.of("syntAX", INVALID), List.of()),
182+
Arguments.of(Map.of(), List.of())
183+
);
184+
}
185+
}

0 commit comments

Comments
 (0)