-
Notifications
You must be signed in to change notification settings - Fork 257
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
feat: Adds pass-thru of optional 'loginHint' and 'idpScopes' params #238
Conversation
@@ -271,22 +271,26 @@ function convertOAuthParamsToQueryParams(oauthParams) { | |||
// Convert our params to their actual OAuth equivalents | |||
var oauthQueryParams = util.removeNils({ |
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.
This list was getting long, and to make it easier to compare to the docs I sorted it. Unfortunately this means updating all the unit tests that specified the expected order.
Params are now in alphabetical order except for scope, which is always last.
}); | ||
}); | ||
|
||
it('passes optional login_hint query param through', function() { |
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.
Here and below are the new tests.
Note there is no update to README because the README already doesn't document the optional params, instead referring to the API docs, so I followed that trend.
}); | ||
}); | ||
|
||
it('passes optional idp_scope query param through when given a string', function() { |
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.
idp_scope/idpScope probably only makes sense if the idp
param is also passed, but didn't see anything specific in the docs and left out any enforcement - we pass to the backend and let it react if it wants.
No description provided.