Add role assumption cycle detection

Author: jeskew

packages/credential-provider-ini/__tests__/index.ts

@@ -27,6 +27,12 @@ const FOO_CREDS = {
   sessionToken: "baz"
 };
 
+const FIZZ_CREDS = {
+  accessKeyId: "fizz",
+  secretAccessKey: "buzz",
+  sessionToken: "pop"
+};
+
 const envAtLoadTime: { [key: string]: string } = [
   ENV_CONFIG_PATH,
   ENV_CREDENTIALS_PATH,
@@ -55,15 +61,12 @@ afterAll(() => {
 });
 
 describe("fromIni", () => {
-  it("should flag a lack of credentials as a non-terminal error", async () => {
-    await fromIni()().then(
-      () => {
-        throw new Error("The promise should have been rejected.");
-      },
-      err => {
-        expect((err as CredentialError).tryNextLink).toBe(true);
-      }
-    );
+  it("should flag a lack of credentials as a non-terminal error", () => {
+    return expect(fromIni()()).rejects.toMatchObject({
+      message:
+        "Profile default could not be found or parsed in shared credentials file.",
+      tryNextLink: true
+    });
   });
 
   describe("shared credentials file", () => {
@@ -491,7 +494,7 @@ source_profile = default`.trim()
       expect(await provider()).toEqual(FOO_CREDS);
     });
 
-    it("should reject the promise with a terminal error if no role assumer provided", async () => {
+    it("should reject the promise with a terminal error if no role assumer provided", () => {
       __addMatcher(
         join(homedir(), ".aws", "credentials"),
         `
@@ -505,17 +508,14 @@ role_arn = arn:aws:iam::123456789:role/foo
 source_profile = bar`.trim()
       );
 
-      await fromIni({ profile: "foo" })().then(
-        () => {
-          throw new Error("The promise should have been rejected");
-        },
-        err => {
-          expect((err as any).tryNextLink).toBeFalsy();
-        }
-      );
+      return expect(fromIni({ profile: "foo" })()).rejects.toMatchObject({
+        message:
+          "Profile foo requires a role to be assumed, but no role assumption callback was provided.",
+        tryNextLink: false
+      });
     });
 
-    it("should reject the promise if the source profile cannot be found", async () => {
+    it("should reject the promise if the source profile cannot be found", () => {
       __addMatcher(
         join(homedir(), ".aws", "credentials"),
         `
@@ -529,14 +529,16 @@ role_arn = arn:aws:iam::123456789:role/foo
 source_profile = bar`.trim()
       );
 
-      await fromIni({ profile: "foo" })().then(
-        () => {
-          throw new Error("The promise should have been rejected");
-        },
-        () => {
-          /* Promise rejected as expected */
-        }
-      );
+      const provider = fromIni({
+        profile: "foo",
+        roleAssumer: jest.fn()
+      });
+
+      return expect(provider()).rejects.toMatchObject({
+        message:
+          "Profile bar could not be found or parsed in shared credentials file.",
+        tryNextLink: false
+      });
     });
 
     it("should allow a profile in ~/.aws/credentials to use a source profile from ~/.aws/config", async () => {
@@ -716,7 +718,7 @@ source_profile = default`.trim()
       expect(await provider()).toEqual(FOO_CREDS);
     });
 
-    it("should reject the promise with a terminal error if a MFA serial is present but no mfaCodeProvider was provided", async () => {
+    it("should reject the promise with a terminal error if a MFA serial is present but no mfaCodeProvider was provided", () => {
       const roleArn = "arn:aws:iam::123456789:role/foo";
       const mfaSerial = "mfaSerial";
       __addMatcher(
@@ -738,14 +740,11 @@ source_profile = default`.trim()
         roleAssumer: () => Promise.resolve(FOO_CREDS)
       });
 
-      await provider().then(
-        () => {
-          throw new Error("The promise should have been rejected");
-        },
-        err => {
-          expect((err as any).tryNextLink).toBeFalsy();
-        }
-      );
+      return expect(provider()).rejects.toMatchObject({
+        message:
+          "Profile foo requires multi-factor authentication, but no MFA code callback was provided.",
+        tryNextLink: false
+      });
     });
   });
 
@@ -771,7 +770,7 @@ aws_session_token = ${FOO_CREDS.sessionToken}`.trim()
     expect(await fromIni()()).toEqual(DEFAULT_CREDS);
   });
 
-  it("should reject credentials with no access key", async () => {
+  it("should reject credentials with no access key", () => {
     __addMatcher(
       join(homedir(), ".aws", "credentials"),
       `
@@ -780,17 +779,14 @@ aws_secret_access_key = ${DEFAULT_CREDS.secretAccessKey}
         `.trim()
     );
 
-    await fromIni()().then(
-      () => {
-        throw new Error("The promise should have been rejected");
-      },
-      () => {
-        /* Promise rejected as expected */
-      }
-    );
+    return expect(fromIni()()).rejects.toMatchObject({
+      message:
+        "Profile default could not be found or parsed in shared credentials file.",
+      tryNextLink: true
+    });
   });
 
-  it("should reject credentials with no secret key", async () => {
+  it("should reject credentials with no secret key", () => {
     __addMatcher(
       join(homedir(), ".aws", "credentials"),
       `
@@ -799,17 +795,14 @@ aws_access_key_id = ${DEFAULT_CREDS.accessKeyId}
         `.trim()
     );
 
-    await fromIni()().then(
-      () => {
-        throw new Error("The promise should have been rejected");
-      },
-      () => {
-        /* Promise rejected as expected */
-      }
-    );
+    return expect(fromIni()()).rejects.toMatchObject({
+      message:
+        "Profile default could not be found or parsed in shared credentials file.",
+      tryNextLink: true
+    });
   });
 
-  it("should not merge profile values together", async () => {
+  it("should not merge profile values together", () => {
     __addMatcher(
       join(homedir(), ".aws", "credentials"),
       `
@@ -826,13 +819,81 @@ aws_secret_access_key = ${FOO_CREDS.secretAccessKey}
         `.trim()
     );
 
-    await fromIni()().then(
-      () => {
-        throw new Error("The promise should have been rejected");
-      },
-      () => {
-        /* Promise rejected as expected */
+    return expect(fromIni()()).rejects.toMatchObject({
+      message:
+        "Profile default could not be found or parsed in shared credentials file.",
+      tryNextLink: true
+    });
+  });
+
+  it("should treat a profile with static credentials and role assumption keys as an assume role profile", () => {
+    __addMatcher(
+      join(homedir(), ".aws", "credentials"),
+      `
+[default]
+aws_secret_access_key = ${DEFAULT_CREDS.secretAccessKey}
+aws_secret_access_key = ${DEFAULT_CREDS.secretAccessKey}
+role_arn = foo
+source_profile = foo
+
+[foo]
+aws_access_key_id = ${FOO_CREDS.accessKeyId}
+aws_secret_access_key = ${FOO_CREDS.secretAccessKey}
+aws_session_token = ${FOO_CREDS.sessionToken}
+        `.trim()
+    );
+
+    const provider = fromIni({
+      roleAssumer(
+        sourceCreds: Credentials,
+        params: AssumeRoleParams
+      ): Promise<Credentials> {
+        expect(sourceCreds).toEqual(FOO_CREDS);
+        expect(params.RoleArn).toEqual("foo");
+
+        return Promise.resolve(FIZZ_CREDS);
       }
+    });
+
+    return expect(provider()).resolves.toEqual(FIZZ_CREDS);
+  });
+
+  it("should reject credentials when profile role assumption creates a cycle", () => {
+    __addMatcher(
+      join(homedir(), ".aws", "credentials"),
+      `
+[default]
+role_arn = foo
+source_profile = foo
+
+[bar]
+role_arn = baz
+source_profile = baz
+
+[fizz]
+role_arn = buzz
+source_profile = foo
+        `.trim()
     );
+
+    __addMatcher(
+      join(homedir(), ".aws", "config"),
+      `
+[profile foo]
+role_arn = bar
+source_profile = bar
+
+[profile baz]
+role_arn = fizz
+source_profile = fizz
+        `.trim()
+    );
+    const provider = fromIni({ roleAssumer: jest.fn() });
+
+    return expect(provider()).rejects.toMatchObject({
+      message:
+        "Detected a cycle attempting to resolve credentials for profile default. Profiles visited: foo, bar, baz, fizz",
+      tryNextLink: false
+    });
   });
 });

packages/credential-provider-ini/index.ts

@@ -93,10 +93,9 @@ interface ParsedIniData {
   [key: string]: Profile;
 }
 
-interface StaticCredsProfile {
+interface StaticCredsProfile extends Profile {
   aws_access_key_id: string;
   aws_secret_access_key: string;
-  aws_session_token?: string;
 }
 
 function isStaticCredsProfile(arg: any): arg is StaticCredsProfile {
@@ -109,12 +108,9 @@ function isStaticCredsProfile(arg: any): arg is StaticCredsProfile {
   );
 }
 
-interface AssumeRoleProfile {
+interface AssumeRoleProfile extends Profile {
   role_arn: string;
   source_profile: string;
-  role_session_name?: string;
-  external_id?: string;
-  mfa_serial?: string;
 }
 
 function isAssumeRoleProfile(arg: any): arg is AssumeRoleProfile {
@@ -135,26 +131,31 @@ function isAssumeRoleProfile(arg: any): arg is AssumeRoleProfile {
  */
 export function fromIni(init: FromIniInit = {}): CredentialProvider {
   return () =>
-    parseKnownFiles(init).then(profiles => {
-      const { profile = process.env[ENV_PROFILE] || DEFAULT_PROFILE } = init;
+    parseKnownFiles(init).then(profiles =>
+      resolveProfileData(getMasterProfileName(init), profiles, init)
+    );
+}
 
-      return resolveProfileData(profile, profiles, init);
-    });
+function getMasterProfileName(init: FromIniInit): string {
+  return init.profile || process.env[ENV_PROFILE] || DEFAULT_PROFILE;
 }
 
 async function resolveProfileData(
   profileName: string,
   profiles: ParsedIniData,
-  options: FromIniInit
+  options: FromIniInit,
+  visitedProfiles: { [profileName: string]: true } = {}
 ): Promise<Credentials> {
   const data = profiles[profileName];
-  if (isStaticCredsProfile(data)) {
-    return Promise.resolve({
-      accessKeyId: data.aws_access_key_id,
-      secretAccessKey: data.aws_secret_access_key,
-      sessionToken: data.aws_session_token
-    });
-  } else if (isAssumeRoleProfile(data)) {
+  if (isAssumeRoleProfile(data)) {
+    const {
+      external_id: ExternalId,
+      mfa_serial,
+      role_arn: RoleArn,
+      role_session_name: RoleSessionName = "aws-sdk-js-" + Date.now(),
+      source_profile
+    } = data;
+
     if (!options.roleAssumer) {
       throw new CredentialError(
         `Profile ${profileName} requires a role to be assumed, but no` +
@@ -163,18 +164,19 @@ async function resolveProfileData(
       );
     }
 
-    const {
-      external_id: ExternalId,
-      mfa_serial,
-      role_arn: RoleArn,
-      role_session_name: RoleSessionName = "aws-sdk-js-" + Date.now(),
-      source_profile
-    } = data;
+    if (source_profile in visitedProfiles) {
+      throw new CredentialError(
+        `Detected a cycle attempting to resolve credentials for profile` +
+          ` ${getMasterProfileName(options)}. Profiles visited: ` +
+          Object.keys(visitedProfiles).join(", "),
+        false
+      );
+    }
 
-    const sourceCreds = fromIni({
-      ...options,
-      profile: source_profile
-    })();
+    const sourceCreds = resolveProfileData(source_profile, profiles, options, {
+      ...visitedProfiles,
+      [source_profile]: true
+    });
     const params: AssumeRoleParams = { RoleArn, RoleSessionName, ExternalId };
     if (mfa_serial) {
       if (!options.mfaCodeProvider) {
@@ -189,6 +191,12 @@ async function resolveProfileData(
     }
 
     return options.roleAssumer(await sourceCreds, params);
+  } else if (isStaticCredsProfile(data)) {
+    return Promise.resolve({
+      accessKeyId: data.aws_access_key_id,
+      secretAccessKey: data.aws_secret_access_key,
+      sessionToken: data.aws_session_token
+    });
   }
 
   throw new CredentialError(