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

Implement rechecking of redactions for room versions v3 #4499

Merged
merged 15 commits into from
Jan 29, 2019

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Jan 28, 2019

This starts implementing support for V3 rooms, while keeping V3 disabled

We add the constant, but don't add it to the known room versions. This
lets us start adding V3 logic, but the servers will never join or create
V3 rooms
@erikjohnston erikjohnston changed the title [WIP] Implement rechecking of redactions for room versions v3 Implement rechecking of redactions for room versions v3 Jan 29, 2019
@erikjohnston erikjohnston requested a review from a team January 29, 2019 12:03
@codecov-io
Copy link

codecov-io commented Jan 29, 2019

Codecov Report

Merging #4499 into develop will decrease coverage by 0.03%.
The diff coverage is 52.94%.

@@             Coverage Diff             @@
##           develop    #4499      +/-   ##
===========================================
- Coverage    74.71%   74.67%   -0.04%     
===========================================
  Files          336      336              
  Lines        34258    34280      +22     
  Branches      5578     5585       +7     
===========================================
+ Hits         25595    25599       +4     
- Misses        7081     7099      +18     
  Partials      1582     1582

synapse/api/constants.py Outdated Show resolved Hide resolved
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erm: do we not need to check that the sender of a redaction matches the original sender when we pull the original event out of the database?

redacter_domain = get_domain_from_id(event.event_id)
redactee_domain = get_domain_from_id(event.redacts)
if redacter_domain == redactee_domain:
if room_version in (RoomVersions.V1, RoomVersions.V2, RoomVersions.VDH_TEST):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we just drop VDH_TEST ?

@@ -63,6 +63,9 @@ def get_send_on_behalf_of(self):
"""
return getattr(self, "send_on_behalf_of", None)

def need_to_check_redaction(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can has docstring please - what does this mean?

synapse/storage/events_worker.py Outdated Show resolved Hide resolved
@erikjohnston
Copy link
Member Author

erm: do we not need to check that the sender of a redaction matches the original sender when we pull the original event out of the database?

Argh, bollocks

@erikjohnston erikjohnston requested a review from a team January 29, 2019 22:18
@erikjohnston
Copy link
Member Author

sytest looks to be unhappy that i killed off VDH_TEST

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Starting in room v3 redaction events are accepted up front, and later
checked to see if the redacter and redactee's domains match.

If the sender of the redaction event is allowed to redact due to auth
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"is allowed to redact any event"

@erikjohnston erikjohnston merged commit e12313b into develop Jan 29, 2019
turt2live added a commit to matrix-org/matrix-spec-proposals that referenced this pull request Jan 31, 2019
Original proposal: #1659
Implementation proofs (some traversing of the PR tree may be required to get all of them):
* matrix-org/synapse#4483
* matrix-org/synapse#4499

This doesn't intentionally change anything from the proposal.

**Implementation details**:

The simple part of this is the introduction of a rooms/v3.html document. The somewhat unclear part is the stuff done to the s2s definitions. This pulls `unsigned_pdu` out to `unsigned_pdu_base` (all fields except `event_id`) where it can be reused in `pdu` and `pdu_v3` (for rooms v3). These definitions are further moved into the room version specifications where they can highlight the exact schemas in detail.

Version 1 has been updated to include the pre-existing event format, however the core principles of the room have not been changed. The same applies to room version 2. Room versions have immutable core principles once in the spec, otherwise these format changes would land in a pre-existing version.

The client-server API event formats will need updating, however that is being punted to a different commit to try and keep these changes reviewable.
@erikjohnston erikjohnston deleted the erikj/redactions_eiah branch March 5, 2019 13:52
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.

3 participants