Fix room checks for /state and /state_ids (#1155)

We would return a 403 first (as the server would not be allowed to
see this event) and only then return a 404 if the event is not in
the given room. We now invert those checks for /state and /state_ids
to make the tests pass.
This commit is contained in:
Kegsay 2020-06-23 11:47:48 +01:00 committed by GitHub
parent 02565c37aa
commit 4220a374ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 11 deletions

View File

@ -33,7 +33,11 @@ func GetEvent(
eventID string, eventID string,
origin gomatrixserverlib.ServerName, origin gomatrixserverlib.ServerName,
) util.JSONResponse { ) util.JSONResponse {
event, err := getEvent(ctx, request, rsAPI, eventID) err := allowedToSeeEvent(ctx, request.Origin(), rsAPI, eventID)
if err != nil {
return *err
}
event, err := fetchEvent(ctx, rsAPI, eventID)
if err != nil { if err != nil {
return *err return *err
} }
@ -47,35 +51,40 @@ func GetEvent(
}} }}
} }
// getEvent returns the requested event, // allowedToSeeEvent returns no error if the server is allowed to see this event,
// otherwise it returns an error response which can be sent to the client. // otherwise it returns an error response which can be sent to the client.
func getEvent( func allowedToSeeEvent(
ctx context.Context, ctx context.Context,
request *gomatrixserverlib.FederationRequest, origin gomatrixserverlib.ServerName,
rsAPI api.RoomserverInternalAPI, rsAPI api.RoomserverInternalAPI,
eventID string, eventID string,
) (*gomatrixserverlib.Event, *util.JSONResponse) { ) *util.JSONResponse {
var authResponse api.QueryServerAllowedToSeeEventResponse var authResponse api.QueryServerAllowedToSeeEventResponse
err := rsAPI.QueryServerAllowedToSeeEvent( err := rsAPI.QueryServerAllowedToSeeEvent(
ctx, ctx,
&api.QueryServerAllowedToSeeEventRequest{ &api.QueryServerAllowedToSeeEventRequest{
EventID: eventID, EventID: eventID,
ServerName: request.Origin(), ServerName: origin,
}, },
&authResponse, &authResponse,
) )
if err != nil { if err != nil {
resErr := util.ErrorResponse(err) resErr := util.ErrorResponse(err)
return nil, &resErr return &resErr
} }
if !authResponse.AllowedToSeeEvent { if !authResponse.AllowedToSeeEvent {
resErr := util.MessageResponse(http.StatusForbidden, "server not allowed to see event") resErr := util.MessageResponse(http.StatusForbidden, "server not allowed to see event")
return nil, &resErr return &resErr
} }
return nil
}
// fetchEvent fetches the event without auth checks. Returns an error if the event cannot be found.
func fetchEvent(ctx context.Context, rsAPI api.RoomserverInternalAPI, eventID string) (*gomatrixserverlib.Event, *util.JSONResponse) {
var eventsResponse api.QueryEventsByIDResponse var eventsResponse api.QueryEventsByIDResponse
err = rsAPI.QueryEventsByID( err := rsAPI.QueryEventsByID(
ctx, ctx,
&api.QueryEventsByIDRequest{EventIDs: []string{eventID}}, &api.QueryEventsByIDRequest{EventIDs: []string{eventID}},
&eventsResponse, &eventsResponse,

View File

@ -98,13 +98,17 @@ func getState(
roomID string, roomID string,
eventID string, eventID string,
) (*gomatrixserverlib.RespState, *util.JSONResponse) { ) (*gomatrixserverlib.RespState, *util.JSONResponse) {
event, resErr := getEvent(ctx, request, rsAPI, eventID) event, resErr := fetchEvent(ctx, rsAPI, eventID)
if resErr != nil { if resErr != nil {
return nil, resErr return nil, resErr
} }
if event.RoomID() != roomID { if event.RoomID() != roomID {
return nil, &util.JSONResponse{Code: http.StatusNotFound, JSON: nil} return nil, &util.JSONResponse{Code: http.StatusNotFound, JSON: jsonerror.NotFound("event does not belong to this room")}
}
resErr = allowedToSeeEvent(ctx, request.Origin(), rsAPI, eventID)
if resErr != nil {
return nil, resErr
} }
var response api.QueryStateAndAuthChainResponse var response api.QueryStateAndAuthChainResponse

View File

@ -350,3 +350,5 @@ setting 'm.room.name' respects room powerlevel
Syncing a new room with a large timeline limit isn't limited Syncing a new room with a large timeline limit isn't limited
Left rooms appear in the leave section of sync Left rooms appear in the leave section of sync
Banned rooms appear in the leave section of sync Banned rooms appear in the leave section of sync
Getting state checks the events requested belong to the room
Getting state IDs checks the events requested belong to the room