fix(federation): Handle permission mismatches for reaction permissions#16948
fix(federation): Handle permission mismatches for reaction permissions#16948
Conversation
e1c9316 to
18dfa71
Compare
|
Hello there, 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.) |
|
Sorry, this some how slipped. |
This fixes issues when federated instances are at different versions and have different permission bits (e.g., one has REACT permission from migration, other doesn't yet). Changes: - Skip local permission checks for federated conversations in middleware, as the host server will validate permissions authoritatively when the request is proxied - Add permission healing on federated room join: sync local permissions with host's authoritative permissions to handle version mismatches Fixes: #16902 Signed-off-by: Florian Ludwig <florian.ludwig@uninow.de>
18dfa71 to
6e8b36a
Compare
|
|
||
| $room = $controller->getRoom(); | ||
|
|
||
| // For federated conversations, skip local permission checks. |
There was a problem hiding this comment.
This sounds a bit risky, while it might be true at the moment, it could be possible that in the future there would be local handling returning data.
I assume it's currently required for 34 servers that require reaction permissions when the federation host is 33 without it.
If that is the case we could simply add that to the react case for now:
if ($permission === RequirePermission::REACT && !($participant->getPermissions() & Attendee::PERMISSIONS_REACT)) {
// Allow reacting with chat permission for now, in case the host server does not have split permissions yet.
if (($participant->getPermissions() & Attendee::PERMISSIONS_CHAT) && $room instanceof Room && $room->isFederatedConversation()) {
return;
}
Summary
This PR fixes issues when federated instances are at different versions and have different permission bits (e.g., one instance has the new
REACTpermission from the migration, while the other doesn't yet).Problem
When the reaction permission feature is deployed across federated Nextcloud instances at different version levels:
Your instance (v34) updates before host (v33): Local permissions include
PERMISSIONS_REACTdue to migration, but host doesn't understand it. The middleware permission check fails locally even though the host would accept the request.Host updates after you: The host's migration adds REACT permission to federated attendees, but the remote participant's local copy doesn't get updated (migrations don't trigger federation notifications).
Host updates before you: Your migration might accidentally grant REACT permission that a moderator had intentionally restricted.
Solution
Skip local permission checks for federated conversations: Since requests are proxied to the host server anyway, the host validates permissions authoritatively. This prevents false negatives from version mismatches.
Add permission healing on federated room join: When a user joins a federated room, sync local permissions with the host's authoritative permissions. This ensures permissions stay in sync across version differences.
Test plan
Fixes: #16902