Merge branch 'hkobew/ec2/useActions' into feature/ec2

Author: Hweinstock

src/ec2/model.ts

@@ -12,7 +12,12 @@ import { isCloud9 } from '../shared/extensionUtilities'
 import { ToolkitError } from '../shared/errors'
 import { SsmClient } from '../shared/clients/ssmClient'
 import { Ec2Client } from '../shared/clients/ec2Client'
-import { VscodeRemoteConnection, ensureDependencies, openRemoteTerminal } from '../shared/remoteSession'
+import {
+    VscodeRemoteConnection,
+    ensureDependencies,
+    getDeniedSsmActions,
+    openRemoteTerminal,
+} from '../shared/remoteSession'
 import { DefaultIamClient } from '../shared/clients/iamClient'
 import { ErrorInformation } from '../shared/errors'
 import { sshAgentSocketVariable, startSshAgent, startVscodeRemote } from '../shared/extensions/ssh'
@@ -72,13 +77,10 @@ export class Ec2ConnectionManager {
         }
     }
 
-    public async hasProperPolicies(IamRoleArn: string): Promise<boolean> {
-        const attachedPolicies = (await this.iamClient.listAttachedRolePolicies(IamRoleArn)).map(
-            policy => policy.PolicyName!
-        )
-        const requiredPolicies = ['AmazonSSMManagedInstanceCore', 'AmazonSSMManagedEC2InstanceDefaultPolicy']
+    public async hasProperPermissions(IamRoleArn: string): Promise<boolean> {
+        const deniedActions = await getDeniedSsmActions(this.iamClient, IamRoleArn)
 
-        return requiredPolicies.length !== 0 && requiredPolicies.every(policy => attachedPolicies.includes(policy))
+        return deniedActions.length === 0
     }
 
     public async isInstanceRunning(instanceId: string): Promise<boolean> {
@@ -108,10 +110,10 @@ export class Ec2ConnectionManager {
             this.throwConnectionError(message, selection, { code: 'EC2SSMPermission' })
         }
 
-        const hasProperPolicies = await this.hasProperPolicies(IamRole!.Arn)
+        const hasPermission = await this.hasProperPermissions(IamRole!.Arn)
 
-        if (!hasProperPolicies) {
-            const message = `Ensure an IAM role with the required policies is attached to the instance. Found attached role: ${
+        if (!hasPermission) {
+            const message = `Ensure an IAM role with the proper permissions is attached to the instance. Found attached role: ${
                 IamRole!.Arn
             }`
             this.throwConnectionError(message, selection, {

src/ecs/util.ts

@@ -18,6 +18,7 @@ import { TaskDefinition } from 'aws-sdk/clients/ecs'
 import { getLogger } from '../shared/logger'
 import { SSM } from 'aws-sdk'
 import { fromExtensionManifest } from '../shared/settings'
+import { getDeniedSsmActions } from '../shared/remoteSession'
 
 interface EcsTaskIdentifer {
     readonly task: string
@@ -41,15 +42,7 @@ export async function checkPermissionsForSsm(
         })
     }
 
-    const deniedActions = await client.getDeniedActions({
-        PolicySourceArn: task.taskRoleArn,
-        ActionNames: [
-            'ssmmessages:CreateControlChannel',
-            'ssmmessages:CreateDataChannel',
-            'ssmmessages:OpenControlChannel',
-            'ssmmessages:OpenDataChannel',
-        ],
-    })
+    const deniedActions = await getDeniedSsmActions(client, task.taskRoleArn)
 
     if (deniedActions.length !== 0) {
         const message = localize(

src/shared/remoteSession.ts

@@ -19,6 +19,8 @@ import { SystemUtilities } from './systemUtilities'
 import { getOrInstallCli } from './utilities/cliUtils'
 import { pushIf } from './utilities/collectionUtils'
 import { ChildProcess } from './utilities/childProcess'
+import { IamClient } from './clients/iamClient'
+import { IAM } from 'aws-sdk'
 
 export interface MissingTool {
     readonly name: 'code' | 'ssm' | 'ssh'
@@ -167,3 +169,17 @@ export async function handleMissingTool(tools: Err<MissingTool[]>) {
         })
     )
 }
+
+export async function getDeniedSsmActions(client: IamClient, roleArn: string): Promise<IAM.EvaluationResult[]> {
+    const deniedActions = await client.getDeniedActions({
+        PolicySourceArn: roleArn,
+        ActionNames: [
+            'ssmmessages:CreateControlChannel',
+            'ssmmessages:CreateDataChannel',
+            'ssmmessages:OpenControlChannel',
+            'ssmmessages:OpenDataChannel',
+        ],
+    })
+
+    return deniedActions
+}

src/test/ec2/model.test.ts

@@ -42,45 +42,6 @@ describe('Ec2ConnectClient', function () {
         })
     })
 
-    describe('hasProperPolicies', async function () {
-        it('correctly determines if proper policies are included', async function () {
-            async function assertAcceptsPolicies(policies: IAM.Policy[], expectedResult: boolean) {
-                sinon.stub(DefaultIamClient.prototype, 'listAttachedRolePolicies').resolves(policies)
-
-                const result = await client.hasProperPolicies('')
-                assert.strictEqual(result, expectedResult)
-
-                sinon.restore()
-            }
-            await assertAcceptsPolicies(
-                [{ PolicyName: 'name' }, { PolicyName: 'name2' }, { PolicyName: 'name3' }],
-                false
-            )
-            await assertAcceptsPolicies(
-                [
-                    { PolicyName: 'AmazonSSMManagedInstanceCore' },
-                    { PolicyName: 'AmazonSSMManagedEC2InstanceDefaultPolicy' },
-                ],
-                true
-            )
-            await assertAcceptsPolicies([{ PolicyName: 'AmazonSSMManagedEC2InstanceDefaultPolicy' }], false)
-            await assertAcceptsPolicies([{ PolicyName: 'AmazonSSMManagedEC2InstanceDefaultPolicy' }], false)
-        })
-
-        it('throws error when sdk throws error', async function () {
-            sinon.stub(DefaultIamClient.prototype, 'listAttachedRolePolicies').throws(new ToolkitError('error'))
-
-            try {
-                await client.hasProperPolicies('')
-                assert.ok(false)
-            } catch {
-                assert.ok(true)
-            }
-
-            sinon.restore()
-        })
-    })
-
     describe('isInstanceRunning', async function () {
         it('only returns true with the instance is running', async function () {
             sinon.stub(Ec2Client.prototype, 'getInstanceStatus').callsFake(async (input: string) => input.split(':')[0])
@@ -131,7 +92,7 @@ describe('Ec2ConnectClient', function () {
         it('throws EC2SSMAgent error if instance is running and has IAM Role, but agent is not running', async function () {
             sinon.stub(Ec2ConnectionManager.prototype, 'isInstanceRunning').resolves(true)
             sinon.stub(Ec2ConnectionManager.prototype, 'getAttachedIamRole').resolves({ Arn: 'testRole' } as IAM.Role)
-            sinon.stub(Ec2ConnectionManager.prototype, 'hasProperPolicies').resolves(true)
+            sinon.stub(Ec2ConnectionManager.prototype, 'hasProperPermissions').resolves(true)
             sinon.stub(SsmClient.prototype, 'getInstanceAgentPingStatus').resolves('offline')
 
             try {
@@ -147,7 +108,7 @@ describe('Ec2ConnectClient', function () {
         it('does not throw an error if all checks pass', async function () {
             sinon.stub(Ec2ConnectionManager.prototype, 'isInstanceRunning').resolves(true)
             sinon.stub(Ec2ConnectionManager.prototype, 'getAttachedIamRole').resolves({ Arn: 'testRole' } as IAM.Role)
-            sinon.stub(Ec2ConnectionManager.prototype, 'hasProperPolicies').resolves(true)
+            sinon.stub(Ec2ConnectionManager.prototype, 'hasProperPermissions').resolves(true)
             sinon.stub(SsmClient.prototype, 'getInstanceAgentPingStatus').resolves('Online')
 
             assert.doesNotThrow(async () => await client.checkForStartSessionError(instanceSelection))