Skip to content
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.

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

Move /send_leave to GMSL #387

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
27 changes: 27 additions & 0 deletions eventversion.go
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
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,11 @@
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 @@
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 All @@ -153,29 +137,29 @@
return nil, spec.BadJSON("No state key was provided in the leave event.")
}
if !event.StateKeyEquals(event.Sender()) {
return nil, spec.BadJSON("Event state key must match the event sender.")
}

Check warning on line 141 in handleleave.go

View check run for this annotation

Codecov / codecov/patch

handleleave.go#L140-L141

Added lines #L140 - L141 were not covered by tests

leavingUser, err := spec.NewUserID(*event.StateKey(), true)
if err != nil {
return nil, spec.Forbidden("The leaving user ID is invalid")
}

Check warning on line 146 in handleleave.go

View check run for this annotation

Codecov / codecov/patch

handleleave.go#L145-L146

Added lines #L145 - L146 were not covered by tests

// Check that the sender belongs to the server that is sending us
// the request. By this point we've already asserted that the sender
// and the state key are equal so we don't need to check both.
sender, err := spec.NewUserID(event.Sender(), true)
if err != nil {
return nil, spec.Forbidden("The sender of the join is invalid")
}

Check warning on line 154 in handleleave.go

View check run for this annotation

Codecov / codecov/patch

handleleave.go#L153-L154

Added lines #L153 - L154 were not covered by tests
if sender.Domain() != origin {
return nil, spec.Forbidden("The sender does not match the server that originated the request")
}

stateEvent, err := querier.CurrentStateEvent(ctx, *rID, spec.MRoomMember, leavingUser.String())
if err != nil {
return nil, err
}

Check warning on line 162 in handleleave.go

View check run for this annotation

Codecov / codecov/patch

handleleave.go#L161-L162

Added lines #L161 - L162 were not covered by tests
// we weren't joined at all
if stateEvent == nil {
return nil, nil
Expand All @@ -186,18 +170,19 @@
}
// we already processed this event
if event.EventID() == stateEvent.EventID() {
return nil, nil
}

Check warning on line 174 in handleleave.go

View check run for this annotation

Codecov / codecov/patch

handleleave.go#L173-L174

Added lines #L173 - L174 were not covered by tests

// 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")
}

Check warning on line 182 in handleleave.go

View check run for this annotation

Codecov / codecov/patch

handleleave.go#L180-L182

Added lines #L180 - L182 were not covered by tests
verifyRequests := []VerifyJSONRequest{{
ServerName: sender.Domain(),
Message: redacted,
Message: event.JSON(),
AtTS: event.OriginServerTS(),
StrictValidityChecking: true,
}}
Expand All @@ -213,12 +198,12 @@
// check membership is set to leave
mem, err := event.Membership()
if err != nil {
util.GetLogger(ctx).WithError(err).Error("event.Membership failed")
return nil, spec.BadJSON("missing content.membership key")
}

Check warning on line 203 in handleleave.go

View check run for this annotation

Codecov / codecov/patch

handleleave.go#L201-L203

Added lines #L201 - L203 were not covered by tests
if mem != spec.Leave {
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
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