From c5c994daee90b289bc05dff56840a67c38641021 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 09:48:37 -0300 Subject: [PATCH 01/24] docs(spec): identity-only blob access Record the design and ADR for removing the storage account key from app settings and minting blob-scoped, short-lived read tokens signed by the app identity. --- .../0008-identity-only-blob-access.md | 43 ++++++++++++ .../identity-only-blob-access/README.md | 64 ++++++++++++++++++ .../identity-only-blob-access/design.md | 66 +++++++++++++++++++ 3 files changed, 173 insertions(+) create mode 100644 spec/architecture/decisions/0008-identity-only-blob-access.md create mode 100644 spec/features/identity-only-blob-access/README.md create mode 100644 spec/features/identity-only-blob-access/design.md diff --git a/spec/architecture/decisions/0008-identity-only-blob-access.md b/spec/architecture/decisions/0008-identity-only-blob-access.md new file mode 100644 index 00000000..842db6d1 --- /dev/null +++ b/spec/architecture/decisions/0008-identity-only-blob-access.md @@ -0,0 +1,43 @@ +# ADR-0008: Identity-Only Blob Access + +**Status:** proposed +**Date:** 2026-10-27 +**Deciders:** HASTE maintainers + +## Context + +The API and queue apps receive the storage account key through +`BLOB_CONNECTION_STRING`. When it is set, read links are container-scoped SAS +tokens backed by a 90-day stored access policy. Container scope means any link +reads every project's data. A user-delegation SAS cannot use a stored policy, so +identity-based signing cannot reuse this design. + +## Options Considered + +### Keep the key and the stored policy + +- No code change. +- Leaves a long-lived key in app settings and container-wide read links. +- Keeps shared-key access enabled on the data account. + +### Blob-scoped, short-lived, identity-signed tokens + +- Each link reads one blob for at most an hour and is signed by the app identity. +- Revocation comes from the delegation key, so there is no policy to maintain. +- Persisted tokens no longer outlive their usefulness; browsers already go + through the identity-authenticated storage proxy. + +## Decision + +Mint read tokens with a single helper that produces blob-scoped, read-only, +60-minute user-delegation SAS tokens. Stop creating the stored policy. Remove +`BLOB_CONNECTION_STRING` from deployed apps and keep honoring it for the local +emulator. Disable shared-key access on the data account afterwards. + +## Consequences + +- Links minted before the change keep working until their policy is deleted. +- Tokens are shorter-lived, so anything that stores a token for later use must + read by path or go through the proxy. +- Deployment config no longer needs the account key. +- The Azure Files mount keeps its key; this decision does not cover it. diff --git a/spec/features/identity-only-blob-access/README.md b/spec/features/identity-only-blob-access/README.md new file mode 100644 index 00000000..d42931b8 --- /dev/null +++ b/spec/features/identity-only-blob-access/README.md @@ -0,0 +1,64 @@ +# Feature: Identity-Only Blob Access + +**Status:** in-review +**Author:** HASTE engineering team +**Date:** 2026-10-27 +**Target Release:** next +**Priority:** P1 +**Work Item:** — + +## Summary + +The API and queue apps carry the storage account key in an app setting +(`BLOB_CONNECTION_STRING`). Its presence also selects the shared-key code path, +which mints container-wide read tokens backed by a 90-day stored access policy. +This feature removes the key from app settings, signs read tokens with the app's +managed identity, and scopes each token to one blob with a short expiry. Once no +caller depends on the key, shared-key access can be disabled on the data +storage account. + +## Motivation + +- A container-scoped token appended to one blob's URL is worth more than the URL + suggests. Data for every project lives in one container separated only by a + path prefix, so container-wide read is service-wide read. +- A user-delegation SAS cannot reference a stored access policy, so the policy + path cannot work once the key is gone. +- The key in app settings is a long-lived credential that can be read by anyone + with access to the app configuration. +- Every other storage consumer already falls back to the managed identity when + the connection string is empty. Two byte-range helpers in `utils/blob.py` do + not. + +## Success Criteria + +- [ ] API and queue apps run without `BLOB_CONNECTION_STRING` in Azure. +- [ ] Read tokens are blob-scoped (`sr=b`), read-only, expire within an hour and + reference no stored access policy. +- [ ] Tokens minted in Azure are user-delegation SAS (signed by the identity). +- [ ] The container read policy is no longer created or maintained. +- [ ] The docker dev stack, which points `BLOB_CONNECTION_STRING` at the local + emulator, keeps working unchanged. +- [ ] Browser downloads and published datasets keep working in dev1 with the + setting removed. +- [ ] Shared-key access can then be disabled on the data storage account + (separate, gated change). + +## Out of Scope + +- The Azure Files account backing the Functions `/data` mount, which needs a key. +- Batch account key and communication-service connection string. +- The Batch auto-storage account. + +## HASTE Components Affected + +| Component | Impact | +|---|---| +| `hastelib/.../utils/blob_sas.py` | New helper that mints blob-scoped read tokens | +| `hastelib/.../utils/blob.py` | Byte-range and download helpers fall back to the identity | +| `hastelib/.../data_layer/azure_blob_storage_data_layer.py` | Uses the helper; stops creating the container policy | +| `hastelib/.../artifact_storage/azure_blob_artifact_storage.py` | Same | +| `infra/modules/functions.bicep`, `.github/scripts/deploy_apps.sh` | Stop setting `BLOB_CONNECTION_STRING` | + +Related: [design.md](design.md), +[ADR-0008](../../architecture/decisions/0008-identity-only-blob-access.md). diff --git a/spec/features/identity-only-blob-access/design.md b/spec/features/identity-only-blob-access/design.md new file mode 100644 index 00000000..c88df63a --- /dev/null +++ b/spec/features/identity-only-blob-access/design.md @@ -0,0 +1,66 @@ +# Design: Identity-Only Blob Access + +## Current behavior + +| Path | With connection string | Without it | +|---|---|---| +| Queue client, blob data layer, artifact store, publishing lease | Account key | `DefaultAzureCredential` + account URL | +| `download_blob_to_tempfile`, `read_blob_range` | Account key | **Fails** (`A connection string or account URL is required`) | +| Read links (`get_file_remote_path`, `get_download_url`) | Container SAS on a 90-day stored policy, signed with the key | Container SAS naming the same policy, signed with a delegation key (rejected by the service) | + +## Target behavior + +### Credential selection + +`utils/blob.py` selects the client in one place: the connection string when set +(the local emulator), otherwise `BLOB_ACCOUNT_URL` with the managed identity. If +neither is set it raises a `ValueError` naming both settings. + +### Read tokens + +One helper, `blob_read_sas`, builds every link minted for clients: + +- scope: the single blob (`sr=b`), read-only; +- expiry: 60 minutes, start backdated 5 minutes for clock skew; +- signing: a user-delegation key when running against Azure; the account key + only for the local emulator; +- no stored access policy. + +The data layer and the artifact store both call it so they cannot drift. + +### Stored policy + +The container read policy (`image-r-policy`) is no longer created or refreshed. +Re-creating it on startup would undo any operational deletion, and deletion is +the way tokens signed under it are revoked. + +### Persisted URLs + +Records persist URLs that carry a token (`labelsUrl`, `embeddingsGeoJSONUrl`, +`featuresSidecarUrl`). Browsers never use that token against storage: the UI +rewrites the host to the managed-identity storage proxy, and the proxy +authenticates with its own identity. Functions and Batch read by blob path and +ignore the token. The persisted token therefore only matters in the local docker +stack, where the browser reads the emulator directly. + +## Risks + +| Risk | Mitigation | +|---|---| +| A caller still needs the key | Tests per path; dev1 run with the setting removed before any other environment | +| Existing 90-day links stop working once the policy is deleted | Deleted deliberately and only after dev1 validation; clients do not use the token in Azure | +| Local stack links expire after the token lifetime | Emulator tokens use the shared key and a longer lifetime | + +## Rollout + +1. Merge code; the setting is still present, so behavior is unchanged except the + token shape. +2. Remove `BLOB_CONNECTION_STRING` from the deployment config; deploy to dev1; + exercise upload, imagery, labeling, training, inference, downloads and + publishing. +3. Repeat for dev2, then production. +4. Delete the stored policy, then disable shared-key access on the data account + in a separate change, one environment at a time. + +Rollback: re-add the setting (code still honors it) and re-enable shared-key +access. From 3e78fd20e496107df1b779631fb9db33776334c5 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 09:49:32 -0300 Subject: [PATCH 02/24] fix(blob): let download and range helpers use the managed identity download_blob_to_tempfile and read_blob_range read only BLOB_CONNECTION_STRING and failed when it was empty. Route both through one credential-aware client that prefers the connection string (local emulator) and otherwise uses BLOB_ACCOUNT_URL with the app identity. --- hastelib/src/hastegeo/core/utils/blob.py | 43 +++++++++++++++++------- hastelib/tests/core/utils/test_blob.py | 42 +++++++++++++++++++++++ 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/hastelib/src/hastegeo/core/utils/blob.py b/hastelib/src/hastegeo/core/utils/blob.py index 63ec891f..7af75c86 100644 --- a/hastelib/src/hastegeo/core/utils/blob.py +++ b/hastelib/src/hastegeo/core/utils/blob.py @@ -59,6 +59,29 @@ def get_blob_service_client( raise ValueError("A connection string or account URL is required") +def get_app_blob_service_client(): + """Return the shared client for the app's own storage account. + + ``BLOB_CONNECTION_STRING`` wins when set because the docker dev stack + points it at the emulator. In Azure it is absent and the app's managed + identity is used through ``BLOB_ACCOUNT_URL``. + + Raises: + ValueError: If neither setting exists, so the error names both. + """ + connection_string = os.environ.get("BLOB_CONNECTION_STRING", "") + account_url = os.environ.get("BLOB_ACCOUNT_URL", "") + if not connection_string and not account_url: + raise ValueError( + "Neither BLOB_CONNECTION_STRING nor BLOB_ACCOUNT_URL is set; " + "cannot reach blob storage." + ) + return get_blob_service_client( + connection_string=connection_string or None, + account_url=account_url or None, + ) + + def get_cached_user_delegation_key( blob_service_client, now: datetime | None = None, @@ -169,15 +192,13 @@ async def download_blob_to_tempfile( ) -> str: """Download the blob at ``url`` to a NamedTemporaryFile and return the path. - Routes the download through the function-app-internal - ``BLOB_CONNECTION_STRING`` (so requests stay on the docker network in - dev) regardless of the container/blob URL the caller hands in. The - caller is responsible for unlinking the returned path when done — use - ``try/finally``. + Routes the download through the function app's own credential (see + :func:`get_app_blob_service_client`) regardless of the container/blob URL + the caller hands in. The caller is responsible for unlinking the returned + path when done — use ``try/finally``. """ - conn_str = os.environ.get("BLOB_CONNECTION_STRING", "") container_name, blob_name = split_blob_url(url) - bsc = get_blob_service_client(connection_string=conn_str) + bsc = get_app_blob_service_client() if max_bytes is not None and max_bytes < 1: raise ValueError("max_bytes must be positive") @@ -262,18 +283,16 @@ async def read_blob_range( ) -> BlobRange: """Read ``length`` bytes from ``offset`` of the blob at ``url``. - Routes the read through the function-app-internal - ``BLOB_CONNECTION_STRING`` (so it stays on the docker network in dev - and on the Azure backbone in prod); only the container/blob path is + Routes the read through the function app's own credential (see + :func:`get_app_blob_service_client`); only the container/blob path is taken from ``url`` — its host and SAS are ignored. ``length=None`` reads to EOF. ``data`` is clamped to the blob size; an ``offset`` at or past EOF yields empty ``data`` (callers should answer ``416``). """ - conn_str = os.environ.get("BLOB_CONNECTION_STRING", "") container_name, blob_name = split_blob_url(url) def _read() -> BlobRange: - bsc = get_blob_service_client(connection_string=conn_str) + bsc = get_app_blob_service_client() blob_client = bsc.get_container_client(container_name).get_blob_client( blob_name ) diff --git a/hastelib/tests/core/utils/test_blob.py b/hastelib/tests/core/utils/test_blob.py index fb90096c..eb1d4e74 100644 --- a/hastelib/tests/core/utils/test_blob.py +++ b/hastelib/tests/core/utils/test_blob.py @@ -21,6 +21,7 @@ clear_blob_client_caches, download_blob_to_tempfile, fetch_url_text, + get_app_blob_service_client, get_blob_service_client, get_cached_user_delegation_key, parse_byte_range, @@ -109,7 +110,48 @@ def test_user_delegation_cache_evicts_oldest_client(self): self.assertEqual(clients[-1].get_user_delegation_key.call_count, 1) +class TestAppBlobServiceClient(unittest.TestCase): + @patch("hastegeo.core.utils.blob.get_blob_service_client") + def test_prefers_connection_string(self, factory): + env = { + "BLOB_CONNECTION_STRING": "emulator", + "BLOB_ACCOUNT_URL": "https://account.test", + } + with patch.dict(os.environ, env): + get_app_blob_service_client() + + factory.assert_called_once_with( + connection_string="emulator", account_url="https://account.test" + ) + + @patch("hastegeo.core.utils.blob.get_blob_service_client") + def test_falls_back_to_identity_without_connection_string(self, factory): + env = {"BLOB_ACCOUNT_URL": "https://account.test"} + with patch.dict(os.environ, env, clear=True): + get_app_blob_service_client() + + factory.assert_called_once_with( + connection_string=None, account_url="https://account.test" + ) + + def test_names_both_settings_when_unconfigured(self): + with patch.dict(os.environ, {}, clear=True): + with self.assertRaises(ValueError) as error: + get_app_blob_service_client() + + self.assertIn("BLOB_CONNECTION_STRING", str(error.exception)) + self.assertIn("BLOB_ACCOUNT_URL", str(error.exception)) + + class TestAsyncBlobHelpers(unittest.IsolatedAsyncioTestCase): + def setUp(self): + patcher = patch.dict( + os.environ, + {"BLOB_ACCOUNT_URL": "https://account.blob.core.windows.net"}, + ) + patcher.start() + self.addCleanup(patcher.stop) + @patch("hastegeo.core.utils.blob.get_blob_service_client") async def test_download_blob_to_tempfile_uses_shared_client(self, factory): blob_client = ( From d6d0a10eee0ea21ae35e3e59a086b8900b4227d4 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 10:08:54 -0300 Subject: [PATCH 03/24] feat(blob): add per-blob read SAS helper Mint read-only, one-blob, expiring tokens with no stored policy, and let the delegation-key cache guarantee a minimum remaining lifetime. --- hastelib/src/hastegeo/core/utils/blob.py | 9 +- hastelib/src/hastegeo/core/utils/blob_sas.py | 43 ++++++++++ hastelib/tests/core/utils/test_blob_sas.py | 86 ++++++++++++++++++++ 3 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 hastelib/src/hastegeo/core/utils/blob_sas.py create mode 100644 hastelib/tests/core/utils/test_blob_sas.py diff --git a/hastelib/src/hastegeo/core/utils/blob.py b/hastelib/src/hastegeo/core/utils/blob.py index 7af75c86..f74c0bfb 100644 --- a/hastelib/src/hastegeo/core/utils/blob.py +++ b/hastelib/src/hastegeo/core/utils/blob.py @@ -85,15 +85,20 @@ def get_app_blob_service_client(): def get_cached_user_delegation_key( blob_service_client, now: datetime | None = None, + min_remaining: timedelta = timedelta(minutes=15), ): - """Return a reusable user-delegation key with a safe refresh margin.""" + """Return a reusable user-delegation key with a safe refresh margin. + + ``min_remaining`` is how long the key must still be valid; callers that + sign a token must pass at least that token's lifetime. + """ current_time = now or datetime.now(timezone.utc) cache_key = blob_service_client with _USER_DELEGATION_KEYS_LOCK: cached = _USER_DELEGATION_KEYS.get(cache_key) if cached is not None: delegation_key, expires_at = cached - if expires_at > current_time + timedelta(minutes=15): + if expires_at > current_time + min_remaining: _USER_DELEGATION_KEYS.move_to_end(cache_key) return delegation_key diff --git a/hastelib/src/hastegeo/core/utils/blob_sas.py b/hastelib/src/hastegeo/core/utils/blob_sas.py new file mode 100644 index 00000000..28c15272 --- /dev/null +++ b/hastelib/src/hastegeo/core/utils/blob_sas.py @@ -0,0 +1,43 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +"""Create short-lived read tokens scoped to one blob.""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +from azure.storage.blob import BlobSasPermissions, generate_blob_sas + +READ_TOKEN_MINUTES = 60 +CLOCK_SKEW_MINUTES = 5 +# A delegation key must outlive every token signed with it. +DELEGATION_KEY_MARGIN = timedelta(minutes=READ_TOKEN_MINUTES + 5) + + +def blob_read_sas( + account_name: str, + container_name: str, + blob_name: str, + account_key: str | None = None, + user_delegation_key=None, + minutes: int = READ_TOKEN_MINUTES, +) -> str: + """Return a read-only SAS token scoped to exactly one blob.""" + if account_key is None and user_delegation_key is None: + raise ValueError( + "Cannot mint a read token without an account or delegation key." + ) + if minutes <= 0: + raise ValueError("minutes must be positive") + + now = datetime.now(timezone.utc) + return generate_blob_sas( + account_name=account_name, + container_name=container_name, + blob_name=blob_name, + account_key=account_key, + user_delegation_key=user_delegation_key, + permission=BlobSasPermissions(read=True), + start=now - timedelta(minutes=CLOCK_SKEW_MINUTES), + expiry=now + timedelta(minutes=minutes), + ) diff --git a/hastelib/tests/core/utils/test_blob_sas.py b/hastelib/tests/core/utils/test_blob_sas.py new file mode 100644 index 00000000..65f89321 --- /dev/null +++ b/hastelib/tests/core/utils/test_blob_sas.py @@ -0,0 +1,86 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +"""A read token must be worth no more than the blob it points at.""" + +import unittest +from datetime import datetime, timedelta, timezone +from unittest.mock import MagicMock +from urllib.parse import parse_qs + +from hastegeo.core.utils.blob import ( + clear_blob_client_caches, + get_cached_user_delegation_key, +) +from hastegeo.core.utils.blob_sas import blob_read_sas + +ACCOUNT = "examplestorage" +CONTAINER = "data" +BLOB = "partition/imagery.tif" +# Not a real credential: any base64 string lets signing run offline. +FAKE_KEY = "Zm9vYmFyYmF6cXV1eGZvb2JhcmJhenF1dXhmb29iYXJiYXpxdXV4Zm9vYmE=" # pragma: allowlist secret # gitleaks:allow + + +def _token(**overrides): + kwargs = { + "account_name": ACCOUNT, + "container_name": CONTAINER, + "blob_name": BLOB, + "account_key": FAKE_KEY, + } + kwargs.update(overrides) + return parse_qs(blob_read_sas(**kwargs)) + + +class TestBlobReadSas(unittest.TestCase): + def test_token_is_read_only_and_scoped_to_one_blob(self): + token = _token() + self.assertEqual(token["sr"], ["b"]) + self.assertEqual(token["sp"], ["r"]) + + def test_token_has_expiry_and_no_stored_policy(self): + token = _token() + self.assertIn("se", token) + self.assertNotIn("si", token) + self.assertLess(token["st"][0], token["se"][0]) + + def test_signature_covers_the_blob_name(self): + self.assertNotEqual( + _token()["sig"], _token(blob_name="other/imagery.tif")["sig"] + ) + + def test_expiry_follows_requested_minutes(self): + short = _token(minutes=5)["se"][0] + long = _token(minutes=60)["se"][0] + self.assertLess(short, long) + + def test_requires_a_signing_key(self): + with self.assertRaises(ValueError): + blob_read_sas(ACCOUNT, CONTAINER, BLOB) + + def test_rejects_non_positive_lifetime(self): + with self.assertRaises(ValueError): + _token(minutes=0) + + +class TestDelegationKeyLifetime(unittest.TestCase): + def setUp(self): + clear_blob_client_caches() + self.addCleanup(clear_blob_client_caches) + + def test_key_without_enough_life_is_refreshed(self): + client = MagicMock() + now = datetime(2026, 1, 1, tzinfo=timezone.utc) + get_cached_user_delegation_key(client, now=now) + # 100 minutes later the cached 2h key has 20 minutes left: enough + # for the default margin but not for a 65 minute token. + later = now + timedelta(minutes=100) + get_cached_user_delegation_key(client, now=later) + self.assertEqual(client.get_user_delegation_key.call_count, 1) + get_cached_user_delegation_key( + client, now=later, min_remaining=timedelta(minutes=65) + ) + self.assertEqual(client.get_user_delegation_key.call_count, 2) + + +if __name__ == "__main__": + unittest.main() From 96b11457f459dab9dc69154349629c79c981f9bb Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 10:10:18 -0300 Subject: [PATCH 04/24] fix(storage): scope data-layer read tokens to one blob get_file_remote_path signed a container-wide token under a stored policy. Sign a one-blob, one-hour token instead, with the app identity on real Azure and the account key only for the emulator. The container read policy is no longer created, so deleting it revokes old tokens for good. --- .../azure_blob_storage_data_layer.py | 45 +++++------ .../test_azure_blob_storage_data_layer.py | 80 +++++++++++-------- 2 files changed, 67 insertions(+), 58 deletions(-) diff --git a/hastelib/src/hastegeo/core/data_layer/azure_blob_storage_data_layer.py b/hastelib/src/hastegeo/core/data_layer/azure_blob_storage_data_layer.py index 243d78cf..81607669 100644 --- a/hastelib/src/hastegeo/core/data_layer/azure_blob_storage_data_layer.py +++ b/hastelib/src/hastegeo/core/data_layer/azure_blob_storage_data_layer.py @@ -9,13 +9,12 @@ import yaml from azure.core.exceptions import ResourceExistsError, ResourceNotFoundError from azure.storage.blob import BlobBlock # type: ignore -from azure.storage.blob import generate_container_sas from ..utils.blob import ( get_blob_service_client, get_cached_user_delegation_key, ) -from ..utils.blob_access_policy import ensure_container_read_policy +from ..utils.blob_sas import DELEGATION_KEY_MARGIN, blob_read_sas from ..utils.metadata import matches_metadata_type from ..utils.parallel import parallel_map from .abstract_data_layer import AbstractDataLayer @@ -41,22 +40,28 @@ def __init__( ) self.user_delegation_key = None self.account_key = self.blob_service_client.credential.account_key + # On real Azure, sign with the app identity so the account key is + # never needed to mint tokens. Azurite keeps the account key. + self.identity_blob_service_client = ( + get_blob_service_client(account_url=account_url) + if str(account_url or "").startswith("https://") + else None + ) else: self.blob_service_client = get_blob_service_client( account_url=account_url ) self.user_delegation_key = None self.account_key = None + self.identity_blob_service_client = self.blob_service_client + # Kept for call compatibility; no container policy is maintained, so + # deleting the legacy one revokes old tokens for good. self.container_read_policy = container_read_policy_name self.container_client = self.blob_service_client.get_container_client( container ) - cache_key = ( - self.blob_service_client.url, - container, - self.container_read_policy, - ) + cache_key = (self.blob_service_client.url, container) with _INITIALIZED_CONTAINERS_LOCK: if cache_key not in _INITIALIZED_CONTAINERS: try: @@ -66,16 +71,8 @@ def __init__( ) except ResourceExistsError: logging.info(f"Container '{container}' already exists.") - self._create_or_update_managed_access_policy() _INITIALIZED_CONTAINERS.add(cache_key) - def _create_or_update_managed_access_policy(self) -> None: - ensure_container_read_policy( - self.container_client, - self.container_read_policy, - expiration_days=90, - ) - def get_file_path( self, identifier, @@ -121,21 +118,23 @@ def get_file_remote_path( # ``check_exists=False`` skips the per-blob existence HEAD request. Use # it when the caller has already confirmed presence in bulk (e.g. via - # ``list_identifiers``) — the container read SAS is identical for every - # blob, so building the URL is a purely local operation. + # ``list_identifiers``) — the token is signed locally for this blob, + # so building the URL needs no request. if not check_exists or blob_client.exists(): + account_key = self.account_key user_delegation_key = self.user_delegation_key - if self.account_key is None: + if self.identity_blob_service_client is not None: + account_key = None user_delegation_key = get_cached_user_delegation_key( - self.blob_service_client + self.identity_blob_service_client, + min_remaining=DELEGATION_KEY_MARGIN, ) - # Generate SAS token with the policy - sas_token = generate_container_sas( + sas_token = blob_read_sas( account_name=self.container_client.account_name, container_name=self.container_client.container_name, - policy_id=self.container_read_policy, + blob_name=blob_name, + account_key=account_key, user_delegation_key=user_delegation_key, - account_key=self.account_key, ) sas_url = f"{blob_client.url}?{sas_token}" diff --git a/hastelib/tests/core/data_layer/test_azure_blob_storage_data_layer.py b/hastelib/tests/core/data_layer/test_azure_blob_storage_data_layer.py index a67de175..54868b3a 100644 --- a/hastelib/tests/core/data_layer/test_azure_blob_storage_data_layer.py +++ b/hastelib/tests/core/data_layer/test_azure_blob_storage_data_layer.py @@ -186,53 +186,57 @@ def test_identifier_listing_excludes_longer_type(self) -> None: self.assertEqual(result, ["a"]) - @patch( - "hastegeo.core.data_layer.azure_blob_storage_data_layer.generate_container_sas", - return_value="sas", - ) @patch( "hastegeo.core.data_layer.azure_blob_storage_data_layer.get_cached_user_delegation_key", return_value="delegation-key", ) - def test_remote_path_fetches_delegation_key_lazily( - self, delegation_key, _generate_sas + def test_remote_path_signs_one_blob_with_the_identity( + self, delegation_key ) -> None: - self.layer.blob_service_client = Mock() + identity_client = Mock() + self.layer.identity_blob_service_client = identity_client self.layer.account_key = None self.layer.user_delegation_key = None - self.layer.container_read_policy = "policy" self.layer.container_client.account_name = "account" self.layer.container_client.container_name = "metadata" self.blob_client.url = "https://account.test/metadata/model_a.json" - result = self.layer.get_file_remote_path( - "a", "model", check_exists=False - ) + with patch( + "hastegeo.core.data_layer.azure_blob_storage_data_layer.blob_read_sas", + return_value="sas", + ) as sas: + result = self.layer.get_file_remote_path( + "a", "model", check_exists=False + ) self.assertEqual(result, f"{self.blob_client.url}?sas") - delegation_key.assert_called_once_with(self.layer.blob_service_client) + self.assertEqual(delegation_key.call_args.args, (identity_client,)) + kwargs = sas.call_args.kwargs + self.assertEqual(kwargs["blob_name"], "partition/model_a.json") + self.assertIsNone(kwargs["account_key"]) + self.assertEqual(kwargs["user_delegation_key"], "delegation-key") - @patch( - "hastegeo.core.data_layer.azure_blob_storage_data_layer.generate_container_sas", - return_value="sas", - ) @patch( "hastegeo.core.data_layer.azure_blob_storage_data_layer.get_cached_user_delegation_key" ) - def test_remote_path_with_account_key_skips_delegation_key( - self, delegation_key, _generate_sas + def test_remote_path_on_the_emulator_uses_the_account_key( + self, delegation_key ) -> None: - self.layer.blob_service_client = Mock() + self.layer.identity_blob_service_client = None self.layer.account_key = "account-key" # pragma: allowlist secret self.layer.user_delegation_key = None - self.layer.container_read_policy = "policy" self.layer.container_client.account_name = "account" self.layer.container_client.container_name = "metadata" - self.blob_client.url = "https://account.test/metadata/model_a.json" + self.blob_client.url = "http://azurite:10000/account/metadata/a.json" - self.layer.get_file_remote_path("a", "model", check_exists=False) + with patch( + "hastegeo.core.data_layer.azure_blob_storage_data_layer.blob_read_sas", + return_value="sas", + ) as sas: + self.layer.get_file_remote_path("a", "model", check_exists=False) delegation_key.assert_not_called() + self.assertEqual(sas.call_args.kwargs["account_key"], "account-key") def test_load_map_preserves_keys_and_missing_records(self) -> None: self.layer.load = Mock( @@ -257,13 +261,7 @@ def tearDown(self) -> None: @patch( "hastegeo.core.data_layer.azure_blob_storage_data_layer.get_blob_service_client" ) - @patch.object( - AzureBlobStorageDataLayer, - "_create_or_update_managed_access_policy", - ) - def test_connection_string_uses_cached_client( - self, create_policy, factory - ): + def test_connection_string_uses_cached_client(self, factory): service = Mock(url="https://account.test") service.credential.account_key = "key" # pragma: allowlist secret factory.return_value = service @@ -275,17 +273,30 @@ def test_connection_string_uses_cached_client( ) self.assertIs(layer.blob_service_client, service) + self.assertIsNone(layer.identity_blob_service_client) factory.assert_called_once_with(connection_string="connection") - create_policy.assert_called_once_with() @patch( "hastegeo.core.data_layer.azure_blob_storage_data_layer.get_blob_service_client" ) - @patch.object( - AzureBlobStorageDataLayer, - "_create_or_update_managed_access_policy", + def test_connection_string_with_https_url_signs_by_identity(self, factory): + service = Mock(url="https://account.test") + service.credential.account_key = "key" # pragma: allowlist secret + factory.return_value = service + + layer = AzureBlobStorageDataLayer( + account_url="https://account.test", + container="metadata", + connection_string="connection", + ) + + self.assertIsNotNone(layer.identity_blob_service_client) + factory.assert_any_call(account_url="https://account.test") + + @patch( + "hastegeo.core.data_layer.azure_blob_storage_data_layer.get_blob_service_client" ) - def test_managed_identity_uses_cached_client(self, create_policy, factory): + def test_managed_identity_uses_cached_client(self, factory): service = Mock(url="https://account.test") service.get_user_delegation_key.return_value = "delegation-key" factory.return_value = service @@ -300,7 +311,6 @@ def test_managed_identity_uses_cached_client(self, create_policy, factory): self.assertIsNone(layer.user_delegation_key) service.get_user_delegation_key.assert_not_called() factory.assert_called_once_with(account_url="https://account.test") - create_policy.assert_called_once_with() if __name__ == "__main__": From e44bbd0b8e785e2cd384aed44657ae48f0454514 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 10:11:17 -0300 Subject: [PATCH 05/24] fix(storage): scope artifact read tokens to one blob get_download_url signed a container-wide token under a stored policy. Sign a one-blob, one-hour token instead, using split_blob_url for artifact paths so Azurite hosts resolve. Remove the now-unused container read policy helper. --- .../azure_blob_artifact_storage.py | 47 ++-- .../hastegeo/core/utils/blob_access_policy.py | 50 ---- .../test_azure_blob_artifact_storage.py | 53 ++-- .../core/utils/test_blob_access_policy.py | 257 ------------------ 4 files changed, 52 insertions(+), 355 deletions(-) delete mode 100644 hastelib/src/hastegeo/core/utils/blob_access_policy.py delete mode 100644 hastelib/tests/core/utils/test_blob_access_policy.py diff --git a/hastelib/src/hastegeo/core/artifact_storage/azure_blob_artifact_storage.py b/hastelib/src/hastegeo/core/artifact_storage/azure_blob_artifact_storage.py index 5fb36941..b0b8f1ea 100644 --- a/hastelib/src/hastegeo/core/artifact_storage/azure_blob_artifact_storage.py +++ b/hastelib/src/hastegeo/core/artifact_storage/azure_blob_artifact_storage.py @@ -17,16 +17,16 @@ from azure.storage.blob import ( BlobSasPermissions, generate_blob_sas, - generate_container_sas, ) from hastegeo.core.utils.blob import ( get_blob_service_client, get_cached_user_delegation_key, + split_blob_url, ) +from hastegeo.core.utils.blob_sas import DELEGATION_KEY_MARGIN, blob_read_sas from hastegeo.core.utils.logs import Logger from hastegeo.core.utils.parallel import configured_worker_count, parallel_map -from ..utils.blob_access_policy import ensure_container_read_policy from .abstract_artifact_storage import AbstractArtifactStorage _INITIALIZED_CONTAINERS = set() @@ -47,8 +47,8 @@ def __init__( super().__init__(partition_key) self.logger = Logger.get_logger(__name__) # A write-only target (e.g. the PC publish container) doesn't mint read - # SAS, so it skips the user-delegation key and the stored read policy — - # which need Storage Blob Delegator / Owner. Write access alone suffices. + # SAS, so it needs no delegation key (Storage Blob Delegator / Owner). + # Write access alone suffices. self.serves_read_sas = serves_read_sas if connection_string: self.blob_service_client = get_blob_service_client( @@ -69,17 +69,14 @@ def __init__( self.user_delegation_key = None self.account_key = None + # Kept for call compatibility; no container policy is maintained, so + # deleting the legacy one revokes old tokens for good. self.container_read_policy = container_read_policy_name self.blob_read_policy = blob_read_policy_name - self.sas_expiration_days = 90 self.container_client = self.blob_service_client.get_container_client( container ) - cache_key = ( - self.blob_service_client.url, - container, - self.container_read_policy, - ) + cache_key = (self.blob_service_client.url, container) with _INITIALIZED_CONTAINERS_LOCK: if cache_key not in _INITIALIZED_CONTAINERS: try: @@ -91,17 +88,8 @@ def __init__( self.logger.info( f"Container '{container}' already exists." ) - if self.serves_read_sas: - self._create_or_update_managed_access_policy() _INITIALIZED_CONTAINERS.add(cache_key) - def _create_or_update_managed_access_policy(self) -> None: - ensure_container_read_policy( - self.container_client, - self.container_read_policy, - expiration_days=self.sas_expiration_days, - ) - def get_file_path( self, identifier: str, extra_partition_keys: list | str = None ) -> str: @@ -142,28 +130,33 @@ def get_download_url( # and use the container_client and blob name to get the blob client # It's a cleaner separation of metaphors blob_client = BlobClient.from_blob_url(artifact_path) + container_name, blob_name = split_blob_url(artifact_path) else: blob_name = self.get_file_path( identifier, extra_partition_keys=extra_partition_keys ) blob_client = self.container_client.get_blob_client(blob_name) + container_name = self.container_client.container_name if not include_sas: return str(blob_client.url) - # Otherwise generate SAS token - + # One blob, read-only, one hour: these URLs reach clients, so the + # token must be worth no more than the object it points at. + account_key = self.account_key user_delegation_key = self.user_delegation_key - if self.account_key is None: + if self.identity_blob_service_client is not None: + account_key = None user_delegation_key = get_cached_user_delegation_key( - self.blob_service_client + self.identity_blob_service_client, + min_remaining=DELEGATION_KEY_MARGIN, ) - sas_token = generate_container_sas( + sas_token = blob_read_sas( account_name=self.container_client.account_name, - container_name=self.container_client.container_name, - policy_id=self.container_read_policy, - account_key=self.account_key, + container_name=container_name, + blob_name=blob_name, + account_key=account_key, user_delegation_key=user_delegation_key, ) return str(f"{blob_client.url}?{sas_token}") diff --git a/hastelib/src/hastegeo/core/utils/blob_access_policy.py b/hastelib/src/hastegeo/core/utils/blob_access_policy.py deleted file mode 100644 index c2848870..00000000 --- a/hastelib/src/hastegeo/core/utils/blob_access_policy.py +++ /dev/null @@ -1,50 +0,0 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. - -"""Maintain one stored read policy without replacing unrelated container ACLs.""" - -from datetime import datetime, timedelta, timezone - -from azure.storage.blob import ( - AccessPolicy, - ContainerClient, - ContainerSasPermissions, -) - - -def ensure_container_read_policy( - container_client: ContainerClient, - policy_id: str, - *, - expiration_days: int, -) -> None: - properties = container_client.get_container_access_policy() - # The getter returns SignedIdentifier objects; the setter takes a mapping. - policies = { - identifier.id: identifier.access_policy - for identifier in properties["signed_identifiers"] - } - now = datetime.now(timezone.utc) - expiration_date = now + timedelta(days=expiration_days) - if policy_id in policies: - policy = policies[policy_id] - # Omitted expiry is valid: the associated SAS may supply it instead. - if policy is None or policy.expiry is None: - return - expiry = policy.expiry - if isinstance(expiry, str): - expiry = datetime.fromisoformat(expiry.replace("Z", "+00:00")) - if expiry.tzinfo is None: - expiry = expiry.replace(tzinfo=timezone.utc) - if expiry >= now: - return - policy.expiry = expiration_date - else: - policies[policy_id] = AccessPolicy( - permission=ContainerSasPermissions(read=True), - expiry=expiration_date, - ) - container_client.set_container_access_policy( - signed_identifiers=policies, - public_access=properties["public_access"], - ) diff --git a/hastelib/tests/core/artifact_storage/test_azure_blob_artifact_storage.py b/hastelib/tests/core/artifact_storage/test_azure_blob_artifact_storage.py index beb23a2d..b2a215df 100644 --- a/hastelib/tests/core/artifact_storage/test_azure_blob_artifact_storage.py +++ b/hastelib/tests/core/artifact_storage/test_azure_blob_artifact_storage.py @@ -92,20 +92,20 @@ def test_fetch_rejects_invalid_worker_configuration(self) -> None: ) @patch( - "hastegeo.core.artifact_storage.azure_blob_artifact_storage.generate_container_sas", + "hastegeo.core.artifact_storage.azure_blob_artifact_storage.blob_read_sas", return_value="sas", ) @patch( "hastegeo.core.artifact_storage.azure_blob_artifact_storage.get_cached_user_delegation_key", return_value="delegation-key", ) - def test_download_url_fetches_delegation_key_lazily( - self, delegation_key, _generate_sas + def test_download_url_signs_one_blob_with_the_identity( + self, delegation_key, sas ) -> None: - self.storage.blob_service_client = Mock() + identity_client = Mock() + self.storage.identity_blob_service_client = identity_client self.storage.account_key = None self.storage.user_delegation_key = None - self.storage.container_read_policy = "policy" self.storage.container_client.account_name = "account" self.storage.container_client.container_name = "artifacts" self.blob_client.url = "https://account.test/artifacts/file.txt" @@ -113,10 +113,33 @@ def test_download_url_fetches_delegation_key_lazily( result = self.storage.get_download_url(identifier="file.txt") self.assertEqual(result, f"{self.blob_client.url}?sas") - delegation_key.assert_called_once_with( - self.storage.blob_service_client + self.assertEqual(delegation_key.call_args.args, (identity_client,)) + kwargs = sas.call_args.kwargs + self.assertEqual(kwargs["blob_name"], "file.txt") + self.assertEqual(kwargs["container_name"], "artifacts") + self.assertIsNone(kwargs["account_key"]) + + @patch( + "hastegeo.core.artifact_storage.azure_blob_artifact_storage.blob_read_sas", + return_value="sas", + ) + def test_download_url_for_artifact_path_signs_its_own_blob( + self, sas + ) -> None: + self.storage.identity_blob_service_client = None + self.storage.account_key = "key" # pragma: allowlist secret + self.storage.user_delegation_key = None + self.storage.container_client.account_name = "devstoreaccount1" + + self.storage.get_download_url( + artifact_path="http://azurite:10000/devstoreaccount1/data/a/b.tif" ) + kwargs = sas.call_args.kwargs + self.assertEqual(kwargs["container_name"], "data") + self.assertEqual(kwargs["blob_name"], "a/b.tif") + self.assertEqual(kwargs["account_key"], "key") + @patch( "hastegeo.core.artifact_storage.azure_blob_artifact_storage.generate_blob_sas", return_value="sas", @@ -171,13 +194,7 @@ def tearDown(self) -> None: @patch( "hastegeo.core.artifact_storage.azure_blob_artifact_storage.get_blob_service_client" ) - @patch.object( - AzureBlobArtifactStorage, - "_create_or_update_managed_access_policy", - ) - def test_connection_string_uses_cached_client( - self, create_policy, factory - ): + def test_connection_string_uses_cached_client(self, factory): service = Mock(url="https://account.test") service.credential.account_key = "key" # pragma: allowlist secret factory.return_value = service @@ -190,7 +207,6 @@ def test_connection_string_uses_cached_client( self.assertIs(storage.blob_service_client, service) factory.assert_called_once_with(connection_string="connection") - create_policy.assert_called_once_with() @patch( "hastegeo.core.artifact_storage.azure_blob_artifact_storage.get_blob_service_client" @@ -213,11 +229,7 @@ def test_managed_identity_uses_cached_client(self, factory): @patch( "hastegeo.core.artifact_storage.azure_blob_artifact_storage.get_blob_service_client" ) - @patch.object( - AzureBlobArtifactStorage, - "_create_or_update_managed_access_policy", - ) - def test_existing_container_is_reused(self, create_policy, factory): + def test_existing_container_is_reused(self, factory): service = Mock(url="https://account.test") service.credential.account_key = "key" # pragma: allowlist secret container = service.get_container_client.return_value @@ -231,7 +243,6 @@ def test_existing_container_is_reused(self, create_policy, factory): ) self.assertIs(storage.container_client, container) - create_policy.assert_called_once_with() if __name__ == "__main__": diff --git a/hastelib/tests/core/utils/test_blob_access_policy.py b/hastelib/tests/core/utils/test_blob_access_policy.py deleted file mode 100644 index 659afb66..00000000 --- a/hastelib/tests/core/utils/test_blob_access_policy.py +++ /dev/null @@ -1,257 +0,0 @@ -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. - -import unittest -from datetime import datetime, timedelta, timezone -from unittest.mock import MagicMock - -from azure.core.exceptions import ClientAuthenticationError -from azure.storage.blob import AccessPolicy, ContainerClient -from azure.storage.blob._generated.models import SignedIdentifier -from hastegeo.core.artifact_storage.azure_blob_artifact_storage import ( - AzureBlobArtifactStorage, -) -from hastegeo.core.data_layer.azure_blob_storage_data_layer import ( - AzureBlobStorageDataLayer, -) - -Storage = AzureBlobArtifactStorage | AzureBlobStorageDataLayer -BACKENDS = (AzureBlobArtifactStorage, AzureBlobStorageDataLayer) -POLICY_ID = "managed-read" - - -class TestBlobAccessPolicy(unittest.TestCase): - def storage( - self, - backend: type[Storage], - policies: list[SignedIdentifier], - public_access: str | None = None, - ) -> Storage: - # Bypass constructors: no credentials, container creation or network. - storage = object.__new__(backend) - storage.container_read_policy = POLICY_ID - storage.sas_expiration_days = 90 - storage.logger = MagicMock() - storage.container_client = MagicMock(spec=ContainerClient) - storage.container_client.get_container_access_policy.return_value = { - "signed_identifiers": policies, - "public_access": public_access, - } - return storage - - def unrelated_policy(self) -> SignedIdentifier: - return SignedIdentifier( - id="unrelated-policy", - access_policy=AccessPolicy( - permission="rl", - start="2020-01-01T00:00:00Z", - expiry="2099-01-01T00:00:00Z", - ), - ) - - def assert_expiry_window( - self, - expiry: datetime, - before: datetime, - after: datetime, - days: int = 90, - ) -> None: - self.assertGreaterEqual(expiry, before + timedelta(days=days)) - self.assertLessEqual(expiry, after + timedelta(days=days)) - - def test_existing_unexpired_policy_does_not_write_acl(self) -> None: - future = datetime.now(timezone.utc) + timedelta(days=30) - for backend in BACKENDS: - for expiry in ( - future, - future.isoformat().replace("+00:00", "Z"), - future.replace(tzinfo=None), - future.replace(tzinfo=None).isoformat(), - ): - with self.subTest(backend=backend.__name__, expiry=expiry): - policy = AccessPolicy(permission="r", expiry=expiry) - storage = self.storage( - backend, - [ - SignedIdentifier( - id=POLICY_ID, access_policy=policy - ), - self.unrelated_policy(), - ], - ) - storage._create_or_update_managed_access_policy() - client = storage.container_client - client.get_container_access_policy.assert_called_once_with() - client.set_container_access_policy.assert_not_called() - - def test_expired_policy_renews_only_expiry_and_preserves_other_acl( - self, - ) -> None: - past = datetime.now(timezone.utc) - timedelta(days=1) - for backend in BACKENDS: - for expiry in (past, past.isoformat().replace("+00:00", "Z")): - for public_access in (None, "blob", "container"): - with self.subTest( - backend=backend.__name__, - expiry=expiry, - public_access=public_access, - ): - unrelated = self.unrelated_policy() - start = "2020-01-01T00:00:00Z" - storage = self.storage( - backend, - [ - SignedIdentifier( - id=POLICY_ID, - access_policy=AccessPolicy( - permission="rl", - start=start, - expiry=expiry, - ), - ), - unrelated, - ], - public_access, - ) - before = datetime.now(timezone.utc) - storage._create_or_update_managed_access_policy() - after = datetime.now(timezone.utc) - write = ( - storage.container_client.set_container_access_policy - ) - write.assert_called_once() - written = write.call_args.kwargs["signed_identifiers"] - self.assertEqual( - set(written), {POLICY_ID, unrelated.id} - ) - self.assertEqual( - written[unrelated.id], unrelated.access_policy - ) - self.assertEqual(written[POLICY_ID].permission, "rl") - self.assertEqual(written[POLICY_ID].start, start) - self.assert_expiry_window( - written[POLICY_ID].expiry, before, after - ) - self.assertEqual( - write.call_args.kwargs["public_access"], - public_access, - ) - - def test_missing_policy_is_added_without_dropping_other_acl(self) -> None: - for backend in BACKENDS: - for public_access in (None, "blob", "container"): - for has_other_policy in (False, True): - with self.subTest( - backend=backend.__name__, - public_access=public_access, - has_other_policy=has_other_policy, - ): - unrelated = self.unrelated_policy() - storage = self.storage( - backend, - [unrelated] if has_other_policy else [], - public_access, - ) - before = datetime.now(timezone.utc) - storage._create_or_update_managed_access_policy() - after = datetime.now(timezone.utc) - write = ( - storage.container_client.set_container_access_policy - ) - write.assert_called_once() - written = write.call_args.kwargs["signed_identifiers"] - self.assertEqual( - set(written), - {POLICY_ID, unrelated.id} - if has_other_policy - else {POLICY_ID}, - ) - if has_other_policy: - self.assertEqual( - written[unrelated.id], unrelated.access_policy - ) - self.assertEqual( - str(written[POLICY_ID].permission), "r" - ) - self.assertIsNone(written[POLICY_ID].start) - self.assert_expiry_window( - written[POLICY_ID].expiry, before, after - ) - self.assertEqual( - write.call_args.kwargs["public_access"], - public_access, - ) - - def test_artifact_policy_uses_configured_expiration_days(self) -> None: - storage = self.storage(AzureBlobArtifactStorage, []) - storage.sas_expiration_days = 7 - before = datetime.now(timezone.utc) - storage._create_or_update_managed_access_policy() - after = datetime.now(timezone.utc) - written = ( - storage.container_client.set_container_access_policy.call_args - ) - self.assert_expiry_window( - written.kwargs["signed_identifiers"][POLICY_ID].expiry, - before, - after, - days=7, - ) - - def test_existing_policy_without_expiry_is_not_rewritten(self) -> None: - for backend in BACKENDS: - for policy in (AccessPolicy(permission="r"), None): - with self.subTest(backend=backend.__name__, policy=policy): - storage = self.storage( - backend, - [ - SignedIdentifier( - id=POLICY_ID, - access_policy=policy, - ) - ], - ) - storage._create_or_update_managed_access_policy() - storage.container_client.set_container_access_policy.assert_not_called() - - def test_invalid_expiry_propagates_without_replacing_policy(self) -> None: - for backend in BACKENDS: - with self.subTest(backend=backend.__name__): - storage = self.storage( - backend, - [ - SignedIdentifier( - id=POLICY_ID, - access_policy=AccessPolicy( - permission="r", expiry="invalid" - ), - ) - ], - ) - with self.assertRaises(ValueError): - storage._create_or_update_managed_access_policy() - storage.container_client.set_container_access_policy.assert_not_called() - - def test_acl_read_authorization_failure_propagates(self) -> None: - for backend in BACKENDS: - with self.subTest(backend=backend.__name__): - storage = self.storage(backend, []) - error = ClientAuthenticationError("ACL read denied") - client = storage.container_client - client.get_container_access_policy.side_effect = error - with self.assertRaises(ClientAuthenticationError) as raised: - storage._create_or_update_managed_access_policy() - self.assertIs(raised.exception, error) - client.set_container_access_policy.assert_not_called() - - def test_acl_write_authorization_failure_propagates(self) -> None: - for backend in BACKENDS: - with self.subTest(backend=backend.__name__): - storage = self.storage(backend, [self.unrelated_policy()]) - error = ClientAuthenticationError("ACL write denied") - client = storage.container_client - client.set_container_access_policy.side_effect = error - with self.assertRaises(ClientAuthenticationError) as raised: - storage._create_or_update_managed_access_policy() - self.assertIs(raised.exception, error) - client.set_container_access_policy.assert_called_once() From 9e1570c6e0f98c03b27e83df08e21d7793961f8f Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 10:15:54 -0300 Subject: [PATCH 06/24] chore(infra): stop provisioning BLOB_CONNECTION_STRING on function apps Blob and queue access is identity-based, so the account-key connection string is no longer set by Bicep or deploy_apps.sh. deploy_apps.sh also deletes the leftover setting from existing apps. Compiled main.json regenerated. --- .github/scripts/deploy_apps.sh | 16 ++++++---------- api/hastefuncapi/function_app.py | 3 ++- infra/main.json | 8 ++++---- infra/modules/functions.bicep | 11 +++-------- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/.github/scripts/deploy_apps.sh b/.github/scripts/deploy_apps.sh index 57460cea..8d96ba23 100644 --- a/.github/scripts/deploy_apps.sh +++ b/.github/scripts/deploy_apps.sh @@ -307,13 +307,6 @@ deploy_function() { size_settings+=("HASTE_MAX_IMAGERY_DOWNLOAD_BYTES=$HASTE_MAX_IMAGERY_DOWNLOAD_BYTES") fi - # Get storage account connection string with error handling - echo "Retrieving storage account connection string..." - BLOB_CONNECTION_STRING=$(az storage account show-connection-string --name "$STORAGE_ACCOUNT" --resource-group "$RESOURCE_GROUP" --query connectionString -o tsv) || { - echo "ERROR: Failed to retrieve storage account connection string. Ensure service principal has 'Storage Account Key Operator Service Role' or 'Reader and Data Access' role on storage account '$STORAGE_ACCOUNT'." >&2 - exit 1 - } - # Get batch account key with error handling echo "Retrieving batch account key..." AZURE_BATCH_ACCOUNT_KEY=$(az batch account keys list --name "$BATCH_ACCOUNT" --resource-group "$SHARED_RESOURCE_GROUP" --query "primary" -o tsv) || { @@ -340,7 +333,6 @@ deploy_function() { "QUEUE_ACCOUNT_URL=https://${STORAGE_ACCOUNT}.queue.core.windows.net" \ "BLOB_ACCOUNT_URL=https://${STORAGE_ACCOUNT}.blob.core.windows.net" \ "BLOB_CONTAINER=data" \ - "BLOB_CONNECTION_STRING=$BLOB_CONNECTION_STRING" \ "AZURE_BATCH_ACCOUNT_NAME=$BATCH_ACCOUNT" \ "AZURE_BATCH_URL=https://${BATCH_ACCOUNT}.${LOCATION}.batch.azure.com" \ "AZURE_BATCH_ACCOUNT_KEY=$AZURE_BATCH_ACCOUNT_KEY" \ @@ -377,9 +369,13 @@ deploy_function() { "PUBLISH_EXPLORER_RENDER_ENABLED=${PUBLISH_EXPLORER_RENDER_ENABLED}" \ "AzureFunctionsWebHost__hostId=${HOST_ID}" \ --output none - fi - # TiTiler reads HASTE imagery only from this storage host (plus its + # Blob and queue access is identity-based. Drop the account-key + # connection string left behind by earlier deployments. + az functionapp config appsettings delete --name "$FUNCTION_NAME" --resource-group "$RESOURCE_GROUP" \ + --setting-names BLOB_CONNECTION_STRING \ + --output none + fi HASTE imagery only from this storage host (plus its # built-in public open-data hosts). Mirrors functions.bicep; without it # HASTE imagery tiles fail closed. if [ "$FUNCTION_NAME" = "$FUNCTION_TITILER_API" ]; then diff --git a/api/hastefuncapi/function_app.py b/api/hastefuncapi/function_app.py index 341d3986..90640836 100644 --- a/api/hastefuncapi/function_app.py +++ b/api/hastefuncapi/function_app.py @@ -3808,7 +3808,8 @@ async def GetBuildingFootprintsGeoJSON( ) # Download via the helper that handles both Azurite and real-Azure - # URL shapes and routes through BLOB_CONNECTION_STRING. + # URL shapes, using the app's own credential rather than the SAS in + # the URL. tmp_path = await download_blob_to_tempfile( footprints_url, suffix=".gpkg" ) diff --git a/infra/main.json b/infra/main.json index 7f380fba..14a786e0 100644 --- a/infra/main.json +++ b/infra/main.json @@ -6,7 +6,7 @@ "_generator": { "name": "bicep", "version": "0.47.16.16243", - "templateHash": "9892879045433928477" + "templateHash": "6938013615074271928" } }, "parameters": { @@ -1547,7 +1547,7 @@ "_generator": { "name": "bicep", "version": "0.47.16.16243", - "templateHash": "510276896164916940" + "templateHash": "4785737563635743185" } }, "parameters": { @@ -1901,7 +1901,7 @@ "value": false }, "appSettings": { - "value": "[concat(createArray(createObject('name', 'env', 'value', 'prod'), createObject('name', 'DEVELOPMENT_MODE', 'value', if(parameters('developmentMode'), 'true', 'false')), createObject('name', 'IMAGE_QUEUE_NAME', 'value', 'image-layers-queue'), createObject('name', 'INFERENCE_QUEUE_NAME', 'value', 'inference-queue'), createObject('name', 'STATS_QUEUE_NAME', 'value', 'stats-queue'), createObject('name', 'TRAIN_QUEUE_NAME', 'value', 'train-queue'), createObject('name', 'ZIP_QUEUE_NAME', 'value', 'zip-queue'), createObject('name', 'EMBEDDING_QUEUE_NAME', 'value', 'embedding-queue'), createObject('name', 'PUBLISH_QUEUE_NAME', 'value', 'publish-queue'), createObject('name', 'IMAGERY_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'METADATA_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'ARTIFACT_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'RUNNER_TYPE', 'value', 'azure_batch'), createObject('name', 'TEMP_DATA_PATH', 'value', '/data'), createObject('name', 'DATA_PATH', 'value', '/data'), createObject('name', 'TITILER_ENDPOINT', 'value', '/api/titiler/'), createObject('name', 'BLOB_CONTAINER', 'value', 'data'), createObject('name', 'BLOB_ACCOUNT_URL', 'value', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.blob), createObject('name', 'QUEUE_ACCOUNT_URL', 'value', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.queue), createObject('name', 'BLOB_CONNECTION_STRING', 'value', format('DefaultEndpointsProtocol=https;AccountName={0};AccountKey={1};EndpointSuffix={2}', parameters('storageAccountName'), listKeys(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').keys[0].value, environment().suffixes.storage)), createObject('name', 'AZURE_BATCH_ACCOUNT_NAME', 'value', parameters('batchAccountName')), createObject('name', 'AZURE_BATCH_URL', 'value', format('https://{0}.{1}.batch.azure.com', parameters('batchAccountName'), parameters('batchAccountLocation'))), createObject('name', 'AZURE_BATCH_ACCOUNT_KEY', 'value', parameters('batchAccountKey')), createObject('name', 'AZURE_BATCH_DOCKER_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('trainingImage'))), createObject('name', 'AZURE_BATCH_IMAGERYPREP_DOCKER_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('imageryprepImage'))), createObject('name', 'AZURE_BATCH_OUTPUT_CONTAINER_URL', 'value', format('{0}data', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.blob)), createObject('name', 'AZURE_BATCH_TRAINING_POOL_ID', 'value', parameters('batchPoolName')), createObject('name', 'AZURE_BATCH_IMAGERYPREP_POOL_ID', 'value', parameters('batchPoolName')), createObject('name', 'AZURE_BATCH_REGISTRY_SERVER', 'value', parameters('acrLoginServer')), createObject('name', 'AZURE_BATCH_REGISTRY_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('trainingImage'))), createObject('name', 'AZURE_BATCH_REGISTRY_IDENTITY_RESOURCE_ID', 'value', parameters('umiResourceId')), createObject('name', 'STATIC_APP_SUBSCRIPTION_ID', 'value', subscription().subscriptionId), createObject('name', 'STATIC_APP_RESOURCE_GROUP', 'value', resourceGroup().name), createObject('name', 'STATIC_APP_NAME', 'value', parameters('staticWebAppName')), createObject('name', 'STATIC_APP_DOMAIN', 'value', parameters('staticAppDomain')), createObject('name', 'EMAIL_CONNECTION_STRING', 'value', parameters('emailConnectionString')), createObject('name', 'EMAIL_SENDER', 'value', format('DoNotReply@{0}', parameters('emailSenderDomain'))), createObject('name', 'AZURE_BATCH_TRAINING_POOL_IDS', 'value', parameters('trainingPoolIds')), createObject('name', 'AZURE_BATCH_INFERENCE_POOL_IDS', 'value', parameters('inferencePoolIds')), createObject('name', 'AZURE_BATCH_IMAGERYPREP_POOL_IDS', 'value', parameters('imageryprepPoolIds')), createObject('name', 'AZURE_BATCH_USE_SAS', 'value', if(parameters('useSas'), 'true', 'false')), createObject('name', 'AZURE_BATCH_MANAGE_POOLS', 'value', if(parameters('managePools'), 'true', 'false')), createObject('name', 'PUBLISHING_ENABLED', 'value', if(parameters('publishingEnabled'), 'true', 'false')), createObject('name', 'PC_PROVIDER_ENABLED', 'value', if(parameters('pcProviderEnabled'), 'true', 'false')), createObject('name', 'PC_GEOCATALOG_URL', 'value', parameters('pcGeocatalogUrl')), createObject('name', 'PC_EXPLORER_URL', 'value', parameters('pcExplorerUrl')), createObject('name', 'PC_INGESTION_SOURCE', 'value', parameters('pcIngestionSource')), createObject('name', 'PC_COLLECTION_PREFIX', 'value', parameters('pcCollectionPrefix')), createObject('name', 'PC_PUBLISHING_LICENSE', 'value', parameters('pcPublishingLicense')), createObject('name', 'PUBLISHING_ORGANIZATION_NAME', 'value', parameters('publishingOrganizationName')), createObject('name', 'PUBLISHING_ORGANIZATION_URL', 'value', parameters('publishingOrganizationUrl')), createObject('name', 'PUBLISH_STORAGE_ACCOUNT_URL', 'value', parameters('publishStorageAccountUrl')), createObject('name', 'PUBLISH_BLOB_CONTAINER', 'value', parameters('publishBlobContainer')), createObject('name', 'PUBLISH_EXPLORER_RENDER_ENABLED', 'value', if(parameters('publishExplorerRenderEnabled'), 'true', 'false'))), createArray(createObject('name', 'HASTE_MAX_UPLOAD_BYTES', 'value', string(parameters('maxUploadBytes'))), createObject('name', 'PUBLISH_ASSESSMENT_MAX_TOTAL_BYTES', 'value', string(parameters('publishAssessmentMaxTotalBytes'))), createObject('name', 'AzureFunctionsWebHost__hostId', 'value', toLower(substring(parameters('functionApiName'), 0, min(32, length(parameters('functionApiName'))))))))]" + "value": "[concat(createArray(createObject('name', 'env', 'value', 'prod'), createObject('name', 'DEVELOPMENT_MODE', 'value', if(parameters('developmentMode'), 'true', 'false')), createObject('name', 'IMAGE_QUEUE_NAME', 'value', 'image-layers-queue'), createObject('name', 'INFERENCE_QUEUE_NAME', 'value', 'inference-queue'), createObject('name', 'STATS_QUEUE_NAME', 'value', 'stats-queue'), createObject('name', 'TRAIN_QUEUE_NAME', 'value', 'train-queue'), createObject('name', 'ZIP_QUEUE_NAME', 'value', 'zip-queue'), createObject('name', 'EMBEDDING_QUEUE_NAME', 'value', 'embedding-queue'), createObject('name', 'PUBLISH_QUEUE_NAME', 'value', 'publish-queue'), createObject('name', 'IMAGERY_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'METADATA_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'ARTIFACT_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'RUNNER_TYPE', 'value', 'azure_batch'), createObject('name', 'TEMP_DATA_PATH', 'value', '/data'), createObject('name', 'DATA_PATH', 'value', '/data'), createObject('name', 'TITILER_ENDPOINT', 'value', '/api/titiler/'), createObject('name', 'BLOB_CONTAINER', 'value', 'data'), createObject('name', 'BLOB_ACCOUNT_URL', 'value', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.blob), createObject('name', 'QUEUE_ACCOUNT_URL', 'value', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.queue), createObject('name', 'AZURE_BATCH_ACCOUNT_NAME', 'value', parameters('batchAccountName')), createObject('name', 'AZURE_BATCH_URL', 'value', format('https://{0}.{1}.batch.azure.com', parameters('batchAccountName'), parameters('batchAccountLocation'))), createObject('name', 'AZURE_BATCH_ACCOUNT_KEY', 'value', parameters('batchAccountKey')), createObject('name', 'AZURE_BATCH_DOCKER_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('trainingImage'))), createObject('name', 'AZURE_BATCH_IMAGERYPREP_DOCKER_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('imageryprepImage'))), createObject('name', 'AZURE_BATCH_OUTPUT_CONTAINER_URL', 'value', format('{0}data', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.blob)), createObject('name', 'AZURE_BATCH_TRAINING_POOL_ID', 'value', parameters('batchPoolName')), createObject('name', 'AZURE_BATCH_IMAGERYPREP_POOL_ID', 'value', parameters('batchPoolName')), createObject('name', 'AZURE_BATCH_REGISTRY_SERVER', 'value', parameters('acrLoginServer')), createObject('name', 'AZURE_BATCH_REGISTRY_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('trainingImage'))), createObject('name', 'AZURE_BATCH_REGISTRY_IDENTITY_RESOURCE_ID', 'value', parameters('umiResourceId')), createObject('name', 'STATIC_APP_SUBSCRIPTION_ID', 'value', subscription().subscriptionId), createObject('name', 'STATIC_APP_RESOURCE_GROUP', 'value', resourceGroup().name), createObject('name', 'STATIC_APP_NAME', 'value', parameters('staticWebAppName')), createObject('name', 'STATIC_APP_DOMAIN', 'value', parameters('staticAppDomain')), createObject('name', 'EMAIL_CONNECTION_STRING', 'value', parameters('emailConnectionString')), createObject('name', 'EMAIL_SENDER', 'value', format('DoNotReply@{0}', parameters('emailSenderDomain'))), createObject('name', 'AZURE_BATCH_TRAINING_POOL_IDS', 'value', parameters('trainingPoolIds')), createObject('name', 'AZURE_BATCH_INFERENCE_POOL_IDS', 'value', parameters('inferencePoolIds')), createObject('name', 'AZURE_BATCH_IMAGERYPREP_POOL_IDS', 'value', parameters('imageryprepPoolIds')), createObject('name', 'AZURE_BATCH_USE_SAS', 'value', if(parameters('useSas'), 'true', 'false')), createObject('name', 'AZURE_BATCH_MANAGE_POOLS', 'value', if(parameters('managePools'), 'true', 'false')), createObject('name', 'PUBLISHING_ENABLED', 'value', if(parameters('publishingEnabled'), 'true', 'false')), createObject('name', 'PC_PROVIDER_ENABLED', 'value', if(parameters('pcProviderEnabled'), 'true', 'false')), createObject('name', 'PC_GEOCATALOG_URL', 'value', parameters('pcGeocatalogUrl')), createObject('name', 'PC_EXPLORER_URL', 'value', parameters('pcExplorerUrl')), createObject('name', 'PC_INGESTION_SOURCE', 'value', parameters('pcIngestionSource')), createObject('name', 'PC_COLLECTION_PREFIX', 'value', parameters('pcCollectionPrefix')), createObject('name', 'PC_PUBLISHING_LICENSE', 'value', parameters('pcPublishingLicense')), createObject('name', 'PUBLISHING_ORGANIZATION_NAME', 'value', parameters('publishingOrganizationName')), createObject('name', 'PUBLISHING_ORGANIZATION_URL', 'value', parameters('publishingOrganizationUrl')), createObject('name', 'PUBLISH_STORAGE_ACCOUNT_URL', 'value', parameters('publishStorageAccountUrl')), createObject('name', 'PUBLISH_BLOB_CONTAINER', 'value', parameters('publishBlobContainer')), createObject('name', 'PUBLISH_EXPLORER_RENDER_ENABLED', 'value', if(parameters('publishExplorerRenderEnabled'), 'true', 'false'))), createArray(createObject('name', 'HASTE_MAX_UPLOAD_BYTES', 'value', string(parameters('maxUploadBytes'))), createObject('name', 'PUBLISH_ASSESSMENT_MAX_TOTAL_BYTES', 'value', string(parameters('publishAssessmentMaxTotalBytes'))), createObject('name', 'AzureFunctionsWebHost__hostId', 'value', toLower(substring(parameters('functionApiName'), 0, min(32, length(parameters('functionApiName'))))))))]" }, "tags": { "value": "[parameters('tags')]" @@ -2656,7 +2656,7 @@ "value": false }, "appSettings": { - "value": "[concat(createArray(createObject('name', 'env', 'value', 'prod'), createObject('name', 'DEVELOPMENT_MODE', 'value', if(parameters('developmentMode'), 'true', 'false')), createObject('name', 'IMAGE_QUEUE_NAME', 'value', 'image-layers-queue'), createObject('name', 'INFERENCE_QUEUE_NAME', 'value', 'inference-queue'), createObject('name', 'STATS_QUEUE_NAME', 'value', 'stats-queue'), createObject('name', 'TRAIN_QUEUE_NAME', 'value', 'train-queue'), createObject('name', 'ZIP_QUEUE_NAME', 'value', 'zip-queue'), createObject('name', 'EMBEDDING_QUEUE_NAME', 'value', 'embedding-queue'), createObject('name', 'PUBLISH_QUEUE_NAME', 'value', 'publish-queue'), createObject('name', 'IMAGERY_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'METADATA_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'ARTIFACT_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'RUNNER_TYPE', 'value', 'azure_batch'), createObject('name', 'TEMP_DATA_PATH', 'value', '/data'), createObject('name', 'DATA_PATH', 'value', '/data'), createObject('name', 'TITILER_ENDPOINT', 'value', '/api/titiler/'), createObject('name', 'BLOB_CONTAINER', 'value', 'data'), createObject('name', 'BLOB_ACCOUNT_URL', 'value', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.blob), createObject('name', 'QUEUE_ACCOUNT_URL', 'value', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.queue), createObject('name', 'BLOB_CONNECTION_STRING', 'value', format('DefaultEndpointsProtocol=https;AccountName={0};AccountKey={1};EndpointSuffix={2}', parameters('storageAccountName'), listKeys(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').keys[0].value, environment().suffixes.storage)), createObject('name', 'AZURE_BATCH_ACCOUNT_NAME', 'value', parameters('batchAccountName')), createObject('name', 'AZURE_BATCH_URL', 'value', format('https://{0}.{1}.batch.azure.com', parameters('batchAccountName'), parameters('batchAccountLocation'))), createObject('name', 'AZURE_BATCH_ACCOUNT_KEY', 'value', parameters('batchAccountKey')), createObject('name', 'AZURE_BATCH_DOCKER_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('trainingImage'))), createObject('name', 'AZURE_BATCH_IMAGERYPREP_DOCKER_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('imageryprepImage'))), createObject('name', 'AZURE_BATCH_OUTPUT_CONTAINER_URL', 'value', format('{0}data', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.blob)), createObject('name', 'AZURE_BATCH_TRAINING_POOL_ID', 'value', parameters('batchPoolName')), createObject('name', 'AZURE_BATCH_IMAGERYPREP_POOL_ID', 'value', parameters('batchPoolName')), createObject('name', 'AZURE_BATCH_REGISTRY_SERVER', 'value', parameters('acrLoginServer')), createObject('name', 'AZURE_BATCH_REGISTRY_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('trainingImage'))), createObject('name', 'AZURE_BATCH_REGISTRY_IDENTITY_RESOURCE_ID', 'value', parameters('umiResourceId')), createObject('name', 'STATIC_APP_SUBSCRIPTION_ID', 'value', subscription().subscriptionId), createObject('name', 'STATIC_APP_RESOURCE_GROUP', 'value', resourceGroup().name), createObject('name', 'STATIC_APP_NAME', 'value', parameters('staticWebAppName')), createObject('name', 'STATIC_APP_DOMAIN', 'value', parameters('staticAppDomain')), createObject('name', 'EMAIL_CONNECTION_STRING', 'value', parameters('emailConnectionString')), createObject('name', 'EMAIL_SENDER', 'value', format('DoNotReply@{0}', parameters('emailSenderDomain'))), createObject('name', 'AZURE_BATCH_TRAINING_POOL_IDS', 'value', parameters('trainingPoolIds')), createObject('name', 'AZURE_BATCH_INFERENCE_POOL_IDS', 'value', parameters('inferencePoolIds')), createObject('name', 'AZURE_BATCH_IMAGERYPREP_POOL_IDS', 'value', parameters('imageryprepPoolIds')), createObject('name', 'AZURE_BATCH_USE_SAS', 'value', if(parameters('useSas'), 'true', 'false')), createObject('name', 'AZURE_BATCH_MANAGE_POOLS', 'value', if(parameters('managePools'), 'true', 'false')), createObject('name', 'PUBLISHING_ENABLED', 'value', if(parameters('publishingEnabled'), 'true', 'false')), createObject('name', 'PC_PROVIDER_ENABLED', 'value', if(parameters('pcProviderEnabled'), 'true', 'false')), createObject('name', 'PC_GEOCATALOG_URL', 'value', parameters('pcGeocatalogUrl')), createObject('name', 'PC_EXPLORER_URL', 'value', parameters('pcExplorerUrl')), createObject('name', 'PC_INGESTION_SOURCE', 'value', parameters('pcIngestionSource')), createObject('name', 'PC_COLLECTION_PREFIX', 'value', parameters('pcCollectionPrefix')), createObject('name', 'PC_PUBLISHING_LICENSE', 'value', parameters('pcPublishingLicense')), createObject('name', 'PUBLISHING_ORGANIZATION_NAME', 'value', parameters('publishingOrganizationName')), createObject('name', 'PUBLISHING_ORGANIZATION_URL', 'value', parameters('publishingOrganizationUrl')), createObject('name', 'PUBLISH_STORAGE_ACCOUNT_URL', 'value', parameters('publishStorageAccountUrl')), createObject('name', 'PUBLISH_BLOB_CONTAINER', 'value', parameters('publishBlobContainer')), createObject('name', 'PUBLISH_EXPLORER_RENDER_ENABLED', 'value', if(parameters('publishExplorerRenderEnabled'), 'true', 'false'))), createArray(createObject('name', 'HASTE_MAX_IMAGERY_DOWNLOAD_BYTES', 'value', string(parameters('maxImageryDownloadBytes'))), createObject('name', 'AzureFunctionsWebHost__hostId', 'value', toLower(substring(parameters('functionQueueName'), 0, min(32, length(parameters('functionQueueName'))))))))]" + "value": "[concat(createArray(createObject('name', 'env', 'value', 'prod'), createObject('name', 'DEVELOPMENT_MODE', 'value', if(parameters('developmentMode'), 'true', 'false')), createObject('name', 'IMAGE_QUEUE_NAME', 'value', 'image-layers-queue'), createObject('name', 'INFERENCE_QUEUE_NAME', 'value', 'inference-queue'), createObject('name', 'STATS_QUEUE_NAME', 'value', 'stats-queue'), createObject('name', 'TRAIN_QUEUE_NAME', 'value', 'train-queue'), createObject('name', 'ZIP_QUEUE_NAME', 'value', 'zip-queue'), createObject('name', 'EMBEDDING_QUEUE_NAME', 'value', 'embedding-queue'), createObject('name', 'PUBLISH_QUEUE_NAME', 'value', 'publish-queue'), createObject('name', 'IMAGERY_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'METADATA_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'ARTIFACT_STORAGE_TYPE', 'value', 'blob'), createObject('name', 'RUNNER_TYPE', 'value', 'azure_batch'), createObject('name', 'TEMP_DATA_PATH', 'value', '/data'), createObject('name', 'DATA_PATH', 'value', '/data'), createObject('name', 'TITILER_ENDPOINT', 'value', '/api/titiler/'), createObject('name', 'BLOB_CONTAINER', 'value', 'data'), createObject('name', 'BLOB_ACCOUNT_URL', 'value', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.blob), createObject('name', 'QUEUE_ACCOUNT_URL', 'value', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.queue), createObject('name', 'AZURE_BATCH_ACCOUNT_NAME', 'value', parameters('batchAccountName')), createObject('name', 'AZURE_BATCH_URL', 'value', format('https://{0}.{1}.batch.azure.com', parameters('batchAccountName'), parameters('batchAccountLocation'))), createObject('name', 'AZURE_BATCH_ACCOUNT_KEY', 'value', parameters('batchAccountKey')), createObject('name', 'AZURE_BATCH_DOCKER_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('trainingImage'))), createObject('name', 'AZURE_BATCH_IMAGERYPREP_DOCKER_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('imageryprepImage'))), createObject('name', 'AZURE_BATCH_OUTPUT_CONTAINER_URL', 'value', format('{0}data', reference(resourceId('Microsoft.Storage/storageAccounts', parameters('storageAccountName')), '2023-05-01').primaryEndpoints.blob)), createObject('name', 'AZURE_BATCH_TRAINING_POOL_ID', 'value', parameters('batchPoolName')), createObject('name', 'AZURE_BATCH_IMAGERYPREP_POOL_ID', 'value', parameters('batchPoolName')), createObject('name', 'AZURE_BATCH_REGISTRY_SERVER', 'value', parameters('acrLoginServer')), createObject('name', 'AZURE_BATCH_REGISTRY_IMAGE', 'value', format('{0}/{1}', parameters('acrLoginServer'), parameters('trainingImage'))), createObject('name', 'AZURE_BATCH_REGISTRY_IDENTITY_RESOURCE_ID', 'value', parameters('umiResourceId')), createObject('name', 'STATIC_APP_SUBSCRIPTION_ID', 'value', subscription().subscriptionId), createObject('name', 'STATIC_APP_RESOURCE_GROUP', 'value', resourceGroup().name), createObject('name', 'STATIC_APP_NAME', 'value', parameters('staticWebAppName')), createObject('name', 'STATIC_APP_DOMAIN', 'value', parameters('staticAppDomain')), createObject('name', 'EMAIL_CONNECTION_STRING', 'value', parameters('emailConnectionString')), createObject('name', 'EMAIL_SENDER', 'value', format('DoNotReply@{0}', parameters('emailSenderDomain'))), createObject('name', 'AZURE_BATCH_TRAINING_POOL_IDS', 'value', parameters('trainingPoolIds')), createObject('name', 'AZURE_BATCH_INFERENCE_POOL_IDS', 'value', parameters('inferencePoolIds')), createObject('name', 'AZURE_BATCH_IMAGERYPREP_POOL_IDS', 'value', parameters('imageryprepPoolIds')), createObject('name', 'AZURE_BATCH_USE_SAS', 'value', if(parameters('useSas'), 'true', 'false')), createObject('name', 'AZURE_BATCH_MANAGE_POOLS', 'value', if(parameters('managePools'), 'true', 'false')), createObject('name', 'PUBLISHING_ENABLED', 'value', if(parameters('publishingEnabled'), 'true', 'false')), createObject('name', 'PC_PROVIDER_ENABLED', 'value', if(parameters('pcProviderEnabled'), 'true', 'false')), createObject('name', 'PC_GEOCATALOG_URL', 'value', parameters('pcGeocatalogUrl')), createObject('name', 'PC_EXPLORER_URL', 'value', parameters('pcExplorerUrl')), createObject('name', 'PC_INGESTION_SOURCE', 'value', parameters('pcIngestionSource')), createObject('name', 'PC_COLLECTION_PREFIX', 'value', parameters('pcCollectionPrefix')), createObject('name', 'PC_PUBLISHING_LICENSE', 'value', parameters('pcPublishingLicense')), createObject('name', 'PUBLISHING_ORGANIZATION_NAME', 'value', parameters('publishingOrganizationName')), createObject('name', 'PUBLISHING_ORGANIZATION_URL', 'value', parameters('publishingOrganizationUrl')), createObject('name', 'PUBLISH_STORAGE_ACCOUNT_URL', 'value', parameters('publishStorageAccountUrl')), createObject('name', 'PUBLISH_BLOB_CONTAINER', 'value', parameters('publishBlobContainer')), createObject('name', 'PUBLISH_EXPLORER_RENDER_ENABLED', 'value', if(parameters('publishExplorerRenderEnabled'), 'true', 'false'))), createArray(createObject('name', 'HASTE_MAX_IMAGERY_DOWNLOAD_BYTES', 'value', string(parameters('maxImageryDownloadBytes'))), createObject('name', 'AzureFunctionsWebHost__hostId', 'value', toLower(substring(parameters('functionQueueName'), 0, min(32, length(parameters('functionQueueName'))))))))]" }, "tags": { "value": "[parameters('tags')]" diff --git a/infra/modules/functions.bicep b/infra/modules/functions.bicep index 35509ebd..07277712 100644 --- a/infra/modules/functions.bicep +++ b/infra/modules/functions.bicep @@ -151,8 +151,7 @@ param publishBlobContainer string = '' @description('Resource tags.') param tags object = {} -// Storage account (env RG) — used to derive the blob/queue URLs and the -// key-based BLOB_CONNECTION_STRING that hastegeo's utils/blob.py path requires. +// Storage account (env RG) — used to derive the blob and queue URLs. resource storageAccount 'Microsoft.Storage/storageAccounts@2023-05-01' existing = { name: storageAccountName } @@ -167,8 +166,8 @@ var titilerStorageHost = split(storageAccount.properties.primaryEndpoints.blob, // Application settings shared by the api and queues apps. hastegeo's Config() // (instantiated at import time) reads these, so they must be present or the // worker cannot index function_app.py. Storage/queue access is identity-based -// (BLOB_ACCOUNT_URL + role grants); only BLOB_CONNECTION_STRING, the Batch key, -// and the ACS string are key-based (per ADR: no Key Vault, deploy-time values). +// (BLOB_ACCOUNT_URL + role grants); only the Batch key and the ACS string are +// key-based (per ADR: no Key Vault, deploy-time values). var appConfigSettings = [ { name: 'env', value: 'prod' } { name: 'DEVELOPMENT_MODE', value: developmentMode ? 'true' : 'false' } @@ -189,10 +188,6 @@ var appConfigSettings = [ { name: 'BLOB_CONTAINER', value: 'data' } { name: 'BLOB_ACCOUNT_URL', value: storageAccount.properties.primaryEndpoints.blob } { name: 'QUEUE_ACCOUNT_URL', value: storageAccount.properties.primaryEndpoints.queue } - { - name: 'BLOB_CONNECTION_STRING' - value: 'DefaultEndpointsProtocol=https;AccountName=${storageAccountName};AccountKey=${storageAccount.listKeys().keys[0].value};EndpointSuffix=${environment().suffixes.storage}' - } { name: 'AZURE_BATCH_ACCOUNT_NAME', value: batchAccountName } { name: 'AZURE_BATCH_URL', value: 'https://${batchAccountName}.${batchAccountLocation}.batch.azure.com' } { name: 'AZURE_BATCH_ACCOUNT_KEY', value: batchAccountKey } From 3bbd001ac0a3d32fdaf4c0c809de62927a83b988 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 10:17:37 -0300 Subject: [PATCH 07/24] feat(blob): sign internal blob URLs at response time Adds sign_internal_blob_url, sign_titiler_tile_url and sign_label_project_urls so stored URLs can stay unsigned and be given a fresh per-blob read token when handed out. --- hastelib/src/hastegeo/core/utils/blob.py | 104 ++++++++++++- .../tests/core/utils/test_blob_signing.py | 139 ++++++++++++++++++ 2 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 hastelib/tests/core/utils/test_blob_signing.py diff --git a/hastelib/src/hastegeo/core/utils/blob.py b/hastelib/src/hastegeo/core/utils/blob.py index f74c0bfb..af9d108e 100644 --- a/hastelib/src/hastegeo/core/utils/blob.py +++ b/hastelib/src/hastegeo/core/utils/blob.py @@ -28,7 +28,7 @@ from collections import OrderedDict from datetime import datetime, timedelta, timezone from typing import NamedTuple, Optional, Tuple -from urllib.parse import urlparse +from urllib.parse import quote, unquote, urlparse _USER_DELEGATION_KEYS = OrderedDict() _USER_DELEGATION_KEY_CACHE_SIZE = 8 @@ -242,6 +242,108 @@ def download() -> str: return await asyncio.to_thread(download) +def blob_url_without_query(url: Optional[str]) -> Optional[str]: + """Return ``url`` without its query string, or ``None`` for a falsy url. + + Stored blob URLs are kept unsigned: tokens are short-lived and minted + when a URL is handed out. + """ + if not url: + return None + return url.split("?", 1)[0] + + +def _internal_blob_client(url: str): + """Return the app's blob client when ``url`` points at its own storage.""" + if not ( + os.environ.get("BLOB_CONNECTION_STRING") + or os.environ.get("BLOB_ACCOUNT_URL") + ): + return None + candidate = urlparse(url) + if candidate.scheme not in {"http", "https"} or not candidate.hostname: + return None + client = get_app_blob_service_client() + account = urlparse(client.url) + if (candidate.scheme, candidate.hostname.lower(), candidate.port) != ( + account.scheme, + (account.hostname or "").lower(), + account.port, + ): + return None + return client + + +def sign_internal_blob_url(url: Optional[str], minutes: int = 60): + """Return ``url`` with a fresh read-only token when it is internal. + + Only blobs in the app's own storage account and ``BLOB_CONTAINER`` are + signed, with any old query replaced by a token scoped to that one blob. + Anything else (provider URLs, other accounts or containers) is returned + untouched. An account key (Azurite, dev) signs directly; otherwise the + app identity's cached user delegation key does. + """ + if not url: + return url + client = _internal_blob_client(url) + if client is None: + return url + from .blob_sas import DELEGATION_KEY_MARGIN, blob_read_sas + + container_name, blob_name = split_blob_url(blob_url_without_query(url)) + if container_name != os.environ.get("BLOB_CONTAINER", "data"): + return url + blob_name = unquote(blob_name) + credential = getattr(client, "credential", None) + account_key = getattr(credential, "account_key", None) + user_delegation_key = None + if not account_key: + user_delegation_key = get_cached_user_delegation_key( + client, min_remaining=DELEGATION_KEY_MARGIN + ) + token = blob_read_sas( + account_name=client.account_name, + container_name=container_name, + blob_name=blob_name, + account_key=account_key, + user_delegation_key=user_delegation_key, + minutes=minutes, + ) + blob_url = client.get_blob_client(container_name, blob_name).url + return f"{blob_url}?{token}" + + +_TILE_SOURCE_PARAM = re.compile(r"(?<=[?&]url=)[^&#]*") + + +def sign_titiler_tile_url(tile_url: Optional[str]) -> Optional[str]: + """Sign the blob URL nested in a TiTiler tile URL's ``url`` parameter.""" + if not tile_url: + return tile_url + + def _sign(match: re.Match) -> str: + source = unquote(match.group(0)) + signed = sign_internal_blob_url(source) + return quote(signed, safe="") if signed != source else match.group(0) + + return _TILE_SOURCE_PARAM.sub(_sign, tile_url) + + +_LABEL_TILE_FIELDS = ("tileUrl", "preEventTileUrl", "postEventTileUrl") + + +def sign_label_project_urls(label_project: dict) -> dict: + """Return a copy of a label project with its tile URLs freshly signed.""" + imagery = label_project.get("imagery") + if not isinstance(imagery, dict): + return label_project + signed = dict(imagery) + for field in _LABEL_TILE_FIELDS: + if signed.get(field): + signed[field] = sign_titiler_tile_url(signed[field]) + return {**label_project, "imagery": signed} + + class BlobRange(NamedTuple): """A slice of a blob plus the metadata needed to build HTTP headers.""" diff --git a/hastelib/tests/core/utils/test_blob_signing.py b/hastelib/tests/core/utils/test_blob_signing.py new file mode 100644 index 00000000..74a35371 --- /dev/null +++ b/hastelib/tests/core/utils/test_blob_signing.py @@ -0,0 +1,139 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +import os +import unittest +from unittest.mock import MagicMock, patch +from urllib.parse import quote + +from hastegeo.core.utils.blob import ( + blob_url_without_query, + clear_blob_client_caches, + sign_label_project_urls, + sign_internal_blob_url, + sign_titiler_tile_url, +) + +ACCOUNT_URL = "https://acct.blob.core.windows.net" +BLOB_URL = f"{ACCOUNT_URL}/data/p1/layer/post.tif" + + +def _identity_client(): + client = MagicMock() + client.url = f"{ACCOUNT_URL}/" + client.account_name = "acct" + client.credential = object() + client.get_blob_client.side_effect = lambda container, blob: MagicMock( + url=f"{ACCOUNT_URL}/{container}/{blob}" + ) + return client + + +class TestBlobUrlWithoutQuery(unittest.TestCase): + def test_strips_query(self): + self.assertEqual( + blob_url_without_query(f"{BLOB_URL}?sv=1&sig=x"), BLOB_URL + ) + + def test_no_query_and_empty(self): + self.assertEqual(blob_url_without_query(BLOB_URL), BLOB_URL) + self.assertIsNone(blob_url_without_query(None)) + self.assertIsNone(blob_url_without_query("")) + + +class TestSignInternalBlobUrl(unittest.TestCase): + def setUp(self): + clear_blob_client_caches() + env = patch.dict( + os.environ, + {"BLOB_ACCOUNT_URL": ACCOUNT_URL, "BLOB_CONTAINER": "data"}, + clear=True, + ) + env.start() + self.addCleanup(env.stop) + self.client = _identity_client() + for target in ( + "hastegeo.core.utils.blob.get_app_blob_service_client", + "hastegeo.core.utils.blob.get_cached_user_delegation_key", + ): + patcher = patch(target) + mock = patcher.start() + self.addCleanup(patcher.stop) + if target.endswith("client"): + mock.return_value = self.client + self.sas = patch( + "hastegeo.core.utils.blob_sas.generate_blob_sas", + return_value="sv=1&sp=r&sig=fresh", + ) + self.generate = self.sas.start() + self.addCleanup(self.sas.stop) + + def test_signs_internal_url_replacing_old_query(self): + signed = sign_internal_blob_url(f"{BLOB_URL}?sv=old&sig=stale") + self.assertEqual(signed, f"{BLOB_URL}?sv=1&sp=r&sig=fresh") + kwargs = self.generate.call_args.kwargs + self.assertEqual(kwargs["blob_name"], "p1/layer/post.tif") + self.assertIsNone(kwargs["account_key"]) + self.assertIsNotNone(kwargs["user_delegation_key"]) + + def test_external_and_other_container_pass_through(self): + external = "https://provider.example.com/a.tif?token=1" + other = f"{ACCOUNT_URL}/other/a.tif?sv=1" + self.assertEqual(sign_internal_blob_url(external), external) + self.assertEqual(sign_internal_blob_url(other), other) + self.generate.assert_not_called() + + def test_other_account_passes_through(self): + url = "https://other.blob.core.windows.net/data/a.tif" + self.assertEqual(sign_internal_blob_url(url), url) + + def test_account_key_credential_signs_with_key(self): + self.client.credential = MagicMock(account_key="k") + sign_internal_blob_url(BLOB_URL) + self.assertEqual(self.generate.call_args.kwargs["account_key"], "k") + + +class TestSignTilerUrls(unittest.TestCase): + def setUp(self): + patcher = patch( + "hastegeo.core.utils.blob.sign_internal_blob_url", + side_effect=lambda url: ( + f"{url}?sig=fresh" if "acct" in url else url + ), + ) + patcher.start() + self.addCleanup(patcher.stop) + + def _tile(self, source): + return ( + "/api/titiler/cog/tiles/WebMercatorQuad/{z}/{x}/{y}" + f"?scale=1&url={quote(source, safe='')}&colormap=x" + ) + + def test_signs_only_nested_source(self): + source = f"{BLOB_URL}?sig=old" + result = sign_titiler_tile_url(self._tile(source)) + self.assertIn("{z}/{x}/{y}", result) + self.assertTrue(result.endswith("&colormap=x")) + self.assertIn(quote("?sig=fresh", safe=""), result) + + def test_external_source_unchanged(self): + tile = self._tile("https://provider.example.com/a.tif") + self.assertEqual(sign_titiler_tile_url(tile), tile) + + def test_label_project_fields_signed_without_mutation(self): + original = { + "imagery": { + "tileUrl": self._tile(BLOB_URL), + "preEventTileUrl": self._tile(BLOB_URL), + "postEventTileUrl": "", + } + } + result = sign_label_project_urls(original) + self.assertIn("fresh", result["imagery"]["tileUrl"]) + self.assertIn("fresh", result["imagery"]["preEventTileUrl"]) + self.assertEqual(result["imagery"]["postEventTileUrl"], "") + self.assertNotIn("fresh", original["imagery"]["tileUrl"]) + + +if __name__ == "__main__": + unittest.main() From 33f10c33a8e49352bab89de65bb92accf8e04631 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 10:18:41 -0300 Subject: [PATCH 08/24] fix(storage): keep stored blob URLs unsigned and sign when served Label tile URLs and visualizer layers no longer embed a token in persisted data; labels sign only around their own raster read. The label project, labeling workspace and project-details endpoints mint fresh per-blob tokens at response time, including for legacy stored URLs that carried a policy SAS. --- api/hastefuncapi/function_app.py | 15 +++++++- .../src/hastegeo/core/processors/labels.py | 6 ++- .../hastegeo/core/processors/visualizer.py | 37 +++++++++++-------- hastelib/src/hastegeo/core/utils/blob.py | 19 ++++++++++ .../tests/core/utils/test_blob_signing.py | 16 ++++++++ 5 files changed, 75 insertions(+), 18 deletions(-) diff --git a/api/hastefuncapi/function_app.py b/api/hastefuncapi/function_app.py index 90640836..d111332a 100644 --- a/api/hastefuncapi/function_app.py +++ b/api/hastefuncapi/function_app.py @@ -123,6 +123,8 @@ from hastegeo.core.utils.blob import ( download_blob_to_tempfile, parse_byte_range, + sign_label_project_urls, + sign_project_preview_urls, ) from hastegeo.core.utils.data import convert_json_to_geojson, filter_roles from hastegeo.core.utils.errors import exception_diagnostics @@ -991,7 +993,9 @@ async def _load_response(): _load_response, refresh=_cache_refresh_requested(req.headers.get("Cache-Control")), ) - _payload = cached_response["payload"] + _payload = await asyncio.to_thread( + sign_project_preview_urls, cached_response["payload"] + ) etag = cached_response["etag"] _perf_headers = perf.headers(_perf, _perf_wall) cache_headers = { @@ -1755,6 +1759,9 @@ async def GetLayerLabelingToolData(req: func.HttpRequest) -> func.HttpResponse: "Label project not found.", status_code=404 ) + match_label_projects = await asyncio.to_thread( + sign_label_project_urls, match_label_projects + ) return func.HttpResponse( json.dumps(match_label_projects), status_code=200 ) @@ -1796,8 +1803,12 @@ async def GetLabelingWorkspace(req: func.HttpRequest) -> func.HttpResponse: image_layer_id=image_layer_id, config=config, ).load() + payload = workspace.model_dump(mode="json") + payload["labelProject"] = await asyncio.to_thread( + sign_label_project_urls, payload["labelProject"] + ) return func.HttpResponse( - json.dumps(workspace.model_dump(mode="json")), + json.dumps(payload), status_code=200, mimetype="application/json", ) diff --git a/hastelib/src/hastegeo/core/processors/labels.py b/hastelib/src/hastegeo/core/processors/labels.py index e51e3e42..e372f1f4 100644 --- a/hastelib/src/hastegeo/core/processors/labels.py +++ b/hastelib/src/hastegeo/core/processors/labels.py @@ -15,6 +15,7 @@ LabelingImagery, LabelProject, ) +from ..utils.blob import blob_url_without_query, sign_internal_blob_url from ..utils.gdal_security import harden_gdal from ..utils.metadata import MetadataUtils @@ -76,6 +77,7 @@ def generate_task_file(self, geom: dict[str, any]) -> None: def get_xyz_url(self, imagery_url: str) -> str: """Generate XYZ URL for the url of a TIF file.""" xyz_url = "" + imagery_url = blob_url_without_query(imagery_url) if imagery_url: urlencoded_imagery_url = requests.utils.quote(imagery_url, safe="") xyz_url = f"{self.titiler_endpoint}cog/tiles/WebMercatorQuad/{{z}}/{{x}}/{{y}}?scale=1&url={urlencoded_imagery_url}" @@ -84,7 +86,9 @@ def get_xyz_url(self, imagery_url: str) -> str: def generate_task_files(self): """Generate task files.""" self.logger.info("Generating task file") - with rasterio.open(self.input_image_fn_post_event) as f: + with rasterio.open( + sign_internal_blob_url(self.input_image_fn_post_event) + ) as f: bounds = shapely.geometry.box(*f.bounds) geom = shapely.geometry.mapping(bounds) if f.crs.to_epsg() != 4326: diff --git a/hastelib/src/hastegeo/core/processors/visualizer.py b/hastelib/src/hastegeo/core/processors/visualizer.py index 6923c39d..b77236b4 100644 --- a/hastelib/src/hastegeo/core/processors/visualizer.py +++ b/hastelib/src/hastegeo/core/processors/visualizer.py @@ -10,6 +10,7 @@ from ..models.prediction_results import ResultsRequest from ..models.projects import ImageLayer, LabelProject, Model, Project from ..models.visualizer import Imagery, Visualizer +from ..utils.blob import blob_url_without_query, sign_internal_blob_url from ..utils.prediction_readiness import artifact_api_url from .metadata import MetadataProcessor from .prediction_results import PredictionResultsProcessor @@ -35,9 +36,11 @@ def build_visualizer( bounds = labels.features[0].bbox or [] if labels.features else [] def raster(url: str | None, colormap: str = "") -> Imagery: + url = blob_url_without_query(url) tile_url = ( f"{titiler_endpoint}cog/tiles/WebMercatorQuad/{{z}}/{{x}}/{{y}}" - f"?scale=1&url={quote(url, safe='')}{colormap}" + f"?scale=1&url={quote(sign_internal_blob_url(url), safe='')}" + f"{colormap}" if url else "" ) @@ -88,20 +91,24 @@ def raster(url: str | None, colormap: str = "") -> Imagery: preDisasterImagery=raster(layer.preEventProcessedImageryUrl), postDisasterImagery=raster(layer.postEventProcessedImageryUrl), predictedDamageLayer=raster(predicted_url) if predicted_url else None, - predictionsLayer=raster(classified_url, colormap) - if classified_url - else None, - footprintTilesUrl=artifact_api_url(model, "footprint_pmtiles") - if layer.footprintPmtilesUrl - else None, - predictionAttrsUrl=prediction_source_url( - model, source, "prediction_attrs" - ) - if source.predictionAttrsUrl - else None, - gpkgUrl=prediction_source_url(model, source, "gpkg") - if source.gpkgUrl - else None, + predictionsLayer=( + raster(classified_url, colormap) if classified_url else None + ), + footprintTilesUrl=( + artifact_api_url(model, "footprint_pmtiles") + if layer.footprintPmtilesUrl + else None + ), + predictionAttrsUrl=( + prediction_source_url(model, source, "prediction_attrs") + if source.predictionAttrsUrl + else None + ), + gpkgUrl=( + prediction_source_url(model, source, "gpkg") + if source.gpkgUrl + else None + ), **source.descriptor(), predictionVersions=prediction_versions(model, layer), supportsThreshold=flavor == "inference", diff --git a/hastelib/src/hastegeo/core/utils/blob.py b/hastelib/src/hastegeo/core/utils/blob.py index af9d108e..c2df4355 100644 --- a/hastelib/src/hastegeo/core/utils/blob.py +++ b/hastelib/src/hastegeo/core/utils/blob.py @@ -21,6 +21,7 @@ import asyncio import functools +import json import os import re import tempfile @@ -330,6 +331,24 @@ def _sign(match: re.Match) -> str: _LABEL_TILE_FIELDS = ("tileUrl", "preEventTileUrl", "postEventTileUrl") +_PREVIEW_FIELDS = ("preEventPreviewUrls", "postEventPreviewUrls") + + +def sign_project_preview_urls(payload: str) -> str: + """Sign image-layer preview URLs in a serialized project-details payload. + + Returns the input string untouched when no layer has previews. + """ + project = json.loads(payload) + layers = project.get("imageLayer") or [] + changed = False + for layer in layers: + for field in _PREVIEW_FIELDS: + urls = layer.get(field) + if urls: + layer[field] = [sign_internal_blob_url(url) for url in urls] + changed = True + return json.dumps(project) if changed else payload def sign_label_project_urls(label_project: dict) -> dict: diff --git a/hastelib/tests/core/utils/test_blob_signing.py b/hastelib/tests/core/utils/test_blob_signing.py index 74a35371..d7ecacb8 100644 --- a/hastelib/tests/core/utils/test_blob_signing.py +++ b/hastelib/tests/core/utils/test_blob_signing.py @@ -120,6 +120,22 @@ def test_external_source_unchanged(self): tile = self._tile("https://provider.example.com/a.tif") self.assertEqual(sign_titiler_tile_url(tile), tile) + def test_preview_urls_signed_in_project_payload(self): + import json + + from hastegeo.core.utils.blob import sign_project_preview_urls + + payload = json.dumps( + {"imageLayer": [{"preEventPreviewUrls": [BLOB_URL]}, {}]} + ) + result = json.loads(sign_project_preview_urls(payload)) + self.assertEqual( + result["imageLayer"][0]["preEventPreviewUrls"], + [f"{BLOB_URL}?sig=fresh"], + ) + empty = json.dumps({"imageLayer": [{}]}) + self.assertEqual(sign_project_preview_urls(empty), empty) + def test_label_project_fields_signed_without_mutation(self): original = { "imagery": { From 68317cc7fba762fb39d9eeafa10f06b14d592e3f Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Tue, 27 Oct 2026 10:19:03 -0300 Subject: [PATCH 09/24] docs(spec): describe unsigned stored URLs signed at response time --- .../0008-identity-only-blob-access.md | 7 ++--- .../identity-only-blob-access/design.md | 29 ++++++++++++++----- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/spec/architecture/decisions/0008-identity-only-blob-access.md b/spec/architecture/decisions/0008-identity-only-blob-access.md index 842db6d1..27987e3b 100644 --- a/spec/architecture/decisions/0008-identity-only-blob-access.md +++ b/spec/architecture/decisions/0008-identity-only-blob-access.md @@ -24,8 +24,7 @@ identity-based signing cannot reuse this design. - Each link reads one blob for at most an hour and is signed by the app identity. - Revocation comes from the delegation key, so there is no policy to maintain. -- Persisted tokens no longer outlive their usefulness; browsers already go - through the identity-authenticated storage proxy. +- Persisted URLs stay unsigned; tokens are minted when URLs are served. ## Decision @@ -37,7 +36,7 @@ emulator. Disable shared-key access on the data account afterwards. ## Consequences - Links minted before the change keep working until their policy is deleted. -- Tokens are shorter-lived, so anything that stores a token for later use must - read by path or go through the proxy. +- Tokens are shorter-lived, so stored URLs are kept unsigned and signed when + served; anything else must read by path or go through the proxy. - Deployment config no longer needs the account key. - The Azure Files mount keeps its key; this decision does not cover it. diff --git a/spec/features/identity-only-blob-access/design.md b/spec/features/identity-only-blob-access/design.md index c88df63a..5a5e436b 100644 --- a/spec/features/identity-only-blob-access/design.md +++ b/spec/features/identity-only-blob-access/design.md @@ -36,20 +36,33 @@ the way tokens signed under it are revoked. ### Persisted URLs -Records persist URLs that carry a token (`labelsUrl`, `embeddingsGeoJSONUrl`, -`featuresSidecarUrl`). Browsers never use that token against storage: the UI -rewrites the host to the managed-identity storage proxy, and the proxy -authenticates with its own identity. Functions and Batch read by blob path and -ignore the token. The persisted token therefore only matters in the local docker -stack, where the browser reads the emulator directly. +Tokens last an hour, so a token must never be the only way to reach a stored +blob. Stored image-layer and label-tile URLs are kept unsigned and a fresh +per-blob token is minted when a URL is handed out: + +- Label tile URLs embed the blob URL without a query, and label generation signs + only around its own raster read. +- The label project, labeling workspace, project details (previews) and + visualizer responses sign at response time. Signing replaces any older query, + so legacy stored URLs that carry a policy token keep working after the policy + is deleted. +- Only blobs in the app's own account and data container are signed; provider + and other-account URLs pass through unchanged. +- The tile service has no grant on the data account, so the nested token in the + tile URL is its only access; this is why signing happens per response. + +Other persisted URLs (`labelsUrl`, `embeddingsGeoJSONUrl`, +`featuresSidecarUrl`) are never used with their token in Azure: the UI goes +through the identity-authenticated storage proxy, and Functions and Batch read +by blob path. ## Risks | Risk | Mitigation | |---|---| | A caller still needs the key | Tests per path; dev1 run with the setting removed before any other environment | -| Existing 90-day links stop working once the policy is deleted | Deleted deliberately and only after dev1 validation; clients do not use the token in Azure | -| Local stack links expire after the token lifetime | Emulator tokens use the shared key and a longer lifetime | +| Existing 90-day links stop working once the policy is deleted | Deleted deliberately and only after dev1 validation; response-time signing replaces tokens on stored URLs | +| An endpoint returns a stored URL without signing it | Dev1 exercise of labeling, visualizer and project views before the policy is deleted | ## Rollout From 1ab624db596e22a1b6575d850deedd68f0848ab0 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Thu, 29 Oct 2026 10:29:33 -0300 Subject: [PATCH 10/24] fix(infra): delete legacy blob setting only after publish The setting was removed before the package was published and a stray edit broke the script's syntax. Delete it after a successful publish so a failed deploy keeps the old code working. --- .github/scripts/deploy_apps.sh | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/.github/scripts/deploy_apps.sh b/.github/scripts/deploy_apps.sh index 8d96ba23..dbe40447 100644 --- a/.github/scripts/deploy_apps.sh +++ b/.github/scripts/deploy_apps.sh @@ -369,13 +369,9 @@ deploy_function() { "PUBLISH_EXPLORER_RENDER_ENABLED=${PUBLISH_EXPLORER_RENDER_ENABLED}" \ "AzureFunctionsWebHost__hostId=${HOST_ID}" \ --output none + fi - # Blob and queue access is identity-based. Drop the account-key - # connection string left behind by earlier deployments. - az functionapp config appsettings delete --name "$FUNCTION_NAME" --resource-group "$RESOURCE_GROUP" \ - --setting-names BLOB_CONNECTION_STRING \ - --output none - fi HASTE imagery only from this storage host (plus its + # TiTiler reads HASTE imagery only from this storage host (plus its # built-in public open-data hosts). Mirrors functions.bicep; without it # HASTE imagery tiles fail closed. if [ "$FUNCTION_NAME" = "$FUNCTION_TITILER_API" ]; then @@ -411,6 +407,15 @@ deploy_function() { close_deploy_access "$FUNCTION_NAME" echo "Function deployment completed successfully." + # Blob and queue access is identity-based. Drop the account-key + # connection string left behind by earlier deployments, only once code + # that no longer needs it is running. + if [ "${SET_APPSETTINGS:-true}" = "true" ]; then + az functionapp config appsettings delete --name "$FUNCTION_NAME" --resource-group "$RESOURCE_GROUP" \ + --setting-names BLOB_CONNECTION_STRING \ + --output none + fi + # Record the wheel this app actually installed. Nothing else on the app # says which hastegeo it is running -- the wheel is pinned into # requirements.txt at deploy time and then forgotten -- so a blank From 42303ac42ac08aff28019dc3257cadcfc24235bf Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Thu, 29 Oct 2026 10:30:23 -0300 Subject: [PATCH 11/24] fix(storage): sign project-details URLs before the ETag is computed Preview URLs are signed inside the cached load so the ETag covers the tokens the client holds and a 304 cannot revive an expired one. Label and zip download URLs in project details are re-signed the same way. --- api/hastefuncapi/function_app.py | 9 +++-- hastelib/src/hastegeo/core/utils/blob.py | 33 +++++++++------- .../tests/core/utils/test_blob_signing.py | 38 +++++++++++-------- 3 files changed, 48 insertions(+), 32 deletions(-) diff --git a/api/hastefuncapi/function_app.py b/api/hastefuncapi/function_app.py index d111332a..884cf9ab 100644 --- a/api/hastefuncapi/function_app.py +++ b/api/hastefuncapi/function_app.py @@ -124,7 +124,7 @@ download_blob_to_tempfile, parse_byte_range, sign_label_project_urls, - sign_project_preview_urls, + sign_project_details_urls, ) from hastegeo.core.utils.data import convert_json_to_geojson, filter_roles from hastegeo.core.utils.errors import exception_diagnostics @@ -975,6 +975,9 @@ async def _load_response(): project = await ProjectDetailsProcessor( project_id=project_id, config=config ).load(include_models=include_models) + # Signed inside the cached load so the ETag covers the tokens the + # client holds and a 304 can never revive an expired one. + await asyncio.to_thread(sign_project_details_urls, project) payload = json.dumps(project) return { "payload": payload, @@ -993,9 +996,7 @@ async def _load_response(): _load_response, refresh=_cache_refresh_requested(req.headers.get("Cache-Control")), ) - _payload = await asyncio.to_thread( - sign_project_preview_urls, cached_response["payload"] - ) + _payload = cached_response["payload"] etag = cached_response["etag"] _perf_headers = perf.headers(_perf, _perf_wall) cache_headers = { diff --git a/hastelib/src/hastegeo/core/utils/blob.py b/hastelib/src/hastegeo/core/utils/blob.py index c2df4355..86818429 100644 --- a/hastelib/src/hastegeo/core/utils/blob.py +++ b/hastelib/src/hastegeo/core/utils/blob.py @@ -21,7 +21,6 @@ import asyncio import functools -import json import os import re import tempfile @@ -332,23 +331,31 @@ def _sign(match: re.Match) -> str: _LABEL_TILE_FIELDS = ("tileUrl", "preEventTileUrl", "postEventTileUrl") _PREVIEW_FIELDS = ("preEventPreviewUrls", "postEventPreviewUrls") +_LAYER_DOWNLOAD_FIELDS = ("labelsUrl",) +_MODEL_DOWNLOAD_FIELDS = ("labelsUrl", "trainingZipUrl", "inferenceZipUrl") -def sign_project_preview_urls(payload: str) -> str: - """Sign image-layer preview URLs in a serialized project-details payload. +def sign_project_details_urls(project: dict) -> dict: + """Sign blob URLs in a project-details dict in place and return it. - Returns the input string untouched when no layer has previews. + Covers layer previews and label downloads, and each model's label and + zip downloads. Stored values are never written back; callers sign the + response copy only. """ - project = json.loads(payload) - layers = project.get("imageLayer") or [] - changed = False - for layer in layers: + for layer in project.get("imageLayer") or []: for field in _PREVIEW_FIELDS: - urls = layer.get(field) - if urls: - layer[field] = [sign_internal_blob_url(url) for url in urls] - changed = True - return json.dumps(project) if changed else payload + if layer.get(field): + layer[field] = [ + sign_internal_blob_url(url) for url in layer[field] + ] + for field in _LAYER_DOWNLOAD_FIELDS: + if layer.get(field): + layer[field] = sign_internal_blob_url(layer[field]) + for model in layer.get("models") or []: + for field in _MODEL_DOWNLOAD_FIELDS: + if model.get(field): + model[field] = sign_internal_blob_url(model[field]) + return project def sign_label_project_urls(label_project: dict) -> dict: diff --git a/hastelib/tests/core/utils/test_blob_signing.py b/hastelib/tests/core/utils/test_blob_signing.py index d7ecacb8..0b36d6a7 100644 --- a/hastelib/tests/core/utils/test_blob_signing.py +++ b/hastelib/tests/core/utils/test_blob_signing.py @@ -120,21 +120,29 @@ def test_external_source_unchanged(self): tile = self._tile("https://provider.example.com/a.tif") self.assertEqual(sign_titiler_tile_url(tile), tile) - def test_preview_urls_signed_in_project_payload(self): - import json - - from hastegeo.core.utils.blob import sign_project_preview_urls - - payload = json.dumps( - {"imageLayer": [{"preEventPreviewUrls": [BLOB_URL]}, {}]} - ) - result = json.loads(sign_project_preview_urls(payload)) - self.assertEqual( - result["imageLayer"][0]["preEventPreviewUrls"], - [f"{BLOB_URL}?sig=fresh"], - ) - empty = json.dumps({"imageLayer": [{}]}) - self.assertEqual(sign_project_preview_urls(empty), empty) + def test_project_details_urls_signed(self): + from hastegeo.core.utils.blob import sign_project_details_urls + + project = { + "imageLayer": [ + { + "preEventPreviewUrls": [BLOB_URL], + "labelsUrl": BLOB_URL, + "models": [ + {"trainingZipUrl": BLOB_URL, "inferenceZipUrl": None} + ], + }, + {}, + ] + } + result = sign_project_details_urls(project) + signed = f"{BLOB_URL}?sig=fresh" + layer = result["imageLayer"][0] + self.assertEqual(layer["preEventPreviewUrls"], [signed]) + self.assertEqual(layer["labelsUrl"], signed) + self.assertEqual(layer["models"][0]["trainingZipUrl"], signed) + self.assertIsNone(layer["models"][0]["inferenceZipUrl"]) + self.assertEqual(result["imageLayer"][1], {}) def test_label_project_fields_signed_without_mutation(self): original = { From a251a5e433c47048b738c47097ebbb227325b287 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Thu, 29 Oct 2026 10:46:26 -0300 Subject: [PATCH 12/24] feat(api): return tile credential expiry with labeling data Label routes now include tileCredentialExpiresAt, matching the lifetime of the per-blob read tokens signed into the tile URLs, so the UI can refresh them before they expire. --- api/hastefuncapi/function_app.py | 5 +++++ api/hastefuncapi/tests/test_loading_routes.py | 1 + hastelib/src/hastegeo/core/utils/blob_sas.py | 7 +++++++ 3 files changed, 13 insertions(+) diff --git a/api/hastefuncapi/function_app.py b/api/hastefuncapi/function_app.py index 884cf9ab..8f4391d4 100644 --- a/api/hastefuncapi/function_app.py +++ b/api/hastefuncapi/function_app.py @@ -126,6 +126,7 @@ sign_label_project_urls, sign_project_details_urls, ) +from hastegeo.core.utils.blob_sas import read_token_expires_at from hastegeo.core.utils.data import convert_json_to_geojson, filter_roles from hastegeo.core.utils.errors import exception_diagnostics from hastegeo.core.utils.gdal_security import ( @@ -1763,6 +1764,9 @@ async def GetLayerLabelingToolData(req: func.HttpRequest) -> func.HttpResponse: match_label_projects = await asyncio.to_thread( sign_label_project_urls, match_label_projects ) + match_label_projects["tileCredentialExpiresAt"] = ( + read_token_expires_at() + ) return func.HttpResponse( json.dumps(match_label_projects), status_code=200 ) @@ -1808,6 +1812,7 @@ async def GetLabelingWorkspace(req: func.HttpRequest) -> func.HttpResponse: payload["labelProject"] = await asyncio.to_thread( sign_label_project_urls, payload["labelProject"] ) + payload["tileCredentialExpiresAt"] = read_token_expires_at() return func.HttpResponse( json.dumps(payload), status_code=200, diff --git a/api/hastefuncapi/tests/test_loading_routes.py b/api/hastefuncapi/tests/test_loading_routes.py index 2d9b2e1b..8b8fb121 100644 --- a/api/hastefuncapi/tests/test_loading_routes.py +++ b/api/hastefuncapi/tests/test_loading_routes.py @@ -90,6 +90,7 @@ async def test_returns_minimum_workspace_response(self) -> None: ) self.assertEqual(response.status_code, 200) + self.assertIn("tileCredentialExpiresAt", response_json(response)) self.assertEqual(response_json(response)["eventTypes"], ["Wildfire"]) self.assertEqual( response_json(response)["imageLayer"]["imageLayerId"], LAYER_ID diff --git a/hastelib/src/hastegeo/core/utils/blob_sas.py b/hastelib/src/hastegeo/core/utils/blob_sas.py index 28c15272..5b31f70d 100644 --- a/hastelib/src/hastegeo/core/utils/blob_sas.py +++ b/hastelib/src/hastegeo/core/utils/blob_sas.py @@ -14,6 +14,13 @@ DELEGATION_KEY_MARGIN = timedelta(minutes=READ_TOKEN_MINUTES + 5) +def read_token_expires_at() -> str: + """Return when tokens signed now stop working, as an ISO 8601 string.""" + return ( + datetime.now(timezone.utc) + timedelta(minutes=READ_TOKEN_MINUTES) + ).isoformat() + + def blob_read_sas( account_name: str, container_name: str, From 02305508c80ef54180970b7c06de5e7a21e4271a Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Thu, 29 Oct 2026 10:47:41 -0300 Subject: [PATCH 13/24] feat(ui): add tile credential refresh helper Tile URLs carry read tokens that expire after 60 minutes. Add a controller that refreshes them shortly before expiry (and on tab focus), merging only the imagery URLs into existing state and TileLayers so unsaved labels and selections are never replaced, with retry and rollback on failure. --- .../TileCredentialRefreshMessage.jsx | 48 +++ ui/src/util/tileCredentialRefresh.js | 246 +++++++++++++ ui/src/util/tileCredentialRefresh.test.js | 343 ++++++++++++++++++ ui/src/util/useTileCredentialRefresh.js | 98 +++++ 4 files changed, 735 insertions(+) create mode 100644 ui/src/Components/OtherComponents/TileCredentialRefreshMessage.jsx create mode 100644 ui/src/util/tileCredentialRefresh.js create mode 100644 ui/src/util/tileCredentialRefresh.test.js create mode 100644 ui/src/util/useTileCredentialRefresh.js diff --git a/ui/src/Components/OtherComponents/TileCredentialRefreshMessage.jsx b/ui/src/Components/OtherComponents/TileCredentialRefreshMessage.jsx new file mode 100644 index 00000000..3e150113 --- /dev/null +++ b/ui/src/Components/OtherComponents/TileCredentialRefreshMessage.jsx @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { + Button, + MessageBar, + MessageBarBody, + makeStyles, + tokens, +} from "@fluentui/react-components"; +import PropTypes from "prop-types"; + +const useStyles = makeStyles({ + root: { + position: "absolute", + top: tokens.spacingVerticalM, + left: "50%", + zIndex: 1200, + width: "min(560px, calc(100% - 32px))", + transform: "translateX(-50%)", + boxShadow: tokens.shadow8, + }, +}); + +const TileCredentialRefreshMessage = ({ visible, onRetry }) => { + const styles = useStyles(); + if (!visible) return null; + + return ( + + + Imagery access could not be refreshed. Existing imagery is retained. + + + + + ); +}; + +TileCredentialRefreshMessage.propTypes = { + visible: PropTypes.bool.isRequired, + onRetry: PropTypes.func.isRequired, +}; + +export default TileCredentialRefreshMessage; diff --git a/ui/src/util/tileCredentialRefresh.js b/ui/src/util/tileCredentialRefresh.js new file mode 100644 index 00000000..41819300 --- /dev/null +++ b/ui/src/util/tileCredentialRefresh.js @@ -0,0 +1,246 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export const TILE_CREDENTIAL_REFRESH_BUFFER_MS = 5 * 60 * 1000; +export const TILE_CREDENTIAL_FALLBACK_REFRESH_MS = 45 * 60 * 1000; +export const TILE_CREDENTIAL_VISIBILITY_WINDOW_MS = 60 * 1000; +export const TILE_CREDENTIAL_RETRY_MS = 60 * 1000; + +export function getTileCredentialRefreshAt( + tileCredentialExpiresAt, + now = Date.now() +) { + const expiresAt = Date.parse(tileCredentialExpiresAt); + if (Number.isFinite(expiresAt)) { + return expiresAt - TILE_CREDENTIAL_REFRESH_BUFFER_MS; + } + return now + TILE_CREDENTIAL_FALLBACK_REFRESH_MS; +} + +export function getTileCredentialRefreshDelay( + tileCredentialExpiresAt, + now = Date.now() +) { + return Math.max( + 0, + getTileCredentialRefreshAt(tileCredentialExpiresAt, now) - now + ); +} + +export function shouldRefreshCredentialsOnVisibility( + refreshAt, + now = Date.now() +) { + return ( + Number.isFinite(refreshAt) && + now + TILE_CREDENTIAL_VISIBILITY_WINDOW_MS >= refreshAt + ); +} + +// Pre-event imagery is optional, so a missing URL is an empty one. +function requireImageryUrls(nextData) { + const imagery = nextData?.imagery; + if (!imagery || typeof imagery !== "object") { + throw new Error("The imagery credential response is incomplete."); + } + return { + preEventTileUrl: imagery.preEventTileUrl || "", + postEventTileUrl: imagery.postEventTileUrl || "", + }; +} + +function transformTileUrl(transform, tileUrl) { + const transformed = transform ? transform(tileUrl) : tileUrl; + if (typeof transformed !== "string" || (tileUrl && !transformed)) { + throw new Error("The imagery credential response contains an invalid URL."); + } + return transformed; +} + +/** + * Updates only credential-bearing imagery fields and existing TileLayers. + * Labels, validation selections, and other in-memory data remain untouched. + */ +export function applyTileCredentialUpdate({ + dataRef, + nextData, + layerBindings = [], + onApplied, +}) { + if (!dataRef) { + throw new Error("An imagery data reference is required."); + } + + const nextImagery = requireImageryUrls(nextData); + const previousData = dataRef.current || {}; + const previousImagery = previousData.imagery || {}; + const mergedData = { + ...previousData, + imagery: { + ...previousImagery, + preEventTileUrl: nextImagery.preEventTileUrl, + postEventTileUrl: nextImagery.postEventTileUrl, + }, + }; + + if ( + Object.prototype.hasOwnProperty.call(nextData, "tileCredentialExpiresAt") + ) { + mergedData.tileCredentialExpiresAt = nextData.tileCredentialExpiresAt; + } else { + delete mergedData.tileCredentialExpiresAt; + } + + const updatedLayers = []; + try { + for (const binding of layerBindings) { + const imageryKey = binding.imageryKey; + const layer = binding.layerRef?.current; + if (!layer || typeof layer.setOptions !== "function") continue; + + const nextUrl = transformTileUrl( + binding.transformTileUrl, + nextImagery[imageryKey] + ); + const previousUrl = transformTileUrl( + binding.transformTileUrl, + previousImagery[imageryKey] || "" + ); + + // Record before calling setOptions so a layer that mutates and then + // throws can still be rolled back. + updatedLayers.push({ layer, previousUrl }); + layer.setOptions({ tileUrl: nextUrl }); + } + + dataRef.current = mergedData; + onApplied?.(mergedData); + return mergedData; + } catch (error) { + dataRef.current = previousData; + for (let index = updatedLayers.length - 1; index >= 0; index -= 1) { + try { + const { layer, previousUrl } = updatedLayers[index]; + layer.setOptions({ tileUrl: previousUrl }); + } catch { + // Preserve the original refresh error; rollback is best-effort. + } + } + throw error; + } +} + +export function createTileCredentialRefreshController({ + fetchCredentials, + applyCredentials, + onError = () => {}, + onSuccess = () => {}, + now = Date.now, + setTimer = (callback, delay) => globalThis.setTimeout(callback, delay), + clearTimer = (timerId) => globalThis.clearTimeout(timerId), + documentTarget = globalThis.document, +}) { + let disposed = false; + let listening = false; + let timerId = null; + let refreshAt = null; + let refreshPromise = null; + + function clearScheduledRefresh() { + if (timerId !== null) { + clearTimer(timerId); + timerId = null; + } + } + + function schedule(data) { + if (disposed) return; + clearScheduledRefresh(); + refreshAt = getTileCredentialRefreshAt( + data?.tileCredentialExpiresAt, + now() + ); + const delay = Math.max(0, refreshAt - now()); + timerId = setTimer(() => { + timerId = null; + return refresh(); + }, delay); + } + + function scheduleRetry() { + if (disposed) return; + clearScheduledRefresh(); + timerId = setTimer(() => { + timerId = null; + return refresh(); + }, TILE_CREDENTIAL_RETRY_MS); + } + + function refresh() { + if (disposed) return Promise.resolve(false); + if (refreshPromise) return refreshPromise; + + refreshPromise = (async () => { + try { + const nextData = await fetchCredentials(); + if (disposed) return false; + await applyCredentials(nextData); + if (disposed) return false; + onSuccess(); + schedule(nextData); + return true; + } catch { + if (!disposed) { + onError(); + scheduleRetry(); + } + return false; + } finally { + refreshPromise = null; + } + })(); + + return refreshPromise; + } + + function handleVisibilityChange() { + if ( + documentTarget?.visibilityState === "visible" && + shouldRefreshCredentialsOnVisibility(refreshAt, now()) + ) { + return refresh(); + } + return undefined; + } + + function start(initialData) { + if (disposed || !initialData) return; + if (!listening && documentTarget?.addEventListener) { + documentTarget.addEventListener( + "visibilitychange", + handleVisibilityChange + ); + listening = true; + } + schedule(initialData); + } + + function dispose() { + if (disposed) return; + disposed = true; + clearScheduledRefresh(); + if (listening && documentTarget?.removeEventListener) { + documentTarget.removeEventListener( + "visibilitychange", + handleVisibilityChange + ); + } + listening = false; + } + + return { + start, + refresh, + dispose, + }; +} diff --git a/ui/src/util/tileCredentialRefresh.test.js b/ui/src/util/tileCredentialRefresh.test.js new file mode 100644 index 00000000..cd8aef52 --- /dev/null +++ b/ui/src/util/tileCredentialRefresh.test.js @@ -0,0 +1,343 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + TILE_CREDENTIAL_FALLBACK_REFRESH_MS, + TILE_CREDENTIAL_REFRESH_BUFFER_MS, + TILE_CREDENTIAL_RETRY_MS, + applyTileCredentialUpdate, + createTileCredentialRefreshController, + getTileCredentialRefreshAt, + getTileCredentialRefreshDelay, +} from "./tileCredentialRefresh.js"; + +function fakeScheduler() { + let nextId = 1; + const timers = new Map(); + const cleared = []; + return { + timers, + cleared, + setTimer(callback, delay) { + const id = nextId; + nextId += 1; + timers.set(id, { callback, delay }); + return id; + }, + clearTimer(id) { + cleared.push(id); + timers.delete(id); + }, + }; +} + +function fakeDocument() { + const listeners = new Set(); + return { + visibilityState: "hidden", + listeners, + addEventListener(eventName, listener) { + if (eventName === "visibilitychange") listeners.add(listener); + }, + removeEventListener(eventName, listener) { + if (eventName === "visibilitychange") listeners.delete(listener); + }, + async dispatchVisibilityChange() { + await Promise.all([...listeners].map((listener) => listener())); + }, + }; +} + +test("calculates expiry refresh five minutes early and a 45-minute fallback", () => { + const now = Date.parse("2026-08-07T12:00:00.000Z"); + const expiresAt = "2026-08-07T13:00:00.000Z"; + + assert.equal( + getTileCredentialRefreshAt(expiresAt, now), + Date.parse(expiresAt) - TILE_CREDENTIAL_REFRESH_BUFFER_MS + ); + assert.equal( + getTileCredentialRefreshDelay(expiresAt, now), + 55 * 60 * 1000 + ); + assert.equal( + getTileCredentialRefreshAt(undefined, now), + now + TILE_CREDENTIAL_FALLBACK_REFRESH_MS + ); +}); + +test("refreshes on visible when the scheduled refresh time is due", async () => { + let clock = Date.parse("2026-08-07T12:00:00.000Z"); + const scheduler = fakeScheduler(); + const documentTarget = fakeDocument(); + let fetchCount = 0; + let applyCount = 0; + const initialData = { + tileCredentialExpiresAt: "2026-08-07T13:00:00.000Z", + }; + + const controller = createTileCredentialRefreshController({ + fetchCredentials: async () => { + fetchCount += 1; + return { + ...initialData, + tileCredentialExpiresAt: "2026-08-07T14:00:00.000Z", + }; + }, + applyCredentials: () => { + applyCount += 1; + }, + now: () => clock, + setTimer: scheduler.setTimer, + clearTimer: scheduler.clearTimer, + documentTarget, + }); + + controller.start(initialData); + clock = getTileCredentialRefreshAt(initialData.tileCredentialExpiresAt, clock); + documentTarget.visibilityState = "visible"; + await documentTarget.dispatchVisibilityChange(); + + assert.equal(fetchCount, 1); + assert.equal(applyCount, 1); + assert.equal(scheduler.timers.size, 1); + controller.dispose(); +}); + +test("updates existing layers without replacing unsaved state", () => { + const unsavedLabels = [{ id: "unsaved-label" }]; + const validationSelections = new Set(["building-7"]); + const dataRef = { + current: { + imagery: { + preEventTileUrl: "https://tiles.example/pre-old/{z}/{x}/{y}", + postEventTileUrl: "https://tiles.example/post-old/{z}/{x}/{y}", + }, + labels: unsavedLabels, + validationSelections, + unsavedEdits: true, + }, + }; + const preLayer = { + calls: [], + setOptions(options) { + this.calls.push(options); + }, + }; + const postLayer = { + calls: [], + setOptions(options) { + this.calls.push(options); + }, + }; + let appliedData = null; + + applyTileCredentialUpdate({ + dataRef, + nextData: { + imagery: { + preEventTileUrl: "https://tiles.example/pre-new/{z}/{x}/{y}", + postEventTileUrl: "https://tiles.example/post-new/{z}/{x}/{y}", + }, + labels: [{ id: "stale-server-label" }], + tileCredentialExpiresAt: "2026-08-07T14:00:00.000Z", + }, + layerBindings: [ + { layerRef: { current: preLayer }, imageryKey: "preEventTileUrl" }, + { layerRef: { current: postLayer }, imageryKey: "postEventTileUrl" }, + ], + onApplied: (data) => { + appliedData = data; + }, + }); + + assert.strictEqual(dataRef.current.labels, unsavedLabels); + assert.strictEqual( + dataRef.current.validationSelections, + validationSelections + ); + assert.equal(dataRef.current.unsavedEdits, true); + assert.deepEqual(preLayer.calls, [ + { tileUrl: "https://tiles.example/pre-new/{z}/{x}/{y}" }, + ]); + assert.deepEqual(postLayer.calls, [ + { tileUrl: "https://tiles.example/post-new/{z}/{x}/{y}" }, + ]); + assert.strictEqual(appliedData, dataRef.current); +}); + +test("updates the data ref when a TileLayer has not been created yet", () => { + const dataRef = { + current: { + imagery: { + preEventTileUrl: "old-pre", + postEventTileUrl: "old-post", + }, + }, + }; + + applyTileCredentialUpdate({ + dataRef, + nextData: { + imagery: { + preEventTileUrl: "new-pre", + postEventTileUrl: "new-post", + }, + }, + layerBindings: [ + { layerRef: { current: null }, imageryKey: "preEventTileUrl" }, + ], + }); + + assert.equal(dataRef.current.imagery.preEventTileUrl, "new-pre"); + assert.equal(dataRef.current.imagery.postEventTileUrl, "new-post"); +}); + +test("a missing pre-event URL is treated as empty, not as a failure", () => { + const dataRef = { + current: { imagery: { postEventTileUrl: "old-post" } }, + }; + + applyTileCredentialUpdate({ + dataRef, + nextData: { imagery: { postEventTileUrl: "new-post" } }, + }); + + assert.equal(dataRef.current.imagery.preEventTileUrl, ""); + assert.equal(dataRef.current.imagery.postEventTileUrl, "new-post"); +}); + +test("a response without imagery is rejected and current data is kept", () => { + const previousData = { imagery: { postEventTileUrl: "old-post" } }; + const dataRef = { current: previousData }; + + assert.throws(() => + applyTileCredentialUpdate({ dataRef, nextData: {} }) + ); + assert.strictEqual(dataRef.current, previousData); +}); + +test("a TileLayer update failure rolls back URLs and retains current data", () => { + const previousData = { + imagery: { + preEventTileUrl: "old-pre", + postEventTileUrl: "old-post", + }, + labels: [{ id: "unsaved" }], + }; + const dataRef = { current: previousData }; + const preCalls = []; + const postCalls = []; + const preLayer = { + setOptions(options) { + preCalls.push(options); + }, + }; + const postLayer = { + setOptions(options) { + postCalls.push(options); + if (options.tileUrl === "new-post") { + throw new Error("renderer rejected update"); + } + }, + }; + + assert.throws(() => + applyTileCredentialUpdate({ + dataRef, + nextData: { + imagery: { + preEventTileUrl: "new-pre", + postEventTileUrl: "new-post", + }, + }, + layerBindings: [ + { layerRef: { current: preLayer }, imageryKey: "preEventTileUrl" }, + { layerRef: { current: postLayer }, imageryKey: "postEventTileUrl" }, + ], + }) + ); + + assert.strictEqual(dataRef.current, previousData); + assert.deepEqual(preCalls, [ + { tileUrl: "new-pre" }, + { tileUrl: "old-pre" }, + ]); + assert.deepEqual(postCalls, [ + { tileUrl: "new-post" }, + { tileUrl: "old-post" }, + ]); +}); + +test("failed refresh retains current data and schedules a recoverable retry", async () => { + const scheduler = fakeScheduler(); + const documentTarget = fakeDocument(); + const currentData = { + imagery: { + preEventTileUrl: "current-pre", + postEventTileUrl: "current-post", + }, + labels: [{ id: "unsaved" }], + }; + let errorCount = 0; + let applyCount = 0; + + const controller = createTileCredentialRefreshController({ + fetchCredentials: async () => { + throw new Error("network unavailable"); + }, + applyCredentials: () => { + applyCount += 1; + }, + onError: () => { + errorCount += 1; + }, + setTimer: scheduler.setTimer, + clearTimer: scheduler.clearTimer, + documentTarget, + }); + + controller.start(currentData); + const refreshed = await controller.refresh(); + + assert.equal(refreshed, false); + assert.equal(applyCount, 0); + assert.equal(errorCount, 1); + assert.strictEqual(currentData.labels[0].id, "unsaved"); + assert.deepEqual( + [...scheduler.timers.values()].map((timer) => timer.delay), + [TILE_CREDENTIAL_RETRY_MS] + ); + controller.dispose(); +}); + +test("cleanup clears the timer, removes visibility listener, and blocks updates", async () => { + const scheduler = fakeScheduler(); + const documentTarget = fakeDocument(); + let fetchCount = 0; + const controller = createTileCredentialRefreshController({ + fetchCredentials: async () => { + fetchCount += 1; + return {}; + }, + applyCredentials: () => {}, + setTimer: scheduler.setTimer, + clearTimer: scheduler.clearTimer, + documentTarget, + }); + + controller.start({}); + assert.equal(documentTarget.listeners.size, 1); + assert.equal(scheduler.timers.size, 1); + + controller.dispose(); + documentTarget.visibilityState = "visible"; + await documentTarget.dispatchVisibilityChange(); + const refreshed = await controller.refresh(); + + assert.equal(documentTarget.listeners.size, 0); + assert.equal(scheduler.timers.size, 0); + assert.equal(scheduler.cleared.length, 1); + assert.equal(fetchCount, 0); + assert.equal(refreshed, false); +}); diff --git a/ui/src/util/useTileCredentialRefresh.js b/ui/src/util/useTileCredentialRefresh.js new file mode 100644 index 00000000..d98c9553 --- /dev/null +++ b/ui/src/util/useTileCredentialRefresh.js @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +import { useCallback, useEffect, useRef, useState } from "react"; +import { apiGet } from "./api"; +import { + applyTileCredentialUpdate, + createTileCredentialRefreshController, +} from "./tileCredentialRefresh"; + +function labelingToolDataEndpoint(projectId, imageLayerId) { + return ( + "GetLayerLabelingToolData?projectId=" + + encodeURIComponent(projectId) + + "&imageLayerId=" + + encodeURIComponent(imageLayerId) + ); +} + +export function useTileCredentialRefresh({ + projectId, + imageLayerId, + dataRef, + layerBindings, + onApplied, +}) { + const [tileRefreshFailed, setTileRefreshFailed] = useState(false); + const controllerRef = useRef(null); + const pendingInitialDataRef = useRef(null); + const configRef = useRef({ + projectId, + imageLayerId, + dataRef, + layerBindings, + onApplied, + }); + + useEffect(() => { + configRef.current = { + projectId, + imageLayerId, + dataRef, + layerBindings, + onApplied, + }; + }, [projectId, imageLayerId, dataRef, layerBindings, onApplied]); + + useEffect(() => { + const controller = createTileCredentialRefreshController({ + fetchCredentials: () => { + const config = configRef.current; + return apiGet( + labelingToolDataEndpoint(config.projectId, config.imageLayerId) + ); + }, + applyCredentials: (nextData) => { + const config = configRef.current; + return applyTileCredentialUpdate({ + dataRef: config.dataRef, + nextData, + layerBindings: config.layerBindings, + onApplied: config.onApplied, + }); + }, + onError: () => setTileRefreshFailed(true), + onSuccess: () => setTileRefreshFailed(false), + }); + + controllerRef.current = controller; + if (pendingInitialDataRef.current) { + controller.start(pendingInitialDataRef.current); + } + + return () => { + controller.dispose(); + if (controllerRef.current === controller) { + controllerRef.current = null; + } + }; + }, [projectId, imageLayerId]); + + const startTileCredentialRefresh = useCallback((initialData) => { + if (!initialData) return; + pendingInitialDataRef.current = initialData; + setTileRefreshFailed(false); + controllerRef.current?.start(initialData); + }, []); + + const retryTileCredentialRefresh = useCallback( + () => controllerRef.current?.refresh() || Promise.resolve(false), + [] + ); + + return { + tileRefreshFailed, + startTileCredentialRefresh, + retryTileCredentialRefresh, + }; +} From 6e1d198d8f53ae2b207e0389809ea34ab23bb879 Mon Sep 17 00:00:00 2001 From: jQuinRivero Date: Thu, 29 Oct 2026 10:50:53 -0300 Subject: [PATCH 14/24] feat(ui): refresh tile credentials in labeling surfaces Wire the refresh controller into the labeling tool, interactive labeler (including the swipe pre-event layer) and building validation. Refreshed imagery URLs are applied to existing TileLayers in place, so drawn labels, selections and the loaded model stay untouched; a banner offers retry or reload if a refresh fails. --- .../BuildingValidation/BuildingValidation.jsx | 29 +++++++++- .../InteractiveLabeler/InteractiveLabeler.jsx | 58 ++++++++++++++++++- .../Components/LabelingTool/LabelingTool.jsx | 40 +++++++++++++ 3 files changed, 123 insertions(+), 4 deletions(-) diff --git a/ui/src/Components/BuildingValidation/BuildingValidation.jsx b/ui/src/Components/BuildingValidation/BuildingValidation.jsx index ddf6b298..5ff1fb8a 100644 --- a/ui/src/Components/BuildingValidation/BuildingValidation.jsx +++ b/ui/src/Components/BuildingValidation/BuildingValidation.jsx @@ -15,6 +15,8 @@ import { } from "./validationConfig.js"; import { loadValidationMapData } from "./loadValidationMapData.js"; import { loadImagery } from "../LabelingTool/LabelingToolHelper.js"; +import { useTileCredentialRefresh } from "../../util/useTileCredentialRefresh.js"; +import TileCredentialRefreshMessage from "../OtherComponents/TileCredentialRefreshMessage.jsx"; import { shouldIgnoreShortcut } from "../keyboardShortcuts.js"; import "../../assets/css/labels.css"; import "../../assets/css/drawingToolbar.css"; @@ -99,7 +101,26 @@ const BuildingValidation = () => { // poking at Azure Maps internals. const preTileUrlRef = useRef(null); const postTileUrlRef = useRef(null); - // De-dupe set: URLs we've already requested. Prevents thrashing the + const layerDataRef = useRef(null); + // Tile URLs carry read tokens that expire; refresh them in place without + // touching labels or the current selection. + const { + tileRefreshFailed, + startTileCredentialRefresh, + retryTileCredentialRefresh, + } = useTileCredentialRefresh({ + projectId, + imageLayerId, + dataRef: layerDataRef, + layerBindings: [ + { layerRef: preImageryRef, imageryKey: "preEventTileUrl" }, + { layerRef: postImageryRef, imageryKey: "postEventTileUrl" }, + ], + onApplied: (data) => { + preTileUrlRef.current = data.imagery?.preEventTileUrl || null; + postTileUrlRef.current = data.imagery?.postEventTileUrl || null; + }, + }); // De-dupe set: URLs we've already requested. Prevents thrashing the // tile server when the user walks back and forth across buildings. const prefetchedTilesRef = useRef(new Set()); @@ -260,6 +281,8 @@ const BuildingValidation = () => { resolveSampleSize, }); setSampleSize(configuredSample); + layerDataRef.current = layerData || null; + startTileCredentialRefresh(layerData); const existingLabels = validationData?.labels || {}; setLabels(existingLabels); @@ -686,6 +709,10 @@ const BuildingValidation = () => { className={`${styles.root} building-validation-page`} data-map-ready={isDatasourceReady ? "true" : "false"} > + {/* Back button — shares the Interactive Labeler navigation surface. */}