Skip to content

Conversation

@liramon1
Copy link
Contributor

@liramon1 liramon1 commented Jul 7, 2025

Problem

While the identity management LSP can manage SSO tokens, it lacks the types and handlers needed to manage IAM credentials. This forces IDE extensions to manage IAM credentials themselves, which adds complexity to them.

Solution

This is part of #572

To facilitate IAM credential management, this PR adds the getIamCredential handler to give IDE extensions a way to retrieve a profile's IAM credentials according to the profile's stored configurations. Each IAM-related configuration is associated with at least one of the new profile types: IamCredentialsProfile, IamSourceProfileProfile, IamCredentialSourceProfile, or IamCredentialProcessProfile. These profiles follow the specification from the AWS CLI and will be used in future language-servers PRs to retrieve IAM credentials in their respective ways.

This also adds the invalidateStsCredential and sendStsCredentialChanged handlers to manage the STS credential lifecycle after an IDE extension calls getIamCredential with a role ARN and parent credentials source inside the requested profile. This will trigger an assume role flow and output an temporary STS credential which the identity LSP will manage.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@liramon1 liramon1 requested a review from a team as a code owner July 7, 2025 16:11
@liramon1 liramon1 changed the title feat(runtimes): add support for IAM and STS credentials management feat(runtimes): add handlers for IAM and STS credentials management Jul 7, 2025
settings?: {
region?: string
sso_session?: string
aws_access_key_id?: string

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to separate Profile in to a "superset" type which consists of separate types SsoProfile and CredentialProfile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not if profiles can be any combination of profile types. Making these fields optional would allow the combined profile type to be decided at runtime and checked using the profile service's current duck typer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might still make sense to separate the settings types while keeping them optional for better readability. Right now it's hard to determine which settings are for which type of profile

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend reaching out to connorstw@ here as he is working on a related project the generate client code from an API definition for Flare. As that would be the primary consumer of this information, there may be better ways to capture it than information-only interfaces.


// getSsoToken
export type SsoTokenId = string // Opaque identifier
export type CredentialId = string // Opaque identifier
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an exported type, could this cause issues with existing clients?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, but I don't see it used in language-servers nor the VS Code, Jetbrains, and Visual Studio toolkits. To be on the safe side, we could separate it into two different IDs. Thoughts?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not needed publicly, may be worth removing export

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent here was to make sure code was using the declared type and not just hard-coding string all over the place. Basically all code should just be treating this as an object and not try to introspect it or use it any other way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then should the SSO tokens and IAM credentials use a single ID type?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are distinct types (even though they are both currently just aliases of string) and should be kept separate.


// Encrypt the IAM credential before sending to client
if (result && !(result instanceof Error) && encryptionKey) {
result.credentials = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this logic out to a util function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

E_CANNOT_OVERWRITE_SSO_SESSION: 'E_CANNOT_OVERWRITE_SSO_SESSION',
E_CANNOT_READ_SHARED_CONFIG: 'E_CANNOT_READ_SHARED_CONFIG',
E_CANNOT_READ_SSO_CACHE: 'E_CANNOT_READ_SSO_CACHE',
E_CANNOT_READ_STS_CACHE: 'E_CANNOT_READ_STS_CACHE',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be reading and writing credentials into the shared config file ~/.aws/credentials. You should use E_CANNOT_READ_SHARED_CONFIG (and the WRITE variation) for these errors. STS is just a service to retrieve and only one way of getting IAM credentials. The naming convention here should be "IAM" instead of "STS".

Copy link
Contributor Author

@liramon1 liramon1 Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the AWS CLI docs, credentials obtained from AssumeRole (which Flare will call if the profile contains a role_arn and some credential source) are cached in ~/.aws/cli/cache, while all other IAM credentials live in ~/.aws/credentials. Should we still only consider ~/.aws/credentials?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I'll send you an SEP offline that goes into more detail and should be considered canon.

Copy link
Contributor Author

@liramon1 liramon1 Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, I think the naming convention can stay as "STS". Technically, the SEP uses the term "temporary credentials", but "STS" is more concise and distinct (and can't be confused with SSO tokens which are also temporary). The cache only stores credentials obtained from STS AssumeRole and not any other IAM credential, so an "STS" prefix fits.

export type ProfileKind =
| 'Unknown'
| 'SsoTokenProfile'
| 'IamUserProfile'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See aws/language-servers#1845 for comments on several of the names here.

settings?: {
region?: string
sso_session?: string
aws_access_key_id?: string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend reaching out to connorstw@ here as he is working on a related project the generate client code from an API definition for Flare. As that would be the primary consumer of this information, there may be better ways to capture it than information-only interfaces.

export type IamCredentialId = string // Opaque identifier

export interface GetIamCredentialOptions {
generateOnInvalidStsCredential?: boolean
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: rename to callStsOnInvalidIamCredential


export interface GetIamCredentialResult {
id: IamCredentialId
credentials: IamCredentials
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was encrypting these in-flight considered or discussed with AppSec similar to SSO tokens?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The credentials are encrypted in-flight using this code, but I have not discussed this with AppSec.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, trying to recall the reasoning for the additional encryption for SSO tokens, but this is fine for now.

code: string
}

export const getMfaCodeRequestType = new ProtocolRequestType<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this PR, the identityService will request an MFA code from the language client if getIamCredential needs to call AssumeRole and the parent credentials' permission to assume roles is locked behind MFA. This assumes the language clients will implement a handler for getMfaCode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll review in more detail in that PR when it is posted.

// Intentionally left blank
}

export const invalidateStsCredentialRequestType = new ProtocolRequestType<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the plan for implementing this (i.e. what API(s) will you call)?

Copy link
Contributor Author

@liramon1 liramon1 Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this PR, invalidateStsCredential will delete the credential from the cache folder using fs unlink.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll review in more detail in that PR when it is posted.

stsCredentialId: IamCredentialId
}

export const stsCredentialChangedRequestType = new ProtocolNotificationType<StsCredentialChangedParams, void>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the implementation plan for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this PR, stsCredentialChanged will be used similarly to ssoTokenChanged. It will fire a notification whenever an IAM credential obtained from AssumedRole is refreshed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll review in more detail in that PR when it is posted.

@liramon1 liramon1 merged commit 6276983 into aws:main Jul 17, 2025
4 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.

4 participants