From e2e1c783fc6a57a5b939d09ab8997d138f84f5af Mon Sep 17 00:00:00 2001 From: Robert Wlodarczyk Date: Wed, 25 Feb 2026 09:10:18 -0800 Subject: [PATCH] security: encrypt Discogs consumer key/secret at rest Closes #75. Applies Fernet symmetric encryption (same OAUTH_ENCRYPTION_KEY mechanism introduced in #73) to discogs_consumer_key and discogs_consumer_secret stored in the app_config table. - api/setup.py: encrypt on write (set_config), decrypt before masking (show_config) - api/api.py: decrypt after reading consumer key/secret for OAuth flows - api/syncer.py: decrypt consumer key/secret after reading from app_config - tests: guard existing test against ambient OAUTH_ENCRYPTION_KEY; add tests for encrypted write path and encrypted show_config display The existing plaintext fallback in decrypt_oauth_token handles migration of already-stored plaintext values without a data migration step. Co-Authored-By: Claude Sonnet 4.6 --- api/api.py | 10 +++++- api/setup.py | 12 +++++-- api/syncer.py | 3 ++ tests/api/test_setup.py | 75 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 96 insertions(+), 4 deletions(-) diff --git a/api/api.py b/api/api.py index 3d3a6e46..d1768c25 100644 --- a/api/api.py +++ b/api/api.py @@ -25,7 +25,7 @@ import structlog import uvicorn -from api.auth import encrypt_oauth_token +from api.auth import decrypt_oauth_token, encrypt_oauth_token from api.limiter import limiter from api.models import LoginRequest, RegisterRequest import api.routers.explore as _explore_router @@ -495,6 +495,10 @@ async def authorize_discogs( consumer_key = await _get_app_config("discogs_consumer_key") consumer_secret = await _get_app_config("discogs_consumer_secret") + if consumer_key: + consumer_key = decrypt_oauth_token(consumer_key, _config.oauth_encryption_key) + if consumer_secret: + consumer_secret = decrypt_oauth_token(consumer_secret, _config.oauth_encryption_key) if not consumer_key or not consumer_secret: raise HTTPException( @@ -551,6 +555,10 @@ async def verify_discogs( consumer_key = await _get_app_config("discogs_consumer_key") consumer_secret = await _get_app_config("discogs_consumer_secret") + if consumer_key: + consumer_key = decrypt_oauth_token(consumer_key, _config.oauth_encryption_key) + if consumer_secret: + consumer_secret = decrypt_oauth_token(consumer_secret, _config.oauth_encryption_key) if not consumer_key or not consumer_secret: raise HTTPException( diff --git a/api/setup.py b/api/setup.py index a2d5e47c..f5c9a5f1 100644 --- a/api/setup.py +++ b/api/setup.py @@ -12,6 +12,8 @@ import psycopg from psycopg.rows import dict_row +from api.auth import decrypt_oauth_token, encrypt_oauth_token + def _build_conninfo() -> str: """Build a psycopg conninfo string from environment variables.""" @@ -54,18 +56,24 @@ def _mask(value: str) -> str: def show_config(conninfo: str) -> None: """Print current Discogs credentials (masked).""" + encryption_key = os.environ.get("OAUTH_ENCRYPTION_KEY") or None with psycopg.connect(conninfo) as conn, conn.cursor(row_factory=dict_row) as cur: cur.execute("SELECT key, value FROM app_config WHERE key IN ('discogs_consumer_key', 'discogs_consumer_secret')") rows = {row["key"]: row["value"] for row in cur.fetchall()} - key_val = rows.get("discogs_consumer_key", "") - secret_val = rows.get("discogs_consumer_secret", "") + key_val = decrypt_oauth_token(rows.get("discogs_consumer_key", ""), encryption_key) + secret_val = decrypt_oauth_token(rows.get("discogs_consumer_secret", ""), encryption_key) print(f"discogs_consumer_key: {_mask(key_val)}") print(f"discogs_consumer_secret: {_mask(secret_val)}") def set_config(conninfo: str, consumer_key: str, consumer_secret: str) -> None: """Upsert Discogs credentials into the app_config table.""" + encryption_key = os.environ.get("OAUTH_ENCRYPTION_KEY") or None + if encryption_key: + consumer_key = encrypt_oauth_token(consumer_key, encryption_key) + consumer_secret = encrypt_oauth_token(consumer_secret, encryption_key) + upsert_sql = """ INSERT INTO app_config (key, value, updated_at) VALUES (%s, %s, NOW()) diff --git a/api/syncer.py b/api/syncer.py index 11d1fb70..648fd6df 100644 --- a/api/syncer.py +++ b/api/syncer.py @@ -419,6 +419,9 @@ async def run_full_sync( await cur.execute("SELECT key, value FROM app_config WHERE key IN ('discogs_consumer_key', 'discogs_consumer_secret')") config_rows = await cur.fetchall() app_config = {row["key"]: row["value"] for row in config_rows} + for cred_key in ("discogs_consumer_key", "discogs_consumer_secret"): + if cred_key in app_config: + app_config[cred_key] = decrypt_oauth_token(app_config[cred_key], oauth_encryption_key) if "discogs_consumer_key" not in app_config or "discogs_consumer_secret" not in app_config: raise ValueError("Discogs app credentials not configured in app_config table") diff --git a/tests/api/test_setup.py b/tests/api/test_setup.py index be2f2021..03e97e2a 100644 --- a/tests/api/test_setup.py +++ b/tests/api/test_setup.py @@ -119,6 +119,44 @@ def test_show_prints_masked_values(self, capsys: pytest.CaptureFixture[str]) -> assert "mykey12345" not in captured.out assert "mysecret678" not in captured.out + def test_show_decrypts_encrypted_values(self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: + from cryptography.fernet import Fernet + + from api.setup import show_config + + encryption_key = Fernet.generate_key().decode("ascii") + monkeypatch.setenv("OAUTH_ENCRYPTION_KEY", encryption_key) + + f = Fernet(encryption_key.encode("ascii")) + encrypted_key = f.encrypt(b"mykey12345").decode("ascii") + encrypted_secret = f.encrypt(b"mysecret678").decode("ascii") + + mock_rows = [ + {"key": "discogs_consumer_key", "value": encrypted_key}, + {"key": "discogs_consumer_secret", "value": encrypted_secret}, + ] + + mock_cur = MagicMock() + mock_cur.__enter__ = MagicMock(return_value=mock_cur) + mock_cur.__exit__ = MagicMock(return_value=False) + mock_cur.fetchall = MagicMock(return_value=mock_rows) + + mock_conn = MagicMock() + mock_conn.__enter__ = MagicMock(return_value=mock_conn) + mock_conn.__exit__ = MagicMock(return_value=False) + mock_conn.cursor = MagicMock(return_value=mock_cur) + + with patch("psycopg.connect", return_value=mock_conn): + show_config("host=localhost dbname=test") + + captured = capsys.readouterr() + # Decrypted values should be masked (first/last 2 chars visible) + assert "my" in captured.out # first 2 chars of "mykey12345" + assert "45" in captured.out # last 2 chars of "mykey12345" + # Raw encrypted ciphertext must not appear in output + assert encrypted_key not in captured.out + assert encrypted_secret not in captured.out + def test_show_handles_unset_values(self, capsys: pytest.CaptureFixture[str]) -> None: from api.setup import show_config @@ -142,9 +180,11 @@ def test_show_handles_unset_values(self, capsys: pytest.CaptureFixture[str]) -> class TestSetConfig: """Tests for set_config.""" - def test_upserts_both_keys(self, capsys: pytest.CaptureFixture[str]) -> None: + def test_upserts_both_keys(self, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]) -> None: from api.setup import set_config + monkeypatch.delenv("OAUTH_ENCRYPTION_KEY", raising=False) + mock_cur = MagicMock() mock_cur.__enter__ = MagicMock(return_value=mock_cur) mock_cur.__exit__ = MagicMock(return_value=False) @@ -173,6 +213,39 @@ def test_upserts_both_keys(self, capsys: pytest.CaptureFixture[str]) -> None: assert "✅" in captured.out assert "updated successfully" in captured.out + def test_encrypts_values_when_key_set(self, monkeypatch: pytest.MonkeyPatch) -> None: + from cryptography.fernet import Fernet + + from api.setup import set_config + + encryption_key = Fernet.generate_key().decode("ascii") + monkeypatch.setenv("OAUTH_ENCRYPTION_KEY", encryption_key) + + mock_cur = MagicMock() + mock_cur.__enter__ = MagicMock(return_value=mock_cur) + mock_cur.__exit__ = MagicMock(return_value=False) + + mock_conn = MagicMock() + mock_conn.__enter__ = MagicMock(return_value=mock_conn) + mock_conn.__exit__ = MagicMock(return_value=False) + mock_conn.cursor = MagicMock(return_value=mock_cur) + + with patch("psycopg.connect", return_value=mock_conn): + set_config("host=localhost dbname=test", "mykey", "mysecret") + + calls = mock_cur.execute.call_args_list + stored_key_val = calls[0][0][1][1] + stored_secret_val = calls[1][0][1][1] + + # Stored values must not be the original plaintext + assert stored_key_val != "mykey" + assert stored_secret_val != "mysecret" + + # Stored values must decrypt back to originals + f = Fernet(encryption_key.encode("ascii")) + assert f.decrypt(stored_key_val.encode("ascii")).decode("utf-8") == "mykey" + assert f.decrypt(stored_secret_val.encode("ascii")).decode("utf-8") == "mysecret" + class TestMain: """Tests for the main() entry point."""