feature #16 add support for client name (azjezz)

This PR was merged into the 0.1-dev branch.

Discussion
----------

add support for client name

fixes: trikoder/oauth2-bundle#209 / trikoder/oauth2-bundle#145

`league/oauth2-server` uses both `name` and `identifier`, so it makes sense to do the same here.

"name" doesn't have to be unique, it's the name of the oauth client which might be used at the user consent page so the user knows which application they are logging into.

Commits
-------

d3b8b1b add support for client name

psalm.xml

@@ -1,7 +1,6 @@
 <?xml version="1.0"?>
 <psalm
         totallyTyped="true"
-        resolveFromConfigFile="true"
         forbidEcho="true"
         strictBinaryOperands="true"
         phpVersion="7.1"

src/Command/CreateClientCommand.php

@@ -57,6 +57,11 @@ protected function configure(): void
                 'Sets allowed scope for client. Use this option multiple times to set multiple scopes.',
                 []
             )
+            ->addArgument(
+                'name',
+                InputArgument::REQUIRED,
+                'The client name'
+            )
             ->addArgument(
                 'identifier',
                 InputArgument::OPTIONAL,
@@ -108,6 +113,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
     private function buildClientFromInput(InputInterface $input): Client
     {
+        $name = $input->getArgument('name');
         /** @var string $identifier */
         $identifier = $input->getArgument('identifier') ?? hash('md5', random_bytes(16));
 
@@ -120,7 +126,7 @@ private function buildClientFromInput(InputInterface $input): Client
         /** @var string $secret */
         $secret = $isPublic ? null : $input->getArgument('secret') ?? hash('sha512', random_bytes(32));
 
-        $client = new Client($identifier, $secret);
+        $client = new Client($name, $identifier, $secret);
         $client->setActive(true);
         $client->setAllowPlainTextPkce($input->getOption('allow-plain-text-pkce'));
 

src/League/Entity/Client.php

@@ -18,12 +18,9 @@ final class Client implements ClientEntityInterface
      */
     private $allowPlainTextPkce = false;
 
-    /**
-     * {@inheritdoc}
-     */
-    public function getName(): string
+    public function setName(string $name): void
     {
-        return (string) $this->getIdentifier();
+        $this->name = $name;
     }
 
     /**

src/League/Repository/ClientRepository.php

@@ -38,7 +38,7 @@ public function getClientEntity($clientIdentifier)
     /**
      * {@inheritdoc}
      */
-    public function validateClient($clientIdentifier, $clientSecret, $grantType)
+    public function validateClient($clientIdentifier, $clientSecret, $grantType): bool
     {
         $client = $this->clientManager->find($clientIdentifier);
 
@@ -64,6 +64,7 @@ public function validateClient($clientIdentifier, $clientSecret, $grantType)
     private function buildClientEntity(ClientModel $client): ClientEntity
     {
         $clientEntity = new ClientEntity();
+        $clientEntity->setName($client->getName());
         $clientEntity->setIdentifier($client->getIdentifier());
         $clientEntity->setRedirectUri(array_map('strval', $client->getRedirectUris()));
         $clientEntity->setConfidential($client->isConfidential());

src/Model/Client.php

@@ -6,6 +6,11 @@
 
 class Client
 {
+    /**
+     * @var string
+     */
+    private $name;
+
     /**
      * @var string
      */
@@ -44,8 +49,9 @@ class Client
     /**
      * @psalm-mutation-free
      */
-    public function __construct(string $identifier, ?string $secret)
+    public function __construct(string $name, string $identifier, ?string $secret)
     {
+        $this->name = $name;
         $this->identifier = $identifier;
         $this->secret = $secret;
     }
@@ -58,6 +64,14 @@ public function __toString(): string
         return $this->getIdentifier();
     }
 
+    /**
+     * @psalm-mutation-free
+     */
+    public function getName(): string
+    {
+        return $this->name;
+    }
+
     /**
      * @psalm-mutation-free
      */

src/Resources/config/doctrine/model/Client.orm.xml

@@ -6,6 +6,7 @@
                    https://raw.github.com/doctrine/doctrine2/master/doctrine-mapping.xsd">
     <entity name="League\Bundle\OAuth2ServerBundle\Model\Client" table="oauth2_client">
         <id name="identifier" type="string" length="32" />
+        <field name="name" type="string" length="128" />
         <field name="secret" type="string" length="128" nullable="true" />
         <field name="redirectUris" type="oauth2_redirect_uri" nullable="true" />
         <field name="grants" type="oauth2_grant" nullable="true" />

tests/Acceptance/CreateClientCommandTest.php

@@ -16,6 +16,7 @@ public function testCreateClient(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
         ]);
 
         $output = $commandTester->getDisplay();
@@ -28,6 +29,7 @@ public function testCreateClientWithIdentifier(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
             'identifier' => 'foobar',
         ]);
 
@@ -41,6 +43,7 @@ public function testCreateClientWithIdentifier(): void
             ->get(ClientManagerInterface::class)
             ->find('foobar');
         $this->assertInstanceOf(Client::class, $client);
+        $this->assertSame('My Awesome OAuth Client', $client->getName());
         $this->assertTrue($client->isConfidential());
         $this->assertNotEmpty($client->getSecret());
         $this->assertFalse($client->isPlainTextPkceAllowed());
@@ -53,6 +56,7 @@ public function testCreatePublicClientWithIdentifier(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
             'identifier' => $clientIdentifier,
             '--public' => true,
         ]);
@@ -81,6 +85,7 @@ public function testCannotCreatePublicClientWithSecret(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
             'identifier' => $clientIdentifier,
             'secret' => 'foo',
             '--public' => true,
@@ -105,6 +110,7 @@ public function testCreateClientWithSecret(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
             'identifier' => 'foobar',
             'secret' => 'quzbaz',
         ]);
@@ -129,6 +135,7 @@ public function testCreateClientWhoIsAllowedToUsePlainPkceChallengeMethod(): voi
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
             'identifier' => 'foobar-123',
             '--allow-plain-text-pkce' => true,
         ]);
@@ -151,6 +158,7 @@ public function testCreateClientWithRedirectUris(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
             'identifier' => 'foobar',
             '--redirect-uri' => ['http://example.org', 'http://example.org'],
         ]);
@@ -171,6 +179,7 @@ public function testCreateClientWithGrantTypes(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
             'identifier' => 'foobar',
             '--grant-type' => ['password', 'client_credentials'],
         ]);
@@ -191,6 +200,7 @@ public function testCreateClientWithScopes(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
+            'name' => 'My Awesome OAuth Client',
             'identifier' => 'foobar',
             '--scope' => ['foo', 'bar'],
         ]);

tests/Acceptance/DeleteClientCommandTest.php

@@ -16,7 +16,7 @@ final class DeleteClientCommandTest extends AbstractAcceptanceTest
 {
     public function testDeleteClient(): void
     {
-        $client = $this->fakeAClient('foobar');
+        $client = $this->fakeAClient('foo', 'foobar');
         $this->getClientManager()->save($client);
 
         $command = $this->command();
@@ -54,9 +54,9 @@ private function findClient(string $identifier): ?Client
             ;
     }
 
-    private function fakeAClient(string $identifier): Client
+    private function fakeAClient(string $name, string $identifier): Client
     {
-        return new Client($identifier, 'quzbaz');
+        return new Client($name, $identifier, 'quzbaz');
     }
 
     private function getClientManager(): ClientManagerInterface

tests/Acceptance/DoctrineAccessTokenManagerTest.php

@@ -23,7 +23,7 @@ public function testClearExpired(): void
 
         $doctrineAccessTokenManager = new DoctrineAccessTokenManager($em);
 
-        $client = new Client('client', 'secret');
+        $client = new Client('client', 'client', 'secret');
         $em->persist($client);
         $em->flush();
 
@@ -80,7 +80,7 @@ public function testClearExpiredWithRefreshToken(): void
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
         $doctrineAccessTokenManager = new DoctrineAccessTokenManager($em);
 
-        $client = new Client('client', 'secret');
+        $client = new Client('client', 'client', 'secret');
         $em->persist($client);
         $em->flush();
 

tests/Acceptance/DoctrineAuthCodeManagerTest.php

@@ -22,7 +22,7 @@ public function testClearExpired(): void
 
         $doctrineAuthCodeManager = new DoctrineAuthCodeManager($em);
 
-        $client = new Client('client', 'secret');
+        $client = new Client('client', 'client', 'secret');
         $em->persist($client);
 
         $testData = $this->buildClearExpiredTestData($client);

tests/Acceptance/DoctrineClientManagerTest.php

@@ -22,7 +22,7 @@ public function testSimpleDelete(): void
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
         $doctrineClientManager = new DoctrineClientManager($em);
 
-        $client = new Client('client', 'secret');
+        $client = new Client('client', 'client', 'secret');
         $em->persist($client);
         $em->flush();
 
@@ -41,7 +41,7 @@ public function testClientDeleteCascadesToAccessTokens(): void
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
         $doctrineClientManager = new DoctrineClientManager($em);
 
-        $client = new Client('client', 'secret');
+        $client = new Client('client', 'client', 'secret');
         $em->persist($client);
         $em->flush();
 
@@ -74,7 +74,7 @@ public function testClientDeleteCascadesToAccessTokensAndRefreshTokens(): void
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
         $doctrineClientManager = new DoctrineClientManager($em);
 
-        $client = new Client('client', 'secret');
+        $client = new Client('client', 'client', 'secret');
         $em->persist($client);
         $em->flush();
 

tests/Acceptance/DoctrineCredentialsRevokerTest.php

@@ -25,7 +25,7 @@ public function testRevokesAllCredentialsForUser(): void
         /** @var EntityManagerInterface $em */
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
 
-        $em->persist($client = new Client('client', 'secret'));
+        $em->persist($client = new Client('client', 'client', 'secret'));
 
         $authCode = $this->buildAuthCode('foo', '+1 minute', $client, $userIdentifier);
         $accessToken = $this->buildAccessToken('bar', '+1 minute', $client, $userIdentifier);
@@ -54,7 +54,7 @@ public function testRevokesAllCredentialsForClient(): void
         /** @var EntityManagerInterface $em */
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
 
-        $em->persist($client = new Client('acme', 'secret'));
+        $em->persist($client = new Client('client', 'acme', 'secret'));
 
         $authCode = $this->buildAuthCode('foo', '+1 minute', $client, 'john');
         $accessToken = $this->buildAccessToken('bar', '+1 minute', $client);

tests/Acceptance/DoctrineRefreshTokenManagerTest.php

@@ -23,7 +23,7 @@ public function testClearExpired(): void
 
         $doctrineRefreshTokenManager = new DoctrineRefreshTokenManager($em);
 
-        $client = new Client('client', 'secret');
+        $client = new Client('client', 'client', 'secret');
         $em->persist($client);
         $em->flush();
 

tests/Acceptance/ListClientsCommandTest.php

@@ -15,7 +15,7 @@ final class ListClientsCommandTest extends AbstractAcceptanceTest
 {
     public function testListClients(): void
     {
-        $client = $this->fakeAClient('foobar');
+        $client = $this->fakeAClient('client', 'foobar');
         $this->getClientManager()->save($client);
 
         $command = $this->command();
@@ -30,7 +30,7 @@ public function testListClients(): void
 
     public function testListClientsWithClientHavingNoSecret(): void
     {
-        $client = $this->fakeAClient('foobar', null);
+        $client = $this->fakeAClient('client', 'foobar', null);
         $this->getClientManager()->save($client);
 
         $command = $this->command();
@@ -70,7 +70,7 @@ public function testListClientColumns(): void
 
         $client =
             $this
-                ->fakeAClient('foobar')
+                ->fakeAClient('client', 'foobar')
                 ->setScopes(...$scopes)
                 ->setRedirectUris(...$redirectUris)
         ;
@@ -91,12 +91,12 @@ public function testListClientColumns(): void
 
     public function testListFiltersClients(): void
     {
-        $clientA = $this->fakeAClient('client-a', 'client-a-secret');
+        $clientA = $this->fakeAClient('client', 'client-a', 'client-a-secret');
         $this->getClientManager()->save($clientA);
 
         $clientB =
             $this
-                ->fakeAClient('client-b', 'client-b-secret')
+                ->fakeAClient('client', 'client-b', 'client-b-secret')
                 ->setScopes(new Scope('client-b-scope'))
         ;
         $this->getClientManager()->save($clientB);
@@ -114,9 +114,9 @@ public function testListFiltersClients(): void
         $this->assertEquals(trim($expected), trim($output));
     }
 
-    private function fakeAClient($identifier, $secret = 'quzbaz'): Client
+    private function fakeAClient(string $name, string $identifier, ?string $secret = 'quzbaz'): Client
     {
-        return new Client($identifier, $secret);
+        return new Client($name, $identifier, $secret);
     }
 
     private function getClientManager(): ClientManagerInterface

tests/Acceptance/UpdateClientCommandTest.php

@@ -85,7 +85,7 @@ public function testDeactivate(): void
 
     private function fakeAClient($identifier): Client
     {
-        return new Client($identifier, 'quzbaz');
+        return new Client('name', $identifier, 'quzbaz');
     }
 
     private function getClientManager(): ClientManagerInterface

tests/Fixtures/FixtureFactory.php

@@ -256,26 +256,26 @@ private static function createClients(): array
     {
         $clients = [];
 
-        $clients[] = (new Client(self::FIXTURE_CLIENT_FIRST, 'secret'))
+        $clients[] = (new Client('name', self::FIXTURE_CLIENT_FIRST, 'secret'))
             ->setRedirectUris(new RedirectUri(self::FIXTURE_CLIENT_FIRST_REDIRECT_URI));
 
-        $clients[] = (new Client(self::FIXTURE_CLIENT_SECOND, 'top_secret'))
+        $clients[] = (new Client('name', self::FIXTURE_CLIENT_SECOND, 'top_secret'))
             ->setRedirectUris(new RedirectUri(self::FIXTURE_CLIENT_SECOND_REDIRECT_URI));
 
-        $clients[] = (new Client(self::FIXTURE_CLIENT_INACTIVE, 'woah'))
+        $clients[] = (new Client('name', self::FIXTURE_CLIENT_INACTIVE, 'woah'))
             ->setActive(false);
 
-        $clients[] = (new Client(self::FIXTURE_CLIENT_RESTRICTED_GRANTS, 'wicked'))
+        $clients[] = (new Client('name', self::FIXTURE_CLIENT_RESTRICTED_GRANTS, 'wicked'))
             ->setGrants(new Grant('password'));
 
-        $clients[] = (new Client(self::FIXTURE_CLIENT_RESTRICTED_SCOPES, 'beer'))
+        $clients[] = (new Client('name', self::FIXTURE_CLIENT_RESTRICTED_SCOPES, 'beer'))
             ->setRedirectUris(new RedirectUri(self::FIXTURE_CLIENT_FIRST_REDIRECT_URI))
             ->setScopes(new Scope(self::FIXTURE_SCOPE_SECOND));
 
-        $clients[] = (new Client(self::FIXTURE_PUBLIC_CLIENT, null))
+        $clients[] = (new Client('name', self::FIXTURE_PUBLIC_CLIENT, null))
             ->setRedirectUris(new RedirectUri(self::FIXTURE_PUBLIC_CLIENT_REDIRECT_URI));
 
-        $clients[] = (new Client(self::FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD, null))
+        $clients[] = (new Client('name', self::FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD, null))
             ->setAllowPlainTextPkce(true)
             ->setRedirectUris(new RedirectUri(self::FIXTURE_PUBLIC_CLIENT_ALLOWED_TO_USE_PLAIN_CHALLENGE_METHOD_REDIRECT_URI));