From 226a9a589275fb7eaaf6870d93061919399208a9 Mon Sep 17 00:00:00 2001
From: Karl Czajkowski <karlcz@isi.edu>
Date: Mon, 9 Sep 2024 16:31:10 -0700
Subject: [PATCH 1/2] add tests for API /path/name;acl/access/role used by our
 hatrac CLI

the role is URL-quoted in the API and needs to be unquoted when
compared to ACL arrays in the database. this seems to be broken since
the code was refactored for Flask and other implicit URL unquoting
was removed from the dispatch path.
---
 test/rest-smoketest.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/test/rest-smoketest.sh b/test/rest-smoketest.sh
index 5712370..0e0b007 100644
--- a/test/rest-smoketest.sh
+++ b/test/rest-smoketest.sh
@@ -49,11 +49,19 @@ 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
+TEST_ACL_URI=/tmp/${RUNKEY}-test-acl-uri.json
 
 cat > ${TEST_ACL_ANON} <<EOF
 ["*"]
 EOF
 
+test_acl_uri_member='scheme://authority/path'
+test_acl_uri_member_quoted='scheme%3A%2F%2Fauthority%2Fpath'
+
+cat > ${TEST_ACL_URI} <<EOF
+["unlikely value", "${test_acl_uri_member}"]
+EOF
+
 cleanup()
 {
     rm -f ${RESPONSE_HEADERS} ${RESPONSE_CONTENT} ${TEST_DATA} ${TEST_ACL_ANON}
@@ -788,7 +796,12 @@ dotest "200::application/json::*" "/ns-${RUNKEY}/foo;acl/"
 dotest "200::application/json::*" "/ns-${RUNKEY}/foo;acl/owner"
 dotest "200::application/json::*" "/ns-${RUNKEY}/foo;acl/owner" --head
 dotest "200::application/json::*" "/ns-${RUNKEY}/foo;acl/create"
-dotest "404::*::*" "/ns-${RUNKEY}/foo/bar;acl/create/DUMMY"
+dotest "204::*::*" "/ns-${RUNKEY}/foo/bar;acl/create" -T ${TEST_ACL_URI} -H "Content-Type: application/json"
+dotest "200::*::*" "/ns-${RUNKEY}/foo/bar;acl/create/${test_acl_uri_member_quoted}"
+dotest "204::*::*" "/ns-${RUNKEY}/foo/bar;acl/create/${test_acl_uri_member_quoted}" -X DELETE
+dotest "404::*::*" "/ns-${RUNKEY}/foo/bar;acl/create/${test_acl_uri_member_quoted}"
+dotest "204::*::*" "/ns-${RUNKEY}/foo/bar;acl/create/${test_acl_uri_member_quoted}" -X PUT
+dotest "200::*::*" "/ns-${RUNKEY}/foo/bar;acl/create/${test_acl_uri_member_quoted}"
 dotest "204::*::*" "/ns-${RUNKEY}/foo/bar;acl/create/DUMMY" -X PUT
 dotest "200::text/plain*::*" "/ns-${RUNKEY}/foo/bar;acl/create/DUMMY" --head
 dotest "204::*::*" "/ns-${RUNKEY}/foo/bar;acl/create/DUMMY" -X DELETE -H "If-Match: *"

From b8e94c241e33346fb0b2aea7909c47f2523cc983 Mon Sep 17 00:00:00 2001
From: Karl Czajkowski <karlcz@isi.edu>
Date: Mon, 9 Sep 2024 16:33:23 -0700
Subject: [PATCH 2/2] add URL quote handling for /path/name;acl/access/role

1. The role must be unquoted when probing the database content
2. The role should be quoted in URLs within NotFound errors
---
 hatrac/model/directory/pgsql.py | 2 +-
 hatrac/rest/acl.py              | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hatrac/model/directory/pgsql.py b/hatrac/model/directory/pgsql.py
index e8b3ec2..79c07e2 100644
--- a/hatrac/model/directory/pgsql.py
+++ b/hatrac/model/directory/pgsql.py
@@ -95,7 +95,7 @@ def get_content(self, client_context, get_data=True):
     def __getitem__(self, role):
         if role not in self:
             raise core.NotFound(
-                'ACL member %s;acl/%s/%s not found.' % (self.resource, self.access, role)
+                'ACL member %s;acl/%s/%s not found.' % (self.resource, self.access, urllib.parse.quote(role, safe=''))
             )
         entry = ACLEntry(role + '\n')
         entry.resource = self.resource
diff --git a/hatrac/rest/acl.py b/hatrac/rest/acl.py
index 50a81eb..ace0188 100644
--- a/hatrac/rest/acl.py
+++ b/hatrac/rest/acl.py
@@ -6,6 +6,7 @@
 
 import json
 from flask import request, g as hatrac_ctx
+from urllib.parse import unquote
 
 from . import app
 from .core import RestHandler, \
@@ -20,6 +21,7 @@ def __init__(self):
     def put(self, access, role, path="/", name="", version=""):
         """Add entry to ACL."""
         self.enforce_firewall('manage_acl')
+        role = unquote(role)
         resource = self.resolve_name_or_version(
             path, name, version
         )
@@ -35,6 +37,7 @@ def put(self, access, role, path="/", name="", version=""):
     def delete(self, access, role, path="/", name="", version=""):
         """Remove entry from ACL."""
         self.enforce_firewall('manage_acl')
+        role = unquote(role)
         resource = self.resolve_name_or_version(
             path, name, version
         )
@@ -50,6 +53,7 @@ def delete(self, access, role, path="/", name="", version=""):
     def get(self, access, role, path="/", name="", version=""):
         """Get entry from ACL."""
         self.get_body = False if request.method == 'HEAD' else True
+        role = unquote(role)
         resource = self.resolve_name_or_version(path, name, version).acls[access]
         self.set_http_etag(hash_list(resource))
         resource = resource[role]