Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Provide ability to login using CAS #295

Merged
merged 6 commits into from Oct 10, 2015
Merged

Provide ability to login using CAS #295

merged 6 commits into from Oct 10, 2015

Conversation

stevenhammerton
Copy link
Contributor

Initial support for CAS login.

Important! - Will login as the user returned from CAS. So if that user already exists, CAS login will login with that account. If no user yet exists with the user returned from CAS, we register and then login.

Doesn't handle CAS logout or anything like that yet, just simple login.

There will be further pull requests for matrix-js-sdk and matrix-react-sdk to show support there.

I've tested this locally using OpenMarket CAS and manually hitting APIs and it appears to work.

Any questions, let me know.

Signed-off-by: Steven Hammerton steven.hammerton@openmarket.com

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@erikjohnston
Copy link
Member

Marrixbot: ok to test

@@ -67,6 +77,16 @@ def on_POST(self, request):
"uri": "%s%s" % (self.idp_redirect_url, relay_state)
}
defer.returnValue((200, result))
elif self.cas_enabled and (login_submission["type"] ==
LoginRestServlet.CAS_TYPE):
url = "%s/proxyValidate" % (self.cas_server_url)
Copy link
Member

Choose a reason for hiding this comment

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

FYI, there should be a comma after self.cas_server_url to make it a tuple, otherwise the brackets don't actually do anything here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code has changed in 059bb26.

@erikjohnston
Copy link
Member

This looks perfectly reasonable once requests has been swapped out with an async alternative :)

@erikjohnston
Copy link
Member

LGTM

@NegativeMjark
Copy link
Contributor

LGTM, other than the merge conflicts

@erikjohnston
Copy link
Member

@stevenhammerton If you could fix the conflict, we'll merge it in :)

@stevenhammerton
Copy link
Contributor Author

@erikjohnston - Fixed conflict by rebase, hope that's okay although I realise you'll probably need to give it a once over again because of that :(

@erikjohnston
Copy link
Member

@stevenhammerton Ah well, it's not a a particularly large PR at least.

Well, this LGTM! Thanks again :)

erikjohnston added a commit that referenced this pull request Oct 10, 2015
Provide ability to login using CAS
@erikjohnston erikjohnston merged commit 347aa3c into matrix-org:develop Oct 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants