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

MSC2230: Store Identity Server in Account Data #2230

Merged
merged 6 commits into from Aug 26, 2019
Merged
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -0,0 +1,74 @@
# MSC2230: Store Identity Server in Account Data

The URL of the Identity Sever to use is currently specified at registration and
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 16, 2019

Member
Suggested change
The URL of the Identity Sever to use is currently specified at registration and
The URL of the Identity Server to use is currently specified at registration and
login time and then used for the lifetime of a login session. If users wish to
specify a custom one, they must do so each time they log in on every client.
Once they have chosen an Identity Server to advertise their 3PIDs on, it would
be normal that they would wish to continue using this Identity Server for all
Identity requests in their account accross all clients. This proposal aims to
make this easier.

## Proposal

The base URL of the Identity Server is to be stored in user account data. It
shall be stored in the same format as in a .well-known file under the key,

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 16, 2019

Member

can you give an example of what this will look like?

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 16, 2019

Member
{
  "type": "m.identity_server",
  "content": {
    "base_url": "https://vector.im"
  }
}

if I understand the docs correctly, I think.

This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 16, 2019

Member
Suggested change
shall be stored in the same format as in a .well-known file under the key,
shall be stored in the same format as in a .well-known file under the event type
`m.identity_server` and shall comprise a single key, `base_url` which is the
base URL of the ID Server to use (that is, the part before `/_matrix`).
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@anoadragon453

anoadragon453 Aug 13, 2019

Member

Including https://?


Upon registration or login, a client MUST refrain from performing any requests
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 13, 2019

Member

We have no framework for enforcing this:

Suggested change
Upon registration or login, a client MUST refrain from performing any requests
Upon registration or login, a client SHOULD refrain from performing any requests
to the Identity Server until the account data has been fetched from the server.
Once it has the account data, it MUST check for the presence of the
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 13, 2019

Member
Suggested change
Once it has the account data, it MUST check for the presence of the
Once it has the account data, it SHOULD check for the presence of the
`m.identity_server` key. If present, the `base_url` in this key MUST be used
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 13, 2019

Member
Suggested change
`m.identity_server` key. If present, the `base_url` in this key MUST be used
`m.identity_server` key. If present, the `base_url` in this key SHOULD be used
as the Identity Server base URL for the duration of the login session. If this
key is not present, the client SHOULD populate it with the ID Server URL
This conversation was marked as resolved by turt2live

This comment has been minimized.

Copy link
@jryans

jryans Aug 20, 2019

Member

After some discussion in the privacy sync (recorded in vector-im/riot-web#10597), it seems easier and reasonable for clients to not write any IS in account data in login / startup.

that was or would have been used in the login/registration process. This may
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 13, 2019

Member

"may" can imply that there's a limited set of options. To reduce ambiguity, we can use a different word:

Suggested change
that was or would have been used in the login/registration process. This may
that was or would have been used in the login/registration process. This could
be either from user input, a .well-known lookup, or a default in the client.

Client MUST listen for changes in the `m.identity_server` account data value
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 13, 2019

Member
Suggested change
Client MUST listen for changes in the `m.identity_server` account data value
Client SHOULD listen for changes in the `m.identity_server` account data value
and update the URL that they use for ID Server requests accordingly UNLESS
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 13, 2019

Member

UNLESS is not a keyword we use. I would recommend just lowercasing it.

Suggested change
and update the URL that they use for ID Server requests accordingly UNLESS
and update the URL that they use for ID Server requests accordingly unless
the login session and choice of ID Server base URL predates this change, in
which case they SHOULD continue to use the value they are currently using.
This conversation was marked as resolved by turt2live

This comment has been minimized.

Copy link
@jryans

jryans Aug 20, 2019

Member

After some discussion in the privacy sync (recorded in vector-im/riot-web#10597), it seems easier and reasonable for clients to apply any changes via account data as they come down.


Clients MAY offer a way for users to change the ID server being used. If they
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@turt2live

turt2live Aug 13, 2019

Member

MAY is not a keyword we use:

Suggested change
Clients MAY offer a way for users to change the ID server being used. If they
Clients can offer a way for users to change the ID server being used. If they
do, the client MUST update the value of `m.identity_server` accordingly.

The `m.identity_server` may be present with a value of `null`. In this case,
This conversation was marked as resolved by dbkr

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 16, 2019

Member

account data is a list of events (which inherently have a content dict) rather a set of completely arbitrary values, so do you mean:

Suggested change
The `m.identity_server` may be present with a value of `null`. In this case,
The `m.identity_server` may be present with a `base_url` of `null`. In this case,
clients MUST treat this as no ID Server URL being set and not perform ID
Server requests, disabling any functionality that requires such requests.

Conversely, if a user wishes to disable ID Server functionality, the client
shall action this by setting the `base_url` of the `m.identity_server`
account data entry to `null`.

### Transition Period

Clients currently logged in with a value configured for the ID Server base
URL SHOULD use the value from `m.identity_server` if it exists or is created,
but otherwise continue to use the URL they had previously. They MUST NOT
populate the `m.identity_server` with their current ID Server base URL.

## Tradeoffs

There are a number of ways to transition to this new scheme. Clients could
populate the account data with their current ID Server URL as soon as
possible, and immediately use any new value seen on account data. This
would a much faster migration but any users with clients using different
ID Servers would suddenly find all their clients using the ID Server of
whichever client they updated first.

## Potential issues

Users will no longer be able to have different clients configured with
different ID Servers.

## Security considerations

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 16, 2019

Member

do clients need to be a bit wary of things they read from the URL to check that they are valid URLs and aren't localhost and that sort of thing?


An attacker would be able to force all a user clients to use a given ID Server

This comment has been minimized.

Copy link
@richvdh

richvdh Aug 16, 2019

Member

it also puts control of the IS to be used in the hands of the HS admin (or a MITM etc).

This comment has been minimized.

Copy link
@uhoreg

uhoreg Aug 17, 2019

Member

This could be solved by signing m.identity_server account data using the user's master cross-signing key. (Once we get that through MSC...)

if they gained control of any of a user's logins.

## Conclusion

This makes the ID server an account setting which means it persists between
logins. The intention would be to phase out clients ever asking for an ID
Server URL at registration or login: this will be much easier for users to
understand whilst still retaining the flexibilty for those who want it.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.