Fix #52 #54
Fix #52 #54
Conversation
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!
Codecov Report
@@ Coverage Diff @@
## master #54 +/- ##
============================================
+ Coverage 94.11% 94.44% +0.32%
- Complexity 21 24 +3
============================================
Files 1 1
Lines 51 54 +3
============================================
+ Hits 48 51 +3
Misses 3 3
Continue to review full report at Codecov.
|
| 'key' => null, | ||
| ]); | ||
|
|
||
| if (!array_key_exists('allowedAlgs', $config) || empty($config['allowedAlgs'])) { |
There was a problem hiding this comment.
Shouldn't just the empty() check suffice?
|
If you want to see warnings about a non-existant key... |
|
Tests please. |
| { | ||
| $key = 'my-custom-key'; | ||
| $auth = new JwtAuthenticate($this->Registry, [ | ||
| 'allowedAlgs' => 'RS256', |
There was a problem hiding this comment.
Setting 'allowedAlgs' to string would have overwritten the default array value even before this patch :) The merging happens when it's an array and that's what needs to be checked.
How so? We basically need to check if user have provided |
|
Fixed in c87d7c4. Thanks. |
|
I think the testcase I have added is of value too, as it makes sure the wrong algo fails! |
2 participants
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!