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

Downloading file with data returns a 403. Downloading to a local url works, generating a url works too. #3860

Closed
ipodishima opened this issue Sep 12, 2024 · 5 comments
Labels
question General question storage Issues related to the Storage category

Comments

@ipodishima
Copy link

Describe the bug

Hey

I noticed something really weird which took me a while to figure out (I didn't required to save the data to a file, so I was looking for other issues like misconfiguration).

After uploading a file, if you download it as data, you get a 403.

▿ StorageError: Received HTTP Response status code 403 Forbidden
Recovery suggestion: Make sure the user has access to the key before trying to download/upload it.
  ▿ accessDenied : 3 elements
    - .0 : "Received HTTP Response status code 403 Forbidden"
    - .1 : "Make sure the user has access to the key before trying to download/upload it."
    - .2 : nil

If I download the same file, same key, to a local url or if I generate a presigned url and download it, it works perfectly.

Steps To Reproduce

1.

Define storage like


import { defineStorage } from "@aws-amplify/backend";

export const projectsStorage = defineStorage({
  name: "project-files",
  access: (allow) => ({
    'projectData/*': [
      allow.authenticated.to(['read', 'write', 'delete']),
    ],
  })
});
  1. Upload a file
let dataString = "My Data"
let data = Data(dataString.utf8)
let uploadTask = Amplify.Storage.uploadData(
    path: .fromString("projectData/myFile.data"),
    data: data
)
  1. Download the file
let downloadTask = Amplify.Storage.downloadData(path: .fromString("projectData/myFile.data"))
let data = try await downloadTask.value
print("Completed: \(data)")

You'll get

▿ StorageError: Received HTTP Response status code 403 Forbidden
Recovery suggestion: Make sure the user has access to the key before trying to download/upload it.
  ▿ accessDenied : 3 elements
    - .0 : "Received HTTP Response status code 403 Forbidden"
    - .1 : "Make sure the user has access to the key before trying to download/upload it."
    - .2 : nil
  1. Use file
let downloadToFileName = FileManager.default.urls(
    for: .documentDirectory,
    in: .userDomainMask
)[0].appendingPathComponent("myFile.txt")

let downloadTask = Amplify.Storage.downloadFile(
    path: .fromString("projectData/myFile.data"),
    local: downloadToFileName,
    options: nil
)
try await downloadTask.value
print("Completed")

And it work perfectly.

NB: My workaround for now is

func downloadData(for key: String,
                  downloadProgressHandler: @escaping (Progress) -> Void) async throws -> Data {
    Logger.amplify.debug("Downloading data at \(key)")
    // For some reasons, this return a 403
    // let downloadTask = Amplify.Storage.downloadData(key: key)
    let destinationURL = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString)
    let downloadTask = Amplify.Storage.downloadFile(path: .fromString(key),
                                                    local: destinationURL)

    for await progress in await downloadTask.progress {
        Logger.amplify.trace("Download progress for \(key): \(progress)")
        downloadProgressHandler(progress)
    }
    try await downloadTask.value
    let data = try Data(contentsOf: destinationURL)
    Logger.amplify.trace("Download completed for \(key)")
    return data
}


### Expected behavior

Downloading data should not return a 403 when the resource is accessible

### Amplify Framework Version

2.39.0

### Amplify Categories

Storage

### Dependency manager

Swift PM

### Swift version

5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)

### CLI version

npx ampx --version 1.2.5

### Xcode version

Xcode 15.4 Build version 15F31d

### Relevant log output

```shell
<details>
<summary>Log Messages</summary>


INSERT LOG MESSAGES HERE
```

Is this a regression?

No

Regression additional context

I don't know, first time using it.

Platforms

iOS

OS Version

17.2

Device

Simulator

Specific to simulators

No

Additional context

No response

@github-actions github-actions bot added pending-triage Issue is pending triage pending-maintainer-response Issue is pending response from an Amplify team member labels Sep 12, 2024
@ruisebas ruisebas added storage Issues related to the Storage category question General question and removed pending-triage Issue is pending triage labels Sep 12, 2024
@ruisebas
Copy link
Member

ruisebas commented Sep 12, 2024

Hi @ipodishima, thanks for opening this issue.

I quickly tried on a simple project and was not able to reproduce this issue following your steps: downloadData(path:) completed successfully.

However, looking at your code snippets I noticed some things that might be causing your issue:

After calling uploadData(path:data:), you are not waiting for the task to complete

let uploadPath = try await uploadTask.value

If you immediately call downloadData(path:) after creating the uploadTask, the actual upload might have not finished yet, so you'd get an error. However this would likely give you a 404 error, so perhaps it's not the actual problem.

In your workaround function, it seems you were previously calling the deprecated downloadData(key:) function instead of downloadData(path:), which is the one you should use since you're using Gen2 and you've also uploaded the data using path instead of key.
This also can likely explain the 403 error, as using key will attempt to access the public/ folder by default, for which you did not define permissions in your backend.

Could you please confirm if either of these is the culprit? If the error still persists, then could you please provide the verbose logs when you perform the steps? You can enable it by doing

Amplify.Logging.logLevel = .verbose

Thanks!

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Sep 12, 2024
@ruisebas ruisebas added the pending-community-response Issue is pending response from the issue requestor label Sep 12, 2024
@ipodishima
Copy link
Author

Heyyyy

Thanks for the really quick response.

Yes, I do wait for the upload to complete, just forgot to paste it.

Tested with

        let downloadTask = Amplify.Storage.downloadData(path: .fromString(key))

And I don't get a 403!

However

Source code doesn't flag this as deprecated nor does the documentation says anything about public which I think is confusing
Screenshot 2024-09-12 at 22 16 21
Screenshot 2024-09-12 at 22 15 56

I just checked the documentation and actually my bad, did not saw that auto completion selected key instead of path and I failed here ahah

@github-actions github-actions bot added pending-maintainer-response Issue is pending response from an Amplify team member and removed pending-community-response Issue is pending response from the issue requestor labels Sep 12, 2024
@ipodishima
Copy link
Author

ipodishima commented Sep 12, 2024

Oh. Funny

In StorageCategoryBehavior protocol, it's flagged as deprecated

Screenshot 2024-09-12 at 22 21 00

Maybe that

public static internal(set) var Storage = StorageCategory()

should be

public static internal(set) var Storage: StorageCategoryBehavior = StorageCategory()

To get the compiler deprecation help?

[EDIT] I confirmed with using private let storage: StorageCategoryBehavior = Amplify.Storage instead of Amplify.Storagedirectly BUT you're losing default parameters

@ruisebas
Copy link
Member

Great to hear the issue's been solved!

And thanks for bringing up the function not being marked as deprecated on Xcode. It's probably related to what you've said and we just need to mark the actual implementation as deprecated as well.

Regarding the public/ folder, in case you're interested, that's just one of the folders that Amplify Gen1 defines by default. When using keys in the Amplify APIs, these folders were not meant to be included as part of the key but rather configured as accessLevels in the API options, with .guest being the default. But all of these is now deprecated and path is the way to go :)

We will take a look at ensuring these old APIs appear as deprecated when attempted to use, but in the meantime I'll close this issue as the actual error has been resolved.

Thanks!

@ruisebas ruisebas closed this as not planned Won't fix, can't repro, duplicate, stale Sep 12, 2024
@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Sep 12, 2024
Copy link
Contributor

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question General question storage Issues related to the Storage category
Projects
None yet
Development

No branches or pull requests

2 participants