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
MSC2964: Usage of OAuth 2.0 authorization code grant and refresh token grant #2964
base: main
Are you sure you want to change the base?
MSC2964: Usage of OAuth 2.0 authorization code grant and refresh token grant #2964
Changes from 3 commits
2af8b49
5962618
38c97a5
e12ee77
20d865d
261e3b0
38bb557
45d510b
d114f82
8fc3ea1
029e1e5
0802d8f
20ee4a3
6e387d8
4a2ed74
40048da
f0e319a
55215c1
21fee1c
2c0625d
24e0290
d145fd2
acfa845
fa506ff
378348e
c859c0b
c1c8312
05748a2
1034122
4830d47
f84428f
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.
So far the spec only requires a username on login and
/login
then returns the MXID. At work we actually utilize this so that the login username has nothing to do with the MXID you have in the end.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 would like to see some more info on what a "login hint to what MXID" is. Can that be the username or does it have to be the mxid? Does the client now have to do the mxid mapping, that so far was pretty much implementation defined?
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.
If I understood correctly, like the name suggests, it's only a hint, to avoid the user having to enter their MXID/username several times.
For example, with a client you would have these steps:
In no way it should limit the choice of username for the user, or force the auth server or homeserver to use it. That's what I understand of the intentions for that field from the OIDC spec anyway.
Although ultimately the auth server is free to do whatever it pleases and might also ignore this parameter altogether.
I would guess it's also up to the auth server and homeserver to decide if the username used for authentication matches the one in the MXID. Irc, it's already the case in Synapse's OIDC config where one can chose to map the username from the auth server to anything else.
To get the final MXID, the client needs to call the homeserver's
GET /whoami
endpoint with the access token that was obtained during auth.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.
Well, often clients actually use separate fields for server and username (or the server is even preconfigured). Users then just enter "FirstName LastName", so would that be an acceptible login hint or does it have to be an mxid?
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.
Couldn't this be replaced with; the client requesting for a seperate token with only the required scope to perform this action, only for it to be dropped (deactivated) immidiately after?
Something about upgrading the same token doesn't sit well with me from a security perspective, it might be a deliberately locked-down token, and/or multiple actions at once could race to upgrade/downgrade the current token.
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.
Having a separate token purely for one scope / set of scopes sounds like a really good idea, especially for race condition reasons. Races could be also avoided by introducing state on AS/OP side, but that is quite out of scope of oauth2 specification (if I'm not mistaken)
At the same time, it would make specification compatible with more possible OP implementations.
Making it single use would be easily doable using jti claim on that token.