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

Fix error handling for missing auth_event #3960

Merged
merged 3 commits into from Oct 2, 2018

Conversation

3 participants
@richvdh
Member

richvdh commented Sep 26, 2018

When we were authorizing an event, if there was no m.room.create in its
auth_events, we would raise a SynapseError with a cryptic message, which
then meant that we would bail out of processing any incoming events, rather
than storing a rejection for the faulty event and moving on.

We should treat the absent event the same as any other auth failure, by
raising an AuthError, so that the event is marked as rejected.

richvdh added some commits Sep 25, 2018

Fix error handling for missing auth_event
When we were authorizing an event, if there was no `m.room.create` in its
auth_events, we would raise a SynapseError with a cryptic message, which then
meant that we would bail out of processing any incoming events, rather than
storing a rejection for the faulty event and moving on.

We should treat the absent event the same as any other auth failure, by
raising an AuthError, so that the event is marked as rejected.

@richvdh richvdh requested a review from matrix-org/synapse-core Sep 26, 2018

@hawkowl

This comment has been minimized.

Contributor

hawkowl commented Sep 26, 2018

@richvdh do we have tests for this behaviour?

@richvdh

This comment has been minimized.

Member

richvdh commented Sep 26, 2018

Not really, I'm afraid. Right now it's hard to test this functionality, because other bugs (notably #3923) stop the relevant bit of code even being reached.

We could hold off on this PR for now, and wait for the fix to #3923, and then make a sytest that checks this properly.

@richvdh richvdh self-assigned this Sep 27, 2018

@richvdh richvdh added this to To Do in Backend Core Team via automation Sep 27, 2018

@richvdh richvdh moved this from To Do to In Progress: Operational/bug fixes in Backend Core Team Sep 28, 2018

richvdh added a commit to matrix-org/sytest that referenced this pull request Oct 1, 2018

richvdh added a commit to matrix-org/sytest that referenced this pull request Oct 1, 2018

@richvdh

This comment has been minimized.

Member

richvdh commented Oct 1, 2018

there is kind-of a test for this now in matrix-org/sytest#499.

@richvdh richvdh removed their assignment Oct 1, 2018

richvdh added a commit to matrix-org/sytest that referenced this pull request Oct 1, 2018

richvdh added a commit to matrix-org/sytest that referenced this pull request Oct 1, 2018

@richvdh richvdh self-assigned this Oct 1, 2018

@richvdh richvdh merged commit 2b8d28b into develop Oct 2, 2018

6 checks passed

ci/circleci: sytestpy2merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy2postgresmerged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3postgresmerged Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@richvdh richvdh deleted the rav/fix_missing_create_event_error branch Oct 2, 2018

@richvdh richvdh moved this from In Progress: Operational/bug fixes to Done - Operations in Backend Core Team Oct 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment