Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate client ID in DID Token. #87

Merged
merged 3 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@

- <PR-#ISSUE> ...

## `1.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

Expand Down
8 changes: 8 additions & 0 deletions magic_admin/magic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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']
Comment on lines +40 to +42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the client already set up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's scoped to the specific Resources :( I ran into issues trying to get it to work with the existing client so I ended up initializing my own.


def _set_api_secret_key(self, api_secret_key):
magic_admin.api_secret_key = api_secret_key or os.environ.get(
Expand Down
6 changes: 6 additions & 0 deletions magic_admin/resources/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.',
)
2 changes: 1 addition & 1 deletion magic_admin/version.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
VERSION = '0.3.3'
VERSION = '1.0.0'
17 changes: 17 additions & 0 deletions tests/integration/magic_test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

import pytest

import magic_admin
Expand All @@ -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)

Expand Down
10 changes: 9 additions & 1 deletion tests/integration/resources/token_test.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
29 changes: 22 additions & 7 deletions tests/unit/magic_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion tests/unit/resources/token_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -164,6 +165,7 @@ def setup_mocks(self):
claim = {
'ext': 8084,
'nbf': 6666,
'aud': '1234',
}

with mock.patch.object(
Expand All @@ -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,
Expand Down
Loading