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

S3Client requestHandler proxy settings ignored when credentials supplied #2356

Closed
MrBlueMoo opened this issue May 7, 2021 · 5 comments · Fixed by #2426
Closed

S3Client requestHandler proxy settings ignored when credentials supplied #2356

MrBlueMoo opened this issue May 7, 2021 · 5 comments · Fixed by #2426
Assignees
Labels
bug This issue is a bug.

Comments

@MrBlueMoo
Copy link

Describe the bug

When supplying requestHandler and credentials to the S3Client constructor, the proxy settings in requestHandler appear to be ignored.

If just supplying requestHandler with no credentials to the S3Client constructor, then the proxy settings work and take effect.

So the issue appears to be the combination of using credentials (loaded from the AWS_ROLE_ARN and AWS_WEB_IDENTITY_TOKEN_FILE environment variables) with the requestHandler.

We have a requirement to be able to use both web identity tokens and use a proxy, so are looking to find a resolution or workaround.

Your environment

SDK version number

"@aws-sdk/client-s3": "^3.14.0",
"@aws-sdk/client-sts": "^3.14.0",
"@aws-sdk/credential-provider-web-identity": "^3.13.1",
"@aws-sdk/node-http-handler": "^3.13.1",
"@aws-sdk/types": "^3.13.1",
"typescript": "^4.2.4",

Is the issue in the browser/Node.js/ReactNative?

Node.js / Typescript

Details of the browser/Node.js/ReactNative version

$ node -v
v14.16.1

Steps to reproduce

Assuming that we have the following environment variables correctly setup and configured: AWS_ROLE_ARN, AWS_WEB_IDENTITY_TOKEN_FILE and http_proxy

The code which demonstrates the issue is as follows:

import { S3Client } from "@aws-sdk/client-s3";
import { NodeHttpHandler } from "@aws-sdk/node-http-handler";
import ProxyAgent from "proxy-agent";
import { getDefaultRoleAssumerWithWebIdentity } from "@aws-sdk/client-sts";
import { fromTokenFile } from "@aws-sdk/credential-provider-web-identity";

const proxy = new ProxyAgent();
const requestHandler = new NodeHttpHandler({ 
  httpAgent: proxy, 
  httpsAgent: proxy 
});
const storageClient = new S3Client({ 
  credentials: fromTokenFile({
    roleAssumerWithWebIdentity: getDefaultRoleAssumerWithWebIdentity()
  }), 
  requestHandler: requestHandler 
});

Which correctly reads the secret information from the web identity token file, however then does not use the proxy and attempts to connect direct.

The following code which only sends the requestHandler in the constructor does not suffer from this problem. The following code works with the proxy:

import { S3Client } from "@aws-sdk/client-s3";
import { NodeHttpHandler } from "@aws-sdk/node-http-handler";
import ProxyAgent from "proxy-agent";

const proxy = new ProxyAgent();
const requestHandler = new NodeHttpHandler({ 
  httpAgent: proxy, 
  httpsAgent: proxy 
});
const storageClient = new S3Client({ 
  requestHandler: requestHandler 
});

For our use however we need both to be able to use the AWS_WEB_IDENTITY_TOKEN_FILE and have a proxy configured.

I cannot find any relevant documentation regarding this behaviour, so it appears to be a bug. If a fix or workaround could be provided that would be much appreciated.

Note: Creating a new ProxyAgent without a URL will search for default proxy settings and does pick up from "http_proxy" and/or "https_proxy" environment variables

Observed behavior

When the credentials are passed into the S3Client constructor, then the requestHandler which applies the proxy settings (defined by ENV variables "http_proxy" and/or "https_proxy") are not used for the subsequent calls to S3.

Connections appear to go direct (not via proxy)

Expected behavior

The proxy settings setup in the requestHandler should still be applied even when credentials are loaded when setting up the S3Client.

The proxy works fine if credentials are not specified in the S3Client constructor. So this appears to be just an issue when BOTH "credentials" and "requestHandler" are passed in.

Screenshots

N/A

Additional context

N/A

@MrBlueMoo MrBlueMoo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 7, 2021
@ajredniwja ajredniwja added this to To do in Investigation May 24, 2021
@AllanZhengYP AllanZhengYP self-assigned this May 24, 2021
@AllanZhengYP
Copy link
Contributor

AllanZhengYP commented May 24, 2021

Hi @MrBlueMoo This is indeed an issue with the credential provider. The requestHandler supplied in the code example only affects the S3 client, not NOT the STS client created behind the scene of getDefaultRoleAssumerWithWebIdentity. Currently only region and logger can be specified to the STS client used to assume the role:

if (!stsClient) {
const { logger, region } = stsOptions;
stsClient = new stsClientCtor({
logger,
// A hack to make sts client uses the credential in current closure.
credentialDefaultProvider: () => async () => closureSourceCreds,
region: decorateDefaultRegion(region),
});
}

I can add the requestHandler to allow overwriting too. So the code will look like this

const storageClient = new S3Client({ 
  credentials: fromTokenFile({
    roleAssumerWithWebIdentity: getDefaultRoleAssumerWithWebIdentity({
      requestHandler: requestHandler 
      // or different request handler 
      // requestHandler: new NodeHttpHandler({ httpAgent, httpsAgent }),
    })
  }), 
  requestHandler: requestHandler 
});

I don't plan to make the requestHandler supplied to S3Client to overwrite the handler of STS client inside of getDefaultRoleAssumerWithWebIdentity. Because they are different clients indeed.

@AllanZhengYP AllanZhengYP removed the needs-triage This issue or PR still needs to be triaged. label May 24, 2021
@AllanZhengYP
Copy link
Contributor

@MrBlueMoo For now, to get around this issue you need to roll the role assumer by yourself. Here's the example:

import { S3Client } from "@aws-sdk/client-s3";
import { NodeHttpHandler } from "@aws-sdk/node-http-handler";
import ProxyAgent from "proxy-agent";
import { STSClient, AssumeRoleWithWebIdentityCommand } from "@aws-sdk/client-sts";
import { fromTokenFile } from "@aws-sdk/credential-provider-web-identity";

const stsProxy = new ProxyAgent();
const stsClient = new STSClient({
  requestHandler:  new NodeHttpHandler({
    httpsAgent: stsProxy // httpAgent is not needed as TLS is enabled in all services
  }),
});
const roleAssumer = async ( params ) => {
  const { Credentials } = await stsClient.send(new AssumeRoleWithWebIdentityCommand(params));
    if (!Credentials || !Credentials.AccessKeyId || !Credentials.SecretAccessKey) {
      throw new Error(`Invalid response from STS.assumeRoleWithWebIdentity call with role ${params.RoleArn}`);
    }
    return {
      accessKeyId: Credentials.AccessKeyId,
      secretAccessKey: Credentials.SecretAccessKey,
      sessionToken: Credentials.SessionToken,
      expiration: Credentials.Expiration,
    };
}
const s3Proxy = new ProxyAgent();
const storageClient = new S3Client({ 
  credentials: fromTokenFile({
    roleAssumerWithWebIdentity: roleAssumer
  }), 
  requestHandler: new NodeHttpHandler({ 
    httpsAgent: s3Proxy // httpAgent is not needed as TLS is enabled in all services
  }); 
});

@ajredniwja
Copy link
Member

#2426 with the fix, should roll out with next release.

@MrBlueMoo
Copy link
Author

@AllanZhengYP @ajredniwja Many thanks for providing both a workaround and a fix in next release (allowing us to specify the requestHandler on the sts client used by default role assumer). I have just implemented the workaround with setting up a role assumer based on the example you provided, and I will verify this all functions as expected with proxy tomorrow.

@ajredniwja ajredniwja moved this from In progress to Done in Investigation Jun 10, 2021
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2021
@ajredniwja ajredniwja removed this from Done in Investigation Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants