uppy icon indicating copy to clipboard operation
uppy copied to clipboard

Companion: Document and/or implement access control concept

Open cfra opened this issue 2 years ago • 6 comments

I am only just starting out with Uppy, so please excuse me if this is already documented or should be obvious.

First, let's briefly describe my requirements, I would assume those are pretty common:

  1. There is a web application with different users that can authenticate.
  2. These users shall be able to upload large files from their computer or from remote URLs to a user-specific tree in an S3 bucket.
  3. The users must not be able to upload to a tree that belongs to another user.
  4. Unauthenticated third parties must not be able to upload anything.

It seems to me as if a standard deployment of the Companion with S3 credentials will allow anybody with access to the companion to perform arbitrary uploads to the S3 buckets without any authentication.

That doesn't seem like a good idea, but looking through the Companion documentation, I could not find much information how I can configure any form of access control.

Could you provide some pointers how companion can be deployed more securely, for example by requiring client requests to be signed with a secret that is only known to the trusted web application, or any other method?

cfra avatar Mar 02 '22 13:03 cfra

For making companion secure, you should use the secret and uploadUrls options.

allow anybody with access to the companion to perform arbitrary uploads to the S3 buckets without any authentication.

Perhaps I misunderstand but you could put Uppy behind your own authentication system, only allow users that are logged-in to upload with Uppy?

[...] from remote URLs to a user-specific tree in an S3 bucket

AFAIK this is something you should handle on server-side. @mifi can this be done with companionHeaders?

Murderlon avatar Mar 03 '22 05:03 Murderlon

Hi. I think there is currently no documented way of providing such access control in companion. One possible way of solving it is by providing the s3.getKey(req, filename, metadata) option to companion. Then provide a custom authentication token inside the request from the client, and validate that token in your own getKey function. Throwing an error if the token is not valid will abort the request. I can see that getKey does only support a synchronous functions, so there would be limited ways in which the token can be validated. However I think we can also support promise returning functions by simply adding the await keyword here: https://github.com/transloadit/uppy/blob/32ea099a93f2a3306f665eb0d62e99b956fc729e/packages/%40uppy/companion/src/server/Uploader.js#L613 ...as well as other places where getKey is called. I can make that change if needed.

mifi avatar Mar 03 '22 12:03 mifi

Thank you for providing this information and for confirming that request validation in the getKey method would indeed be the way to go.

For now, I think I will resort to running my own implementation of the required functionality, because I don't feel that I have a good understanding of everything that Companion can do and the associated security implications of running Companion with write access to my buckets.

cfra avatar Mar 03 '22 19:03 cfra

Hey @mifi

I just wanted to double-check my understanding from this thread - I'm also new to Companion and I'm not totally clear about the best way to deploy it.

Are we saying that there is no native way to prevent unauthorised 3rd parties from using companion to upload files? As far as I understand, the secret and uploadUrls only ensure that files are uploaded to the correct destination. It doesn't prevent, for example, someone unauthorised using your companion server to flood your backend with file uploads?

Perhaps I misunderstand but you could put Uppy behind your own authentication system, only allow users that are logged-in to upload with Uppy?

Do you have any pointers on the best way to do this? Presumably, we could deploy a reverse proxy that would validate JWT tokens upstream, but it's not clear which endpoints need to be unauthenticated in order for companion server to engage in the oAuth flow with providers?

I feel like I must be missing something because presumably its pretty common that most users of companion don't want randoms to be able to upload files to their backend...

linfordbacon-ngenius avatar May 04 '22 09:05 linfordbacon-ngenius

Are we saying that there is no native way to prevent unauthorised 3rd parties from using companion to upload files? As far as I understand, the secret and uploadUrls only ensure that files are uploaded to the correct destination. It doesn't prevent, for example, someone unauthorised using your companion server to flood your backend with file uploads?

Hi. AFAIK uppy/companion was never designed to handle authentication and instead it's designed to accept files from anyone. I think someone flooding the backend, s3 bucket etc is considered a DoS attack, so it should instead be solved by scaling up companion and by DDoS protection measures.

Do you have any pointers on the best way to do this? Presumably, we could deploy a reverse proxy that would validate JWT tokens upstream, but it's not clear which endpoints need to be unauthenticated in order for companion server to engage in the oAuth flow with providers?

I think it could be easier if you run companion in the "plugging into existing express server" mode. Then you can add a middleware that will check and reject requests missing an authentication token. I'm also not 100% sure which endpoints need to be excluded from this check, because I think this is not something that we have been actively testing and supporting.

I need to discuss with the team whether we want to start actively supporting authentication for all uploads. If we want authentication support we need to provide mechanisms or documentation for how to secure:

  • companion's internal endpoints:
    • all endpoints except oauth callbacks and related endpoints
    • s3 signing
    • s3 multipart signing
  • companion's upload targets (companion would somehow need to pass credentials along to these and they would also need to verify the credentials):
    • tus server
    • multipart server

mifi avatar May 05 '22 19:05 mifi

Thanks, @mifi - that's really clarified the situation for me. I'll keep on the lookout for any developments in this area.

linfordbacon-ngenius avatar May 09 '22 12:05 linfordbacon-ngenius