Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XWIKI-20907: Introduce the notion of required rights #3285

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
25c9986
XWIKI-20907: Introduce the notion of required rights
michitux Jul 18, 2024
f0b6872
XWIKI-20907: Introduce the notion of required rights
michitux Jul 19, 2024
c90ceae
XWIKI-20907: Introduce the notion of required rights
michitux Jul 24, 2024
42b3db5
XWIKI-20907: Introduce the notion of required rights
michitux Jul 24, 2024
0686535
XWIKI-20907: Introduce the notion of required rights
michitux Jul 24, 2024
521d800
XWIKI-20907: Introduce the notion of required rights
michitux Aug 26, 2024
79367c6
XWIKI-20907: Introduce the notion of required rights
michitux Aug 29, 2024
6a4fbed
XWIKI-20907: Introduce the notion of required rights
michitux Aug 29, 2024
0b093ec
XWIKI-20907: Introduce the notion of required rights
michitux Aug 29, 2024
d606791
XWIKI-20907: Introduce the notion of required rights
michitux Aug 29, 2024
e05a1a9
XWIKI-20907: Introduce the notion of required rights
michitux Aug 30, 2024
4ba7b78
XWIKI-20907: Introduce the notion of required rights
michitux Sep 3, 2024
ba336cf
XWIKI-20907: Introduce the notion of required rights
michitux Sep 3, 2024
6806f5f
XWIKI-20907: Introduce the notion of required rights
michitux Sep 3, 2024
093fbc6
XWIKI-20907: Introduce the notion of required rights
michitux Sep 3, 2024
513c448
XWIKI-20907: Introduce the notion of required rights
michitux Oct 17, 2024
e8f38f5
XWIKI-20907: Introduce the notion of required rights
michitux Oct 17, 2024
ca7f5f5
XWIKI-20907: Introduce the notion of required rights
michitux Oct 17, 2024
1e2f995
XWIKI-20907: Introduce the notion of required rights
michitux Oct 17, 2024
04e0d94
XWIKI-20907: Introduce the notion of required rights
michitux Oct 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions xwiki-platform-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,23 @@

Single justification example:
-->





<revapi.differences>
<justification>Change in generated class of the REST model to add the
enforceRequiredRights property.</justification>
<criticality>highlight</criticality>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like an allowed to me (I don't see what this change could really break in practice).

<differences>
<item>
<code>java.annotation.attributeValueChanged</code>
<old>class org.xwiki.rest.model.jaxb.Page</old>
<new>class org.xwiki.rest.model.jaxb.Page</new>
<annotationType>javax.xml.bind.annotation.XmlType</annotationType>
<attribute>propOrder</attribute>
<oldValue>{"language", "majorVersion", "minorVersion", "hidden", "created", "creator", "creatorName", "modified", "modifier", "modifierName", "originalMetadataAuthor", "originalMetadataAuthorName", "comment", "content", "clazz", "objects", "attachments", "hierarchy"}</oldValue>
<newValue>{"language", "majorVersion", "minorVersion", "hidden", "enforceRequiredRights", "created", "creator", "creatorName", "modified", "modifier", "modifierName", "originalMetadataAuthor", "originalMetadataAuthorName", "comment", "content", "clazz", "objects", "attachments", "hierarchy"}</newValue>
</item>
</differences>
</revapi.differences>
</analysisConfiguration>
</configuration>
</plugin>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,4 +221,16 @@ default boolean isRestricted()
{
return false;
}

/**
* @return {@code true} if required rights defined in a {@code XWiki.RequiredRightClass} object shall be
* enforced, meaning that editing will be limited to users with these rights and content of this document can't
* use more rights than defined in the object, {@code false} otherwise
* @since 16.6.0RC1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will definitely not go in 16.6.0RC1, but anyway hard to tell right now what will be the version.

*/
@Unstable
default boolean isEnforceRequiredRights()
{
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.rendering.block.XDOM;
import org.xwiki.rendering.syntax.Syntax;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.DocumentAuthorizationManager;
import org.xwiki.security.authorization.Right;

import com.xpn.xwiki.XWikiContext;
Expand Down Expand Up @@ -75,7 +75,7 @@ public class DefaultWikiComponentBridge implements WikiComponentConstants, WikiC
private ContentParser renderingBridge;

@Inject
private AuthorizationManager authorization;
private DocumentAuthorizationManager authorization;

@Override
public Syntax getSyntax(DocumentReference reference) throws WikiComponentException
Expand Down Expand Up @@ -201,7 +201,8 @@ public boolean hasProgrammingRights(DocumentReference reference) throws WikiComp
{
XWikiDocument document = getDocument(reference);

return this.authorization.hasAccess(Right.PROGRAM, document.getAuthorReference(), null);
return this.authorization.hasAccess(Right.PROGRAM, null, document.getAuthorReference(),
document.getDocumentReference());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.rendering.block.XDOM;
import org.xwiki.rendering.syntax.Syntax;
import org.xwiki.security.authorization.DocumentAuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.test.annotation.ComponentList;
import org.xwiki.test.junit5.mockito.ComponentTest;
import org.xwiki.test.junit5.mockito.InjectComponentManager;
Expand All @@ -60,7 +62,9 @@
import com.xpn.xwiki.web.Utils;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -107,6 +111,9 @@ class DefaultWikiComponentBridgeTest implements WikiComponentConstants
@MockComponent
private Execution execution;

@MockComponent
private DocumentAuthorizationManager documentAuthorizationManager;

@InjectMockComponents
private DefaultWikiComponentBridge bridge;

Expand All @@ -128,6 +135,7 @@ public void configure() throws Exception

when(this.execution.getContext()).thenReturn(context);
when(this.xwiki.getDocument(DOC_REFERENCE, xwikiContext)).thenReturn(this.componentDoc);
when(this.componentDoc.getDocumentReference()).thenReturn(DOC_REFERENCE);
}

@Test
Expand Down Expand Up @@ -264,4 +272,17 @@ void getSyntax() throws Exception

assertEquals(Syntax.XWIKI_2_1, this.bridge.getSyntax(DOC_REFERENCE));
}

@Test
void hasProgrammingRights() throws Exception
{
when(this.componentDoc.getAuthorReference()).thenReturn(AUTHOR_REFERENCE);

assertFalse(this.bridge.hasProgrammingRights(DOC_REFERENCE));

when(this.documentAuthorizationManager.hasAccess(Right.PROGRAM, null, AUTHOR_REFERENCE, DOC_REFERENCE))
.thenReturn(true);

assertTrue(this.bridge.hasProgrammingRights(DOC_REFERENCE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
import org.xwiki.rendering.transformation.MacroTransformationContext;
import org.xwiki.rendering.util.ParserUtils;
import org.xwiki.security.authorization.AuthorExecutor;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.DocumentAuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.velocity.VelocityEngine;
import org.xwiki.velocity.VelocityManager;
Expand Down Expand Up @@ -120,7 +120,7 @@ public class DefaultGadgetSource implements GadgetSource
private JobProgressManager progress;

@Inject
private AuthorizationManager authorizationManager;
private DocumentAuthorizationManager authorizationManager;

/**
* Prepare the parser to parse the title and content of the gadget into blocks.
Expand Down Expand Up @@ -199,7 +199,9 @@ private List<Gadget> prepareGadgets(List<BaseObject> objects, Syntax sourceSynta
String gadgetTitle;

XWikiDocument ownerDocument = xObject.getOwnerDocument();
if (this.authorizationManager.hasAccess(Right.SCRIPT, ownerDocument.getAuthorReference(), ownerDocument.getDocumentReference())) {
if (!ownerDocument.isRestricted() && this.authorizationManager.hasAccess(Right.SCRIPT,
EntityType.DOCUMENT, ownerDocument.getAuthorReference(), ownerDocument.getDocumentReference()))
{
gadgetTitle =
this.evaluateVelocityTitle(velocityContext, velocityEngine, key, title, ownerDocument);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.mockito.stubbing.Answer;
import org.xwiki.context.Execution;
import org.xwiki.context.ExecutionContext;
import org.xwiki.model.EntityType;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.DocumentReferenceResolver;
import org.xwiki.model.reference.EntityReference;
Expand All @@ -41,7 +42,7 @@
import org.xwiki.rendering.transformation.MacroTransformationContext;
import org.xwiki.rendering.transformation.TransformationContext;
import org.xwiki.security.authorization.AuthorExecutor;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.DocumentAuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.test.junit5.mockito.ComponentTest;
import org.xwiki.test.junit5.mockito.InjectMockComponents;
Expand Down Expand Up @@ -76,7 +77,7 @@ class DefaultGadgetSourceTest
private AuthorExecutor authorExecutor;

@MockComponent
private AuthorizationManager authorizationManager;
private DocumentAuthorizationManager authorizationManager;

@Mock
private DocumentReference documentReference;
Expand Down Expand Up @@ -164,7 +165,8 @@ void getGadgets() throws Exception
when(gadgetObject1.getLargeStringValue("content")).thenReturn("Some content");
when(gadgetObject1.getStringValue("position")).thenReturn("0");
when(gadgetObject1.getNumber()).thenReturn(42);
when(this.authorizationManager.hasAccess(Right.SCRIPT, ownerAuthorReference, ownerSourceReference)).thenReturn(true);
when(this.authorizationManager.hasAccess(Right.SCRIPT, EntityType.DOCUMENT, ownerAuthorReference,
ownerSourceReference)).thenReturn(true);
when(this.velocityEngine.evaluate(any(), any(), any(), eq("Gadget 1"))).then((Answer<Void>) invocation -> {
Object[] args = invocation.getArguments();
StringWriter stringWriter = (StringWriter) args[1];
Expand Down Expand Up @@ -197,7 +199,8 @@ void getGadgetWithoutScriptRight() throws Exception
when(gadgetObject1.getLargeStringValue("content")).thenReturn("Some other content");
when(gadgetObject1.getStringValue("position")).thenReturn("2");
when(gadgetObject1.getNumber()).thenReturn(12);
when(this.authorizationManager.hasAccess(Right.SCRIPT, ownerAuthorReference, ownerSourceReference)).thenReturn(false);
when(this.authorizationManager.hasAccess(Right.SCRIPT, EntityType.DOCUMENT, ownerAuthorReference,
ownerSourceReference)).thenReturn(false);

List<Gadget> gadgets = this.defaultGadgetSource.getGadgets(testSource, macroTransformationContext);
assertEquals(1, gadgets.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
import org.xwiki.rendering.parser.ParseException;
import org.xwiki.rendering.parser.Parser;
import org.xwiki.rendering.util.ParserUtils;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.DocumentAuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.velocity.VelocityEngine;
import org.xwiki.velocity.VelocityManager;
Expand Down Expand Up @@ -112,7 +112,7 @@ public abstract class AbstractDocumentTitleDisplayer implements DocumentDisplaye
private ConfigurationSource xwikicfg;

@Inject
private AuthorizationManager authorizationManager;
private DocumentAuthorizationManager authorizationManager;

/**
* Used to get the default document reference, which normally is used to represent the home page of a space.
Expand Down Expand Up @@ -177,7 +177,7 @@ private XDOM displayTitle(DocumentModelBridge document, DocumentDisplayerParamet
String title = rawTitle;
// Evaluate the title only if the document is not restricted and its content's author has script
// right, otherwise use the raw title.
if (!document.isRestricted() && this.authorizationManager.hasAccess(Right.SCRIPT,
if (!document.isRestricted() && this.authorizationManager.hasAccess(Right.SCRIPT, EntityType.DOCUMENT,
document.getContentAuthorReference(), document.getDocumentReference())) {
title = evaluateTitle(document, parameters);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
import org.xwiki.rendering.block.WordBlock;
import org.xwiki.rendering.block.XDOM;
import org.xwiki.rendering.parser.Parser;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.DocumentAuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.test.mockito.MockitoComponentMockingRule;

Expand Down Expand Up @@ -123,8 +123,8 @@ public void whenSettingTheContextDocumentTheContextWikiIsAlsoSet() throws Except
WikiReference currentWikiReference = new WikiReference("currentWiki");
when(modelContext.getCurrentEntityReference()).thenReturn(currentWikiReference);

AuthorizationManager authorizationManager = this.mocker.getInstance(AuthorizationManager.class);
when(authorizationManager.hasAccess(eq(Right.SCRIPT), any(), any())).thenReturn(true);
DocumentAuthorizationManager authorizationManager = this.mocker.getInstance(DocumentAuthorizationManager.class);
when(authorizationManager.hasAccess(eq(Right.SCRIPT), eq(EntityType.DOCUMENT), any(), any())).thenReturn(true);

DocumentAccessBridge dab = this.mocker.getInstance(DocumentAccessBridge.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@
import org.xwiki.component.wiki.internal.bridge.WikiBaseObjectComponentBuilder;
import org.xwiki.eventstream.EventStreamException;
import org.xwiki.localization.ContextualLocalizationManager;
import org.xwiki.model.EntityType;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.EntityReference;
import org.xwiki.model.reference.LocalDocumentReference;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.DocumentAuthorizationManager;
import org.xwiki.security.authorization.Right;

import com.xpn.xwiki.doc.XWikiDocument;
Expand All @@ -61,7 +62,7 @@ public class UntypedRecordableEventDescriptorComponentBuilder implements WikiBas
public static final String BOUNDED_XOBJECT_CLASS = "XWiki.EventStream.Code.EventClass";

@Inject
private AuthorizationManager authorizationManager;
private DocumentAuthorizationManager authorizationManager;

@Inject
private ContextualLocalizationManager contextualLocalizationManager;
Expand Down Expand Up @@ -106,8 +107,7 @@ public List<WikiComponent> buildComponents(BaseObject baseObject) throws WikiCom
private void checkRights(DocumentReference documentReference, DocumentReference authorReference)
throws EventStreamException
{
if (!this.authorizationManager.hasAccess(Right.ADMIN, authorReference, documentReference.getWikiReference()))
{
if (!this.authorizationManager.hasAccess(Right.ADMIN, EntityType.WIKI, authorReference, documentReference)) {
throw new EventStreamException("Registering Untyped Events requires wiki administration rights.");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@
import org.junit.Rule;
import org.junit.Test;
import org.xwiki.component.wiki.WikiComponent;
import org.xwiki.model.EntityType;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.DocumentReferenceResolver;
import org.xwiki.query.Query;
import org.xwiki.query.QueryManager;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.DocumentAuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.test.mockito.MockitoComponentMockingRule;

import com.xpn.xwiki.doc.XWikiDocument;
Expand All @@ -39,6 +41,7 @@
import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

/**
Expand All @@ -59,7 +62,7 @@ public class UntypedRecordableEventDescriptorComponentBuilderTest

private DocumentReferenceResolver<String> documentReferenceResolver;

private AuthorizationManager authorizationManager;
private DocumentAuthorizationManager authorizationManager;

private DocumentReference event1;
private DocumentReference event2;
Expand All @@ -71,7 +74,7 @@ public void setUp() throws Exception
queryManager = mocker.registerMockComponent(QueryManager.class);
modelBridge = mocker.registerMockComponent(ModelBridge.class);
documentReferenceResolver = mocker.registerMockComponent(DocumentReferenceResolver.class);
authorizationManager = mocker.registerMockComponent(AuthorizationManager.class);
authorizationManager = mocker.registerMockComponent(DocumentAuthorizationManager.class);

Query query = mock(Query.class);
when(queryManager.createQuery(any(), any())).thenReturn(query);
Expand Down Expand Up @@ -104,10 +107,12 @@ public void testBuildComponent() throws Exception
when(parentDocument.getDocumentReference()).thenReturn(documentReference);

// Ensure that the user rights are correctly checked
when(this.authorizationManager.hasAccess(any(), any(), any())).thenReturn(true);
when(this.authorizationManager.hasAccess(any(), any(), any(), any())).thenReturn(true);

List<WikiComponent> result = this.mocker.getComponentUnderTest().buildComponents(baseObject);

assertEquals(1, result.size());

verify(this.authorizationManager).hasAccess(Right.ADMIN, EntityType.WIKI, null, documentReference);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.xwiki.filter.FilterEventParameters;
import org.xwiki.filter.FilterException;
import org.xwiki.filter.annotation.Default;
import org.xwiki.stability.Unstable;

/**
* Document related events.
Expand Down Expand Up @@ -197,6 +198,14 @@ public interface WikiDocumentFilter
*/
String PARAMETER_REVISION_MINOR = "revision_minor";

// required rights
/**
* @type {@link Boolean}
* @since 16.6.0RC1
*/
@Unstable
String PARAMETER_ENFORCE_REQUIRED_RIGHTS = "enforce_required_rights";

// Events

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ public class XARDocumentModel extends XarDocumentModel
new EventParameter(XWikiWikiDocumentFilter.PARAMETER_REVISION_EFFECTIVEMETADATA_AUTHOR));
put(ELEMENT_REVISION_ORIGINALMEDATAAUTHOR,
new EventParameter(XWikiWikiDocumentFilter.PARAMETER_REVISION_ORIGINALMETADATA_AUTHOR));
put(ELEMENT_ENFORCE_REQUIRED_RIGHTS,
new EventParameter(XWikiWikiDocumentFilter.PARAMETER_ENFORCE_REQUIRED_RIGHTS, Boolean.class));
put(ELEMENT_REVISION_COMMENT, new EventParameter(XWikiWikiDocumentFilter.PARAMETER_REVISION_COMMENT));
put(ELEMENT_REVISION_DATE, new EventParameter(XWikiWikiDocumentFilter.PARAMETER_REVISION_DATE, Date.class));
put(ELEMENT_REVISION_MINOR,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,15 @@ public EventParameter(String name)
*/
public static final String ROLEHINT_15 = ROLEHINT_PREFIX + XarDocumentModel.VERSION_15;

/**
* @since 16.6.0RC1
*/
public static final String ROLEHINT_16 = ROLEHINT_PREFIX + XarDocumentModel.VERSION_16;

/**
* @since 7.2M1
*/
public static final String ROLEHINT_CURRENT = ROLEHINT_15;
public static final String ROLEHINT_CURRENT = ROLEHINT_16;

/**
* @since 6.2M1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ private void readDocument(XMLStreamReader xmlReader, Object filter, XARInputFilt
// Initialize with a few defaults (thing that don't exist in old XAR format)
this.currentDocumentRevisionParameters.put(XWikiWikiDocumentFilter.PARAMETER_SYNTAX, Syntax.XWIKI_1_0);
this.currentDocumentRevisionParameters.put(XWikiWikiDocumentFilter.PARAMETER_HIDDEN, false);
this.currentDocumentRevisionParameters.put(XWikiWikiDocumentFilter.PARAMETER_ENFORCE_REQUIRED_RIGHTS, false);

// Reference
String referenceString = xmlReader.getAttributeValue(null, XARDocumentModel.ATTRIBUTE_DOCUMENT_REFERENCE);
Expand Down
Loading