org.xwiki.platform:xwiki-platform-oldcore makes Incorrect Use of Privileged APIs with DocumentAuthors

@@ -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;
@@ -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
@@ -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 {
@@ -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
@@ -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)) {
@@ -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());
+        }
     }
 }

@@ -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();
+    }
+}

@@ -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;
@@ -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
@@ -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();
+    }
 }