From 0b09c0dbc173531af1318c77a47cf82f843467bd Mon Sep 17 00:00:00 2001 From: JoshuaHarris391 Date: Wed, 5 Aug 2026 12:40:42 +1000 Subject: [PATCH] feat(indexd): add check-download verification of registered objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registration alone does not prove a file can be downloaded. Two failure modes stay invisible until a user clicks the file in the portal: an Indexd record with no storage URL, which can never download, and a record Fence refuses to sign a URL for — in one prod incident the API key's user held create but not read-storage on the record's authz resource, so every signed-URL request returned 401 while everything else looked healthy. check-download walks the exact chain the portal hits (Indexd record -> storage URLs -> DRS object -> access methods -> Fence signed URL) and reports PASS/FAIL per object. It exits non-zero on any failure, so it can gate deployment steps. The env selects the API key secret and the key's JWT selects the commons, so no URL is passed and Gen3Auth is constructed without an endpoint — avoiding the WTS fallback that surfaces as a misleading 502. With no GUIDs given, the newest objects are sampled from the indexd registry (latest revision per baseid, scoped to the commons via indexd_endpoint). --- README.md | 40 ++ src/g3dt/cli/indexd_cmds.py | 69 ++- src/g3dt/indexd/file_access.py | 274 +++++++++++ .../services/indexd/verify_file_access.py | 126 +++++ tests/test_cli_commands.py | 73 ++- tests/test_file_access.py | 440 ++++++++++++++++++ 6 files changed, 1015 insertions(+), 7 deletions(-) create mode 100644 src/g3dt/indexd/file_access.py create mode 100644 src/g3dt/services/indexd/verify_file_access.py create mode 100644 tests/test_file_access.py diff --git a/README.md b/README.md index e61321a..5268f37 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,46 @@ Synthetic data is only schema-valid against the dictionary that generated it, so refuses a batch that doesn't match the version being uploaded (override with `--allow-version-mismatch`). +## Verifying download access (check-download) + +Registration alone does not prove a file can be downloaded. Two failure modes +are invisible until a user clicks the file in the portal: an Indexd record +with no storage URL (nothing to download, ever), and a record Fence refuses +to sign a URL for. `g3dt indexd check-download` walks the exact chain the +portal hits — Indexd record → storage URL → DRS object → access methods → +Fence signed URL — and reports PASS/FAIL per object, exiting non-zero if any +object fails so it can gate a deployment step. + +Run it before a release, and after registering new files. The env selects the +API key secret and the key's JWT selects the commons, so there is no URL to +pass (and none to get wrong). + +```bash +g3dt indexd check-download --env staging # sample the 25 newest +g3dt indexd check-download --env staging --limit 50 +g3dt indexd check-download --env prod PREFIX/ PREFIX/ +``` + +With no GUIDs, the newest objects for the env's commons are sampled from the +indexd registry (latest revision per baseid). The registry may live in a +different AWS account than the commons being checked; if the env's AWS +profile cannot reach it, pass GUIDs explicitly. + +Reading a failure: + +| Symptom | Meaning | +|---|---| +| `Indexd status: 404` | the object is not registered — a registration problem, not a download one | +| `urls: []` / no access methods | registered but with no storage location; it can never download | +| `Access endpoint … 401` | authorization: the API key's user lacks `read-storage` on the record's `authz` resource — an authz gap, not a broken key | +| `Access endpoint … 500` | Fence has the permission but failed to sign — a service-side fault | + +On a 401, compare what the record requires +(`https://commons.example.org/index/`, the `authz` field) with what the +key's user actually holds (`https://commons.example.org/user/user`): +downloads require `read-storage` on the record's authz resource, which a user +holding only `create` does not have. + ## Development ```bash diff --git a/src/g3dt/cli/indexd_cmds.py b/src/g3dt/cli/indexd_cmds.py index c0f9d4d..51b3009 100644 --- a/src/g3dt/cli/indexd_cmds.py +++ b/src/g3dt/cli/indexd_cmds.py @@ -1,20 +1,26 @@ -"""`g3dt indexd` — register S3 files with Gen3 indexd (data-plane). +"""`g3dt indexd` — register S3 files with Gen3 indexd, and verify they download. -Long-running, so it supports ``--on ec2``. +``register`` is a long data-plane op, so it supports ``--on ec2``. +``check-download`` is a read-only HTTP check that takes seconds and whose +whole value is the PASS/FAIL in your terminal, so it is local-only. """ from __future__ import annotations -from typing import List +from typing import List, Optional import typer -from g3dt.cli._internal import dispatch +from g3dt.cli._internal import dispatch, runner from g3dt.cli._internal.dispatch import Target -from g3dt.cli._internal.resolve import study_of +from g3dt.cli._internal.resolve import env_of, study_of -app = typer.Typer(no_args_is_help=True, help="Register files with Gen3 indexd.") +app = typer.Typer( + no_args_is_help=True, + help="Register files with Gen3 indexd and verify download access.", +) _REGISTER = "services/indexd/register_indexd.py" +_CHECK_DOWNLOAD = "services/indexd/verify_file_access.py" @app.command() @@ -67,3 +73,54 @@ def remote_cli(env_name): dispatch.run_or_dispatch( on, env, _REGISTER, build_args, "indexd-register", remote_cli=remote_cli, ) + + +@app.command(name="check-download") +def check_download( + guids: Optional[List[str]] = typer.Argument( + None, + help="Object GUIDs, e.g. PREFIX/. Omit to sample the most " + "recently registered objects from the indexd registry.", + ), + env: str = typer.Option(..., "--env", "-e", help="Environment, e.g. test."), + limit: int = typer.Option( + 25, "--limit", "-n", + help="How many objects to sample when no GUIDs are given.", + ), + key_path: Optional[str] = typer.Option( + None, "--key-path", + help="Break-glass: local Gen3 API key JSON file, instead of the " + "env's secret.", + ), +) -> None: + """Prove registered objects are downloadable end to end. + + Walks Indexd -> DRS -> Fence signed URL for each GUID and exits non-zero + if any object fails, so it can gate a deployment step. Read-only and + local-only (seconds, not a long job — there is nothing to dispatch to EC2). + + The env selects the API key secret and the key's JWT selects the commons, + so a staging env checks staging. There is no URL to pass. + + With no GUIDs, the newest --limit objects for this commons are sampled + from the indexd registry (latest revision per baseid). The registry may + live in a different AWS account than the commons — sampling needs an env + whose AWS profile can reach it; otherwise pass GUIDs explicitly. + + Examples: + g3dt indexd check-download --env staging # sample 25 newest + g3dt indexd check-download --env staging --limit 50 + g3dt indexd check-download --env prod PREFIX/aaa PREFIX/bbb + """ + # Validate the env before spawning a subprocess: an unknown env should + # fail here with the config error, not deep inside the script. + e = env_of(env) + + args: List[str] = ["--env", e.name] + if key_path: + args += ["--key-path", key_path] + if guids: + args += list(guids) + else: + args += ["--limit", str(limit)] + runner.run(runner.python_script(_CHECK_DOWNLOAD, *args)) diff --git a/src/g3dt/indexd/file_access.py b/src/g3dt/indexd/file_access.py new file mode 100644 index 0000000..941dece --- /dev/null +++ b/src/g3dt/indexd/file_access.py @@ -0,0 +1,274 @@ +"""Verify that registered file objects are actually downloadable. + +Walks the full chain a user hits when they click a file in the portal: + + Indexd record -> storage URLs -> DRS object -> access methods -> signed URL + +and reports PASS/FAIL per object. Two failure modes this catches that nothing +else does: + + - the Indexd record exists but has no URLs / DRS access methods, so the + object can never be downloaded; + - the object has a storage URL but Fence fails to sign a download URL + (e.g. HTTP 500 from the access endpoint). + +**The environment selects the credential, and the credential selects the +commons.** The caller supplies an env; its ``aws_secret_name`` yields the Gen3 +API key (Secrets Manager, or a local file for the documented absolute-path +convention), and the commons URL is derived from that key's JWT ``iss`` claim. +There is therefore no URL for an operator to get wrong. + +That matters for a concrete reason: :class:`gen3.auth.Gen3Auth` silently falls +back to the Workspace Token Service whenever an explicitly-passed ``endpoint`` +disagrees with a ``refresh_file``'s issuer (see ``gen3/auth.py``, the +``elif refresh_file:`` branch). WTS is not deployed on these commons (e.g. +``commons.example.org``), so that path fails with a misleading +``502 Bad Gateway`` on ``/wts/external_oidc/``. Constructing +``Gen3Auth(refresh_token=)`` with no ``endpoint`` never enters that +branch, which makes the mismatch structurally impossible rather than merely +guarded. +""" +from __future__ import annotations + +import json +from typing import Any, Callable, List, Optional, Tuple + +import requests +from gen3.auth import Gen3Auth + +from g3dt import config as g3dt_config +from g3dt import resolver +from g3dt.upload.metadata_submitter import ( + commons_url_from_jwt, + create_boto3_session, + get_gen3_api_key_aws_secret, +) + + +def registry_sample_sql( + database: str, table: str, indexd_endpoint: str, limit: int +) -> str: + """Build the SQL that samples the most recently registered objects. + + Latest revision per ``baseid`` only (re-registering a file creates a new + ``did`` under the same ``baseid``; older revisions are superseded and + would fail a download check spuriously), filtered to one commons via + ``indexd_endpoint`` — that filter is what keeps another environment's + registrations out of the sample when environments share a registry table. + """ + return f""" + SELECT did FROM ( + SELECT did, registered_at, + ROW_NUMBER() OVER ( + PARTITION BY baseid + ORDER BY registered_at DESC + ) AS row_num + FROM "{database}"."{table}" + WHERE indexd_endpoint = '{indexd_endpoint}' + ) + WHERE row_num = 1 + ORDER BY registered_at DESC + LIMIT {int(limit)} + """ + + +def sample_recent_guids(env_cfg, commons_url: str, limit: int) -> List[str]: + """Return the ``limit`` most recently registered GUIDs for a commons. + + Queries the env's indexd registry table (Athena) with the env's AWS + profile. Every name comes from the resolver: the registry is the + conventional ``indexd_registry`` table in the env's metadata Glue DB, and + the Athena output location / workgroup are the env's own. + + The registry may live in a different AWS account than the commons being + checked, so the env must be one whose ``aws_profile`` can reach it — + otherwise pass explicit GUIDs and skip sampling entirely. + + The returned ``did`` values already include their prefix, so they feed + straight into :func:`verify_objects`. + """ + from g3dt.utils.athena_utils import AthenaConfig, AthenaQuery + + rc = resolver.resolve( + g3dt_config.require_project(), + g3dt_config.env_base(env_cfg.name), + profile=env_cfg.aws_profile, + ) + database = rc.metadata_db + table = g3dt_config.INDEXD_REGISTRY_TABLE + + sql = registry_sample_sql( + database, table, f"{commons_url}/index/index", limit + ) + query = AthenaQuery( + AthenaConfig( + aws_region=env_cfg.region, + aws_profile=env_cfg.aws_profile, + athena_s3_output=rc.athena_output_location, + workgroup=rc.athena_workgroup, + ) + ) + try: + df = query.query_athena(sql, database, ctas_approach=False) + except Exception as exc: + raise RuntimeError( + f"could not query {database}.{table} with profile " + f"'{env_cfg.aws_profile}': {exc}. The indexd registry may live " + "in a different AWS account than the commons — use an env whose " + "AWS profile can reach the registry, or pass explicit GUIDs to " + "check-download instead." + ) from exc + return df["did"].dropna().tolist() + + +def api_key_for_env(env_cfg, key_path: Optional[str] = None) -> dict: + """Load the Gen3 API key an environment authenticates with. + + Precedence: + 1. ``key_path`` — an explicit local key file (break-glass override); + 2. the env's ``aws_secret_name`` as an absolute path (local file — the + documented path-style convention, as in ``register_indexd.py``); + 3. the env's ``aws_secret_name`` as a Secrets Manager secret. + + Args: + env_cfg: Resolved :class:`~g3dt.config.EnvConfig`. + key_path: Optional path to a local Gen3 API key JSON file. + + Returns: + dict: The parsed API key, e.g. ``{"api_key": "", ...}``. + """ + if key_path: + with open(key_path, "r") as handle: + return json.load(handle) + + secret_name = env_cfg.aws_secret_name + if secret_name.startswith("/"): + with open(secret_name, "r") as handle: + return json.load(handle) + + session = create_boto3_session( + aws_profile=env_cfg.aws_profile, + aws_region=env_cfg.region, + ) + return get_gen3_api_key_aws_secret( + secret_name=secret_name, + region_name=env_cfg.region, + session=session, + ) + + +def commons_auth(api_key: dict) -> Tuple[str, Gen3Auth]: + """Return the commons URL and an authenticated client for an API key. + + Both are derived from the same token, so they cannot disagree. Note the + deliberate absence of an ``endpoint`` argument to ``Gen3Auth`` — passing + one alongside a credential is what triggers the Workspace Token Service + fallback described in the module docstring. + + Returns: + tuple: ``(commons_url, Gen3Auth)``. + """ + return commons_url_from_jwt(api_key["api_key"]), Gen3Auth(refresh_token=api_key) + + +def get_json( + commons_url: str, auth: Gen3Auth, path: str +) -> Tuple[requests.Response, Any]: + """GET a commons path, returning the response and its parsed body.""" + response = requests.get(f"{commons_url}{path}", auth=auth, timeout=30) + try: + body = response.json() + except ValueError: + body = response.text + return response, body + + +def verify_object( + commons_url: str, + auth: Gen3Auth, + object_id: str, + emit: Callable[[str], None] = print, +) -> bool: + """Return True if one object is fully downloadable, else explain why not. + + Args: + commons_url: Base URL of the commons to check. + auth: Authenticated Gen3 client. + object_id: The object GUID, e.g. ``PREFIX/``. + emit: Where to write progress (defaults to ``print``). + """ + emit(f"\n{'=' * 80}") + emit(f"Checking: {object_id}") + + index_response, index_record = get_json(commons_url, auth, f"/index/{object_id}") + emit(f"Indexd status: {index_response.status_code}") + if not index_response.ok: + emit(str(index_record)) + emit("FAIL: no Indexd record.") + return False + + urls = index_record.get("urls", []) + emit(f"Indexd DID: {index_record.get('did')}") + emit(f"Indexd URLs: {json.dumps(urls, indent=2)}") + + drs_response, drs_object = get_json( + commons_url, auth, f"/ga4gh/drs/v1/objects/{object_id}" + ) + emit(f"DRS object status: {drs_response.status_code}") + if not drs_response.ok: + emit(str(drs_object)) + emit("FAIL: no DRS object.") + return False + + access_methods = drs_object.get("access_methods", []) + emit(f"DRS access methods: {json.dumps(access_methods, indent=2)}") + + if not urls or not access_methods: + emit( + "FAIL: the Indexd record has no usable storage location, " + "so the object cannot be downloaded." + ) + return False + + ok = True + for method in access_methods: + access_id = method.get("access_id") + if not access_id: + continue + + access_response, access_body = get_json( + commons_url, auth, f"/ga4gh/drs/v1/objects/{object_id}/access/{access_id}" + ) + emit(f"Access endpoint ({access_id}) status: {access_response.status_code}") + emit( + "Access response: " + + ( + json.dumps(access_body, indent=2) + if isinstance(access_body, (dict, list)) + else str(access_body) + ) + ) + if not access_response.ok: + emit( + "FAIL: the object has a storage URL, but the server " + "failed to generate a usable download URL." + ) + ok = False + + if ok: + emit("PASS: object is downloadable.") + return ok + + +def verify_objects( + commons_url: str, + auth: Gen3Auth, + object_ids: List[str], + emit: Callable[[str], None] = print, +) -> List[str]: + """Verify several objects; return the GUIDs that failed (empty if all pass).""" + return [ + object_id + for object_id in object_ids + if not verify_object(commons_url, auth, object_id, emit=emit) + ] diff --git a/src/g3dt/services/indexd/verify_file_access.py b/src/g3dt/services/indexd/verify_file_access.py new file mode 100644 index 0000000..8541f5a --- /dev/null +++ b/src/g3dt/services/indexd/verify_file_access.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 +"""Verify that registered file objects are actually downloadable. + +Walks the chain a portal user hits — Indexd record -> storage URLs -> DRS +object -> access methods -> Fence signed URL — and reports PASS/FAIL per +object. Exits non-zero if any object fails, so it can gate a deployment step. + +The environment selects the credential and the credential selects the commons: +``--env`` resolves the env's ``aws_secret_name`` (AWS Secrets Manager, or a +local file when the value is an absolute path), and the commons URL comes from +that key's JWT. There is no URL to pass and therefore none to get wrong — see +``src/g3dt/indexd/file_access.py`` for why that matters. + +GUIDs are optional: with none given, the script samples the most recently +registered objects for this commons from the indexd registry (Athena) — the +latest revision per baseid, newest first, ``--limit`` of them. The registry +may live in a different AWS account than the commons, so sampling needs an +env whose AWS profile can reach it; explicit GUIDs skip Athena entirely. + +Prefer the CLI wrapper, which resolves the env the same way: + + g3dt indexd check-download --env staging # auto-sample + g3dt indexd check-download --env staging PREFIX/ + +Direct usage: + + poetry run python src/g3dt/services/indexd/verify_file_access.py \ + --env staging PREFIX/005d97ab-... PREFIX/ffff19f8-... +""" + +import argparse +import sys + +from g3dt import config as g3dt_config +from g3dt.indexd.file_access import ( + api_key_for_env, + commons_auth, + sample_recent_guids, + verify_objects, +) + + +def parse_args(): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "guids", + nargs="*", + help="object GUIDs, e.g. PREFIX/uuid; omit to sample the registry", + ) + parser.add_argument( + "--limit", + type=int, + default=25, + help="how many recently registered objects to sample when no GUIDs " + "are given (default: 25)", + ) + parser.add_argument( + "--env", + required=True, + help="environment, e.g. staging (selects the API key secret)", + ) + parser.add_argument( + "--key-path", + default=None, + help="break-glass: local Gen3 API key JSON file, instead of the env's secret", + ) + return parser.parse_args() + + +def main() -> int: + args = parse_args() + + try: + env_cfg = g3dt_config.resolve_env(args.env) + except g3dt_config.ConfigError as exc: + print(f"ERROR: {exc}", file=sys.stderr) + return 2 + + try: + api_key = api_key_for_env(env_cfg, args.key_path) + except Exception as exc: # boto/OSError/JSON — all mean "no usable key" + print( + f"ERROR: could not load the Gen3 API key for env '{args.env}': {exc}", + file=sys.stderr, + ) + return 2 + + commons, auth = commons_auth(api_key) + # Provenance: an operator can eyeball exactly what was checked, and with + # which credential, before trusting a PASS. + print( + f"Env: {env_cfg.name} | Secret: {args.key_path or env_cfg.aws_secret_name} " + f"| Commons: {commons}" + ) + + guids = args.guids + if not guids: + print( + f"No GUIDs given — sampling the {args.limit} most recently " + f"registered object(s) for {commons} from the indexd registry." + ) + try: + guids = sample_recent_guids(env_cfg, commons, args.limit) + except RuntimeError as exc: + print(f"ERROR: {exc}", file=sys.stderr) + return 2 + if not guids: + print( + f"ERROR: the registry has no objects for {commons}/index/index" + " — nothing to check. Register files first, or pass GUIDs.", + file=sys.stderr, + ) + return 2 + print(f"Sampled: {', '.join(guids)}") + + failures = verify_objects(commons, auth, guids) + + print(f"\n{'=' * 80}") + print(f"Checked {len(guids)} object(s): {len(failures)} failure(s).") + for guid in failures: + print(f" FAILED: {guid}") + return 1 if failures else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_cli_commands.py b/tests/test_cli_commands.py index 688a0be..96fced7 100644 --- a/tests/test_cli_commands.py +++ b/tests/test_cli_commands.py @@ -13,7 +13,7 @@ from typer.testing import CliRunner from g3dt.cli.main import app -from g3dt.config import EnvConfig, StudyConfig, dictionary_url +from g3dt.config import ConfigError, EnvConfig, StudyConfig, dictionary_url runner = CliRunner() @@ -378,3 +378,74 @@ def test_k8s_restart_schema_passes_env_argo_args(mock_run, _env): assert argv[0] == "bash" assert argv[1].endswith("services/k8s_ops/argocd_restart_schema.sh") assert "-d" in argv and "-a" in argv and "-n" in argv + + +@patch("g3dt.config.resolve_env", side_effect=ConfigError("No SSM parameters found under /etl/nope")) +@patch("g3dt.cli._internal.runner.run") +def test_indexd_check_download_validates_env_before_spawning(mock_run, _resolve): + """ + Background: + check-download resolves the env locally (env_of) before it spawns the + worker script, so an unknown env fails right here with the config + error — not seconds later, deep inside a subprocess whose traceback + buries the real problem. + + Inputs: g3dt indexd check-download --env nope, where env resolution + raises ConfigError + Expected Output: a clean non-zero exit carrying the config message; the + worker subprocess is never invoked. + """ + result = runner.invoke(app, ["indexd", "check-download", "--env", "nope"]) + assert result.exit_code == 1 + mock_run.assert_not_called() + assert "No SSM parameters found" in result.output + + +@patch("g3dt.cli.indexd_cmds.env_of", side_effect=_env_cfg) +@patch("g3dt.cli._internal.runner.run") +def test_indexd_check_download_forwards_guids_in_order(mock_run, _env): + """ + Background: + Explicit GUIDs must bypass registry sampling entirely (they need no + Athena access — e.g. spot-checking an object that just failed in the + portal), so --limit is NOT forwarded alongside them. + + Inputs: g3dt indexd check-download --env staging PREFIX/aaa PREFIX/bbb + Expected Output: + - exit code 0 + - runs verify_file_access.py via the current interpreter with the env + and both GUIDs as trailing positionals, in the order given, and no + --limit flag + """ + result = runner.invoke( + app, + ["indexd", "check-download", "--env", "staging", + "PREFIX/aaa", "PREFIX/bbb"], + ) + assert result.exit_code == 0, result.output + argv = _argv(mock_run) + assert argv[0] == sys.executable + assert argv[1].endswith("services/indexd/verify_file_access.py") + assert argv[2:] == ["--env", "staging", "PREFIX/aaa", "PREFIX/bbb"] + assert "--limit" not in argv + + +@patch("g3dt.cli.indexd_cmds.env_of", side_effect=_env_cfg) +@patch("g3dt.cli._internal.runner.run") +def test_indexd_check_download_samples_registry_with_limit(mock_run, _env): + """ + Background: + With no GUIDs given, the wrapped script samples the newest registered + objects for the env's commons from the indexd registry, so the + default pre-flight is a single command with no copy-paste. The CLI's + --limit controls the sample size (default 25). + + Inputs: g3dt indexd check-download --env staging --limit 5 (no GUIDs) + Expected Output: --limit 5 forwarded and NO trailing GUID positionals, + which tells the script to sample the registry itself. + """ + result = runner.invoke( + app, ["indexd", "check-download", "--env", "staging", "--limit", "5"] + ) + assert result.exit_code == 0, result.output + assert _argv(mock_run)[2:] == ["--env", "staging", "--limit", "5"] diff --git a/tests/test_file_access.py b/tests/test_file_access.py new file mode 100644 index 0000000..8e786fa --- /dev/null +++ b/tests/test_file_access.py @@ -0,0 +1,440 @@ +"""Tests for the indexd file-access verification logic. + +These cover how the Gen3 credential and the commons URL are resolved (the part +that caused a real incident — see test_commons_auth_never_passes_endpoint), and +the four outcomes of walking the download chain. All AWS and HTTP calls are +mocked; nothing touches a real commons. +""" +import base64 +import json +from unittest.mock import MagicMock, patch + +import pytest + +from g3dt.config import EnvConfig +from g3dt.indexd.file_access import ( + api_key_for_env, + commons_auth, + verify_object, + verify_objects, +) +from g3dt.resolver import ResolvedConfig + +MODULE = "g3dt.indexd.file_access" + + +def _b64(obj): + return base64.urlsafe_b64encode(json.dumps(obj).encode()).decode().rstrip("=") + + +def _fake_jwt(iss): + """Build a structurally valid (unsigned) JWT carrying an `iss` claim.""" + header = _b64({"alg": "RS256", "typ": "JWT"}) + sig = base64.urlsafe_b64encode(b"sig").decode().rstrip("=") + return f"{header}.{_b64({'iss': iss})}.{sig}" + + +STAGING_KEY = {"api_key": _fake_jwt("https://staging.commons.example.org/user")} + + +def _env_cfg(secret_name="gen3_api_key_staging"): + return EnvConfig( + name="staging", + is_ec2=False, + region="ap-southeast-2", + dictionary_version="v1", + aws_profile="etl_staging", + aws_secret_name=secret_name, + schema_s3_uri="u", + domain="d", + app_name="a", + namespace="n", + cluster_name="c", + schema_repo="Org/schema-repo", + ) + + +def _response(status, body): + """A stand-in for a requests.Response with a JSON body.""" + resp = MagicMock() + resp.status_code = status + resp.ok = 200 <= status < 300 + resp.json.return_value = body + return resp + + +# --------------------------------------------------------------------------- +# Credential resolution +# --------------------------------------------------------------------------- + +@patch(f"{MODULE}.get_gen3_api_key_aws_secret") +@patch(f"{MODULE}.create_boto3_session") +def test_api_key_for_env_fetches_the_envs_secret(mock_session, mock_load): + """ + Test that the environment's configured secret is what gets loaded. + + Background: + The CLI passes only an env; everything else must be derived from the + env's config so a staging run can never authenticate with a prod key. + + Inputs: an EnvConfig whose aws_secret_name is 'gen3_api_key_staging' + Expected Output: + - a boto3 session built with the env's profile and region + - get_gen3_api_key_aws_secret called with that secret name and region + - the returned key dict passed straight back to the caller + """ + mock_load.return_value = STAGING_KEY + + result = api_key_for_env(_env_cfg()) + + assert result is STAGING_KEY + mock_session.assert_called_once_with( + aws_profile="etl_staging", aws_region="ap-southeast-2" + ) + assert mock_load.call_args.kwargs["secret_name"] == "gen3_api_key_staging" + assert mock_load.call_args.kwargs["region_name"] == "ap-southeast-2" + + +@patch(f"{MODULE}.get_gen3_api_key_aws_secret") +@patch(f"{MODULE}.create_boto3_session") +def test_api_key_for_env_prefers_an_explicit_key_file(mock_session, mock_load, tmp_path): + """ + Test the break-glass local key override. + + Background: + An operator debugging a broken secret needs a way to test with a key + downloaded from the portal. When that path is given, no AWS call should + happen at all. + + Inputs: key_path pointing at a local JSON key file + Expected Output: the file's contents are returned; Secrets Manager and the + boto3 session are never touched. + """ + key_file = tmp_path / "key.json" + key_file.write_text(json.dumps(STAGING_KEY)) + + result = api_key_for_env(_env_cfg(), key_path=str(key_file)) + + assert result == STAGING_KEY + mock_session.assert_not_called() + mock_load.assert_not_called() + + +@patch(f"{MODULE}.get_gen3_api_key_aws_secret") +@patch(f"{MODULE}.create_boto3_session") +def test_api_key_for_env_reads_path_style_secret_name_as_local_file( + mock_session, mock_load, tmp_path +): + """ + Test the documented absolute-path convention for aws_secret_name. + + Background: + An env may declare its aws_secret_name as an absolute path — the + dual-mode convention register_indexd.py already honours: a path-style + value is a local Gen3 API key file, anything else is a Secrets + Manager secret name. check-download must follow the same rule so an + env that registers files can always verify them. + + Inputs: an EnvConfig whose aws_secret_name is '/abs/path/key.json' + Expected Output: the file's contents are returned; no AWS call happens. + """ + key_file = tmp_path / "key.json" + key_file.write_text(json.dumps(STAGING_KEY)) + + result = api_key_for_env(_env_cfg(secret_name=str(key_file))) + + assert result == STAGING_KEY + mock_session.assert_not_called() + mock_load.assert_not_called() + + +# --------------------------------------------------------------------------- +# Commons resolution — the incident this feature exists to prevent +# --------------------------------------------------------------------------- + +@patch(f"{MODULE}.Gen3Auth") +def test_commons_auth_derives_url_from_the_key(mock_gen3auth): + """ + Test that the commons URL comes from the API key's JWT issuer. + + Background: + The key IS the environment selector. A staging key must produce the + staging commons without anyone typing a URL. + + Inputs: an API key whose iss is 'https://staging.commons.example.org/user' + Expected Output: commons == 'https://staging.commons.example.org' + """ + commons, _auth = commons_auth(STAGING_KEY) + assert commons == "https://staging.commons.example.org" + + +@patch(f"{MODULE}.Gen3Auth") +def test_commons_auth_never_passes_endpoint(mock_gen3auth): + """ + Test that Gen3Auth is constructed with the key alone — no endpoint. + + Background: + This is the whole reason the command resolves credentials this way. + gen3/auth.py switches to the Workspace Token Service whenever an + explicit `endpoint` disagrees with a credential's issuer. WTS is not + deployed on these commons, so that path fails with a misleading + '502 Bad Gateway' on /wts/external_oidc/ — which reads like broken + object_id links but is purely a client-side mismatch. Passing + refresh_token and NO endpoint never enters that branch, making the + mismatch structurally impossible instead of merely guarded. + + Inputs: a staging API key + Expected Output: Gen3Auth called once with refresh_token= only; no + 'endpoint' and no 'refresh_file' keyword anywhere in the call. + """ + commons_auth(STAGING_KEY) + + mock_gen3auth.assert_called_once_with(refresh_token=STAGING_KEY) + assert "endpoint" not in mock_gen3auth.call_args.kwargs + assert "refresh_file" not in mock_gen3auth.call_args.kwargs + + +# --------------------------------------------------------------------------- +# The download chain +# --------------------------------------------------------------------------- + +@patch(f"{MODULE}.requests.get") +def test_verify_object_passes_when_the_whole_chain_resolves(mock_get): + """ + Test the happy path: Indexd -> DRS -> signed URL all succeed. + + Inputs: an Indexd record with a storage URL, a DRS object with one access + method, and an access endpoint that returns a signed URL + Expected Output: True, and the final line emitted is the PASS message. + """ + mock_get.side_effect = [ + _response(200, {"did": "PREFIX/aaa", "urls": ["s3://bucket/f.csv"]}), + _response(200, {"access_methods": [{"access_id": "s3"}]}), + _response(200, {"url": "https://signed"}), + ] + lines = [] + + assert verify_object("https://commons", MagicMock(), "PREFIX/aaa", emit=lines.append) + assert lines[-1] == "PASS: object is downloadable." + + +@patch(f"{MODULE}.requests.get") +def test_verify_object_fails_when_there_is_no_indexd_record(mock_get): + """ + Test the unregistered-object case. + + Inputs: Indexd returns 404 + Expected Output: False, and no further calls are made (no point asking DRS + about an object that does not exist). + """ + mock_get.side_effect = [_response(404, {"error": "no record"})] + lines = [] + + assert not verify_object("https://commons", MagicMock(), "PREFIX/aaa", emit=lines.append) + assert mock_get.call_count == 1 + assert "FAIL: no Indexd record." in lines + + +@patch(f"{MODULE}.requests.get") +def test_verify_object_fails_when_the_record_has_no_storage_location(mock_get): + """ + Test the 'registered but undownloadable' failure mode. + + Background: + An Indexd record can exist with an empty `urls` list — the metadata + looks fine in the portal, but there is nothing to download. This is one + of the two real failure modes this checker exists to catch. + + Inputs: Indexd 200 with urls=[]; DRS 200 with access_methods=[] + Expected Output: False, with the 'no usable storage location' explanation. + """ + mock_get.side_effect = [ + _response(200, {"did": "PREFIX/aaa", "urls": []}), + _response(200, {"access_methods": []}), + ] + lines = [] + + assert not verify_object("https://commons", MagicMock(), "PREFIX/aaa", emit=lines.append) + assert any("no usable storage location" in line for line in lines) + + +@patch(f"{MODULE}.requests.get") +def test_verify_object_fails_when_fence_cannot_sign(mock_get): + """ + Test the 'storage URL exists but Fence errors' failure mode. + + Background: + The second real failure mode: the record and DRS object are healthy, + but the access endpoint 500s, so a user still cannot download the file. + + Inputs: a healthy record and DRS object, access endpoint returns 500 + Expected Output: False, with the 'failed to generate a usable download URL' + explanation. + """ + mock_get.side_effect = [ + _response(200, {"did": "PREFIX/aaa", "urls": ["s3://bucket/f.csv"]}), + _response(200, {"access_methods": [{"access_id": "s3"}]}), + _response(500, {"error": "service failure"}), + ] + lines = [] + + assert not verify_object("https://commons", MagicMock(), "PREFIX/aaa", emit=lines.append) + assert any("failed to generate a usable download URL" in line for line in lines) + + +@patch(f"{MODULE}.verify_object") +def test_verify_objects_returns_only_the_failed_guids(mock_verify): + """ + Test that the caller can derive an exit code from the result. + + Background: + The script exits non-zero when any object fails, so this command can + gate a deployment step. That depends on failures being reported + precisely rather than as a bare boolean. + + Inputs: three GUIDs where the middle one fails + Expected Output: ['PREFIX/bbb'] — only the failure, order preserved. + """ + mock_verify.side_effect = [True, False, True] + + failures = verify_objects( + "https://commons", MagicMock(), ["PREFIX/aaa", "PREFIX/bbb", "PREFIX/ccc"] + ) + + assert failures == ["PREFIX/bbb"] + + +# --------------------------------------------------------------------------- +# Registry sampling (auto-extracted GUIDs for check-download) +# --------------------------------------------------------------------------- + +def _rc(): + """The slice of the env's SSM tree that sampling reads (resolver names).""" + return ResolvedConfig( + project="etl", + env="staging", + params={ + "glue/db/metadata": "etl_staging_metadata_db", + "athena/outputLocation": "s3://etl-staging-metadata/athena-query-results/", + "athena/workgroup": "etl_staging_wg", + }, + ) + + +def test_registry_sample_sql_scopes_to_one_commons_and_latest_revision(): + """ + Test that the sampling SQL cannot leak objects from another environment + or return superseded revisions. + + Background: + Environments can write to a shared registry table, distinguished + only by the indexd_endpoint column — without that filter a "prod" + check would happily sample staging objects and prove nothing. And + re-registering a file creates a new did under the same baseid, so + without the latest-revision window function the sample could contain + old dids that legitimately no longer download. + + Inputs: database, table, a prod indexd endpoint, limit 10 + Expected Output: SQL containing the endpoint equality filter, the + ROW_NUMBER window partitioned by baseid, and LIMIT 10. + """ + from g3dt.indexd.file_access import registry_sample_sql + + sql = registry_sample_sql( + "etl_staging_metadata_db", + "indexd_registry", + "https://commons.example.org/index/index", + 10, + ) + + assert "indexd_endpoint = 'https://commons.example.org/index/index'" in sql + assert "PARTITION BY baseid" in sql + assert "row_num = 1" in sql + assert "LIMIT 10" in sql + + +@patch("g3dt.utils.athena_utils.AthenaQuery") +@patch(f"{MODULE}.resolver.resolve") +@patch(f"{MODULE}.g3dt_config.require_project", return_value="etl") +def test_sample_recent_guids_returns_dids_for_the_derived_endpoint( + _project, mock_resolve, mock_query_cls +): + """ + Test that sampling queries the registry with the env's own AWS session + and resolver-provided names, and returns plain did strings ready for + verify_objects. + + Background: + The dids in the registry already include the "PREFIX/" prefix, so + the sample must be forwarded untouched — historically operators + double-prefixed GUIDs pasted from Athena and got spurious 404s. The + registry database, Athena output location and workgroup all come + from SSM via the resolver — nothing is hard-coded. + + Inputs: a resolver returning the env's metadata DB and Athena names, a + staging env, commons https://staging.commons.example.org, limit 2 + Expected Output: the two dids from the query result, in order; the SQL + passed to Athena filters on the commons URL + /index/index; the resolver + is asked for this project/env with the env's profile. + """ + import pandas as pd + + from g3dt.indexd.file_access import sample_recent_guids + + mock_resolve.return_value = _rc() + mock_query_cls.return_value.query_athena.return_value = pd.DataFrame( + {"did": ["PREFIX/aaa", "PREFIX/bbb"]} + ) + + guids = sample_recent_guids( + _env_cfg(), "https://staging.commons.example.org", 2 + ) + + assert guids == ["PREFIX/aaa", "PREFIX/bbb"] + sql = mock_query_cls.return_value.query_athena.call_args[0][0] + assert "https://staging.commons.example.org/index/index" in sql + assert '"etl_staging_metadata_db"."indexd_registry"' in sql + mock_resolve.assert_called_once_with("etl", "staging", profile="etl_staging") + athena_cfg = mock_query_cls.call_args[0][0] + assert athena_cfg.athena_s3_output == ( + "s3://etl-staging-metadata/athena-query-results/" + ) + assert athena_cfg.workgroup == "etl_staging_wg" + + +@patch("g3dt.utils.athena_utils.AthenaQuery") +@patch(f"{MODULE}.resolver.resolve") +@patch(f"{MODULE}.g3dt_config.require_project", return_value="etl") +def test_sample_recent_guids_explains_cross_account_athena_failures( + _project, mock_resolve, mock_query_cls +): + """ + Test that an Athena failure surfaces as advice, not a raw stack trace. + + Background: + The registry table may live in a different AWS account than the + commons being checked (e.g. a prod commons whose registry sits in a + shared staging account). Running check-download with a profile that + authenticates against the wrong account is the most likely sampling + failure in practice. The error must tell the operator the two ways + out: use an env whose profile can reach the registry, or pass + explicit GUIDs and skip sampling. + + Inputs: AthenaQuery.query_athena raising an exception + Expected Output: RuntimeError explaining the cross-account possibility + and naming explicit GUIDs as the fallback. + """ + from g3dt.indexd.file_access import sample_recent_guids + + mock_resolve.return_value = _rc() + mock_query_cls.return_value.query_athena.side_effect = Exception( + "EntityNotFoundException: Database etl_staging_metadata_db not found" + ) + + with pytest.raises(RuntimeError) as excinfo: + sample_recent_guids( + _env_cfg(), "https://staging.commons.example.org", 10 + ) + + assert "different AWS account" in str(excinfo.value) + assert "explicit GUIDs" in str(excinfo.value)