-
Notifications
You must be signed in to change notification settings - Fork 397
Clarifications to SSO login/UIA #2608
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
Conversation
including a bunch of text about security
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.
Generally lgtm - just some questions and minor clarifications
|
||
An overview of the process is as follows: | ||
|
||
0. The Matrix client calls |GET /login|_ to find the supported login |
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.
Please can we start numbered lists at one?
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.
we can, though in this case step zero felt very much like a preliminary step.
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.
It's just a bit awkward to have things start at zero, as it implies that it's not a real step (at least imo).
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.
that implication is correct, imo :)
<../index.html#mapping-from-other-character-sets>`_ may be useful. | ||
This confirmation could take place before redirecting to the SSO | ||
authentication page (when handling the | ||
``/_matrix/client/%CLIENT_MAJOR_VERSION%/auth/m.login.sso/fallback/web`` |
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.
fwiw we have a history of shortening URLs to after the version (ie: /auth/m.login.sso/fallback/web
), largely for legibility.
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.
mmhmm. There was some inconsistency here and I went this way: do you think I should go the other?
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 don't personally have a preference, just mentioning it in the event it felt annoying to have to do it every time.
Co-authored-by: Travis Ralston <travpc@gmail.com>
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.
Thanks!
1. To initiate the ``m.login.sso`` login type, the Matrix client instructs the | ||
user's browser to navigate to the |/login/sso/redirect|_ endpoint on the | ||
user's homeserver. |
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 know it's taken from the current text but it's a bit too specific, in my opinion. It's not really necessary to assume that SSO requires a web view which is generally, but not necessarily the case. The flow between (1) and (4) only requires HTTP(S). (3) doesn't need even that: interaction with the Auth server can be driven by API (voice authentication through a SIP call; local agent providing SSO; you name it). The wording here also implies that the user's browser is somewhat separate from the client while the text later suggests using an embedded web view for native applications.
1. To initiate the ``m.login.sso`` login type, the Matrix client instructs the | |
user's browser to navigate to the |/login/sso/redirect|_ endpoint on the | |
user's homeserver. | |
1. To initiate the ``m.login.sso`` login type, the Matrix client dispatches | |
a call to the |/login/sso/redirect|_ endpoint on the user's homeserver. | |
Depending on the client architecture, the call and further flow can | |
either be performed by the client itself or the client may use a suitable | |
embedded view or external application (e.g. a web browser if the flow is | |
web-based). This client-side party of the authentication process (the Matrix | |
client itself or the view/application) is further referred to as | |
"authenticating agent". |
Further proposed changes below.
|
||
1. To initiate the ``m.login.sso`` login type, the Matrix client instructs the | ||
user's browser to navigate to the |/login/sso/redirect|_ endpoint on the | ||
user's homeserver. | ||
|
||
2. The homeserver responds with an HTTP redirect to the SSO user interface, | ||
which the browser follows. |
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.
which the browser follows. | |
which the authenticating agent follows. |
3. The authentication server and the homeserver interact to verify the user's | ||
identity and other authentication information, potentially using a number of | ||
redirects. |
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.
Surely there was a typo? Interaction is not with the homeserver at this stage.
3. The authentication server and the homeserver interact to verify the user's | |
identity and other authentication information, potentially using a number of | |
redirects. | |
3. The authentication server and the authenticating agent interact | |
to verify the user's identity and other authentication information, | |
potentially using a number of redirects. |
4. The browser is directed to the ``redirectUrl`` provided by the client with | ||
a ``loginToken`` query parameter for the client to log in with. |
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.
4. The browser is directed to the ``redirectUrl`` provided by the client with | |
a ``loginToken`` query parameter for the client to log in with. | |
4. Upon successful completion of (3) the authentication server directs | |
the authenticating agent to the ``redirectUrl`` provided by the client at step (1), | |
adding a ``loginToken`` query parameter for the client to log in with. |
For native applications, typically steps 1 to 4 are carried out by opening an | ||
embedded web view. |
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.
For native applications, typically steps 1 to 4 are carried out by opening an | |
embedded web view. | |
For native applications and web-based authentication flows, typically | |
steps 1 to 4 are carried out by opening an embedded web view. |
|
||
Instead, the state could be stored in `localStorage | ||
<https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage>`_ or | ||
in a cookie. |
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.
in a cookie. | |
in a cookie; native clients should also keep state local to them. |
and the REST endpoints which consume the ticket. A good reference for how CAS could | ||
be implemented is available in the older `r0.4.0 version <https://matrix.org/docs/spec/client_server/r0.4.0.html#cas-based-client-login>`_ | ||
of this specification. | ||
The server should handle |
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.
Just for the sake of clarity
The server should handle | |
The homeserver should handle |
| | | | | ||
| |<========(3) Authentication process================>| | ||
| | | | | ||
| |<--(4) redirect to redirectUrl--| | |
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 it's really the homeserver invoking the redirect then how does it figure out that the auth flow has been successful? The sequence has a gap between 3 and 4.
#. The homeserver should generate a short-term login token. This is an opaque | ||
token, suitable for use with the ``m.login.token`` type of the |/login|_ | ||
API. The lifetime of this token SHOULD be limited to around five | ||
seconds. |
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.
This seems to assume relatively quick roundtrip between the client and the homeserver?
parameters. If it already includes one or more ``loginToken`` parameters, | ||
they should be removed before adding the new one.) | ||
|
||
#. The homeserver redirects the user's browser to the URI thus built. |
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.
#. The homeserver redirects the user's browser to the URI thus built. | |
#. The homeserver redirects the client to the URI thus built. |
including a bunch of text about security.