Skip to content

Commit

Permalink
fix(credential-provider-ini): support sso-session based profile as so…
Browse files Browse the repository at this point in the history
…urce_profile (#4820)

Previously, when credential-provider-ini resolved credentials for a
source profile, it loaded the profile properties, checked if the profile
was an SSO profile, validated that all required sso_* properties were
present and then resolved credentials with credential-provider-sso
module.

When source profile was an SSO profile that did not use an SSO session,
this logic worked fine as the profile must include all the sso_*
properties for SSO to succeed. However, when source profile was an SSO
profile that used an SSO session, credential-provider-ini only resolved
profile properties directly from the profile but not from the related
SSO session.

This caused sso-session based profiles to fail validation as some of the
required sso_* properties are only defined in the referenced sso-session
section. And hence the provider failed to resolve credentials for SSO
session based source_profiles.

This commit changes credential-provider-ini module to not resolve or
validate SSO profile properties but delegate that all to the
credential-provider-sso module that already contains all the logic
needed to load and resolve profile properties and AWS credentials for
both sso-session and non-sso-session based profiles.

Instead of passing profile properties to credential-provider-sso
fromSSO() method, new version only passes the profile name there and
lets the credential-provider-sso module load and resolve the SSO
profile with the logic that already exist in that module.

Unit tests have been adjusted to function with the updated behavior.

Following scenarios were also tested manually:

* Resolve credentials for SSO profile without sso_session option
* Resolve credentials for SSO profile with sso_session option
* Resolve credentials for an assumed role profile whose source profile
  is an SSO profile without sso_session option
* Resolve credentials for an assumed role profile whose source profile
  is an SSO profile with sso_session option

AWS SDK resolved credentials correctly in all four cases.

Fixes #4757.
  • Loading branch information
sjakthol authored Jan 30, 2024
1 parent 7d66163 commit b87f5e0
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,6 @@ describe(resolveProfileData.name, () => {
(resolveSsoCredentials as jest.Mock).mockImplementation(() => Promise.resolve(mockCreds));
const receivedCreds = await resolveProfileData(mockProfileName, mockProfiles, mockOptions);
expect(receivedCreds).toStrictEqual(mockCreds);
expect(resolveSsoCredentials).toHaveBeenCalledWith(mockProfiles[mockProfileName]);
expect(resolveSsoCredentials).toHaveBeenCalledWith(mockProfileName);
});
});
2 changes: 1 addition & 1 deletion packages/credential-provider-ini/src/resolveProfileData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const resolveProfileData = async (
}

if (isSsoProfile(data)) {
return await resolveSsoCredentials(data);
return await resolveSsoCredentials(profileName);
}

// If the profile cannot be parsed or contains neither static credentials
Expand Down
81 changes: 7 additions & 74 deletions packages/credential-provider-ini/src/resolveSsoCredentials.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,107 +19,40 @@ describe(isSsoProfile.name, () => {
});

describe(resolveSsoCredentials.name, () => {
const getMockOriginalSsoProfile = () => ({
sso_start_url: "mock_sso_start_url",
sso_account_id: "mock_sso_account_id",
sso_region: "mock_sso_region",
sso_role_name: "mock_sso_role_name",
});

const getMockValidatedSsoProfile = <T>(add: T = {} as T) => ({
sso_start_url: "mock_validated_sso_start_url",
sso_account_id: "mock_validated_sso_account_id",
sso_region: "mock_validated_sso_region",
sso_role_name: "mock_validated_sso_role_name",
...add,
});

afterEach(() => {
jest.clearAllMocks();
});

it("throws error when validation fails", async () => {
const mockProfile = getMockOriginalSsoProfile();
const expectedError = new Error("error from validateSsoProfile");
(validateSsoProfile as jest.Mock).mockImplementation(() => {
throw expectedError;
});
try {
await resolveSsoCredentials(mockProfile);
fail(`expected ${expectedError}`);
} catch (error) {
expect(error).toStrictEqual(expectedError);
}
expect(validateSsoProfile).toHaveBeenCalledWith(mockProfile);
});

it("throws error when fromSSO throws error", async () => {
const mockProfile = getMockOriginalSsoProfile();
const mockValidatedProfile = getMockValidatedSsoProfile();
const mockProfileName = "mockProfileName";
const expectedError = new Error("error from fromSSO");

(validateSsoProfile as jest.Mock).mockReturnValue(mockValidatedProfile);
(fromSSO as jest.Mock).mockReturnValue(() => Promise.reject(expectedError));

try {
await resolveSsoCredentials(mockProfile);
await resolveSsoCredentials(mockProfileName);
fail(`expected ${expectedError}`);
} catch (error) {
expect(error).toStrictEqual(expectedError);
}
expect(validateSsoProfile).toHaveBeenCalledWith(mockProfile);
expect(fromSSO).toHaveBeenCalledWith({
ssoStartUrl: mockValidatedProfile.sso_start_url,
ssoAccountId: mockValidatedProfile.sso_account_id,
ssoRegion: mockValidatedProfile.sso_region,
ssoRoleName: mockValidatedProfile.sso_role_name,
profile: mockProfileName,
});
});

it("calls fromSSO when validation succeeds", async () => {
const mockProfile = getMockOriginalSsoProfile();
const mockValidatedProfile = getMockValidatedSsoProfile();

it("calls fromSSO", async () => {
const mockProfileName = "mockProfileName";
const mockCreds: AwsCredentialIdentity = {
accessKeyId: "mockAccessKeyId",
secretAccessKey: "mockSecretAccessKey",
};

(validateSsoProfile as jest.Mock).mockReturnValue(mockValidatedProfile);
(fromSSO as jest.Mock).mockReturnValue(() => Promise.resolve(mockCreds));

const receivedCreds = await resolveSsoCredentials(mockProfile);
const receivedCreds = await resolveSsoCredentials(mockProfileName);
expect(receivedCreds).toStrictEqual(mockCreds);
expect(validateSsoProfile).toHaveBeenCalledWith(mockProfile);
expect(fromSSO).toHaveBeenCalledWith({
ssoStartUrl: mockValidatedProfile.sso_start_url,
ssoAccountId: mockValidatedProfile.sso_account_id,
ssoRegion: mockValidatedProfile.sso_region,
ssoRoleName: mockValidatedProfile.sso_role_name,
});
});

it("calls fromSSO with optional sso session name", async () => {
const mockProfile = getMockOriginalSsoProfile();
const mockValidatedProfile = getMockValidatedSsoProfile({
sso_session: "test-session",
});

const mockCreds: AwsCredentialIdentity = {
accessKeyId: "mockAccessKeyId",
secretAccessKey: "mockSecretAccessKey",
};

(validateSsoProfile as jest.Mock).mockReturnValue(mockValidatedProfile);
(fromSSO as jest.Mock).mockReturnValue(() => Promise.resolve(mockCreds));

await resolveSsoCredentials(mockProfile);
expect(fromSSO).toHaveBeenCalledWith({
ssoStartUrl: mockValidatedProfile.sso_start_url,
ssoAccountId: mockValidatedProfile.sso_account_id,
ssoRegion: mockValidatedProfile.sso_region,
ssoRoleName: mockValidatedProfile.sso_role_name,
ssoSession: mockValidatedProfile.sso_session,
profile: mockProfileName,
});
});
});
11 changes: 3 additions & 8 deletions packages/credential-provider-ini/src/resolveSsoCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,10 @@ import type { Profile } from "@smithy/types";
/**
* @internal
*/
export const resolveSsoCredentials = async (data: Partial<SsoProfile>) => {
const { fromSSO, validateSsoProfile } = await import("@aws-sdk/credential-provider-sso");
const { sso_start_url, sso_account_id, sso_session, sso_region, sso_role_name } = validateSsoProfile(data);
export const resolveSsoCredentials = async (profile: string) => {
const { fromSSO } = await import("@aws-sdk/credential-provider-sso");
return fromSSO({
ssoStartUrl: sso_start_url,
ssoAccountId: sso_account_id,
ssoSession: sso_session,
ssoRegion: sso_region,
ssoRoleName: sso_role_name,
profile,
})();
};

Expand Down

0 comments on commit b87f5e0

Please sign in to comment.