From cc84d99bf47e6852b7a12cf50be5695f36ad8f70 Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Mon, 30 Oct 2023 14:41:09 -0500 Subject: [PATCH 1/5] Generate scitokens.conf files even for public namespaces, to support public read/auth write (SOFTWARE-5727) --- src/stashcache.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/stashcache.py b/src/stashcache.py index 39467e53c..fc97be44e 100644 --- a/src/stashcache.py +++ b/src/stashcache.py @@ -370,8 +370,6 @@ def generate_cache_scitokens(global_data: GlobalData, fqdn: str, suppress_errors for vo_name, stashcache_obj in vos_data.stashcache_by_vo_name.items(): for namespace in stashcache_obj.namespaces.values(): # type: Namespace - if namespace.is_public(): - continue if not namespace_allows_cache_resource(namespace, cache_resource): continue if not resource_allows_namespace(cache_resource, namespace): @@ -486,8 +484,6 @@ def generate_origin_scitokens(global_data: GlobalData, fqdn: str, suppress_error for vo_name, stashcache_obj in vos_data.stashcache_by_vo_name.items(): for namespace in stashcache_obj.namespaces.values(): - if namespace.is_public(): - continue if not namespace_allows_origin_resource(namespace, origin_resource): continue if not resource_allows_namespace(origin_resource, namespace): @@ -562,7 +558,7 @@ def _namespace_dict(ns: Namespace): nsdict = { "path": ns.path, "readhttps": not ns.is_public(), - "usetokenonread": any(isinstance(a, SciTokenAuth) for a in ns.authz_list), + "usetokenonread": not ns.is_public() and any(isinstance(a, SciTokenAuth) for a in ns.authz_list), "writebackhost": ns.writeback, "dirlisthost": ns.dirlist, "caches": [], From 3d142e544b41a8a345525186b856c7f1930a96f1 Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Tue, 31 Oct 2023 17:38:38 -0500 Subject: [PATCH 2/5] Add tests for public read/auth write (i.e. scitokens.conf being generated for a namespace with PUBLIC authorization) --- src/tests/data/testvo.yaml | 4 ++++ src/tests/test_stashcache.py | 23 ++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/tests/data/testvo.yaml b/src/tests/data/testvo.yaml index 1a25cbe89..58f2338f3 100644 --- a/src/tests/data/testvo.yaml +++ b/src/tests/data/testvo.yaml @@ -45,6 +45,10 @@ DataFederations: - Path: /testvo/PUBLIC Authorizations: - PUBLIC + - SciTokens: + Issuer: https://test.wisc.edu + Base Path: /testvo + Map Subject: False AllowedOrigins: # sc-origin.test.wisc.edu - TEST_STASHCACHE_ORIGIN diff --git a/src/tests/test_stashcache.py b/src/tests/test_stashcache.py index d5552cc2b..2654fb6b7 100644 --- a/src/tests/test_stashcache.py +++ b/src/tests/test_stashcache.py @@ -25,8 +25,9 @@ EMPTY_LINE_REGEX = re.compile(r'^\s*(#|$)') # Empty or comment-only lines I2_TEST_CACHE = "osg-sunnyvale-stashcache.nrp.internet2.edu" # ^^ one of the Internet2 caches; these serve both public and LIGO data +# fake origins in our test data: TEST_ITB_HELM_ORIGIN = "helm-origin.osgdev.test.io" -# ^^ a fake origin that's in our test data +TEST_SC_ORIGIN = "sc-origin.test.wisc.edu" # Some DNs I can use for testing and the hashes they map to. @@ -127,6 +128,26 @@ def test_scitokens_issuer_sections(self, client: flask.Flask): print(f"Generated origin scitokens.conf text:\n{origin_scitokens_conf}\n", file=sys.stderr) raise + def test_scitokens_issuer_public_read_auth_write(self, client: flask.Flask): + test_global_data = get_test_global_data(global_data) + origin_scitokens_conf = stashcache.generate_origin_scitokens( + test_global_data, TEST_SC_ORIGIN) + assert origin_scitokens_conf.strip(), "Generated scitokens.conf empty" + + cp = ConfigParser() + cp.read_string(origin_scitokens_conf, "origin_scitokens.conf") + try: + assert "Global" in cp, "Missing Global section" + assert "Issuer test.wisc.edu" in cp, \ + "Expected issuer missing" + assert "base_path" in cp["Issuer test.wisc.edu"], \ + "'Issuer test.wisc.edu' section missing expected attribute" + assert cp["Issuer test.wisc.edu"]["base_path"] == "/testvo", \ + "'Issuer test.wisc.edu' section has wrong base path" + except AssertionError: + print(f"Generated origin scitokens.conf text:\n{origin_scitokens_conf}\n", file=sys.stderr) + raise + def test_None_fdqn_isnt_error(self, client: flask.Flask): stashcache.generate_cache_authfile(global_data, None) From 52b705469d8921b2d6f6ce844831cf45c6924d4d Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Wed, 8 Nov 2023 16:15:05 -0600 Subject: [PATCH 3/5] Fix test issuer section name in test_scitokens_issuer_public_read_auth_write() --- src/tests/test_stashcache.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/tests/test_stashcache.py b/src/tests/test_stashcache.py index 2654fb6b7..c4b489126 100644 --- a/src/tests/test_stashcache.py +++ b/src/tests/test_stashcache.py @@ -138,12 +138,12 @@ def test_scitokens_issuer_public_read_auth_write(self, client: flask.Flask): cp.read_string(origin_scitokens_conf, "origin_scitokens.conf") try: assert "Global" in cp, "Missing Global section" - assert "Issuer test.wisc.edu" in cp, \ + assert "Issuer https://test.wisc.edu" in cp, \ "Expected issuer missing" - assert "base_path" in cp["Issuer test.wisc.edu"], \ - "'Issuer test.wisc.edu' section missing expected attribute" - assert cp["Issuer test.wisc.edu"]["base_path"] == "/testvo", \ - "'Issuer test.wisc.edu' section has wrong base path" + assert "base_path" in cp["Issuer https://test.wisc.edu"], \ + "'Issuer https://test.wisc.edu' section missing expected attribute" + assert cp["Issuer https://test.wisc.edu"]["base_path"] == "/testvo", \ + "'Issuer https://test.wisc.edu' section has wrong base path" except AssertionError: print(f"Generated origin scitokens.conf text:\n{origin_scitokens_conf}\n", file=sys.stderr) raise From 04ff88da101e4adaf24bbaa6a1a0a74cc6fa79c2 Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Tue, 31 Oct 2023 18:10:58 -0500 Subject: [PATCH 4/5] Add namespaces json correctness test for public read/auth write --- src/tests/data/testvo.yaml | 1 + src/tests/test_stashcache.py | 20 +++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/tests/data/testvo.yaml b/src/tests/data/testvo.yaml index 58f2338f3..9f3214183 100644 --- a/src/tests/data/testvo.yaml +++ b/src/tests/data/testvo.yaml @@ -56,6 +56,7 @@ DataFederations: - TEST_STASHCACHE_ORIGIN_2000 AllowedCaches: - ANY + Writeback: "https://sc-origin.test.wisc.edu:1095" - Path: /testvo/itb/helm-origin/PUBLIC Authorizations: diff --git a/src/tests/test_stashcache.py b/src/tests/test_stashcache.py index c4b489126..9e799fe69 100644 --- a/src/tests/test_stashcache.py +++ b/src/tests/test_stashcache.py @@ -128,8 +128,26 @@ def test_scitokens_issuer_sections(self, client: flask.Flask): print(f"Generated origin scitokens.conf text:\n{origin_scitokens_conf}\n", file=sys.stderr) raise - def test_scitokens_issuer_public_read_auth_write(self, client: flask.Flask): + def test_scitokens_issuer_public_read_auth_write_namespaces_info(self, client: flask.Flask): test_global_data = get_test_global_data(global_data) + + namespaces_json = stashcache.get_namespaces_info(test_global_data) + namespaces = namespaces_json["namespaces"] + testvo_PUBLIC_namespace_list = [ + ns for ns in namespaces if ns.get("path") == "/testvo/PUBLIC" + ] + assert testvo_PUBLIC_namespace_list, "/testvo/PUBLIC namespace not found" + ns = testvo_PUBLIC_namespace_list[0] + assert ns["usetokenonread"] is False, \ + "usetokenonread is wrong for public namespace" + assert ns["readhttps"] is False, \ + "readhttps is wrong for public namespace" + assert ns["writebackhost"] == f"https://{TEST_SC_ORIGIN}:1095", \ + "writebackhost is wrong for namespace with auth write" + + def test_scitokens_issuer_public_read_auth_write_scitokens_conf(self, client: flask.Flask): + test_global_data = get_test_global_data(global_data) + origin_scitokens_conf = stashcache.generate_origin_scitokens( test_global_data, TEST_SC_ORIGIN) assert origin_scitokens_conf.strip(), "Generated scitokens.conf empty" From fe3ecb3ed2de85da3e88f0015135d9597f703e43 Mon Sep 17 00:00:00 2001 From: Matyas Selmeci Date: Wed, 8 Nov 2023 17:17:16 -0600 Subject: [PATCH 5/5] Don't stop parsing the auth list at the first PUBLIC auth method --- src/webapp/data_federation.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/webapp/data_federation.py b/src/webapp/data_federation.py index 06a700fe3..204c05540 100644 --- a/src/webapp/data_federation.py +++ b/src/webapp/data_federation.py @@ -196,7 +196,7 @@ def __init__( self.credential_generation = credential_generation def is_public(self) -> bool: - return self.authz_list and self.authz_list[0].is_public + return any(x for x in self.authz_list if x.is_public) def _parse_authz_scitokens(attributes: Dict, authz: Dict) -> Tuple[AuthMethod, Optional[str]]: @@ -392,8 +392,5 @@ def parse_authz_list(self, path: str, unparsed_authz_list: List[Union[str, Dict] if err: self.errors.add(f"Namespace {path}: {err}") continue - if parsed_authz.is_public: - return [parsed_authz] - else: - authz_list.append(parsed_authz) + authz_list.append(parsed_authz) return authz_list