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

MSC3659: Invite Rules #3659

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

MSC3659: Invite Rules #3659

wants to merge 13 commits into from

Conversation

joshqou
Copy link

@joshqou joshqou commented Jan 19, 2022

Rendered

Signed-off-by: Josh Qou jqou@icloud.com

@joshqou joshqou changed the title MSC3653: Invite Rules MSC3659: Invite Rules Jan 19, 2022
@turt2live turt2live added proposal A matrix spec change proposal proposal-in-review client-server Client-Server API needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Jan 19, 2022
proposals/3659-invite-rules.md Show resolved Hide resolved
proposals/3659-invite-rules.md Outdated Show resolved Hide resolved
1. `"all"`: Break from the `m.invite_rules` check and continue.
2. `"has-shared-room"`: Get all Rooms Invitee is in, and check if the Inviter has a `"join"` membership state.
If the Inviter does not have at least one shared room, Reject the invite request.
3. `"has-direct-room"`: Get the Invitee's account state `"m.direct"` exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

Reliance on m.direct might be a bad thing since this is more of a client hint in my view. If we get Cannonical DMs reliance on sharing a Cannonical DM that is currently open as in your currently joined to it would make this more reliable and nullify this specific concern.

Copy link
Author

Choose a reason for hiding this comment

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

Using m.direct seemed to me to be the best way to implement a "friends only" type of invite rule. I'm open to alternatives

Copy link
Contributor

Choose a reason for hiding this comment

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

Much better in my view is using a space to achive this. Spaces for Contacts list is something i have been saying forever.

@turt2live
Copy link
Member

@joshqou when you get the chance, please sign off on the changes as per the contributing guidelines. Usually this is done with a comment or edit to the PR description like so:

Signed-off-by: Your Name <email@example.org>

- Added Alternatives
- Added Potential Issues
- Moved unstable prefix to it's own section
- Added additional RuleItem types, m.shared_room and m.target_room.
### `m.invite_rules`

An invite rules state contains one required key, and two optional keys.
- `"invite_rule"`: A required String-Enum which has four values
Copy link
Author

@joshqou joshqou Jan 22, 2022

Choose a reason for hiding this comment

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

An additional rule which would allow a user to "rate limit" how fast they can recieve invites may be useful to combat multi-user invite spamming. This could be implemented by having an "invite_last_received" date for the user and doing a seconds since check

proposals/3659-invite-rules.md Outdated Show resolved Hide resolved
proposals/3659-invite-rules.md Outdated Show resolved Hide resolved
proposals/3659-invite-rules.md Outdated Show resolved Hide resolved
Comment on lines +81 to +83
Implementations may wish to utilise result caching where applicable to improve performance. Such as for rules that may require comparing the joined rooms of each user.

*Such cache would be storing the resulting `Boolean` returned during `RuleItem` evaluation, **not** the `RuleItemAction` which is picked from the defined `"pass"` or `"fail"` keys.*
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this performance note is worth including in the spec. Implementations can come to it on their own.

{
"type": "m.invite_rules",
"content": {
"rules": [
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if simple linear logic is enough here. For example what if I want to allow members of space to invite me to rooms in that space. Or what if I want people I share rooms with to be able to DM me but not invite me to a public room?

Copy link
Author

Choose a reason for hiding this comment

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

There are edgecases which the rules system can't account for but it's the best compromise between giving the user enough control to be useful, and not overcomplicating the specification.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this could be helped by adding and and or rules in the future. However do to the nesting of pass and fail inside the rule that would be a weird extension. Maybe it would be best to separate the condition from the action so that it would be more natural to add these more complex and and or rules in the future?

Copy link
Author

Choose a reason for hiding this comment

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

I thought about adding m.conditional previously but I decided against it due to the added complexity it would end up bringing. Adding RuleItems that change execution flow would make ruleset evaluation more complicated without much initial benefit, I'd rather introduce something like that as an additional MSC to make it easier for homeserver developers to add basic support for Invite Rules.

proposals/3659-invite-rules.md Outdated Show resolved Hide resolved
* `"any"`: Always evaluates as True.
* `"has-shared-room"`: Evaluates as True if the Inviter shares at least one room with the Invitee.
* `"has-direct-room"`: Evaluates as True if the Inviter has an active room defined in the Invitee's `m.direct` account data state. *Active is defined as "if both the Invitee and Inviter are present".*
* `"none"`: Always evaluates as False.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the point of this?

Copy link
Author

Choose a reason for hiding this comment

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

m.invite_rule was mostly copy-pasted from the previous version of the rules system, when it included the invite_rule key in the root of the event.

Copy link
Author

Choose a reason for hiding this comment

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

any and none have been removed, m.invite_rule is now m.compare. has-shared-room and has-direct-room have unique logic compared to other rules so keeping them grouped together makes sense for implementations

proposals/3659-invite-rules.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants