feat(auth): remove callbackUrl parameter on authentication (#59)

ForestAdmin · Sep 14, 2022 · e097ff9 · e097ff9
1 parent b4999be
commit e097ff9
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 35 deletions.
diff --git a/src/Auth/AuthManager.php b/src/Auth/AuthManager.php
@@ -36,15 +36,14 @@ public function __construct(OidcClientManager $oidc)
     }
 
     /**
-     * @param string $callbackUrl
-     * @param int    $renderingId
+     * @param int $renderingId
      * @return string
      * @throws GuzzleException
      * @throws JsonException
      */
-    public function start(string $callbackUrl, int $renderingId)
+    public function start(int $renderingId)
     {
-        $client = $this->oidc->getClientForCallbackUrl($callbackUrl);
+        $client = $this->oidc->makeForestProvider();
 
         return $client->getAuthorizationUrl(
             [
@@ -54,18 +53,17 @@ public function start(string $callbackUrl, int $renderingId)
     }
 
     /**
-     * @param string $redirectUrl
-     * @param array  $params
+     * @param array $params
      * @return string
-     * @throws JsonException
-     * @throws IdentityProviderException
      * @throws GuzzleException
+     * @throws IdentityProviderException
+     * @throws JsonException
      */
-    public function verifyCodeAndGenerateToken(string $redirectUrl, array $params): string
+    public function verifyCodeAndGenerateToken(array $params): string
     {
         $this->stateIsValid($params);
 
-        $forestProvider = $this->oidc->getClientForCallbackUrl($redirectUrl);
+        $forestProvider = $this->oidc->makeForestProvider();
         $forestProvider->setRenderingId($this->getRenderingIdFromState($params['state']));
         if (config('app.debug')) {
             // @codeCoverageIgnoreStart

diff --git a/src/Auth/OidcClientManager.php b/src/Auth/OidcClientManager.php
@@ -38,29 +38,27 @@ public function __construct(ForestApiRequester $forestApi)
     }
 
     /**
-     * @param string $callbackUrl
-     * @return ForestProvider|string
+     * @return ForestProvider
      * @throws GuzzleException
      */
-    public function getClientForCallbackUrl(string $callbackUrl)
+    public function makeForestProvider()
     {
-        $cacheKey = $callbackUrl . '-' . config('forest.api.secret') . '-client-data';
+        $cacheKey = config('forest.api.secret') . '-client-data';
 
         try {
             $config = $this->retrieve();
             Cache::remember(
                 $cacheKey,
                 self::TTL,
-                function () use ($config, $callbackUrl) {
+                function () use ($config) {
                     $clientCredentials = $this->register(
                         [
                             'token_endpoint_auth_method' => 'none',
                             'registration_endpoint'      => $config['registration_endpoint'],
-                            'redirect_uris'              => [$callbackUrl],
                             'application_type'           => 'web'
                         ]
                     );
-                    $clientData = ['client_id' => $clientCredentials['client_id'], 'issuer' => $config['issuer']];
+                    $clientData = ['client_id' => $clientCredentials['client_id'], 'issuer' => $config['issuer'], 'redirect_uri' => $clientCredentials['redirect_uris'][0]];
 
                     return $clientData;
                 }
@@ -73,7 +71,7 @@ function () use ($config, $callbackUrl) {
             Cache::get($cacheKey)['issuer'],
             [
                 'clientId'     => Cache::get($cacheKey)['client_id'],
-                'redirectUri'  => $callbackUrl,
+                'redirectUri'  => Cache::get($cacheKey)['redirect_uri'],
             ]
         );
     }

diff --git a/src/Commands/ForestClear.php b/src/Commands/ForestClear.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace ForestAdmin\LaravelForestAdmin\Commands;
+
+use Illuminate\Console\Command;
+use Illuminate\Support\Facades\Cache;
+
+/**
+ * Class ForestClear
+ *
+ * @package Laravel-forestadmin
+ * @license GNU https://www.gnu.org/licenses/licenses.html
+ * @link    https://github.com/ForestAdmin/laravel-forestadmin
+ * @codeCoverageIgnore
+ */
+class ForestClear extends Command
+{
+    /**
+     * @var string
+     */
+    protected $signature = 'forest:clear';
+
+    /**
+     * @var string
+     */
+    protected $description = 'Clear the ForestProvider data cache key';
+
+    /**
+     * @return int
+     */
+    public function handle()
+    {
+        Cache::forget(config('forest.api.secret') . '-client-data');
+    }
+}
diff --git a/src/ForestServiceProvider.php b/src/ForestServiceProvider.php
@@ -2,6 +2,7 @@
 
 namespace ForestAdmin\LaravelForestAdmin;
 
+use ForestAdmin\LaravelForestAdmin\Commands\ForestClear;
 use ForestAdmin\LaravelForestAdmin\Commands\ForestInstall;
 use ForestAdmin\LaravelForestAdmin\Commands\SendApimap;
 use ForestAdmin\LaravelForestAdmin\Http\Middleware\ForestCors;
@@ -37,6 +38,7 @@ public function boot(Kernel $kernel): void
         if ($this->app->runningInConsole()) {
             $this->commands(
                 [
+                    ForestClear::class,
                     ForestInstall::class,
                     SendApimap::class,
                 ]

diff --git a/src/Http/Controllers/AuthController.php b/src/Http/Controllers/AuthController.php
@@ -46,7 +46,7 @@ public function login()
 
         return response()->json(
             [
-                'authorizationUrl' => $this->auth->start(config('app.url') . route('forest.auth.callback', [], false), $renderingId),
+                'authorizationUrl' => $this->auth->start($renderingId),
             ]
         );
     }
@@ -67,7 +67,7 @@ public function logout()
      */
     public function callback()
     {
-        $token = $this->auth->verifyCodeAndGenerateToken(config('app.url') . route('forest.auth.callback', [], false), request()->all());
+        $token = $this->auth->verifyCodeAndGenerateToken(request()->all());
         $tokenData = JWT::decode($token, new Key(config('forest.api.auth-secret'), 'HS256'));
 
         return response()->json(compact('token', 'tokenData'));

diff --git a/tests/Feature/AuthControllerTest.php b/tests/Feature/AuthControllerTest.php
@@ -71,7 +71,7 @@ public function testLogin(): void
 
         $auth = $this->prophesize(AuthManager::class);
         $auth
-            ->start(route('forest.auth.callback'), 1)
+            ->start(1)
             ->shouldBeCalled()
             ->willReturn($return);
 
@@ -109,7 +109,7 @@ public function testCallback(): void
 
         $auth = $this->prophesize(AuthManager::class);
         $auth
-            ->verifyCodeAndGenerateToken(route('forest.auth.callback'), Argument::any())
+            ->verifyCodeAndGenerateToken(Argument::any())
             ->shouldBeCalled()
             ->willReturn($jwt);
 

diff --git a/tests/Unit/AuthManagerTest.php b/tests/Unit/AuthManagerTest.php
@@ -36,11 +36,10 @@ public function testStart(): void
         $return = '123ABC';
         $oidc = $this->makeOidc($return, true);
 
-        /** @var AuthManager auth */
         $this->auth = app()->make(AuthManager::class);
         $this->auth->oidc = $oidc->reveal();
 
-        $start = $this->auth->start('localhost/foo', 1);
+        $start = $this->auth->start(1);
         parse_str(parse_url($start, PHP_URL_QUERY), $output);
 
         $this->assertSame($output['state'], '{"renderingId":1}');
@@ -61,7 +60,7 @@ public function testVerifyCodeAndGenerateToken(): void
         $this->auth->oidc = $oidc->reveal();
 
         $data = ['code' => 'test', 'state' => '{"renderingId":1}'];
-        $token = $this->auth->verifyCodeAndGenerateToken('mock_host/foo', $data);
+        $token = $this->auth->verifyCodeAndGenerateToken($data);
 
         $this->assertSame($return, $token);
     }
@@ -185,7 +184,7 @@ private function makeOidc(string $return, $withAuthorization = false): ObjectPro
 
         $oidc = $this->prophesize(OidcClientManager::class);
         $oidc
-            ->getClientForCallbackUrl(Argument::type('string'))
+            ->makeForestProvider()
             ->shouldBeCalled()
             ->willReturn(
                 $provider->reveal()

diff --git a/tests/Unit/OidcClientManagerTest.php b/tests/Unit/OidcClientManagerTest.php
@@ -96,16 +96,16 @@ public function testRegister(): void
      * @throws \JsonException
      * @return void
      */
-    public function testGetClientForCallbackUrl(): void
+    public function testMakeForestProvider(): void
     {
-        $this->oidc = new OidcClientManager($this->makeForestApiGetAndPost(json_encode(['client_id' => 1], JSON_THROW_ON_ERROR)));
-        $clientForCallbackUrl = $this->oidc->getClientForCallbackUrl('mock_host/foo');
+        $this->oidc = new OidcClientManager($this->makeForestApiGetAndPost(json_encode(['client_id' => 1, 'redirect_uris' => ['http://backend.api']], JSON_THROW_ON_ERROR)));
+        $clientForCallbackUrl = $this->oidc->makeForestProvider();
 
         $this->assertInstanceOf(ForestProvider::class, $clientForCallbackUrl);
-        $this->assertIsArray(Cache::get('mock_host/foo-' . config('forest.api.secret') . '-client-data'));
+        $this->assertIsArray(Cache::get(config('forest.api.secret') . '-client-data'));
         $this->assertSame(
-            Cache::get('mock_host/foo-' . config('forest.api.secret') . '-client-data'),
-            ['client_id' => 1, 'issuer' => self::mockedConfig()['issuer']]
+            Cache::get(config('forest.api.secret') . '-client-data'),
+            ['client_id' => 1, 'issuer' => self::mockedConfig()['issuer'], 'redirect_uri' => 'http://backend.api']
         );
     }
 
@@ -114,12 +114,12 @@ public function testGetClientForCallbackUrl(): void
      * @throws \JsonException
      * @return void
      */
-    public function testGetClientForCallbackUrlException(): void
+    public function testMakeForestProviderException(): void
     {
         $this->oidc = new OidcClientManager($this->makeForestApiGetAndPost());
         $this->expectException(ForestApiException::class);
         $this->expectExceptionMessage(ErrorMessages::REGISTRATION_FAILED);
-        $this->oidc->getClientForCallbackUrl('mock_host/foo');
+        $this->oidc->makeForestProvider();
     }
 
     /**
@@ -251,7 +251,7 @@ public function makeForestApiGetAndPost(?string $body = null)
             );
 
         $forestApi
-            ->post(Argument::type('string'), Argument::size(0), Argument::size(4), Argument::size(1))
+            ->post(Argument::type('string'), Argument::size(0), Argument::size(3), Argument::size(1))
             ->shouldBeCalled()
             ->willReturn(
                 new Response(200, [], $body)