Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MSC2260: Update the auth rules for
m.room.aliases
events #2260MSC2260: Update the auth rules for
m.room.aliases
events #2260Changes from 5 commits
a80fdff
3ab0b63
9ba4c7f
5ec80dd
4d45afd
ca4aad1
21daa6a
31fa78b
784b0e9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should still let users contribute entries to the event, because otherwise they will have to tell the moderator out-of-band to add their room. As you say, it also increases the risk that when the alias is removed from the directory in question, the event won't be updated if a normal user can't update it.
If a room admin deliberately wants to lock down who can advertise aliases to just be mods/ops, then they can do that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is different to what you said at https://gist.github.com/ara4n/0ec423e7c321acbb53eed04ceb4dd7df#gistcomment-3012860 though. Have you changed your mind or is one of us misunderstanding?
If we continue to allow regular users to update the aliases event without regard to power levels, then we still have an abuse problem that is hard to control.
Or do you mean that
aliases
events should be subject to PLs, but for public rooms, we should emit a PL event that lowers the required level foraliases
to 0? In which case, I think this makes sense.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think i am misunderstanding. My assumption was that the special cases for 4a and 4b still take effect (meaning that you can't publish m.room.aliases on behalf of remote servers). However, on 4c, normal PL rules take effect - so it's possible to let anyone in the room advertise an alias via
m.room.aliases
, or alternatively put a PL threshold and only let ops/admins do so. My hunch would be to allow anyone to set one by default (so default to PL threshold of 0 for that state event).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, so:
this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, or just define the default PL for
m.room.aliases
to be 0.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that seems an unnecessary complication.
I've updated the MSC to say that we will lower the PL for
m.room.aliases
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is having default PL 0 for this really a good idea? I don't know if I am making a wild assumption here but I'm not sure how many everyday users will examine the power levels when creating a new room unless they have specific reason to.
This would therefore still leave lots of rooms owned by inexperienced users open to drive by-style attacks where someone just joins the room, adds a whole bunch of evil aliases and leaves again (possibly some hours before anyone with a suitable PL even notices).
"Defaults reveal the soul" and all that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, in that scenario we still need the ability for the room admin to gag randoms from adding remote aliases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...or require the user to be in the room in order to set a
m.room.alias
, so we can stop abuse by banning them, which you'd get if we fell through to the default auth rules.