Websocket for provider based upload progress is not secured
Initial checklist
- [x] I understand this is a bug report and questions should be posted in the Community Forum
- [x] I searched issues and couldn’t find anything (or linked relevant results below)
Link to runnable example
No response
Steps to reproduce
Currently there is no way to add authentication to this websocket connection https://github.com/transloadit/uppy/blob/main/packages/%40uppy/companion/src/server/socket.js#L20
Expected behavior
Allow to inject a auth callback in the configuration to verify who is connecting.
I'm happy to contribute a fix in case this is a valid concern from your perspective.
Actual behavior
No authentication possible.
Granular authentication is not something that we have needed to support - see #3527
However I think you can implement a custom getKey option that gets passed some context that you can use to authenticate uploads: https://uppy.io/docs/companion/#s3getkey-filename-metadata-req-
This is not really satisfying the security auditors of my client. The idea would be to close the Websocket right in case it's not valid.
Would you be interested if I extend the setupSocket function https://github.com/transloadit/uppy/blob/main/packages/%40uppy/companion/src/server/socket.js#L20 with a second paramter options with callback right at the beginning and make it async
wss.on('connection', (ws, req) => {
to
wss.on('connection', async (ws, req) => {
if (options.onConnection) {
await options.onConnection(ws, req);
}
I have two concerns about this:
- Even if we were to authenticate the websocket connection, it would still allow people to download files without authenticating, because a download/upload is first started by calling the
:provider/get:fileId. Then only after the download/stream has started, the client will make a second request to the websocket. - The second request cannot be done without the first request because there's a unique, secret UUID being generated and passed to the client (called
tokenin the code). So I don't think it's a problem that this second endpoint is not authenticated.
In the case of authenticating the first endpoint, I don't think we need to provide an API for that, because it's just an express endpoint: https://github.com/transloadit/uppy/blob/b1e33bcef7ae1ecfa85386a16447ae6d8649312a/packages/%40uppy/companion/src/companion.js#L239-L246
and I think when using companion as an express middleware, one can just create an authentication mdidleware before attaching companion, e.g.:
app.use('/:providerName/get/:id', myAuthMiddleware)