Merge pull request #163 from os2display/feature/openid-connect-code-flow

turegjorup · web-flow · commit d9b39babdcf7 · 2023-09-29T13:12:28.000+02:00
Code authorization flow
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,11 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+- [#163](https://github.com/os2display/display-api-service/pull/163)
+  Upgraded `itk-dev/openid-connect-bundle` to use code authorization flow. Updated OpenAPI spec accordingly.
+
 ## [1.4.0] - 2023-09-14
+
 - [#160](https://github.com/os2display/display-api-service/pull/160)
   Added app:feed:list-feed-source command. Removed listing from app:feed:remove-feed-source command.
 - [#159](https://github.com/os2display/display-api-service/pull/159)
diff --git a/README.md b/README.md
@@ -61,7 +61,7 @@ You can now obtain a token by sending a `POST` request to the
 
 ```curl
 curl -X 'POST' \
-  'http://displayapiservice.local.itkdev.dk/authentication/token' \
+  'http://displayapiservice.local.itkdev.dk/v1/authentication/token' \
   -H 'accept: application/json' \
   -H 'Content-Type: application/json' \
   -d '{
@@ -159,4 +159,4 @@ act -P ubuntu-latest=shivammathur/node:latest pull_request
 
 We use [SemVer](http://semver.org/) for versioning.
 For the versions available, see the
-[tags on this repository](https://github.com/itk-dev/openid-connect/tags).
+[tags on this repository](https://github.com/os2display/display-api-service/tags).
diff --git a/composer.json b/composer.json
@@ -17,7 +17,7 @@
         "doctrine/doctrine-migrations-bundle": "^3.1",
         "doctrine/orm": "^2.9",
         "gesdinet/jwt-refresh-token-bundle": "^1.0",
-        "itk-dev/openid-connect-bundle": "^2.0",
+        "itk-dev/openid-connect-bundle": "^3.0",
         "justinrainbow/json-schema": "^5.2",
         "kubawerlos/php-cs-fixer-custom-fixers": "^3.11",
         "lexik/jwt-authentication-bundle": "^2.14",
diff --git a/composer.lock b/composer.lock
diff --git a/config/packages/itkdev_openid_connect.yaml b/config/packages/itkdev_openid_connect.yaml
@@ -2,7 +2,7 @@ itkdev_openid_connect:
     cache_options:
         cache_pool: 'cache.app' # Cache item pool for caching discovery document and CLI login tokens
     cli_login_options:
-        cli_redirect: '%env(CLI_REDIRECT)%' # Redirect route for CLI login
+        route: '%env(CLI_REDIRECT)%' # Redirect route for CLI login
     openid_providers:
         # Define one or more providers
         # [providerKey]:
diff --git a/public/api-spec-v1.json b/public/api-spec-v1.json
@@ -23,7 +23,7 @@
                 ],
                 "responses": {
                     "200": {
-                        "description": "Get JWT token from OIDC token",
+                        "description": "Get JWT token from OIDC code",
                         "content": {
                             "application/json": {
                                 "schema": {
@@ -33,7 +33,7 @@
                         }
                     }
                 },
-                "summary": "Get JWT token to login from OIDC token",
+                "summary": "Get JWT token to login from OIDC code",
                 "description": "",
                 "parameters": [
                     {
@@ -47,8 +47,8 @@
                         }
                     },
                     {
-                        "name": "id_token",
-                        "description": "OIDC id token",
+                        "name": "code",
+                        "description": "OIDC code",
                         "in": "query",
                         "required": false,
                         "example": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
diff --git a/public/api-spec-v1.yaml b/public/api-spec-v1.yaml
@@ -17,12 +17,12 @@ paths:
         - Authentication
       responses:
         200:
-          description: 'Get JWT token from OIDC token'
+          description: 'Get JWT token from OIDC code'
           content:
             application/json:
               schema:
                 $ref: '#/components/schemas/Token'
-      summary: 'Get JWT token to login from OIDC token'
+      summary: 'Get JWT token to login from OIDC code'
       description: ''
       parameters:
         -
@@ -34,8 +34,8 @@ paths:
           schema:
             type: string
         -
-          name: id_token
-          description: 'OIDC id token'
+          name: code
+          description: 'OIDC code'
           in: query
           required: false
           example: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c
diff --git a/src/Command/User/AddUserCommand.php b/src/Command/User/AddUserCommand.php
@@ -111,11 +111,11 @@ protected function initialize(InputInterface $input, OutputInterface $output): v
      */
     protected function interact(InputInterface $input, OutputInterface $output): void
     {
-        if (null !== $input->getArgument('email') &&
-            null !== $input->getArgument('password') &&
-            null !== $input->getArgument('full-name') &&
-            null !== $input->getArgument('role') &&
-            null !== $input->getArgument('tenant-keys')
+        if (null !== $input->getArgument('email')
+            && null !== $input->getArgument('password')
+            && null !== $input->getArgument('full-name')
+            && null !== $input->getArgument('role')
+            && null !== $input->getArgument('tenant-keys')
         ) {
             return;
         }
diff --git a/src/Controller/AuthOidcController.php b/src/Controller/AuthOidcController.php
@@ -32,7 +32,7 @@ public function __construct(
     #[Route('/v1/authentication/oidc/token', name: 'authentication_oidc_token', methods: ['GET'])]
     public function getToken(Request $request): Response
     {
-        if ($request->query->has('state') && $request->query->has('id_token')) {
+        if ($request->query->has('state') && $request->query->has('code')) {
             try {
                 $passport = $this->oidcAuthenticator->authenticate($request);
 
@@ -71,7 +71,12 @@ public function getUrls(Request $request, SessionInterface $session): Response
             $session->set('oauth2nonce', $nonce);
 
             $data = [
-                'authorizationUrl' => $provider->getAuthorizationUrl(['state' => $state, 'nonce' => $nonce]),
+                'authorizationUrl' => $provider->getAuthorizationUrl([
+                    'state' => $state,
+                    'nonce' => $nonce,
+                    'response_type' => 'code',
+                    'scope' => 'openid email profile',
+                ]),
                 'endSessionUrl' => $provider->getEndSessionUrl(),
             ];
 
diff --git a/src/Entity/Tenant/Media.php b/src/Entity/Tenant/Media.php
@@ -138,7 +138,7 @@ public function setSha(string $sha): self
         return $this;
     }
 
-    public function setFile(?File $file = null): self
+    public function setFile(File $file = null): self
     {
         $this->file = $file;
 
diff --git a/src/Feed/KobaFeedType.php b/src/Feed/KobaFeedType.php
@@ -6,7 +6,6 @@
 use App\Entity\Tenant\FeedSource;
 use App\Exceptions\MissingFeedConfigurationException;
 use App\Service\FeedService;
-use Exception;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Uid\Ulid;
@@ -72,7 +71,7 @@ public function getData(Feed $feed): array
         foreach ($resources as $resource) {
             try {
                 $bookings = $this->getBookingsFromResource($kobaHost, $kobaApiKey, $resource, $kobaGroup, $from, $to);
-            } catch (Exception) {
+            } catch (\Exception) {
                 $this->logger->error('KobaFeedType: Get bookings from resources failed.');
                 continue;
             }
diff --git a/src/Filter/SharedWithMe.php b/src/Filter/SharedWithMe.php
@@ -19,7 +19,7 @@ class SharedWithMe extends AbstractContextAwareFilter
     private Security $security;
 
     public function __construct(
-        ManagerRegistry $managerRegistry, Security $security, ?RequestStack $requestStack = null, LoggerInterface $logger = null, array $properties = [], NameConverterInterface $nameConverter = null
+        ManagerRegistry $managerRegistry, Security $security, RequestStack $requestStack = null, LoggerInterface $logger = null, array $properties = [], NameConverterInterface $nameConverter = null
     ) {
         parent::__construct($managerRegistry, $requestStack, $logger, $properties, $nameConverter);
         $this->security = $security;
diff --git a/src/OpenApi/OpenApiFactory.php b/src/OpenApi/OpenApiFactory.php
@@ -255,7 +255,7 @@ public function __invoke(array $context = []): OpenApi
                 tags: ['Authentication'],
                 responses: [
                     '200' => [
-                        'description' => 'Get JWT token from OIDC token',
+                        'description' => 'Get JWT token from OIDC code',
                         'content' => [
                             'application/json' => [
                                 'schema' => [
@@ -265,7 +265,7 @@ public function __invoke(array $context = []): OpenApi
                         ],
                     ],
                 ],
-                summary: 'Get JWT token to login from OIDC token',
+                summary: 'Get JWT token to login from OIDC code',
                 parameters: [
                     [
                         'name' => 'state',
@@ -278,8 +278,8 @@ public function __invoke(array $context = []): OpenApi
                         ],
                     ],
                     [
-                        'name' => 'id_token',
-                        'description' => 'OIDC id token',
+                        'name' => 'code',
+                        'description' => 'OIDC code',
                         'in' => 'query',
                         'required' => false,
                         'example' => 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c',
diff --git a/src/Security/AzureOidcAuthenticator.php b/src/Security/AzureOidcAuthenticator.php
@@ -11,7 +11,6 @@
 use ItkDev\OpenIdConnectBundle\Security\OpenIdLoginAuthenticator;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\Exception\CustomUserMessageAuthenticationException;
@@ -30,9 +29,9 @@ class AzureOidcAuthenticator extends OpenIdLoginAuthenticator
     private EntityManagerInterface $entityManager;
     private TenantFactory $tenantFactory;
 
-    public function __construct(EntityManagerInterface $entityManager, SessionInterface $session, OpenIdConfigurationProviderManager $providerManager, TenantFactory $tenantFactory)
+    public function __construct(EntityManagerInterface $entityManager, OpenIdConfigurationProviderManager $providerManager, TenantFactory $tenantFactory)
     {
-        parent::__construct($providerManager, $session);
+        parent::__construct($providerManager);
 
         $this->entityManager = $entityManager;
         $this->tenantFactory = $tenantFactory;
diff --git a/src/Serializer/RelationNormalizer.php b/src/Serializer/RelationNormalizer.php
@@ -122,8 +122,8 @@ public function supportsNormalization($data, string $format = null, array $conte
             PlaylistScreenRegion::class,
         ];
 
-        return in_array($context['resource_class'], $types) &&
-            'collection' === $context['operation_type'] &&
-            $data instanceof Paginator;
+        return in_array($context['resource_class'], $types)
+            && 'collection' === $context['operation_type']
+            && $data instanceof Paginator;
     }
 }
diff --git a/tests/Api/LayoutsTest.php b/tests/Api/LayoutsTest.php
@@ -31,7 +31,7 @@ public function testGetLayoutCollection(): void
         // @TODO: We should have a test here matching the json schema for ScreenLayout, but it's not possible as it
         //        contains a sub-resource ScreenLayoutRegions. Figure out if matching the keys in the array is possible
         //        to validate data structure.
-//        $this->assertMatchesResourceCollectionJsonSchema(ScreenLayout::class);
+        //        $this->assertMatchesResourceCollectionJsonSchema(ScreenLayout::class);
     }
 
     public function testGetLayoutItem(): void
diff --git a/tests/Api/MediaTest.php b/tests/Api/MediaTest.php
@@ -31,7 +31,7 @@ public function testGetCollection(): void
         $this->assertCount(10, $response->toArray()['hydra:member']);
 
         // @TODO: hydra:member[0].assets: Object value found, but an array is required
-//        $this->assertMatchesResourceCollectionJsonSchema(Media::class);
+        //        $this->assertMatchesResourceCollectionJsonSchema(Media::class);
     }
 
     public function testGetItem(): void
@@ -72,7 +72,7 @@ public function testGetItem(): void
         ]);
 
         // @TODO: hydra:member[0].assets: Object value found, but an array is required
-//        $this->assertMatchesResourceItemJsonSchema(Media::class);
+        //        $this->assertMatchesResourceItemJsonSchema(Media::class);
     }
 
     public function testMediaUrlFromForeignTenant(): void
@@ -156,6 +156,6 @@ public function testMediaUpload(): void
         );
 
         // @TODO: hydra:member[0].assets: Object value found, but an array is required
-//        $this->assertMatchesResourceItemJsonSchema(Media::class);
+        //        $this->assertMatchesResourceItemJsonSchema(Media::class);
     }
 }
diff --git a/tests/Api/SlidesTest.php b/tests/Api/SlidesTest.php
diff --git a/tests/Api/TemplatesTest.php b/tests/Api/TemplatesTest.php

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@`
`23`	`23`	`],`
`24`	`24`	`"responses": {`
`25`	`25`	`"200": {`
`26`		`- "description": "Get JWT token from OIDC token",`
	`26`	`+ "description": "Get JWT token from OIDC code",`
`27`	`27`	`"content": {`
`28`	`28`	`"application/json": {`
`29`	`29`	`"schema": {`
`@@ -33,7 +33,7 @@`
`33`	`33`	`}`
`34`	`34`	`}`
`35`	`35`	`},`
`36`		`- "summary": "Get JWT token to login from OIDC token",`
	`36`	`+ "summary": "Get JWT token to login from OIDC code",`
`37`	`37`	`"description": "",`
`38`	`38`	`"parameters": [`
`39`	`39`	`{`
`@@ -47,8 +47,8 @@`
`47`	`47`	`}`
`48`	`48`	`},`
`49`	`49`	`{`
`50`		`- "name": "id_token",`
`51`		`- "description": "OIDC id token",`
	`50`	`+ "name": "code",`
	`51`	`+ "description": "OIDC code",`
`52`	`52`	`"in": "query",`
`53`	`53`	`"required": false,`
`54`	`54`	`"example": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",`
Original file line number	Diff line number	Diff line change
`@@ -138,7 +138,7 @@ public function setSha(string $sha): self`
`138`	`138`	`return $this;`
`139`	`139`	`}`
`140`	`140`
`141`		`- public function setFile(?File $file = null): self`
	`141`	`+ public function setFile(File $file = null): self`
`142`	`142`	`{`
`143`	`143`	`$this->file = $file;`
`144`	`144`