Move token sending to authorization header instead of body parameter #1337
Conversation
filmaj
added
enhancement
| @@ -521,9 +522,6 @@ describe('WebClient', function () { | |||
| const file = parts.files[0]; | |||
| // the filename is picked up from the the ReadableStream since it originates from fs | |||
| assert.include(file, { fieldname: 'someBinaryField', filename: 'train.jpg' }); | |||
|
|
|||
| assert.lengthOf(parts.fields, 1); | |||
| assert.deepInclude(parts.fields, { fieldname: 'token', value: token }); | |||
There was a problem hiding this comment.
With the token no longer a part of the request body, this part needed removal.
packages/web-api/src/WebClient.ts
Outdated
| @@ -183,6 +183,9 @@ export class WebClient extends Methods { | |||
| this.logger = getLogger(WebClient.loggerName, logLevel ?? LogLevel.INFO, logger); | |||
| } | |||
|
|
|||
| // eslint-disable-next-line no-param-reassign | |||
| if (!headers.Authorization) headers.Authorization = `Bearer ${this.token}`; | |||
There was a problem hiding this comment.
this.token can be absent. For instance, some API calls (such as oauth.v2.access with code parameter, openid.connect.token with code parameter, api.test) do not require token in the authorization header. Can you update the logic to check if this.token exists too?
| @@ -425,7 +426,7 @@ describe('WebClient', function () { | |||
| it('should properly serialize simple API arguments', function () { | |||
| const scope = nock('https://slack.com') | |||
| // NOTE: this could create false negatives if the serialization order changes (it shouldn't matter) | |||
| .post(/api/, 'token=xoxb-faketoken&team_id=T12345678&foo=stringval&bar=42&baz=false') | |||
| .post(/api/, 'team_id=T12345678&foo=stringval&bar=42&baz=false') | |||
| .reply(200, { ok: true }); | |||
| return this.client.apiCall('method', { foo: 'stringval', bar: 42, baz: false, team_id: 'T12345678' }) | |||
There was a problem hiding this comment.
Can we override the token in the authorization header if a developer passes a token like this.client.apiCall('method', { token: 'xoxb-' })? Switching tokens is not a rare use case. For instance, the singleton WebClient does not hold any token and your app utilizes multiple user tokens for user related API calls.
…ll() method options parameter.
|
@seratch I believe I have addressed your comments, and also added another test for the Authorization header overriding behaviour via |
Fixes #1132
I also removed some trailing whitespace because that's the kind of nitpicky dude I am