Release 5.1.0 #230
Release 5.1.0 #230
Conversation
* Added State Storage / Check * Added StateHandler interface Added Dummy, Session StateHandlers Moved state related generation, validation to handlers Restore Tests * Add Docs
README.md
Outdated
| @@ -24,6 +24,14 @@ Check our docs page to get a complete guide on how to install it in an existing | |||
|
|
|||
| > If you find something wrong in our docs, PR are welcome in our docs repo: https://github.com/auth0/docs | |||
|
|
|||
| ## Security Upgrade Notes 5.1.0+ | |||
|
|
|||
| **State validation** is now default behaviour for improved security. By default this will automatically use **Session Storage** and will | |||
There was a problem hiding this comment.
probably not intended carriage return
| * @author Auth0 | ||
| */ | ||
| class DummyStateHandler implements StateHandler | ||
| { |
There was a problem hiding this comment.
class braces style is different from the method one?
There was a problem hiding this comment.
Shouldn't be but not consistent in the library (yet)
| * | ||
| * @author Auth0 | ||
| */ | ||
| interface StateHandler { |
There was a problem hiding this comment.
inconsistent brace style 😛
| @@ -8,6 +8,9 @@ | |||
| use Auth0\SDK\Store\SessionStore; | |||
| use Auth0\SDK\Store\StoreInterface; | |||
| use Auth0\SDK\API\Authentication; | |||
| use Auth0\SDK\API\Helpers\State\StateHandler; | |||
There was a problem hiding this comment.
I don't see StateHandler being instantiated or referenced here. I do see SessionStateHandler and DummyStateHandler though
There was a problem hiding this comment.
It's mentioned in a docblock, which is flagged in my IDE
| @@ -238,7 +256,8 @@ public function __construct(array $config) { | |||
| $this->refresh_token = $this->store->get("refresh_token"); | |||
| } | |||
|
|
|||
| public function login($state = null, $connection = null) { | |||
| public function login($state = null, $connection = null, $additional_params = []) { | |||
There was a problem hiding this comment.
I guess this is NOT a breaking change because you're passing [] as default value when the parameter is missing. Am I correct?
src/Auth0.php
Outdated
| $params['response_mode'] = $this->response_mode; | ||
|
|
||
| if($additional_params) { |
There was a problem hiding this comment.
if my statement is correct, then this will always be either [] or some (I say good) value passed by the user. In any case, this if is always true. Maybe the check should be if($additional_params && $additional_params.length/size>0)
There was a problem hiding this comment.
That if will check for truth-y values so an empty array or null will skip it. That said, if someone passed in a 1 or a string or something, they'll get an error. Probably should be something like:
if(!empty($additional_params) && is_array($additional_params)) {
$params = array_replace($params, $additional_params);
}There was a problem hiding this comment.
I will accept the change, however as long as the parameter type is documented somewhere, users shouldn't be passing invalid (non-array) values. So next time: doc + if($var) should be enough.
| use Auth0\SDK\API\Helpers\State\SessionStateHandler; | ||
| use Auth0\SDK\Store\SessionStore; | ||
|
|
||
| class StateHandlerTest extends \PHPUnit_Framework_TestCase { |
There was a problem hiding this comment.
PHP namespace thing, means 'use the global namespace'
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Closed issues
Added
Changed