From 0b12b71e84455a59be728159f4bbef23fcc4991d Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Tue, 21 Jan 2025 11:07:28 +0100 Subject: [PATCH] fixes #15 - no issuer_id from settings --- .github/workflows/tests.yaml | 2 +- src/edutap/wallet_google/api.py | 8 ++----- src/edutap/wallet_google/handlers/fastapi.py | 1 - src/edutap/wallet_google/handlers/validate.py | 15 +++++++++---- src/edutap/wallet_google/settings.py | 4 +--- tests/conftest.py | 7 +++++++ .../test_wallet_google_plugins/plugins.py | 8 ++++--- tests/integration/test_CRULM.py | 11 +++++++--- tests/test_api_save_link.py | 21 ++++++++++++------- tests/test_handler_fastapi.py | 6 +++--- tests/test_settings.py | 4 ++-- 11 files changed, 53 insertions(+), 34 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index ec57599..121c64d 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -60,7 +60,7 @@ jobs: if: steps.changed-models-or-api.outputs.any_changed == 'true' run: | echo "${{ secrets.SERVICE_ACCOUNT_JSON }}" > /tmp/edutap-wallet-google-credentials.json - echo "EDUTAP_WALLET_GOOGLE_ISSUER_ID=${{ secrets.ISSUER_ID }}" > .env + echo "EDUTAP_WALLET_GOOGLE_TEST_ISSUER_ID=${{ secrets.ISSUER_ID }}" > .env echo "EDUTAP_WALLET_GOOGLE_CREDENTIALS_FILE=/tmp/edutap-wallet-google-credentials.json" >> .env echo "EDUTAP_WALLET_GOOGLE_INTEGRATION_TEST_PREFIX=${{ github.run_id }}" >> .env - name: Test with tox diff --git a/src/edutap/wallet_google/api.py b/src/edutap/wallet_google/api.py index 71c0913..05ef0ec 100644 --- a/src/edutap/wallet_google/api.py +++ b/src/edutap/wallet_google/api.py @@ -290,13 +290,9 @@ def listing( raise ValueError("resource_id of a class must be given to list its objects") params["classId"] = resource_id elif name.endswith("Class"): - is_pageable = True if not issuer_id: - issuer_id = session_manager.settings.issuer_id - if not issuer_id: - raise ValueError( - "'issuer_id' must be passed as keyword argument or set in environment" - ) + raise ValueError("issuer_id must be given to list classes") + is_pageable = True params["issuerId"] = issuer_id if is_pageable: diff --git a/src/edutap/wallet_google/handlers/fastapi.py b/src/edutap/wallet_google/handlers/fastapi.py index b91d854..8945859 100644 --- a/src/edutap/wallet_google/handlers/fastapi.py +++ b/src/edutap/wallet_google/handlers/fastapi.py @@ -145,7 +145,6 @@ async def handle_image(request: Request, encrypted_image_id: str): # needs to be included after the routers are defined router = APIRouter( prefix=session_manager.settings.handler_prefix, - tags=["edutap", "google_wallet"], ) router.include_router(router_callback) router.include_router(router_images) diff --git a/src/edutap/wallet_google/handlers/validate.py b/src/edutap/wallet_google/handlers/validate.py index b7d039b..231745e 100644 --- a/src/edutap/wallet_google/handlers/validate.py +++ b/src/edutap/wallet_google/handlers/validate.py @@ -138,11 +138,18 @@ def verified_signed_message(data: CallbackData) -> SignedMessage: Verifies the signature of the callback data. and returns the parsed SignedMessage """ + # parse message + message = SignedMessage.model_validate_json(data.signedMessage) + # get issuer_id + if not message.classId or "." not in message.classId: + raise ValueError("Missing classId") + issuer_id = message.classId.split(".")[0] + + # shortcut if signature validation is disabled settings = session_manager.settings if settings.handler_callback_verify_signature == "0": - # shortcut if signature validation is disabled - return SignedMessage.model_validate_json(data.signedMessage) + return message if data.protocolVersion != PROTOCOL_VERSION: raise ValueError("Invalid protocolVersion") @@ -166,7 +173,7 @@ def verified_signed_message(data: CallbackData) -> SignedMessage: signature = base64.decodebytes(bytes(data.signature, "utf-8")) signed_data = _construct_signed_data( "GooglePayWallet", - settings.issuer_id, + issuer_id, PROTOCOL_VERSION, data.signedMessage, ) @@ -175,4 +182,4 @@ def verified_signed_message(data: CallbackData) -> SignedMessage: except (ValueError, InvalidSignature): raise ValueError("Invalid signature") - return SignedMessage.model_validate_json(data.signedMessage) + return message diff --git a/src/edutap/wallet_google/settings.py b/src/edutap/wallet_google/settings.py index ee384b7..3046fb9 100644 --- a/src/edutap/wallet_google/settings.py +++ b/src/edutap/wallet_google/settings.py @@ -1,5 +1,4 @@ from pathlib import Path -from pydantic import EmailStr from pydantic import Field from pydantic import HttpUrl from pydantic_settings import BaseSettings @@ -47,8 +46,7 @@ class Settings(BaseSettings): credentials_file: Path = ROOT_DIR / "tests" / "data" / "credentials_fake.json" credentials_scopes: list[str] = SCOPES - issuer_account_email: EmailStr | None = None - issuer_id: str = Field(default="") + test_issuer_id: str = Field(default="") fernet_encryption_key: str = "" diff --git a/tests/conftest.py b/tests/conftest.py index a42ca07..628afbe 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -99,3 +99,10 @@ def mock_settings(): @pytest.fixture def mock_fernet_encryption_key(mock_settings): mock_settings.fernet_encryption_key = "TDTPJVv24gha-jRX0apPgPpMDN2wX1kVSNNZdWXcz8E=" + + +@pytest.fixture +def test_issuer_id(): + from edutap.wallet_google.session import session_manager + + yield session_manager.settings.test_issuer_id diff --git a/tests/data/test_wallet_google_plugins/plugins.py b/tests/data/test_wallet_google_plugins/plugins.py index 812415b..3e71029 100644 --- a/tests/data/test_wallet_google_plugins/plugins.py +++ b/tests/data/test_wallet_google_plugins/plugins.py @@ -24,6 +24,8 @@ async def image_by_id(self, image_id: str) -> ImageData: class TestCallbackHandler: """ Implementation of edutap.wallet_google.protocols.CallbackHandler + + Used in tests to simulate a callback handler and possible errors. """ async def handle( @@ -35,8 +37,8 @@ async def handle( count: int, nonce: str, ) -> None: - if class_id == "TIMEOUT": + if class_id.startswith("TIMEOUT"): await asyncio.sleep(exp_time_millis / 1000) - elif class_id: + elif nonce: return - raise ValueError("class_id is required") + raise ValueError("test case errors if nonce is 0") diff --git a/tests/integration/test_CRULM.py b/tests/integration/test_CRULM.py index b72282e..cd0ab13 100644 --- a/tests/integration/test_CRULM.py +++ b/tests/integration/test_CRULM.py @@ -130,7 +130,7 @@ def test_class_object_cru(type_base, class_data, object_data, integration_test_i ############################ # test class class_base = f"{integration_test_id}.{class_type}.test_CRU.wallet_google.edutap" - class_data["id"] = f"{session_manager.settings.issuer_id}.{class_base}" + class_data["id"] = f"{session_manager.settings.test_issuer_id}.{class_base}" data = new(class_type, class_data) @@ -186,14 +186,19 @@ def test_class_object_cru(type_base, class_data, object_data, integration_test_i assert result_message.id == class_data["id"] # list all - result_list = [x for x in listing(name=class_type)] + result_list = [ + x + for x in listing( + name=class_type, issuer_id=session_manager.settings.test_issuer_id + ) + ] assert len(result_list) > 0 ############################ # test object object_data["classId"] = class_data["id"] object_base = f"{integration_test_id}.{object_type}.test_CRU.wallet_google.edutap" - object_data["id"] = f"{session_manager.settings.issuer_id}.{object_base}" + object_data["id"] = f"{session_manager.settings.test_issuer_id}.{object_base}" odata = new(object_type, object_data) # create diff --git a/tests/test_api_save_link.py b/tests/test_api_save_link.py index e0506a3..59bf56e 100644 --- a/tests/test_api_save_link.py +++ b/tests/test_api_save_link.py @@ -33,7 +33,7 @@ def test_create_claims(): ), ] expected = { - "iss": "123456789", + "iss": "test@example.com", "aud": "google", "typ": "savetowallet", "iat": "1737115974", @@ -55,7 +55,7 @@ def test_create_claims(): } claims = api._create_claims( - "123456789", + "test@example.com", [], models, iat=datetime.datetime(2025, 1, 17, 12, 12, 54, 0, datetime.timezone.utc), @@ -74,7 +74,6 @@ def test_create_claims(): def test_api_save_link(mock_settings): from edutap.wallet_google.settings import ROOT_DIR - mock_settings.issuer_id = "1234567890123456789" mock_settings.credentials_file = ( ROOT_DIR / "tests" / "data" / "credentials_fake.json" ) @@ -84,17 +83,23 @@ def test_api_save_link(mock_settings): link = api.save_link( [ api.new( - "Reference", {"id": "test-1.edutap.eu", "model_name": "GenericObject"} + "Reference", + { + "id": "1234567890123456789.test-1.edutap.eu", + "model_name": "GenericObject", + }, ), api.new( "OfferObject", - {"id": "test-2.edutap.eu", "classId": "test-class-1.edutap.eu"}, + { + "id": "1234567890123456789.test-2.edutap.eu", + "classId": "1234567890123456789.test-class-1.edutap.eu", + }, ), ], - iat=datetime.datetime(2025, 1, 22, 10, 20, 0, 0, datetime.timezone.utc) - + iat=datetime.datetime(2025, 1, 22, 10, 20, 0, 0, datetime.timezone.utc), ) - expected = "https://pay.google.com/gp/v/save/eyJ0eXAiOiAiSldUIiwgImFsZyI6ICJSUzI1NiIsICJraWQiOiAiMTIzNDU2Nzg5MGFiY2RlZjEyMzQ1Njc4OTBhYmNkZWYxMjM0NTY3OCJ9.eyJpc3MiOiAiZWR1dGFwLXRlc3QtZXhhbXBsZUBzb2RpdW0tcmF5LTEyMzQ1Ni5pYW0uZ3NlcnZpY2VhY2NvdW50LmNvbSIsICJhdWQiOiAiZ29vZ2xlIiwgInR5cCI6ICJzYXZldG93YWxsZXQiLCAiaWF0IjogIjE3Mzc1NDEyMDAiLCAiZXhwIjogIiIsICJwYXlsb2FkIjogeyJvZmZlck9iamVjdHMiOiBbeyJpZCI6ICJ0ZXN0LTIuZWR1dGFwLmV1IiwgImNsYXNzSWQiOiAidGVzdC1jbGFzcy0xLmVkdXRhcC5ldSIsICJzdGF0ZSI6ICJTVEFURV9VTlNQRUNJRklFRCIsICJoYXNMaW5rZWREZXZpY2UiOiBmYWxzZSwgImRpc2FibGVFeHBpcmF0aW9uTm90aWZpY2F0aW9uIjogZmFsc2UsICJub3RpZnlQcmVmZXJlbmNlIjogIk5PVElGSUNBVElPTl9TRVRUSU5HU19GT1JfVVBEQVRFU19VTlNQRUNJRklFRCJ9XSwgImdlbmVyaWNPYmplY3RzIjogW3siaWQiOiAidGVzdC0xLmVkdXRhcC5ldSJ9XX0sICJvcmlnaW5zIjogW119.u8xDMKKdPBB0yjYqR-uM4eAYMEskRZyv_AOBhGkZ0oswvr-nVOs4jogXZo6cOmSvzjE_tRviNf_GHDelOaND-c4AqNwTg13DRG0c-aNWKbROTlrZefG0dusPcAuhTwzG-gsDn_sCstHWy8gkKQOmb_x4RjRB-b_gsv2uhmeKtNPvofxBNLUHbOefYKL12PPII9kI00Dl0pAyh0dgqI3yew0197a2rYl6_lOlYfO4jd784b-3CDCDKpOZnEjqBBedbLSDhKdWV10eo9mz6OsgqydERuUDDzhJopkwz6BIFL_HA_IHeAaiLtoSNbuOqc7zUecgOHlqecaWBZhV_-WPkQ" + expected = "https://pay.google.com/gp/v/save/eyJ0eXAiOiAiSldUIiwgImFsZyI6ICJSUzI1NiIsICJraWQiOiAiMTIzNDU2Nzg5MGFiY2RlZjEyMzQ1Njc4OTBhYmNkZWYxMjM0NTY3OCJ9.eyJpc3MiOiAiZWR1dGFwLXRlc3QtZXhhbXBsZUBzb2RpdW0tcmF5LTEyMzQ1Ni5pYW0uZ3NlcnZpY2VhY2NvdW50LmNvbSIsICJhdWQiOiAiZ29vZ2xlIiwgInR5cCI6ICJzYXZldG93YWxsZXQiLCAiaWF0IjogIjE3Mzc1NDEyMDAiLCAiZXhwIjogIiIsICJwYXlsb2FkIjogeyJvZmZlck9iamVjdHMiOiBbeyJpZCI6ICIxMjM0NTY3ODkwMTIzNDU2Nzg5LnRlc3QtMi5lZHV0YXAuZXUiLCAiY2xhc3NJZCI6ICIxMjM0NTY3ODkwMTIzNDU2Nzg5LnRlc3QtY2xhc3MtMS5lZHV0YXAuZXUiLCAic3RhdGUiOiAiU1RBVEVfVU5TUEVDSUZJRUQiLCAiaGFzTGlua2VkRGV2aWNlIjogZmFsc2UsICJkaXNhYmxlRXhwaXJhdGlvbk5vdGlmaWNhdGlvbiI6IGZhbHNlLCAibm90aWZ5UHJlZmVyZW5jZSI6ICJOT1RJRklDQVRJT05fU0VUVElOR1NfRk9SX1VQREFURVNfVU5TUEVDSUZJRUQifV0sICJnZW5lcmljT2JqZWN0cyI6IFt7ImlkIjogIjEyMzQ1Njc4OTAxMjM0NTY3ODkudGVzdC0xLmVkdXRhcC5ldSJ9XX0sICJvcmlnaW5zIjogW119.OJcaFwQ9XlKvMb0t-aokejx39zeHiPMIkA5EQSU7oPXgFy740OHFN0-vsw-8ABMc4HB-ZipuH6ZN1MhbOc1baabPl4s8XLU_Z43NMUXmKKz2KExt2F7qiuy_04aeFZQVBGoZOCQicjD_qrh6YqGYHdQFkcjtOAAdy0JPwVsfdiowTdDv4EdH2eZXLvE3bKYeu1rvD6oBS5xCIDwu77t3DurrKYf1yYfOL4YpnNAwru-o7MM_J-FKzVur6wJ0xQgYCP_UtX8PxHoj2-YXQ0yXlypgw8Vpo069hcrOVyQaRdrFEVf_TKi6IBLpPr6zxk19NhmJ9qjwIUygMEkyfRf3Mw" assert link == expected diff --git a/tests/test_handler_fastapi.py b/tests/test_handler_fastapi.py index 1454860..ba89b1d 100644 --- a/tests/test_handler_fastapi.py +++ b/tests/test_handler_fastapi.py @@ -43,7 +43,7 @@ def test_callback_disabled_signature_check_ERROR(mock_settings): mock_settings.handler_callback_verify_signature = "0" callback_data = CallbackData(**real_callback_data) - callback_data.signedMessage = '{"classId":"","objectId":"","eventType":"save","expTimeMillis":1734366082269,"count":1,"nonce":""}' + callback_data.signedMessage = '{"classId":"1.x","objectId":"1.y","eventType":"save","expTimeMillis":1734366082269,"count":1,"nonce":""}' from edutap.wallet_google.handlers.fastapi import router @@ -73,7 +73,7 @@ def raise_not_implemented(): ) callback_data = CallbackData(**real_callback_data) - callback_data.signedMessage = '{"classId":"","objectId":"","eventType":"save","expTimeMillis":1734366082269,"count":1,"nonce":""}' + callback_data.signedMessage = '{"classId":"1.x","objectId":"1.y","eventType":"save","expTimeMillis":1734366082269,"count":1,"nonce":"abcde"}' from edutap.wallet_google.handlers.fastapi import router @@ -96,7 +96,7 @@ def test_callback_disabled_signature_check_TIMEOUT(mock_settings): mock_settings.handlers_callback_timeout = 0.1 callback_data = CallbackData(**real_callback_data) - callback_data.signedMessage = '{"classId":"TIMEOUT","objectId":"","eventType":"save","expTimeMillis":250,"count":1,"nonce":""}' + callback_data.signedMessage = '{"classId":"TIMEOUT.x","objectId":"1.x","eventType":"save","expTimeMillis":250,"count":1,"nonce":"abcde"}' from edutap.wallet_google.handlers.fastapi import router diff --git a/tests/test_settings.py b/tests/test_settings.py index 51da492..4c56bd9 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -19,7 +19,7 @@ def test_base_settings(): def test_local_settings(monkeypatch): monkeypatch.setenv( - "EDUTAP_WALLET_GOOGLE_ISSUER_ID", + "EDUTAP_WALLET_GOOGLE_TEST_ISSUER_ID", "1234567890123456789", ) monkeypatch.setenv( @@ -29,7 +29,7 @@ def test_local_settings(monkeypatch): settings = Settings() - assert settings.issuer_id == "1234567890123456789" + assert settings.test_issuer_id == "1234567890123456789" assert settings.credentials_file.exists()