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

Fix up auth rules #1591

Merged
merged 7 commits into from
Aug 31, 2018
Merged

Fix up auth rules #1591

merged 7 commits into from
Aug 31, 2018

Conversation

erikjohnston
Copy link
Member

Note that the first commit is a bunch of re-indentation.

This is still missing the third party invite stuff, but I'll add that separately.

RST expects sub lists to be indented by three or more spaces. By doing
so we can then rely on `#.` for automatic numbering.
They are still missing third party invites.
@turt2live
Copy link
Member

Fixes #1305

turt2live added a commit to turt2live/matrix-doc that referenced this pull request Aug 29, 2018
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.

sorry to pick holes in the formatting, but consistency is important imho.


a. If ``membership`` is ``join``:
a. If ``membership`` is ``join``:
Copy link
Member

Choose a reason for hiding this comment

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

erm; the extra indentation means that this is rendered as a <blockquote>. I don't really think we should be hacking around the shitness of the matrix.org CSS with this sort of thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh poo. Its not about getting round matrix.org CSS but actually getting them to be semantically sub lists (mainly so the auto numbering works). Turns out that I only needed one extra space.

previous events - *i.e.* it is the first event in the room.
1. If type is ``m.room.create``:

a. Reject if it has any previous events
Copy link
Member

Choose a reason for hiding this comment

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

full-stop

a. Reject if it has any previous events
b. Reject if the domain of the ``room_id`` does not match the domain of the
``sender``.
c. Reject if ``content.room_version`` key is an unrecognized version
Copy link
Member

Choose a reason for hiding this comment

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

full-stop

previous events - *i.e.* it is the first event in the room.
1. If type is ``m.room.create``:

a. Reject if it has any previous events
Copy link
Member

Choose a reason for hiding this comment

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

all the existing rules were written as: "if <foo>, reject|allow." It would be really nice to be consistent.

a. Reject if it has any previous events
b. Reject if the domain of the ``room_id`` does not match the domain of the
``sender``.
c. Reject if ``content.room_version`` key is an unrecognized version
Copy link
Member

Choose a reason for hiding this comment

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

recognised

c. Reject if ``content.room_version`` key is an unrecognized version
d. Otherwise, allow.

#. Reject if event does not have a ``m.room.create`` in its ``auth_events``
Copy link
Member

Choose a reason for hiding this comment

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

full-stop

Copy link
Member

Choose a reason for hiding this comment

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

[etc]

1. If type is ``m.room.create``:

a. Reject if it has any previous events
b. Reject if the domain of the ``room_id`` does not match the domain of the
Copy link
Member

Choose a reason for hiding this comment

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

isn't an m.room.create supposed to have a creator?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes, annoyingly synapse will only explode when checking the first join.

@erikjohnston
Copy link
Member Author

Formatting and consistency is important, and hopefully that is now better :)

Sorry about messing up the indent, silly RST :/

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 otherwise

a. Reject if event has no ``state_key``
b. Allow if and only if sender's domain matches ``state_key``
a. If event has no ``state_key``, reject
b. If sender's domain doesn't matches ``state_key``, reject.
Copy link
Member

Choose a reason for hiding this comment

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

doesn't match

a. If it has any previous events, reject.
b. If the domain of the ``room_id`` does not match the domain of the
``sender``, reject.
c. If ``content.room_version`` key is an unrecognised version, reject.
Copy link
Member

Choose a reason for hiding this comment

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

If content.room_version is present and is not a recognised version, reject.


#. If type is ``m.room.aliases``:

a. Reject if event has no ``state_key``
b. Allow if and only if sender's domain matches ``state_key``
a. If event has no ``state_key``, reject
Copy link
Member

Choose a reason for hiding this comment

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

full stop.

@erikjohnston
Copy link
Member Author

Thank you for your patience

@erikjohnston erikjohnston merged commit 0adfd1e into master Aug 31, 2018
RiotTranslateBot pushed a commit to RiotTranslateBot/matrix-doc that referenced this pull request Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants