kreait / laravel-firebase

A Laravel package for the Firebase PHP Admin SDK
https://github.com/kreait/firebase-php
MIT License
992 stars 163 forks source link

Add s3 path #169

Closed aessam13 closed 1 year ago

aessam13 commented 1 year ago

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Fixes # (issue) or Closes # (issue)

Checklist:

jeromegamez commented 1 year ago

Remote URLs are not necessarily S3 paths (you introduced just the https:// prefix).

That being said, I don't think the package should encourage using remote URLs to retrieve secure credentials (I'm not even sure the underlying SDK or the Google Auth library supports this).

One possibility could be to use the Laravel File Storage to have an abstraction that reads the file contents for the SDK, but the package doesn't support this yet.

You're welcome to create a PR (please with a PR description then 😅) or a feature request in the issues tab, but I can't give an ETA on when this feature would be added.

Thank you for bringing my attention to the PHP version constraints, I will update them in a moment.

If you want to continue talking about this, feel free to add further comments, but for the time being, I will close the PR because I really don't want to promote using remote locations - incorrect/insecure usage would open the package up for "your library allowed hackers to destroy my project/product" complaints, and I wouldn't like that either 😅.

jeromegamez commented 1 year ago

Update to the version constraints: I just noticed that I already updated them (https://github.com/kreait/laravel-firebase/blob/main/composer.json, I was already wondering why I hadn't) - I don't know how, but it's possible that you targeted an old version of the main branch when creating the PR 🤔