Add new share type federated_group and provider for VO app #52992

smesterheide · 2025-05-20T14:18:18Z

Summary

The PR aims to add a new share type for remote groups on multiple federated NC instances.

Original PR: Add new share type for federated groups #34132

Changes

New NC shareType federated_group and corresponding OCM shareType federation
CloudFederationProvider are registered for a resourceType and shareType instead of a resourceType (file) alone.
Migration for share_external table
- The user column length is increased from 64 to 255 characters to allow long group names

TODO

...

Checklist

Code is properly formatted
Sign-off message is added to all commits
Tests (unit, integration, api and/or acceptance) are included
Screenshots before/after for front-end changes
Documentation (manuals or wiki) has been updated or is not required
Backports requested where applicable (ex: critical bugfixes)

Signed-off-by: Sandro Mesterheide <[email protected]> Add support for share type federated_group in files_sharing app Signed-off-by: Sandro Mesterheide <[email protected]> Add share type support for federation providers Signed-off-by: Sandro Mesterheide <[email protected]> Add migration for share_external table Signed-off-by: Sandro Mesterheide <[email protected]> fix currentUser reference php-cs-fixer psalm feedback Co-authored-by: Dirk Olbertz <[email protected]> Signed-off-by: Sandro Mesterheide <[email protected]>

miaulalala · 2025-05-21T14:53:08Z

Hi @smesterheide , thanks for opening this PR!

I unfortunately was unable compile the entire discussion surrounding the old PR, so I am curious about your use case.

From what I can tell, we already support federated group shares. Is there an issue with federated group shares that you're trying to address?

Thanks a lot!

github-actions · 2025-06-04T02:10:52Z

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

smesterheide · 2025-07-01T07:27:32Z

Hi @smesterheide , thanks for opening this PR!

I unfortunately was unable compile the entire discussion surrounding the old PR, so I am curious about your use case.

From what I can tell, we already support federated group shares. Is there an issue with federated group shares that you're trying to address?

Thanks a lot!

Hi @miaulalala this PR adds support for a new share type TYPE_FEDERATED_GROUP which is implemented by the VO federation app. The share type allows federated group shares with more than one remote server. The other changes facilitate the use of the new share type. Thanks for reaching out. I hope we can some traction. :)

lib/public/Federation/Exceptions/ProviderDoesNotExistsException.php

lib/public/Share/IShare.php

apps/files_sharing/lib/Migration/Version26000Date20230117143027.php

Signed-off-by: Sandro Mesterheide <[email protected]>

smesterheide · 2025-09-15T06:51:09Z

@susnux @skjnldsv Is there anything to improve this PR or make it easier for you to review?

susnux · 2025-09-15T08:51:08Z

apps/cloud_federation_api/lib/Controller/RequestHandlerController.php

+		if ($shareType === 'federation') {
+			// Allow creation of pending shares by federation provider
+		}


susnux

I am not sure that we want to integrate this into Nextclouds public server core for an app that is not accessible on our appstore ( https://apps.nextcloud.com/apps/vo_federation seems to be orphaned ) as we only did integrations like this for Nextcloud managed apps like Deck, Talk, and Teams (Circles). But this has to be decided by others.
cc @sorbaugh @AndyScherzinger

susnux · 2025-09-15T08:54:16Z

lib/public/Share/IShare.php

+	 * Extension to remote group share
+	 * @since 32.0.0
+	 */
+	public const TYPE_FEDERATED_GROUP = 14;


If this is for an external app then the name of the type should reflect this - if this is a general type such as per OCM and there are multiple apps that potentially implement this then its fine I guess

AndyScherzinger · 2025-09-15T11:25:30Z

I am not sure that we want to integrate this into Nextclouds public server core for an app that is not accessible on our appstore ( https://apps.nextcloud.com/apps/vo_federation seems to be orphaned ) as we only did integrations like this for Nextcloud managed apps like Deck, Talk, and Teams (Circles). But this has to be decided by others.
cc @sorbaugh @AndyScherzinger

@susnux we have to take over maintenance for both part app and core, hence we will ship the app in the future, hence also fix the publishing situation around the app store

provokateurin

Like the code says: Note to developers: Do not add new share types here.

It's impossible to say if this PR makes sense, if we don't have access to a recent working version vo_federation.

We also want to get rid of the share provider concept as it currently exists, as it is not very performant and also leads to a lot of copy-paste code (like we see in this PR).

If the mentioned proposal is going to be part of OCM, it should just be implemented in the existing cloud_federation_api app.

provokateurin · 2025-09-16T07:22:27Z

apps/files_sharing/lib/Migration/Version32000Date20230117143027.php

+		if (!$table->hasColumn('remote_share_type')) {
+			$table->addColumn('remote_share_type', Types::INTEGER, [
+				'notnull' => false,
+				'length' => 4,
+			]);
+			$changed = true;
+		}


The table already has a column share_type

provokateurin · 2025-09-16T07:22:52Z

core/img/actions/organization.svg

Should be an outlined icon as per our new icon guidelines

provokateurin · 2025-09-16T07:24:05Z

lib/private/Federation/CloudFederationShare.php

 		if ($shareType === 'group' || $shareType === IShare::TYPE_REMOTE_GROUP) {
 			$this->share['shareType'] = 'group';
+		} elseif ($shareType === 'federation' || $shareType === IShare::TYPE_FEDERATED_GROUP) {
+			// OCM proposel, currently only supported by Nextcloud


Suggested change

// OCM proposel, currently only supported by Nextcloud

// OCM proposal, currently only supported by Nextcloud

Also, where can we find this OCM proposal?

provokateurin · 2025-09-16T07:28:22Z

lib/public/Federation/Exceptions/ProviderDoesNotExistsException.php

Breaking change, not allowed

provokateurin · 2025-09-16T07:28:33Z

lib/public/Federation/ICloudFederationProviderManager.php

Also breaking changes

provokateurin · 2025-09-16T07:28:44Z

lib/public/Share/IShare.php


+	/**
+	 * Extension to remote group share
+	 * @since 32.0.0


Suggested change

* @since 32.0.0

* @since 33.0.0

smesterheide requested review from a team and nickvergessen as code owners May 20, 2025 14:18

smesterheide requested review from ArtificialOwl, artonge, leftybournes, nfebe and szaimen and removed request for a team May 20, 2025 14:18

smesterheide mentioned this pull request May 20, 2025

Add cloud id resolver interface #52993

Merged

5 tasks

szaimen removed their request for review May 21, 2025 12:37

smesterheide force-pushed the feat/vo-federation-features branch from 3a5b87e to bfac1be Compare May 21, 2025 13:29

github-actions bot added the feedback-requested label Jun 4, 2025

susnux reviewed Jul 16, 2025

View reviewed changes

lib/public/Federation/Exceptions/ProviderDoesNotExistsException.php Outdated Show resolved Hide resolved

susnux reviewed Jul 16, 2025

View reviewed changes

lib/public/Share/IShare.php Outdated Show resolved Hide resolved

susnux reviewed Jul 16, 2025

View reviewed changes

apps/files_sharing/lib/Migration/Version26000Date20230117143027.php Outdated Show resolved Hide resolved

fix: address versioning and localization issues

d4a1525

Signed-off-by: Sandro Mesterheide <[email protected]>

smesterheide force-pushed the feat/vo-federation-features branch from 2cd9ea5 to d4a1525 Compare August 11, 2025 08:19

skjnldsv added enhancement 2. developing Work in progress feature: sharing needs review Needs review to determine if still applicable or covered by other Issues labels Aug 19, 2025

susnux reviewed Sep 15, 2025

View reviewed changes

provokateurin requested changes Sep 16, 2025

View reviewed changes

	// OCM proposel, currently only supported by Nextcloud
	// OCM proposal, currently only supported by Nextcloud

Uh oh!

Add new share type federated_group and provider for VO app #52992

Are you sure you want to change the base?

Add new share type federated_group and provider for VO app #52992

Uh oh!

Conversation

smesterheide commented May 20, 2025

Summary

Changes

TODO

Checklist

Uh oh!

miaulalala commented May 21, 2025

Uh oh!

github-actions bot commented Jun 4, 2025

Uh oh!

smesterheide commented Jul 1, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smesterheide commented Sep 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

susnux left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndyScherzinger commented Sep 15, 2025

Uh oh!

provokateurin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants