Chain authentication storage #2718
Chain authentication storage #2718
Conversation
The example is a bit flawed -- session is always queried first anyways when you use AuthenticationService::(has|get)Identity(). However, I've seen developers wanting to chain oauth, http, and db before (this was an issue with ZfcUser) -- I think @EvanDotPro may have created something like your authentication chain in that module as well), and as such, this is definitely a useful addition. |
Assigned @EvanDotPro to evaluate. |
Needs tests, for sure, @roelvanduijnhoven ! |
Allright! As soon as I receive positive feedback that this component could make it into master and it is not a duplicate of something already present in ZfcUser I start writing tests. |
Write tests anyway. Don't leave the tests as an after thought... |
I originally wrote this component for the use case I presented in the PRs introduction. @weierophinney was right however that the example was flawed. In my own project I now use the Still it could be usefull to other people. Therefore I just published a new commit introducing tests for this storage. These test should show more clearly my original intention. |
@roelvanduihnhoven This looks reasonable, particularly with the new test cases. I'm going to leave some detailed feedback on code momentarily -- however, my bigger question is: do you feel you can incorporate that feedback and get some docs on this storage adapter in place by end of this week? If so, I'll keep it scheduled for 2.1; otherwise, I'll clear the milestone, and push it to 2.2. Thanks! Also, please rebase this off current master so that it can merge cleanly. :) |
* @param StorageInterface $storage | ||
* @param integer $priority | ||
*/ | ||
public function add( StorageInterface $storage, $priority = 1 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CS: remove spaces inside the opening and closing parentheses; this is true throughout the code.
Run the code through |
chain multiple storages together.
The comments have all been processed @weierophinney and I have rebased against the latest master. I could write some documentation about this storage adapter, no problem. Proposal would be to write a subsection under the identity persistence section on this page this page. Does that sound allright? |
@roelvanduijnhoven Re: documentation -- yes, exactly there. I'll merge momentarily! |
- Remove leading/trailing space inside method argument lists.
Merged to develop branch, for release with 2.1.0. |
- Remove leading/trailing space inside method argument lists.
This component can be usefull if you want to store the identity in multiple places. For example once in the database and once in
$_SESSION
. If either one of the storages is non-empty then the chain should also be non-empty.On top of that the order in which the storages are accessed is important. A use case that is often used: the identity is stored in the database and can be obtained using a cookie on the client. However one wants to retrieve this data from the database only on the first request of a browser session and therefore the identity is stored in
$_SESSION
.Such configuration might look like:
I developed the component for a website I am developing, but though it might be of use to other people.