Skip to content
Snippets Groups Projects
Unverified Commit c80c1ff8 authored by Michael Hamann's avatar Michael Hamann Committed by GitHub
Browse files

XWIKI-21439: Security Cache ConflictingInsertionException (#2673)

* Remove parameters from entity references when constructing a security reference.
* Add a method for removing parameters to EntityReference
* Make sure to keep entity references unmodified when possible
parent fce99fa2
No related branches found
No related tags found
No related merge requests found
......@@ -32,6 +32,7 @@
import org.xwiki.model.EntityType;
import org.xwiki.model.internal.reference.DefaultSymbolScheme;
import org.xwiki.model.internal.reference.LocalizedStringEntityReferenceSerializer;
import org.xwiki.stability.Unstable;
/**
* Represents a reference to an Entity (Document, Attachment, Space, Wiki, etc).
......@@ -513,6 +514,43 @@ public boolean hasParent(EntityReference expectedParent)
return actualParent != null;
}
/**
* Get a reference that is equal to this reference with all parameters removed.
*
* @param recursive whether to remove parameters recursively from all ancestors
* @return the reference without parameters
* @since 15.10.1
* @since 15.5.5
* @since 14.10.20
*/
@Unstable
public EntityReference removeParameters(boolean recursive)
{
EntityReference current;
if (!recursive) {
if (getParameters().isEmpty()) {
current = this;
} else {
current = new EntityReference(getName(), getType(), getParent());
}
} else {
current = null;
for (EntityReference entry : getReversedReferenceChain()) {
// Create a new reference with the same name and type as the current one but without parameters, but
// only if this is actually necessary, i.e., if there are actually any parameters or any ancestor has
// been modified.
if (entry.getParent() == current && entry.getParameters().isEmpty()) {
current = entry;
} else {
current = new EntityReference(entry.getName(), entry.getType(), current);
}
}
}
return current;
}
@Override
public String toString()
{
......
......@@ -55,6 +55,7 @@ public interface XWikiBridge
/**
* Right now the security module logic only works with DOCUMENT based reference so PAGE reference need to be
* converted.
* This also removes parameters from the reference as they aren't supported by the security module.
*
* @param reference the reference
* @return the compatible reference
......
......@@ -119,8 +119,11 @@ public EntityReference toCompatibleEntityReference(EntityReference reference)
return reference;
}
// Discard the parameters since they are not used by the authorization system but cause issues when comparing
// references for equality.
// Make sure the reference is complete
EntityReference compatibleReference = this.currentResolver.resolve(reference, reference.getType());
EntityReference compatibleReference = this.currentResolver.resolve(reference.removeParameters(true),
reference.getType());
// Convert to PAGE reference to DOCUMENT reference since the security system design does not work well with PAGE
// one (which have different kinds of right at the same level)
......
......@@ -19,6 +19,8 @@
*/
package org.xwiki.security.internal;
import java.util.Map;
import javax.inject.Named;
import org.junit.jupiter.api.Test;
......@@ -27,6 +29,7 @@
import org.xwiki.model.reference.EntityReference;
import org.xwiki.model.reference.EntityReferenceResolver;
import org.xwiki.model.reference.PageReference;
import org.xwiki.model.reference.SpaceReference;
import org.xwiki.test.junit5.mockito.InjectMockComponents;
import org.xwiki.test.junit5.mockito.MockComponent;
......@@ -35,7 +38,11 @@
import com.xpn.xwiki.test.junit5.mockito.OldcoreTest;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;
......@@ -88,4 +95,48 @@ public void toCompatibleEntityReferenceWhenPage()
assertEquals(documentReference, this.bridge.toCompatibleEntityReference(pageReference));
}
@Test
void toCompatibleEntityReferenceWithParameters()
{
SpaceReference parentReference = new SpaceReference("wiki", "space");
DocumentReference documentReference = new DocumentReference("page", parentReference, Map.of("param", "value"));
DocumentReference noParameterReference = new DocumentReference("page", parentReference);
when(this.currentResolver.resolve(any(), eq(EntityType.DOCUMENT)))
.thenAnswer(invocation -> invocation.getArgument(0));
EntityReference actual = this.bridge.toCompatibleEntityReference(documentReference);
assertEquals(noParameterReference, actual);
assertNotEquals(documentReference, actual);
assertSame(parentReference, actual.getParent());
}
@Test
void toCompatibleEntityReferenceWithRecursiveParameters()
{
EntityReference wikiReference = new EntityReference("wiki", EntityType.WIKI, Map.of("wiki", "wikiValue"));
EntityReference spaceReference = new EntityReference("space", EntityType.SPACE, wikiReference,
Map.of("space", "spaceValue"));
DocumentReference documentReference = new DocumentReference("page", spaceReference, Map.of("param", "value"));
DocumentReference noParameterReference = new DocumentReference("wiki", "space", "page");
when(this.currentResolver.resolve(any(), eq(EntityType.DOCUMENT)))
.thenAnswer(invocation -> invocation.getArgument(0));
EntityReference actual = this.bridge.toCompatibleEntityReference(documentReference);
assertEquals(noParameterReference, actual);
assertNotEquals(documentReference, actual);
}
@Test
void toCompatibleEntityReferenceWithoutParametersReturnsSame()
{
DocumentReference documentReference = new DocumentReference("wiki", "space", "page");
when(this.currentResolver.resolve(any(), eq(EntityType.DOCUMENT)))
.thenAnswer(invocation -> invocation.getArgument(0));
assertSame(documentReference, this.bridge.toCompatibleEntityReference(documentReference));
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment