Skip to content

Remove session token creation for dav requests during loadApps#31343

Merged
DeepDiver1975 merged 2 commits into
masterfrom
remove-dav-token-create
May 14, 2018
Merged

Remove session token creation for dav requests during loadApps#31343
DeepDiver1975 merged 2 commits into
masterfrom
remove-dav-token-create

Conversation

@tomneedham

@tomneedham tomneedham commented May 3, 2018

Copy link
Copy Markdown
Contributor

Description

There was a new code path added here: #28879 to create session tokens for dav requests. This is a partial revert. DAV requests use basic auth by default, which should be picked up here:

public function tryBasicAuthLogin(IRequest $request) {

and token creation here:

$this->createSessionToken($request, $this->getUser()->getUID(), $user, $password);

Which leaves this code path redundant. This can result in duplicate tokens because created causing UniqueViolation exceptions and confusing the client.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tomneedham tomneedham added this to the development milestone May 3, 2018
@tomneedham tomneedham self-assigned this May 3, 2018
@tomneedham tomneedham requested a review from IljaN May 3, 2018 10:30
@tomneedham

Copy link
Copy Markdown
Contributor Author

cc @SamuAlfageme

@codecov

codecov Bot commented May 3, 2018

Copy link
Copy Markdown

Codecov Report

Merging #31343 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #31343      +/-   ##
============================================
+ Coverage     62.58%    62.6%   +0.01%     
+ Complexity    18227    18220       -7     
============================================
  Files          1145     1145              
  Lines         68414    68397      -17     
  Branches       1234     1234              
============================================
- Hits          42820    42819       -1     
+ Misses        25233    25217      -16     
  Partials        361      361
Flag Coverage Δ Complexity Δ
#javascript 52.05% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 63.8% <ø> (+0.01%) 18220 <ø> (-7) ⬇️
Impacted Files Coverage Δ Complexity Δ
lib/private/legacy/app.php 64.53% <ø> (+2.11%) 180 <0> (-7) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update becb9f2...a4e2e7b. Read the comment docs.

@phil-davis

Copy link
Copy Markdown
Contributor

Backport stable10 #31428

@PVince81

Copy link
Copy Markdown
Contributor

Regression: #31731

@DeepDiver1975

Copy link
Copy Markdown
Member

Regression: #31731

👐

@PVince81 PVince81 modified the milestones: development, QA Jun 13, 2018
@lock

lock Bot commented Jul 30, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants