Allow public clients

laravel · Aug 11, 2019 · be7500a · be7500a
1 parent 5ec0dae
commit be7500a
Show file tree

Hide file tree

Showing 10 changed files with 161 additions and 35 deletions.
diff --git a/database/migrations/2019_07_17_000001_make_client_secret_nullable.php b/database/migrations/2019_07_17_000001_make_client_secret_nullable.php
@@ -0,0 +1,32 @@
+<?php
+
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Database\Migrations\Migration;
+
+class MakeClientSecretNullable extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('oauth_clients', function (Blueprint $table) {
+            $table->string('secret', 100)->nullable()->change();
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('oauth_clients', function (Blueprint $table) {
+            $table->string('secret', 100)->change();
+        });
+    }
+}
diff --git a/resources/js/components/Clients.vue b/resources/js/components/Clients.vue
@@ -125,6 +125,22 @@
                                     </span>
                                 </div>
                             </div>
+
+                            <!-- Confidential -->
+                            <div class="form-group row">
+                                <label class="col-md-3 col-form-label">Confidential</label>
+
+                                <div class="col-md-9">
+                                    <div class="checkbox">
+                                        <label>
+                                            <input type="checkbox" v-model="createForm.confidential">
+                                        </label>
+                                    </div>
+                                    <span class="form-text text-muted">
+                                        Require the client to authenticate with a secret.
+                                    </span>
+                                </div>
+                            </div>
                         </form>
                     </div>
 
@@ -222,7 +238,8 @@
                 createForm: {
                     errors: [],
                     name: '',
-                    redirect: ''
+                    redirect: '',
+                    confidential: true
                 },
 
                 editForm: {

diff --git a/src/Bridge/ClientRepository.php b/src/Bridge/ClientRepository.php
@@ -37,7 +37,7 @@ public function getClientEntity($clientIdentifier)
         }
 
         return new Client(
-            $clientIdentifier, $record->name, $record->redirect, ! is_null($record->secret)
+            $clientIdentifier, $record->name, $record->redirect, $record->confidential()
         );
     }
 
@@ -52,7 +52,7 @@ public function validateClient($clientIdentifier, $clientSecret, $grantType)
             return false;
         }
 
-        return hash_equals($record->secret, (string) $clientSecret);
+        return ! $record->confidential() || hash_equals($record->secret, (string) $clientSecret);
     }
 
     /**
@@ -72,11 +72,11 @@ protected function handlesGrant($record, $grantType)
             case 'authorization_code':
                 return ! $record->firstParty();
             case 'personal_access':
-                return $record->personal_access_client;
+                return $record->personal_access_client && $record->confidential();
             case 'password':
                 return $record->password_client;
             case 'client_credentials':
-                return ! empty($record->secret);
+                return $record->confidential();
             default:
                 return true;
         }

diff --git a/src/Client.php b/src/Client.php
@@ -92,4 +92,14 @@ public function skipsAuthorization()
     {
         return false;
     }
+
+    /**
+     * Determine if the client is a confidential client.
+     *
+     * @return bool
+     */
+    public function confidential()
+    {
+        return ! empty($this->secret);
+    }
 }
diff --git a/src/ClientRepository.php b/src/ClientRepository.php
@@ -106,14 +106,15 @@ public function personalAccessClient()
      * @param  string  $redirect
      * @param  bool  $personalAccess
      * @param  bool  $password
+     * @param  bool  $confidential
      * @return \Laravel\Passport\Client
      */
-    public function create($userId, $name, $redirect, $personalAccess = false, $password = false)
+    public function create($userId, $name, $redirect, $personalAccess = false, $password = false, $confidential = true)
     {
         $client = Passport::client()->forceFill([
             'user_id' => $userId,
             'name' => $name,
-            'secret' => Str::random(40),
+            'secret' => ($confidential || $personalAccess) ? Str::random(40) : null,
             'redirect' => $redirect,
             'personal_access_client' => $personalAccess,
             'password_client' => $password,

diff --git a/src/Http/Controllers/ClientController.php b/src/Http/Controllers/ClientController.php
@@ -73,10 +73,12 @@ public function store(Request $request)
         $this->validation->make($request->all(), [
             'name' => 'required|max:255',
             'redirect' => ['required', $this->redirectRule],
+            'confidential' => 'boolean',
         ])->validate();
 
         return $this->clients->create(
-            $request->user()->getKey(), $request->name, $request->redirect
+            $request->user()->getKey(), $request->name, $request->redirect,
+            false, false, (bool) $request->input('confidential', true)
         )->makeVisible('secret');
     }
 

diff --git a/tests/AccessTokenControllerTest.php b/tests/AccessTokenControllerTest.php
@@ -6,6 +6,7 @@
 use Lcobucci\JWT\Parser;
 use Zend\Diactoros\Response;
 use PHPUnit\Framework\TestCase;
+use Illuminate\Container\Container;
 use Laravel\Passport\TokenRepository;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;

diff --git a/tests/AuthorizationControllerTest.php b/tests/AuthorizationControllerTest.php
@@ -14,7 +14,6 @@
 use Laravel\Passport\TokenRepository;
 use Laravel\Passport\ClientRepository;
 use Psr\Http\Message\ResponseInterface;
-use Illuminate\Contracts\Config\Repository;
 use Psr\Http\Message\ServerRequestInterface;
 use League\OAuth2\Server\AuthorizationServer;
 use Illuminate\Contracts\Routing\ResponseFactory;
@@ -72,31 +71,6 @@ public function test_authorization_view_is_presented()
         ));
     }
 
-    public function test_authorization_exceptions_are_handled()
-    {
-        Container::getInstance()->instance(ExceptionHandler::class, $exceptions = m::mock());
-        Container::getInstance()->instance(Repository::class, $config = m::mock());
-        $exceptions->shouldReceive('report')->once();
-        $config->shouldReceive('get')->once()->andReturn(true);
-
-        $server = m::mock(AuthorizationServer::class);
-        $response = m::mock(ResponseFactory::class);
-
-        $controller = new AuthorizationController($server, $response);
-
-        $server->shouldReceive('validateAuthorizationRequest')->andThrow(new Exception('whoops'));
-
-        $request = m::mock(Request::class);
-        $request->shouldReceive('session')->andReturn($session = m::mock());
-
-        $clients = m::mock(ClientRepository::class);
-        $tokens = m::mock(TokenRepository::class);
-
-        $this->assertEquals('whoops', $controller->authorize(
-            m::mock(ServerRequestInterface::class), $request, $clients, $tokens
-        )->getContent());
-    }
-
     public function test_request_is_approved_if_valid_token_exists()
     {
         Passport::tokensCan([

diff --git a/tests/BridgeClientRepositoryTest.php b/tests/BridgeClientRepositoryTest.php
@@ -74,6 +74,15 @@ public function test_password_grant_is_permitted()
         $this->assertTrue($this->repository->validateClient(1, 'secret', 'password'));
     }
 
+    public function test_public_client_password_grant_is_permitted()
+    {
+        $client = $this->clientModelRepository->findActive(1);
+        $client->password_client = true;
+        $client->secret = null;
+
+        $this->assertTrue($this->repository->validateClient(1, null, 'password'));
+    }
+
     public function test_password_grant_is_prevented()
     {
         $this->assertFalse($this->repository->validateClient(1, 'secret', 'password'));
@@ -84,6 +93,14 @@ public function test_authorization_code_grant_is_permitted()
         $this->assertTrue($this->repository->validateClient(1, 'secret', 'authorization_code'));
     }
 
+    public function test_public_client_authorization_code_grant_is_permitted()
+    {
+        $client = $this->clientModelRepository->findActive(1);
+        $client->secret = null;
+
+        $this->assertTrue($this->repository->validateClient(1, null, 'authorization_code'));
+    }
+
     public function test_authorization_code_grant_is_prevented()
     {
         $client = $this->clientModelRepository->findActive(1);
@@ -105,6 +122,15 @@ public function test_personal_access_grant_is_prevented()
         $this->assertFalse($this->repository->validateClient(1, 'secret', 'personal_access'));
     }
 
+    public function test_public_client_personal_access_grant_is_prevented()
+    {
+        $client = $this->clientModelRepository->findActive(1);
+        $client->personal_access_client = true;
+        $client->secret = null;
+
+        $this->assertFalse($this->repository->validateClient(1, null, 'personal_access'));
+    }
+
     public function test_client_credentials_grant_is_permitted()
     {
         $this->assertTrue($this->repository->validateClient(1, 'secret', 'client_credentials'));
@@ -133,6 +159,24 @@ public function test_grant_types_disallows_request()
 
         $this->assertFalse($this->repository->validateClient(1, 'secret', 'authorization_code'));
     }
+
+    public function test_refresh_grant_is_permitted()
+    {
+        $this->assertTrue($this->repository->validateClient(1, 'secret', 'refresh_token'));
+    }
+
+    public function test_public_refresh_grant_is_permitted()
+    {
+        $client = $this->clientModelRepository->findActive(1);
+        $client->secret = null;
+
+        $this->assertTrue($this->repository->validateClient(1, null, 'refresh_token'));
+    }
+
+    public function test_refresh_grant_is_prevented()
+    {
+        $this->assertFalse($this->repository->validateClient(1, 'wrong-secret', 'refresh_token'));
+    }
 }
 
 class BridgeClientRepositoryTestClientStub
@@ -153,4 +197,9 @@ public function firstParty()
     {
         return $this->personal_access_client || $this->password_client;
     }
+
+    public function confidential()
+    {
+        return ! empty($this->secret);
+    }
 }
diff --git a/tests/ClientControllerTest.php b/tests/ClientControllerTest.php
@@ -48,7 +48,7 @@ public function test_clients_can_be_stored()
 
         $clients->shouldReceive('create')
             ->once()
-            ->with(1, 'client name', 'http://localhost')
+            ->with(1, 'client name', 'http://localhost', false, false, true)
             ->andReturn($client = new Client);
 
         $redirectRule = m::mock(RedirectRule::class);
@@ -60,6 +60,46 @@ public function test_clients_can_be_stored()
         ], [
             'name' => 'required|max:255',
             'redirect' => ['required', $redirectRule],
+            'confidential' => 'boolean',
+        ])->andReturn($validator);
+        $validator->shouldReceive('validate')->once();
+
+        $controller = new ClientController(
+            $clients, $validator, $redirectRule
+        );
+
+        $this->assertEquals($client, $controller->store($request));
+    }
+
+    public function test_public_clients_can_be_stored()
+    {
+        $clients = m::mock(ClientRepository::class);
+
+        $request = Request::create(
+            '/',
+            'GET',
+            ['name' => 'client name', 'redirect' => 'http://localhost', 'confidential' => false]
+        );
+        $request->setUserResolver(function () {
+            return new ClientControllerFakeUser;
+        });
+
+        $clients->shouldReceive('create')
+            ->once()
+            ->with(1, 'client name', 'http://localhost', false, false, false)
+            ->andReturn($client = new Client);
+
+        $redirectRule = m::mock(RedirectRule::class);
+
+        $validator = m::mock(Factory::class);
+        $validator->shouldReceive('make')->once()->with([
+            'name' => 'client name',
+            'redirect' => 'http://localhost',
+            'confidential' => false,
+        ], [
+            'name' => 'required|max:255',
+            'redirect' => ['required', $redirectRule],
+            'confidential' => 'boolean',
         ])->andReturn($validator);
         $validator->shouldReceive('validate')->once();