Skip to content

Commit ee642f9

Browse files
michituxmichitux
committed
XWIKI-22736: Improve right checks in property values provider
* Check VIEW right before rendering titles * Use generic icons when the specific icon can’t be accessed * Don't provide any metadata when the page itself can’t be accessed. * Add tests.
1 parent 3d451e9 commit ee642f9

File tree

8 files changed

+209
-74
lines changed

8 files changed

+209
-74
lines changed

‎xwiki-platform-core/xwiki-platform-rest/xwiki-platform-rest-server/src/main/java/org/xwiki/rest/internal/resources/classes/AbstractDocumentListClassPropertyValuesProvider.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
import org.xwiki.rest.model.jaxb.PropertyValue;
4040
import org.xwiki.rest.model.jaxb.PropertyValues;
4141
import org.xwiki.rest.resources.classes.ClassPropertyValuesProvider;
42+
import org.xwiki.security.authorization.ContextualAuthorizationManager;
43+
import org.xwiki.security.authorization.Right;
4244

4345
import com.xpn.xwiki.XWiki;
4446
import com.xpn.xwiki.XWikiContext;
@@ -61,6 +63,9 @@ public abstract class AbstractDocumentListClassPropertyValuesProvider<T extends
6163
@Inject
6264
protected IconManager iconManager;
6365

66+
@Inject
67+
protected ContextualAuthorizationManager contextualAuthorizationManager;
68+
6469
@Inject
6570
@Named("compact")
6671
private EntityReferenceSerializer<String> compactSerializer;
@@ -92,7 +97,7 @@ public PropertyValue getValue(ClassPropertyReference propertyReference, Object r
9297
DocumentReference documentReference = this.documentReferenceResolver.resolve(reference, propertyReference);
9398

9499
PropertyValue propertyValue = null;
95-
if (exists(documentReference)) {
100+
if (this.contextualAuthorizationManager.hasAccess(Right.VIEW, documentReference) && exists(documentReference)) {
96101
propertyValue = super.getValue(propertyReference, documentReference);
97102
}
98103

‎xwiki-platform-core/xwiki-platform-rest/xwiki-platform-rest-server/src/main/java/org/xwiki/rest/internal/resources/classes/GroupsClassPropertyValuesProvider.java

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.xwiki.query.QueryBuilder;
3737
import org.xwiki.rendering.syntax.Syntax;
3838
import org.xwiki.rest.model.jaxb.PropertyValues;
39+
import org.xwiki.security.authorization.Right;
3940
import org.xwiki.wiki.user.UserScope;
4041
import org.xwiki.wiki.user.WikiUserManager;
4142

@@ -94,19 +95,22 @@ protected Map<String, Object> getIcon(DocumentReference groupReference)
9495
{
9596
Map<String, Object> icon = new HashMap<>();
9697
XWikiContext xcontext = this.xcontextProvider.get();
97-
try {
98-
XWikiDocument groupProfileDocument = xcontext.getWiki().getDocument(groupReference, xcontext);
99-
XWikiAttachment avatarAttachment = getFirstImageAttachment(groupProfileDocument, xcontext);
100-
if (avatarAttachment != null) {
101-
icon.put(IconManager.META_DATA_URL, xcontext.getWiki().getURL(avatarAttachment.getReference(),
102-
"download", "width=30&height=30&keepAspectRatio=true", null, xcontext));
103-
icon.put(IconManager.META_DATA_ICON_SET_TYPE, IconType.IMAGE.name());
98+
if (this.contextualAuthorizationManager.hasAccess(Right.VIEW, groupReference)) {
99+
try {
100+
XWikiDocument groupProfileDocument = xcontext.getWiki().getDocument(groupReference, xcontext);
101+
XWikiAttachment avatarAttachment = getFirstImageAttachment(groupProfileDocument, xcontext);
102+
if (avatarAttachment != null) {
103+
icon.put(IconManager.META_DATA_URL, xcontext.getWiki().getURL(avatarAttachment.getReference(),
104+
"download", "width=30&height=30&keepAspectRatio=true", null, xcontext));
105+
icon.put(IconManager.META_DATA_ICON_SET_TYPE, IconType.IMAGE.name());
106+
}
107+
} catch (XWikiException e) {
108+
this.logger.warn(
109+
"Failed to read the avatar of group [{}]. Root cause is [{}]. Using the default avatar instead.",
110+
groupReference.getName(), ExceptionUtils.getRootCauseMessage(e));
104111
}
105-
} catch (XWikiException e) {
106-
this.logger.warn(
107-
"Failed to read the avatar of group [{}]. Root cause is [{}]. Using the default avatar instead.",
108-
groupReference.getName(), ExceptionUtils.getRootCauseMessage(e));
109112
}
113+
110114
if (!icon.containsKey(IconManager.META_DATA_URL)) {
111115
try {
112116
icon = this.iconManager.getMetaData(DEFAULT_ICON_NAME);
@@ -132,14 +136,17 @@ private XWikiAttachment getFirstImageAttachment(XWikiDocument document, XWikiCon
132136
@Override
133137
protected String getLabel(DocumentReference groupReference, Object currentLabel)
134138
{
135-
try {
136-
XWikiContext xcontext = this.xcontextProvider.get();
137-
return xcontext.getWiki().getDocument(groupReference, xcontext).getRenderedTitle(Syntax.PLAIN_1_0,
138-
xcontext);
139-
} catch (XWikiException e) {
140-
this.logger.warn("Failed to get the title of group [{}]. Root cause is [{}].",
141-
this.entityReferenceSerializer.serialize(groupReference), ExceptionUtils.getRootCauseMessage(e));
142-
return super.getLabel(groupReference, currentLabel);
139+
if (this.contextualAuthorizationManager.hasAccess(Right.VIEW, groupReference)) {
140+
try {
141+
XWikiContext xcontext = this.xcontextProvider.get();
142+
return xcontext.getWiki().getDocument(groupReference, xcontext).getRenderedTitle(Syntax.PLAIN_1_0,
143+
xcontext);
144+
} catch (XWikiException e) {
145+
this.logger.warn("Failed to get the title of group [{}]. Root cause is [{}].",
146+
this.entityReferenceSerializer.serialize(groupReference), ExceptionUtils.getRootCauseMessage(e));
147+
}
143148
}
149+
150+
return super.getLabel(groupReference, currentLabel);
144151
}
145152
}

‎xwiki-platform-core/xwiki-platform-rest/xwiki-platform-rest-server/src/main/java/org/xwiki/rest/internal/resources/classes/PageClassPropertyValuesProvider.java

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.xwiki.rendering.syntax.Syntax;
3838
import org.xwiki.rest.model.jaxb.PropertyValues;
3939
import org.xwiki.security.authorization.AuthorExecutor;
40+
import org.xwiki.security.authorization.Right;
4041

4142
import com.xpn.xwiki.XWiki;
4243
import com.xpn.xwiki.XWikiContext;
@@ -95,17 +96,23 @@ protected Map<String, Object> getIcon(DocumentReference documentReference)
9596

9697
private String getLabel(EntityReference entityReference)
9798
{
98-
String label;
99-
try {
100-
XWikiContext xcontext = this.xcontextProvider.get();
101-
XWikiDocument document;
102-
document = xcontext.getWiki().getDocument(entityReference, xcontext).getTranslatedDocument(xcontext);
103-
label = document.getRenderedTitle(Syntax.PLAIN_1_0, xcontext);
104-
} catch (XWikiException e) {
105-
this.logger.error("Error while loading the document [{}]. Root cause is [{}]", entityReference,
106-
ExceptionUtils.getRootCause(e));
107-
if (entityReference instanceof DocumentReference) {
108-
label = super.getLabel((DocumentReference) entityReference, "");
99+
String label = null;
100+
101+
if (this.contextualAuthorizationManager.hasAccess(Right.VIEW, entityReference)) {
102+
try {
103+
XWikiContext xcontext = this.xcontextProvider.get();
104+
XWikiDocument document;
105+
document = xcontext.getWiki().getDocument(entityReference, xcontext).getTranslatedDocument(xcontext);
106+
label = document.getRenderedTitle(Syntax.PLAIN_1_0, xcontext);
107+
} catch (XWikiException e) {
108+
this.logger.error("Error while loading the document [{}]. Root cause is [{}]", entityReference,
109+
ExceptionUtils.getRootCauseMessage(e), e);
110+
}
111+
}
112+
113+
if (label == null) {
114+
if (entityReference instanceof DocumentReference documentReference) {
115+
label = super.getLabel(documentReference, "");
109116
} else {
110117
label = entityReference.getName();
111118
}

‎xwiki-platform-core/xwiki-platform-rest/xwiki-platform-rest-server/src/main/java/org/xwiki/rest/internal/resources/classes/UsersClassPropertyValuesProvider.java

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.xwiki.model.reference.DocumentReference;
3636
import org.xwiki.query.QueryBuilder;
3737
import org.xwiki.rest.model.jaxb.PropertyValues;
38+
import org.xwiki.security.authorization.Right;
3839
import org.xwiki.user.UserConfiguration;
3940
import org.xwiki.user.UserProperties;
4041
import org.xwiki.user.UserPropertiesResolver;
@@ -106,21 +107,25 @@ protected PropertyValues getAllowedValues(UsersClass propertyDefinition, int lim
106107
protected Map<String, Object> getIcon(DocumentReference userReference)
107108
{
108109
Map<String, Object> icon = new HashMap<>();
109-
XWikiContext xcontext = this.xcontextProvider.get();
110-
try {
111-
XWikiDocument userProfileDocument = xcontext.getWiki().getDocument(userReference, xcontext);
112-
String avatar = userProfileDocument.getStringValue("avatar");
113-
XWikiAttachment avatarAttachment = userProfileDocument.getAttachment(avatar);
114-
if (avatarAttachment != null && avatarAttachment.isImage(xcontext)) {
115-
icon.put(IconManager.META_DATA_URL, xcontext.getWiki().getURL(avatarAttachment.getReference(),
116-
"download", "width=30&height=30&keepAspectRatio=true", null, xcontext));
117-
icon.put(IconManager.META_DATA_ICON_SET_TYPE, IconType.IMAGE.name());
110+
111+
if (this.contextualAuthorizationManager.hasAccess(Right.VIEW, userReference)) {
112+
XWikiContext xcontext = this.xcontextProvider.get();
113+
try {
114+
XWikiDocument userProfileDocument = xcontext.getWiki().getDocument(userReference, xcontext);
115+
String avatar = userProfileDocument.getStringValue("avatar");
116+
XWikiAttachment avatarAttachment = userProfileDocument.getAttachment(avatar);
117+
if (avatarAttachment != null && avatarAttachment.isImage(xcontext)) {
118+
icon.put(IconManager.META_DATA_URL, xcontext.getWiki().getURL(avatarAttachment.getReference(),
119+
"download", "width=30&height=30&keepAspectRatio=true", null, xcontext));
120+
icon.put(IconManager.META_DATA_ICON_SET_TYPE, IconType.IMAGE.name());
121+
}
122+
} catch (XWikiException e) {
123+
this.logger.warn(
124+
"Failed to read the avatar of user [{}]. Root cause is [{}]. Using the default avatar instead.",
125+
userReference.getName(), ExceptionUtils.getRootCauseMessage(e));
118126
}
119-
} catch (XWikiException e) {
120-
this.logger.warn(
121-
"Failed to read the avatar of user [{}]. Root cause is [{}]. Using the default avatar instead.",
122-
userReference.getName(), ExceptionUtils.getRootCauseMessage(e));
123127
}
128+
124129
if (!icon.containsKey(IconManager.META_DATA_URL)) {
125130
try {
126131
icon = this.iconManager.getMetaData(DEFAULT_ICON_NAME);

‎xwiki-platform-core/xwiki-platform-rest/xwiki-platform-rest-server/src/test/java/org/xwiki/rest/internal/resources/classes/AbstractListClassPropertyValuesProviderTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,21 @@
2020
package org.xwiki.rest.internal.resources.classes;
2121

2222
import java.util.Arrays;
23+
import java.util.Map;
2324

2425
import javax.inject.Named;
2526
import javax.inject.Provider;
2627

2728
import org.xwiki.component.manager.ComponentManager;
2829
import org.xwiki.component.util.DefaultParameterizedType;
30+
import org.xwiki.icon.IconManager;
2931
import org.xwiki.model.reference.ClassPropertyReference;
3032
import org.xwiki.model.reference.DocumentReference;
3133
import org.xwiki.query.Query;
3234
import org.xwiki.query.QueryBuilder;
3335
import org.xwiki.query.QueryParameter;
3436
import org.xwiki.rest.resources.classes.ClassPropertyValuesProvider;
37+
import org.xwiki.security.authorization.ContextualAuthorizationManager;
3538
import org.xwiki.test.junit5.mockito.InjectComponentManager;
3639
import org.xwiki.test.junit5.mockito.MockComponent;
3740

@@ -42,6 +45,7 @@
4245
import com.xpn.xwiki.objects.classes.ListClass;
4346
import com.xpn.xwiki.objects.classes.PropertyClass;
4447

48+
import static org.mockito.ArgumentMatchers.anyString;
4549
import static org.mockito.Mockito.mock;
4650
import static org.mockito.Mockito.when;
4751

@@ -69,16 +73,27 @@ public abstract class AbstractListClassPropertyValuesProviderTest
6973
@Named("usedValues")
7074
protected QueryBuilder<ListClass> usedValuesQueryBuilder;
7175

76+
@MockComponent
77+
protected ContextualAuthorizationManager authorizationManager;
78+
7279
@InjectComponentManager
7380
protected ComponentManager componentManager;
7481

82+
@MockComponent
83+
protected IconManager iconManager;
84+
85+
@MockComponent
86+
@Named("readonly")
87+
protected Provider<XWikiContext> readonlyXWikiContextProvider;
88+
7589
public void configure() throws Exception
7690
{
7791
Provider<XWikiContext> xcontextProvider = this.componentManager.getInstance(XWikiContext.TYPE_PROVIDER);
7892
BaseClass xclass = mock(BaseClass.class);
7993
DocumentReference authorReference = new DocumentReference("wiki", "Users", "Alice");
8094

8195
when(xcontextProvider.get()).thenReturn(this.xcontext);
96+
when(this.readonlyXWikiContextProvider.get()).thenReturn(this.xcontext);
8297
when(this.xcontext.getWiki()).thenReturn(this.xwiki);
8398
when(this.classDocument.getXClass()).thenReturn(xclass);
8499
when(this.classDocument.getDocumentReference()).thenReturn(this.classReference);
@@ -89,6 +104,8 @@ public void configure() throws Exception
89104
when(this.usedValuesQuery.bindValue("text")).thenReturn(queryParameter);
90105
when(queryParameter.anyChars()).thenReturn(queryParameter);
91106
when(queryParameter.literal("foo")).thenReturn(queryParameter);
107+
108+
when(this.iconManager.getMetaData(anyString())).thenAnswer(i -> mock(Map.class, i.<String>getArgument(0)));
92109
}
93110

94111
protected void addProperty(String name, PropertyClass definition, boolean withQueryBuilders) throws Exception

‎xwiki-platform-core/xwiki-platform-rest/xwiki-platform-rest-server/src/test/java/org/xwiki/rest/internal/resources/classes/GroupsClassPropertyValuesProviderTest.java

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
import org.junit.jupiter.api.BeforeEach;
2626
import org.junit.jupiter.api.Test;
27+
import org.junit.jupiter.params.ParameterizedTest;
28+
import org.junit.jupiter.params.provider.ValueSource;
2729
import org.xwiki.model.reference.AttachmentReference;
2830
import org.xwiki.model.reference.ClassPropertyReference;
2931
import org.xwiki.model.reference.DocumentReference;
@@ -41,8 +43,10 @@
4143
import com.xpn.xwiki.objects.classes.GroupsClass;
4244

4345
import static org.junit.jupiter.api.Assertions.assertEquals;
44-
import static org.junit.jupiter.api.Assertions.assertTrue;
46+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
47+
import static org.mockito.ArgumentMatchers.any;
4548
import static org.mockito.Mockito.mock;
49+
import static org.mockito.Mockito.mockingDetails;
4650
import static org.mockito.Mockito.times;
4751
import static org.mockito.Mockito.verify;
4852
import static org.mockito.Mockito.when;
@@ -53,7 +57,7 @@
5357
* @version $Id$
5458
*/
5559
@ComponentTest
56-
public class GroupsClassPropertyValuesProviderTest extends AbstractListClassPropertyValuesProviderTest
60+
class GroupsClassPropertyValuesProviderTest extends AbstractListClassPropertyValuesProviderTest
5761
{
5862
@InjectMockComponents
5963
private GroupsClassPropertyValuesProvider provider;
@@ -73,11 +77,13 @@ public void configure() throws Exception
7377
.thenReturn("url/to/noavatar.png");
7478
}
7579

76-
@Test
77-
public void getValuesLocal() throws Exception
80+
@ParameterizedTest
81+
@ValueSource(booleans = { true, false })
82+
void getValuesLocal(boolean hasAccess) throws Exception
7883
{
7984
when(this.wikiUserManager.getUserScope(this.classReference.getWikiReference().getName()))
8085
.thenReturn(UserScope.LOCAL_ONLY);
86+
when(this.authorizationManager.hasAccess(any(), any())).thenReturn(hasAccess);
8187

8288
DocumentReference devsReference = new DocumentReference("wiki", "Groups", "Devs");
8389
XWikiDocument devsProfile = mock(XWikiDocument.class, "devs");
@@ -103,18 +109,32 @@ public void getValuesLocal() throws Exception
103109

104110
assertEquals(2, values.getPropertyValues().size());
105111

106-
assertEquals("Developers", values.getPropertyValues().get(0).getMetaData().get("label"));
107-
assertTrue(values.getPropertyValues().get(0).getMetaData().get("icon") instanceof Map);
108-
109-
assertEquals("Administrators", values.getPropertyValues().get(1).getMetaData().get("label"));
110-
assertTrue(values.getPropertyValues().get(1).getMetaData().get("icon") instanceof Map);
112+
Object devsLabel = values.getPropertyValues().get(0).getMetaData().get("label");
113+
if (hasAccess) {
114+
assertEquals("Developers", devsLabel);
115+
} else {
116+
assertEquals("Devs", devsLabel);
117+
}
118+
assertInstanceOf(Map.class, values.getPropertyValues().get(0).getMetaData().get("icon"));
119+
120+
Object adminLabel = values.getPropertyValues().get(1).getMetaData().get("label");
121+
if (hasAccess) {
122+
assertEquals("Administrators", adminLabel);
123+
} else {
124+
assertEquals("Admins", adminLabel);
125+
}
126+
assertInstanceOf(Map.class, values.getPropertyValues().get(1).getMetaData().get("icon"));
111127
Map icon = (Map) values.getPropertyValues().get(1).getMetaData().get("icon");
112-
assertEquals("url/to/admins/image", icon.get("url"));
113-
assertEquals("IMAGE", icon.get("iconSetType"));
128+
if (hasAccess) {
129+
assertEquals("url/to/admins/image", icon.get("url"));
130+
assertEquals("IMAGE", icon.get("iconSetType"));
131+
} else {
132+
assertEquals("group", mockingDetails(icon).getMockCreationSettings().getMockName().toString());
133+
}
114134
}
115135

116136
@Test
117-
public void getValuesLocalAndGlobal() throws Exception
137+
void getValuesLocalAndGlobal() throws Exception
118138
{
119139
// We can have local groups.
120140
when(this.wikiUserManager.getUserScope(this.classReference.getWikiReference().getName()))

0 commit comments

Comments
 (0)