Implement SSO for logins #1297

kegsay · 2020-08-24T16:13:54Z

Spec: https://matrix.org/docs/spec/client_server/r0.6.1#sso-client-login
Sytests:

    × Interactive authentication types include SSO
    × Can perform interactive authentication with SSO
    × The user must be consistent through an interactive authentication session with SSO
    × The operation must be consistent through an interactive authentication session
    × Can login with 3pid and password using m.login.password
    × login types include SSO
    × /login/cas/redirect redirects if the old m.login.cas login type is listed
    × Can login with new user via CAS

The text was updated successfully, but these errors were encountered:

anandv96 · 2020-08-31T10:17:48Z

I'm working on this issue with @rohitmohan96 . Can you assign it to us?

duritong · 2020-11-19T06:58:31Z

Will CAS the only supported SSO mechanism or will you also add SAML? How will one be able todo usermapping?

olanod · 2021-08-24T07:22:41Z

PR was closed, nobody that could work on this? 😞

tommie · 2021-09-26T09:49:47Z

I'm forking @anandv96's PR and continuing. Since it included a solution to #403, and we need that first, I'll start by splitting and trying to get hat closed first.

…gin.sso. For login token: * m.login.token will require deleting the token after completeAuth has generated an access token, so a cleanup function is returned by Type.Login. * Allowing different login types will require parsing the /login body twice: first to extract the "type" and then the type-specific parsing. Thus, we will have to buffer the request JSON in /login, like UserInteractive already does. For SSO: * NewUserInteractive will have to also use GetAccountByLocalpart. It makes more sense to just pass a (narrowed-down) accountDB interface to it than adding more function pointers. Code quality: * Passing around (and down-casting) interface{} for login request types has drawbacks in terms of type-safety, and no inherent benefits. We always decode JSON anyway. Hence renaming to Type.LoginFromJSON. Code that directly uses LoginTypePassword with parsed data can still use Login. * Removed a TODO for SSO. This is already tracked in matrix-org#1297. * httputil.UnmarshalJSON is useful because it returns a JSONResponse. This change is intended to have no functional changes.

@anandv96

This is forked from @anandv96's matrix-org#1374. Closes matrix-org#1297.

ghost · 2021-11-23T10:00:12Z

Is this still being worked on? This is the only thing holding me back from switching to Dandrite.

tommie · 2021-11-23T10:07:23Z

No. I'm waiting for a review of PR #2014. So far, no response.

…gin.sso. For login token: * m.login.token will require deleting the token after completeAuth has generated an access token, so a cleanup function is returned by Type.Login. * Allowing different login types will require parsing the /login body twice: first to extract the "type" and then the type-specific parsing. Thus, we will have to buffer the request JSON in /login, like UserInteractive already does. For SSO: * NewUserInteractive will have to also use GetAccountByLocalpart. It makes more sense to just pass a (narrowed-down) accountDB interface to it than adding more function pointers. Code quality: * Passing around (and down-casting) interface{} for login request types has drawbacks in terms of type-safety, and no inherent benefits. We always decode JSON anyway. Hence renaming to Type.LoginFromJSON. Code that directly uses LoginTypePassword with parsed data can still use Login. * Removed a TODO for SSO. This is already tracked in matrix-org#1297. * httputil.UnmarshalJSON is useful because it returns a JSONResponse. This change is intended to have no functional changes.

ynerant · 2022-02-02T17:44:23Z

Hi!
I am also very interested in this feature, this is the main reason I still using Synapse for a personal project.
Do you know if 1) the PR #2014 is going to be merged soon, and 2) if that means that other login types will be supported once the PR got merged?
Thanks!

kegsay · 2022-02-04T19:39:17Z

I am aiming to get #2014 merged soon yes, we're basically happy with it at this point. That is a pre-requisite for allowing SSO flows.

* Add GOPATH to PATH in find-lint.sh. The user doesn't necessarily have it in PATH. * Refactor LoginTypePassword and Type to support m.login.token and m.login.sso. For login token: * m.login.token will require deleting the token after completeAuth has generated an access token, so a cleanup function is returned by Type.Login. * Allowing different login types will require parsing the /login body twice: first to extract the "type" and then the type-specific parsing. Thus, we will have to buffer the request JSON in /login, like UserInteractive already does. For SSO: * NewUserInteractive will have to also use GetAccountByLocalpart. It makes more sense to just pass a (narrowed-down) accountDB interface to it than adding more function pointers. Code quality: * Passing around (and down-casting) interface{} for login request types has drawbacks in terms of type-safety, and no inherent benefits. We always decode JSON anyway. Hence renaming to Type.LoginFromJSON. Code that directly uses LoginTypePassword with parsed data can still use Login. * Removed a TODO for SSO. This is already tracked in #1297. * httputil.UnmarshalJSON is useful because it returns a JSONResponse. This change is intended to have no functional changes. * Support login tokens in User API. This adds full lifecycle functions for login tokens: create, query, delete. * Support m.login.token in /login. * Fixes for PR review. * Set @matrix-org/dendrite-core as repository code owner * Return event NID from `StoreEvent`, match PSQL vs SQLite behaviour, tweak backfill persistence (#2071) Co-authored-by: kegsay <[email protected]> Co-authored-by: Neil Alexander <[email protected]>

compgeniuses · 2022-04-04T06:06:19Z

I am curious About one thing, Why is this still in Opn state, if it has been done and merged, or was it only components of its implementation that were implemented.

If an issue was implemented, and merged, or Done, should it not be marked as closed state, so that other people could know its done.

Or maybe Add new tags like.

Partially-Done
Being-implemented
etcetra

tommie · 2022-04-04T08:08:23Z

It's not done. This was only a part of it. Using tags sounds like a good idea.

The next step is rebasing and creating the next PR from https://github.com/tommie/dendrite/commits/loginsso. @kegsay also asked me for some post-merge cleanups in #2014, so that also needs to get done.

compgeniuses · 2022-04-07T08:02:08Z

ok sure

scatterd · 2022-05-22T18:57:01Z

@tommie can you use some help with this? I built dendrite docker images in order to run my own instance. SSO is a feature I'd very much appreciate. I'm a newbie here on github and with Go as well, so I'm not entirely sure I can be very helpful. However, I do have a ready-to-go test environment and could at least help testing the code by integrating some SSO provider.

But I'm open for other tasks as well should you come up with a better idea.

tommie · 2022-05-22T19:59:17Z

The sad truth is I only use Dendrite for family, so the lack of SSO was solved by hard-coding users and passwords. :) So my intrinsic motivation disappeared after the initial work.

Thanks for the offer to help out. You can play around with https://github.com/tommie/dendrite/commits/loginsso and see what's missing there. PR #2014 was a base of that branch, but it contains the actual OAuth-logic.

Since there are multiple people who want this, I really should take the time to get the ball rolling on a new PR. I'll have a look tomorrow to assess how much merge conflict there is in the branch. (Also: the spec may have moved since I last looked at it, but that's probably a secondary concern. I don't remember if Sytests cover this functionality well.)

scatterd · 2022-05-22T22:10:42Z

The sad truth is I only use Dendrite for family, so the lack of SSO was solved by hard-coding users and passwords. :) So my intrinsic motivation disappeared after the initial work.

That's something I'm familiar with ;) Btw I too intend to use it for friends and family only at the moment.

Thanks for the offer to help out. You can play around with https://github.com/tommie/dendrite/commits/loginsso and see what's missing there. PR #2014 was a base of that branch, but it contains the actual OAuth-logic.

Sure, I will take a look. Do not expect anything coming from this but I'll see what I can do.

Since there are multiple people who want this, I really should take the time to get the ball rolling on a new PR. I'll have a look tomorrow to assess how much merge conflict there is in the branch. (Also: the spec may have moved since I last looked at it, but that's probably a secondary concern. I don't remember if Sytests cover this functionality well.)

I really appreciate that, thanks!

@anandv96

This is forked from @anandv96's matrix-org#1374. Closes matrix-org#1297.

tommie · 2022-05-23T14:49:34Z

Rebased my branch on main: tommie@c9ad720

The only change was that accountDB seems to not be leaking into clientapi/ nowadays. Using exposed UserAPI functions instead.

Looking through the code, it looks like account registration is missing. Synapse performs registration implicitly on SSO login, so it's a matter of creating an account, device and ID association if the SSO ID isn't recognized: https://github.com/matrix-org/synapse/blob/a00462dd9927558532b030593f8914ade53b7214/synapse/handlers/sso.py#L372

The code currently uses 3PID to store this association, but Synapse actually has a separate table for the (auth provider, 3PID) -> MXID mapping. I wonder if that matters: could the same email address be used for different accounts at different auth providers? Anyway, it also seems Synapse allows the user to not associate an email 3PID with the account, so perhaps this should be kept separate for that reason. Since the current code doesn't ensure the email received from IdP is verified, this needs to change. BTW, I'm not sure using email is the right approach for this assocation. Using the "sub" attribute is more stable: https://openid.net/specs/openid-connect-core-1_0.html#UserInfo

A complication is that Synapse also provides a way for the user to confirm what the localpart should be. See https://github.com/matrix-org/synapse/blob/a00462dd9927558532b030593f8914ade53b7214/synapse/handlers/sso.py#L569, https://github.com/matrix-org/synapse/blob/a00462dd9927558532b030593f8914ade53b7214/synapse/handlers/sso.py#L816 and https://github.com/matrix-org/synapse/blob/003cc6910af177fec86ae7f43683d146975c7f4b/synapse/res/templates/sso_auth_account_details.html

Essentially, SSO has its own registration sub-flow, separate from /register, and it will require a fair bit of more code to support: https://github.com/matrix-org/synapse/blob/a00462dd9927558532b030593f8914ade53b7214/synapse/handlers/sso.py#L542

I added a TODO-point for now: https://github.com/tommie/dendrite/blob/loginsso/clientapi/routing/sso.go#L178

Another thing is that only GitHub is implemented as an auth provider. That's probably good as a start. A smaller thing that's missing is support for OpenID Connect discovery URLs. It's perhaps more of a "nice to have" to make configuration/extension simpler.

Test coverage of SSO in SyTest is minimal. It only checks that m.login.sso is announced: https://github.com/matrix-org/sytest/blob/develop/tests/12login/02cas.pl

There are tests called "SSO", but they're really CAS tests: https://github.com/matrix-org/sytest/blob/develop/tests/10apidoc/13ui-auth.pl

It would be nice to have Sytests for this, including a fake Oauth2 server, to ensure it works. But my Perl isn't good enough for it to be worth it for me to write, I think.

SyTest requires this change: tommie/sytest@dbf9bb1

Summary It's certainly not done yet. :)

tommie · 2022-05-23T16:19:06Z

Implemented separate SSO association storage and non-interactive account registration.

scatterd · 2022-05-24T18:48:20Z

Awesome! Thanks for taking the time to work on this :)

While I can't answer your questions concerning the implementation I instead tried to run your code. This config snippet gets dendrite up and running.

client_api:
  login:
    sso:
      enabled: true
      providers:
        - type: github
          id: github
          name: github.com
          oidc:
            client_id: abc123
            client_secret: abc123

But at this point I'm missing the redirectUrl. As far as I can tell the config key doesn't exist yet. Please correct me if I'm wrong.

tommie · 2022-05-24T19:48:51Z

The redirect URL is indeed internal. Synapse places it under /_synapse/client/oidc/callback, but I just placed it next to the speced redirect endpoint: https://github.com/tommie/dendrite/blob/loginsso/clientapi/routing/routing.go#L566

So that would be /_matrix/client/v3/login/sso/callback to register with GitHub.

What we send to the OIDC provider is that: https://github.com/tommie/dendrite/blob/loginsso/clientapi/routing/sso.go#L81

The callback will in turn redirect to the redirect_url provided by the client: https://github.com/tommie/dendrite/blob/loginsso/clientapi/routing/sso.go#L48

tommie · 2022-05-24T21:55:14Z

Looks like the client secret isn't sent when requesting the access token. I'm reworking the OIDC/OAuth2 bits. The code states GitHub is an OpenID Connect provider, which it isn't. It only supports OAuth2.

tommie · 2022-05-25T16:40:11Z

Made some fixes.

I remembered that I saw Complement being a Go replacement for SyTest, so I've been hacking on that:

Created an HTTP mock server.
Converted the trivial has-SSO-flow from SyTest.
Added a full SSO OIDC integration test.
Fixed things that were broken.

There's now a non-trivial chance logging in with Google might work, so I created a draft PR: #2492

Edit I'm not adding a test for GitHub since there are hard-coded URLs, and I don't feel like making the URLs configurable just for testing that provider.

scatterd · 2022-05-27T07:25:46Z

Alright, I've done some tests with Google login.

I tried Element desktop but it wouldn't even query the available login schemes and I couldn't find a quick way to hardcode the URL. So I gave up on that one for the moment.

Then I switched to my mobile and tested with syphon. That didn't work either. But the app is currently in open alpha and from what I see it's the client at fault. I might open an issue over there later on.

Now it got interesting with element.io
Google login get's advertised on the login screen and the client seems to do what it should. Here's an excerpt from my reverse proxy log.

GET /_matrix/client/r0/login
GET /_matrix/client/r0/login/sso/redirect/google?redirectUrl=https://app.element.io...

Then after logging in with Google comes the callback:

GET /_matrix/client/v3/login/sso/callback?provider=google&state=...

And this request doesn't contain the oidc_nonce cookie. So everything I get is

{"errcode":"M_MISSING_ARGUMENT","error":"no nonce cookie: http: named cookie not present"}

It's due to sameOrigin being set to strict. But that's where I'm stuck because the domain set in the cookie and the request's target domain are the same. If I set sameOrigin to none I do not comply with Google's validation rules anymore. At this point I have to admit that I haven't read the OAuth/OIDC specification yet. Guess it's time to do that now. It might make things clearer for me.

Anyways, if anyone visiting this issue is interested in doing it's own test here is the config I used:

client_api:
  login:
    sso:
      enabled: true
      callback_url: https://example.com/_matrix/client/v3/login/sso/callback
      providers:
        - id: google
          type: oidc
          name: google.com
          oidc:
            client_id: *******************.apps.googleusercontent.com
            client_secret: *******************
            discovery_url: https://accounts.google.com/.well-known/openid-configuration

tommie · 2022-05-27T07:46:30Z

Thanks for the testing!

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite

We have a navigation event or a redirect after a navigation event sending us to the callback, depending on provider.

Following the link in httpwg/http-extensions#2104:

https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-rfc6265bis-10#section-5.2 says

A request is "same-site" if the following criteria are true:

The request is not the result of a cross-site redirect. That is,
the origin of every url in the request's url list is same-site
with the request's current url's origin.

So if the end of OAuth2 is a POST to IdP, with a redirect to Dendrite, that's not same-site.

This is what Synapse sets:

Max-Age=3600; Path=/_synapse/client/oidc; HttpOnly; Secure; SameSite=None

In fact, it also sets a oidc_session_no_samesite cookie without the SameSite attribute and Secure. I'm assuming that's some transition workaround. Confirmed here: https://github.com/matrix-org/synapse/blob/6ff99e3bea481790782c252c5433e9a88f65c4b0/synapse/handlers/oidc.py#L55

I'm hoping we don't need that workaround. I'll get that fixed.

matrix-org#1297 (comment)

scatterd · 2022-05-27T09:08:36Z

I see, that's why it's not sameSite. That does make sense.

Your latest commit fixed the nonce cookie problem for me. Thanks for that! Now I get the error message again saying

failed to retrieve OIDC access token
[...] because it doesn't comply with Google's OAuth 2.0 policy [...]

I'm here right now.

https://developers.google.com/identity/protocols/oauth2/web-server says client_id and redirect_uri are required auth parameters. Shouldn't they be part of the query?

query="map[authuser:[0] code:[...some string...] prompt:[none] provider:[google] scope:[email profile https://www.googleapis.com/auth/userinfo.profile https://www.googleapis.com/auth/userinfo.email openid] state:[...same as nonce...]]"

I'm not sure, might as well be mistaken. It would be nice if Google gave me a hint more useful.

tommie · 2022-05-27T10:12:02Z

The redirect_uri, client_id and others are sent in the POST body: https://github.com/tommie/dendrite/blob/loginsso/clientapi/auth/sso/oauth2.go#L119

More specifically https://developers.google.com/identity/protocols/oauth2/web-server#exchange-authorization-code

(I don't really understand why they require the redirect_uri there, since it's never returned.)

BTW, it's actually the OIDC we're using: https://developers.google.com/identity/protocols/oauth2/openid-connect#exchangecode

This is what my uncommitted test case sends:

client_id=aclientid&client_secret=aclientsecret&code=acode&grant_type=authorization_code&redirect_uri=%2F_matrix%2Fclient%2Fr0%2Flogin%2Fsso%2Fcallback%3Fprovider%3Dgoogle

It seems to be a URI with only a path, which is because req.URL doesn't contain the host: https://github.com/tommie/dendrite/blob/loginsso/clientapi/routing/sso.go#L181

Will fix. :)

tommie · 2022-05-27T12:56:27Z

Maybe we should move the testing discussion to #2492 to keep the noise down on this FR.

tbrandirali · 2022-12-30T15:02:27Z

What the state of this feature? It's been pretty quiet for a while here...

genofire · 2022-12-30T18:42:11Z

#2492 was closed :(

compgeniuses · 2022-12-31T06:28:05Z

so basically completed.

tommie · 2022-12-31T08:21:22Z

#2492 was killed, and external contributions have been shelved:

We recently updated our contributing guidelines. This PR is being closed because it isn't a feature we want to maintain going forwards. If you need this feature, it is possible to have a sidecar process handle registration, which upon success registers an account on Dendrite.

When we have more bandwidth as a team, we would be very interested in supporting this natively.

@anandv96

This is forked from @anandv96's matrix-org#1374. Closes matrix-org#1297.

matrix-org#1297 (comment)

VPaulV · 2023-06-23T11:32:38Z

Hi guys! Do we have sso login working after 6880886?

ghost · 2023-07-12T14:31:28Z

Hi guys! Do we have sso login working after 6880886?

Do we have any update on this, mayhaps?

Rexogamer · 2023-07-13T09:48:19Z

It's listed as closing this issue so I'd imagine the answer is yes?

disconn3ct · 2023-07-13T13:17:08Z

@Rexogamer if you read above, you will see #1297 (comment)

It is closed because they are not accepting external contributors. So we're all stuck with full-force Matrix or going back to Discord..

robcohen · 2024-01-17T15:27:01Z

So no official update on this feature in almost a year?

kegsay added help wanted More difficult than good-first-issue but not impossible! are-we-synapse-yet This issue or PR involves Sytests in AWSY labels Aug 24, 2020

kegsay assigned anandv96 and unassigned anandv96 Sep 1, 2020

anandv96 mentioned this issue Sep 2, 2020

[WIP] Implement m.login.token and SSO login initial changes #1374

Closed

2 tasks

anandv96 removed their assignment Sep 26, 2021

tommie mentioned this issue Sep 26, 2021

Support for m.login.token #2014

Merged

2 tasks

tommie added a commit to tommie/dendrite that referenced this issue Sep 27, 2021

Support for m.login.sso.

f2bccd7

This is forked from @anandv96's matrix-org#1374. Closes matrix-org#1297.

kegsay mentioned this issue Jan 21, 2022

support other forms of login other than password, depending on config options #2026

Closed

ghost mentioned this issue Mar 19, 2022

Document configuration of OpenID Connect #2175

Open

tommie added a commit to tommie/dendrite that referenced this issue May 23, 2022

Support for m.login.sso.

4a4e7d6

This is forked from @anandv96's matrix-org#1374. Closes matrix-org#1297.

tommie added a commit to tommie/dendrite that referenced this issue May 27, 2022

Change cookie oidc_nonce to SameSite=None.

618e18f

matrix-org#1297 (comment)

samip5 pushed a commit to samip5/dendrite that referenced this issue Jan 6, 2023

Support for m.login.sso.

6880886

This is forked from @anandv96's matrix-org#1374. Closes matrix-org#1297.

samip5 pushed a commit to samip5/dendrite that referenced this issue Jan 6, 2023

Change cookie oidc_nonce to SameSite=None.

6c25681

matrix-org#1297 (comment)

This was referenced Oct 31, 2024

Implement SSO for logins element-hq/dendrite#1297

Open

support other forms of login other than password, depending on config options element-hq/dendrite#2026

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SSO for logins #1297

Implement SSO for logins #1297

kegsay commented Aug 24, 2020 •

edited

Loading

anandv96 commented Aug 31, 2020

duritong commented Nov 19, 2020

olanod commented Aug 24, 2021

tommie commented Sep 26, 2021

ghost commented Nov 23, 2021

tommie commented Nov 23, 2021

ynerant commented Feb 2, 2022

kegsay commented Feb 4, 2022

compgeniuses commented Apr 4, 2022

tommie commented Apr 4, 2022

compgeniuses commented Apr 7, 2022

scatterd commented May 22, 2022

tommie commented May 22, 2022

scatterd commented May 22, 2022

tommie commented May 23, 2022

tommie commented May 23, 2022

scatterd commented May 24, 2022

tommie commented May 24, 2022

tommie commented May 24, 2022

tommie commented May 25, 2022 •

edited

Loading

scatterd commented May 27, 2022

tommie commented May 27, 2022

scatterd commented May 27, 2022

tommie commented May 27, 2022 •

edited

Loading

tommie commented May 27, 2022

tbrandirali commented Dec 30, 2022

genofire commented Dec 30, 2022

compgeniuses commented Dec 31, 2022

tommie commented Dec 31, 2022

VPaulV commented Jun 23, 2023

ghost commented Jul 12, 2023

Rexogamer commented Jul 13, 2023

disconn3ct commented Jul 13, 2023

robcohen commented Jan 17, 2024

Implement SSO for logins #1297

Implement SSO for logins #1297

Comments

kegsay commented Aug 24, 2020 • edited Loading

anandv96 commented Aug 31, 2020

duritong commented Nov 19, 2020

olanod commented Aug 24, 2021

tommie commented Sep 26, 2021

ghost commented Nov 23, 2021

tommie commented Nov 23, 2021

ynerant commented Feb 2, 2022

kegsay commented Feb 4, 2022

compgeniuses commented Apr 4, 2022

tommie commented Apr 4, 2022

compgeniuses commented Apr 7, 2022

scatterd commented May 22, 2022

tommie commented May 22, 2022

scatterd commented May 22, 2022

tommie commented May 23, 2022

tommie commented May 23, 2022

scatterd commented May 24, 2022

tommie commented May 24, 2022

tommie commented May 24, 2022

tommie commented May 25, 2022 • edited Loading

scatterd commented May 27, 2022

tommie commented May 27, 2022

scatterd commented May 27, 2022

tommie commented May 27, 2022 • edited Loading

tommie commented May 27, 2022

tbrandirali commented Dec 30, 2022

genofire commented Dec 30, 2022

compgeniuses commented Dec 31, 2022

tommie commented Dec 31, 2022

VPaulV commented Jun 23, 2023

ghost commented Jul 12, 2023

Rexogamer commented Jul 13, 2023

disconn3ct commented Jul 13, 2023

robcohen commented Jan 17, 2024

kegsay commented Aug 24, 2020 •

edited

Loading

tommie commented May 25, 2022 •

edited

Loading

tommie commented May 27, 2022 •

edited

Loading