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
MSC2140: Terms of Service for ISes and IMs #2140
MSC2140: Terms of Service for ISes and IMs #2140
Changes from all commits
23af87e
32c7fc6
cf48030
276e2b6
d4ca0c2
9ca3ccc
a63e442
4ba9b2a
2555801
8ae4755
abb4071
2c09580
6f374dc
0dae2d5
9e0d8b9
5709427
af691b5
1d75828
ba7047c
4edf826
6273868
58cf083
2694bb1
21b9eaf
b5326de
10a6a59
f95197b
4be283c
83bb386
45d6309
786d5bc
fe14d3c
8af35be
2d11217
5374030
f02e4c2
d00dfb7
03e6ab0
7f65364
ac6b9bd
79dbad2
10858bf
4c72c37
e28f7aa
d15c9df
1a66934
30dcc28
bf8a1e5
701d340
9bb6ad8
f474b31
6e061b1
25a47af
a1de6ff
d9269b0
e4bdc28
12377fb
6d00673
4073d94
6931541
8bd9d7c
4ea8f64
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.
Do we also want to drop lookups done without using the hashing aware endpoints?
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 reality we'll probably end up having non-hashed on v1 and hashed on v2, then deprecating non-hashed and v1 at the same time. I didn't wamt to tie this too much to the hashing msc so not sure whether to add that here or not.
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.
v2 supports non-hashed so there's no reason to keep v1 around.
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.
Would it make sense to uniquely identify by tuple of
(<type>, <version>, <url>)
? That would give de-duplication while still allowing justhttps://matrix.org/legal.html
, as well as not having to do any crazyn²
lookups?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.
Possibly - otoh I sort of like forcing the version to be in the URL for general URL transparency.
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, I'm just a little nervous that if we end up having three or more different services, each with multiple policies, with some churn of versions of the years, plus an increasing number of languages, then we'll end up with quite a lot of different stuff to check.
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.
Travis points out that we discussed this kind of thing in #2140 (comment) as eventually the conclusion was that de-duping by URL is probably the lesser evil.
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.
Could we have a dictionary
policy_id -> policy_url
instead of an array of URLs?That will help to know which types of document the user has already signed. Matrix clients could then detect an update of one policy.
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.
It might also be useful to know the version of the document the user saw too, so could have:
...maybe?
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.
For this MSC, the policy ID feels like it is not meant to be trusted and could change at any time, so I am wary of making any assumptions about it. Maybe the MSC should explicitly state something like: "The terms response contains these policy IDs largely to match similar behaviour from the homeserver, but don't assume anything about them, they may change at any time, etc. The URL is the thing that matters."
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.
the homeserver version being MSC1692 where the policy ID actually means something. The homeserver version supports optional and non-optional changes to policies whereas this MSC does not (for valid reasons on both fronts, I believe).
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.
Should we just deprecate the delete endpoint?
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.
It's still needed for clients to remove the 3pid from the HS though?
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.
The default state going forward (in Riot at least) will be that users first add a 3PID to the HS only. Then, in a separate step, they can also choose to share it with the IS as well.
It would be ideal if we could bind to the IS via the HS for a 3PID already known to HS (which is what will soon be happening quite often) in a single operation. Currently though, using the
POST /_matrix/client/r0/account/3pid
flow to re-add a 3PID with binding will fail withM_THREEPID_IN_USE
. (It actually fails during the earlierPOST /_matrix/client/r0/account/3pid/email/requestToken
step.)Since these CS APIs are authed, the HS knows whether the 3PID is in use by the authed user. It would be best to change the spec to allow re-adding an in use 3PID if it's in use by the current user.
In the short term, Riot will work around this by using two step operation of removing the 3PID from the HS and then re-adding with bind true.
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.
There are several different user flows involving 3PIDs, including:
It would be best from a data flow and privacy perspective if flows 1 - 3 could all be done using the HS only, possibly delegating to a token sending server if needed. matrix-org/synapse#5751 is heading in that direction.
For case 4 of sharing a 3PID for discovery, we currently proxy via the HS to the IS for this, mainly so that the HS has a record of 3PID associations that can be unbound when an account is deactivated.
With v2 IS APIs and terms requirements, it becomes more complex for the HS to do this proxying. The user may need to agree terms with the IS, but the client isn't communicating with the IS directly.
@dbkr has written up several different options of how we could move forward. Option 4 there:
seems appealing, but it's so far unclear how to resolve the authentication issue.
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.
At the moment, we are leaning towards option 2:
@anoadragon453 is planning to MSC this change.
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.
MSC2229 now describes 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.
Maybe this proposal should be updated to reference the new MSC as the suggested way of rebinding?
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.
(in general, please link to the proposal PR instead of the markdown, as people delete branches sometimes: #2229 )
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'm struggling to follow if this thread has been resolved or not?
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 believe the technical problem is solved by #2229. It would be nice to mention that in this proposal, so I believe that's the only action left to resolve this thread.