-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add new CSRF manager for unit testing purposes #21894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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.
do we expect to have multiple implementation here?
|
a lot of stuff foe just one little tiny token .... |
This adds a new CSRF manager for unit testing purposes, it's interface is based upon https://github.com/symfony/security-csrf. Due to some of our required custom changes it is however not possible to use the Symfony component directly.
Otherwise somebody else might be able to note down the CSRF token before login on a shared computer.
d48e514 to
12b22c2
Compare
To be fair, most of it is simply boiler plate code (e.g. copyright header, class declaration, PHPDocs) and in the end it's simply moving static code out of some place and putting it in a unit-tested other place. As well as giving additional functionality such as invalidating a token (which was actually the reason for the change). Actual logic:
The alternative would have been adding more static untested code. In fact, I'm tempted to say the actual logic has not really increased at all. Other comments have been addressed. |
|
Looks a lot cleaner and yay for more non-static code! I did some tests and everything still works and the code did not burn down my house. 👍 |
|
Code looks good and it works 👍 |
Add new CSRF manager for unit testing purposes
This adds a new CSRF manager for unit testing purposes, it's interface is based upon https://github.com/symfony/security-csrf. Due to some of our required custom changes it is however not possible to use the Symfony component directly. Most of this are unit test adjustments.
Furthermore, this regenerates the CSRF token upon login because otherwise somebody else might be able to note down the CSRF token before login on a shared computer.
cc @rullzer @MorrisJobke @owncloud/security
Fixes #12159