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

getAwsTemporaryUrl() method does not pass the $options to $client->createPresignedRequest() #41143

Closed
saeeddirect1 opened this issue Feb 21, 2022 · 8 comments

Comments

@saeeddirect1
Copy link

  • Laravel Version: 9.1.0
  • PHP Version: 7.4.4
  • Database Driver & Version: MariaDB 10.3

Description:

Some Background: We wanted to create S3 temporaryURL for future such as a temporary signed url that is allowed in a specific time window e.g. tomorrow 00:00 to 23:00. Upon research, we found put that Aws S3 allows "start_time" value to be set when sending presigned url request.

We know that temporaryUrl() method on the Storage disk takes the $options array and then passes it to Illuminate/Filesystem/FilesystemAdapter::getAwsTemporaryUrl() , BUT the thing is that inside the getAwsTemporaryUrl(), $options are passed only to the $client->getCommand() and NOT $client->createPresignedRequest()

hence passing 'start_time' key in $options has no effect.

We are currently solving the issue for ourselves by creating custom client and passing the options.

$uri = $client->createPresignedRequest(

Steps To Reproduce:

@driesvints
Copy link
Member

getAwsTemporaryUrl doesn't exists in Laravel v9. What method do you mean?

@driesvints
Copy link
Member

If you pass $options as the third param for createPresignedRequest does it work for you?

@aneesdev
Copy link

aneesdev commented Feb 21, 2022

getAwsTemporaryUrl doesn't exists in Laravel v9. What method do you mean?

he meant in laravel 8.x.

@aneesdev
Copy link

aneesdev commented Feb 21, 2022

If you pass $options as the third param for createPresignedRequest does it work for you?

yes we tried to add it like this and it worked

$uri = $client->createPresignedRequest(
    $command, $expiration, ['start_time' => '1645459200'] // <- $options array
)->getUri();

@aneesdev
Copy link

aneesdev commented Feb 21, 2022

@driesvints same thing in Laravel 9.x... the temporaryUrl() method takes $options array but doesn't pass it to createPresignedRequest() method

@aneesdev
Copy link

aneesdev commented Feb 21, 2022

@driesvints maybe we can allow a fourth argument $presignedOptions array instead of existing $options which gets merge with:

$command = $client->getCommand('GetObject', array_merge([
            'Bucket' => $adapter->getBucket(),
            'Key' => $adapter->getPathPrefix().$path,
        ], $options)); 

because maybe $options which are merged in $client->getCommand('GetObject') is different and createPresignedRequest() method takes different

@saeeddirect1
Copy link
Author

getAwsTemporaryUrl doesn't exists in Laravel v9. What method do you mean?

My bad, I meant 8.x

But even in 9.x, $options array is not passed to createPresignedRequest() in temporaryUrl() in AwsS3v3Adapter::class

@driesvints
Copy link
Member

This sounds more like a feature request. Feel free to send in a pr to 9.x 👍

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

No branches or pull requests

3 participants