39 query param support #47
Conversation
simplified test see #39
When requesting an access token (via `TokenCache` or via `getAccessToken`) one can now specify an object `queryParams` in the `OAuthConfig`. The given key-value pairs will be added to the reuqest as query parameters. `createAuthCodeRequestUri` accepts an optional parameter `queryParams`. The given key-value pairs will be added to the returned url string as query parameters. close #39
| getTokenInfo | ||
| } from '../../src/index'; | ||
|
|
||
| chai.use(chaiAsPromised); | ||
| const expect = chai.expect; | ||
|
|
||
| const port = '30001'; | ||
| const host = `http://127.0.0.1:${30001}`; |
src/oauth-tooling.ts
Outdated
|
|
||
| const promise = new Promise(function(resolve, reject) { | ||
|
|
||
| fetch(accessTokenEndpoint + '?realm=' + realm, { | ||
| const queryString = qs.stringify(Object.assign({ realm: realm }, queryParams)); |
There was a problem hiding this comment.
this is safe if queryParams is not set?
There was a problem hiding this comment.
It is, see examples in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign
"[...]null and undefined will be ignored."
There was a problem hiding this comment.
Object.assign({ realm }, queryParams));
or with spread:
const x = {
...{ realm },
...queryParams}
);There was a problem hiding this comment.
Good point, will tackle that.
test/unit/index.ts
Outdated
| redirectUri, queryParams); | ||
|
|
||
| // then | ||
| const expected = authorizationEndpoint + |
There was a problem hiding this comment.
Should we use template strings?
There was a problem hiding this comment.
Yes you are right, I would suggest this solution:
const expected = `${authorizationEndpoint}` +
`?client_id=${clientId}` +
`&redirect_uri=${redirectUri}` +
`&response_type=code` +
`&realm=${EMPLOYEES_REALM}` +
`&foo=bar`;cause with multi line template strings we have the problem that
whitespaces are not ignored. So that would destroy the formatting, right?
There was a problem hiding this comment.
True, alternative idea:
const expected = [
``,
``
].join();
use spread instead of object.assign see #39
|
👍 |
1 similar comment
|
👍 |
close #39