pnfsmanager: allow restricted user with UPLOAD to create parent direc…

…tories

Motivation:

When creating a macaroon to allow uploading of data, the desired path
may not already exist.  Without restrictions, WebDAV will auto-create
parent directory items that are missing, or the client can create these
directory elements explicitly with MKCOL.

With restrictions (such as from a macaroon) such directory creation
currently requires the MANAGE activity.  However, MANAGE activity also
allows the user to create unrelated directories, delete directories,
rename existing data, move data around, which is undesirable if the user
should be allowed only to upload data.

Modification:

Update restrictions to allow the discovery of whether child paths are
restricted.

Update permissions test to avoid the MANAGE restriction check if the
user is allowed to upload a child element.

Result:

A user with a macaroon that authorises them to upload data into a
particular directory will be able to create parent directories to
achieve uploading the data.

Note:

  1. The user cannot create the target path as a directory, only
     ancestor directories.  The path is intepreted as the path
     of a single file.  If multiple files should be authorised
     then the path should already exist as a directory.

  2. If the macaroon has no path restriction then the user can
     create directories throughout dCache.  This is similar to
     how such a user is able to upload data anywhere in dCache.

  3. There is no distinction between directories created with MKCOL and
     those created automatically with a PUT request.

Target: master
Request: 4.2
Request: 4.1
Request: 4.0
Request: 3.2
Ticket: http://rt.dcache.org/Ticket/Display.html?id=9503
Require-notes: yes
Require-book: yes

modules/common/src/main/java/org/dcache/auth/attributes/Activity.java

@@ -59,7 +59,9 @@ public enum Activity
 
     /**
      * Create a new file within dCache.  Note that creating new directory or a
-     * new sym-link is the MANAGE Activity.
+     * new sym-link normally requires the MANAGE Activity, but UPLOAD is
+     * sufficient to create missing parent directories for an otherwise allowed
+     * upload.
      * <p>
      * There are two kinds of UPLOAD activity: a specific upload and querying
      * support for uploading.  For the former, the target is the path of the

modules/common/src/main/java/org/dcache/auth/attributes/DenyActivityRestriction.java

@@ -73,6 +73,12 @@ public boolean isRestricted(Activity activity, FsPath directory, String name)
         return denied.contains(activity);
     }
 
+    @Override
+    public boolean hasUnrestrictedChild(Activity activity, FsPath parent)
+    {
+        return !denied.contains(activity);
+    }
+
     @Override
     public int hashCode()
     {

modules/common/src/main/java/org/dcache/auth/attributes/PrefixRestriction.java

@@ -65,6 +65,12 @@ public boolean isRestricted(Activity activity, FsPath directory, String child)
         return isRestricted(activity, directory.child(child));
     }
 
+    @Override
+    public boolean hasUnrestrictedChild(Activity activity, FsPath parent)
+    {
+        return prefixes.stream().anyMatch(p -> p.hasPrefix(parent) && !p.equals(parent));
+    }
+
     @Override
     public int hashCode()
     {

modules/common/src/main/java/org/dcache/auth/attributes/Restriction.java

@@ -122,6 +122,12 @@ public interface Restriction extends LoginAttribute, Serializable
      */
     boolean isRestricted(Activity activity, FsPath directory, String child);
 
+    /**
+     * Return true iff there is a child of the supplied path whether the
+     * activity is not restricted.
+     */
+    boolean hasUnrestrictedChild(Activity activity, FsPath parent);
+
     /**
      * Whether another object is an equivalent restriction.
      * @param other The object to compare

modules/common/src/main/java/org/dcache/auth/attributes/Restrictions.java

@@ -181,6 +181,12 @@ public boolean isRestricted(Activity activity, FsPath directory, String name)
             return false;
         }
 
+        @Override
+        public boolean hasUnrestrictedChild(Activity activity, FsPath parent)
+        {
+            return !restrictions.stream().anyMatch(r -> !r.hasUnrestrictedChild(activity, parent));
+        }
+
         @Override
         public boolean equals(Object other)
         {

modules/common/src/test/java/org/dcache/auth/attributes/DenyAcitivityRestrictionTests.java

@@ -93,4 +93,21 @@ public void shouldRestrictTwoActivities()
         assertThat(r.isRestricted(DELETE, path), is(equalTo(true)));
         assertThat(r.isRestricted(MANAGE, path), is(equalTo(true)));
     }
+
+    @Test
+    public void shouldHaveUnrestrictedChild()
+    {
+        FsPath path = FsPath.create("/some/arbitrary/path");
+
+        Restriction r = new DenyActivityRestriction(DELETE, MANAGE);
+
+        assertThat(r.hasUnrestrictedChild(DOWNLOAD, path), is(equalTo(true)));
+        assertThat(r.hasUnrestrictedChild(LIST, path), is(equalTo(true)));
+        assertThat(r.hasUnrestrictedChild(READ_METADATA, path), is(equalTo(true)));
+        assertThat(r.hasUnrestrictedChild(UPDATE_METADATA, path), is(equalTo(true)));
+        assertThat(r.hasUnrestrictedChild(UPLOAD, path), is(equalTo(true)));
+
+        assertThat(r.hasUnrestrictedChild(DELETE, path), is(equalTo(false)));
+        assertThat(r.hasUnrestrictedChild(MANAGE, path), is(equalTo(false)));
+    }
 }

modules/common/src/test/java/org/dcache/auth/attributes/PrefixRestrictionTest.java

@@ -0,0 +1,87 @@
+/* dCache - http://www.dcache.org/
+ *
+ * Copyright (C) 2018 Deutsches Elektronen-Synchrotron
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program 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 Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.dcache.auth.attributes;
+
+import org.junit.Test;
+
+import diskCacheV111.util.FsPath;
+
+import static org.dcache.auth.attributes.Activity.LIST;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.*;
+
+public class PrefixRestrictionTest
+{
+    @Test
+    public void shouldNotHaveUnrestrictedChildFromRootForEmptyPrefix()
+    {
+        Restriction r = new PrefixRestriction();
+
+        assertThat(r.hasUnrestrictedChild(LIST, FsPath.ROOT), is(equalTo(false)));
+    }
+
+
+    @Test
+    public void shouldNotHaveUnrestrictedChildFromPathForEmptyPrefix()
+    {
+        Restriction r = new PrefixRestriction();
+
+        assertThat(r.hasUnrestrictedChild(LIST, FsPath.create("/foo/bar")), is(equalTo(false)));
+    }
+
+    @Test
+    public void shouldHaveUnrestrictedChildFromRootForSinglePrefix()
+    {
+        Restriction r = new PrefixRestriction(FsPath.create("/foo/bar"));
+
+        assertThat(r.hasUnrestrictedChild(LIST, FsPath.ROOT), is(equalTo(true)));
+    }
+
+    @Test
+    public void shouldHaveUnrestrictedChildFromParentForSinglePrefix()
+    {
+        Restriction r = new PrefixRestriction(FsPath.create("/foo/bar"));
+
+        assertThat(r.hasUnrestrictedChild(LIST, FsPath.create("/foo")), is(equalTo(true)));
+    }
+
+    @Test
+    public void shouldHaveUnrestrictedChildFromSameDirForSinglePrefix()
+    {
+        Restriction r = new PrefixRestriction(FsPath.create("/foo/bar"));
+
+        assertThat(r.hasUnrestrictedChild(LIST, FsPath.create("/foo/bar")), is(equalTo(false)));
+    }
+
+    @Test
+    public void shouldHaveUnrestrictedChildFromSiblingDirForSinglePrefix()
+    {
+        Restriction r = new PrefixRestriction(FsPath.create("/foo/bar"));
+
+        assertThat(r.hasUnrestrictedChild(LIST, FsPath.create("/foo/baz")), is(equalTo(false)));
+    }
+
+    @Test
+    public void shouldHaveUnrestrictedChildFromChildForSinglePrefix()
+    {
+        Restriction r = new PrefixRestriction(FsPath.create("/foo/bar"));
+
+        assertThat(r.hasUnrestrictedChild(LIST, FsPath.create("/foo/bar/baz")), is(equalTo(false)));
+    }
+}

modules/common/src/test/java/org/dcache/auth/attributes/RestrictionsTests.java

@@ -21,6 +21,8 @@
 
 import org.junit.Test;
 
+import java.util.EnumSet;
+
 import diskCacheV111.util.FsPath;
 
 import static org.dcache.auth.attributes.Activity.*;
@@ -259,4 +261,42 @@ public void shouldConcatTwoNonSubsumptionAndSubsumptionAsComposite()
         assertThat(concat.isRestricted(UPDATE_METADATA, path), is(equalTo(false)));
     }
 
+    @Test
+    public void shouldCombinePathAndActivityRestrictions()
+    {
+        Restriction onlyUpload = new DenyActivityRestriction(EnumSet.complementOf(EnumSet.of(UPLOAD)));
+        Restriction pathRestriction = new PrefixRestriction(FsPath.create("/foo/bar/latest-results.dat"));
+
+        Restriction concat = Restrictions.concat(onlyUpload, pathRestriction);
+
+        assertThat(concat.isRestricted(UPLOAD, FsPath.ROOT), is(equalTo(true)));
+        assertThat(concat.isRestricted(DOWNLOAD, FsPath.ROOT), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(UPLOAD, FsPath.ROOT), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(DOWNLOAD, FsPath.ROOT), is(equalTo(false)));
+
+        assertThat(concat.isRestricted(UPLOAD, FsPath.create("/foo")), is(equalTo(true)));
+        assertThat(concat.isRestricted(DOWNLOAD, FsPath.create("/foo")), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(UPLOAD, FsPath.create("/foo")), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(DOWNLOAD, FsPath.create("/foo")), is(equalTo(false)));
+
+        assertThat(concat.isRestricted(UPLOAD, FsPath.create("/foo/bar")), is(equalTo(true)));
+        assertThat(concat.isRestricted(DOWNLOAD, FsPath.create("/foo/bar")), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(UPLOAD, FsPath.create("/foo/bar")), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(DOWNLOAD, FsPath.create("/foo/bar")), is(equalTo(false)));
+
+        assertThat(concat.isRestricted(UPLOAD, FsPath.create("/foo/bar/latest-results.dat")), is(equalTo(false)));
+        assertThat(concat.isRestricted(DOWNLOAD, FsPath.create("/foo/bar/latest-results.dat")), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(UPLOAD, FsPath.create("/foo/bar/latest-results.dat")), is(equalTo(false)));
+        assertThat(concat.hasUnrestrictedChild(DOWNLOAD, FsPath.create("/foo/bar/latest-results.dat")), is(equalTo(false)));
+
+        assertThat(concat.isRestricted(UPLOAD, FsPath.create("/baz")), is(equalTo(true)));
+        assertThat(concat.isRestricted(DOWNLOAD, FsPath.create("/baz")), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(UPLOAD, FsPath.create("/baz")), is(equalTo(false)));
+        assertThat(concat.hasUnrestrictedChild(DOWNLOAD, FsPath.create("/baz")), is(equalTo(false)));
+
+        assertThat(concat.isRestricted(UPLOAD, FsPath.create("/foo/baz")), is(equalTo(true)));
+        assertThat(concat.isRestricted(DOWNLOAD, FsPath.create("/foo/baz")), is(equalTo(true)));
+        assertThat(concat.hasUnrestrictedChild(UPLOAD, FsPath.create("/foo/baz")), is(equalTo(false)));
+        assertThat(concat.hasUnrestrictedChild(DOWNLOAD, FsPath.create("/foo/baz")), is(equalTo(false)));
+    }
 }

modules/dcache/src/main/java/diskCacheV111/namespace/PnfsManagerV3.java

@@ -1168,7 +1168,12 @@ public void createEntry(PnfsCreateEntryMessage pnfsMessage)
             switch (type) {
             case DIR:
                 _log.info("create directory {}", path);
-                checkRestrictionOnParent(pnfsMessage, MANAGE);
+                // as a special case, if the user is allowed to upload into
+                // a child directory then they are also allowed to create this
+                // directory
+                if (!pnfsMessage.getRestriction().hasUnrestrictedChild(UPLOAD, pnfsMessage.getFsPath())) {
+                    checkRestrictionOnParent(pnfsMessage, MANAGE);
+                }
 
                 PnfsId pnfsId = _nameSpaceProvider.createDirectory(subject, path,
                         assign);