Bug #13477

Fix the bug: now the publications aliases cannot be shared to other
people for security reasons.

Indeed, the publication aliasing covers two features:
* A way to allow access for publications to users having no access
  for the EDM in which those publications are located.
* A way to multi categorize a publication;
For the first point, for security reasons, the users have only read
rights on the publication aliases. They have then all the functions
mapped to their role that don't break this read-only constraint. Only
users having access for the EDM or the folder in the EDM in which the
alias has been created can then access, through the alias, for the
publication.
For the second point, the multi categorization feature is limited by
the way the publications cannot be directly modified in those different
categorizations; they cannot be modified by their aliases. In fact, the
feature is more a consequence of the first one than a thinkfully one.

Beside this, the folder or publication sharing is a way to share
information both to other users in Silverpeas and external users. This
is why the feature can be a way to counterpass the access constraint on
publications with aliases. So, this fix ensures now the publication
aliases cannot be shared to other people than those expected by the
contributor who created the aliases.

Author: mmoqui

core-library/src/main/java/org/silverpeas/core/contribution/publication/model/Location.java

@@ -118,7 +118,7 @@ public int hashCode() {
     return super.hashCode();
   }
 
-  public class Alias implements Serializable {
+  public static class Alias implements Serializable {
     private String userId;
     private transient String userName;
     private Date date;

core-library/src/main/java/org/silverpeas/core/contribution/publication/model/PublicationDetail.java

@@ -1067,8 +1067,8 @@ public String getPermalink() {
   }
 
   public boolean isSharingAllowedForRolesFrom(final UserDetail user) {
-    if (!isValid()) {
-      // a not valid publication can not be shared
+    if (!isValid() || isAlias()) {
+      // neither a non validated publication nor an alias can not be shared
       return false;
     }
 

core-library/src/main/java/org/silverpeas/core/contribution/publication/service/DefaultPublicationService.java

@@ -752,6 +752,17 @@ public Collection<PublicationDetail> getDetailsByFatherPK(NodePK fatherPK, Strin
     }
   }
 
+  @Override
+  public Collection<PublicationDetail> getVisiblePublicationsIn(NodePK fatherPK) {
+    Collection<PublicationDetail> publications = getDetailsByFatherPK(fatherPK, null, true);
+    for (PublicationDetail publication : publications) {
+      var aliases = getAllAliases(publication.getPK());
+      //noinspection SuspiciousMethodCalls
+      publication.setAlias(aliases.contains(fatherPK));
+    }
+    return publications;
+  }
+
   @Override
   public Collection<PublicationDetail> getDetailsByFatherPK(NodePK fatherPK, String sorting,
       boolean filterOnVisibilityPeriod, String userId) {

core-library/src/main/java/org/silverpeas/core/contribution/publication/service/PublicationService.java

@@ -278,15 +278,16 @@ Pair<Collection<Location>, Collection<Location>> setAliases(PublicationPK pubPK,
   void removeAliases(PublicationPK pubPK, Collection<Location> aliases);
 
   /**
-   * Gets all the publications attached to the specified father.
+   * Gets all the publications attached to the specified father. The aliasing property of the
+   * publications isn't set.
    * @param fatherPK the identifying key of the father.
    * @return a collection of {@link PublicationDetail} instances.
    */
   Collection<PublicationDetail> getDetailsByFatherPK(NodePK fatherPK);
 
   /**
    * Gets all the publications attached to the specified father ordered as indicated by the sorting
-   * directive.
+   * directive. The aliasing property of the publications isn't set.
    * @param fatherPK the identifying key of the father.
    * @param sorting a sorting directive. Must be in the form of
    * "P.[publication detail attribute] (DESC|ASC)"
@@ -296,7 +297,7 @@ Pair<Collection<Location>, Collection<Location>> setAliases(PublicationPK pubPK,
 
   /**
    * Gets all the publications attached to the specified father, ordered as indicated by the sorting
-   * directive, according to the visibility.
+   * directive, according to the visibility. The aliasing property of the publications isn't set.
    * @param fatherPK the identifying key of the father.
    * @param sorting a sorting directive. Must be in the form of
    * "P.[publication detail attribute] (DESC|ASC)"
@@ -305,6 +306,14 @@ Pair<Collection<Location>, Collection<Location>> setAliases(PublicationPK pubPK,
    */
   Collection<PublicationDetail> getDetailsByFatherPK(NodePK fatherPK, String sorting, boolean filterOnVisibilityPeriod);
 
+  /**
+   * Gets all the publications that are currently visible and located into the specified node.
+   * For each returned publications, their aliasing property is valued.
+   * @param fatherPK the unique identifier of the node father of the publications.
+   * @return a list of {@link PublicationDetail} instances.
+   */
+  Collection<PublicationDetail> getVisiblePublicationsIn(NodePK fatherPK);
+
   /**
    * Gets all the publications attached to the specified father, ordered as indicated by the sorting
    * directive, according to the visibility, and that was authored or updated by the specified

core-library/src/main/java/org/silverpeas/core/util/OsEnum.java

@@ -31,8 +31,8 @@ public enum OsEnum {
   WINDOWS_XP("Windows XP", true), WINDOWS_9X("win9x", true), WINDOWS_VISTA("Windows Vista", true),
   WINDOWS_SEVEN("Windows 7", true), LINUX("Linux", false), MAC_OSX("Mac OS X", false),
   OS_400("os/400", false), Z_OS("z/os", false), OPENVMS("openvms", false), NETWARE("netware", false);
-  protected final boolean windows;
-  protected final String name;
+  private final boolean windows;
+  private final String name;
 
   OsEnum(String name, boolean windows) {
     this.windows = windows;

core-library/src/main/java/org/silverpeas/core/util/file/FileUtil.java

@@ -46,7 +46,9 @@
 import java.io.IOException;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
+import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
@@ -104,7 +106,7 @@ public static String getMimeType(final String fileName) {
   private static String computeMimeType(final String fileName) {
     String mimeType = null;
     File file = new File(fileName);
-    if (file.exists()) {
+    if (Files.exists(file.toPath())) {
       mimeType = getMimeTypeByMetadata(file);
     }
     final String fileExtension = FileRepositoryManager.getFileExtension(fileName).toLowerCase();
@@ -448,10 +450,33 @@ public static void validateFilename(String fileName, String intendedDir)
    * @param value a string data representing a path.
    * @return the given data.
    */
-  public static String verifyTaintedData(String value) {
+  public static String checkTaintedData(String value) {
     if (isDefined(value) && value.contains("..")) {
       throw new IllegalArgumentException(String.format("Value '%s' is forbidden", value));
     }
     return value;
   }
+
+  /**
+   * Checks the specified file name doesn't contain invalid characters for the underlying OS,
+   * otherwise it throws an {@link java.nio.file.InvalidPathException} exception. If an attempt
+   * to go back in the filesystem is detected, then an {@link IllegalArgumentException} is thrown.
+   * @param fileName the name of the file
+   * @throws IllegalArgumentException if the filename is empty, exceeds the number of characters
+   * limitation or is a path.
+   * @throws InvalidPathException if the filename contains invalid characters for the underlying OS.
+   */
+  public static void checkFileName(String fileName) {
+    if (fileName == null || fileName.isEmpty() || fileName.length() > 255) {
+      throw new InvalidPathException(fileName == null ? "": fileName,
+          "The file name is invalid: either it is empty or exceeds the size limit");
+    }
+    boolean isWindows = System.getProperty("os.name").toLowerCase().contains("win");
+    if ((isWindows && fileName.contains("\\")) || fileName.contains("/")) {
+      throw new IllegalArgumentException(String.format(
+          "File name '%s' isn't a filename but a path", fileName));
+    }
+    checkTaintedData(fileName);
+    Paths.get(fileName);
+  }
 }

core-library/src/test/java/org/silverpeas/core/util/FileUtilTest.java

@@ -268,23 +268,23 @@ void testValidateFileNameKo() {
   }
 
   @Test
-  void testVerifyTaintedData() {
-    assertThat(FileUtil.verifyTaintedData("a.b"), is("a.b"));
-    assertThat(FileUtil.verifyTaintedData(""), is(""));
-    assertThat(FileUtil.verifyTaintedData("null"), is("null"));
-    assertThat(FileUtil.verifyTaintedData("."), is("."));
-    assertThat(FileUtil.verifyTaintedData(File.separator + "."), is(File.separator + "."));
-    assertThat(FileUtil.verifyTaintedData("." + File.separator), is("." + File.separator));
+  void testCheckTaintedData() {
+    assertThat(FileUtil.checkTaintedData("a.b"), is("a.b"));
+    assertThat(FileUtil.checkTaintedData(""), is(""));
+    assertThat(FileUtil.checkTaintedData("null"), is("null"));
+    assertThat(FileUtil.checkTaintedData("."), is("."));
+    assertThat(FileUtil.checkTaintedData(File.separator + "."), is(File.separator + "."));
+    assertThat(FileUtil.checkTaintedData("." + File.separator), is("." + File.separator));
     assertThrows(IllegalArgumentException.class,
-        () -> FileUtil.verifyTaintedData(File.separator + ".." + File.separator));
+        () -> FileUtil.checkTaintedData(File.separator + ".." + File.separator));
     assertThrows(IllegalArgumentException.class,
-        () -> FileUtil.verifyTaintedData(".." + File.separator));
+        () -> FileUtil.checkTaintedData(".." + File.separator));
     assertThrows(IllegalArgumentException.class,
-        () -> FileUtil.verifyTaintedData(File.separator + ".."));
+        () -> FileUtil.checkTaintedData(File.separator + ".."));
     assertThrows(IllegalArgumentException.class,
-        () -> FileUtil.verifyTaintedData(".."));
+        () -> FileUtil.checkTaintedData(".."));
     assertThrows(IllegalArgumentException.class,
-        () -> FileUtil.verifyTaintedData("a..b"));
+        () -> FileUtil.checkTaintedData("a..b"));
   }
 
 }

core-services/sharing/src/main/java/org/silverpeas/core/sharing/model/NodeAccessControl.java

@@ -41,7 +41,7 @@
  */
 public class NodeAccessControl extends AbstractShareableAccessControl {
 
-  NodeAccessControl(NodeTicket ticket) {
+  NodeAccessControl(Ticket ticket) {
     super(ticket);
   }
 
@@ -64,7 +64,7 @@ protected Collection<NodePK> getPublicationFathers(ResourceReference pk) {
     return getPublicationService().getAllFatherPKInSamePublicationComponentInstance(new PublicationPK(pk.getId(), pk.getInstanceId()));
   }
 
-  protected Collection<Location> getPublicationAliases(ResourceReference pk) {
+  protected Collection<Location> getPublicationLocations(ResourceReference pk) {
     return getPublicationService().getAllLocations(new PublicationPK(pk.getId(),
         pk.getInstanceId()));
   }
@@ -81,10 +81,10 @@ private boolean isPublicationReadable(ResourceReference pk, String instanceId,
     } else {
       // special case of an alias between two ECM applications
       // check if publication which contains attachment is an alias into this node
-      Collection<Location> locations = getPublicationAliases(pk);
+      Collection<Location> locations = getPublicationLocations(pk);
       for (Location location : locations) {
-        NodePK aliasPK = new NodePK(location.getId(), location.getInstanceId());
-        if (authorizedNodes.contains(aliasPK)) {
+        NodePK father = new NodePK(location.getId(), location.getInstanceId());
+        if (!location.isAlias() && authorizedNodes.contains(father)) {
           return true;
         }
       }

core-services/sharing/src/main/java/org/silverpeas/core/sharing/model/NodeTicket.java

@@ -26,7 +26,6 @@
 import org.silverpeas.core.node.model.NodeDetail;
 import org.silverpeas.core.node.model.NodePK;
 import org.silverpeas.core.node.service.NodeService;
-import org.silverpeas.core.sharing.security.AccessControlContext;
 import org.silverpeas.core.sharing.security.ShareableAccessControl;
 import org.silverpeas.core.sharing.security.ShareableNode;
 import org.silverpeas.core.sharing.security.ShareableResource;

core-services/sharing/src/main/java/org/silverpeas/core/sharing/model/PublicationAccessControl.java

@@ -33,7 +33,7 @@
  */
 public class PublicationAccessControl extends AbstractShareableAccessControl {
 
-  PublicationAccessControl(PublicationTicket ticket) {
+  PublicationAccessControl(Ticket ticket) {
     super(ticket);
   }
 

core-services/sharing/src/main/java/org/silverpeas/core/sharing/model/SimpleFileAccessControl.java

@@ -31,7 +31,7 @@
  */
 public class SimpleFileAccessControl extends AbstractShareableAccessControl {
 
-  SimpleFileAccessControl(SimpleFileTicket ticket) {
+  SimpleFileAccessControl(Ticket ticket) {
     super(ticket);
   }
 

core-services/sharing/src/main/java/org/silverpeas/core/sharing/model/VersionFileAccessControl.java

@@ -31,7 +31,7 @@
  */
 public class VersionFileAccessControl extends AbstractShareableAccessControl {
 
-  VersionFileAccessControl(VersionFileTicket ticket) {
+  VersionFileAccessControl(Ticket ticket) {
     super(ticket);
   }
 

core-services/sharing/src/main/java/org/silverpeas/core/sharing/security/AbstractShareableAccessControl.java

@@ -37,9 +37,6 @@ protected AbstractShareableAccessControl(Ticket ticket) {
     this.ticket = ticket;
   }
 
-  @Override
-  public abstract boolean isReadable(final AccessControlContext resource);
-
   protected Ticket getSharingTicket() {
     return ticket;
   }

core-war/src/main/java/org/silverpeas/web/sharing/servlets/FileSharingRequestRouter.java

@@ -49,10 +49,6 @@ public class FileSharingRequestRouter extends ComponentRequestRouter<FileSharing
 
   private static final long serialVersionUID = -8855028133035807994L;
 
-  /**
-   * This method has to be implemented in the component request rooter class. returns the session
-   * control bean name to be put in the request object ex : for almanach, returns "almanach"
-   */
   @Override
   public String getSessionControlBeanName() {
     return "FileSharing";

core-web/src/main/java/org/silverpeas/core/webapi/attachment/AbstractAttachmentResource.java

@@ -51,7 +51,7 @@ public String getComponentId() {
     return componentId;
   }
 
-  protected Response getFileContent(String attachmentId) {
+  protected Response getAttachmentContent(String attachmentId) {
     final SimpleDocument attachment = AttachmentServiceProvider.getAttachmentService()
         .searchDocumentById(new SimpleDocumentPK(attachmentId), null);
     if (attachment == null) {

core-web/src/main/java/org/silverpeas/core/webapi/attachment/AbstractSimpleDocumentResource.java

@@ -59,10 +59,6 @@ protected String getBundleLocation() {
     return "org.silverpeas.util.attachment.multilang.attachment";
   }
 
-  /**
-   * Check the file.
-   * @param fileToCheck
-   */
   protected void checkUploadedFile(File fileToCheck) {
     try {
 
@@ -85,10 +81,6 @@ protected void checkUploadedFile(File fileToCheck) {
     }
   }
 
-  /**
-   * Manages the runtime errors.
-   * @param re
-   */
   protected void performRuntimeException(RuntimeException re) {
     String transverseExceptionMessage = SilverpeasTransverseErrorUtil
         .performExceptionMessage(re, getUserPreferences().getLanguage());

core-web/src/main/java/org/silverpeas/core/webapi/attachment/AttachmentResource.java

@@ -54,7 +54,7 @@ protected String getResourceBasePath() {
   @Path("{id}/{name}")
   @Produces(MediaType.APPLICATION_OCTET_STREAM)
   public Response getFileContent(final @PathParam("id") String attachmentId) {
-    return super.getFileContent(attachmentId);
+    return getAttachmentContent(attachmentId);
   }
 
   @Override

core-web/src/main/java/org/silverpeas/core/webapi/attachment/SharedAttachmentResource.java

@@ -35,6 +35,7 @@
 import org.silverpeas.core.util.MimeTypes;
 import org.silverpeas.core.util.ZipUtil;
 import org.silverpeas.core.util.file.FileRepositoryManager;
+import org.silverpeas.core.util.file.FileUtil;
 
 import javax.ws.rs.*;
 import javax.ws.rs.core.MediaType;
@@ -43,6 +44,7 @@
 import javax.ws.rs.core.UriBuilderException;
 import java.io.*;
 import java.net.URI;
+import java.nio.file.Files;
 import java.util.StringTokenizer;
 import java.util.UUID;
 
@@ -67,7 +69,7 @@ protected String getResourceBasePath() {
   @Path("{id}/{name}")
   @Produces(MediaType.APPLICATION_OCTET_STREAM)
   public Response getFileContent(final @PathParam("id") String attachmentId) {
-    return super.getFileContent(attachmentId);
+    return getAttachmentContent(attachmentId);
   }
 
   @GET
@@ -112,9 +114,10 @@ public ZipEntity zipFiles(@PathParam("ids") String attachmentIds) {
   @Path("zipcontent/{name}")
   @Produces(MediaType.APPLICATION_OCTET_STREAM)
   public Response getZipContent(@PathParam("name") String zipFileName) {
+    checkFileName(zipFileName);
     String zipFilePath = FileRepositoryManager.getTemporaryPath() + zipFileName;
     File realFile = new File(zipFilePath);
-    if (!realFile.exists() && !realFile.isFile()) {
+    if (!Files.exists(realFile.toPath()) && !Files.isRegularFile(realFile.toPath())) {
       return Response.ok().build();
     } else {
       try(ByteArrayOutputStream output = new ByteArrayOutputStream();
@@ -140,4 +143,12 @@ protected String getToken() {
     return token;
   }
 
+  private void checkFileName(String fileName) {
+    try {
+      FileUtil.checkFileName(fileName);
+    } catch (IllegalArgumentException e) {
+      throw new BadRequestException(e.getMessage());
+    }
+  }
+
 }