Skip to content

Commit c3a345b

Browse files
authored
fix(cli): can not assume role from 2-level SSO (#23702)
The CLI used to support this: ``` [profile sso] ... [profile one] ... source_profile = sso ``` But not this: ``` [profile sso] ... [profile one] ... source_profile = sso [profile two] ... source_profile = one ``` The reason was that: * We have to do an explicit detection of SSO source profiles in our `PatchedSharedIniFileCredentials` because the upstream `SharedIniFileCredentials` has no SSO support at all; and * When we recursed we would recurse using the `SharedIniFileCredentials` class. In combination, this means that we could only recurse **one level**, because in `SharedIniFileCredentials` we wouldn't support SSO profiles at all. Fix this by recursing using `PatchedSharedIniFileCredentials`, so that we can support SSO source profiles an arbitrary amount of nesting levels deep. While investigating this, also fixed the following issues: - SSO profiles would be detected using the incorrect key: `sso_start_url` can be specified either on the profile section, or a new `[sso-session]` section. `sso_account_id` however always must be on the profile section, so check on that. - Dropped support for reading the STS AssumeRole `region` from the `[default]` section. After investigating by both the JS SDK team and myself, noticed that the AWS CLI does **not** support reading the region from there. While we are both in agreement this is a bug, all customers expect the CDK CLI to behave exactly the same as the AWS CLI, so we have to keep bug-for-bug compatibility. - Drop the `credentials/config` file loading patch. The upstream SDK has fixed this behavior in the mean time, so we can rely on the passed-in profile data again. Fixes #23520. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 1fefc88 commit c3a345b

File tree

1 file changed

+42
-67
lines changed

1 file changed

+42
-67
lines changed

packages/aws-cdk/lib/api/aws-auth/aws-sdk-inifile.ts

+42-67
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,13 @@ import * as AWS from 'aws-sdk';
77
* There are a number of issues in the upstream version of SharedIniFileCredentials
88
* that need fixing:
99
*
10-
* 1. The upstream aws-sdk contains an incorrect instantiation of an `AWS.STS`
11-
* client, which *should* have taken the region from the requested profile
12-
* but doesn't. It will use the region from the default profile, which
13-
* may not exist, defaulting to `us-east-1` (since we switched to
14-
* AWS_STS_REGIONAL_ENDPOINTS=regional, that default is not even allowed anymore
15-
* and the absence of a default region will lead to an error).
10+
* 1. The upstream aws-sdk does not support the 'credential_source' option. Meaning credentials
11+
* for assume-role cannot be fetched using EC2/ESC metadata.
1612
*
17-
* 2. The simple fix is to get the region from the `config` file. profiles
18-
* are made up of a combination of `credentials` and `config`, and the region is
19-
* generally in `config` with the rest in `credentials`. However, a bug in
20-
* `getProfilesFromSharedConfig` overwrites ALL `config` data with `credentials`
21-
* data, so we also need to do extra work to fish the `region` out of the config.
22-
*
23-
* 3. The 'credential_source' option is not supported. Meaning credentials
24-
* for assume-role cannot be fetched using EC2/ESC metadata.
25-
*
26-
* See https://github.com/aws/aws-sdk-js/issues/3418 for all the gory details.
27-
* See https://github.com/aws/aws-sdk-js/issues/1916 for some more glory details.
13+
* 2. The upstream aws-sdk does not support SSO profiles as the source of RoleProfiles,
14+
* because it will always use the `SharedIniFileCredentials` provider to load
15+
* source credentials, but in order to support SSO profiles you must use a
16+
* separate class (`SsoCredentials).
2817
*/
2918
export class PatchedSharedIniFileCredentials extends AWS.SharedIniFileCredentials {
3019
declare private profile: string;
@@ -60,21 +49,29 @@ export class PatchedSharedIniFileCredentials extends AWS.SharedIniFileCredential
6049
var sourceProfile = roleProfile.source_profile;
6150
var credentialSource = roleProfile.credential_source;
6251

63-
const credentialError = (AWS as any).util.error(
64-
new Error(`When using 'role_arn' in profile ('${this.profile}'), you must also configure exactly one of 'source_profile' or 'credential_source'`),
65-
{ code: 'SharedIniFileCredentialsProviderFailure' },
66-
);
67-
68-
if (sourceProfile && credentialSource) {
69-
throw credentialError;
70-
}
71-
72-
if (!sourceProfile && !credentialSource) {
73-
throw credentialError;
52+
if (!!sourceProfile === !!credentialSource) {
53+
throw (AWS as any).util.error(
54+
new Error(`When using 'role_arn' in profile ('${this.profile}'), you must also configure exactly one of 'source_profile' or 'credential_source'`),
55+
{ code: 'SharedIniFileCredentialsProviderFailure' },
56+
);
7457
}
7558

76-
const profiles = loadProfilesProper(this.filename);
77-
const region = profiles[this.profile]?.region ?? profiles.default?.region ?? 'us-east-1';
59+
// Confirmed this against AWS CLI behavior -- the region must be in the assumED profile,
60+
// otherwise `us-east-1`. From the upstream comment in `aws-sdk-js`:
61+
// -------- comment from aws-sdk-js -------------------
62+
// Experimentation shows that the AWS CLI (tested at version 1.18.136)
63+
// ignores the following potential sources of a region for the purposes of
64+
// this AssumeRole call:
65+
//
66+
// - The [default] profile
67+
// - The AWS_REGION environment variable
68+
//
69+
// Ignoring the [default] profile for the purposes of AssumeRole is arguably
70+
// a bug in the CLI since it does use the [default] region for service
71+
// calls... but right now we're matching behavior of the other tool.
72+
// -------------------------------------------------
73+
74+
const region = roleProfile?.region ?? 'us-east-1';
7875

7976
const stsCreds = sourceProfile ? this.sourceProfileCredentials(sourceProfile, creds) : this.credentialSourceCredentials(credentialSource);
8077

@@ -121,7 +118,6 @@ export class PatchedSharedIniFileCredentials extends AWS.SharedIniFileCredential
121118
}
122119

123120
private sourceProfileCredentials(sourceProfile: string, profiles: Record<string, Record<string, string>>) {
124-
125121
var sourceProfileExistanceTest = profiles[sourceProfile];
126122

127123
if (typeof sourceProfileExistanceTest !== 'object') {
@@ -132,11 +128,23 @@ export class PatchedSharedIniFileCredentials extends AWS.SharedIniFileCredential
132128
);
133129
}
134130

135-
if (sourceProfileExistanceTest.sso_start_url) {
131+
// We need to do a manual check here if the source profile (providing the
132+
// credentials for the AssumeRole) is an SSO profile. That's because
133+
// `SharedIniFileCredentials` itself doesn't support providing credentials from
134+
// arbitrary profiles, only for StaticCredentials and AssumeRole type
135+
// profiles; if it's an SSO profile you need to instantiate a special
136+
// Credential Provider for that.
137+
//
138+
// ---
139+
//
140+
// An SSO profile can be configured in 2 ways (put all the info in the profile
141+
// section, or put half of it in an `[sso-session]` block), but in both cases
142+
// the primary profile block must have the `sso_account_id` key
143+
if (sourceProfileExistanceTest.sso_account_id) {
136144
return new AWS.SsoCredentials({ profile: sourceProfile });
137145
}
138146

139-
return new AWS.SharedIniFileCredentials(
147+
return new PatchedSharedIniFileCredentials(
140148
(AWS as any).util.merge(this.options || {}, {
141149
profile: sourceProfile,
142150
preferStaticCredentials: true,
@@ -148,7 +156,6 @@ export class PatchedSharedIniFileCredentials extends AWS.SharedIniFileCredential
148156
// the aws-sdk for js does not support 'credential_source' (https://github.com/aws/aws-sdk-js/issues/1916)
149157
// so unfortunately we need to implement this ourselves.
150158
private credentialSourceCredentials(sourceCredential: string) {
151-
152159
// see https://docs.aws.amazon.com/credref/latest/refdocs/setting-global-credential_source.html
153160
switch (sourceCredential) {
154161
case 'Environment': {
@@ -166,36 +173,4 @@ export class PatchedSharedIniFileCredentials extends AWS.SharedIniFileCredential
166173
}
167174

168175
}
169-
}
170-
171-
/**
172-
* A function to load profiles from disk that MERGES credentials and config instead of overwriting
173-
*
174-
* @see https://github.com/aws/aws-sdk-js/blob/5ae5a7d7d24d1000dbc089cc15f8ed2c7b06c542/lib/util.js#L956
175-
*/
176-
function loadProfilesProper(filename: string) {
177-
const util = (AWS as any).util; // Does exists even though there aren't any typings for it
178-
const iniLoader = util.iniLoader;
179-
const profiles: Record<string, Record<string, string>> = {};
180-
let profilesFromConfig: Record<string, Record<string, string>> = {};
181-
if (process.env[util.configOptInEnv]) {
182-
profilesFromConfig = iniLoader.loadFrom({
183-
isConfig: true,
184-
filename: process.env[util.sharedConfigFileEnv],
185-
});
186-
}
187-
var profilesFromCreds: Record<string, Record<string, string>> = iniLoader.loadFrom({
188-
filename: filename ||
189-
(process.env[util.configOptInEnv] && process.env[util.sharedCredentialsFileEnv]),
190-
});
191-
for (const [name, profile] of Object.entries(profilesFromConfig)) {
192-
profiles[name] = profile;
193-
}
194-
for (const [name, profile] of Object.entries(profilesFromCreds)) {
195-
profiles[name] = {
196-
...profiles[name],
197-
...profile,
198-
};
199-
}
200-
return profiles;
201-
}
176+
}

0 commit comments

Comments
 (0)