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

MSC1731: Mechanism for redirecting to an alternative server during SSO login #1731

Closed
wants to merge 1 commit into from

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Nov 24, 2018

Rendered

Full disclosure for this MSC: the reason for me proposing it is that I have a project which requires users to log in via SAML to a portal server; based on the results of the SAML authentication, the user should automatically be logged into one of several homeservers.


As well as returning a `loginToken` at step 5 above, we could add a
`homeserver` parameter. Clients would then add `/_matrix/client/...` to this
URL to form valid C-S endpoints for step 6 and onwards.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very important that clients don't blindly talk to the same HS as steps 1-3 else they will leak their login token. If we can make that impossible/difficult to do e.g. by setting a cookie on the target HS that would be preferable than just appending a query param, though admittedly harder to implement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and would break backwards-compat :(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, we don't require anyone to support cookies in the C-S API right now.

I'm not sure how much of a concern it is if the login token gets leaked to the portal server: I'm pretty sure there are other, more reliable ways for an evil portal server administrator to get the user's credentials, given the portal server is sitting in the middle of the SSO flow.

@turt2live
Copy link
Member

The generality of MSC1730 is a lot more appealing to me. It gives servers a way to redirect clients to other servers without complicating the login flow for the user (as SSO is some amount more complex for the user than logging in via other methods). Not to mention MSC1730 is a lot easier to implement for clients than supporting SSO auth (assuming they didn't go the route of fallback).

@ara4n ara4n added proposal-in-review proposal A matrix spec change proposal labels Nov 26, 2018
@richvdh
Copy link
Member Author

richvdh commented Nov 26, 2018

(I've updated the description of this MSC to give the reasons for proposing it)

@richvdh
Copy link
Member Author

richvdh commented Nov 26, 2018

The generality of MSC1730 is a lot more appealing to me. It gives servers a way to redirect clients to other servers without complicating the login flow for the user (as SSO is some amount more complex for the user than logging in via other methods). Not to mention MSC1730 is a lot easier to implement for clients than supporting SSO auth (assuming they didn't go the route of fallback).

I am strongly inclined to agree with you. For my current project, this MSC has appeal since it is a trivial enhancement to what is there already, but in the general case of usefulness to the matrix ecosystem, MSC1730 seems more powerful, and makes this one redundant.

@richvdh
Copy link
Member Author

richvdh commented Nov 26, 2018

@mscbot fcp cancel

@turt2live
Copy link
Member

I think you mean:

@mscbot fcp close

@anoadragon453
Copy link
Member

@mscbot fcp close

@mscbot
Copy link
Collaborator

mscbot commented Nov 26, 2018

Team member @anoadragon453 has proposed to close this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@mscbot mscbot added proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-close labels Nov 26, 2018
@richvdh richvdh added obsolete A proposal which has been overtaken by other proposals and removed T-Core disposition-close proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Dec 17, 2018
@richvdh
Copy link
Member Author

richvdh commented Dec 17, 2018

since #1730 has now completed its MSC, and nobody objected to this being closed, I'm assuming that people are happy with that.

@richvdh richvdh closed this Dec 17, 2018
@turt2live turt2live added the kind:feature MSC for not-core and not-maintenance stuff label Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature MSC for not-core and not-maintenance stuff obsolete A proposal which has been overtaken by other proposals proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants