Create federated share on mount #379

schiessle · 2016-07-12T10:30:13Z

this way the owner sees all mounted public links and control them individually.

This changes the work flow:

The user now adds his federated cloud ID instead of just the server URL to the "Add to your ownCloud" input field (using the server URL is still possible to maintain backward compatibility)
The ownCloud will send a federated share to the user given in the input field
The user will be re-directed to his ownCloud and will see a notification that he received a new federated share
This way the owner see who mounted his public link, can adjust permissions and remove single mounts or the public link without affecting the other users
keep old behavior as fall-back

mention-bot · 2016-07-12T10:30:15Z

@schiessle, thanks for your PR! By analyzing the annotation information on this pull request, we identified @icewind1991, @ErikPel and @blizzz to be potential reviewers

MorrisJobke · 2016-07-12T11:55:45Z

apps/federatedfilesharing/tests/Controller/SaveToOwnCloudControllerTest.php

+		$this->addressHandler = $this->getMockBuilder('OCA\FederatedFileSharing\AddressHandler')
+			->disableOriginalConstructor()->getMock();
+		$this->rootFolder = $this->getMock('OCP\Files\IRootFolder');
+		$this->userManager = $this->getMock('OCP\IUserManager');


cc @rullzer ;)

O yes... please use:

$this->getMockBuilder('...')->disableOriginalConstructor()->getMock()

Else there are a shitload of warnings in phpunit 5.4

ChristophWurst · 2016-07-12T20:39:02Z

apps/federatedfilesharing/appinfo/routes.php

+/**
+ * @author Björn Schießle <[email protected]>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.


because I started it at ownCloud, the pull request also exists in the core repository... But I should at least change my email address

schiessle · 2016-07-13T09:43:28Z

Ready to review, but please do me a favour and don't look to much at legacyMountPublicLink(). It is just a 1:1 copy of apps/files_sharing/ajax/external.php to keep backward compatibility. I don't want to rewrite this code, it worked until now and it should just continue to work for the next few months/years until we can remove it. 😃

nickvergessen · 2016-07-13T15:08:56Z

Tested file/folder share and could add it with link and cloud id,
Will look at the code now

nickvergessen · 2016-07-13T15:10:46Z

apps/federatedfilesharing/lib/Controller/SaveToNextcloudController.php

+use OCP\IUserSession;
+use OCP\Share\IManager;
+
+class SaveToNextcloudController extends Controller {


I'd suggest to avoid Nextcloud in class names?

Do you have a idea for a better name? "ConvertPubliLinkToFederatedShareController" maybe?

schiessle · 2016-07-14T09:06:47Z

@nickvergessen I renamed the "SaveToNextcloudController" to "MountPublicLinkController"... please have a final look. Thanks!

…loud this way the owner sees all mounted public links and control them individually

MorrisJobke · 2016-07-14T12:33:43Z

If the share fails to be created the loading spinner is still shown (even if you got an error message)

MorrisJobke · 2016-07-14T12:39:20Z

I would love to see an integration test for this. We cannot risk this feature to break.

I'm currently debugging, why it is not working over here.

MorrisJobke · 2016-07-14T12:42:52Z

entering a wrong URL: the spinner is still there but it should be an arrow again

LukasReschke · 2016-07-14T13:49:11Z

apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php

+		$httpClient = $this->clientService->newClient();
+
+		try {
+			$response = $httpClient->post($remote . '/index.php/apps/federatedfilesharing/createFederatedShare',


Needs some validation logic on $remote whether it's a domain or at least forbid slashes and : and @.

Otherwise somebody could send requests to any arbitrary URL, this is something one better wants to avoid :)

forbid slashes? people that have installed their cloud in a subfolde rwill love you?

don't understand this one. As said by @nickvergessen you need slashes if your Nextcloud is installed in a sub folder.

How do you want to decide which a address is pointing to a Nextcloud and which is not before you tried to send a federated share?

Hrm. True. What a pity. In this case 🙈 ;-)

…ted cloud id

schiessle · 2016-07-14T14:40:37Z

I'm currently debugging, why it is not working over here.

@MorrisJobke the spinner issue is fixed... anything else not working for you?

…res are forbidden

MorrisJobke · 2016-07-15T09:56:02Z

The spinner is still there:

MorrisJobke · 2016-07-15T09:56:28Z

Beside that it looks good and works 👍

MorrisJobke · 2016-07-15T09:57:03Z

The spinner is still there:

When I hit a second time enter in the input field the spinner is replaced by an arrow (but also if there needs to be a spinner).

schiessle · 2016-07-15T12:46:11Z

are you sure that you reloaded the web page and refreshed the browser cache? This is how it look for me:

When I hit a second time enter in the input field the spinner is replaced by an arrow (but also if there needs to be a spinner).

Yes, we just toggle the spinner on click.

LukasReschke · 2016-07-15T15:24:01Z

Went over the code with @schiessle. Code looks awesome. Let's do some more functional testing and then get this in 🚀 👍 🍻

MorrisJobke · 2016-07-18T07:22:42Z

Tested and works now 👍

schiessle added the 2. developing Work in progress label Jul 12, 2016

schiessle added this to the Nextcloud Next milestone Jul 12, 2016

MorrisJobke reviewed Jul 12, 2016
View reviewed changes

schiessle force-pushed the create_federated_share_on_mount branch 2 times, most recently from 2a77328 to 22dde3d Compare July 12, 2016 16:58

schiessle added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 12, 2016

ChristophWurst reviewed Jul 12, 2016
View reviewed changes

schiessle added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 13, 2016

schiessle force-pushed the create_federated_share_on_mount branch 2 times, most recently from aea08bc to ed56000 Compare July 13, 2016 09:34

schiessle added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 13, 2016

schiessle force-pushed the create_federated_share_on_mount branch 4 times, most recently from d73e463 to 3fa8d03 Compare July 13, 2016 11:19

schiessle added the feature: federation label Jul 13, 2016

schiessle force-pushed the create_federated_share_on_mount branch from 3fa8d03 to a1388cf Compare July 13, 2016 14:15

nickvergessen reviewed Jul 13, 2016
View reviewed changes

Björn Schießle added 2 commits July 14, 2016 11:22

create a real federated share if a user add a public link to his ownC…

f5d0b46

…loud this way the owner sees all mounted public links and control them individually

add unit tests

a0ae16a

LukasReschke reviewed Jul 14, 2016
View reviewed changes

schiessle force-pushed the create_federated_share_on_mount branch from 3c2c4d7 to 03f2e5e Compare July 14, 2016 14:32

schiessle added 7 commits July 14, 2016 16:39

continue to accept the URL of the remote server instead of the federa…

33a685b

…ted cloud id

s/ownCloud/Nextcloud

2f23054

move some stuff over to the federatedfilesharing app

5bbba49

allow to mount public link from Nextclouds <= 9

55b990b

remove unused parameter

dc53788

adjust copyright notices

56d3100

rename SaveToNextcloudController to MountPublicLinkController

c7f6461

schiessle force-pushed the create_federated_share_on_mount branch from 03f2e5e to c7f6461 Compare July 14, 2016 14:40

schiessle force-pushed the create_federated_share_on_mount branch from fccdc6f to b972ba1 Compare July 14, 2016 18:47

don't convert link share to federated share if outgoing federated sha…

f8a531c

…res are forbidden

schiessle force-pushed the create_federated_share_on_mount branch from b972ba1 to f8a531c Compare July 14, 2016 18:48

MorrisJobke merged commit 4032811 into master Jul 18, 2016

MorrisJobke deleted the create_federated_share_on_mount branch July 18, 2016 07:22

gisforgirard mentioned this pull request Feb 19, 2021

[Snyk] Upgrade v-tooltip from 2.0.3 to 2.1.2 gisforgirard/server#16

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create federated share on mount #379

Create federated share on mount #379

schiessle commented Jul 12, 2016 •

edited

Loading

mention-bot commented Jul 12, 2016

MorrisJobke Jul 12, 2016

rullzer Jul 12, 2016

ChristophWurst Jul 12, 2016

schiessle Jul 13, 2016

schiessle commented Jul 13, 2016

nickvergessen commented Jul 13, 2016

nickvergessen Jul 13, 2016

schiessle Jul 13, 2016

schiessle commented Jul 14, 2016

MorrisJobke commented Jul 14, 2016

MorrisJobke commented Jul 14, 2016

MorrisJobke commented Jul 14, 2016 •

edited by schiessle

Loading

LukasReschke Jul 14, 2016

LukasReschke Jul 14, 2016

nickvergessen Jul 14, 2016

schiessle Jul 14, 2016 •

edited

Loading

LukasReschke Jul 14, 2016

schiessle commented Jul 14, 2016

MorrisJobke commented Jul 15, 2016

MorrisJobke commented Jul 15, 2016

MorrisJobke commented Jul 15, 2016

schiessle commented Jul 15, 2016

LukasReschke commented Jul 15, 2016

MorrisJobke commented Jul 18, 2016

Create federated share on mount #379

Create federated share on mount #379

Conversation

schiessle commented Jul 12, 2016 • edited Loading

mention-bot commented Jul 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schiessle commented Jul 13, 2016

nickvergessen commented Jul 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schiessle commented Jul 14, 2016

MorrisJobke commented Jul 14, 2016

MorrisJobke commented Jul 14, 2016

MorrisJobke commented Jul 14, 2016 • edited by schiessle Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schiessle Jul 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schiessle commented Jul 14, 2016

MorrisJobke commented Jul 15, 2016

MorrisJobke commented Jul 15, 2016

MorrisJobke commented Jul 15, 2016

schiessle commented Jul 15, 2016

LukasReschke commented Jul 15, 2016

MorrisJobke commented Jul 18, 2016

schiessle commented Jul 12, 2016 •

edited

Loading

MorrisJobke commented Jul 14, 2016 •

edited by schiessle

Loading

schiessle Jul 14, 2016 •

edited

Loading