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

Fix S3 private fetch and add prefix #20

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

afiqiqmal
Copy link

  • Fix error for cloud storage S3 if the upload file is private
  • Add prefix upload path

@maxeckel
Copy link
Owner

maxeckel commented Nov 2, 2021

Hey @afiqiqmal, thank you for the PR, that's really welcome!

I'm with you adding the prefix, but I'm not so sure about the private files. The Problem is, that the temporary url gets saved into the block and when it timed out, that block is basically useless and won't be able to display the upload.

@afiqiqmal
Copy link
Author

hmm.. For what i'm understand, if using S3, by default, the visibility of the storage is private. Basically on my case, i want to use private files in cloud. The problem is where when retrieving the private files without temporaryUrl cause a problem which S3 blocked public access.

Basically it is just an idea for my case problem. You may drop this PR if this PR is not suitable.

@maxeckel
Copy link
Owner

maxeckel commented Nov 6, 2021

Hey @afiqiqmal,

I think you have a solid point here, I'm just concerned about the images being not available anymore after the temporary url is expired. I think in most cases, when the files are private and not accessible through e.g. CloudFront CDN, people use a dedicated route which will "stream" the files from S3 through your application to your visitor.
I've seen this pattern used in cases, where the uploaded files should only be accessible for e.g. authenticated users.

I'm thinking about a more "general" approach, as people might not only use S3. For example, by default files are seen as public (to not break existing functionality), but in cases where they are private we could allow for a customization in how the URL for the image should be generated. Maybe something like in spaties medialibrary (https://spatie.be/docs/laravel-medialibrary/v9/advanced-usage/generating-custom-urls). Even though they don't need to worry about the temp url issue, as the url is not persisted (as far as I know).

What do you think about it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants