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

Ensure IS in account data is read / written as specced #10597

Closed
jryans opened this issue Aug 20, 2019 · 5 comments
Closed

Ensure IS in account data is read / written as specced #10597

jryans opened this issue Aug 20, 2019 · 5 comments

Comments

@jryans
Copy link
Collaborator

jryans commented Aug 20, 2019

We should ensure that both reading and written the IS in account data follows the model specced by MSC2230.

@turt2live
Copy link
Member

@jryans 2140 or 2230? If 2230, I think this is solved by matrix-org/matrix-react-sdk#3320

@jryans
Copy link
Collaborator Author

jryans commented Aug 20, 2019

Ah right, 2230, thanks. I updated the top comment to clarify.

I opened this after @dbkr mentioned something about clients needing to write the IS value to account data after login, as I couldn't recall that happening in Riot changes so far.

It's quite possible there was a miscommunication and/or I misinterpreted. Looking at MSC2230, it seems like clients are meant to:

  • Write to account data when the user changes the IS in Settings
  • Read from account data and apply a new IS value if it appears in the account data
    • If m.identity_server is missing entirely, use a default IS value from config, .well-known, etc.
    • If m.identity_server is present and null, disable all IS features

If that's a correct reading, then I think Riot is following the spec indeed.

@dbkr, maybe you can clarify / repeat your mental thread from this morning?

@jryans
Copy link
Collaborator Author

jryans commented Aug 20, 2019

In the privacy sync, we agreed that clients should not write any IS in account data in login / startup and clients should apply any changes via account data as they come down.

@dbkr suggests we may need to update the MSC to match this.

@dbkr
Copy link
Member

dbkr commented Aug 20, 2019

So there are difference between the impl and the spec, but our conclusion is that we want to do what the impl currently does (ie. update account data only if explicitly set by the user in settings, always use a value if it comes down in account data)

@jryans
Copy link
Collaborator Author

jryans commented Aug 20, 2019

I have added comments to the MSC on the bits that seem like they need to be updated to match Riot. Since there's nothing for Riot to change, I think we can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants