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

MSC1721: Rename m.login.cas to m.login.sso #1721

Merged
merged 2 commits into from Dec 1, 2018

Conversation

Projects
None yet
7 participants
@richvdh
Copy link
Member

richvdh commented Nov 15, 2018

@richvdh richvdh changed the title Proposal to rename m.login.cas to m.login.sso MSC1721: Rename m.login.cas to m.login.sso Nov 15, 2018

@richvdh richvdh added the proposal label Nov 15, 2018

@turt2live turt2live added the T-Core label Nov 15, 2018

@turt2live
Copy link
Member

turt2live left a comment

generally seems straightforward to me. I assume implementations are recommended to support the CAS endpoints for now, and to advertise CAS and SSO login stages until clients can be reasonably updated.

`/_matrix/client/r0/login/cas/redirect`: they should issue a redirect to
their configured single-sign-on system.

4. Servers should probably rename the post-authentication callback endpoint

This comment has been minimized.

Copy link
@turt2live

turt2live Nov 15, 2018

Member

s/probably// imo

This comment has been minimized.

Copy link
@richvdh

richvdh Nov 22, 2018

Author Member

I've actually removed this section altogether. The server's behaviour needs to be completely different depending on whether it's handling a CAS or a SAML response, so it makes sense to leave it as /login/cas/ticket for a CAS response.


1. `m.login.sso` should be defined as a valid login type for return from `GET
/login`. (We should probably mention `m.login.cas` in the spec while we are
there).

This comment has been minimized.

Copy link
@KitsuneRal

KitsuneRal Nov 18, 2018

Member

Will it return m.long.sso along with m.login.cas or instead of it? I'd probably recommend "along" for the transition period (but I'd really be interested to know how many clients are even watching for m.login.cas - probably there's nothing worth discussing).

This comment has been minimized.

Copy link
@Half-Shot

Half-Shot Nov 19, 2018

Contributor

s/m.long.sso/m.login.sso

This comment has been minimized.

Copy link
@richvdh

richvdh Nov 19, 2018

Author Member

yup think it's best to return both for now.

riot-web has some very rudimentary support for m.login.cas, but that's it afaik

@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented Nov 22, 2018

This doesn't seem terribly contentious.

@mscbot fcp merge

@mscbot

This comment has been minimized.

Copy link
Collaborator

mscbot commented Nov 22, 2018

Team member @richvdh has proposed to merge 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

This comment has been minimized.

Copy link
Collaborator

mscbot commented Nov 26, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@mscbot

This comment has been minimized.

Copy link
Collaborator

mscbot commented Dec 1, 2018

The final comment period, with a disposition to merge, as per the review above, is now complete.

@anoadragon453 anoadragon453 merged commit 1e9a7d9 into master Dec 1, 2018

7 checks passed

ci/circleci: build-dev-scripts Your tests passed on CircleCI!
Details
ci/circleci: build-docs Your tests passed on CircleCI!
Details
ci/circleci: build-swagger Your tests passed on CircleCI!
Details
ci/circleci: check-docs Your tests passed on CircleCI!
Details
ci/circleci: validate-docs Your tests passed on CircleCI!
Details
docs Click details to preview the HTML documentation.
Details
swagger Click to preview the swagger build.
Details
@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented Dec 17, 2018

@richvdh richvdh deleted the rav/proposal/sso_login branch Dec 17, 2018

@ara4n

This comment has been minimized.

Copy link
Member

ara4n commented Dec 17, 2018

gah, very annoying that deleting the branch removes the rendered URL from the MSC: https://github.com/matrix-org/matrix-doc/blob/rav/proposal/sso_login/proposals/1721-rename-cas-to-sso.md

I'm going to hit the restore button if that's ok

@ara4n ara4n restored the rav/proposal/sso_login branch Dec 17, 2018

@richvdh

This comment has been minimized.

Copy link
Member Author

richvdh commented Dec 17, 2018

doh. though now that this has merged it might make more sense to change the link to point to master.

@turt2live

This comment has been minimized.

Copy link
Member

turt2live commented Jan 17, 2019

Merged 🎉

Spec PR was #1789

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.