Skip to content

[stable10] User IUserSession in Share20OcsController ctor#34696

Closed
phil-davis wants to merge 3 commits into
stable10from
stable10-handle-accept-decline-share
Closed

[stable10] User IUserSession in Share20OcsController ctor#34696
phil-davis wants to merge 3 commits into
stable10from
stable10-handle-accept-decline-share

Conversation

@phil-davis

@phil-davis phil-davis commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

Backport #34622

Note: there were various conflicts to resolve when backporting this. Many of them were just in lines that replaced currentUser with userSession->getUser() because lines at or around those points also had older code in stable10 with stuff like \OCP\Share::STATE_ACCEPTED

Points of note were:
apps/files_sharing/lib/AppInfo/Application.php had code changes in a block that does not exist in stable10 so that file is not backported.

apps/files_sharing/tests/Controller/Share20OcsControllerTest.php backports to apps/files_sharing/tests/API/Share20OcsTest.php (git knew that!)

tests/acceptance/features/apiAuth/ocsPUTAuth.feature already had the "new" test scenario examples in stable10 (hmm, why was that?)

@phil-davis phil-davis self-assigned this Mar 6, 2019
@codecov

codecov Bot commented Mar 6, 2019

Copy link
Copy Markdown

Codecov Report

Merging #34696 into stable10 will decrease coverage by 18.97%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             stable10   #34696       +/-   ##
===============================================
- Coverage          64%   45.03%   -18.98%     
===============================================
  Files            1276      116     -1160     
  Lines           75801    11465    -64336     
  Branches         1291     1291               
===============================================
- Hits            48515     5163    -43352     
+ Misses          26907     5923    -20984     
  Partials          379      379
Flag Coverage Δ Complexity Δ
#javascript 53.22% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 30.64% <ø> (-34.51%) 0 <ø> (-19247)
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 59.45% <0%> (-21.64%) 0% <0%> (ø)
apps/updatenotification/templates/admin.php
lib/private/Encryption/Keys/Storage.php
lib/private/App/CodeChecker/NodeVisitor.php
lib/private/RedisFactory.php
apps/dav/lib/Avatars/AvatarNode.php
...s/dav/appinfo/Migrations/Version20170202213905.php
apps/dav/lib/Upload/ChunkLocationProvider.php
apps/files/lib/AppInfo/Application.php
apps/systemtags/list.php
... and 1147 more

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 eb2fbcb...fd1057c. Read the comment docs.

@codecov

codecov Bot commented Mar 6, 2019

Copy link
Copy Markdown

Codecov Report

Merging #34696 into stable10 will decrease coverage by 1.16%.
The diff coverage is 91.91%.

Impacted file tree graph

@@              Coverage Diff               @@
##             stable10   #34696      +/-   ##
==============================================
- Coverage       65.17%      64%   -1.17%     
- Complexity      19244    19256      +12     
==============================================
  Files            1212     1276      +64     
  Lines           68476    75834    +7358     
  Branches            0     1291    +1291     
==============================================
+ Hits            44627    48540    +3913     
- Misses          23849    26915    +3066     
- Partials            0      379     +379
Flag Coverage Δ Complexity Δ
#javascript 53.22% <ø> (?) 0 <ø> (?)
#phpunit 65.15% <91.91%> (-0.02%) 19256 <1> (+12)
Impacted Files Coverage Δ Complexity Δ
...haring/lib/Controller/RequestHandlerController.php 82.18% <100%> (+2.31%) 30 <0> (+4) ⬆️
apps/files_sharing/lib/API/Share20OCS.php 93.48% <100%> (ø) 184 <0> (ø) ⬇️
...amework/Middleware/Security/SecurityMiddleware.php 93.84% <25%> (-4.52%) 24 <0> (+2)
lib/public/AppFramework/OCSController.php 95.34% <50%> (-2.22%) 13 <0> (+1)
apps/dav/lib/Connector/Sabre/CorsPlugin.php 89.79% <73.33%> (-7.83%) 19 <1> (+2)
core/register_command.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
core/js/oc-dialogs.js 2.66% <0%> (ø) 0% <0%> (?)
apps/comments/js/commentmodel.js 100% <0%> (ø) 0% <0%> (?)
settings/js/admin-apps.js 6.46% <0%> (ø) 0% <0%> (?)
core/js/config.js 3.33% <0%> (ø) 0% <0%> (?)
... and 60 more

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 076d734...3a039f8. Read the comment docs.

@DeepDiver1975 DeepDiver1975 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.

@DeepDiver1975

Copy link
Copy Markdown
Member

maybe we should backport #33180 as well?

@PVince81

@phil-davis

Copy link
Copy Markdown
Contributor Author

ocsPUTAuth.feature seems to be missing - https://github.com/owncloud/core/pull/34622/files#diff-960259729d5ea049d7dccedc7accf2e9R14

In stable10 the test scenarios were already there and correct. When I back-ported there were no changes to make.
https://github.com/owncloud/core/blob/stable10/tests/acceptance/features/apiAuth/ocsPUTAuth.feature

@DeepDiver1975

Copy link
Copy Markdown
Member

maybe we should backport #33180 as well?

or remove these changes to aa9f637#diff-910e48efa188d7af32364e0de3db5a2b - they are not necessary because the IUser in ctor issue only allies to controllers ....

@phil-davis

Copy link
Copy Markdown
Contributor Author

The backport was not easy. So I have probably made some errors, or stuff needs to be implemented differently in stable10. I am waiting for drone to be functional again, and restart CI for this to see what it reports.

Or @DeepDiver1975 do you want to start from the beginning and do the cherry-pick backport yourself. Then you will see the conflicts and maybe resolve them much more easily than me.

@phil-davis phil-davis force-pushed the stable10-handle-accept-decline-share branch 2 times, most recently from 4ae4c1f to 370e159 Compare March 8, 2019 15:56
DeepDiver1975 and others added 3 commits March 10, 2019 08:27
Properly handle unauthorised response in OCSController implementations

401/Unauthorized in DAV responses requires CORS headers as well

Handle accept and decline of invalid share id

Unit tests for accept and decline of invalid share id

Adjust unauthorized tests for ocs sharing api
@phil-davis phil-davis force-pushed the stable10-handle-accept-decline-share branch from 370e159 to 3a039f8 Compare March 10, 2019 02:43
@DeepDiver1975

Copy link
Copy Markdown
Member

let's see how far we get: #34786

@phil-davis

Copy link
Copy Markdown
Contributor Author

Closing in favor of PR #34786 - it sorts this out and passes CI

@phil-davis phil-davis closed this Mar 18, 2019
@phil-davis phil-davis deleted the stable10-handle-accept-decline-share branch March 18, 2019 10:15
@lock lock Bot locked as resolved and limited conversation to collaborators Mar 17, 2020
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.

3 participants