From 9e4969ed77b8d5a63be9a950ff51a3342ad84bce Mon Sep 17 00:00:00 2001 From: Hans-Joachim Michl Date: Fri, 28 Apr 2017 11:51:58 +0200 Subject: [PATCH 1/5] Fix #52 Only add the default Alg HS256 if none are provided in the custom config, the array gets merged deep, so any default value set will be available whatever gets set from the config (in addition to it). This is a serve security thread: When setting RS256 it's possible to make up a new token with the public key only that would verify with the HS256 algo! --- src/Auth/JwtAuthenticate.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Auth/JwtAuthenticate.php b/src/Auth/JwtAuthenticate.php index 2746c8d..ab813e3 100644 --- a/src/Auth/JwtAuthenticate.php +++ b/src/Auth/JwtAuthenticate.php @@ -91,13 +91,16 @@ public function __construct(ComponentRegistry $registry, $config) 'header' => 'authorization', 'prefix' => 'bearer', 'parameter' => 'token', - 'allowedAlgs' => ['HS256'], 'queryDatasource' => true, 'fields' => ['username' => 'id'], 'unauthenticatedException' => '\Cake\Network\Exception\UnauthorizedException', 'key' => null, ]); - + if(!array_key_exists('allowedAlgs',$config) || empty($config['allowedAlgs']) { + $this->config([ + 'allowedAlgs' => ['HS256'], + ]); + } parent::__construct($registry, $config); } From 51171dd20a0539b7fcc18de958049520c6194986 Mon Sep 17 00:00:00 2001 From: Hans-Joachim Michl Date: Fri, 28 Apr 2017 11:59:57 +0200 Subject: [PATCH 2/5] Update JwtAuthenticate.php --- src/Auth/JwtAuthenticate.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Auth/JwtAuthenticate.php b/src/Auth/JwtAuthenticate.php index ab813e3..16c59d2 100644 --- a/src/Auth/JwtAuthenticate.php +++ b/src/Auth/JwtAuthenticate.php @@ -96,11 +96,13 @@ public function __construct(ComponentRegistry $registry, $config) 'unauthenticatedException' => '\Cake\Network\Exception\UnauthorizedException', 'key' => null, ]); - if(!array_key_exists('allowedAlgs',$config) || empty($config['allowedAlgs']) { + + if (!array_key_exists('allowedAlgs', $config) || empty($config['allowedAlgs']) { $this->config([ 'allowedAlgs' => ['HS256'], ]); } + parent::__construct($registry, $config); } From 8aca1d4f012977b520dec9b62af3fffaeb5ac246 Mon Sep 17 00:00:00 2001 From: Hans-Joachim Michl Date: Fri, 28 Apr 2017 12:06:17 +0200 Subject: [PATCH 3/5] Update JwtAuthenticate.php --- src/Auth/JwtAuthenticate.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Auth/JwtAuthenticate.php b/src/Auth/JwtAuthenticate.php index 16c59d2..188a7a9 100644 --- a/src/Auth/JwtAuthenticate.php +++ b/src/Auth/JwtAuthenticate.php @@ -97,7 +97,7 @@ public function __construct(ComponentRegistry $registry, $config) 'key' => null, ]); - if (!array_key_exists('allowedAlgs', $config) || empty($config['allowedAlgs']) { + if (!array_key_exists('allowedAlgs', $config) || empty($config['allowedAlgs'])) { $this->config([ 'allowedAlgs' => ['HS256'], ]); From 8d86bd213a760ec630427f86f7320be6494d553d Mon Sep 17 00:00:00 2001 From: Hans-Joachim Michl Date: Wed, 31 May 2017 17:43:41 +0200 Subject: [PATCH 4/5] Added testcase --- tests/TestCase/Auth/JwtAuthenticateTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/TestCase/Auth/JwtAuthenticateTest.php b/tests/TestCase/Auth/JwtAuthenticateTest.php index 5ddb8c5..d985305 100644 --- a/tests/TestCase/Auth/JwtAuthenticateTest.php +++ b/tests/TestCase/Auth/JwtAuthenticateTest.php @@ -276,4 +276,24 @@ public function testCustomKey() $result = $auth->getUser($request, $this->response); $this->assertEquals($payload, $result); } + + /** + * test if allowedAlgs gets overwritten, not merged with default config. + * + * @expectedException InvalidArgumentException + */ + public function testOverwriteAlgs() + { + $key = 'my-custom-key'; + $auth = new JwtAuthenticate($this->Registry, [ + 'allowedAlgs' => 'RS256', + ]); + + $payload = ['sub' => 100]; + $token = Jwt::encode($payload, $key); + $request = new Request(); + $request->env('HTTP_AUTHORIZATION', 'Bearer ' . $token); + + $result = $auth->getUser($request, $this->response); + } } From ef1890c0279384992e7eb64bcc1d29b24242bc98 Mon Sep 17 00:00:00 2001 From: Hans-Joachim Michl Date: Thu, 1 Jun 2017 02:40:22 +0200 Subject: [PATCH 5/5] Fix testcase --- tests/TestCase/Auth/JwtAuthenticateTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/TestCase/Auth/JwtAuthenticateTest.php b/tests/TestCase/Auth/JwtAuthenticateTest.php index d985305..2b63bc4 100644 --- a/tests/TestCase/Auth/JwtAuthenticateTest.php +++ b/tests/TestCase/Auth/JwtAuthenticateTest.php @@ -280,13 +280,13 @@ public function testCustomKey() /** * test if allowedAlgs gets overwritten, not merged with default config. * - * @expectedException InvalidArgumentException + * @expectedException UnexpectedValueException */ public function testOverwriteAlgs() { $key = 'my-custom-key'; $auth = new JwtAuthenticate($this->Registry, [ - 'allowedAlgs' => 'RS256', + 'allowedAlgs' => ['RS256'], ]); $payload = ['sub' => 100];