Added a DoctrineTokenProvider in Security/Core/Authentication/RememberMe #6057
Conversation
|
@TerjeBr The InMemoryTokenProvider is not an example, is an actual implementation which doesn't rely on external libraries (Doctrine). So I believe your class should be moved to the Doctrine Bridge namespace. |
| */ | ||
| class DoctrineTokenProvider implements TokenProviderInterface | ||
| { | ||
| private $em; |
|
This should indeed be moved to the bridge and should be tested |
|
This is actually a copy of some code that I am using in my application at work. I just changed the namespace and added some more comments before I made this pull request. You said it should be moved to the doctrine bridge. If you could tell me the exact directory, I can update my pull request accordingly. |
|
The bridge is here: https://github.com/symfony/symfony/tree/master/src/Symfony/Bridge/Doctrine |
|
Yes I know, but which sub directory should the file DoctrineTokenProvider.php go into? |
|
I am finished with this one. (I do not know how to add tests or documentation.) What happens next? Will this be merged as it is? |
|
@TerjeBr Nope, is not ready yet. If you don't know how to add tests or docs, at least you need to squash your commits into only one. |
This PR was merged into the master branch. Commits ------- d1b5093 Try to make sure cookies get deleted from the TokenProvider when no longer in use Discussion ---------- Delete cookies from the TokenProvider that is no longer in use Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Todo: - License of the code: MIT When the user logs in, or login fails for some reason, the old "remember me" cookie should be deleted from the TokenProvider if you are using the PersistentTokenBasedRememberMeServices. As the code is now, the token is only deleted on logout. --------------------------------------------------------------------------- by TerjeBr at 2012-11-20T13:45:54Z So, anything else that needs to be done before this is merged? --------------------------------------------------------------------------- by TerjeBr at 2012-11-21T10:30:53Z Ok, I have corrected the typo in the comment and squashed the commit. --------------------------------------------------------------------------- by schmittjoh at 2012-11-21T10:36:29Z btw, ``canceled`` (more American) and ``cancelled`` (more British) are both correct English forms. On Wed, Nov 21, 2012 at 11:30 AM, Terje Bråten <notifications@github.com>wrote: > Ok, I have corrected the typo in the comment and squashed the commit. > > — > Reply to this email directly or view it on GitHub<#6055 (comment)>. > > --------------------------------------------------------------------------- by schmittjoh at 2012-11-21T10:40:24Z As a side-note have you verified that this does not break the cookie theft protection? --------------------------------------------------------------------------- by TerjeBr at 2012-11-21T10:51:10Z Yes, cookie theft protection is still there and is functioning well. --------------------------------------------------------------------------- by TerjeBr at 2012-11-21T11:14:04Z I am using this together with the DoctrineTokenProvider in issue #6057 in my own project and done some extensive testing on it. --------------------------------------------------------------------------- by TerjeBr at 2012-11-23T10:30:34Z Is this ready to be merged now?
|
Thank you |
|
Any updates on this? |
| use Symfony\Component\Security\Core\Authentication\RememberMe\PersistentToken; | ||
| use Symfony\Component\Security\Core\Exception\TokenNotFoundException; | ||
| use Doctrine\DBAL\Connection; | ||
| use Doctrine\DBAL\Types\Type as DocType; |
There was a problem hiding this comment.
This should probably be renamed to DoctrineType as Doc can have many different meanings.
|
ping @beberlei |
| { | ||
| /** | ||
| * Doctrine DBAL database connection | ||
| * Service id: doctrine.dbal.default_connection |
There was a problem hiding this comment.
you should remove this comment. the bridge is not aware of service ids (and you can define several connections which will have different ids)
There was a problem hiding this comment.
This comment is meant as an example of where you can find a doctrine connection that is needed in the constructor.
I am sure many programmers will find it useful. Those programmers who already have a doctrine connection from another source they want to use are free to ignore that comment. It is just meant as hint to where you can get one, as I am sure that is the most common problem.
| * `lastUsed` datetime NOT NULL, | ||
| * `class` varchar(100) NOT NULL, | ||
| * `username` varchar(200) NOT NULL | ||
| * ); |
There was a problem hiding this comment.
you should provide a Schema class for this (as done for the ACL system)
There was a problem hiding this comment.
What is a Schema class? (and what is the ACL system?)
Do you have any URL references where I can read more about it?
There was a problem hiding this comment.
There was a problem hiding this comment.
@pborreli Just a link to the source code of Schema is very little helpful, when what I obviously need is some kind of introduction to the topic, or some good documentation.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ok, and where should I put this Schema then?
(What should be the name of the file, and in which directory?)
|
So, any updates on this? |
|
Ping. What happens next with this? |
|
I've cherry-picked the first commit only as the schema is not really useful as is. Feel free to submit another PR for the schema as done in other places in the framework. |
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Todo: Add tests and documentation
License of the code: MIT
In the directory Symfony/Component/Security/Core/Authentication/RememberMe there is only one TokenProvider that can be used, and that is the InMemoryTokenProvider.
I think it should be an example DoctrineTokenProvider there as well, that gives an example of how to store the tokens in the database.