Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multipart upload to work with only write access #13381

Open
3 tasks done
mohammedsahl opened this issue May 14, 2024 · 13 comments
Open
3 tasks done

Allow multipart upload to work with only write access #13381

mohammedsahl opened this issue May 14, 2024 · 13 comments
Assignees
Labels
feature-request Request a new feature Storage Related to Storage components/category

Comments

@mohammedsahl
Copy link

mohammedsahl commented May 14, 2024

Before opening, please confirm:

JavaScript Framework

Angular

Amplify APIs

Storage

Amplify Version

v6

Amplify Categories

storage

Backend

Amplify CLI

Environment information

  System:
    OS: macOS 14.4.1
    CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
    Memory: 13.75 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.12.1 - ~/.nvm/versions/node/v20.12.1/bin/node
    Yarn: 1.22.22 - ~/.nvm/versions/node/v20.12.1/bin/yarn
    npm: 10.5.0 - ~/.nvm/versions/node/v20.12.1/bin/npm
    pnpm: 9.1.1 - ~/.nvm/versions/node/v20.12.1/bin/pnpm
  Browsers:
    Chrome: 124.0.6367.202
    Safari: 17.4.1
  npmPackages:
    graphql-ttl-transformer-v2-beta: ^2.1.1 => 2.1.1 
    prettier: ^3.2.5 => 3.2.5 
    prettier-plugin-organize-imports: ^3.2.4 => 3.2.4 
  npmGlobalPackages:
    @angular/cli: 17.3.0
    @aws-amplify/cli: 12.12.0
    corepack: 0.17.0
    eslint: 8.57.0
    http-server: 14.1.1
    karma-cli: 2.0.0
    npm: 10.5.0
    prettier: 2.8.8
    yarn: 1.22.19

Describe the bug

Uploading images (or files too I'm not sure) greater than 5MB ends in a "403 Forbidden". The file uploads successfully (I can go to my bucket on S3 and download the uploaded file) but then the last HEAD method call ends in a 403. This doesn't happen with file sizes <5MB.

Expected behavior

Image upload should respond with 2xx

Reproduction steps

  1. Spin up a sample angular app
  2. Setup backend
  3. Make a request using uploadData({...}) with an image larger than 5MB
  4. See error

Code Snippet

// Put your code below this line.
return new Observable((subscriber) => {
      fetchAuthSession().then((cred) => {

        event.s3Object.identityId = cred.identityId;
        event.request = uploadData({
          path: `${keyPrefix}/${cred.identityId}/${randomFilename}`,
          data: file,
          options: {
            useAccelerateEndpoint: true,
            metadata: {
              fileName: json_encode(file.name),
              ...metadata,
            },
            contentType: file.type,
            onProgress(transferProgressEvent: TransferProgressEvent) {
              event.progressEvent = transferProgressEvent;
              subscriber.next(event);
            },
          },
        });

        event.request.result
          .then((result) => {
            event.progressEvent = null;
            event.s3Object.key = result.path;
            subscriber.next(event);
            subscriber.complete();
          })
          .catch((error: AxiosError | Error) => {
            // Log error using a logger :/ ...
            event.progressEvent = null;
            event.error = error;
            subscriber.error(event);
          });
      });
    })

Log output

This shows up in the chrome dev console
// Put your logs below this line

zone.js:2685 HEAD https://{bucket}.s3-accelerate.amazonaws.com/uploads/us-east-1%3A4cbb3bda-c2f5-4733-8e36-c6106b3bf0ea/7c30fde3-aa1f-464f-ba03-52102c31c499.jpeg 403 (Forbidden)

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

Note how every request before the HEAD was successful

Screenshot 2024-05-14 at 5 42 01 PM
@mohammedsahl mohammedsahl added the pending-triage Issue is pending triage label May 14, 2024
@cwomack cwomack added the Storage Related to Storage components/category label May 15, 2024
@cwomack cwomack self-assigned this May 15, 2024
@cwomack
Copy link
Contributor

cwomack commented May 17, 2024

Hello, @mohammedsahl and thanks for opening this issue. What's interesting here is that the multi-part upload is going to be triggered when the file size is larger than 5 MB. It's possible that the headObject call (here) may be failing due to the finalKey potentially missing the prefixes. We're going to proactively mark this as a bug and investigate further.

@cwomack cwomack added bug Something isn't working and removed pending-triage Issue is pending triage labels May 17, 2024
@ashika112
Copy link
Contributor

@mohammedsahl As followup can check the network tab between the two calls and see if there is a difference in requestURL between them?

@mohammedsahl
Copy link
Author

Hey @ashika112, I'm not sure which two calls you're referring to. Do you mean:

  1. The difference in requestURL between the 4MB upload and 6MB upload or
  2. the difference in requestURL between the last two network calls of the 6MB upload (I see an OPTIONS before the failing HEAD).

If it's case 1: I see that the requestURL has the ?uploads suffix for the 6MB upload and no suffix for the 4MB one. Also I notice for the 4MB upload the first two calls are OPTIONS and PUT. For the 6MB upload the first two calls are OPTIONS and POST

If it's case 2: I see no difference between the last two network calls of the 6MB upload

OPTIONS HEAD
Screenshot 2024-05-21 at 2 03 41 PM Screenshot 2024-05-21 at 2 03 46 PM

@ashika112
Copy link
Contributor

ashika112 commented May 22, 2024

Hey @mohammedsahl Thanks, Sorry for not being clear. I wanted to know about difference in Request URL between the last PUT request and HEAD. I tried uploading a 10MB file and i dont see the issue happening. I tried both jpeg and video.

@ashika112
Copy link
Contributor

Screenshot 2024-05-21 at 11 34 00 PM

@ashika112
Copy link
Contributor

@mohammedsahl Which version are you using? Also, just wanted to let you know in case you didnt know this, for path construction you could alternatively use the below (this is sample from my test code)

  const res = uploadData(
    {
      path: ({ identityId }) => `protected/${identityId}/${event.target.files[0].name}`,
      data: event.target.files[0]
    })

@ashika112 ashika112 self-assigned this May 22, 2024
@mohammedsahl
Copy link
Author

mohammedsahl commented May 22, 2024

Hey @ashika112 thanks for the tip. We already fetch the creds for other purposes so we'll go with that. But I'll keep it in mind :)

I have ~6.3.0 downloaded. But from my package-lock it seems I have 6.4.1

Screenshot 2024-05-22 at 9 27 20 AM

Also no difference between the last PUT request and HEAD besides PUT having partNumber and uploadId query params.

Screenshot 2024-05-22 at 9 25 42 AM

I guess one difference is that I'm using angular not vue but I doubt that's the root cause

@cwomack cwomack removed their assignment May 22, 2024
@ashika112
Copy link
Contributor

ashika112 commented May 22, 2024

@mohammedsahl Thanks for the version. You are right Vue vs Angular should not make a difference here.

The only other thing i could think about after looking into all screenshot is , IAM permission might be messed up here. It looks like you are using your own custom prefix, my guess would be in IAM opermission put might have been allowed but not HEAD which is need to for multipart validation. Can you check that and see how ur permission is setup?

Also, are you using Amplify CLI or Amplify Gen2 for your backend? Just wanted to double check here.

If you are interested in us taking a look at ur policy, can you redact and put ur policy created so i can help further :)
Another thing that could be helpful would be knowing your custom prefix usage.

@ashika112
Copy link
Contributor

I am removing bug label on this since we can reproduce and 403 shouldn't happen unless there is a issue in IAM policy. If as we deep dive we find a bug i will bring it back in.

@ashika112 ashika112 added pending-response Issue is pending response from the issue requestor and removed bug Something isn't working labels May 22, 2024
@mohammedsahl
Copy link
Author

Hey @ashika112 you're right it was the IAM permissions. We solved the 403 the issue by adding "s3:GetObject" to the policy. However that's not a viable fix for us since we only want users to add objects to the bucket and not read them. We noticed "s3:GetObjectAttributes" exists too but I get a 403 forbidden with that too.

@github-actions github-actions bot removed the pending-response Issue is pending response from the issue requestor label May 22, 2024
@ashika112
Copy link
Contributor

ashika112 commented May 22, 2024

@mohammedsahl Good to know that helped. I see your point with the permission. Let me look into the code and put some thought into this, see if we can change something here.

@cwomack cwomack changed the title Upload of image >5MB using uploadData() ends in 403 Forbidden error Allow multipart upload to work with only write access May 29, 2024
@cwomack cwomack added the feature-request Request a new feature label May 29, 2024
@cwomack
Copy link
Contributor

cwomack commented May 29, 2024

@mohammedsahl, marking this as a feature request at this point and updating title to better reflect the core issue/ask here to allow for users to add/write objects to the bucket and not read them.

@hisham
Copy link

hisham commented Jun 4, 2024

Thanks @cwomack - you might want to also mark this as a "VP" (version parity) issue between v5 and v6, as this was working just fine in v5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request a new feature Storage Related to Storage components/category
Projects
None yet
Development

No branches or pull requests

4 participants