From b6047f5b7d332fe77139309759f25c9c914077ac Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Mon, 16 Mar 2026 00:18:13 +0100 Subject: [PATCH] =?UTF-8?q?feat(loyalty):=20Google=20Wallet=20production?= =?UTF-8?q?=20readiness=20=E2=80=94=2010=20hardening=20items?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix rate limiter to extract real client IP and handle sync/async endpoints - Rate-limit public enrollment (10/min) and program info (30/min) endpoints - Add 409 Conflict to non-retryable status codes in retry decorator - Cache private key in get_save_url() to avoid re-reading JSON per call - Make update_class() return bool success status with error-level logging - Move Google Wallet config from core to loyalty module config - Document time.sleep() safety in retry decorator (threadpool execution) - Add per-card retry (1 retry, 2s delay) to wallet sync task - Add logo URL reachability check (HEAD request) to validate_config() - Add 26 comprehensive unit tests for GoogleWalletService Co-Authored-By: Claude Opus 4.6 --- app/core/config.py | 10 +- app/modules/loyalty/config.py | 6 + app/modules/loyalty/routes/api/storefront.py | 3 + .../loyalty/services/google_wallet_service.py | 91 ++- app/modules/loyalty/tasks/wallet_sync.py | 37 +- .../tests/unit/test_google_wallet_service.py | 622 +++++++++++++++++- .../loyalty/tests/unit/test_wallet_service.py | 40 +- middleware/decorators.py | 69 +- 8 files changed, 791 insertions(+), 87 deletions(-) diff --git a/app/core/config.py b/app/core/config.py index d8c83bdd..56d9dae4 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -217,14 +217,6 @@ class Settings(BaseSettings): # ============================================================================= cloudflare_enabled: bool = False # Set to True when using CloudFlare proxy - # ============================================================================= - # GOOGLE WALLET (LOYALTY MODULE) - # ============================================================================= - loyalty_google_issuer_id: str | None = None - loyalty_google_service_account_json: str | None = None # Path to service account JSON - loyalty_google_wallet_origins: list[str] = [] # Allowed origins for save-to-wallet JWT - loyalty_default_logo_url: str = "https://rewardflow.lu/static/modules/loyalty/shared/img/default-logo-200.png" - # ============================================================================= # APPLE WALLET (LOYALTY MODULE) # ============================================================================= @@ -234,7 +226,7 @@ class Settings(BaseSettings): loyalty_apple_signer_cert_path: str | None = None loyalty_apple_signer_key_path: str | None = None - model_config = {"env_file": ".env"} + model_config = {"env_file": ".env", "extra": "ignore"} # Singleton settings instance diff --git a/app/modules/loyalty/config.py b/app/modules/loyalty/config.py index 4e34b735..b901117a 100644 --- a/app/modules/loyalty/config.py +++ b/app/modules/loyalty/config.py @@ -36,6 +36,12 @@ class ModuleConfig(BaseSettings): apple_signer_cert_path: str | None = None # Pass signing certificate apple_signer_key_path: str | None = None # Pass signing key + # Google Wallet + google_issuer_id: str | None = None + google_service_account_json: str | None = None # Path to service account JSON + google_wallet_origins: list[str] = [] # Allowed origins for save-to-wallet JWT + default_logo_url: str = "https://rewardflow.lu/static/modules/loyalty/shared/img/default-logo-200.png" + # QR code settings qr_code_size: int = 300 # pixels diff --git a/app/modules/loyalty/routes/api/storefront.py b/app/modules/loyalty/routes/api/storefront.py index 1387549b..ea4c68a3 100644 --- a/app/modules/loyalty/routes/api/storefront.py +++ b/app/modules/loyalty/routes/api/storefront.py @@ -26,6 +26,7 @@ from app.modules.loyalty.schemas import ( ) from app.modules.loyalty.services import card_service, program_service, wallet_service from app.modules.tenancy.exceptions import StoreNotFoundException +from middleware.decorators import rate_limit storefront_router = APIRouter() logger = logging.getLogger(__name__) @@ -37,6 +38,7 @@ logger = logging.getLogger(__name__) @storefront_router.get("/loyalty/program") +@rate_limit(max_requests=30, window_seconds=60) def get_program_info( request: Request, db: Session = Depends(get_db), @@ -62,6 +64,7 @@ def get_program_info( @storefront_router.post("/loyalty/enroll") +@rate_limit(max_requests=10, window_seconds=60) def self_enroll( request: Request, data: CardEnrollRequest, diff --git a/app/modules/loyalty/services/google_wallet_service.py b/app/modules/loyalty/services/google_wallet_service.py index 63936fe9..1bc2ad56 100644 --- a/app/modules/loyalty/services/google_wallet_service.py +++ b/app/modules/loyalty/services/google_wallet_service.py @@ -20,7 +20,7 @@ from typing import Any import requests from sqlalchemy.orm import Session -from app.core.config import settings +from app.modules.loyalty.config import config from app.modules.loyalty.exceptions import ( GoogleWalletNotConfiguredException, WalletIntegrationException, @@ -51,7 +51,7 @@ def _retry_on_failure(func): # Don't retry client errors (400, 401, 403, 404, 409) exc_msg = str(exc) if any(f":{code}" in exc_msg or f" {code} " in exc_msg - for code in ("400", "401", "403", "404")): + for code in ("400", "401", "403", "404", "409")): logger.error("Google Wallet API client error (not retryable): %s", exc) break if attempt < MAX_RETRIES - 1: @@ -63,6 +63,9 @@ def _retry_on_failure(func): wait, exc, ) + # Safe: all loyalty routes are sync def, so FastAPI runs them + # in a threadpool. time.sleep() only blocks the worker thread, + # not the async event loop. time.sleep(wait) else: logger.error( @@ -83,13 +86,14 @@ class GoogleWalletService: self._credentials = None self._http_client = None self._signer = None + self._private_key: str | None = None @property def is_configured(self) -> bool: """Check if Google Wallet is configured.""" return bool( - settings.loyalty_google_issuer_id - and settings.loyalty_google_service_account_json + config.google_issuer_id + and config.google_service_account_json ) def validate_config(self) -> dict[str, Any]: @@ -103,8 +107,8 @@ class GoogleWalletService: result: dict[str, Any] = { "configured": self.is_configured, - "issuer_id": settings.loyalty_google_issuer_id, - "service_account_path": settings.loyalty_google_service_account_json, + "issuer_id": config.google_issuer_id, + "service_account_path": config.google_service_account_json, "credentials_valid": False, "errors": [], } @@ -112,7 +116,7 @@ class GoogleWalletService: if not self.is_configured: return result - sa_path = settings.loyalty_google_service_account_json + sa_path = config.google_service_account_json if not os.path.isfile(sa_path): result["errors"].append(f"Service account file not found: {sa_path}") return result @@ -145,6 +149,21 @@ class GoogleWalletService: except (OSError, ValueError) as exc: result["errors"].append(f"Failed to load credentials: {exc}") + # Check logo URL reachability (warning only, not a blocking error) + logo_url = config.default_logo_url + if logo_url: + result["warnings"] = result.get("warnings", []) + try: + resp = requests.head(logo_url, timeout=5, allow_redirects=True) + if resp.status_code >= 400: + result["warnings"].append( + f"Default logo URL returned HTTP {resp.status_code}: {logo_url}" + ) + except requests.RequestException as exc: + result["warnings"].append( + f"Default logo URL unreachable: {logo_url} ({exc})" + ) + return result def _get_credentials(self): @@ -152,7 +171,7 @@ class GoogleWalletService: if self._credentials: return self._credentials - if not settings.loyalty_google_service_account_json: + if not config.google_service_account_json: raise GoogleWalletNotConfiguredException() try: @@ -162,7 +181,7 @@ class GoogleWalletService: self._credentials = ( service_account.Credentials.from_service_account_file( - settings.loyalty_google_service_account_json, + config.google_service_account_json, scopes=scopes, ) ) @@ -180,13 +199,23 @@ class GoogleWalletService: from google.auth.crypt import RSASigner self._signer = RSASigner.from_service_account_file( - settings.loyalty_google_service_account_json, + config.google_service_account_json, ) return self._signer except (ValueError, OSError, KeyError) as exc: logger.error("Failed to create RSA signer: %s", exc) raise WalletIntegrationException("google", str(exc)) from exc + def _get_private_key(self) -> str: + """Get the private key from the service account JSON, with caching.""" + if self._private_key: + return self._private_key + + with open(config.google_service_account_json) as f: + sa_data = json.load(f) + self._private_key = sa_data["private_key"] + return self._private_key + def _get_http_client(self): """Get authenticated HTTP client.""" if self._http_client: @@ -221,7 +250,7 @@ class GoogleWalletService: if not self.is_configured: raise GoogleWalletNotConfiguredException() - issuer_id = settings.loyalty_google_issuer_id + issuer_id = config.google_issuer_id class_id = f"{issuer_id}.loyalty_program_{program.id}" # issuerName is required by Google Wallet API @@ -246,7 +275,7 @@ class GoogleWalletService: # Google must be able to fetch the image, so it needs a public URL logo_url = program.logo_url if not logo_url: - logo_url = settings.loyalty_default_logo_url + logo_url = config.default_logo_url class_data["programLogo"] = { "sourceUri": {"uri": logo_url}, } @@ -289,10 +318,14 @@ class GoogleWalletService: raise WalletIntegrationException("google", str(exc)) from exc @_retry_on_failure - def update_class(self, db: Session, program: LoyaltyProgram) -> None: - """Update a LoyaltyClass when program settings change.""" + def update_class(self, db: Session, program: LoyaltyProgram) -> bool: + """Update a LoyaltyClass when program settings change. + + Returns: + True if update succeeded, False otherwise. + """ if not program.google_class_id: - return + return False class_data = { "programName": program.display_name, @@ -312,14 +345,17 @@ class GoogleWalletService: json=class_data, ) - if response.status_code not in (200, 201): - logger.warning( - "Failed to update Google Wallet class %s: %s", - program.google_class_id, - response.status_code, - ) + if response.status_code in (200, 201): + return True + logger.error( + "Failed to update Google Wallet class %s: %s", + program.google_class_id, + response.status_code, + ) + return False except (requests.RequestException, ValueError, AttributeError) as exc: logger.error("Failed to update Google Wallet class: %s", exc) + return False # ========================================================================= # LoyaltyObject Operations (Card-level) @@ -344,7 +380,7 @@ class GoogleWalletService: if not program.google_class_id: self.create_class(db, program) - issuer_id = settings.loyalty_google_issuer_id + issuer_id = config.google_issuer_id object_id = f"{issuer_id}.loyalty_card_{card.id}" object_data = self._build_object_data(card, object_id) @@ -490,9 +526,9 @@ class GoogleWalletService: credentials = self._get_credentials() now = datetime.now(tz=UTC) - origins = settings.loyalty_google_wallet_origins or [] + origins = config.google_wallet_origins or [] - issuer_id = settings.loyalty_google_issuer_id + issuer_id = config.google_issuer_id object_id = card.google_object_id or f"{issuer_id}.loyalty_card_{card.id}" if card.google_object_id: @@ -517,11 +553,8 @@ class GoogleWalletService: "exp": now + timedelta(hours=1), } - # Load the private key directly from the service account file - # (RSASigner doesn't expose .key; PyJWT needs the PEM string) - with open(settings.loyalty_google_service_account_json) as f: - sa_data = json.load(f) - private_key = sa_data["private_key"] + # PyJWT needs the PEM string; cached after first load + private_key = self._get_private_key() token = jwt.encode( claims, diff --git a/app/modules/loyalty/tasks/wallet_sync.py b/app/modules/loyalty/tasks/wallet_sync.py index 9f4086c9..01689207 100644 --- a/app/modules/loyalty/tasks/wallet_sync.py +++ b/app/modules/loyalty/tasks/wallet_sync.py @@ -66,20 +66,38 @@ def sync_wallet_passes() -> dict: google_synced = 0 apple_synced = 0 + failed_card_ids = [] for card in cards: - try: - results = wallet_service.sync_card_to_wallets(db, card) - if results.get("google_wallet"): - google_synced += 1 - if results.get("apple_wallet"): - apple_synced += 1 - except Exception as e: - logger.warning(f"Failed to sync card {card.id} to wallets: {e}") + synced = False + for attempt in range(2): # 1 retry + try: + results = wallet_service.sync_card_to_wallets(db, card) + if results.get("google_wallet"): + google_synced += 1 + if results.get("apple_wallet"): + apple_synced += 1 + synced = True + break + except Exception as e: + if attempt == 0: + logger.warning( + f"Failed to sync card {card.id} (attempt 1/2), " + f"retrying in 2s: {e}" + ) + import time + time.sleep(2) + else: + logger.error( + f"Failed to sync card {card.id} after 2 attempts: {e}" + ) + if not synced: + failed_card_ids.append(card.id) logger.info( f"Wallet sync complete: {len(cards)} cards checked, " - f"{google_synced} Google, {apple_synced} Apple" + f"{google_synced} Google, {apple_synced} Apple, " + f"{len(failed_card_ids)} failed" ) return { @@ -87,6 +105,7 @@ def sync_wallet_passes() -> dict: "cards_checked": len(cards), "google_synced": google_synced, "apple_synced": apple_synced, + "failed_card_ids": failed_card_ids, } except Exception as e: diff --git a/app/modules/loyalty/tests/unit/test_google_wallet_service.py b/app/modules/loyalty/tests/unit/test_google_wallet_service.py index a6fb280e..21b48cfd 100644 --- a/app/modules/loyalty/tests/unit/test_google_wallet_service.py +++ b/app/modules/loyalty/tests/unit/test_google_wallet_service.py @@ -1,18 +1,622 @@ -"""Unit tests for GoogleWalletService.""" +"""Unit tests for GoogleWalletService. + +Covers: validate_config, create_class, update_class, create_object, +_build_object_data, get_save_url, _retry_on_failure, get_class_status. +""" + +import json +import os +import tempfile +from unittest.mock import MagicMock, patch import pytest -from app.modules.loyalty.services.google_wallet_service import GoogleWalletService +from app.modules.loyalty.exceptions import ( + GoogleWalletNotConfiguredException, + WalletIntegrationException, +) +from app.modules.loyalty.services.google_wallet_service import ( + GoogleWalletService, + _retry_on_failure, +) + +MOCK_ISSUER_ID = "1234567890" +MOCK_SA_PATH = "/fake/service_account.json" + + +def _make_service(issuer_id=MOCK_ISSUER_ID, sa_path=MOCK_SA_PATH): + """Create a GoogleWalletService with mocked credentials and HTTP client.""" + service = GoogleWalletService() + service._credentials = MagicMock() + service._credentials.service_account_email = "test@test.iam.gserviceaccount.com" + service._http_client = MagicMock() + return service + + +def _mock_response(status_code, json_data=None, text="{}"): + """Create a mock HTTP response.""" + resp = MagicMock() + resp.status_code = status_code + resp.text = text + resp.json.return_value = json_data or {} + return resp + + +def _make_program(program_id=1, class_id=None, stamps=False, points=True): + """Create a mock LoyaltyProgram.""" + program = MagicMock() + program.id = program_id + program.google_class_id = class_id + program.display_name = "Test Rewards" + program.card_color = "#4285F4" + program.logo_url = None + program.hero_image_url = None + program.merchant.name = "Test Merchant" + program.is_stamps_enabled = stamps + program.is_points_enabled = points + program.stamps_target = 10 + return program + + +def _make_card(card_id=1, program=None, active=True, google_object_id=None): + """Create a mock LoyaltyCard.""" + card = MagicMock() + card.id = card_id + card.card_number = "LC-ABCD1234" + card.is_active = active + card.stamp_count = 3 + card.points_balance = 150 + card.google_object_id = google_object_id + card.google_object_jwt = None + card.program = program or _make_program(class_id=f"{MOCK_ISSUER_ID}.loyalty_program_1") + return card + + +# ============================================================================ +# TestValidateConfig +# ============================================================================ @pytest.mark.unit @pytest.mark.loyalty -class TestGoogleWalletService: - """Test suite for GoogleWalletService.""" +class TestValidateConfig: + """Tests for validate_config().""" - def setup_method(self): - self.service = GoogleWalletService() + @patch("app.modules.loyalty.services.google_wallet_service.requests") + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_valid_service_account(self, mock_config, mock_requests): + """Valid service account file results in credentials_valid: True.""" + sa_data = { + "type": "service_account", + "project_id": "test-project", + "private_key": "-----BEGIN RSA PRIVATE KEY-----\nfake\n-----END RSA PRIVATE KEY-----\n", + "client_email": "test@test.iam.gserviceaccount.com", + } + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + json.dump(sa_data, f) + sa_path = f.name - def test_service_instantiation(self): - """Service can be instantiated.""" - assert self.service is not None + try: + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = sa_path + mock_config.default_logo_url = "" + mock_requests.head.return_value = _mock_response(200) + + service = GoogleWalletService() + service._credentials = MagicMock() # Skip real credential loading + + result = service.validate_config() + + assert result["configured"] is True + assert result["credentials_valid"] is True + assert result["errors"] == [] + finally: + os.unlink(sa_path) + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_missing_file(self, mock_config): + """Missing service account file produces error.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = "/nonexistent/path.json" + + service = GoogleWalletService() + result = service.validate_config() + + assert result["configured"] is True + assert result["credentials_valid"] is False + assert any("not found" in e for e in result["errors"]) + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_invalid_json(self, mock_config): + """Invalid JSON in service account file produces error.""" + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + f.write("not valid json {{{") + sa_path = f.name + + try: + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = sa_path + mock_config.default_logo_url = "" + + service = GoogleWalletService() + result = service.validate_config() + + assert result["credentials_valid"] is False + assert any("Invalid JSON" in e for e in result["errors"]) + finally: + os.unlink(sa_path) + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_wrong_credential_type(self, mock_config): + """Non-service_account type produces error.""" + sa_data = { + "type": "authorized_user", + "project_id": "test", + "private_key": "fake", + "client_email": "test@test.com", + } + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + json.dump(sa_data, f) + sa_path = f.name + + try: + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = sa_path + mock_config.default_logo_url = "" + + service = GoogleWalletService() + result = service.validate_config() + + assert any("Invalid credential type" in e for e in result["errors"]) + finally: + os.unlink(sa_path) + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_missing_required_fields(self, mock_config): + """Missing required fields produce errors.""" + sa_data = {"type": "service_account"} # Missing project_id, private_key, client_email + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + json.dump(sa_data, f) + sa_path = f.name + + try: + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = sa_path + mock_config.default_logo_url = "" + + service = GoogleWalletService() + result = service.validate_config() + + assert result["credentials_valid"] is False + assert len(result["errors"]) >= 3 # project_id, private_key, client_email + finally: + os.unlink(sa_path) + + +# ============================================================================ +# TestCreateClass +# ============================================================================ + + +@pytest.mark.unit +@pytest.mark.loyalty +class TestCreateClass: + """Tests for create_class().""" + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_201_creates_class(self, mock_config): + """201 response sets google_class_id and returns class_id.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + mock_config.default_logo_url = "https://example.com/logo.png" + + service = _make_service() + service._http_client.post.return_value = _mock_response(201) + + db = MagicMock() + program = _make_program() + + result = service.create_class(db, program) + + expected_id = f"{MOCK_ISSUER_ID}.loyalty_program_{program.id}" + assert result == expected_id + assert program.google_class_id == expected_id + db.commit.assert_called() + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_409_still_sets_class_id(self, mock_config): + """409 (already exists) still sets class_id — idempotent.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + mock_config.default_logo_url = "https://example.com/logo.png" + + service = _make_service() + service._http_client.post.return_value = _mock_response(409) + + db = MagicMock() + program = _make_program() + + result = service.create_class(db, program) + + expected_id = f"{MOCK_ISSUER_ID}.loyalty_program_{program.id}" + assert result == expected_id + assert program.google_class_id == expected_id + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_500_raises_exception(self, mock_config): + """500 response raises WalletIntegrationException.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + mock_config.default_logo_url = "https://example.com/logo.png" + + service = _make_service() + service._http_client.post.return_value = _mock_response(500, {"error": "internal"}) + + db = MagicMock() + program = _make_program() + + with pytest.raises(WalletIntegrationException): + service.create_class(db, program) + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_not_configured_raises(self, mock_config): + """Raises GoogleWalletNotConfiguredException when not configured.""" + mock_config.google_issuer_id = None + mock_config.google_service_account_json = None + + service = GoogleWalletService() + db = MagicMock() + program = _make_program() + + with pytest.raises(GoogleWalletNotConfiguredException): + service.create_class(db, program) + + +# ============================================================================ +# TestUpdateClass +# ============================================================================ + + +@pytest.mark.unit +@pytest.mark.loyalty +class TestUpdateClass: + """Tests for update_class().""" + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_200_returns_true(self, mock_config): + """200 response returns True.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + + service = _make_service() + service._http_client.patch.return_value = _mock_response(200) + + db = MagicMock() + program = _make_program(class_id=f"{MOCK_ISSUER_ID}.loyalty_program_1") + + assert service.update_class(db, program) is True + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_non_200_returns_false(self, mock_config): + """Non-200 response returns False and logs error.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + + service = _make_service() + service._http_client.patch.return_value = _mock_response(500) + + db = MagicMock() + program = _make_program(class_id=f"{MOCK_ISSUER_ID}.loyalty_program_1") + + assert service.update_class(db, program) is False + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_no_class_id_returns_false(self, mock_config): + """Returns False early when program has no google_class_id.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + + service = _make_service() + db = MagicMock() + program = _make_program(class_id=None) + + assert service.update_class(db, program) is False + service._http_client.patch.assert_not_called() + + +# ============================================================================ +# TestCreateObject +# ============================================================================ + + +@pytest.mark.unit +@pytest.mark.loyalty +class TestCreateObject: + """Tests for create_object().""" + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_auto_creates_class_when_none(self, mock_config): + """Auto-creates class when program.google_class_id is None.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + mock_config.default_logo_url = "https://example.com/logo.png" + + service = _make_service() + # First call creates class (201), second creates object (201) + service._http_client.post.side_effect = [ + _mock_response(201), # create_class + _mock_response(201), # create_object + ] + + db = MagicMock() + program = _make_program(class_id=None) + card = _make_card(program=program) + + result = service.create_object(db, card) + + assert result == f"{MOCK_ISSUER_ID}.loyalty_card_{card.id}" + assert service._http_client.post.call_count == 2 + + @patch("app.modules.loyalty.services.google_wallet_service.config") + @patch("app.modules.loyalty.services.google_wallet_service.time") + def test_500_retried_via_decorator(self, mock_time, mock_config): + """500 error triggers retry via _retry_on_failure decorator.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + + service = _make_service() + program = _make_program(class_id=f"{MOCK_ISSUER_ID}.loyalty_program_1") + card = _make_card(program=program) + + # First call 500, second 201 + service._http_client.post.side_effect = [ + _mock_response(500, {"error": "transient"}), + _mock_response(201), + ] + + db = MagicMock() + result = service.create_object(db, card) + + assert result == f"{MOCK_ISSUER_ID}.loyalty_card_{card.id}" + assert service._http_client.post.call_count == 2 + + +# ============================================================================ +# TestBuildObjectData +# ============================================================================ + + +@pytest.mark.unit +@pytest.mark.loyalty +class TestBuildObjectData: + """Tests for _build_object_data().""" + + def test_stamps_program(self): + """Stamps program includes loyaltyPoints + secondaryLoyaltyPoints.""" + service = GoogleWalletService() + program = _make_program(stamps=True, points=False) + program.stamps_target = 10 + card = _make_card(program=program) + card.stamp_count = 3 + + data = service._build_object_data(card, "test_obj_1") + + assert data["loyaltyPoints"]["label"] == "Stamps" + assert data["loyaltyPoints"]["balance"]["int"] == 3 + assert "secondaryLoyaltyPoints" in data + assert data["secondaryLoyaltyPoints"]["balance"]["int"] == 10 + + def test_points_program(self): + """Points program includes loyaltyPoints only, no secondary.""" + service = GoogleWalletService() + program = _make_program(stamps=False, points=True) + card = _make_card(program=program) + card.points_balance = 150 + + data = service._build_object_data(card, "test_obj_1") + + assert data["loyaltyPoints"]["label"] == "Points" + assert data["loyaltyPoints"]["balance"]["int"] == 150 + assert "secondaryLoyaltyPoints" not in data + + def test_inactive_card(self): + """Inactive card has state INACTIVE.""" + service = GoogleWalletService() + card = _make_card(active=False) + + data = service._build_object_data(card, "test_obj_1") + + assert data["state"] == "INACTIVE" + + def test_barcode_strips_dashes(self): + """Barcode value has dashes removed from card_number.""" + service = GoogleWalletService() + card = _make_card() + card.card_number = "LC-ABCD-1234" + + data = service._build_object_data(card, "test_obj_1") + + assert data["barcode"]["value"] == "LCABCD1234" + assert data["barcode"]["alternateText"] == "LC-ABCD-1234" + + +# ============================================================================ +# TestGetSaveUrl +# ============================================================================ + + +@pytest.mark.unit +@pytest.mark.loyalty +class TestGetSaveUrl: + """Tests for get_save_url().""" + + @patch("jwt.encode", return_value="test_token") + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_existing_object_references_by_id(self, mock_config, mock_jwt_encode): + """When object exists, JWT payload references by ID only.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + mock_config.google_wallet_origins = ["https://example.com"] + + service = _make_service() + service._private_key = "fake-key" + + card = _make_card(google_object_id=f"{MOCK_ISSUER_ID}.loyalty_card_1") + db = MagicMock() + + url = service.get_save_url(db, card) + + assert url == "https://pay.google.com/gp/v/save/test_token" + claims = mock_jwt_encode.call_args[0][0] + assert claims["payload"]["loyaltyObjects"][0] == {"id": card.google_object_id} + + @patch("jwt.encode", return_value="fat_token") + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_object_creation_fails_uses_fat_jwt(self, mock_config, mock_jwt_encode): + """When object creation fails, JWT embeds full object data.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + mock_config.google_wallet_origins = [] + + service = _make_service() + service._private_key = "fake-key" + # create_object will fail + service._http_client.post.return_value = _mock_response(500, {"error": "fail"}) + + card = _make_card(google_object_id=None) + db = MagicMock() + + url = service.get_save_url(db, card) + + assert "fat_token" in url + claims = mock_jwt_encode.call_args[0][0] + obj = claims["payload"]["loyaltyObjects"][0] + assert "classId" in obj + + @patch("jwt.encode", return_value="token") + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_jwt_claims_structure(self, mock_config, mock_jwt_encode): + """JWT claims include iss, aud, origins, typ, exp.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + mock_config.google_wallet_origins = ["https://app.example.com"] + + service = _make_service() + service._private_key = "fake-key" + + card = _make_card(google_object_id=f"{MOCK_ISSUER_ID}.loyalty_card_1") + db = MagicMock() + + service.get_save_url(db, card) + + claims = mock_jwt_encode.call_args[0][0] + assert claims["iss"] == "test@test.iam.gserviceaccount.com" + assert claims["aud"] == "google" + assert claims["origins"] == ["https://app.example.com"] + assert claims["typ"] == "savetowallet" + assert "iat" in claims + assert "exp" in claims + + +# ============================================================================ +# TestRetryDecorator +# ============================================================================ + + +@pytest.mark.unit +@pytest.mark.loyalty +class TestRetryDecorator: + """Tests for _retry_on_failure decorator.""" + + @patch("app.modules.loyalty.services.google_wallet_service.time") + def test_succeeds_first_try(self, mock_time): + """No retry when function succeeds on first try.""" + call_count = 0 + + @_retry_on_failure + def always_succeed(): + nonlocal call_count + call_count += 1 + return "ok" + + result = always_succeed() + + assert result == "ok" + assert call_count == 1 + mock_time.sleep.assert_not_called() + + @patch("app.modules.loyalty.services.google_wallet_service.time") + def test_retries_on_500_then_succeeds(self, mock_time): + """Retries on 500 error and succeeds on second attempt.""" + call_count = 0 + + @_retry_on_failure + def fail_then_succeed(): + nonlocal call_count + call_count += 1 + if call_count == 1: + raise WalletIntegrationException("google", "Server error 500 ") + return "ok" + + result = fail_then_succeed() + + assert result == "ok" + assert call_count == 2 + mock_time.sleep.assert_called_once() + + @patch("app.modules.loyalty.services.google_wallet_service.time") + def test_404_not_retried(self, mock_time): + """404 error is not retried — immediate failure.""" + call_count = 0 + + @_retry_on_failure + def always_404(): + nonlocal call_count + call_count += 1 + raise WalletIntegrationException("google", "Not found: 404 error") + + with pytest.raises(WalletIntegrationException): + always_404() + + assert call_count == 1 + mock_time.sleep.assert_not_called() + + +# ============================================================================ +# TestGetClassStatus +# ============================================================================ + + +@pytest.mark.unit +@pytest.mark.loyalty +class TestGetClassStatus: + """Tests for get_class_status().""" + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_200_returns_status(self, mock_config): + """200 response returns dict with review_status.""" + mock_config.google_issuer_id = MOCK_ISSUER_ID + mock_config.google_service_account_json = MOCK_SA_PATH + + service = _make_service() + service._http_client.get.return_value = _mock_response( + 200, + {"reviewStatus": "APPROVED", "programName": "Test Rewards"}, + ) + + result = service.get_class_status("test_class_id") + + assert result is not None + assert result["review_status"] == "APPROVED" + assert result["class_id"] == "test_class_id" + + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_not_configured_returns_none(self, mock_config): + """Returns None when Google Wallet is not configured.""" + mock_config.google_issuer_id = None + mock_config.google_service_account_json = None + + service = GoogleWalletService() + result = service.get_class_status("test_class_id") + + assert result is None diff --git a/app/modules/loyalty/tests/unit/test_wallet_service.py b/app/modules/loyalty/tests/unit/test_wallet_service.py index 9dc2a073..14e606e0 100644 --- a/app/modules/loyalty/tests/unit/test_wallet_service.py +++ b/app/modules/loyalty/tests/unit/test_wallet_service.py @@ -224,15 +224,15 @@ class TestCreateWalletObjectsApple: class TestGoogleWalletCreateObject: """Tests that GoogleWalletService.create_object populates DB fields correctly.""" - @patch("app.modules.loyalty.services.google_wallet_service.settings") - def test_create_object_sets_google_object_id(self, mock_settings, db, wt_card, wt_program): + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_create_object_sets_google_object_id(self, mock_config, db, wt_card, wt_program): """create_object sets card.google_object_id in the database.""" from app.modules.loyalty.services.google_wallet_service import ( GoogleWalletService, ) - mock_settings.loyalty_google_issuer_id = "1234567890" - mock_settings.loyalty_google_service_account_json = "/fake/path.json" + mock_config.google_issuer_id = "1234567890" + mock_config.google_service_account_json = "/fake/path.json" # Pre-set class_id on program so create_class isn't called wt_program.google_class_id = "1234567890.loyalty_program_1" @@ -255,15 +255,15 @@ class TestGoogleWalletCreateObject: db.refresh(wt_card) assert wt_card.google_object_id == expected_object_id - @patch("app.modules.loyalty.services.google_wallet_service.settings") - def test_create_object_handles_409_conflict(self, mock_settings, db, wt_card, wt_program): + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_create_object_handles_409_conflict(self, mock_config, db, wt_card, wt_program): """create_object handles 409 (already exists) by still setting the object_id.""" from app.modules.loyalty.services.google_wallet_service import ( GoogleWalletService, ) - mock_settings.loyalty_google_issuer_id = "1234567890" - mock_settings.loyalty_google_service_account_json = "/fake/path.json" + mock_config.google_issuer_id = "1234567890" + mock_config.google_service_account_json = "/fake/path.json" wt_program.google_class_id = "1234567890.loyalty_program_1" db.commit() @@ -282,15 +282,15 @@ class TestGoogleWalletCreateObject: db.refresh(wt_card) assert wt_card.google_object_id == f"1234567890.loyalty_card_{wt_card.id}" - @patch("app.modules.loyalty.services.google_wallet_service.settings") - def test_create_class_sets_google_class_id(self, mock_settings, db, wt_program): + @patch("app.modules.loyalty.services.google_wallet_service.config") + def test_create_class_sets_google_class_id(self, mock_config, db, wt_program): """create_class sets program.google_class_id in the database.""" from app.modules.loyalty.services.google_wallet_service import ( GoogleWalletService, ) - mock_settings.loyalty_google_issuer_id = "1234567890" - mock_settings.loyalty_google_service_account_json = "/fake/path.json" + mock_config.google_issuer_id = "1234567890" + mock_config.google_service_account_json = "/fake/path.json" assert wt_program.google_class_id is None @@ -322,17 +322,17 @@ class TestGoogleWalletCreateObject: class TestEnrollmentWalletCreation: """Tests that enrollment triggers wallet object creation and populates DB fields.""" - @patch("app.modules.loyalty.services.google_wallet_service.settings") + @patch("app.modules.loyalty.services.google_wallet_service.config") def test_enrollment_creates_google_wallet_object( - self, mock_settings, db, wt_program, test_merchant, test_customer + self, mock_config, db, wt_program, test_merchant, test_customer ): """Full enrollment flow creates Google Wallet class + object in DB.""" from app.modules.loyalty.services.google_wallet_service import ( google_wallet_service, ) - mock_settings.loyalty_google_issuer_id = "1234567890" - mock_settings.loyalty_google_service_account_json = "/fake/path.json" + mock_config.google_issuer_id = "1234567890" + mock_config.google_service_account_json = "/fake/path.json" # Mock the HTTP client to simulate Google API success mock_http = MagicMock() @@ -373,13 +373,13 @@ class TestEnrollmentWalletCreation: # Wallet fields should be None since not configured assert card.google_object_id is None - @patch("app.modules.loyalty.services.google_wallet_service.settings") + @patch("app.modules.loyalty.services.google_wallet_service.config") def test_enrollment_with_apple_wallet_sets_serial( - self, mock_settings, db, wt_program_with_apple, test_customer + self, mock_config, db, wt_program_with_apple, test_customer ): """Enrollment sets apple_serial_number when program has apple_pass_type_id.""" - mock_settings.loyalty_google_issuer_id = None - mock_settings.loyalty_google_service_account_json = None + mock_config.google_issuer_id = None + mock_config.google_service_account_json = None from app.modules.loyalty.services.card_service import card_service card = card_service.enroll_customer( diff --git a/middleware/decorators.py b/middleware/decorators.py index 2766301d..6f935dc4 100644 --- a/middleware/decorators.py +++ b/middleware/decorators.py @@ -8,31 +8,78 @@ This module provides classes and functions for: - Consistent error handling for rate limit violations """ +import asyncio from functools import wraps -from app.exceptions.base import RateLimitException # Add this import +from starlette.requests import Request + +from app.exceptions.base import RateLimitException +from middleware.cloudflare import get_real_client_ip from middleware.rate_limiter import RateLimiter # Initialize rate limiter instance rate_limiter = RateLimiter() +def _find_request(*args, **kwargs) -> Request | None: + """Extract a Request object from function args/kwargs.""" + # Check kwargs first (FastAPI usually passes request= as keyword) + for val in kwargs.values(): + if isinstance(val, Request): + return val + # Check positional args (e.g. self, request, ...) + for val in args: + if isinstance(val, Request): + return val + return None + + def rate_limit(max_requests: int = 100, window_seconds: int = 3600): - """Rate limiting decorator for FastAPI endpoints.""" + """Rate limiting decorator for FastAPI endpoints. + + Works with both sync and async endpoint functions. + Extracts the real client IP from the Request object for per-client limiting. + """ def decorator(func): - @wraps(func) - async def wrapper(*args, **kwargs): - client_id = "anonymous" # In production, extract from request + if asyncio.iscoroutinefunction(func): - if not rate_limiter.allow_request(client_id, max_requests, window_seconds): - # Use custom exception instead of HTTPException - raise RateLimitException( - message="Rate limit exceeded", retry_after=window_seconds + @wraps(func) + async def async_wrapper(*args, **kwargs): + request = _find_request(*args, **kwargs) + client_id = ( + get_real_client_ip(request) if request else "anonymous" ) - return await func(*args, **kwargs) + if not rate_limiter.allow_request( + client_id, max_requests, window_seconds + ): + raise RateLimitException( + message="Rate limit exceeded", + retry_after=window_seconds, + ) - return wrapper + return await func(*args, **kwargs) + + return async_wrapper + + @wraps(func) + def sync_wrapper(*args, **kwargs): + request = _find_request(*args, **kwargs) + client_id = ( + get_real_client_ip(request) if request else "anonymous" + ) + + if not rate_limiter.allow_request( + client_id, max_requests, window_seconds + ): + raise RateLimitException( + message="Rate limit exceeded", + retry_after=window_seconds, + ) + + return func(*args, **kwargs) + + return sync_wrapper return decorator