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

[Feature Request] in fromTemporaryCredentials, get the region for the STSClient from the client that's using the temporary credentials #6565

Closed
2 tasks
jedwards1211 opened this issue Oct 15, 2024 · 11 comments
Assignees
Labels
closed-for-staleness feature-request New feature or enhancement. May require GitHub community feedback. p2 This is a standard priority issue queued This issues is on the AWS team's backlog

Comments

@jedwards1211
Copy link

jedwards1211 commented Oct 15, 2024

Describe the feature

We were getting a "Region is missing" error when doing this:

const ec2 = new EC2Client({
  region: 'us-west-2',
  credentials: fromTemporaryCredentials({
    params: {
      RoleArn: '...',
      RoleSessionName: '...',
      DurationSeconds: 3600,
    },
    masterCredentials: { ... },
  }),
})
const { Images = [] } = await ec2.send(
  new DescribeImagesCommand({ ImageIds: [ImageId] })
)

This threw us for a loop, because region: 'us-west-2' is right there.

After a lot of debugging, turns out it's because the STSClient created by fromTemporaryCredentials doesn't get that region setting.

The workaround is to pass clientConfig: { region: 'us-west-2' } to fromTemporaryCredentials, but I don't think we should have to do this. It's not obvious that we need to do this since clientConfig is optional, the "Region is missing" error message only added to the confusion, and it took a lot of debugging to figure out what was actually the problem. Should be easier.

Use Case

Doing virtually anything with temporary credentials

Proposed Solution

fromTemporaryCredentials should somehow be able to get the region of the enclosing client it is passed as credentials to, so that we don't have to explicitly pass clientConfig: { region: ... }. This would be more user-friendly, ergonomic behavior.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

SDK version used

3.651.1

Environment details (OS name and version, etc.)

macOS Sonoma 14.4.1

@jedwards1211 jedwards1211 added feature-request New feature or enhancement. May require GitHub community feedback. needs-triage This issue or PR still needs to be triaged. labels Oct 15, 2024
@zshzbh zshzbh self-assigned this Oct 15, 2024
@zshzbh
Copy link
Contributor

zshzbh commented Oct 23, 2024

@jedwards1211 ,

Thanks for the feedback! I can confirm this issue exists. Will talk to the team regarding the next steps!

@zshzbh zshzbh added p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Oct 23, 2024
@zshzbh
Copy link
Contributor

zshzbh commented Nov 20, 2024

Hey @jedwards1211

I upgraded the package version and the error is gone. Please let me know if the issue persists.
Code I have:

const client = new EC2Client({
  region: "us-east-1",
  credentials: fromTemporaryCredentials({
    //region: "us-east-1",
    params: {
      RoleArn: "arn:aws:iam::XXXXXXX:role/ec2",
      RoleSessionName: "session1",
    },
  }),
});
const command = new DescribeInstancesCommand({});
const response = await client.send(command);

And I got completed; httpStatusCode: 200 response.

The package version I have -

 "@aws-sdk/credential-providers": "^3.696.0"

Please let me know if the issue persists after upgrading the package version.

@jedwards1211
Copy link
Author

jedwards1211 commented Nov 20, 2024

@zshzbh I'm afraid not, I get:

/Users/andy/gh/clarity/deploy/node_modules/.pnpm/@aws-sdk+client-sts@3.696.0/node_modules/@aws-sdk/client-sts/dist-cjs/auth/httpAuthSchemeProvider.js:12
                throw new Error("expected `region` to be configured for `aws.auth#sigv4`");
                      ^

Error: expected `region` to be configured for `aws.auth#sigv4`
    at /Users/andy/gh/clarity/deploy/node_modules/.pnpm/@aws-sdk+client-sts@3.696.0/node_modules/@aws-sdk/client-sts/dist-cjs/auth/httpAuthSchemeProvider.js:12:23
    at Object.defaultSTSHttpAuthSchemeParametersProvider [as httpAuthSchemeParametersProvider] (/Users/andy/gh/clarity/deploy/node_modules/.pnpm/@aws-sdk+client-sts@3.696.0/node_modules/@aws-sdk/client-sts/dist-cjs/auth/httpAuthSchemeProvider.js:13:15)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at /Users/andy/gh/clarity/deploy/node_modules/.pnpm/@smithy+core@2.5.3/node_modules/@smithy/core/dist-cjs/index.js:65:5
    at /Users/andy/gh/clarity/deploy/node_modules/.pnpm/@aws-sdk+middleware-logger@3.696.0/node_modules/@aws-sdk/middleware-logger/dist-cjs/index.js:34:22
    at /Users/andy/gh/clarity/deploy/node_modules/.pnpm/@aws-sdk+credential-providers@3.696.0_@aws-sdk+client-sso-oidc@3.696.0/node_modules/@aws-sdk/credential-providers/dist-cjs/fromTemporaryCredentials.js:50:33
    at coalesceProvider (/Users/andy/gh/clarity/deploy/node_modules/.pnpm/@smithy+core@2.5.3/node_modules/@smithy/core/dist-cjs/index.js:356:18)
    at /Users/andy/gh/clarity/deploy/node_modules/.pnpm/@smithy+core@2.5.3/node_modules/@smithy/core/dist-cjs/index.js:374:18
    at /Users/andy/gh/clarity/deploy/node_modules/.pnpm/@smithy+core@2.5.3/node_modules/@smithy/core/dist-cjs/index.js:86:17
    at /Users/andy/gh/clarity/deploy/node_modules/.pnpm/@aws-sdk+middleware-logger@3.696.0/node_modules/@aws-sdk/middleware-logger/dist-cjs/index.js:34:22
    at /Users/andy/gh/clarity/deploy/temp.ts:18:20

Node.js v20.10.0

Code:

import { DescribeInstancesCommand, EC2Client } from '@aws-sdk/client-ec2'
import { fromTemporaryCredentials } from '@aws-sdk/credential-providers'

const region = 'us-west-2'
const awsConfig = {
  region,
  credentials: fromTemporaryCredentials({
    params: {
      RoleArn: 'arn:aws:iam::XXXXXXXXXXX:role/kes-jcore',
      RoleSessionName: 'deploy-clarity',
    },
    // clientConfig: { region },
  }),
}
;(async () => {
  const client = new EC2Client(awsConfig)
  const command = new DescribeInstancesCommand({})
  const response = await client.send(command)
  console.log(response)
})()

I made sure it wasn't loading region from the environment or config on disk when running it:

$ AWS_REGION= babel-node --extensions .ts temp.ts

(Or, you can comment out any region setting in ~/.aws/config and ~/.aws/credentials)

Are you sure you weren't accidentally loading a region from environment or config when testing this?

Also according to TS, fromTemporaryCredentials doesn't accept a region option directly like you have commented out in your code; you have to pass clientConfig: { region } to it.

@zshzbh
Copy link
Contributor

zshzbh commented Nov 26, 2024

You are right! I have region = us-west-2 in .aws/config file and I didn't comment it.

I just commented it out, and I see this error

Error: Region is missing

And when I add clientConfig: { region: "us-east-1" }, I can see the 200 result.

Code I have -

import { GetObjectCommand, S3Client } from "@aws-sdk/client-s3";
import { fromTemporaryCredentials } from "@aws-sdk/credential-providers";


const config = {
  region: "us-east-1",
  credentials: fromTemporaryCredentials({
    params: {
      RoleArn: "arn:aws:iam::XXX:role/s3-role",
      RoleSessionName: "session1",
    },
    // clientConfig: { region: "us-east-1" } // comment it to see the error, uncomment it to see the 200 OK result. 
  }),
}
const s3 = new S3Client(config);

const res = await s3.send(
  new GetObjectCommand({
    Bucket: "new-bucket-maggie-ma",
    Key: `hello-s31.txt`,

    
  })
);

console.log(res);

@jedwards1211
Copy link
Author

Okay thanks for checking that. Btw is STSClient supposed to require a region in general? Operations like getting the caller identity or getting temporary credentials don't seem like they would need a particular region. i guess it just determines which datacenter handles the request?

I'm not sure what the best way to restructure internal SDK code would be to accomplish this, but it would be nice if the temporary credential provider could get the region of the client using it, and use that as the default region for the STSClient. Either the enclosing client would have to pass its config when it calls the credential provider, or it would have to pass a getRegion method to the credential provider, or something else along these lines.

@zshzbh zshzbh added the queued This issues is on the AWS team's backlog label Nov 26, 2024
@zshzbh
Copy link
Contributor

zshzbh commented Nov 26, 2024

Just checked with the team.

We can pass the outer client as identityProperties when resolving credentials
Definition https://github.com/smithy-lang/smithy-typescript/blob/f4e1a45b667bfa0b95f78cddfb027b6c6f01272b/packages/types/src/identity/identity.ts#L14-L16
The client can be consumed to extract resolved region in fromTemporaryCredentials, but it'll also be available to other credential providers.

And at meanwhile, we will update the docs to reduce confusion.

@zshzbh
Copy link
Contributor

zshzbh commented Nov 26, 2024

To answer this question - STSClient doesn't require a region in general. but for fromTemporaryCredentials, its underlying property check requires region. I don't think we want to restructure that part as it's pretty much the foundational
item. What SDK team can do is to pass the outer client region config to fromTemporaryCredentials configs.

Okay thanks for checking that. Btw is STSClient supposed to require a region in general? Operations like getting the caller identity or getting temporary credentials don't seem like they would need a particular region. i guess it just determines which datacenter handles the request?

I'm not sure what the best way to restructure internal SDK code would be to accomplish this, but it would be nice if the temporary credential provider could get the region of the client using it, and use that as the default region for the STSClient. Either the enclosing client would have to pass its config when it calls the credential provider, or it would have to pass a getRegion method to the credential provider, or something else along these lines.

@jedwards1211
Copy link
Author

Okay means sense, thanks!

@zshzbh
Copy link
Contributor

zshzbh commented Nov 27, 2024

Also want to add some additional info here - A Regional endpoint is the URL of the entry point within a particular region for an AWS web service. AWS recommends using Regional AWS Security Token Service (AWS STS) endpoints instead of the global endpoint to reduce latency, build in redundancy, and increase session token validity. Although the global (legacy) AWS STS endpoint https://sts.amazonaws.com/ is highly available, it’s hosted in a single AWS Region, US East (N. Virginia), and like other endpoints, it doesn’t provide automatic failover to endpoints in other Regions. Ref

@rajleechaudry
Copy link

Same or similar issue from React-Native.

I get my choice of error, either:

[Error: Credential is missing] if I use: clientConfig: { region: 'us-east-1' }

--Or--

Region is missing if I use: new CognitoIdentityClient({ region: 'us-east-1' })

Here is my code:

const cognitoClient = new CognitoIdentityProviderClient({region: 'us-east-1',
                credentials: fromTemporaryCredentials({
                    params: {
                        RoleArn: 'arn:aws:iam::XXXXXXXXX:role/YYYYY-CheckIfUserExistsROLE',
                        RoleSessionName: "session1"
                    },
                    clientConfig: new CognitoIdentityClient({ region: 'us-east-1' })
                })
            })

            console.info(cognitoClient)

            const checkUsername = new AdminGetUserCommand({
                UserPoolId: 'MY-USER-POOL-ID',
                Username: 'some-email@some-email.com'
            })

            cognitoClient.send(checkUsername).then((response) => {

                console.info(response)

            }).catch((error) => {

                console.error(error)

            })

Using:

import { AdminGetUserCommand, CognitoIdentityProviderClient } from "@aws-sdk/client-cognito-identity-provider"
import { fromTemporaryCredentials } from "@aws-sdk/credential-providers"
import { ReadableStream } from 'web-streams-polyfill'

import { CognitoIdentityClient } from "@aws-sdk/client-cognito-identity"
import 'react-native-get-random-values'
import 'react-native-url-polyfill/auto'

globalThis.ReadableStream = ReadableStream

Relevant package.json:

"@aws-amplify/react-native": "^1.1.6",
"@aws-amplify/rtn-web-browser": "^1.1.1",
"@aws-amplify/ui-react-native": "^2.3.1",
"@aws-sdk/client-cognito-identity": "^3.731.1",
"@aws-sdk/client-cognito-identity-provider": "^3.731.1",
"@aws-sdk/credential-providers": "^3.731.1",
"aws-amplify": "^6.12.0",
"web-streams-polyfill": "^4.1.0"

Maybe I can't call the AdminGet* commands server side? But I'd expect to see a very different error. This looks like I don't make my way out of the sdk when executing .send (**)

I use a similar setup with fromCognitoIdentityPool, which works.
Any ideas/solutions?

@kuhe kuhe self-assigned this Jan 21, 2025
@kuhe kuhe added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Jan 22, 2025
@kuhe
Copy link
Contributor

kuhe commented Jan 23, 2025

This feature was released in https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.734.0.

const ec2 = new EC2Client({
  credentials: fromTemporaryCredentials({
    params: {
      RoleArn: '...',
      RoleSessionName: '...',
      DurationSeconds: 3600,
    },
  }),
});

The "outer" EC2Client's region, profile, requestHandler (except http2) are all passed to the inner STSClient of the fromTemporaryCredentials provider.

You may be able to omit the "masterCredentials" as well if you're simply using the default Node.js AWS SDK credential chain.

It's not wrong to be explicit about options, though, and values found in fromTemporaryCredentials({ clientConfig }) will still take priority.

@kuhe kuhe added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed pending-release This issue will be fixed by an approved PR that hasn't been released yet. labels Jan 23, 2025
@github-actions github-actions bot added closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-for-staleness feature-request New feature or enhancement. May require GitHub community feedback. p2 This is a standard priority issue queued This issues is on the AWS team's backlog
Projects
None yet
Development

No branches or pull requests

4 participants