diff --git a/AGENTS.md b/AGENTS.md index 0bf04fc..1a769a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -52,6 +52,12 @@ Storage Service (Lambda) → DynamoDB (BSOs) **HAWK Authorizer** (`lambda/src/entrypoint/hawk_authorizer.py`): Lambda authorizer for API Gateway +**Environment Configuration:** +- `CLOCK_SKEW_TOLERANCE`: OIDC JWT validation skew (Token API: 300s) +- `OIDC_CACHE_TTL_SECONDS`: OIDC provider config and JWKS cache TTL (Token API: 3600s / 1 hour) +- `HAWK_TIMESTAMP_SKEW_TOLERANCE`: HAWK timestamp validation skew (60s default) +- `TOKEN_DURATION`: HAWK credential duration (300s) + ## Project Structure ``` @@ -127,6 +133,7 @@ tools/ # CLI tools for testing - **HAWK credentials**: Expire after 300 seconds - clients must refresh via Token Service - **Test structure**: Must mirror `src/` structure (e.g., `tests/routes/bso/test_read.py` ↔ `src/routes/bso/read.py`) - **Fixtures**: Use `mock_service_provider`, `mock_storage_manager` from `conftest.py` +- **DynamoDB GSI**: `list_collections` uses `UserCollectionsIndex` GSI for efficient queries - collection metadata items include `user_id` attribute ### TypeScript/CDK - **Strict mode**: All code paths must return values diff --git a/lambda/README.md b/lambda/README.md index a284bfa..d407296 100644 --- a/lambda/README.md +++ b/lambda/README.md @@ -83,16 +83,6 @@ class NewRoute(BaseRoute): } ``` -## TODO Items - -All route implementations currently contain TODO comments for: -- Authentication validation integration -- Proper timestamp generation -- Request payload parsing and validation -- Response header management (X-Last-Modified, X-If-Unmodified-Since) -- Batch operation logic -- Quota enforcement - ## Testing Each route class can be unit tested independently by instantiating the class and calling the `handle` method with mock API Gateway events. diff --git a/lambda/src/environment/service_provider.py b/lambda/src/environment/service_provider.py index e0a71cb..102ba7b 100644 --- a/lambda/src/environment/service_provider.py +++ b/lambda/src/environment/service_provider.py @@ -120,9 +120,28 @@ def oidc_config(self) -> dict: @cached_property def clock_skew_tolerance(self) -> int: - """Get clock skew tolerance in seconds""" + """ + Get clock skew tolerance in seconds for OIDC JWT validation + """ return int(os.environ["CLOCK_SKEW_TOLERANCE"]) + @cached_property + def oidc_cache_ttl_seconds(self) -> int: + """ + Get OIDC configuration cache TTL in seconds (default 3600 / 1 hour) + """ + return int(os.environ.get("OIDC_CACHE_TTL_SECONDS", "3600")) + + @cached_property + def hawk_timestamp_skew_tolerance(self) -> int: + """ + Get HAWK timestamp skew tolerance in seconds + + Separate from OIDC clock_skew_tolerance to allow independent configuration + of HAWK timestamp validation vs OIDC JWT validation + """ + return int(os.environ["HAWK_TIMESTAMP_SKEW_TOLERANCE"]) + @cached_property def retry_after_seconds(self) -> int: """Get Retry-After value for 503 responses in seconds (default 30)""" @@ -135,6 +154,7 @@ def oidc_validator(self) -> OIDCValidator: provider_url=self.oidc_config["provider_url"], client_id=self.oidc_config["client_id"], clock_skew_tolerance=self.clock_skew_tolerance, + cache_ttl_seconds=self.oidc_cache_ttl_seconds, ) @cached_property @@ -163,6 +183,10 @@ def token_api_router(self): def token_cache_table_name(self): return os.environ.get("TOKEN_CACHE_TABLE_NAME") + @cached_property + def token_duration(self) -> int: + return int(os.environ["TOKEN_DURATION"]) + @cached_property def token_cache_table(self): """Create DynamoDB Table resource for token cache""" @@ -172,4 +196,8 @@ def token_cache_table(self): @cached_property def hawk_service(self) -> HawkService: """Create HAWK service for authentication""" - return HawkService(token_cache_table=self.token_cache_table) + return HawkService( + token_cache_table=self.token_cache_table, + timestamp_skew_tolerance=self.hawk_timestamp_skew_tolerance, + token_duration=self.token_duration, + ) diff --git a/lambda/src/services/hawk_service.py b/lambda/src/services/hawk_service.py index e650beb..48ac086 100644 --- a/lambda/src/services/hawk_service.py +++ b/lambda/src/services/hawk_service.py @@ -51,17 +51,20 @@ class HawkService: - All validation helper methods """ - TIMESTAMP_SKEW_SECONDS = 60 # Allow 60 second clock skew - TOKEN_DURATION_SECONDS = 300 # 5 minutes - - def __init__(self, token_cache_table): + def __init__( + self, token_cache_table, timestamp_skew_tolerance: int = 60, token_duration: int = 300 + ): """ Initialize HAWK service with DynamoDB table. Args: token_cache_table: boto3 DynamoDB Table resource for token cache + timestamp_skew_tolerance: Allowed clock skew in seconds + token_duration: Duration a token is valid for """ self.token_cache_table = token_cache_table + self.timestamp_skew_tolerance = timestamp_skew_tolerance + self.token_duration = token_duration def validate( self, authorization_header: str, method: str, path: str, host: str, port: int @@ -229,7 +232,7 @@ def validate_timestamp(self, timestamp: int) -> bool: True if timestamp is within acceptable skew """ now = int(time.time()) - return abs(now - timestamp) <= self.TIMESTAMP_SKEW_SECONDS + return abs(now - timestamp) <= self.timestamp_skew_tolerance def build_normalized_string( self, @@ -384,7 +387,7 @@ def generate_hawk_credentials(self, user_id: str, generation: int) -> HawkCreden Used by Token Server when issuing tokens. """ - expiry = int(time.time()) + self.TOKEN_DURATION_SECONDS + expiry = int(time.time()) + self.token_duration hawk_id = self.generate_hawk_id(user_id, generation, expiry) hawk_key = self.generate_hawk_key() diff --git a/lambda/src/services/oidc_validator.py b/lambda/src/services/oidc_validator.py index a0c3605..717bc1a 100644 --- a/lambda/src/services/oidc_validator.py +++ b/lambda/src/services/oidc_validator.py @@ -15,7 +15,7 @@ ) from src.shared.oidc import OIDCProviderConfig, OIDCTokenClaims -# Cache TTL in seconds (1 hour) +# Default cache TTL in seconds (1 hour) - maintained for backward compatibility CACHE_TTL_SECONDS = 3600 @@ -31,7 +31,13 @@ class OIDCValidator: - User identifier extraction from sub claim """ - def __init__(self, provider_url: str, client_id: str, clock_skew_tolerance: int = 300): + def __init__( + self, + provider_url: str, + client_id: str, + clock_skew_tolerance: int = 300, + cache_ttl_seconds: int = CACHE_TTL_SECONDS, + ): """ Initialize OIDC validator with provider configuration. @@ -39,10 +45,12 @@ def __init__(self, provider_url: str, client_id: str, clock_skew_tolerance: int provider_url: Base URL of OIDC provider (e.g., https://auth.example.com) client_id: Expected audience claim value (OAuth client ID) clock_skew_tolerance: Maximum allowed clock skew in seconds (default 300 / 5 minutes) + cache_ttl_seconds: Cache TTL for provider config and JWKS in seconds (default 3600 / 1 hour) """ self.provider_url = provider_url.rstrip("/") self.client_id = client_id self.clock_skew_tolerance = clock_skew_tolerance + self.cache_ttl_seconds = cache_ttl_seconds self._provider_config: Optional[OIDCProviderConfig] = None self._provider_config_timestamp: float = 0 self._jwk_client: Optional[PyJWKClient] = None @@ -52,7 +60,7 @@ def _is_cache_valid(self) -> bool: if self._provider_config is None: return False current_time = datetime.now(timezone.utc).timestamp() - return (current_time - self._provider_config_timestamp) < CACHE_TTL_SECONDS + return (current_time - self._provider_config_timestamp) < self.cache_ttl_seconds def discover_provider_config(self) -> OIDCProviderConfig: """ @@ -117,7 +125,7 @@ def _get_jwk_client(self) -> PyJWKClient: self._jwk_client = PyJWKClient( config.jwks_uri, cache_keys=True, - lifespan=CACHE_TTL_SECONDS, + lifespan=self.cache_ttl_seconds, ) return self._jwk_client diff --git a/lambda/src/services/storage_manager.py b/lambda/src/services/storage_manager.py index c505507..c40e339 100644 --- a/lambda/src/services/storage_manager.py +++ b/lambda/src/services/storage_manager.py @@ -71,6 +71,7 @@ def _encode_collection_data(self, user_id: str, collection_data: CollectionData) col_data = collection_data.to_dict() col_data[_PK] = self._collection_pk(user_id, collection_data.name) col_data[_SK] = self._metadata_sk() + col_data["user_id"] = user_id # GSI partition key for efficient user queries return col_data def get_collection(self, user_id: str, collection_name: str) -> CollectionData: @@ -376,20 +377,29 @@ def list_collections(self, user_id: str) -> List[CollectionData]: """ collections = [] - # Query for all collections for this user - # Use begins_with to match USER#{user_id}#COLLECTION# - response = self.table.scan( - FilterExpression="begins_with(PK, :user_prefix) AND SK = :metadata", - ExpressionAttributeValues={ - ":user_prefix": f"USER#{user_id}#COLLECTION#", - ":metadata": self._metadata_sk(), - }, + # Query GSI for all collections for this user + response = self.table.query( + IndexName="UserCollectionsIndex", + KeyConditionExpression="user_id = :user_id", + ExpressionAttributeValues={":user_id": user_id}, ) for item in response.get("Items", []): collection = CollectionData.from_dict(item) collections.append(collection) + # Handle pagination if there are more results + while "LastEvaluatedKey" in response: + response = self.table.query( + IndexName="UserCollectionsIndex", + KeyConditionExpression="user_id = :user_id", + ExpressionAttributeValues={":user_id": user_id}, + ExclusiveStartKey=response["LastEvaluatedKey"], + ) + for item in response.get("Items", []): + collection = CollectionData.from_dict(item) + collections.append(collection) + return collections def get_collection_objects( diff --git a/lambda/src/services/token_generator.py b/lambda/src/services/token_generator.py index 9db497f..25012d8 100644 --- a/lambda/src/services/token_generator.py +++ b/lambda/src/services/token_generator.py @@ -79,6 +79,6 @@ def generate_token(self, user_id: str, uid: int, generation: int) -> TokenRespon key=credentials.hawk_key, api_endpoint=api_endpoint, uid=uid, - duration=self._hawk_service.TOKEN_DURATION_SECONDS, + duration=self._hawk_service.token_duration, hashalg=self.HASH_ALGORITHM, ) diff --git a/lambda/tests/conftest.py b/lambda/tests/conftest.py index 49b9b77..6b5934a 100644 --- a/lambda/tests/conftest.py +++ b/lambda/tests/conftest.py @@ -64,7 +64,9 @@ def setup_environment( monkeypatch.setenv("OIDC_SECRET_ARN", oidc_secret_arn) monkeypatch.setenv("BASE_DOMAIN", base_domain) monkeypatch.setenv("CLOCK_SKEW_TOLERANCE", "300") + monkeypatch.setenv("HAWK_TIMESTAMP_SKEW_TOLERANCE", "60") monkeypatch.setenv("RETRY_AFTER_SECONDS", "30") + monkeypatch.setenv("TOKEN_DURATION", "300") class MockServiceProvider(ServiceProvider): diff --git a/lambda/tests/integration/test_e2e_flow.py b/lambda/tests/integration/test_e2e_flow.py index d667161..a08f2fe 100644 --- a/lambda/tests/integration/test_e2e_flow.py +++ b/lambda/tests/integration/test_e2e_flow.py @@ -401,14 +401,15 @@ def test_info_collections_scoped_to_user( info_event = build_storage_event(method="GET", path="/info/collections", user_id=user1_id) modified_time = time.time() - # Mock scan returns only User1's collections + # Mock query with GSI returns only User1's collections dynamodb_stubber.add_response( - "scan", + "query", { "Items": [ { "PK": {"S": f"USER#{user1_id}#COLLECTION#bookmarks"}, "SK": {"S": "METADATA"}, + "user_id": {"S": user1_id}, "name": {"S": "bookmarks"}, "modified": {"N": str(modified_time)}, "count": {"N": "5"}, @@ -417,6 +418,7 @@ def test_info_collections_scoped_to_user( { "PK": {"S": f"USER#{user1_id}#COLLECTION#tabs"}, "SK": {"S": "METADATA"}, + "user_id": {"S": user1_id}, "name": {"S": "tabs"}, "modified": {"N": str(modified_time)}, "count": {"N": "3"}, diff --git a/lambda/tests/integration/test_token_server_mozilla_spec.py b/lambda/tests/integration/test_token_server_mozilla_spec.py index e529b44..b2fa24b 100644 --- a/lambda/tests/integration/test_token_server_mozilla_spec.py +++ b/lambda/tests/integration/test_token_server_mozilla_spec.py @@ -15,7 +15,6 @@ import time from unittest.mock import patch -import pytest from botocore.stub import ANY from src.entrypoint.token_api import lambda_handler as token_handler @@ -934,24 +933,6 @@ def test_x_timestamp_on_401_response( timestamp_int = int(timestamp) assert timestamp_int > 0 - @pytest.mark.skip(reason="503 error handling happens at infrastructure level before handler") - def test_retry_after_on_503_response( - self, - mock_service_provider, - secretsmanager_stubber, - sample_lambda_context, - ): - """ - Test Retry-After header on 503 response. - - Note: This test is skipped because 503 errors from Secrets Manager - occur during ServiceProvider initialization, before the request handler - can catch them. In production, API Gateway would handle these errors. - - **Validates: Requirement 15.1** - """ - pass - def test_www_authenticate_on_401_response( self, mock_service_provider, diff --git a/lambda/tests/services/test_oidc_validator.py b/lambda/tests/services/test_oidc_validator.py index ff39a17..2b36296 100644 --- a/lambda/tests/services/test_oidc_validator.py +++ b/lambda/tests/services/test_oidc_validator.py @@ -7,7 +7,7 @@ import pytest from jwt import PyJWKClientError -from src.services.oidc_validator import CACHE_TTL_SECONDS, OIDCValidator +from src.services.oidc_validator import OIDCValidator from src.shared.exceptions import ( InvalidCredentialsError, InvalidTokenError, @@ -69,6 +69,16 @@ def test_init_custom_clock_skew_tolerance(self, provider_url, client_id): validator = OIDCValidator(provider_url, client_id, clock_skew_tolerance=600) assert validator.clock_skew_tolerance == 600 + def test_init_default_cache_ttl_seconds(self, provider_url, client_id): + """Test that default cache_ttl_seconds is 3600 seconds""" + validator = OIDCValidator(provider_url, client_id) + assert validator.cache_ttl_seconds == 3600 + + def test_init_custom_cache_ttl_seconds(self, provider_url, client_id): + """Test that custom cache_ttl_seconds is stored correctly""" + validator = OIDCValidator(provider_url, client_id, cache_ttl_seconds=7200) + assert validator.cache_ttl_seconds == 7200 + class TestDiscoverProviderConfig: """Test discover_provider_config method""" @@ -118,9 +128,9 @@ def test_discover_provider_config_cache_expiry(self, validator, mock_provider_co # First call validator.discover_provider_config() - # Simulate cache expiry + # Simulate cache expiry using the validator's cache_ttl_seconds validator._provider_config_timestamp = ( - datetime.now(timezone.utc).timestamp() - CACHE_TTL_SECONDS - 1 + datetime.now(timezone.utc).timestamp() - validator.cache_ttl_seconds - 1 ) # Second call should fetch again @@ -128,6 +138,38 @@ def test_discover_provider_config_cache_expiry(self, validator, mock_provider_co assert mock_get.call_count == 2 + def test_discover_provider_config_custom_cache_ttl( + self, provider_url, client_id, mock_provider_config + ): + """Test that custom cache TTL is respected""" + validator = OIDCValidator(provider_url, client_id, cache_ttl_seconds=1800) + + with patch("src.services.oidc_validator.requests.get") as mock_get: + mock_response = MagicMock() + mock_response.json.return_value = mock_provider_config + mock_response.raise_for_status = MagicMock() + mock_get.return_value = mock_response + + # First call + validator.discover_provider_config() + + # Simulate time passage less than custom TTL (1800s) + validator._provider_config_timestamp = datetime.now(timezone.utc).timestamp() - 1000 + + # Second call should use cache + validator.discover_provider_config() + + # Should only call once due to caching + assert mock_get.call_count == 1 + + # Simulate cache expiry beyond custom TTL + validator._provider_config_timestamp = datetime.now(timezone.utc).timestamp() - 1800 - 1 + + # Third call should fetch again + validator.discover_provider_config() + + assert mock_get.call_count == 2 + def test_discover_provider_config_timeout(self, validator): """Test ServiceUnavailableError on timeout""" import requests # type: ignore[import-untyped] @@ -665,7 +707,30 @@ def test_get_jwk_client_creates_client(self, validator, mock_provider_config): mock_jwk_class.assert_called_once_with( mock_provider_config["jwks_uri"], cache_keys=True, - lifespan=CACHE_TTL_SECONDS, + lifespan=validator.cache_ttl_seconds, + ) + + def test_get_jwk_client_custom_cache_ttl(self, provider_url, client_id, mock_provider_config): + """Test that _get_jwk_client uses custom cache TTL""" + validator = OIDCValidator(provider_url, client_id, cache_ttl_seconds=7200) + + with patch("src.services.oidc_validator.requests.get") as mock_get: + mock_response = MagicMock() + mock_response.json.return_value = mock_provider_config + mock_response.raise_for_status = MagicMock() + mock_get.return_value = mock_response + + with patch("src.services.oidc_validator.PyJWKClient") as mock_jwk_class: + mock_jwk_instance = MagicMock() + mock_jwk_class.return_value = mock_jwk_instance + + client = validator._get_jwk_client() + + assert client is mock_jwk_instance + mock_jwk_class.assert_called_once_with( + mock_provider_config["jwks_uri"], + cache_keys=True, + lifespan=7200, ) def test_get_jwk_client_caches_client(self, validator, mock_provider_config): diff --git a/lambda/tests/services/test_storage_manager.py b/lambda/tests/services/test_storage_manager.py index 2ef956b..9354322 100644 --- a/lambda/tests/services/test_storage_manager.py +++ b/lambda/tests/services/test_storage_manager.py @@ -171,6 +171,7 @@ def test_create_or_update_collection_without_objects( "Item": { "PK": "USER#test-user-123#COLLECTION#bookmarks", "SK": "METADATA", + "user_id": "test-user-123", "name": "bookmarks", "modified": Decimal(mock_timestamp), "count": 0, @@ -223,6 +224,7 @@ def test_create_or_update_collection_with_objects( "Item": { "PK": "USER#test-user-123#COLLECTION#bookmarks", "SK": "METADATA", + "user_id": "test-user-123", "name": "bookmarks", "modified": Decimal(datetime.fromtimestamp(mock_timestamp).timestamp()), "count": 2, @@ -320,6 +322,7 @@ def test_update_collection( "Item": { "PK": "USER#test-user-123#COLLECTION#bookmarks", "SK": "METADATA", + "user_id": "test-user-123", "name": "bookmarks", "modified": Decimal(datetime.fromtimestamp(mock_timestamp).timestamp()), "count": 2, @@ -449,12 +452,13 @@ def test_delete_collection( def test_list_collections(self, storage_manager, dynamodb_stubber, storage_table_name): """Test listing all collections""" dynamodb_stubber.add_response( - "scan", + "query", { "Items": [ { "PK": {"S": "USER#test-user-123#COLLECTION#bookmarks"}, "SK": {"S": "METADATA"}, + "user_id": {"S": "test-user-123"}, "name": {"S": "bookmarks"}, "modified": {"N": "1234567890.12"}, "count": {"N": "5"}, @@ -463,6 +467,7 @@ def test_list_collections(self, storage_manager, dynamodb_stubber, storage_table { "PK": {"S": "USER#test-user-123#COLLECTION#history"}, "SK": {"S": "METADATA"}, + "user_id": {"S": "test-user-123"}, "name": {"S": "history"}, "modified": {"N": "1234567891.00"}, "count": {"N": "10"}, @@ -472,11 +477,9 @@ def test_list_collections(self, storage_manager, dynamodb_stubber, storage_table }, { "TableName": storage_table_name, - "FilterExpression": "begins_with(PK, :user_prefix) AND SK = :metadata", - "ExpressionAttributeValues": { - ":user_prefix": "USER#test-user-123#COLLECTION#", - ":metadata": "METADATA", - }, + "IndexName": "UserCollectionsIndex", + "KeyConditionExpression": "user_id = :user_id", + "ExpressionAttributeValues": {":user_id": "test-user-123"}, }, ) @@ -491,21 +494,100 @@ def test_list_collections(self, storage_manager, dynamodb_stubber, storage_table def test_list_collections_empty(self, storage_manager, dynamodb_stubber, storage_table_name): """Test listing collections when none exist""" dynamodb_stubber.add_response( - "scan", + "query", {"Items": []}, { "TableName": storage_table_name, - "FilterExpression": "begins_with(PK, :user_prefix) AND SK = :metadata", - "ExpressionAttributeValues": { - ":user_prefix": "USER#test-user-123#COLLECTION#", - ":metadata": "METADATA", - }, + "IndexName": "UserCollectionsIndex", + "KeyConditionExpression": "user_id = :user_id", + "ExpressionAttributeValues": {":user_id": "test-user-123"}, }, ) collections = storage_manager.list_collections("test-user-123") assert collections == [] + def test_list_collections_with_pagination( + self, storage_manager, dynamodb_stubber, storage_table_name + ): + """Test listing collections with pagination""" + # Stub first page with LastEvaluatedKey + dynamodb_stubber.add_response( + "query", + { + "Items": [ + { + "PK": {"S": "USER#test-user-123#COLLECTION#bookmarks"}, + "SK": {"S": "METADATA"}, + "user_id": {"S": "test-user-123"}, + "name": {"S": "bookmarks"}, + "modified": {"N": "1234567890.12"}, + "count": {"N": "5"}, + "usage": {"N": "1024"}, + }, + { + "PK": {"S": "USER#test-user-123#COLLECTION#history"}, + "SK": {"S": "METADATA"}, + "user_id": {"S": "test-user-123"}, + "name": {"S": "history"}, + "modified": {"N": "1234567891.00"}, + "count": {"N": "10"}, + "usage": {"N": "2048"}, + }, + ], + "LastEvaluatedKey": { + "PK": {"S": "USER#test-user-123#COLLECTION#history"}, + "SK": {"S": "METADATA"}, + "user_id": {"S": "test-user-123"}, + }, + }, + { + "TableName": storage_table_name, + "IndexName": "UserCollectionsIndex", + "KeyConditionExpression": "user_id = :user_id", + "ExpressionAttributeValues": {":user_id": "test-user-123"}, + }, + ) + + # Stub second page (no more items) + dynamodb_stubber.add_response( + "query", + { + "Items": [ + { + "PK": {"S": "USER#test-user-123#COLLECTION#passwords"}, + "SK": {"S": "METADATA"}, + "user_id": {"S": "test-user-123"}, + "name": {"S": "passwords"}, + "modified": {"N": "1234567892.00"}, + "count": {"N": "15"}, + "usage": {"N": "3072"}, + }, + ] + }, + { + "TableName": storage_table_name, + "IndexName": "UserCollectionsIndex", + "KeyConditionExpression": "user_id = :user_id", + "ExpressionAttributeValues": {":user_id": "test-user-123"}, + "ExclusiveStartKey": { + "PK": "USER#test-user-123#COLLECTION#history", + "SK": "METADATA", + "user_id": "test-user-123", + }, + }, + ) + + collections = storage_manager.list_collections("test-user-123") + + assert len(collections) == 3 + assert collections[0].name == "bookmarks" + assert collections[0].count == 5 + assert collections[1].name == "history" + assert collections[1].count == 10 + assert collections[2].name == "passwords" + assert collections[2].count == 15 + def test_get_collection_objects(self, storage_manager, dynamodb_stubber, storage_table_name): """Test getting objects from collection""" dynamodb_stubber.add_response( @@ -841,6 +923,7 @@ def test_create_collection_with_failed_object( "Item": { "PK": "USER#test-user-123#COLLECTION#bookmarks", "SK": "METADATA", + "user_id": "test-user-123", "name": "bookmarks", "modified": Decimal(datetime.fromtimestamp(mock_timestamp).timestamp()), "count": 1, @@ -920,6 +1003,7 @@ def test_update_collection_with_failed_object( "Item": { "PK": "USER#test-user-123#COLLECTION#bookmarks", "SK": "METADATA", + "user_id": "test-user-123", "name": "bookmarks", "modified": Decimal(datetime.fromtimestamp(mock_timestamp).timestamp()), "count": 0, @@ -1269,6 +1353,7 @@ def test_update_collection_with_mixed_success_fail( "Item": { "PK": "USER#test-user-123#COLLECTION#bookmarks", "SK": "METADATA", + "user_id": "test-user-123", "name": "bookmarks", "modified": Decimal(datetime.fromtimestamp(mock_timestamp).timestamp()), "count": 1, @@ -1343,6 +1428,7 @@ def test_update_collection_with_sortindex_and_ttl( "Item": { "PK": "USER#test-user-123#COLLECTION#bookmarks", "SK": "METADATA", + "user_id": "test-user-123", "name": "bookmarks", "modified": Decimal(datetime.fromtimestamp(mock_timestamp).timestamp()), "count": 1, @@ -2193,14 +2279,15 @@ def test_get_quota( storage_table_name, ): """Test getting quota information""" - # list_collections uses scan + # list_collections uses query with GSI dynamodb_stubber.add_response( - "scan", + "query", { "Items": [ { "PK": {"S": "USER#test-user-123#COLLECTION#bookmarks"}, "SK": {"S": "METADATA"}, + "user_id": {"S": "test-user-123"}, "name": {"S": "bookmarks"}, "modified": {"N": "1234567890.00"}, "count": {"N": "5"}, @@ -2209,6 +2296,7 @@ def test_get_quota( { "PK": {"S": "USER#test-user-123#COLLECTION#history"}, "SK": {"S": "METADATA"}, + "user_id": {"S": "test-user-123"}, "name": {"S": "history"}, "modified": {"N": "1234567880.00"}, "count": {"N": "10"}, @@ -2216,7 +2304,12 @@ def test_get_quota( }, ] }, - None, + { + "TableName": storage_table_name, + "IndexName": "UserCollectionsIndex", + "KeyConditionExpression": "user_id = :user_id", + "ExpressionAttributeValues": {":user_id": "test-user-123"}, + }, ) usage_kb, quota_kb = storage_manager.get_quota("test-user-123") diff --git a/lambda/tests/services/test_token_generator.py b/lambda/tests/services/test_token_generator.py index 338f5d3..4fdbe65 100644 --- a/lambda/tests/services/test_token_generator.py +++ b/lambda/tests/services/test_token_generator.py @@ -25,7 +25,7 @@ class TestTokenGenerator: def mock_hawk_service(self): """Mock HawkService for testing orchestration""" service = MagicMock() - service.TOKEN_DURATION_SECONDS = 300 + service.token_duration = 300 return service @pytest.fixture @@ -247,7 +247,6 @@ def test_generate_token_duration_from_hawk_service( generation = 0 mock_hawk_service.generate_hawk_credentials.return_value = mock_hawk_credentials - mock_hawk_service.TOKEN_DURATION_SECONDS = 300 token = token_generator.generate_token(user_id, uid, generation) diff --git a/lib/stacks/service.ts b/lib/stacks/service.ts index a6d7715..5e9907f 100644 --- a/lib/stacks/service.ts +++ b/lib/stacks/service.ts @@ -14,7 +14,13 @@ import { SpecRestApi, } from "aws-cdk-lib/aws-apigateway"; import {Certificate, CertificateValidation} from "aws-cdk-lib/aws-certificatemanager"; -import {AttributeType, BillingMode, Table, TableEncryption} from "aws-cdk-lib/aws-dynamodb"; +import { + AttributeType, + BillingMode, + ProjectionType, + Table, + TableEncryption, +} from "aws-cdk-lib/aws-dynamodb"; import {Role, ServicePrincipal} from "aws-cdk-lib/aws-iam"; import {Architecture, IFunction, Runtime} from "aws-cdk-lib/aws-lambda"; import {LogGroup, RetentionDays} from "aws-cdk-lib/aws-logs"; @@ -85,7 +91,7 @@ export class ServiceStack extends Stack { } private buildStorageTable(): Table { - return new Table(this, "StorageTable", { + const table = new Table(this, "StorageTable", { tableName: `ffsync-storage-${this.props.stageType.toLowerCase()}`, encryption: TableEncryption.AWS_MANAGED, partitionKey: { @@ -105,6 +111,22 @@ export class ServiceStack extends Stack { ? RemovalPolicy.RETAIN_ON_UPDATE_OR_DELETE : RemovalPolicy.DESTROY, }); + + // Add GSI for efficient user collection queries + table.addGlobalSecondaryIndex({ + indexName: "UserCollectionsIndex", + partitionKey: { + name: "user_id", + type: AttributeType.STRING, + }, + sortKey: { + name: "name", + type: AttributeType.STRING, + }, + projectionType: ProjectionType.ALL, + }); + + return table; } private buildTokenUsersTable(): Table { @@ -161,6 +183,8 @@ export class ServiceStack extends Stack { environment: { STAGE: this.props.stageType.toLowerCase(), TOKEN_CACHE_TABLE_NAME: this.tokenCacheTable.tableName, + HAWK_TIMESTAMP_SKEW_TOLERANCE: "60", + TOKEN_DURATION: "300", }, }); @@ -218,7 +242,10 @@ export class ServiceStack extends Stack { TOKEN_USERS_TABLE_NAME: this.tokenUsersTable.tableName, TOKEN_CACHE_TABLE_NAME: this.tokenCacheTable.tableName, CLOCK_SKEW_TOLERANCE: "300", + OIDC_CACHE_TTL_SECONDS: "3600", + HAWK_TIMESTAMP_SKEW_TOLERANCE: "60", RETRY_AFTER_SECONDS: "30", + TOKEN_DURATION: "300", }, });