Fix federated group shares when no longer found in remote server #44587
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This will be needed to add tests with unavailable servers, as the local server is started by the script that runs the tests rather than from the test features themselves, so only the remote server can be stopped and started again. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
…server Once the receiver server is up again trying to access the federated share should cause it to be removed in the receiver server as well, as it will be no longer found in the sharer server. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
…server Once the sharer server is up again the federated share will be still there, as the sharer server missed the notification from the receiver server about deleting the share. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Although groups are removed after each scenario it seems that their remote group shares are not, so there might be dangling remote group shares that could interfere with the following scenarios. To solve that a new endpoint is added to the "testing" app to explicitly clear the remote shares. Note that any dangling remote storage was already removed with the OCC command, so only the "share_external" table is cleared. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
…remote When a member of the group that received a federated group share removes it the share is just removed for that specific user, but it does not affect other users in the group. Due to that the share is simply marked again as pending for that user. This even applies when the remote server is no longer reachable, as it is not possible to know if the server is just temporarily down or gone forever. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
…remote When the sharer removes a federated group share it is removed for all the members of the group that received it. Due to that, if the remote server was temporary down when the share was removed but once it is running again a user tries to access the share should be removed for all of them, rather than being marked as pending for that user (which is not the current behaviour and will need to be fixed in a follow up commit). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When a federated group share with another server is deleted a notification is sent to the remote server to delete the share. If that notification is not received then when the remote server tries to access the share again it will not be found in the sharer server, so it will be locally unshared in the remote server. This works fine for user shares, as in that case the share is deleted in the remote server, but locally unsharing a received federated group share only marks it as pending again for the user that tried to access it. Due to that the received federated group share is left dangling in the remote server, and it is kept forever in the list of pending shares for the users in the group. To solve that now the received federated group share is fully removed instead of simply unshared when no longer available in the sharer server. Note that all of the above applies only when the sharer server is still available and it returns a "not found" (or "forbidden") error when the remote server tries to access the share. If the sharer server is not available accessing the received share does not fully remove the group share, as in that case it is not possible to know if the sharer server is temporarily unavailable or gone forever. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This will be needed to test group shares with several users, as the external share managers returned by "createManagerForUser" use the group manager stored in the test case attribute. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
force-pushed
the
fix-federated-group-shares-when-no-longer-found-in-remote-server
branch
from
ba420d5 to
718560a
Compare
This was referenced Apr 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a federated group share with another server is deleted a notification is sent to the remote server to delete the share. If that notification is not received then when the remote server tries to access the share again it will not be found in the sharer server, so it will be locally unshared in the remote server.
This works fine for user shares, as in that case the share is deleted in the remote server, but locally unsharing a received federated group share only marks it as pending again for the user that tried to access it. Due to that the received federated group share is left dangling in the remote server, and it is kept forever in the list of pending shares for the users in the group.
To solve that now the received federated group share is fully removed instead of simply unshared when no longer available in the sharer server; if the share was a user share it will be locally unshared like before, but if it is a group share it will be fully deleted.
Note that all of the above applies only when the sharer server is still available and it returns a "not found" error when the remote server tries to access the share. If the sharer server is not available accessing the received share just fails, as in that case it is not possible to know if the sharer server is temporarily unavailable or gone forever.
Besides the fix this pull request also adds an integration test to verify the behaviour (Delete federated group share with another server when temporary unreachable) as well as several other integration tests for deletion scenarios. Unit tests were also added for
removeSharewith group shares.TODO / Open questions:
ForbiddenExceptionis received. I did that for consistency with the behaviour of user shares, but is that right or should the removal be limited toNotFoundException?removeSharewas reused by adding a boolean$forceparameter (which I am not a fan of). Should a different, explicit method be introduced instead?share_externaltable is explicitly cleared before running a scenario. But... could that be a bug in itself? Should the remote group shares be automatically removed if the group they belong to is removed?