Skip to content

Bugfix/user in share20ocscontroller#34568

Closed
DeepDiver1975 wants to merge 3 commits into
masterfrom
bugfix/user-in-share20ocscontroller
Closed

Bugfix/user in share20ocscontroller#34568
DeepDiver1975 wants to merge 3 commits into
masterfrom
bugfix/user-in-share20ocscontroller

Conversation

@DeepDiver1975

@DeepDiver1975 DeepDiver1975 commented Feb 20, 2019

Copy link
Copy Markdown
Member

Description

  1. Share20OCSController did not use IUserSession which causes issues:
[Tue Feb 19 17:19:23 2019] TypeError: Argument 8 passed to OCA\Files_Sharing\Controller\Share20OcsController::__construct() must implement interface OCP\IUser, null given, called in /home/deepdiver/Development/owncloud/core/apps/files_sharing/lib/AppInfo/Application.php on line 94 at /home/deepdiver/Development/owncloud/core/apps/files_sharing/lib/Controller/Share20OcsController.php#83
  1. Unauthorized responses are incorrect.
    Before:
$ curl 'http://deepdiver:8080/ocs/v1.php/apps/files_sharing/api/v1/shares' -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.109 Safari/537.36' -H 'Referer: http://localhost:9876/debug.html' -H 'Origin: http://localhost:9876' -H 'authorization: Basic YWRtaW46YWRtaW4xNTUwNTkzMDMyNTU0' -H 'OCS-APIREQUEST: true' -H 'content-type: application/x-www-form-urlencoded' --data 'shareType=3&path=%2F%E6%96%8v
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Expire in 200 ms for 4 (transfer 0x55a7d5b22810)
* Connected to deepdiver (127.0.0.1) port 8080 (#0)
> POST /ocs/v1.php/apps/files_sharing/api/v1/shares HTTP/1.1
> Host: deepdiver:8080
> Accept: */*
> Accept-Encoding: deflate, gzip
> User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.109 Safari/537.36
> Referer: http://localhost:9876/debug.html
> Origin: http://localhost:9876
> authorization: Basic YWRtaW46YWRtaW4xNTUwNTkzMDMyNTU0
> OCS-APIREQUEST: true
> content-type: application/x-www-form-urlencoded
> Content-Length: 55
> 
* upload completely sent off: 55 out of 55 bytes
< HTTP/1.1 401 Unauthorized
< Host: deepdiver:8080
< Date: Wed, 20 Feb 2019 08:09:47 +0000
< Connection: close
< X-Powered-By: PHP/7.2.15-1+0~20190209065005.16+buster~1.gbp3ad8c0
< Expires: Thu, 19 Nov 1981 08:52:00 GMT
< Pragma: no-cache
< Set-Cookie: .....
< X-XSS-Protection: 1; mode=block
< X-Content-Type-Options: nosniff
< X-Frame-Options: SAMEORIGIN
< X-Robots-Tag: none
< X-Download-Options: noopen
< X-Permitted-Cross-Domain-Policies: none
< Set-Cookie: ocrhbj031xi4=6k31nopbargntflcbrlsjqsah8; path=/; HttpOnly
< Cache-Control: no-cache, must-revalidate
< Content-Type: application/json; charset=utf-8
< Content-Security-Policy: default-src 'none';manifest-src 'self';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self';connect-src 'self';media-src 'self'
< Content-Length: 43
< 
* Closing connection 0
{"message":"Current user is not logged in"}

After:

$ curl 'http://deepdiver:8080/ocs/v1.php/apps/files_sharing/api/v1/shares' -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.109 Safari/537.36' -H 'Referer: http://localhost:9876/debug.html' -H 'Origin: http://localhost:9876' -H 'authorization: Basic YWRtaW46YWRtaW4xNTUwNTkzMDMyNTU0' -H 'OCS-APIREQUEST: true' -H 'content-type: application/x-www-form-urlencoded' --data 'shareType=3&path=%2F%E6%96%87%E4%BB%B61550593032554.txt' --compressed -v
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Expire in 200 ms for 4 (transfer 0x55df41ea1810)
* Connected to deepdiver (127.0.0.1) port 8080 (#0)
> POST /ocs/v1.php/apps/files_sharing/api/v1/shares HTTP/1.1
> Host: deepdiver:8080
> Accept: */*
> Accept-Encoding: deflate, gzip
> User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.109 Safari/537.36
> Referer: http://localhost:9876/debug.html
> Origin: http://localhost:9876
> authorization: Basic YWRtaW46YWRtaW4xNTUwNTkzMDMyNTU0
> OCS-APIREQUEST: true
> content-type: application/x-www-form-urlencoded
> Content-Length: 55
> 
* upload completely sent off: 55 out of 55 bytes
< HTTP/1.1 200 OK
< Host: deepdiver:8080
< Date: Wed, 20 Feb 2019 08:20:12 +0000
< Connection: close
< X-Powered-By: PHP/7.2.15-1+0~20190209065005.16+buster~1.gbp3ad8c0
< Expires: Thu, 19 Nov 1981 08:52:00 GMT
< Pragma: no-cache
< Set-Cookie: ......
< X-XSS-Protection: 1; mode=block
< X-Content-Type-Options: nosniff
< X-Frame-Options: SAMEORIGIN
< X-Robots-Tag: none
< X-Download-Options: noopen
< X-Permitted-Cross-Domain-Policies: none
< Set-Cookie: ocrhbj031xi4=judinpdhs9le7ht61ncf79vjd9; path=/; HttpOnly
< Cache-Control: no-cache, must-revalidate
< Content-Type: application/xml; charset=utf-8
< Content-Security-Policy: default-src 'none';manifest-src 'self';script-src 'self' 'unsafe-eval';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self';connect-src 'self';media-src 'self'
< Content-Length: 213
< 
<?xml version="1.0"?>
<ocs>
 <meta>
  <status>failure</status>
  <statuscode>997</statuscode>
  <message>Unauthorised</message>
  <totalitems></totalitems>
  <itemsperpage></itemsperpage>
 </meta>
 <data/>
</ocs>
* Closing connection 0

Related Issue

Motivation and Context

Don't break behavior!

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@PVince81 PVince81 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@PVince81

Copy link
Copy Markdown
Contributor

php build/OCPSinceChecker.php
Parsing all files in lib/public for the presence of @since or @deprecated on each method...

PHP Warning:  count(): Parameter must be an array or an object that implements Countable in /drone/src/build/OCPSinceChecker.php on line 89
PHPDoc is needed for OCP\AppFramework\OCSController::handleException

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/user-in-share20ocscontroller branch from fbf8f28 to 2b6a07d Compare February 21, 2019 10:29
@codecov

codecov Bot commented Feb 21, 2019

Copy link
Copy Markdown

Codecov Report

Merging #34568 into master will decrease coverage by 16.72%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #34568       +/-   ##
=============================================
- Coverage     65.16%   48.43%   -16.73%     
=============================================
  Files          1200      109     -1091     
  Lines         69684    10504    -59180     
  Branches       1283     1283               
=============================================
- Hits          45407     5088    -40319     
+ Misses        23903     5042    -18861     
  Partials        374      374
Flag Coverage Δ Complexity Δ
#javascript 53.1% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 38.17% <ø> (-28.38%) 0 <ø> (-18364)
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 1078 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 217e803...2b6a07d. Read the comment docs.

@codecov

codecov Bot commented Feb 21, 2019

Copy link
Copy Markdown

Codecov Report

Merging #34568 into master will decrease coverage by <.01%.
The diff coverage is 86.58%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #34568      +/-   ##
============================================
- Coverage     65.22%   65.22%   -0.01%     
- Complexity    18434    18438       +4     
============================================
  Files          1203     1203              
  Lines         69816    69827      +11     
  Branches       1280     1280              
============================================
+ Hits          45540    45544       +4     
- Misses        23904    23911       +7     
  Partials        372      372
Flag Coverage Δ Complexity Δ
#javascript 53.05% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 66.62% <86.58%> (-0.01%) 18438 <1> (+4)
Impacted Files Coverage Δ Complexity Δ
...e/AppFramework/DependencyInjection/DIContainer.php 73.28% <ø> (ø) 79 <0> (ø) ⬇️
apps/files_sharing/lib/AppInfo/Application.php 50.98% <100%> (ø) 17 <0> (ø) ⬇️
...amework/Middleware/Security/SecurityMiddleware.php 93.84% <25%> (-4.52%) 24 <0> (+2)
apps/dav/lib/Connector/Sabre/CorsPlugin.php 89.79% <73.33%> (-7.83%) 19 <1> (+2)
...es_sharing/lib/Controller/Share20OcsController.php 85.31% <93.54%> (ø) 193 <0> (ø) ⬇️

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 1517b62...2418390. Read the comment docs.

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/user-in-share20ocscontroller branch from 2b6a07d to 56b135b Compare February 21, 2019 13:13
@phil-davis

Copy link
Copy Markdown
Contributor

@DeepDiver1975 All the failing federation scenarios are when something is shared on the "local" server and the receiving user on the "remote" federated server does something in the share that they have received (uploads a file... to the share). The action does not seem to be effective - i.e. the result is not seen on the "local" server.

In the drone setup, the "local" server is running the code of the PR branch. The "remote" federated server is running the code in the last QA tarball (i.e. recent code, but not the PR). That helps a little bit to know that the a new change will work in a federated environment with old versions.

Perhaps the PR code is causing some change in the way that incoming "old style" requests from the "remote" server are not working/valid on the "new style" local server. Or perhaps there is some issue with the test runner code (which matches the new PR) correctly accessing the API on the "old" remote server.

Post if you make progress. I will look locally when I get to the office in the morning.

@phil-davis

Copy link
Copy Markdown
Contributor

Rebase after PRs #34577 and #34582 are merged to master. That will provide nicer handling of the federation scenarios.

The issue seems to be around:

[Fri Feb 22 12:55:55 2019] 192.168.1.12:42184 [201]: /index.php/apps/federatedfilesharing/shares
[Fri Feb 22 12:55:55 2019] 192.168.1.12:42186 [200]: /ocs/v1.php/apps/files_sharing/api/v1/remote_shares/pending
[Fri Feb 22 12:55:55 2019] GuzzleHttp\Exception\ServerException: Server error response [url] http://172.17.0.1:8080/ocs/v2.php/cloud/shares/84/accept?format=json [status code] 500 [reason phrase] Internal Server Error at /home/phil/federated/core/lib/composer/guzzlehttp/guzzle/src/Exception/RequestException.php#89
[Fri Feb 22 12:55:55 2019] 192.168.1.12:42188 [200]: /ocs/v1.php/apps/files_sharing/api/v1/remote_shares/pending/77
[Fri Feb 22 12:55:56 2019] 192.168.1.12:42206 [200]: /ocs/v2.php/apps/testing/api/v1/lockprovisioning
[Fri Feb 22 12:55:56 2019] 192.168.1.12:42210 [200]: /ocs/v1.php/apps/testing/api/v1/apps
[Fri Feb 22 12:55:57 2019] Exception: {"Exception":"GuzzleHttp\\Exception\\ServerException","Message":"Server error response [url] http:\/\/172.17.0.1:8080\/ocs\/v2.php\/cloud\/shares\/69\/decline?format=json [status code] 500 [reason phrase] Internal Server Error","Code":500,"Trace":"#0 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/Subscriber\/HttpError.php(32): GuzzleHttp\\Exception\\RequestException::create(Object(GuzzleHttp\\Message\\Request), Object(GuzzleHttp\\Message\\Response))\n#1 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/Event\/Emitter.php(108): GuzzleHttp\\Subscriber\\HttpError->onComplete(Object(GuzzleHttp\\Event\\CompleteEvent), 'complete')\n#2 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/RequestFsm.php(91): GuzzleHttp\\Event\\Emitter->emit('complete', Object(GuzzleHttp\\Event\\CompleteEvent))\n#3 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/RequestFsm.php(132): GuzzleHttp\\RequestFsm->__invoke(Object(GuzzleHttp\\Transaction))\n#4 \/home\/phil\/federated\/core\/lib\/composer\/react\/promise\/src\/FulfilledPromise.php(25): GuzzleHttp\\RequestFsm->GuzzleHttp\\{closure}(Array)\n#5 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/ringphp\/src\/Future\/CompletedFutureValue.php(55): React\\Promise\\FulfilledPromise->then(Object(Closure), NULL, NULL)\n#6 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/Message\/FutureResponse.php(43): GuzzleHttp\\Ring\\Future\\CompletedFutureValue->then(Object(Closure), NULL, NULL)\n#7 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/RequestFsm.php(134): GuzzleHttp\\Message\\FutureResponse::proxy(Object(GuzzleHttp\\Ring\\Future\\CompletedFutureArray), Object(Closure))\n#8 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/Client.php(165): GuzzleHttp\\RequestFsm->__invoke(Object(GuzzleHttp\\Transaction))\n#9 \/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/Client.php(150): GuzzleHttp\\Client->send(Object(GuzzleHttp\\Message\\Request))\n#10 \/home\/phil\/federated\/core\/lib\/private\/Http\/Client\/Client.php(202): GuzzleHttp\\Client->post('http:\/\/172.17.0...', Array)\n#11 \/home\/phil\/federated\/core\/apps\/federatedfilesharing\/lib\/Notifications.php(346): OC\\Http\\Client\\Client->post('http:\/\/172.17.0...', Array)\n#12 \/home\/phil\/federated\/core\/apps\/federatedfilesharing\/lib\/Notifications.php(277): OCA\\FederatedFileSharing\\Notifications->tryHttpPostToShareEndpoint('172.17.0.1:8080', '\/69\/decline', Array)\n#13 \/home\/phil\/federated\/core\/apps\/federatedfilesharing\/lib\/Notifications.php(238): OCA\\FederatedFileSharing\\Notifications->sendUpdateToRemote('http:\/\/172.17.0...', 69, 'uSLaGrYmBeeIQOo', 'decline')\n#14 \/home\/phil\/federated\/core\/apps\/federatedfilesharing\/lib\/AppInfo\/Application.php(242): OCA\\FederatedFileSharing\\Notifications->sendDeclineShare('http:\/\/172.17.0...', 69, 'uSLaGrYmBeeIQOo')\n#15 \/home\/phil\/federated\/core\/lib\/composer\/symfony\/event-dispatcher\/EventDispatcher.php(212): OCA\\FederatedFileSharing\\AppInfo\\Application->OCA\\FederatedFileSharing\\AppInfo\\{closure}(Object(OCP\\Share\\Events\\DeclineShare), 'OCP\\\\Share\\\\Event...', Object(Symfony\\Component\\EventDispatcher\\EventDispatcher))\n#16 \/home\/phil\/federated\/core\/lib\/composer\/symfony\/event-dispatcher\/EventDispatcher.php(44): Symfony\\Component\\EventDispatcher\\EventDispatcher->doDispatch(Array, 'OCP\\\\Share\\\\Event...', Object(OCP\\Share\\Events\\DeclineShare))\n#17 \/home\/phil\/federated\/core\/apps\/files_sharing\/lib\/External\/Manager.php(389): Symfony\\Component\\EventDispatcher\\EventDispatcher->dispatch('OCP\\\\Share\\\\Event...', Object(OCP\\Share\\Events\\DeclineShare))\n#18 \/home\/phil\/federated\/core\/apps\/files_sharing\/lib\/Hooks.php(86): OCA\\Files_Sharing\\External\\Manager->removeUserShares('user0')\n#19 \/home\/phil\/federated\/core\/lib\/private\/legacy\/hook.php(105): OCA\\Files_Sharing\\Hooks::deleteUser(Array)\n#20 \/home\/phil\/federated\/core\/lib\/private\/Server.php(344): OC_Hook::emit('OC_User', 'post_deleteUser', Array)\n#21 \/home\/phil\/federated\/core\/lib\/private\/Hooks\/EmitterTrait.php(99): OC\\Server->OC\\{closure}(Object(OC\\User\\User))\n#22 \/home\/phil\/federated\/core\/lib\/private\/Hooks\/PublicEmitter.php(33): OC\\Hooks\\BasicEmitter->emit('\\\\OC\\\\User', 'postDelete', Array)\n#23 \/home\/phil\/federated\/core\/lib\/private\/User\/User.php(279): OC\\Hooks\\PublicEmitter->emit('\\\\OC\\\\User', 'postDelete', Array)\n#24 \/home\/phil\/federated\/core\/apps\/provisioning_api\/lib\/Users.php(350): OC\\User\\User->delete()\n#25 \/home\/phil\/federated\/core\/lib\/private\/legacy\/api.php(148): OCA\\Provisioning_API\\Users->deleteUser(Array)\n#26 \/home\/phil\/federated\/core\/lib\/private\/Route\/Router.php(342): OC_API::call(Array)\n#27 \/home\/phil\/federated\/core\/ocs\/v1.php(64): OC\\Route\\Router->match('\/ocs\/cloud\/user...')\n#28 {main}","File":"\/home\/phil\/federated\/core\/lib\/composer\/guzzlehttp\/guzzle\/src\/Exception\/RequestException.php","Line":89}

When the remote federated server gets an API request to accept a pending remote share, it sends off to the local server. The local server gets a 500, but the remote server does not pass that on. The remote server responds t its API request with an HTTP 200 an no OCS status.

DeepDiver1975 added a commit to owncloud/owncloud-sdk that referenced this pull request Feb 22, 2019
@DeepDiver1975

Copy link
Copy Markdown
Member Author

When the remote federated server gets an API request to accept a pending remote share, it sends off to the local server. The local server gets a 500, but the remote server does not pass that on. The remote server responds t its API request with an HTTP 200 an no OCS status.

@phil-davis Can I ask you to help out here to find the log messages for the 500 status code? I'm a bit lost 😕

@DeepDiver1975 DeepDiver1975 force-pushed the bugfix/user-in-share20ocscontroller branch from 0880f94 to 4e69c57 Compare February 22, 2019 14:17
DeepDiver1975 added a commit to owncloud/owncloud-sdk that referenced this pull request Feb 22, 2019
@phil-davis phil-davis self-assigned this Feb 23, 2019
@phil-davis

Copy link
Copy Markdown
Contributor

Assigned myself also so I notice this on Monday.

DeepDiver1975 added a commit to owncloud/owncloud-sdk that referenced this pull request Feb 25, 2019
@DeepDiver1975

Copy link
Copy Markdown
Member Author

Assigned myself also so I notice this on Monday.

@phil-davis any news? Is there anything I can do? THX

@phil-davis phil-davis force-pushed the bugfix/user-in-share20ocscontroller branch from 4e69c57 to 2418390 Compare February 25, 2019 12:27
@phil-davis

phil-davis commented Feb 25, 2019

Copy link
Copy Markdown
Contributor

Here is the next clue ;)

make test-acceptance-api BEHAT_FEATURE=tests/acceptance/features/apiFederation/federated.feature:104

After running this once, it passes but on the remote federated server:

mysql> select * from oc_share_external;
+----+------------------------+-----------+-----------------+----------+----------------+-------+-------+--------------------+----------------------------------+----------+
| id | remote                 | remote_id | share_token     | password | name           | owner | user  | mountpoint         | mountpoint_hash                  | accepted |
+----+------------------------+-----------+-----------------+----------+----------------+-------+-------+--------------------+----------------------------------+----------+
| 48 | http://172.17.0.1:8080 |        49 | vTdG3SOeXrc6zbw |          | /textfile0.txt | user1 | user0 | /textfile0 (2).txt | c4a6b97dcb0b8fe5825ea5945caea083 |        1 |
+----+------------------------+-----------+-----------------+----------+----------------+-------+-------+--------------------+----------------------------------+----------+
1 row in set (0.01 sec)

user0 is on the remote federated server. After the scenario, user0 has been deleted. But there is still an entry in oc_share_external

When I run the scenario again, it fails because the scenario creates a new federated share, and now there are 2 of them:

mysql> select * from oc_share_external;
+----+------------------------+-----------+-----------------+----------+----------------+-------+-------+--------------------+----------------------------------+----------+
| id | remote                 | remote_id | share_token     | password | name           | owner | user  | mountpoint         | mountpoint_hash                  | accepted |
+----+------------------------+-----------+-----------------+----------+----------------+-------+-------+--------------------+----------------------------------+----------+
| 48 | http://172.17.0.1:8080 |        49 | vTdG3SOeXrc6zbw |          | /textfile0.txt | user1 | user0 | /textfile0 (2).txt | c4a6b97dcb0b8fe5825ea5945caea083 |        1 |
| 49 | http://172.17.0.1:8080 |        50 | wxam6WvectQRmV2 |          | /textfile0.txt | user1 | user0 | /textfile0 (3).txt | 2e2a7796ce4db5c01643e66180c71d1e |        1 |
+----+------------------------+-----------+-----------------+----------+----------------+-------+-------+--------------------+----------------------------------+----------+
2 rows in set (0.00 sec)

The scenario tries to accept that stale external share entry and upload-overwrite /textfile0 (2).txt but that is an entry that no longer exists on the local server (because user1 on the local server was deleted and recreated...).

When checkout master and run the scenatio again, then oc_share_external table gets cleaned out, and everything is good.

I think the underlying problem is that oc_share_external is not being cleared when a user is deleted.

On the local server, when the scenario is finishing and deleting the users, I see:

[Mon Feb 25 20:56:45 2019] 10.49.210.15:36188 [500]: /ocs/v2.php/cloud/shares/49/decline?format=json

It seems that, as it is deleting a user, the federated server sends a decline for the remote share(s) that the user has been offered. And that decline fails, and the federated server keeps the old stale entries in its database.

When running the scenario from master, I see a successful decline processed on the local server:

[Mon Feb 25 21:17:22 2019] 10.49.210.15:37446 [200]: /ocs/v2.php/cloud/shares/50/decline?format=json

@phil-davis

Copy link
Copy Markdown
Contributor

Maybe another clue:

git checkout test/user-in-share20ocscontroller
curl http://admin:admin@172.17.0.1:8080/ocs/v2.php/cloud/shares/456/decline?format=json -X POST
{"ocs":{"meta":{"status":"failure","statuscode":500,"message":"Share with id 456 does not exist","totalitems":"","itemsperpage":""},"data":[]}}

But from master:

git checkout master
curl http://admin:admin@172.17.0.1:8080/ocs/v2.php/cloud/shares/456/decline?format=json -X POST
"response is empty"

The test scenario cleanup deletes the local users first, then the remote users. So the local user (and things that the local user shared) do not exist any more. Then the remote user is deleted, and the above kind of thing happens - that 500 status is returned when the share decline is done, and the remote system does not cope nicely with it.

@DeepDiver1975

Copy link
Copy Markdown
Member Author

git checkout master
curl http://admin:admin@172.17.0.1:8080/ocs/v2.php/cloud/shares/456/decline?format=json -X POST
"response is empty"

this is wrong - a proper OCS response has to be sent and has to be accepted.
This is the reason for this change.

What is the response on stable10? Mind checking this @phil-davis ? THX

@phil-davis

Copy link
Copy Markdown
Contributor

I tried stable10 and the release tags v10.1.0 v10.0.10 v10.0.1 - they all give an empty response.

@phil-davis

Copy link
Copy Markdown
Contributor

@DeepDiver1975 see PR #34622 that is on top of this. It makes the response be 410 STATUS_GONE. Then the tests pass, because the remote system copes better with a 4nn response, and seems to continue on with removing the knowledge of the remote share.

You might like that code, or might want to just return a 400 ?

@individual-it

individual-it commented Feb 27, 2019

Copy link
Copy Markdown
Member

is this related #34626 ?

@phil-davis phil-davis removed their assignment Feb 27, 2019
@individual-it

Copy link
Copy Markdown
Member

one more #34636

@PVince81

PVince81 commented Mar 1, 2019

Copy link
Copy Markdown
Contributor

@DeepDiver1975

Copy link
Copy Markdown
Member Author

closed in favor of #34622

@DeepDiver1975 DeepDiver1975 deleted the bugfix/user-in-share20ocscontroller branch March 5, 2019 07:26
@phil-davis

Copy link
Copy Markdown
Contributor

The code here will get backported as part of #34622

@lock lock Bot locked as resolved and limited conversation to collaborators Mar 10, 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.

4 participants