Skip to content

Commit

Permalink
XWIKI-20380: Add missing check on document authors
Browse files Browse the repository at this point in the history
  • Loading branch information
surli committed Nov 17, 2022
1 parent 8c6c078 commit 905cdd7
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.xwiki.context.ExecutionContext;
import org.xwiki.display.internal.DocumentDisplayerParameters;
import org.xwiki.model.document.DocumentAuthors;
import org.xwiki.model.internal.document.SafeDocumentAuthors;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.DocumentReferenceResolver;
import org.xwiki.model.reference.EntityReferenceSerializer;
Expand Down Expand Up @@ -2161,6 +2162,9 @@ public boolean hasAccessLevel(String level, String user)
/**
* Verifies if the user identified by {@code userReference} has the access identified by {@code right} on this
* document.
* Note that this method does not override {@link Api#hasAccess(Right, DocumentReference)}: they share same
* signature but on the {@code Api} one the {@link DocumentReference} parameter is about the entity where to check
* the right, while here it's about the user to check right for.
*
* @param right the right to check
* @param userReference the user to check the right for
Expand All @@ -2172,6 +2176,19 @@ public boolean hasAccess(Right right, DocumentReference userReference)
return getAuthorizationManager().hasAccess(right, userReference, getDocumentReference());
}

/**
* Verifies if the context user has the access identified by {@code right} on the current context document.
* @param right the right to check
* @return {@code true} if the user has the specified right on this document, {@code false} otherwise
* @since 14.10RC1
* @since 14.4.7
*/
@Unstable
public boolean hasAccess(Right right)
{
return hasAccess(right, getXWikiContext().getUserReference());
}

public boolean getLocked()
{
try {
Expand Down Expand Up @@ -2595,7 +2612,7 @@ public void save(String comment, boolean minorEdit) throws XWikiException
{
if (hasAccessLevel("edit")) {

DocumentAuthors authors = this.getAuthors();
DocumentAuthors authors = getDoc().getAuthors();
authors.setOriginalMetadataAuthor(getCurrentUserReferenceResolver().resolve(CurrentUserReference.INSTANCE));
// If the current author does not have PR don't let it set current user as author of the saved document
// since it can lead to right escalation
Expand Down Expand Up @@ -2692,7 +2709,7 @@ public void saveAsAuthor(String comment, boolean minorEdit) throws XWikiExceptio
{
XWikiContext xcontext = getXWikiContext();

getAuthors()
getDoc().getAuthors()
.setOriginalMetadataAuthor(getCurrentUserReferenceResolver().resolve(CurrentUserReference.INSTANCE));
DocumentReference author = getEffectiveAuthorReference();
if (hasAccess(Right.EDIT, author)) {
Expand Down Expand Up @@ -3310,6 +3327,12 @@ public int getLocalReferenceMaxLength()
@Unstable
public DocumentAuthors getAuthors()
{
return doc.getAuthors();
if (this.hasAccess(Right.PROGRAM)) {
// We're using getDoc here to ensure to have a cloned doc
return getDoc().getAuthors();
} else {
// in this case we don't care if the doc is cloned or not since it's readonly
return new SafeDocumentAuthors(this.doc.getAuthors());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.model.internal.document;

import java.util.Objects;

import org.xwiki.model.document.DocumentAuthors;
import org.xwiki.script.internal.safe.AbstractSafeObject;
import org.xwiki.script.wrap.AbstractWrappingObject;
import org.xwiki.user.UserReference;

/**
* Safe implementation of {@link DocumentAuthors} that doesn't allow to use any setters. This implementation aims at
* being used in script services. All setters throw an {@link UnsupportedOperationException}.
*
* @version $Id$
* @since 14.10RC1
* @since 14.4.7
*/
public class SafeDocumentAuthors extends AbstractWrappingObject<DocumentAuthors> implements DocumentAuthors
{
/**
* Default constructor.
*
* @param documentAuthors the wrapped authors to use for getters.
*/
public SafeDocumentAuthors(DocumentAuthors documentAuthors)
{
super(documentAuthors);
}

@Override
public UserReference getContentAuthor()
{
return getWrapped().getContentAuthor();
}

@Override
public void setContentAuthor(UserReference contentAuthor)
{
throw new UnsupportedOperationException(AbstractSafeObject.FORBIDDEN);
}

@Override
public UserReference getEffectiveMetadataAuthor()
{
return getWrapped().getEffectiveMetadataAuthor();
}

@Override
public void setEffectiveMetadataAuthor(UserReference metadataAuthor)
{
throw new UnsupportedOperationException(AbstractSafeObject.FORBIDDEN);
}

@Override
public UserReference getOriginalMetadataAuthor()
{
return getWrapped().getOriginalMetadataAuthor();
}

@Override
public void setOriginalMetadataAuthor(UserReference originalMetadataAuthor)
{
throw new UnsupportedOperationException(AbstractSafeObject.FORBIDDEN);
}

@Override
public UserReference getCreator()
{
return getWrapped().getCreator();
}

@Override
public void setCreator(UserReference creator)
{
throw new UnsupportedOperationException(AbstractSafeObject.FORBIDDEN);
}

@Override
public void copyAuthors(DocumentAuthors documentAuthors)
{
throw new UnsupportedOperationException(AbstractSafeObject.FORBIDDEN);
}

@Override
public boolean equals(Object obj)
{
if (obj instanceof SafeDocumentAuthors) {
return Objects.equals(this.getWrapped(), ((SafeDocumentAuthors) obj).getWrapped());
}
return super.equals(obj);
}

@Override
public int hashCode()
{
return super.hashCode();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
import org.junit.jupiter.api.Test;
import org.xwiki.component.manager.ComponentLookupException;
import org.xwiki.component.util.DefaultParameterizedType;
import org.xwiki.model.document.DocumentAuthors;
import org.xwiki.model.internal.document.SafeDocumentAuthors;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.ObjectReference;
import org.xwiki.observation.ObservationManager;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.test.junit5.mockito.MockComponent;
import org.xwiki.test.mockito.MockitoComponentManager;
Expand All @@ -47,11 +50,15 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@OldcoreTest
Expand Down Expand Up @@ -353,4 +360,34 @@ void saveAsAuthorWhenNoPR(MockitoComponentManager componentManager) throws XWiki

assertEquals(userContextReference, document.getAuthors().getEffectiveMetadataAuthor());
}

@Test
void getAuthors()
{
DocumentAuthors documentAuthors = mock(DocumentAuthors.class);
AuthorizationManager mockAuthorizationManager = this.oldcore.getMockAuthorizationManager();
XWikiContext context = this.oldcore.getXWikiContext();
DocumentReference userReference = new DocumentReference("xwiki", "XWiki", "Foo");
context.setUserReference(userReference);
DocumentReference currentDocReference = mock(DocumentReference.class, "currentDocRef");
XWikiDocument currentDoc = mock(XWikiDocument.class);
when(currentDoc.getAuthors()).thenReturn(documentAuthors);
Document document = new Document(currentDoc, context);

when(currentDoc.getDocumentReference()).thenReturn(currentDocReference);

when(mockAuthorizationManager.hasAccess(Right.PROGRAM, userReference, currentDocReference)).thenReturn(false);
DocumentAuthors obtainedAuthors = document.getAuthors();
assertTrue(obtainedAuthors instanceof SafeDocumentAuthors);
assertEquals(new SafeDocumentAuthors(documentAuthors), obtainedAuthors);

verify(mockAuthorizationManager).hasAccess(Right.PROGRAM, userReference, currentDocReference);

when(mockAuthorizationManager.hasAccess(Right.PROGRAM, userReference, currentDocReference)).thenReturn(true);
when(currentDoc.clone()).thenReturn(currentDoc);
obtainedAuthors = document.getAuthors();
assertSame(documentAuthors, obtainedAuthors);
verify(mockAuthorizationManager, times(2)).hasAccess(Right.PROGRAM, userReference, currentDocReference);
verify(currentDoc).clone();
}
}

0 comments on commit 905cdd7

Please sign in to comment.