Merge pull request #68 from informatics-isi-edu/authz_consistency_fixes

Authz consistency fixes

docs/REST-API.md

@@ -726,7 +726,7 @@ as a document:
 	Host: authority_name
 	Accept: application/json
 	If-None-Match: etag_value
-	
+
 for which the successful response is:
 
     200 OK
@@ -816,18 +816,20 @@ each resource type is:
 - Namespace
   - `owner`: lists roles considered to be owners of the namespace.
   - `create`: lists roles permitted to create new children in the namespace.
+  - `read`: lists roles permitted to read the list of child names in the namespace.
   - `subtree-owner`: lists roles considered to be owners of any namespace, object, or object version beneath the namespace.
   - `subtree-create`: lists roles permitted to create new children of the namespace or of any namespace beneath the namespace.
   - `subtree-update`: lists roles permitted to update data on any object beneath the namespace.
-  - `subtree-read`: lists roles permitted to read any object version beneath the namespace.
+  - `subtree-read`: lists roles permitted to read any object version or list children of any namespace beneath the namespace.
 - Object
   - `owner`: lists roles considered to be owners of the object.
   - `update`: lists roles permitted to update object with new versions.
+  - `read`: lists roles permitted to list versions of the named object.
   - `subtree-owner`: lists roles considered to be owners of any object version for the object.
-  - `subtree-read`: lists roles permitted to read any object version for the object.
+  - `subtree-read`: lists roles permitted to read any object version content for the object.
 - Object Version
   - `owner`: lists roles considered to be owners of the object version.
-  - `read`: lists roles permitted to read the object version.
+  - `read`: lists roles permitted to read the object version content.
 
 ### Lifecycle and Ownership
 

hatrac/core.py

@@ -246,7 +246,8 @@ def is_object(self):
         return False
 
     def get_content(self, client_context, get_data=True):
-        self.container.resource.enforce_acl(['owner', 'ancestor_owner'], client_context)
+        # subtree-read is copied from object to object-version
+        self.container.resource.enforce_acl(['owner', 'read', 'ancestor_owner', 'ancestor_read', 'subtree-read'], client_context)
         body = self + '\n'
         return len(body), Metadata({'content-type': 'text/plain'}), body
 
@@ -255,7 +256,8 @@ def is_object(self):
         return False
 
     def get_content(self, client_context, get_data=True):
-        self.container.resource.enforce_acl(['owner', 'ancestor_owner'], client_context)
+        # subtree-read is copied from object to object-version
+        self.container.resource.enforce_acl(['owner', 'read', 'ancestor_owner', 'ancestor_read', 'subtree-read'], client_context)
         body = base64.b64encode(self) + b'\n'
         return len(body), Metadata({'content-type': 'text/plain'}), body
 
@@ -313,11 +315,12 @@ def is_object(self):
         return False
 
     def get_content(self, client_context, get_data=True):
-        self.resource.enforce_acl(['owner', 'ancestor_owner'], client_context)
+        # subtree-read is copied from object to object-version
+        self.resource.enforce_acl(['owner', 'read', 'ancestor_owner', 'ancestor_read', 'subtree-read'], client_context)
         body = jsonWriter(self.to_http()) + b'\n'
         nbytes = len(body)
         return nbytes, Metadata({'content-type': 'application/json'}), body
-    
+
     def _sql_encoded_val(self, k, v):
         enc, dec = self._sql_codecs.get(k, (lambda x: x, lambda x: x))
         return enc(v)

hatrac/model/directory/pgsql.py

@@ -241,8 +241,8 @@ def drop_acl_role(self, access, role, client_context):
 
 class HatracNamespace (HatracName):
     """Represent a bound namespace."""
-    _acl_names = ['owner', 'create', 'subtree-owner', 'subtree-create', 'subtree-read', 'subtree-update']
-    _ancestor_acl_names = ['ancestor_owner', 'ancestor_create']
+    _acl_names = ['owner', 'create', 'read', 'subtree-owner', 'subtree-create', 'subtree-read', 'subtree-update']
+    _ancestor_acl_names = ['ancestor_owner', 'ancestor_create', 'ancestor_read']
 
     def __init__(self, directory, **args):
         HatracName.__init__(self, directory, **args)
@@ -264,7 +264,7 @@ def get_content(self, client_context, get_data=True):
 class HatracObject (HatracName):
     """Represent a bound object."""
     _acl_names = ['owner', 'update', 'read', 'subtree-owner', 'subtree-read']
-    _ancestor_acl_names = ['ancestor_owner', 'ancestor_update']
+    _ancestor_acl_names = ['ancestor_owner', 'ancestor_update', 'ancestor_read']
 
     def __init__(self, directory, **args):
         HatracName.__init__(self, directory, **args)
@@ -483,7 +483,7 @@ def _prepare_hatrac_stmts(self):
           WHERE n.name = $1 AND (NOT n.is_deleted OR NOT $2) ;
 
         PREPARE hatrac_version_lookup (int8, text) AS
-          SELECT v.*, n.name, n.pid, n.ancestors, %(owner_acl)s, %(read_acl)s
+          SELECT v.*, n.name, n.pid, n.ancestors, n."subtree-owner", n."subtree-read", %(owner_acl)s, %(read_acl)s
           FROM hatrac.version v
           JOIN hatrac.name n ON (v.nameid = n.id)
           WHERE v.nameid = $1 AND v.version = $2 ;
@@ -1135,7 +1135,8 @@ def _upload_cancel(self, upload, conn=None, cur=None):
         self._delete_upload(conn, cur, upload)
         return lambda : self.storage.cancel_upload(upload.name, upload.job)
 
-    @db_wrap(enforce_acl=(2, 4, ['owner', 'read', 'ancestor_owner', 'ancestor_read']))
+    # subtree-owner and subtree-read are from the parent object name record
+    @db_wrap(enforce_acl=(2, 4, ['owner', 'read', 'ancestor_owner', 'ancestor_read', 'subtree-owner', 'subtree-read']))
     def get_version_content_range(self, object, objversion, get_slice, client_context, get_data=True, conn=None, cur=None):
         """Return (nbytes, data_generator) pair for specific version."""
         if objversion.is_deleted:

test/rest-smoketest.sh

@@ -36,8 +36,8 @@ EOF
     exit 1
 }
 
-[[ -z $GOAUTH  && -z $COOKIES ]] && error
-[[ -n $COOKIES && ! -r $COOKIES ]] && error
+[[ -z $GOAUTH  && -z $COOKIES ]] && error must set COOKIES or GOAUTH env var
+[[ -n $COOKIES && ! -r $COOKIES ]] && error COOKIES file must be readable
 
 #Supported deployments: amazons3 or filesystem
 DEPLOYMENT=${DEPLOYMENT:-filesystem}
@@ -48,10 +48,15 @@ RUNKEY=smoketest-$RANDOM
 RESPONSE_HEADERS=/tmp/${RUNKEY}-response-headers
 RESPONSE_CONTENT=/tmp/${RUNKEY}-response-content
 TEST_DATA=/tmp/${RUNKEY}-test-data
+TEST_ACL_ANON=/tmp/${RUNKEY}-test-acl-anon.json
+
+cat > ${TEST_ACL_ANON} <<EOF
+["*"]
+EOF
 
 cleanup()
 {
-    rm -f ${RESPONSE_HEADERS} ${RESPONSE_CONTENT} ${TEST_DATA}
+    rm -f ${RESPONSE_HEADERS} ${RESPONSE_CONTENT} ${TEST_DATA} ${TEST_ACL_ANON}
     rm -f /tmp/parts-${RUNKEY}*
     rm -f /tmp/dummy-${RUNKEY}
 }
@@ -74,7 +79,8 @@ mycurl()
     if [[ -n $GOAUTH ]] 
     then
         curl_options+=( -H "Authorization: Globus-Goauthtoken $GOAUTH" )
-    else
+    elif [[ -n $COOKIES ]]
+    then
 	curl_options+=( -b "$COOKIES" -c "$COOKIES" )
     fi
     curl "${curl_options[@]}" "$@"
@@ -221,10 +227,32 @@ dotest()
     pattern="$1"
     url="$2"
     shift 2
+    dotest_auth "$pattern" "$url" true "$@"
+}
+
+dotest_anon()
+{
+    pattern="$1"
+    url="$2"
+    shift 2
+    dotest_auth "$pattern" "$url" false "$@"
+}
+
+dotest_auth()
+{
+    pattern="$1"
+    url="$2"
+    auth="$3"
+    shift 3
 
     request=( "$@" "${BASE_URL}$url" )
     printf "%s " "${request[@]}"  >&2
-    summary=$(mycurl "${request[@]}" )
+    if [[ "$auth" = true ]]
+    then
+        summary=$(mycurl "${request[@]}" )
+    else
+        summary=$(COOKIES= GOAUTH= mycurl "${request[@]}" )
+    fi
 
     if [[ "${request[0]}" = '--head' ]]
     then
@@ -335,6 +363,15 @@ dotest "200::text/html*::*" "/ns-${RUNKEY}/foo" -H "Accept: text/html"
 dotest "200::application/json::*" "/ns-${RUNKEY}/foo?cid=smoke" --head
 dotest "409::*::*" "/ns-${RUNKEY}/foo?cid=smoke" -X PUT -H "Content-Type: application/json"
 
+# check namespace authz corner cases
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo"
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/bar"
+dotest "204::*::*" "/ns-${RUNKEY}/foo;acl/read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json"
+dotest_anon "200::application/json::*" "/ns-${RUNKEY}/foo"
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/bar"
+dotest "204::*::*" "/ns-${RUNKEY}/foo;acl/subtree-read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json"
+dotest_anon "200::application/json::*" "/ns-${RUNKEY}/foo/bar"
+
 # test objects
 md5=$(mymd5sum < $0)
 sha=$(mysha256sum < $0)
@@ -344,6 +381,51 @@ dotest "201::text/uri-list::*" /ns-${RUNKEY}/foo/obj1 -X PUT -T $0 -H "Content-T
 obj1_vers0="$(cat ${RESPONSE_CONTENT})"
 obj1_vers0="${obj1_vers0#/hatrac}"
 
+# sanity check object status for owner
+dotest "200::*::*" "/ns-${RUNKEY}/foo/obj1"
+dotest "200::*::*" "/ns-${RUNKEY}/foo/obj1;versions"
+dotest "200::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/"
+dotest "200::*::*" "${obj1_vers0}"
+dotest "200::*::*" "${obj1_vers0};metadata/"
+
+# check read authz corner cases
+# namespace subtree-read (set above) grants all reads to objects and versions below
+dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1"
+dotest_anon "200::*::*" "${obj1_vers0}"
+dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;versions"
+dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/"
+dotest_anon "200::*::*" "${obj1_vers0};metadata/"
+# without read, all read interfaces are blocked
+dotest "204::*::*" "/ns-${RUNKEY}/foo;acl/subtree-read" -X DELETE
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1"
+dotest_anon "401::*::*" "${obj1_vers0}"
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;versions"
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/"
+dotest_anon "401::*::*" "${obj1_vers0};metadata/"
+# obj read grants versions listing but not content access
+dotest "204::*::*" "/ns-${RUNKEY}/foo/obj1;acl/read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json"
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1"
+dotest_anon "401::*::*" "${obj1_vers0}"
+dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;versions"
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/"
+dotest_anon "401::*::*" "${obj1_vers0};metadata/"
+# vers read grants content access
+dotest "204::*::*" "/ns-${RUNKEY}/foo/obj1;acl/read" -X DELETE
+dotest "204::*::*" "${obj1_vers0};acl/read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json"
+dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1"
+dotest_anon "200::*::*" "${obj1_vers0}"
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;versions"
+dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/"
+dotest_anon "200::*::*" "${obj1_vers0};metadata/"
+# obj subtree-read grants content access
+dotest "204::*::*" "${obj1_vers0};acl/read" -X DELETE
+dotest "204::*::*" "/ns-${RUNKEY}/foo/obj1;acl/subtree-read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json"
+dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1"
+dotest_anon "200::*::*" "${obj1_vers0}"
+dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;versions"
+dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/"
+dotest_anon "200::*::*" "${obj1_vers0};metadata/"
+
 # metadata on object-version
 dotest "200::application/json::*" "${obj1_vers0};metadata/"
 dotest "200::application/json::*" "${obj1_vers0};metadata/?cid=smoke"