Repository: scitokens/scitokens

Issue:
aud value ANY not accepted when in an array
A token with audience value:
```
      ["ANY"]
```
is not accepted as valid. Should it be?

In [the _validate_aud method on the Enforcer object](https://github.com/scitokens/scitokens/blob/2be3e7dc5781b8f24ed20b21a6b660c5cb80a814/src/scitokens/scitokens.py#L596C2-L602C24) it looks like `ANY` is only treated as special if it is a stand-alone string. 

If this is not the desired behavior, I think it would work to test if `value` is a list and, if so, return `True` if the list contains the string `"ANY"`. 

Discussion:

Patch:
diff --git a/src/scitokens/scitokens.py b/src/scitokens/scitokens.py
index bf9cd74..b88b638 100644
--- a/src/scitokens/scitokens.py
+++ b/src/scitokens/scitokens.py
@@ -598,9 +598,7 @@ class Enforcer(object):
             return False
         elif self._audience == "ANY":
             return False
-        elif value == "ANY":
-            return True
-        
+
         # Convert the value and self._audience both to sets
         # Then perform set intersection
         values = []
@@ -609,6 +607,11 @@ class Enforcer(object):
         else:
             values = value
         set_value = set(values)
+
+        # Check if "ANY" is in the set_value, and always accept if that is true
+        if "ANY" in set_value:
+            return True
+
         audiences = []
         if not isinstance(self._audience, list):
             audiences = [self._audience]
diff --git a/src/scitokens/tools/admin_add_key.py b/src/scitokens/tools/admin_add_key.py
deleted file mode 100644
index 892f401..0000000
--- a/src/scitokens/tools/admin_add_key.py
+++ /dev/null
@@ -1,25 +0,0 @@
-import argparse
-from scitokens.utils.keycache import KeyCache
-
-def add_args():
-    """
-    Generate the ArgumentParser object for the CLI.
-    """
-    parser = argparse.ArgumentParser(description='Remove a local cached token')
-    parser.add_argument('issuer', help='issuer')
-    parser.add_argument('key_id', help='key_id')
-    args = parser.parse_args()
-    return args
-
-def main():
-    args = add_args()
-    keycache = KeyCache()
-    res = keycache.add_key(args.issuer, args.key_id)
-    if res != None:
-        print("Successfully added token with issuer = {} and key_id = {}!".format(args.issuer, args.key_id))
-        print(res)
-    else:
-        print("Invalid issuer = {} and key_id = {} !".format(args.issuer, args.key_id))
-
-if __name__ == "__main__":
-    main()
diff --git a/src/scitokens/tools/admin_list_keys.py b/src/scitokens/tools/admin_list_keys.py
deleted file mode 100644
index 2bdb239..0000000
--- a/src/scitokens/tools/admin_list_keys.py
+++ /dev/null
@@ -1,14 +0,0 @@
-from scitokens.utils.keycache import KeyCache
-
-def main():
-    # TODO: Make this work
-    keycache = KeyCache()
-    res = keycache.list_keys()
-    header = ["issuer", "expiration", "key_id", "keydata", "next_update"]
-    print("{:<30} | {:<19} | {:<35} | {:<20} | {}".format(header[0], header[1], header[2], header[3], header[4]))
-    print("-" * 135)
-    for record in res:
-        print("{:<30} | {:<19} | {:<35} | {:<20} | {}".format(record[0], record[1], record[2], record[3][:20], record[4]))
-
-if __name__ == "__main__":
-    main()
diff --git a/src/scitokens/tools/admin_remove_key.py b/src/scitokens/tools/admin_remove_key.py
deleted file mode 100644
index 9dde68d..0000000
--- a/src/scitokens/tools/admin_remove_key.py
+++ /dev/null
@@ -1,24 +0,0 @@
-import argparse
-from scitokens.utils.keycache import KeyCache
-
-def add_args():
-    """
-    Generate the ArgumentParser object for the CLI.
-    """
-    parser = argparse.ArgumentParser(description='Remove a local cached token')
-    parser.add_argument('issuer', help='issuer')
-    parser.add_argument('key_id', help='key_id')
-    args = parser.parse_args()
-    return args
-
-def main():
-    args = add_args()
-    keycache = KeyCache()
-    res = keycache.remove_key(args.issuer, args.key_id)
-    if res == True:
-        print("Successfully deleted token with issuer = {} and key_id = {}!".format(args.issuer, args.key_id))
-    else:
-        print("Token with issuer = {} and key_id = {} does NOT exist!".format(args.issuer, args.key_id))
-
-if __name__ == "__main__":
-    main()
diff --git a/src/scitokens/tools/admin_update_keys.py b/src/scitokens/tools/admin_update_keys.py
deleted file mode 100644
index e2f76b3..0000000
--- a/src/scitokens/tools/admin_update_keys.py
+++ /dev/null
@@ -1,23 +0,0 @@
-import argparse
-from scitokens.utils.keycache import KeyCache
-
-def add_args():
-    """
-    Generate the ArgumentParser object for the CLI.
-    """
-    parser = argparse.ArgumentParser(description='Update all tokens in the cache')
-    parser.add_argument('-f', '--force', action='store_true', help='Force refresh all tokens')
-    args = parser.parse_args()
-    return args
-
-def main():
-    args = add_args()
-    keycache = KeyCache()
-    res = keycache.update_all_keys(force_refresh=args.force)
-    for i in res:
-        print(i)
-
-if __name__ == "__main__":
-    main()
-
-
diff --git a/src/scitokens/utils/keycache.py b/src/scitokens/utils/keycache.py
index d47098a..373f1ba 100644
--- a/src/scitokens/utils/keycache.py
+++ b/src/scitokens/utils/keycache.py
@@ -28,8 +28,6 @@ import cryptography.hazmat.primitives.asymmetric.rsa as rsa
 from scitokens.utils.errors import SciTokensException, MissingKeyException, NonHTTPSIssuer, UnableToCreateCache, UnsupportedKeyException
 from scitokens.utils import long_from_bytes
 import scitokens.utils.config as config
-from cryptography.hazmat.primitives import serialization
-from urllib.error import URLError
 
 
 CACHE_FILENAME = "scitokens_keycache.sqllite"
@@ -137,7 +135,7 @@ class KeyCache(object):
         conn.close()
 
 
-    def getkeyinfo(self, issuer, key_id=None, insecure=False, force_refresh=False):
+    def getkeyinfo(self, issuer, key_id=None, insecure=False):
         """
         Get the key information
 
@@ -180,43 +178,14 @@ class KeyCache(object):
 
             # If it's not time to update the key, but the key is still valid
             elif self._check_validity(row):
-                # If force_refresh is set, then update the key
-                if force_refresh:
-                    try:
-                        # update the keycache
-                        public_key, cache_timer = self._get_issuer_publickey(issuer, key_id, insecure)
-                        self.addkeyinfo(issuer, key_id, public_key, cache_timer)
-                        return public_key
-                    except ValueError as ex:
-                        logging.exception("Unable to parse JSON stored in keycache.  "
-                              "This likely means the database format needs"
-                              "to be updated, which we will now do automatically.\n{0}".format(str(ex)))
-                        self._delete_cache_entry(issuer, key_id)
-                        raise ex
-                    except URLError as ex:
-                        raise URLError("Unable to get key from issuer.\n{0}".format(str(ex)))
-                    except MissingKeyException as ex:
-                        raise MissingKeyException("Unable to force refresh key. \n{0}".format(str(ex)))
-                
                 keydata = self._parse_key_data(row['issuer'], row['key_id'], row['keydata'])
                 if keydata:
                     return load_pem_public_key(keydata.encode(), backend=backends.default_backend())
                 
                 # update the keycache
-                try:
-                    public_key, cache_timer = self._get_issuer_publickey(issuer, key_id, insecure)
-                    self.addkeyinfo(issuer, key_id, public_key, cache_timer)
-                    return public_key
-                except ValueError as ex:
-                        logging.exception("Unable to parse JSON stored in keycache.  "
-                              "This likely means the database format needs"
-                              "to be updated, which we will now do automatically.\n{0}".format(str(ex)))
-                        self._delete_cache_entry(issuer, key_id)
-                        raise ex
-                except URLError as ex:
-                    raise URLError("Unable to get key from issuer.\n{0}".format(str(ex)))
-                except Exception as ex:
-                    raise MissingKeyException("Key in keycache is expired and unable to get a new key.\n{0}".format(str(ex)))
+                public_key, cache_timer = self._get_issuer_publickey(issuer, key_id, insecure)
+                self.addkeyinfo(issuer, key_id, public_key, cache_timer)
+                return public_key
 
 
             # If it's not time to update the key, and the key is not valid
@@ -229,21 +198,10 @@ class KeyCache(object):
 
         # If it reaches here, then no key was found in the SQL
         # Try checking the issuer (negative cache?)
-        try:
-            public_key, cache_timer = self._get_issuer_publickey(issuer, key_id, insecure)
-            self.addkeyinfo(issuer, key_id, public_key, cache_timer)
-            return public_key
-        except ValueError as ex:
-            logging.exception("Unable to parse JSON stored in keycache.  "
-                              "This likely means the database format needs"
-                              "to be updated, which we will now do automatically.\n{0}".format(str(ex)))
-            self._delete_cache_entry(issuer, key_id)
-            raise ex
-        except URLError as ex:
-            raise URLError("Unable to get key from issuer.\n{0}".format(str(ex)))
-        except Exception as ex:
-            raise MissingKeyException("No key was found in keycache and unable to get key.\n{0}".format(str(ex)))
+        public_key, cache_timer = self._get_issuer_publickey(issuer, key_id, insecure)
+        self.addkeyinfo(issuer, key_id, public_key, cache_timer)
 
+        return public_key
 
     @classmethod
     def _check_validity(cls, key_info):
@@ -411,68 +369,3 @@ class KeyCache(object):
         # We can also close the connection if we are done with it.
         # Just be sure any changes have been committed or they will be lost.
         conn.close()
-
-
-    def list_keys(self):
-        """
-        List all keys in keycache
-        """
-        conn = sqlite3.connect(self.cache_location)
-        curs = conn.cursor()
-        res = curs.execute("SELECT issuer, DATETIME(expiration, 'unixepoch'), key_id, keydata, DATETIME(next_update, 'unixepoch') FROM keycache")
-        tokens = res.fetchall()
-        
-        conn.close()
-        return tokens
-    
-
-    def remove_key(self, issuer, key_id):
-        """
-        Remove a specific key from keycache
-        """
-        conn = sqlite3.connect(self.cache_location)
-        curs = conn.cursor()
-        
-        res = curs.execute("SELECT * FROM keycache WHERE issuer = ? AND key_id = ?", [issuer, key_id])
-        if res.fetchone() is None:
-            conn.close()
-            return False
-        
-        res = curs.execute("DELETE FROM keycache WHERE issuer = ? AND key_id = ?", [issuer, key_id])
-        res = curs.fetchall()
-        conn.commit()
-        conn.close()
-        return True
-
-
-    def add_key(self, issuer, key_id, force_refresh=False):
-        """
-        Add a key or update an existing one in keycache
-        """
-        pubkey = self.getkeyinfo(issuer, key_id, force_refresh=force_refresh)
-        if pubkey is None:
-            return None
-    
-        pubkey_pem = pubkey.public_bytes(
-            encoding=serialization.Encoding.PEM,
-            format=serialization.PublicFormat.SubjectPublicKeyInfo
-        )
-        return pubkey_pem
-    
-
-    def update_all_keys(self, force_refresh=False):
-        """
-        Update all keys in keycache
-        If force_refresh is True, we refresh all keys regardless of update time
-        """
-        conn = sqlite3.connect(self.cache_location)
-        curs = conn.cursor()
-        res = curs.execute("SELECT issuer, key_id FROM keycache")
-        tokens = res.fetchall()
-        conn.close()
-        
-        res = []
-        for issuer, key_id in tokens:
-            updated = self.add_key(issuer, key_id, force_refresh=force_refresh)
-            res.append(updated)
-        return res


Test patch:
diff --git a/tests/test_scitokens.py b/tests/test_scitokens.py
index 92ecac4..11a435e 100644
--- a/tests/test_scitokens.py
+++ b/tests/test_scitokens.py
@@ -142,6 +142,14 @@ class TestEnforcer(unittest.TestCase):
         self._token2["aud"] = "ANY"
         self.assertTrue(enf.test(self._token2, "read", "/foo/bar"), msg=enf.last_failure)
 
+        # Now set the audience to ANY
+        self._token2["aud"] = ["ANY"]
+        self.assertTrue(enf.test(self._token2, "read", "/foo/bar"), msg=enf.last_failure)
+
+        # Now set the audience to ANY
+        self._token2["aud"] = ["notathing.com", "ANY"]
+        self.assertTrue(enf.test(self._token2, "read", "/foo/bar"), msg=enf.last_failure)
+
         # Now to the correct audience
         self._token2["aud"] = "https://example.unl.edu"
         self.assertTrue(enf.test(self._token2, "read", "/foo/bar"), msg=enf.last_failure)