Merge pull request #3725 from jrjohnson/firewall-fix

Require authentication to access by default
ilios · Nov 15, 2021 · b87f2d9 · b87f2d9
2 parents fd9f915 + fb967f3
commit b87f2d9
Show file tree

Hide file tree

Showing 16 changed files with 418 additions and 70 deletions.
diff --git a/config/packages/security.yaml b/config/packages/security.yaml
@@ -1,8 +1,16 @@
 security:
   enable_authenticator_manager: true
   access_control:
-    - { path: ^/api/doc, roles: PUBLIC_ACCESS }
-    - { path: ^/(auth|application)/(login|config|logout), roles: PUBLIC_ACCESS }
+    - { path: '^/api/doc', roles: PUBLIC_ACCESS }
+    - { path: '^/api$', roles: PUBLIC_ACCESS }
+    - { path: '^/application/config', roles: PUBLIC_ACCESS }
+    - { path: '^/auth/(login|logout)', roles: PUBLIC_ACCESS }
+    - { path: '^/auth', roles: IS_AUTHENTICATED_FULLY }
+    - { path: '^/api', roles: IS_AUTHENTICATED_FULLY }
+    - { path: '^/application', roles: IS_AUTHENTICATED_FULLY }
+    - { path: '^/upload', roles: IS_AUTHENTICATED_FULLY }
+    - { path: '^/error', roles: IS_AUTHENTICATED_FULLY }
+    - { path: '^/', roles: PUBLIC_ACCESS }
   access_decision_manager:
     allow_if_all_abstain: false
     strategy: unanimous
@@ -18,32 +26,7 @@ security:
     dev:
       pattern: ^/(_(profiler|wdt)|css|images|js)/
       security: false
-    authenticated_auth:
-      pattern:    ^/auth
-      stateless: true
-      custom_authenticators:
-        - App\Security\JsonWebTokenAuthenticator
-      provider: session_user
-    authenticated_application:
-      pattern:    ^/application
-      stateless: true
-      custom_authenticators:
-        - App\Security\JsonWebTokenAuthenticator
-      provider: session_user
-    upload:
-      pattern:    ^/upload
-      stateless: true
-      custom_authenticators:
-        - App\Security\JsonWebTokenAuthenticator
-      provider: session_user
-    errors:
-      pattern:    ^/errors
-      stateless: true
-      custom_authenticators:
-        - App\Security\JsonWebTokenAuthenticator
-      provider: session_user
-    default:
-      pattern:    ^/api
+    main:
       stateless: true
       custom_authenticators:
         - App\Security\JsonWebTokenAuthenticator

diff --git a/src/Controller/AuthController.php b/src/Controller/AuthController.php
@@ -17,6 +17,8 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 
+use function sleep;
+
 class AuthController extends AbstractController
 {
     /**
@@ -35,15 +37,12 @@ public function loginAction(Request $request, AuthenticationInterface $authentic
     public function whoamiAction(TokenStorageInterface $tokenStorage): JsonResponse
     {
         $token = $tokenStorage->getToken();
-        if ($token?->isAuthenticated()) {
-            /** @var SessionUserInterface $sessionUser */
-            $sessionUser = $token->getUser();
-            if ($sessionUser instanceof SessionUserInterface) {
-                return new JsonResponse(['userId' => $sessionUser->getId()], Response::HTTP_OK);
-            }
+        $sessionUser = $token?->getUser();
+        if (!$token?->isAuthenticated() || !$sessionUser instanceof SessionUserInterface) {
+            throw new Exception('Attempted to access whoami with no valid user');
         }
 
-        return new JsonResponse(['userId' => null], Response::HTTP_UNAUTHORIZED);
+        return new JsonResponse(['userId' => $sessionUser->getId()], Response::HTTP_OK);
     }
 
     /**
@@ -56,16 +55,14 @@ public function tokenAction(
         JsonWebTokenManager $jwtManager
     ): JsonResponse {
         $token = $tokenStorage->getToken();
-        if ($token?->isAuthenticated()) {
-            $sessionUser = $token->getUser();
-            if ($sessionUser instanceof SessionUserInterface) {
-                $ttl = $request->get('ttl') ? $request->get('ttl') : 'PT8H';
-                $jwt = $jwtManager->createJwtFromSessionUser($sessionUser, $ttl);
-                return new JsonResponse(['jwt' => $jwt], Response::HTTP_OK);
-            }
+        $sessionUser = $token?->getUser();
+        if (!$token?->isAuthenticated() || !$sessionUser instanceof SessionUserInterface) {
+            throw new Exception('Attempted to access token with no valid user');
         }
 
-        return new JsonResponse(['jwt' => null], Response::HTTP_UNAUTHORIZED);
+        $ttl = $request->get('ttl') ? $request->get('ttl') : 'PT8H';
+        $jwt = $jwtManager->createJwtFromSessionUser($sessionUser, $ttl);
+        return new JsonResponse(['jwt' => $jwt], Response::HTTP_OK);
     }
 
     /**
@@ -92,28 +89,25 @@ public function invalidateTokensAction(
     ): JsonResponse {
         $now = new \DateTime();
         $token = $tokenStorage->getToken();
-        if ($token?->isAuthenticated()) {
-            /** @var SessionUserInterface $sessionUser */
-            $sessionUser = $token->getUser();
-            if ($sessionUser instanceof SessionUserInterface) {
-                /** @var UserInterface $user */
-                $user = $userRepository->findOneBy(['id' => $sessionUser->getId()]);
-                $authentication = $authenticationRepository->findOneBy(['user' => $user->getId()]);
-                if (!$authentication) {
-                    $authentication = $authenticationRepository->create();
-                    $authentication->setUser($user);
-                }
+        $sessionUser = $token?->getUser();
+        if (!$token?->isAuthenticated() || !$sessionUser instanceof SessionUserInterface) {
+            throw new Exception('Attempted to access invalidate tokens with no valid user');
+        }
 
-                $authentication->setInvalidateTokenIssuedBefore($now);
-                $authenticationRepository->update($authentication);
+        /** @var UserInterface $user */
+        $user = $userRepository->findOneBy(['id' => $sessionUser->getId()]);
+        $authentication = $authenticationRepository->findOneBy(['user' => $user->getId()]);
+        if (!$authentication) {
+            $authentication = $authenticationRepository->create();
+            $authentication->setUser($user);
+        }
 
-                sleep(1);
-                $jwt = $jwtManager->createJwtFromSessionUser($sessionUser);
+        $authentication->setInvalidateTokenIssuedBefore($now);
+        $authenticationRepository->update($authentication);
 
-                return new JsonResponse(['jwt' => $jwt], Response::HTTP_OK);
-            }
-        }
+        sleep(1);
+        $jwt = $jwtManager->createJwtFromSessionUser($sessionUser);
 
-        throw new Exception('Attempted to invalidate token with no valid user');
+        return new JsonResponse(['jwt' => $jwt], Response::HTTP_OK);
     }
 }
diff --git a/tests/AbstractEndpointTest.php b/tests/AbstractEndpointTest.php
@@ -960,6 +960,28 @@ protected function badPostTest(array $data, $code = Response::HTTP_BAD_REQUEST)
         $this->assertJsonResponse($response, $code);
     }
 
+    /**
+     * Test POSTing without authentication to the API
+     */
+    protected function anonymousDeniedPostTest(array $data)
+    {
+        $endpoint = $this->getPluralName();
+        $responseKey = $this->getCamelCasedPluralName();
+        $this->createJsonRequest(
+            'POST',
+            $this->getUrl(
+                $this->kernelBrowser,
+                "app_api_${endpoint}_post",
+                ['version' => $this->apiVersion]
+            ),
+            json_encode([$responseKey => [$data]]),
+        );
+
+        $response = $this->kernelBrowser->getResponse();
+
+        $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
+    }
+
     /**
      * Test POSTing bad data to the API
      * @param array $data
@@ -985,6 +1007,28 @@ protected function badPutTest(array $data, $id, $code = Response::HTTP_BAD_REQUE
         $this->assertJsonResponse($response, $code);
     }
 
+    /**
+     * Test PUTing as anonymous to the API
+     */
+    protected function anonymousDeniedPutTest(array $data)
+    {
+        $endpoint = $this->getPluralName();
+        $responseKey = $this->getCamelCasedPluralName();
+        $this->createJsonRequest(
+            'PUT',
+            $this->getUrl(
+                $this->kernelBrowser,
+                "app_api_${endpoint}_put",
+                ['version' => $this->apiVersion, 'id' => $data['id']]
+            ),
+            json_encode([$responseKey => [$data]]),
+        );
+
+        $response = $this->kernelBrowser->getResponse();
+
+        $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
+    }
+
     /**
      * When relational data is sent to the API ensure it
      * is recorded on the non-owning side of the relationship
@@ -1126,6 +1170,28 @@ protected function patchJsonApiTest(array $data, object $postData)
         return $fetchedResponseData;
     }
 
+    /**
+     * Test PATCHing as anonymous to the API
+     */
+    protected function anonymousDeniedPatchTest(array $data)
+    {
+        $endpoint = $this->getPluralName();
+        $responseKey = $this->getCamelCasedPluralName();
+        $this->createJsonRequest(
+            'PATCH',
+            $this->getUrl(
+                $this->kernelBrowser,
+                "app_api_${endpoint}_patch",
+                ['version' => $this->apiVersion, 'id' => $data['id']]
+            ),
+            json_encode([$responseKey => [$data]]),
+        );
+
+        $response = $this->kernelBrowser->getResponse();
+
+        $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
+    }
+
     /**
      * Test deleting an object from the API
      *
@@ -1191,6 +1257,49 @@ protected function notFoundTest($badId)
         $this->assertJsonResponse($response, Response::HTTP_NOT_FOUND);
     }
 
+    /**
+     * Ensure that anonymous users cannot access the resource
+     */
+    protected function anonymousAccessDeniedOneTest()
+    {
+        $endpoint = $this->getPluralName();
+        $loader = $this->getDataLoader();
+        $data = $loader->getOne();
+        $this->createJsonRequest(
+            'GET',
+            $this->getUrl(
+                $this->kernelBrowser,
+                "app_api_${endpoint}_getone",
+                ['version' => $this->apiVersion, 'id' => $data['id']]
+            ),
+        );
+
+        $response = $this->kernelBrowser->getResponse();
+
+        $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
+    }
+
+    /**
+     * Ensure that anonymous users cannot access the resource
+     */
+    protected function anonymousAccessDeniedAllTest()
+    {
+        $endpoint = $this->getPluralName();
+        $loader = $this->getDataLoader();
+        $this->createJsonRequest(
+            'GET',
+            $this->getUrl(
+                $this->kernelBrowser,
+                "app_api_${endpoint}_getall",
+                ['version' => $this->apiVersion]
+            ),
+        );
+
+        $response = $this->kernelBrowser->getResponse();
+
+        $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
+    }
+
     /**
      * Test that a filter returns the expected data
      * @param array $filters we are using

diff --git a/tests/Controller/ApiControllerTest.php b/tests/Controller/ApiControllerTest.php
@@ -72,4 +72,31 @@ public function testBadVersion()
         $response = $this->kernelBrowser->getResponse();
         $this->assertJsonResponse($response, Response::HTTP_NOT_FOUND);
     }
+
+    public function testApiInfoAuthenticated()
+    {
+        $this->kernelBrowser->request(
+            'GET',
+            '/api',
+            [],
+            [],
+            ['HTTP_X-JWT-Authorization' => 'Token ' . $this->getTokenForUser($this->kernelBrowser, 1)],
+        );
+
+        $response = $this->kernelBrowser->getResponse();
+        $this->assertEquals(Response::HTTP_OK, $response->getStatusCode());
+        $this->assertStringContainsString('<h1>API Info</h1>', $response->getContent());
+    }
+
+    public function testApiInfoNotAuthenticated()
+    {
+        $this->kernelBrowser->request(
+            'GET',
+            '/api',
+        );
+
+        $response = $this->kernelBrowser->getResponse();
+        $this->assertEquals(Response::HTTP_OK, $response->getStatusCode());
+        $this->assertStringContainsString('<h1>API Info</h1>', $response->getContent());
+    }
 }
diff --git a/tests/Controller/AuthControllerTest.php b/tests/Controller/AuthControllerTest.php
@@ -158,9 +158,6 @@ public function testWhoAmIUnauthenticated()
 
         $response = $this->kernelBrowser->getResponse();
         $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
-        $response = json_decode($response->getContent(), true);
-        $this->assertArrayHasKey('userId', $response);
-        $this->assertSame($response['userId'], null);
     }
 
     public function testWhoAmIExpiredToken()
@@ -242,9 +239,6 @@ public function testGetTokenForUnauthenticatedUser()
         );
         $response = $this->kernelBrowser->getResponse();
         $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
-        $response = json_decode($response->getContent(), true);
-        $this->assertArrayHasKey('jwt', $response);
-        $this->assertSame($response['jwt'], null);
     }
 
     public function testGetTokenForExpiredToken()
@@ -301,7 +295,7 @@ public function testInvalidateTokenForUnauthenticatedUser()
         );
 
         $response = $this->kernelBrowser->getResponse();
-        $this->assertJsonResponse($response, Response::HTTP_INTERNAL_SERVER_ERROR);
+        $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
     }
 
     protected function getExpiredToken(int $userId): string

diff --git a/tests/Controller/ErrorControllerTest.php b/tests/Controller/ErrorControllerTest.php
@@ -53,4 +53,23 @@ public function testIndex()
         $response = $this->kernelBrowser->getResponse();
         $this->assertEquals(Response::HTTP_NO_CONTENT, $response->getStatusCode(), $response->getContent());
     }
+
+    public function testAnonymousAccessDenied()
+    {
+        $faker = FakerFactory::create();
+
+        $data = [
+            'mainMessage' => $faker->text(100),
+            'stack' => $faker->text(1000)
+        ];
+        $this->makeJsonRequest(
+            $this->kernelBrowser,
+            'POST',
+            '/errors',
+            json_encode(['data' => json_encode($data)])
+        );
+
+        $response = $this->kernelBrowser->getResponse();
+        $this->assertEquals(Response::HTTP_UNAUTHORIZED, $response->getStatusCode());
+    }
 }
diff --git a/tests/Controller/UploadControllerTest.php b/tests/Controller/UploadControllerTest.php
@@ -65,6 +65,22 @@ public function testUploadFile()
         $this->assertSame($data['filename'], 'TESTFILE.txt');
         $this->assertSame($data['fileHash'], md5_file(__FILE__));
     }
+    public function testAnonymousUploadFileDenied()
+    {
+        $client = static::createClient();
+
+        $this->makeJsonRequest(
+            $client,
+            'POST',
+            '/upload',
+            null,
+            [],
+            ['file' => $this->fakeTestFile]
+        );
+
+        $response = $client->getResponse();
+        $this->assertJsonResponse($response, Response::HTTP_UNAUTHORIZED);
+    }
 
     public function testBadUpload()
     {