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

[10.x] Do not add token to AWS credentials without validating it first #48297

Merged
merged 2 commits into from
Sep 5, 2023

Conversation

mmehmet
Copy link
Contributor

@mmehmet mmehmet commented Sep 5, 2023

In the CacheManager and FilesystemManager the config that is used to instantiate a new AWS client checks for key and secret values within provided array, before adding those to the credentials element of the config. Unfortunately it also adds a token value, without a similar validation - which can lead to including a blank token value unnecessarily.

Worse still, including a blank token value within that config's credentials array can cause requests using these clients to fail - producing errors such as this when using a DynamoDbClient (which extends the AwsClient) created by a CacheManager that has included such a blank token value:

UnrecognizedClientException (client): The security token included in the request is invalid. - {
    "__type": "com.amazon.coral.service#UnrecognizedClientException",
    "message": "The security token included in the request is invalid."
} {"exception":"[object] (Aws\\DynamoDb\\Exception\\DynamoDbException(code: 0): Error executing \"PutItem\" on \"https://dynamodb.ap-southeast-2.amazonaws.com\"; AWS HTTP error: Client error: `POST https://dynamodb.ap-southeast-2.amazonaws.com` resulted in a `400 Bad Request` response:
{\"__type\":\"com.amazon.coral.service#UnrecognizedClientException\",\"message\":\"The security token included in the request i (truncated...)
UnrecognizedClientException (client): The security token included in the request is invalid. - {\"__type\":\"com.amazon.coral.service#UnrecognizedClientException\",\"message\":\"The security token included in the request is invalid.\"} at /var/task/vendor/aws/aws-sdk-php/src/WrappedHttpHandler.php:195)
[stacktrace]

Not including a (blank) token value in the config fixes this problem.

Michael Mehmet added 2 commits September 5, 2023 09:21
…including this creates an error with the `AwsClient` (which `DynamoDbClient` extends) in recent versions of the AWS PHP SDK:

```Invalid configuration value provided for "token"...```
…t was already present_ within the config.

Adding a blank `token` value into this array element - simply because two other values (`key` and `secret`) happened to be found within the config - can break the `AwsClient`/`S3Client` being built by these managers.

It is also cleaner to have a separate check for this `token` value - rather than assume that because you found two other values, you may as well go ahead and add this third value into the mix too, without having validated it first.
@crynobone crynobone changed the title Do not add token to AWS credentials without validating it first [10.x] Do not add token to AWS credentials without validating it first Sep 5, 2023
@joedixon
Copy link
Contributor

joedixon commented Sep 5, 2023

Thanks @mmehmet - we only need to ensure token is removed from the top-level array as per the pull request you linked recently.

If you don't want to send a token with the credentials array, then you can remove it from your config file. It's not something that ships with the default config so is only an issue if you are updating it.

@taylorotwell taylorotwell merged commit 909ea24 into laravel:10.x Sep 5, 2023
19 checks passed
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