Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Properly verify and error on power-level mismatch in /batch_send endpoint #13229

Conversation

sumnerevans
Copy link
Contributor

@sumnerevans sumnerevans commented Jul 8, 2022

Closes #13216

Signed-off-by: Sumner Evans sumner@beeper.com

Outstanding Problem

In the test_batch_send_with_permission_msc2716_room_version test, I set the historical power level to 0 before calling the /batch_send endpoint: https://github.com/matrix-org/synapse/pull/13229/files#diff-7ec980d681db7426675aa46dfb312defa64ba0a6fa763f97eac107e9567c7e7fR438-R451

However, when the batch-send endpoint gets to the permission checks, it gets the power levels and historical is at 100, making the test fail.

Things I've done to debug include:

  • Ran self.helper.get_state again after the send_state to ensure that the power level is correct. This returned the expected value for historical of 0.
  • In get_named_levels, when the power levels event is retrieved, it has historical set to 100, which makes me think that maybe the auth_events being passed in are stale?

Clues I have:

  • When the insertion event is created using self.event_creation_handler.create_event, it gets initial_state_event_ids passed in. This doesn't include the power levels event.

Asked for spec clarification here: https://github.com/matrix-org/matrix-spec-proposals/pull/2716/files#r924472086

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

…orrectly

Signed-off-by: Sumner Evans <sumner@beeper.com>
…lly get persisted

Signed-off-by: Sumner Evans <sumner@beeper.com>
…upport MSC2716 power levels

Signed-off-by: Sumner Evans <sumner@beeper.com>
…send historical events in room version org.matrix.msc2716v3

Signed-off-by: Sumner Evans <sumner@beeper.com>
…historical events in room version org.matrix.msc2716v3

Signed-off-by: Sumner Evans <sumner@beeper.com>
Signed-off-by: Sumner Evans <sumner@beeper.com>
@sumnerevans sumnerevans force-pushed the batch-send-verify-power-levels branch from bf761c8 to 7d12a2f Compare July 21, 2022 18:29
@sumnerevans
Copy link
Contributor Author

Closing this due to the following reasons:

  • it's not entirely clear what should be happening here according to the spec, and I don't want to keep re-implementing this every time the spec changes.
  • Beeper is no longer going to be using Synapse's batch send endpoint as we are using an alternative homeserver implementation for bridges.

Anyone who is interested in implementing this functionality is welcome to use this as a starting point.

@MadLittleMods MadLittleMods mentioned this pull request Apr 12, 2023
27 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disallow /batch_send usage in a room where the room version doesn't support it or is not the room creator
1 participant