Skip to content

[stable13] Don't perform CSRF check on OCS routes with Bearer auth - #8099

Merged
rullzer merged 2 commits into
stable13from
7873_13
Feb 28, 2018
Merged

[stable13] Don't perform CSRF check on OCS routes with Bearer auth#8099
rullzer merged 2 commits into
stable13from
7873_13

Conversation

@rullzer

Copy link
Copy Markdown
Member

Backport of #7873

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Please wait until 13.0.0 is out ⚠️

@rullzer

Copy link
Copy Markdown
MemberAuthor

Review time! we can merge 13.0.1 stuff!

* @param bool $exception
*/
public function testCsrfOcsController(Controller $controller, $hasOcsApiHeader, $exception) {
public function testCsrfOcsController(Controller $controller, bool $hasOcsApiHeader, bool $hasBearerAuth, bool $exception) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this breaks because stable13 needs to be testable on 5.6

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argument 2 passed to Test\AppFramework\Middleware\Security\SecurityMiddlewareTest::testCsrfOcsController() must be an instance of Test\AppFramework\Middleware\Security\bool, boolean given

->method('getHeader')
->with('OCS-APIREQUEST')
->willReturn($hasOcsApiHeader ? 'true' : null);
->will(self::returnCallback(function ($header) use ($hasOcsApiHeader, $hasBearerAuth) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

willReturnMap ? ;)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't have a default I think

@codecov

codecovBot commented Feb 7, 2018

Copy link
Copy Markdown

Codecov Report

Merging #8099 into stable13 will increase coverage by <.01%.
The diff coverage is 60%.

@@ Coverage Diff @@## stable13 #8099 +/- ##
==============================================
+ Coverage 51.23% 51.23% +<.01% - Complexity 24990 24991 +1 
==============================================
Files 1608 1608 Lines 95106 95109 +3 Branches 1376 1376 ==============================================
+ Hits 48730 48733 +3 
Misses 46376 46376
Impacted FilesCoverage ΔComplexity Δ
...amework/Middleware/Security/SecurityMiddleware.php78.48% <60%> (-1.79%)26 <0> (+1)
.../dav/lib/Connector/Sabre/Exception/InvalidPath.php81.81% <0%> (ø)3% <0%> (ø)⬇️
apps/files_trashbin/lib/Trashbin.php72.53% <0%> (+0.24%)136% <0%> (ø)⬇️
lib/private/Files/Cache/Propagator.php96.2% <0%> (+1.26%)16% <0%> (ø)⬇️

Fixes#5694
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer

Copy link
Copy Markdown
MemberAuthor

Relevant tests pass

@rullzer
rullzer merged commit 2c6f3c8 into stable13Feb 28, 2018
@rullzer
rullzer deleted the 7873_13 branch February 28, 2018 13:34
@MorrisJobkeMorrisJobke mentioned this pull request Mar 9, 2018
10 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@rullzer@nickvergessen@MorrisJobke