Permit service_name in direct uploads

[gmcgibbon]

Summary

For apps with multiple services (eg. a public one and a private one), they should be able to choose which service they want to directly upload to.

How should we expose this in the JS API? Currently, you use direct_upload: true to denote a direct upload file field. I was thinking of allowing something along the lines of direct_upload: { service: "x" }. We could also try to infer it from the ActiveStorage::Attached::One/Many object, but I don't see an easy way of doing that. WDYT?

cc @javan @georgeclaghorn

[javan]

For apps with multiple services (eg. a public one and a private one), they should be able to choose which service they want to directly upload to.

Making that choice param / data-* attribute based means the app isn't really choosing, the client is. Someone can open the web inspector, tamper with the params, and upload files to a different service. I think that logic should live firmly in the app itself, but I'm not familiar enough with the recent multi-service changes to suggest how.

[gmcgibbon]

Someone can open the web inspector, tamper with the params, and upload files to a different service. I think that logic should live firmly in the app itself, but I'm not familiar enough with the recent multi-service changes to suggest how.

So we do have a validation on blobs that would restrict the value to a valid service, so there's no chance of malformed data. We could alternatively make separate direct upload endpoints for each service, but that would likely be a breaking change. I think either way the client has to ultimately indicate which service to directly upload to (unless it is the default one).

[javan]

That only validates that the service exists if I'm reading correctly, and the data could still be tampered with to make it reference the wrong service. Like, I could fiddle with the params and put files in your private service instead of the public service.

[gmcgibbon]

That only validates that the service exists if I'm reading correctly, and the data could still be tampered with to make it reference the wrong service

Ah, good catch. Yes, that could definitely be abused if the client knew names of other buckets. I'll think about this a little more and see if I can make everything server-side. Thanks for the feedback! ❤️

[gmcgibbon]

This is tough because service names are specified in attachment associations and the direct upload will create the blob before the attachment is created. I thought of signing the service_name, but I think there's still room for exploitation if a form has two direct upload attachment fields going to different services, they could be switched.

[rails-bot]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[santib]

That only validates that the service exists if I'm reading correctly, and the data could still be tampered with to make it reference the wrong service. Like, I could fiddle with the params and put files in your private service instead of the public service.

@javan Sorry I'm late, but I think that as longs as services are private, attackers won't be able to upload there (because they would be missing a secret/signature), right? So maybe there is no a real need to encrypt things and the solution would be simpler

Update: I just realized that the request asking for the upload url is the one specifying the service name, so yes we need to make sure it doesn't get tampered 👌