Skip to content

Commit

Permalink
Replace tuupola/slim-jwt-auth with our fork
Browse files Browse the repository at this point in the history
slim-jwt-auth currently does not support firebase/php-jwt 6.
This raise critical security alerts when our containers are scanned
(namely CVE-2021-46743).

There is an issue on the slim-jwt-auth repo for this, which I
have commented on, asking for further info:

tuupola/slim-jwt-auth#217

In the meantime, I have taken a copy of the key part of that
package and reworked it slightly so that it functions with
php-jwt v6. I also removed a lot of the options we're not
using.

The existing middleware which functions with slim-jwt-auth is
commented out but still present. To reinstate that package,
we just need to remove this commit.
  • Loading branch information
townxelliot committed May 19, 2022
1 parent 1ccc944 commit 90926f3
Show file tree
Hide file tree
Showing 8 changed files with 450 additions and 118 deletions.
8 changes: 7 additions & 1 deletion service-admin/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@
"ministryofjustice/opg-lpa-datamodels": "^13.3",
"php-http/guzzle6-adapter": "^2.0.2",
"slim/flash": "^0.4.0",
"tuupola/slim-jwt-auth": "^3.5.2",

"firebase/php-jwt": "^6.0",
"psr/http-message": "^1.0",
"tuupola/http-factory": "^0.4.0|^1.0.2",
"tuupola/callable-handler": "^0.3.0|^0.4.0|^1.0",
"psr/http-server-middleware": "^1.0",

"laminas/laminas-authentication": "^2.8",
"laminas/laminas-cache": "^3.1.2",
"laminas/laminas-cache-storage-adapter-memory": "2.0",
Expand Down
132 changes: 31 additions & 101 deletions service-admin/composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions service-admin/config/autoload/mezzio.global.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,17 @@

'api_base_uri' => getenv('OPG_LPA_ENDPOINTS_API') ?: null,

'admin_accounts' => (getenv('OPG_LPA_COMMON_ADMIN_ACCOUNTS') ? explode(',', getenv('OPG_LPA_COMMON_ADMIN_ACCOUNTS')) : []),
'admin_accounts' => (
getenv('OPG_LPA_COMMON_ADMIN_ACCOUNTS') ?
explode(',', getenv('OPG_LPA_COMMON_ADMIN_ACCOUNTS')) : []
),

'jwt' => [
'secret' => getenv('OPG_LPA_ADMIN_JWT_SECRET') ?: null,
'path' => '/',
'header' => 'lpa-admin',
'cookie' => 'lpa-admin',
'ttl' => 60 * 15, // 15 minutes
'ttl' => 60 * 15, // 15 minutes
'algo' => 'HS256',
],

Expand Down
12 changes: 7 additions & 5 deletions service-admin/config/pipeline.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
<?php

/**
* Setup middleware pipeline:
*/

declare(strict_types=1);

use App\Middleware;
Expand All @@ -17,10 +21,7 @@
use Mezzio\Router\Middleware\RouteMiddleware;
use Laminas\Stratigility\Middleware\ErrorHandler;

/**
* Setup middleware pipeline:
*/
return function (Application $app, MiddlewareFactory $factory, ContainerInterface $container) : void {
return function (Application $app, MiddlewareFactory $factory, ContainerInterface $container): void {
// The error handler should be the first (most outer) middleware to catch
// all Exceptions.
$app->pipe(ErrorHandler::class);
Expand Down Expand Up @@ -60,7 +61,8 @@

// Set up the custom middleware to handle sessions and authorization
$app->pipe(Middleware\Session\SessionMiddleware::class);
$app->pipe(JwtAuthentication::class);
//$app->pipe(JwtAuthentication::class);
$app->pipe(Middleware\Session\JwtMiddleware::class);
$app->pipe(Middleware\Authorization\AuthorizationMiddleware::class);
$app->pipe(Middleware\Session\CsrfMiddleware::class);
$app->pipe(Middleware\Flash\SlimFlashMiddleware::class);
Expand Down
8 changes: 5 additions & 3 deletions service-admin/src/App/src/ConfigProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace App;

use App\Logging\LoggingErrorListenerDelegatorFactory;
use Tuupola\Middleware\JwtAuthentication;
//use Tuupola\Middleware\JwtAuthentication;
use Laminas\Stratigility\Middleware\ErrorHandler;

/**
Expand Down Expand Up @@ -64,8 +64,10 @@ public function getDependencies(): array
Handler\UserFindHandler::class => Handler\UserFindHandlerFactory::class,

// Middleware
JwtAuthentication::class =>
Middleware\Session\JwtAuthenticationFactory::class,
//JwtAuthentication::class =>
// Middleware\Session\JwtAuthenticationFactory::class,
Middleware\Session\JwtMiddleware::class =>
Middleware\Session\JwtMiddlewareFactory::class,
Middleware\Authorization\AuthorizationMiddleware::class =>
Middleware\Authorization\AuthorizationMiddlewareFactory::class,
Middleware\Session\SessionMiddleware::class =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,23 +90,23 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
$roles = ['guest'];

if (is_string($token)) {
// Attempt to get a user with the token value
// Attempt to get a user with the token value
$result = $this->authenticationService->verify($token);

$identity = $result->getIdentity();

if ($identity instanceof Identity) {
// Try to get the user details
// Try to get the user details
$user = $this->userService->fetch($identity->getUserId() ?? '');

// There is something wrong with the user here so throw an exception
// There is something wrong with the user here so throw an exception
if (!$user instanceof User) {
throw new Exception('Can not find a user for ID ' . $identity->getUserId());
}

$roles[] = 'authenticated-user';
} else {
// Clear the bad token
// Clear the bad token
$this->clearTokenData();
}
}
Expand All @@ -123,7 +123,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
// Check each role to see if the user has access to the route
foreach ($roles as $role) {
if ($this->rbac->hasRole($role) && $this->rbac->isGranted($role, $matchedRoute->getName())) {
// Catch any unauthorized exceptions and trigger a sign out if required
// Catch any unauthorized exceptions and trigger a sign out if required
try {
return $handler->handle($request->withAttribute('user', $user));
} catch (ApiException $ae) {
Expand All @@ -136,7 +136,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
}
}

// If there is no user (not logged in) then redirect to the sign in screen
// If there is no user (not logged in) then redirect to the sign in screen
if (is_null($user)) {
return new RedirectResponse($this->urlHelper->generate('sign.in'));
}
Expand Down
Loading

0 comments on commit 90926f3

Please sign in to comment.