filesUploadV2 does not foward token override argument to sub methods

[maclockard]

Packages:

Select all that apply:

• @slack/web-api
• @slack/rtm-api
• @slack/webhooks
• @slack/oauth
• @slack/socket-mode
• @slack/types
• I don't know

Reproducible in:

The Slack SDK version

"slack/web-api": "^6.9.0"

Node.js runtime version

v16.20.1

OS info

ProductName:            macOS
ProductVersion:         13.5
BuildVersion:           22G74
Darwin Kernel Version 22.6.0: Wed Jul  5 22:21:53 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6020

Bug:

The filesUploadV2 does not forward the token override argument passed into its options to all of the SDK calls it makes. See this block of code here: https://github.com/slackapi/node-slack-sdk/blob/%40slack/web-api%406.9.0/packages/web-api/src/WebClient.ts#L419-L436

The result is that is the token override has permissions the clients base token does not, some calls may unexpectedly fail or happen on behalf of another client.

[WilliamBergamin]

Hi @maclockard thanks for writing in 💯

This could definitely be a bug, could you provide me with more context on how this issue is being created?

For some context the Bolt JS framework will create an instance of the WebClient for each token it encounters, this allows methods such as filesUploadV2 to use same token between multiple requests

[maclockard]

So in this case we have a shared WebClient with no token defined. We use the token override argument so that requests can use a specific users credentials.

I can see an argument that one should maybe create a WebClient per request/token. In that case there should not be a token override argument at all, since making some requests with it vs the base token with no transparency to the caller feels ripe for security issues.

[github-actions]

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out. If you think this issue needs to be prioritized, please comment to get the thread going again! Maintainers also review issues marked as stale on a regular basis and comment or adjust status if the issue needs to be reprioritized.

[maclockard]

Still relevant

[filmaj]

Looking at this issue, I believe the problem is that we don't pass token to the following two calls:

• to the fetchAllUploadURLExternal method, which in turn calls files.getUploadURLExternal API, which does accept a token
• to the completeFileUploads method, which in turn calls files.completeUploadURLExternal API, which does accept a token

Since both of the above methods are private, I think changing these methods to accept the options passed into the main filesUploadV2 method as an additional argument is acceptable and would not be considered a breaking change. This way, these two methods could take into account any overridden token provided by the developer.

[filmaj]

I added tests exhibiting this failure to the fileUploadV2-token-override branch; will play around with solutions in there.

[filmaj]

FYI the fix here was released in v7 of web-api.