Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added parameter to disable "Add to your nextcloud" option in public shares #33715

Closed
wants to merge 1 commit into from

Conversation

pboguslawski
Copy link
Contributor

Added app files_sharing parameter showExternalShareOption to allow one to hide
"Add to your nextcloud" option in public shares (shown by default). Use

occ config:app:set files_sharing showExternalShareOption --value='no'

to hide and

occ config:app:set files_sharing showExternalShareOption --value='yes'

to restore.

Related: #12395
Author-Change-Id: IB#1123210
Signed-off-by: Pawel Boguslawski [email protected]

…hares

Added app `files_sharing` parameter `showExternalShareOption` to allow one to hide
"Add to your nextcloud" option in public shares (shown by default). Use
```
occ config:app:set files_sharing showExternalShareOption --value='no'
```
to hide and
```
occ config:app:set files_sharing showExternalShareOption --value='yes'
```
to restore.

Related: nextcloud#12395
Author-Change-Id: IB#1123210
Signed-off-by: Pawel Boguslawski <[email protected]>
@jancborchardt
Copy link
Member

@PVince81 @schiessle should this not simply be linked to the Federated Sharing options? When federated sharing is disabled, this should not be shown.

@schiessle
Copy link
Member

Yes, I would also expect that we hide it if outgoing federated shares are disabled on the server

@jancborchardt
Copy link
Member

@pboguslawski could you adjust your pull request so that the behavior is simply linked to the existing setting and doesn’t need another option? Thank you! :)

@pboguslawski
Copy link
Contributor Author

According to

https://docs.nextcloud.com/server/latest/user_manual/th/files/sharing.html#adding-a-public-share-to-your-nextcloud

Add to your nextcloud option allows one to add also external nc directories into local nc account; disabling local nc federation should probably be independent of dissallowing users to link to external nc directories, thats why separate param in this PR.

Disabling local nc federation may also disable adding external directories but this should be considered as separate PR if required.

If you still doesn't see point in separate param - please suggest existing parameter that should be used.

@jancborchardt
Copy link
Member

@schiessle what is the technical name of the existing param that should be used? I'd still say that yes, it should be the same setting.

@blizzz blizzz added this to the Nextcloud 29 milestone Nov 23, 2023
@jeayalar
Copy link

Added app files_sharing parameter showExternalShareOption to allow one to hide "Add to your nextcloud" option in public shares (shown by default). Use

occ config:app:set files_sharing showExternalShareOption --value='no'

to hide and

occ config:app:set files_sharing showExternalShareOption --value='yes'

to restore.

Related: #12395 Author-Change-Id: IB#1123210 Signed-off-by: Pawel Boguslawski [email protected]

The occ command previously shared to hide the "Add to your nextcloud" option, does not seem to work in NC 27.1.4. At least I couldn't manage to hide it using:

occ config:app:set files_sharing showExternalShareOption --value='no'

@jancborchardt
Copy link
Member

@AndyScherzinger this needs input/assessment from an engineer familiar with the matter. I gave my design feedback above. :)

@AndyScherzinger
Copy link
Member

@sorbaugh is back, so rather let him handle it than me 😃 🙏

This was referenced Mar 12, 2024
@skjnldsv
Copy link
Member

Add to your nextcloud option allows one to add also external nc directories into local nc account; disabling local nc federation should probably be independent of dissallowing users to link to external nc directories, thats why separate param in this PR.

Disabling local nc federation may also disable adding external directories but this should be considered as separate PR if required.

I understand the point, I'd say considering those options are pretty related, and for the sake of user experience and simplicity, we should keep the same setting :)

Do you mind rebasing your PR and using the isOutgoingServer2serverShareEnabled method?

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 15, 2024
This was referenced Mar 18, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@pboguslawski
Copy link
Contributor Author

Do you mind rebasing your PR and using the isOutgoingServer2serverShareEnabled method?

Isn't files_sharing.outgoing_server2server_share_enabled = yes required for public webdav shares to work? Disallowing external shares mounting locally should not block using public webdav in opposite direction so using isOutgoingServer2serverShareEnabled method is not good solution here.

This was referenced Jul 30, 2024
@Altahrim Altahrim mentioned this pull request Aug 5, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 6, 2024
@skjnldsv skjnldsv added this to the Nextcloud 31 milestone Aug 6, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 31 milestone Aug 14, 2024
@skjnldsv
Copy link
Member

Do you mind rebasing your PR and using the isOutgoingServer2serverShareEnabled method?

Isn't files_sharing.outgoing_server2server_share_enabled = yes required for public webdav shares to work? Disallowing external shares mounting locally should not block using public webdav in opposite direction so using isOutgoingServer2serverShareEnabled method is not good solution here.

Not anymore!
We should edit the setting label btw. Since nc 28, you can disable federated sharing but still have link shares.

So this issue is now resolved 🎉
Thank you nonetheless for the PR @pboguslawski, that was an interesting discussion and I'm glad it's solved now

Sign up for free to 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.

8 participants