-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add support for CAS auth #198
Conversation
@@ -23,22 +23,29 @@ sdk.loadModule(require('../modules/VectorConferenceHandler')); | |||
|
|||
var lastLocationHashSet = null; | |||
|
|||
|
|||
function parseQueryParams(location) { |
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.
AFAIK vector will be passed through browserify which supports core node moules so you can just do:
var qs = require("querystring");
var params = qs.parse("foo=bar&baz=3");
console.log(params.foo) // "bar"
Using this is preferable because it handles common edge cases which are easy to forget when rolling your own (e.g. foo=bar&baz
is valid (baz
is a 0-length string) but this implementation will not catch that).
@kegsay - Some additional commits added, basically moved all of the CAS logic down to react-sdk, keeping only UI stuff here. Also now using querystring to parse the name / value pairs from the fragment. |
var lastLocationHashSet = null; | ||
|
||
|
||
// We want to support some name / value pairs in the fragment | ||
// so we're re-using query string ike format |
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.
s/ike/like/
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.
Fixed in 293ee1b
LGTM other than very minor things. Pending @dbkr feedback. |
LGTM |
Nudge Looks like both reviewers are happy, any chance this can be merged soon :) |
Initial and basic CAS login.
Flow is:
Doesn't do anything with logout yet.
Also, the CAS login page will only be shown if CAS is the first login flow returned from the HS
Tested locally and works.
No doubt plenty of room for improvement on this works but it's a starting point and I'm happy to be given pointers for improvements before it get's merged!
Related to:
matrix-org/synapse#295
matrix-org/matrix-js-sdk#20
matrix-org/matrix-react-sdk#19
Signed-off-by: Steven Hammerton [email protected]