search

ahmedkandel / nova-s3-multipart-upload

A Laravel Nova resource tool to upload files directly to Amazon S3. You can (upload | download | delete) single, multiple, small or big files.
MIT License
22 stars 22 forks source link

1.3.0 release broke role based s3 authentication #23

Closed tractorcow closed 2 years ago

tractorcow commented 2 years ago

https://github.com/ahmedkandel/nova-s3-multipart-upload/commit/695f95221e6c38c99f7e52ce8510ddf9a2954453#diff-d381368b790cbf818204c612d2985bfbac9f3bffd3466c426ff1ffb7f02e9ae2R56-R63

Any application that uses role-based authentication would have worked prior to this change. The new version of this module discards the existing s3 client connected to the filesystem adapter, and constructs a new one, assuming key + secret authentication instead.

When we updated to 1.3 this of course immediately broke. :)

Could the current code work the same if we restore this line?

$this->s3Client = Storage::disk($this->tool->disk)->getDriver()->getAdapter()->getClient();
ahmedkandel commented 2 years ago

Laravel 9.x has migrated from Flysystem 1.x to 3.x so the Storage::disk($this->tool->disk)->getDriver()->getAdapter()->getClient() which works for Laravel 8.x is not available anymore starting from Laravel 9.x and will be Storage::disk($this->tool->disk)->getClient() here.

In order to support Laravel 8.x and 9.x we found it is more convenient to decouple from Laravel storage and instantiate S3Client using the disk config.

ahmedkandel commented 2 years ago

To solve your problem either you get the client from Laravel filesystem depending on the Laravel version or to use the S3Client directly but with optional credentials as discussed here.

PRs are welcome.

tractorcow commented 2 years ago

Thanks for the explanation. It makes sense to me. I'll see about writing up a PR to address.

Is likely we could still manually construct the client the way you are now, but tweak the credentials.

tractorcow commented 2 years ago

Please see https://github.com/ahmedkandel/nova-s3-multipart-upload/pull/25. Hope you find it helpful :)