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

Spec on m.login.token #2412

Closed
Sorunome opened this issue Jan 17, 2020 · 7 comments
Closed

Spec on m.login.token #2412

Sorunome opened this issue Jan 17, 2020 · 7 comments
Labels
clarification An area where the spec could do with being more explicit client-server Client-Server API

Comments

@Sorunome
Copy link
Contributor

Currently the spec to m.login.token is that it just must be "a token", nothing more specific. According to @richvdh it should only allow tokens generated by the homeserver. Perhaps clarify that?

Reason for this: Currently, with the synapse doc on password providers it looks like, with "get_supported_login_types" and "check_auth" you should be able to make your own auth provider for m.login.token which, well, you can't. Synapse expects the token to be a macron in any case.

@Sorunome
Copy link
Contributor Author

Personally soru think it'd be better to stick with the current spec that m.login.token could be any token and adapt synapse to reflect that with auth providers. In addition, synapse undocumented m.login.jwt sounds like it should merge with m.login.token

@turt2live turt2live added clarification An area where the spec could do with being more explicit client-server Client-Server API labels Jan 17, 2020
@turt2live
Copy link
Member

jwt used to be part of the spec but was removed. Synapse issues and the spec shouldn't be this intertwined: the spec just needs to clarify what the token is supposed to be used for, and an MSC can change that. Where Synapse is not compliant, that is a Synapse issue.

@richvdh
Copy link
Member

richvdh commented Jan 20, 2020

@Sorunome: Firstly, the synapse development docs are completely irrelevant from the spec's point of view, so let's put that aside.

According to @richvdh it should only allow tokens generated by the homeserver.

I'm not sure that's quite what I said, but in any case I think I'd like to clarify a bit.

Currently, the only way you can get a login token is via the SSO login flow (in which the login token is generated by the homeserver, incidentally, so it's currently true that m.login.token will only accept tokens generated by the homeserver), but it's conceivable that in the future the spec will include other ways of generating login tokens (such as via an external authentication server). I'm therefore not sure that there is much we can add to the m.login.token section which will honour this vision.

m.login.token could be any token

This is nonsense, and certainly not the intention of the current spec. It implies that I could make up any token I like (letmeinplease), send it to matrix.org as a login token, and expect it to work. Clearly the receiving homeserver needs to know how to interpret the token.

It sounds like you want to allow a client to bring its own token to the login endpoint - in which case it sounds like this would have to be deployment-specific: ie, only clients configured specifically for your server could use that flow. I'm currently unconvinced that such a thing needs adding to the spec, but if it does, it sounds like a different thing to m.login.token.

@turt2live: I'm afraid I don't really understand what you're saying, though:

jwt used to be part of the spec but was removed.

[Citation needed]

the spec just needs to clarify what the token is supposed to be used for

I don't really understand what you envision here: are you talking about clarifications to the (token-based login section)[https://matrix.org/docs/spec/client_server/r0.6.0#token-based], or something else? What would a login token be used for other than token-based login?

Where Synapse is not compliant, that is a Synapse issue.

I don't think Synapse is not-compliant?

@Sorunome
Copy link
Contributor Author

but it's conceivable that in the future the spec will include other ways of generating login tokens (such as via an external authentication server).

(see relevant MSC)

This is nonsense, and certainly not the intention of the current spec. It implies that I could make up any token I like (letmeinplease), send it to matrix.org as a login token, and expect it to work. Clearly the receiving homeserver needs to know how to interpret the token.

While yes, you could make up any token, the homeserver is likely to reject such token. What soru meant is, that it sounds (to her) reasonable, to not specify in the spec how all tokens should look like, and thus leave room for homeservers to implement own token types while staying on-spec. She can, however, see if that is not wanted (stating opinion here). So, in sorus opinion, it would be like m.login.token takes at least, let's say, the current token for SSO, but can optionally also take other tokens not specified by the spec.

It sounds like you want to allow a client to bring its own token to the login endpoint

Allowing other types of tokens actually has lots of potential for bots and appservices, see the relevant MSC for such a specific usecase.

ie, only clients configured specifically for your server could use that flow.

Exactly, the bot/appservice/etc would have to be configured to work with the homesrver. Think how, similarly to how synapse currently allows password providers, it would allow custom token providers.

I'm currently unconvinced that such a thing needs adding to the spec, but if it does, it sounds like a different thing to m.login.token.

It's still a token, though. Anyhow, in the relevant MSC only a single, pre-defined token type is added, instead of allowing "any" token like discussed in here.

@turt2live
Copy link
Member

People keep getting tripped up over what the token login thing does. We've tried to clarify it a bunch in the past, but clearly those efforts aren't working: people still think they can provide an access token to it, set some sort of secret in their account/homeserver config, and use it magically with stuff like SSO. The point of clarification is to add a bunch of words to say what exactly the spec wants it to be used for besides "stuff" (which is what the current description amounts to unless you read the SSO sections).

For JWT: looks like I've rewritten history in my head. It's tracked as https://github.com/matrix-org/matrix-doc/issues/1658 and never made it to the spec, though I thought it did. We must have removed something else related to auth that I forgot about.

Synapse's compliance had to do with me misremembering JWT's fate.

@Half-Shot
Copy link
Contributor

Should this be closed now that #2611 has been merged?

@turt2live
Copy link
Member

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification An area where the spec could do with being more explicit client-server Client-Server API
Projects
None yet
Development

No branches or pull requests

4 participants