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.

Sign up for GitHub

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

Jump to bottom

[11.x] Fix: Prevent invalid AWS credentials options being created #53633

Merged
merged 1 commit into from
Nov 26, 2024
Merged

[11.x] Fix: Prevent invalid AWS credentials options being created #53633

merged 1 commit into from
Nov 26, 2024

Conversation

robchett
Copy link
Contributor

Improves on 909ea24 which could create credentials arrays with only a token

@robchett
[10.x] Fix: Prevent invalid AWS credentials options being created
ef47f6a 
Improves on 909ea24 which could create credentials arrays with only a token
@robchett robchett changed the title [10.x] Fix: Prevent invalid AWS credentials options being created [11.x] Fix: Prevent invalid AWS credentials options being created Nov 22, 2024
robchett added a commit to robchett/laravel-bridge that referenced this pull request Nov 22, 2024
@robchett
Fix: only set aws tokens if a key is specified
df8353b 
laravel/framework#53633 fixes laravel side here but it won't be patched into older versions
@robchett robchett mentioned this pull request Nov 22, 2024
Open
@taylorotwell
Copy link
Member

taylorotwell commented Nov 25, 2024 •
edited
Loading

Hey Robert! Can you elaborate a bit on this one? What is the problem with the current code? We use this logic quite a bit with Vapor so just want to make sure we aren't breaking something here.

Please mark as ready for review when you respond so I see it again.

@taylorotwell taylorotwell marked this pull request as draft November 25, 2024 22:35
@robchett
Copy link
Contributor Author

An upstream bug in https://github.com/brefphp/laravel-bridge was causing credentials to be generated for AWS services with just a token and no key/secret. It's fixed here: brefphp/laravel-bridge#165, however I thought it would be good to prevent it in the Laravel source as well.

Only the code on src/Illuminate/Filesystem/FilesystemManager.php and src/Illuminate/Cache/CacheManager.php is incorrect in their current behaviour, the others are updated for consistency.

If preferred I can remove the changes to:

  • src/Illuminate/Bus/BusServiceProvider.php
  • src/Illuminate/Mail/MailManager.php
  • src/Illuminate/Queue/Connectors/SqsConnector.php
  • src/Illuminate/Queue/QueueServiceProvider.php

@robchett robchett marked this pull request as ready for review November 26, 2024 13:03
@taylorotwell taylorotwell merged commit 44a5a5d into laravel:11.x Nov 26, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@robchett @taylorotwell