This repository was archived by the owner on Apr 26, 2024. It is now read-only.
Implement MSC1915 - 3PID unbind APIs#4982
Merged
erikjohnston merged 9 commits intodevelopfrom Apr 3, 2019
Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4982 +/- ##
===========================================
- Coverage 60.6% 60.55% -0.05%
===========================================
Files 332 331 -1
Lines 34235 34239 +4
Branches 5655 5659 +4
===========================================
- Hits 20749 20735 -14
- Misses 12011 12028 +17
- Partials 1475 1476 +1 |
Member
Author
|
(Sytests are on their way) |
Member
Author
|
(Sytest has revealed some issues) |
This will then be used to know which IS to default to when unbinding the threepid.
This changes the behaviour from using the server specified trusted identity server to using the IS that used during the binding of the 3PID, if known. This is the behaviour specified by MSC1915.
By default the homeserver will use the identity server used during the binding of the 3PID to unbind the 3PID. However, we need to allow clients to explicitly ask the homeserver to unbind via a particular identity server, for the case where the 3PID was bound out of band from the homeserver. Implements MSC915.
We assume, as we did before, that users bound their threepid to one of the trusted identity servers. So we simply fill the new table with all threepids in `user_threepids` joined with the trusted identity servers.
9a45505 to
e6ab96d
Compare
e6ab96d to
39fb971
Compare
Member
Author
|
Now with tests: matrix-org/sytest#595 |
richvdh
suggested changes
Apr 2, 2019
Member
richvdh
left a comment
There was a problem hiding this comment.
A few nits. Can you also check the copyright declarations on the files you're making significant changes to?
| medium=threepid["medium"], | ||
| address=threepid["address"], | ||
| ) | ||
| if threepid.get("id_server"): |
Member
There was a problem hiding this comment.
why is this in the dict rather than a function param? (and anyway, the docstring needs updating)
Member
Author
There was a problem hiding this comment.
Broadly because this is how its handled in the bind path where the threepid dict contains the identity server. I don't mind pulling it out but then we probably want to also pull out medium and address
| this. | ||
|
|
||
| We do this by grandfathering in existing user threepids assuming that | ||
| they used one of the server configured trusted identity servers. |
Member
There was a problem hiding this comment.
one of, or all of? (surely it should be one of, but the code does all of)
Member
Author
There was a problem hiding this comment.
We assume that they used "one of", but we don't know which one so we have to include all the id servers.
b8635c9 to
4ef5d17
Compare
turt2live
added a commit
to matrix-org/matrix-spec-proposals
that referenced
this pull request
May 28, 2019
As per [MSC1915](#1915) Implementation proof: matrix-org/synapse#4982 The only alteration made which differs from the proposal is clarity on how to handle homeservers not knowing the `id_server`. All other differences are unintentional.
turt2live
added a commit
to matrix-org/matrix-spec-proposals
that referenced
this pull request
May 28, 2019
As per [MSC1915](#1915) Implementation proof: matrix-org/synapse#4982 The only alteration made which differs from the proposal is clarity on how to handle homeservers not knowing the `id_server`. All other differences are unintentional.
turt2live
added a commit
to matrix-org/matrix-spec-proposals
that referenced
this pull request
May 28, 2019
As per [MSC1915](#1915) Implementation proof: * matrix-org/synapse#4982 * matrix-org/sydent#160 The only alteration made which differs from the proposal is clarity on how to handle homeservers not knowing the `id_server`. All other differences are unintentional.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
The commits should make sense by themselves, but the thrust of this PR is that we now keep track of which IS a given 3PID is bound via so we can use that when we come to unbind.
Fixes #4962