From 1a27f3f1ba0b696cc829f0dc59ecf4c79fc7c35b Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 3 Apr 2023 19:26:38 +0200 Subject: [PATCH 1/2] Remove hard coded secret for CTMS/basket integration tests --- .gitignore | 1 + Makefile | 9 ++++++++- bin/integration-test.sh | 3 --- ctms/bin/client_credentials.py | 17 ++++++++++++++++ docker-compose.yaml | 4 ++-- tests/integration/basket.env | 3 --- tests/integration/ctms-db-init.sql | 20 ------------------- .../test_basket_waitlist_subscription.py | 10 ++++------ 8 files changed, 32 insertions(+), 35 deletions(-) delete mode 100644 tests/integration/ctms-db-init.sql diff --git a/.gitignore b/.gitignore index 26fab617..3d3cdbab 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ .coverage .DS_STORE .env +.env.tests .idea/ .mypy_cache/ .tox/ diff --git a/Makefile b/Makefile index 1d5b4a44..3a84f4f5 100644 --- a/Makefile +++ b/Makefile @@ -87,8 +87,15 @@ ifneq (1, ${MK_KEEP_DOCKER_UP}) ${DOCKER_COMPOSE} down endif +.env.tests: .env setup $(INSTALL_STAMP) + cp tests/integration/basket.env .env.tests + poetry run ctms/bin/client_credentials.py -e master@local.host integration-test --save-file creds.json + echo "CTMS_CLIENT_ID=$(jq -r .client_id creds.json)" >> .env.tests + echo "CTMS_CLIENT_SECRET=$(jq -r .client_secret creds.json)" >> .env.tests + rm creds.json + .PHONY: integration-test -integration-test: .env setup $(INSTALL_STAMP) +integration-test: .env.tests ${DOCKER_COMPOSE} --profile integration-test up --wait basket bin/integration-test.sh ifneq (1, ${MK_KEEP_DOCKER_UP}) diff --git a/bin/integration-test.sh b/bin/integration-test.sh index 55552256..393fb593 100755 --- a/bin/integration-test.sh +++ b/bin/integration-test.sh @@ -14,8 +14,5 @@ export TZ=UTC # Create newsletters in basket cat tests/integration/basket-db-init.sql | $DOCKER_COMPOSE exec -T mysql mysql -u root -h mysql basket -# Create token in CTMS (will only work with specific CTMS_SECRET, see .sql source) -cat tests/integration/ctms-db-init.sql | $DOCKER_COMPOSE exec -T postgres psql --user postgres -d postgres - # docker-compose run basket django-admin loaddata ./basket/news/fixtures/newsletters.json $POETRY_RUN pytest tests/integration/ diff --git a/ctms/bin/client_credentials.py b/ctms/bin/client_credentials.py index 6efa9cb2..99f0724e 100755 --- a/ctms/bin/client_credentials.py +++ b/ctms/bin/client_credentials.py @@ -2,6 +2,7 @@ """Generate OAuth2 client credentials.""" import argparse +import json import re from secrets import token_urlsafe @@ -99,6 +100,7 @@ def main(db, settings, test_args=None): parser = argparse.ArgumentParser(description="Create or update client credentials.") parser.add_argument("name", help="short name of the client") parser.add_argument("-e", "--email", help="contact email for the client") + parser.add_argument("--save-file", help="output credentials into file in JSON format") parser.add_argument( "--enable", action="store_true", help="enable a disabled client" ) @@ -115,6 +117,7 @@ def main(db, settings, test_args=None): enable = args.enable disable = args.disable rotate = args.rotate_secret + save_file = args.save_file if not re.match(r"^[-_.a-zA-Z0-9]*$", name): print( @@ -158,6 +161,13 @@ def main(db, settings, test_args=None): sample_email=email, enabled=enabled, ) + if save_file: + with open(save_file, "w") as f: + json.dump({ + "client_id": existing.client_id, + "client_secret": new_secret, + }, f) + print(f"Credentials saved to {save_file!r}.") else: print(f"Credentials for {name} are updated.") else: @@ -171,6 +181,13 @@ def main(db, settings, test_args=None): print_new_credentials( client_id, client_secret, settings, sample_email=email, enabled=enabled ) + if save_file: + with open(save_file, "w") as f: + json.dump({ + "client_id": client_id, + "client_secret": client_secret, + }, f) + print(f"Credentials saved to {save_file!r}.") return 0 diff --git a/docker-compose.yaml b/docker-compose.yaml index 3bbc6c8a..59710bc6 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -48,7 +48,7 @@ services: profiles: [integration-test] image: mozmeao/basket env_file: - - ./tests/integration/basket.env + - .env.tests command: ./bin/run-dev.sh ports: @@ -69,7 +69,7 @@ services: profiles: [integration-test] image: mozmeao/basket env_file: - - ./tests/integration/basket.env + - .env.tests command: ./bin/run-worker.sh depends_on: diff --git a/tests/integration/basket.env b/tests/integration/basket.env index 9e90842b..b2f47b0d 100644 --- a/tests/integration/basket.env +++ b/tests/integration/basket.env @@ -3,9 +3,6 @@ REDIS_URL=redis://redis:6379 CTMS_ENABLED=True CTMS_URL=http://web:8000 -CTMS_CLIENT_ID=id_integration-test -# See `ctms-db-init.sql` -CTMS_CLIENT_SECRET=bogus_MzOWu8UMz5N6M4--2iX9jgJ05JX5MziH6KeH8dI6hrw # pragma: allowlist secret DEBUG=True ALLOWED_HOSTS=* SECRET_KEY=sssssssshhhhhhhhhh # pragma: allowlist secret diff --git a/tests/integration/ctms-db-init.sql b/tests/integration/ctms-db-init.sql deleted file mode 100644 index 65bd5482..00000000 --- a/tests/integration/ctms-db-init.sql +++ /dev/null @@ -1,20 +0,0 @@ -INSERT INTO api_client ( - client_id, - email, - enabled, - hashed_secret, - create_timestamp, - update_timestamp -) VALUES ( - 'id_integration-test', -- client_id - 'master@local.host', -- email - true, -- enabled - -- client_id=id_integration-test - -- secret=bogus_MzOWu8UMz5N6M4--2iX9jgJ05JX5MziH6KeH8dI6hrw pragma: allowlist secret - -- To generate it, we used: - -- $ CTMS_DB_URL="postgresql://ctmsuser:ctmsuser@localhost/ctms" CTMS_SECRET_KEY="some-secret" poetry run ctms/bin/client_credentials.py -e master@local.host integration-test # pragma: allowlist secret - -- Use the same secret key when running CTMS, or integration tests authentication won't work. - '$argon2id$v=19$m=65536,t=3,p=4$zznnfG+NMeZ8D8HYu/f+Hw$edz+Kc8/8MqL9yig2MXFTAlnpJ2vnkUyvnuRe5QERcA', -- hashed_secret - '2023-03-15 16:52:38.542769+01'::timestamp, -- create_timestamp - '2023-03-15 16:52:38.542769+01'::timestamp -- update_timestamp -) ON CONFLICT DO NOTHING; diff --git a/tests/integration/test_basket_waitlist_subscription.py b/tests/integration/test_basket_waitlist_subscription.py index 6c16d14b..9f38645c 100644 --- a/tests/integration/test_basket_waitlist_subscription.py +++ b/tests/integration/test_basket_waitlist_subscription.py @@ -1,5 +1,4 @@ -import os -import time +from pathlib import Path from uuid import uuid4 import backoff @@ -7,18 +6,17 @@ import requests from pydantic import BaseSettings -TEST_FOLDER = os.path.dirname(os.path.realpath(__file__)) +ROOT_FOLDER = Path(__file__).parent.parent class Settings(BaseSettings): basket_server_url: str = "http://127.0.0.1:9000" ctms_server_url: str = "http://127.0.0.1:8000" - # We initialize CTMS api client id/secret in `ctms-db-init.sql` - ctms_client_id: str = "id_integration-test" + ctms_client_id: str ctms_client_secret: str class Config: - env_file = os.path.join(TEST_FOLDER, "basket.env") + env_file = str(ROOT_FOLDER / ".env.tests") settings = Settings() From af86c9f0841032736d8abeddf345f66c808c8c7b Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 9 Jan 2025 10:21:45 +0100 Subject: [PATCH 2/2] Fix PLR0915 Too many statements --- ctms/bin/client_credentials.py | 41 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/ctms/bin/client_credentials.py b/ctms/bin/client_credentials.py index f94ef2b8..1b40b152 100755 --- a/ctms/bin/client_credentials.py +++ b/ctms/bin/client_credentials.py @@ -87,6 +87,18 @@ def print_new_credentials( ) +def save_json(output, client_id, client_secret): + with open(output, "w") as f: + json.dump( + { + "client_id": client_id, + "client_secret": client_secret, + }, + f, + ) + print(f"Credentials saved to {output!r}.") + + def main(db, settings, test_args=None): # noqa: PLR0912 """ Process the command line and create or update client credentials @@ -100,7 +112,9 @@ def main(db, settings, test_args=None): # noqa: PLR0912 parser = argparse.ArgumentParser(description="Create or update client credentials.") parser.add_argument("name", help="short name of the client") parser.add_argument("-e", "--email", help="contact email for the client") - parser.add_argument("--save-file", help="output credentials into file in JSON format") + parser.add_argument( + "--save-file", help="output credentials into file in JSON format" + ) parser.add_argument( "--enable", action="store_true", help="enable a disabled client" ) @@ -129,10 +143,8 @@ def main(db, settings, test_args=None): # noqa: PLR0912 print("Can only pick one of --enable and --disable") return 1 - if name.startswith("id_"): - client_id = name - else: - client_id = f"id_{name}" + client_id = name if name.startswith("id_") else f"id_{name}" + existing = get_api_client_by_id(db, client_id) if existing: if disable and existing.enabled: @@ -142,10 +154,7 @@ def main(db, settings, test_args=None): # noqa: PLR0912 else: enabled = None - if rotate: - new_secret = create_secret() - else: - new_secret = None + new_secret = create_secret() if rotate else None if new_secret is None and enabled is None and email in (None, existing.email): print(f"Nothing to change for existing credentials for {name}.") @@ -162,12 +171,7 @@ def main(db, settings, test_args=None): # noqa: PLR0912 enabled=enabled, ) if save_file: - with open(save_file, "w") as f: - json.dump({ - "client_id": existing.client_id, - "client_secret": new_secret, - }, f) - print(f"Credentials saved to {save_file!r}.") + save_json(save_file, existing.client_id, new_secret) else: print(f"Credentials for {name} are updated.") else: @@ -182,12 +186,7 @@ def main(db, settings, test_args=None): # noqa: PLR0912 client_id, client_secret, settings, sample_email=email, enabled=enabled ) if save_file: - with open(save_file, "w") as f: - json.dump({ - "client_id": client_id, - "client_secret": client_secret, - }, f) - print(f"Credentials saved to {save_file!r}.") + save_json(save_file, client_id, client_secret) return 0