From b9462577e0324ea615a0b1612744f1214e2bfb00 Mon Sep 17 00:00:00 2001 From: Ravi Bhankharia Date: Wed, 28 Jun 2023 08:52:38 -0700 Subject: [PATCH 1/3] Validate client ID in DID Token. --- CHANGELOG.md | 8 +++++++ magic_admin/magic.py | 8 +++++++ magic_admin/resources/token.py | 6 +++++ tests/integration/magic_test.py | 17 +++++++++++++ tests/integration/resources/token_test.py | 10 +++++++- tests/unit/magic_test.py | 29 +++++++++++++++++------ tests/unit/resources/token_test.py | 7 +++++- 7 files changed, 76 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2db1ac..902aaf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ - ... +## `2.0.0` - 07/05/2023 + +#### Added + +- PR-#87: Add Magic Connect Admin SDK support for Token Resource [#111](https://github.com/magiclabs/magic-admin-js/pull/111) ([@magic-ravi](https://github.com/magic-ravi)) + - [Security Enhancement]: Validate `aud` using Magic client ID. + - Pull client ID from Magic servers if not provided in constructor. + ## `0.3.3` - 05/02/2023 diff --git a/magic_admin/magic.py b/magic_admin/magic.py index b04989f..78a494d 100644 --- a/magic_admin/magic.py +++ b/magic_admin/magic.py @@ -2,7 +2,9 @@ import magic_admin from magic_admin.config import api_secret_api_key_missing_message +from magic_admin.config import base_url from magic_admin.error import AuthenticationError +from magic_admin.http_client import RequestsClient from magic_admin.resources.base import ResourceComponent @@ -13,6 +15,8 @@ class Magic: + v1_client_info = base_url + '/v1/admin/client/get' + def __getattr__(self, attribute_name): try: return getattr(self._resource, attribute_name) @@ -24,6 +28,7 @@ def __getattr__(self, attribute_name): def __init__( self, api_secret_key=None, + client_id=None, retries=RETRIES, timeout=TIMEOUT, backoff_factor=BACKOFF_FACTOR, @@ -32,6 +37,9 @@ def __init__( self._resource.setup_request_client(retries, timeout, backoff_factor) self._set_api_secret_key(api_secret_key) + init_requests_client = RequestsClient(retries, timeout, backoff_factor) + magic_admin.client_id = client_id or \ + init_requests_client.request('get', self.v1_client_info).data['client_id'] def _set_api_secret_key(self, api_secret_key): magic_admin.api_secret_key = api_secret_key or os.environ.get( diff --git a/magic_admin/resources/token.py b/magic_admin/resources/token.py index c218158..3119f91 100644 --- a/magic_admin/resources/token.py +++ b/magic_admin/resources/token.py @@ -4,6 +4,7 @@ from eth_account.messages import defunct_hash_message from web3.auto import w3 +import magic_admin from magic_admin.error import DIDTokenExpired from magic_admin.error import DIDTokenInvalid from magic_admin.error import DIDTokenMalformed @@ -172,3 +173,8 @@ def validate(cls, did_token): 'check the "nbf" field and regenerate a new token with a suitable ' 'value.', ) + + if claim['aud'] != magic_admin.client_id: + raise DIDTokenInvalid( + message='"aud" field does not match your client. Please check your secret key.', + ) diff --git a/tests/integration/magic_test.py b/tests/integration/magic_test.py index 930f169..2a03160 100644 --- a/tests/integration/magic_test.py +++ b/tests/integration/magic_test.py @@ -1,3 +1,5 @@ +from unittest import mock + import pytest import magic_admin @@ -10,6 +12,21 @@ class TestMagic: api_secret_key = 'troll_goat' + @pytest.fixture(autouse=True) + def setup(self): + self.mocked_rc = mock.Mock( + request=mock.Mock( + return_value=mock.Mock( + data={ + 'client_id': '1234', + }, + ), + ), + ) + # self.mocked_rc.request= + with mock.patch('magic_admin.magic.RequestsClient', return_value=self.mocked_rc): + yield + def test_init_with_secret_key(self): Magic(api_secret_key=self.api_secret_key) diff --git a/tests/integration/resources/token_test.py b/tests/integration/resources/token_test.py index a612433..b52ff9d 100644 --- a/tests/integration/resources/token_test.py +++ b/tests/integration/resources/token_test.py @@ -1,3 +1,7 @@ +from unittest import mock + +from pretend import stub + from magic_admin.resources.token import Token from testing.data.did_token import claim from testing.data.did_token import future_did_token @@ -21,4 +25,8 @@ def test_get_public_address(self): assert Token.get_public_address(future_did_token) == public_address def test_validate(self): - Token.validate(future_did_token) + with mock.patch( + 'magic_admin.resources.token.magic_admin', + new=stub(client_id='did:magic:731848cc-084e-41ff-bbdf-7f103817ea6b'), + ): + Token.validate(future_did_token) diff --git a/tests/unit/magic_test.py b/tests/unit/magic_test.py index 2420210..c64111e 100644 --- a/tests/unit/magic_test.py +++ b/tests/unit/magic_test.py @@ -14,24 +14,39 @@ class TestMagic: api_secret_key = 'troll_goat' + @pytest.fixture(autouse=True) + def setup(self): + self.mocked_resource_component = mock.Mock() + self.mocked_request_client = mock.Mock( + request=mock.Mock( + return_value=mock.Mock( + data={ + 'client_id': '1234', + }, + ), + ), + ) + with mock.patch( + 'magic_admin.magic.ResourceComponent', + return_value=self.mocked_resource_component, + ), mock.patch( + 'magic_admin.magic.RequestsClient', + return_value=self.mocked_request_client, + ): + yield + @pytest.fixture(autouse=True) def teardown(self): yield magic_admin.api_secret_key = None def test_init(self): - mocked_rc = mock.Mock() - with mock.patch( - 'magic_admin.magic.ResourceComponent', - return_value=mocked_rc, - ) as mock_resource_component, mock.patch( 'magic_admin.magic.Magic._set_api_secret_key', ) as mock_set_api_secret_key: Magic(api_secret_key=self.api_secret_key) - mock_resource_component.assert_called_once_with() - mocked_rc.setup_request_client.setup_request_client( + self.mocked_resource_component.setup_request_client.assert_called_once_with( RETRIES, TIMEOUT, BACKOFF_FACTOR, diff --git a/tests/unit/resources/token_test.py b/tests/unit/resources/token_test.py index f416ffb..1aebe0f 100644 --- a/tests/unit/resources/token_test.py +++ b/tests/unit/resources/token_test.py @@ -3,6 +3,7 @@ from unittest import mock import pytest +from pretend import stub from magic_admin.error import DIDTokenExpired from magic_admin.error import DIDTokenInvalid @@ -164,6 +165,7 @@ def setup_mocks(self): claim = { 'ext': 8084, 'nbf': 6666, + 'aud': '1234', } with mock.patch.object( @@ -185,7 +187,10 @@ def setup_mocks(self): ) as epoch_time_now, mock.patch( 'magic_admin.resources.token.apply_did_token_nbf_grace_period', return_value=claim['nbf'], - ) as apply_did_token_nbf_grace_period: + ) as apply_did_token_nbf_grace_period, mock.patch( + 'magic_admin.resources.token.magic_admin', + new=stub(client_id='1234'), + ): yield self.mock_funcs( proof, claim, From 61de34674688bcd3983a7ed4e408be6e58acebb4 Mon Sep 17 00:00:00 2001 From: Ravi Bhankharia Date: Wed, 5 Jul 2023 14:43:04 -0700 Subject: [PATCH 2/3] Bump version to match changelog. --- magic_admin/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/magic_admin/version.py b/magic_admin/version.py index 5358856..1e5a605 100644 --- a/magic_admin/version.py +++ b/magic_admin/version.py @@ -1 +1 @@ -VERSION = '0.3.3' +VERSION = '1.0.0' From 1b495175ef79be62a7283e202199adf73be02538 Mon Sep 17 00:00:00 2001 From: Ravi Bhankharia <89037241+magic-ravi@users.noreply.github.com> Date: Wed, 5 Jul 2023 16:44:31 -0500 Subject: [PATCH 3/3] Update version to 1.0.0 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 902aaf4..420499d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ - ... -## `2.0.0` - 07/05/2023 +## `1.0.0` - 07/05/2023 #### Added