Merge pull request #22 from magiclabs/ravi-sc80633-ValidateAud

Validate `aud` in DID token. Pull client ID from Magic servers if not passed in constructor
magiclabs · Jul 10, 2023 · f64c10a · f64c10a
2 parents ae83478 + d82e05c
commit f64c10a
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 36 deletions.
diff --git a/.php_cs.dist → .php-cs-fixer.dist.php b/.php_cs.dist → .php-cs-fixer.dist.php
@@ -1,6 +1,7 @@
 <?php
+$config = new PhpCsFixer\Config();
 
-return PhpCsFixer\Config::create()
+return $config
     ->setRiskyAllowed(true)
     ->setRules([
         // Rulesets
@@ -14,7 +15,6 @@
         'fopen_flags' => true,
         'linebreak_after_opening_tag' => true,
         'native_function_invocation' => true,
-        'method_argument_space' => ['ensure_fully_multiline' => true],
         'ordered_imports' => true,
 
         // --- Diffs from @PhpCsFixer / @PhpCsFixer:risky ---

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,15 @@
 
 - <PR-#ISSUE> ...
 
+## `1.0.0` - 7/5/2023
+
+#### Added
+
+- PR-#21: Add Magic Connect Admin SDK support for Token Resource.
+  - [Security Enhancement]: Validate `aud` using Magic client ID.
+  - Pull client ID from Magic servers if not provided in constructor.
+
+
 ## `0.3.0` - 2/17/2023
 
 #### Added

diff --git a/Makefile b/Makefile
@@ -0,0 +1,10 @@
+.PHONY: install test format
+
+install:
+	composer install
+
+test:
+	./vendor/bin/phpunit tests/$(TEST_FILE)
+
+format:
+	php-cs-fixer fix -v --using-cache=no .
diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-0.1.4
+1.0.0
diff --git a/lib/Magic.php b/lib/Magic.php
@@ -11,26 +11,38 @@ class Magic
     public $token;
     public $api_secret_key;
     public $user;
-    public $wallet_type;
+    public $client_id;
 
     public function __construct(
         $api_secret_key = null,
         $timeout = TIMEOUT,
         $retries = RETRIES,
-        $backoff_factor = BACKOFF_FACTOR
+        $backoff_factor = BACKOFF_FACTOR,
+        $client_id = null
     ) {
         $this->api_secret_key = $api_secret_key;
-        $this->token = new \MagicAdmin\Resource\Token();
+        $request_client = new \MagicAdmin\HttpClient($api_secret_key, $timeout, $retries, $backoff_factor);
+        if ($client_id != null) {
+          $this->client_id = $client_id;
+        } else {
+          $this->client_id = $this->_get_client_id($request_client);
+        }
+        $this->token = new \MagicAdmin\Resource\Token($this->client_id);
         $this->user = new \MagicAdmin\Resource\User(
-            $this->api_secret_key,
-            $timeout,
-            $retries,
-            $backoff_factor
+            $request_client,
+            $this->token
         );
     }
 
     public function _set_platform($platform)
     {
         $this->user->_set_platform($platform);
     }
+
+    public function _get_client_id($request_client)
+    {
+        $response = $request_client->request('get', '/v1/admin/client/get', []);
+
+        return $response->data->client_id;
+    }
 }
diff --git a/lib/Resource/Token.php b/lib/Resource/Token.php
@@ -8,6 +8,13 @@
 
 class Token
 {
+    private $client_id;
+
+    public function __construct($client_id)
+    {
+        $this->client_id = $client_id;
+    }
+
     public $required_fields = [
         'iat',
         'ext',
@@ -23,13 +30,13 @@ public function _check_required_fields($claim)
         $missing_fields = [];
         foreach ($this->required_fields as $field) {
             if (\is_array($claim) && !isset($claim[$field])) {
-                \array_push($missing_fields, $field);
+                $missing_fields[] = $field;
             }
         }
 
         if (\count($missing_fields) > 0) {
             throw new \MagicAdmin\Exception\DIDTokenException(
-                'DID token is missing required field(s):' . \json_encode($missing_fields)
+                'DID token is missing required field(s):' . json_encode($missing_fields)
             );
         }
 
@@ -39,7 +46,7 @@ public function _check_required_fields($claim)
     public function decode($did_token)
     {
         try {
-            $decoded_did_token = \json_decode(\utf8_decode(\base64_decode($did_token, true)));
+            $decoded_did_token = json_decode(utf8_decode(base64_decode($did_token, true)));
         } catch (\Exception $e) {
             throw new \MagicAdmin\Exception\DIDTokenException(
                 'DID token is malformed. It has to be a based64 encoded JSON serialized string. DIDTokenException(' . $e->getMessage() . ')'
@@ -55,7 +62,7 @@ public function decode($did_token)
         $proof = $decoded_did_token[0];
 
         try {
-            $claim = \json_decode($decoded_did_token[1]);
+            $claim = json_decode($decoded_did_token[1]);
         } catch (\Exception $e) {
             throw new \MagicAdmin\Exception\DIDTokenException(
                 'DID token is malformed. Given claim should be a JSON serialized string. DIDTokenException(' . $e->getMessage() . ')'
@@ -83,9 +90,9 @@ public function validate($did_token)
     {
         list($proof, $claim) = $this->decode($did_token);
 
-        $recovered_address = Eth::ecRecover(\json_encode($claim), $proof);
+        $recovered_address = Eth::ecRecover(json_encode($claim), $proof);
 
-        if ($recovered_address !== \strtolower($this->get_public_address($did_token))) {
+        if ($recovered_address !== strtolower($this->get_public_address($did_token))) {
             throw new \MagicAdmin\Exception\DIDTokenException(
                 'Signature mismatch between "proof" and "claim". Please generate a new token with an intended issuer.'
             );
@@ -104,5 +111,11 @@ public function validate($did_token)
                 'Given DID token cannot be used at this time. Please check the "nbf" field and regenerate a new token with a suitable value.'
             );
         }
+
+        if ($claim->aud !== $this->client_id) {
+            throw new \MagicAdmin\Exception\DIDTokenException(
+                'Audience does not match client ID. Please ensure your secret key matches the application which generated the DID token.'
+            );
+        }
     }
 }
diff --git a/lib/Resource/User.php b/lib/Resource/User.php
@@ -10,17 +10,17 @@ class User
     public $request_client;
     public $token;
 
-    public function __construct($api_secret_key, $timeout, $retries, $backoff_factor)
+    public function __construct($request_client, $token)
     {
-        $this->token = new Token();
-        $this->request_client = new \MagicAdmin\HttpClient($api_secret_key, $timeout, $retries, $backoff_factor);
+      $this->request_client = $request_client;
+      $this->token = $token;
     }
 
     public function get_metadata_by_issuer_and_wallet($issuer, $wallet_type)
     {
         return $this->request_client->request('get', $this->v1_user_info, ['issuer' => $issuer, 'wallet_type' => $wallet_type]);
     }
-    
+
     public function get_metadata_by_issuer($issuer)
     {
         return $this->get_metadata_by_issuer_and_wallet($issuer, Wallet::NONE);
@@ -30,7 +30,7 @@ public function get_metadata_by_public_address_and_wallet($public_address, $wall
     {
         return $this->get_metadata_by_issuer(
             \MagicAdmin\Util\DidToken::construct_issuer_with_public_address($public_address),
-            $wallet_type,
+            $wallet_type
         );
     }
 

diff --git a/tests/MagicTest.php b/tests/MagicTest.php
@@ -3,6 +3,7 @@
 require_once __DIR__ . '../../init.php';
 
 //use MagicAdmin;
+use MagicAdmin\HttpClient;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -16,19 +17,22 @@ final class MagicTest extends TestCase
     public $timeout;
     public $retries;
     public $backoff_factor;
+    public $client_id = "client_id";
+    public $mockedHttpClient;
 
-    protected function setUp(): void
+  protected function setUp(): void
     {
         $this->api_secret_key = 'magic_admin';
         $this->timeout = 10;
         $this->retries = 3;
         $this->backoff_factor = 0.02;
-        $this->magic = new \MagicAdmin\Magic($this->api_secret_key, $this->timeout, $this->retries, $this->backoff_factor);
+        $this->magic = new \MagicAdmin\Magic($this->api_secret_key, $this->timeout, $this->retries, $this->backoff_factor, $this->client_id);
+        $this->mockedHttpClient = $this->createMock(HttpClient::class);
     }
 
     public function testRetrievesApiSecretKey()
     {
-        static::assertSame($this->magic->api_secret_key, $this->api_secret_key);
+      static::assertSame($this->magic->api_secret_key, $this->api_secret_key);
     }
 
     public function testRetrievesTimeout()
@@ -45,4 +49,22 @@ public function testRetrievesBackoffFactor()
     {
         static::assertSame($this->magic->user->request_client->_backoff_factor, $this->backoff_factor);
     }
+
+    public function testRetrievesClientId()
+    {
+      static::assertSame($this->magic->client_id, $this->client_id);
+    }
+
+    public function testRetrievesClientIdFromMagic()
+    {
+      $clientId = 'test_client_id';
+      $this->mockedHttpClient->method('request')
+        ->willReturn((object)[
+          'data' => (object)[
+            'client_id' => $clientId
+          ]
+        ]);
+      static::assertSame($clientId, $this->magic->_get_client_id($this->mockedHttpClient));
+    }
+
 }
diff --git a/tests/Resource/TokenTest.php b/tests/Resource/TokenTest.php
@@ -12,7 +12,8 @@ final class TokenTest extends TestCase
 
     protected function setUp(): void
     {
-        $this->token = new \MagicAdmin\Resource\Token();
+      $client_id = "did:magic:731848cc-084e-41ff-bbdf-7f103817ea6b";
+      $this->token = new \MagicAdmin\Resource\Token($client_id);
     }
 
     public function testCheckRequiredFields()
@@ -58,7 +59,8 @@ final class TokenDecodeTest extends TestCase
 
     protected function setUp(): void
     {
-        $this->token = new \MagicAdmin\Resource\Token();
+      $client_id = "client_id";
+      $this->token = new \MagicAdmin\Resource\Token($client_id);
     }
 
     public function testDecodeRaisesErrorIfDidTokenIsMalformed()
@@ -105,7 +107,8 @@ final class TokenValidateTest extends TestCase
 
     protected function setUp(): void
     {
-        $this->token = new \MagicAdmin\Resource\Token();
+      $client_id = 'did:magic:f54168e9-9ce9-47f2-81c8-7cb2a96b26ba';
+      $this->token = new \MagicAdmin\Resource\Token($client_id);
     }
 
     /**

diff --git a/tests/Resource/UserTest.php b/tests/Resource/UserTest.php
@@ -1,5 +1,6 @@
 <?php
 
+use MagicAdmin\HttpClient;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -14,19 +15,17 @@ final class UserTest extends TestCase
     private $wallet_type;
     private $wallet;
     private $wallets;
+    private $token;
 
     protected function setUp(): void
     {
-        $api_secret_key = 'magic_admin';
-        $timeout = 10;
-        $retries = 3;
-        $backoff_factor = 0.02;
+        $this->mockedHttpClient = $this->createMock(HttpClient::class);
+        $client_id = "client_id";
+        $this->token = new \MagicAdmin\Resource\Token($client_id);
         $this->user = new \MagicAdmin\Resource\User(
-            $api_secret_key,
-            $timeout,
-            $retries,
-            $backoff_factor
-        );
+              $this->mockedHttpClient,
+              $this->token
+          );
         $this->issuer = 'did:ethr:0xabA53bd22b2673C6c42ffA11C251B45D8CcBe4a4';
         $this->public_address = '0xabA53bd22b2673C6c42ffA11C251B45D8CcBe4a4';
         $this->wallet_type = \MagicAdmin\Resource\Wallet::SOLANA;