Skip to content

Move /send_leave to GMSL #387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Move /send_leave to GMSL #387

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
bc7260c
Add send_leave
S7evinK May 24, 2023
9e1377a
Add HandleSendLeave tests
S7evinK May 25, 2023
2640a7e
Update interface
S7evinK May 30, 2023
645785d
Remove dupe test
S7evinK May 30, 2023
b7a785d
Use version consts
S7evinK May 30, 2023
5d099f9
Merge branch 'main' of github.com:matrix-org/gomatrixserverlib into s…
S7evinK Jun 1, 2023
540062a
PR review comments, let IRoomVersion handle send leave
S7evinK Jun 2, 2023
1f82150
Revert "PR review comments, let IRoomVersion handle send leave"
S7evinK Nov 22, 2023
717c829
Merge branch 'main' of github.com:matrix-org/gomatrixserverlib into s…
S7evinK Nov 22, 2023
e3b02c9
Make tests/lint happy for now
S7evinK Nov 22, 2023
2369def
Some tweaks
S7evinK Nov 23, 2023
498c486
Fix oops
S7evinK Nov 23, 2023
185a49a
Move membership check further to the top, so we can avoid unneeded DB
S7evinK Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions eventversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gomatrixserverlib

import (
"context"
"fmt"

"github.com/matrix-org/gomatrixserverlib/spec"
Expand Down Expand Up @@ -30,6 +31,8 @@ type IRoomVersion interface {
NewEventFromUntrustedJSON(eventJSON []byte) (result PDU, err error)
NewEventBuilder() *EventBuilder
NewEventBuilderFromProtoEvent(pe *ProtoEvent) *EventBuilder

HandleSendLeave(ctx context.Context, event PDU, origin spec.ServerName, eventID, roomID string, querier CurrentStateQuerier, verifier JSONVerifier) (PDU, error)
}

// StateResAlgorithm refers to a version of the state resolution algorithm.
Expand Down Expand Up @@ -112,6 +115,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnocksForbidden,
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV2: RoomVersionImpl{
ver: RoomVersionV2,
Expand All @@ -126,6 +130,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnocksForbidden,
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV3: RoomVersionImpl{
ver: RoomVersionV3,
Expand All @@ -140,6 +145,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnocksForbidden,
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV4: RoomVersionImpl{
ver: RoomVersionV4,
Expand All @@ -154,6 +160,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnocksForbidden,
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV5: RoomVersionImpl{
ver: RoomVersionV5,
Expand All @@ -168,6 +175,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnocksForbidden,
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV6: RoomVersionImpl{
ver: RoomVersionV6,
Expand All @@ -182,6 +190,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnocksForbidden,
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV7: RoomVersionImpl{
ver: RoomVersionV7,
Expand All @@ -196,6 +205,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnockOnly,
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV8: RoomVersionImpl{
ver: RoomVersionV8,
Expand All @@ -210,6 +220,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnockOnly,
allowRestrictedJoinsInEventAuth: RestrictedOnly,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV9: RoomVersionImpl{
ver: RoomVersionV9,
Expand All @@ -224,6 +235,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnockOnly,
allowRestrictedJoinsInEventAuth: RestrictedOnly,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
RoomVersionV10: RoomVersionImpl{
ver: RoomVersionV10,
Expand All @@ -238,6 +250,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnockOrKnockRestricted,
allowRestrictedJoinsInEventAuth: RestrictedOrKnockRestricted,
requireIntegerPowerLevels: true,
handleSendLeaveFunc: handleSendLeave,
},
"org.matrix.msc3667": RoomVersionImpl{ // based on room version 7
ver: RoomVersion("org.matrix.msc3667"),
Expand All @@ -252,6 +265,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnockOnly,
allowRestrictedJoinsInEventAuth: NoRestrictedJoins,
requireIntegerPowerLevels: true,
handleSendLeaveFunc: handleSendLeave,
},
"org.matrix.msc3787": RoomVersionImpl{ // roughly, the union of v7 and v9
ver: RoomVersion("org.matrix.msc3787"),
Expand All @@ -266,6 +280,7 @@ var roomVersionMeta = map[RoomVersion]IRoomVersion{
allowKnockingInEventAuth: KnockOrKnockRestricted,
allowRestrictedJoinsInEventAuth: RestrictedOrKnockRestricted,
requireIntegerPowerLevels: false,
handleSendLeaveFunc: handleSendLeave,
},
}

Expand Down Expand Up @@ -334,6 +349,7 @@ type RoomVersionImpl struct {
powerLevelsIncludeNotifications bool
requireIntegerPowerLevels bool
stable bool
handleSendLeaveFunc func(ctx context.Context, event PDU, origin spec.ServerName, eventID, roomID string, querier CurrentStateQuerier, verifier JSONVerifier) (PDU, error)
}

func (v RoomVersionImpl) Version() RoomVersion {
Expand Down Expand Up @@ -431,6 +447,17 @@ func (v RoomVersionImpl) RedactEventJSON(eventJSON []byte) ([]byte, error) {
return v.redactionAlgorithm(eventJSON)
}

// HandleSendLeave handles requests to `/send_leave`
func (v RoomVersionImpl) HandleSendLeave(ctx context.Context,
event PDU,
Copy link
Member

Choose a reason for hiding this comment

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

This is problematic if v != event.Version().

origin spec.ServerName,
eventID, roomID string,
querier CurrentStateQuerier,
verifier JSONVerifier,
) (PDU, error) {
return v.handleSendLeaveFunc(ctx, event, origin, eventID, roomID, querier, verifier)
}

func (v RoomVersionImpl) NewEventFromTrustedJSON(eventJSON []byte, redacted bool) (result PDU, err error) {
return newEventFromTrustedJSON(eventJSON, redacted, v)
}
Expand Down
31 changes: 8 additions & 23 deletions handleleave.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,11 @@ type CurrentStateQuerier interface {
CurrentStateEvent(ctx context.Context, roomID spec.RoomID, eventType string, stateKey string) (PDU, error)
}

// HandleSendLeave handles requests to `/send_leave
// Returns the parsed event and an error.
func HandleSendLeave(ctx context.Context,
requestContent []byte,
// handleSendLeave handles requests to `/send_leave
// Returns the parsed event or an error.
func handleSendLeave(ctx context.Context,
event PDU,
Copy link
Member

Choose a reason for hiding this comment

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

The proposal I mentioned in #387 (comment) was to keep this function public, and to serve as the entry point for callers, because:

Whilst the caller could call roomVer.HandleSendLeave this then creates bad symmetry, as there are cases where you don't know the room version at this point (e.g invites).

origin spec.ServerName,
roomVersion RoomVersion,
eventID, roomID string,
querier CurrentStateQuerier,
verifier JSONVerifier,
Expand All @@ -122,21 +121,6 @@ func HandleSendLeave(ctx context.Context,
return nil, err
}

verImpl, err := GetRoomVersion(roomVersion)
if err != nil {
return nil, spec.UnsupportedRoomVersion(fmt.Sprintf("QueryRoomVersionForRoom returned unknown version: %s", roomVersion))
}

// Decode the event JSON from the request.
event, err := verImpl.NewEventFromUntrustedJSON(requestContent)
switch err.(type) {
case BadJSONError:
return nil, spec.BadJSON(err.Error())
case nil:
default:
return nil, spec.NotJSON("The request body could not be decoded into valid JSON. " + err.Error())
}

// Check that the room ID is correct.
if (event.RoomID()) != roomID {
return nil, spec.BadJSON("The room ID in the request path must match the room ID in the leave event JSON")
Expand Down Expand Up @@ -190,14 +174,15 @@ func HandleSendLeave(ctx context.Context,
}

// Check that the event is signed by the server sending the request.
redacted, err := verImpl.RedactEventJSON(event.JSON())
resultEvent := event
event.Redact()
if err != nil {
util.GetLogger(ctx).WithError(err).Errorf("unable to redact event")
return nil, spec.BadJSON("The event JSON could not be redacted")
}
verifyRequests := []VerifyJSONRequest{{
ServerName: sender.Domain(),
Message: redacted,
Message: event.JSON(),
AtTS: event.OriginServerTS(),
StrictValidityChecking: true,
}}
Expand All @@ -220,5 +205,5 @@ func HandleSendLeave(ctx context.Context,
return nil, spec.BadJSON("The membership in the event content must be set to leave")
}

return event, nil
return resultEvent, nil
}
58 changes: 22 additions & 36 deletions handleleave_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,14 @@ func (v *noopJSONVerifier) VerifyJSONs(ctx context.Context, requests []VerifyJSO

func TestHandleSendLeave(t *testing.T) {
type args struct {
ctx context.Context
requestContent []byte
origin spec.ServerName
roomVersion RoomVersion
eventID string
roomID string
querier CurrentStateQuerier
verifier JSONVerifier
ctx context.Context
event PDU
origin spec.ServerName
roomVersion RoomVersion
eventID string
roomID string
querier CurrentStateQuerier
verifier JSONVerifier
}

_, sk, err := ed25519.GenerateKey(rand.Reader)
Expand Down Expand Up @@ -318,79 +318,65 @@ func TestHandleSendLeave(t *testing.T) {
}{
{
name: "invalid roomID",
args: args{roomID: "@notvalid:localhost"},
wantErr: assert.Error,
},
{
name: "invalid room version",
args: args{roomID: "!notvalid:localhost", roomVersion: "-1"},
wantErr: assert.Error,
},
{
name: "invalid content body",
args: args{roomID: "!notvalid:localhost", roomVersion: RoomVersionV1, requestContent: []byte("{")},
wantErr: assert.Error,
},
{
name: "not canonical JSON",
args: args{roomID: "!notvalid:localhost", roomVersion: RoomVersionV10, requestContent: []byte(`{"int":9007199254740992}`)}, // number to large, not canonical json
args: args{roomID: "@notvalid:localhost", roomVersion: RoomVersionV10},
wantErr: assert.Error,
},
{
name: "wrong roomID in request",
args: args{roomID: "!notvalid:localhost", roomVersion: RoomVersionV10, requestContent: createEvent.JSON()},
args: args{roomID: "!notvalid:localhost", roomVersion: RoomVersionV10, event: createEvent},
wantErr: assert.Error,
},
{
name: "wrong eventID in request",
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, requestContent: createEvent.JSON()},
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, event: createEvent},
wantErr: assert.Error,
},
{
name: "empty statekey",
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, eventID: createEvent.EventID(), requestContent: createEvent.JSON()},
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, eventID: createEvent.EventID(), event: createEvent},
wantErr: assert.Error,
},
{
name: "wrong request origin",
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
args: args{roomID: "!valid:localhost", roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
wantErr: assert.Error,
},
{
name: "never joined the room no-ops",
args: args{roomID: "!valid:localhost", querier: dummyQuerier{}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
args: args{roomID: "!valid:localhost", querier: dummyQuerier{}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
wantErr: assert.NoError,
},
{
name: "already left the room no-ops",
args: args{roomID: "!valid:localhost", querier: dummyQuerier{pdu: leaveEvent}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
args: args{roomID: "!valid:localhost", querier: dummyQuerier{pdu: leaveEvent}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
wantErr: assert.NoError,
},
{
name: "JSON validation fails",
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{err: fmt.Errorf("err")}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{err: fmt.Errorf("err")}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
wantErr: assert.Error,
},
{
name: "JSON validation fails 2",
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{Error: fmt.Errorf("err")}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{Error: fmt.Errorf("err")}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
wantErr: assert.Error,
},
{
name: "membership not set to leave",
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: joinEvent.EventID(), requestContent: joinEvent.JSON()},
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: joinEvent.EventID(), event: joinEvent},
wantErr: assert.Error,
},
{
name: "membership set to leave",
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), requestContent: leaveEvent.JSON()},
args: args{ctx: context.Background(), roomID: "!valid:localhost", querier: dummyQuerier{pdu: createEvent}, verifier: &noopJSONVerifier{results: []VerifyJSONResult{{}}}, origin: validUser.Domain(), roomVersion: RoomVersionV10, eventID: leaveEvent.EventID(), event: leaveEvent},
wantErr: assert.NoError,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := HandleSendLeave(tt.args.ctx, tt.args.requestContent, tt.args.origin, tt.args.roomVersion, tt.args.eventID, tt.args.roomID, tt.args.querier, tt.args.verifier)
if !tt.wantErr(t, err, fmt.Sprintf("HandleSendLeave(%v, %v, %v, %v, %v, %v, %v, %v)", tt.args.ctx, tt.args.requestContent, tt.args.origin, tt.args.roomVersion, tt.args.eventID, tt.args.roomID, tt.args.querier, tt.args.verifier)) {
verImpl := MustGetRoomVersion(tt.args.roomVersion)
_, err := verImpl.HandleSendLeave(tt.args.ctx, tt.args.event, tt.args.origin, tt.args.eventID, tt.args.roomID, tt.args.querier, tt.args.verifier)
if !tt.wantErr(t, err, fmt.Sprintf("handleSendLeave(%v, %v, %v, %v, %v, %v, %v, %v)", tt.args.ctx, tt.args.event, tt.args.origin, tt.args.roomVersion, tt.args.eventID, tt.args.roomID, tt.args.querier, tt.args.verifier)) {
return
}
})
Expand Down