Merge pull request #363 from liamdawson/improve-allowed-account-handling

Improve allowed account handling

CHANGELOG.md

@@ -10,7 +10,17 @@ The format is based on [Keep a Changelog], and this project adheres to
 
 ## [Unreleased]
 
-[Unreleased]: https://github.com/envato/stack_master/compare/v2.13.0...HEAD
+[Unreleased]: https://github.com/envato/stack_master/compare/v2.13.1...HEAD
+
+## [2.13.1] - 2021-10-11
+
+### Changed
+
+- Avoid an API call to check account aliases if all `allowed_accounts` look like AWS account IDs ([#363])
+- Provide a more contextual error message if fetching account aliases failed during allowed accounts check ([#363])
+
+[2.13.1]: https://github.com/envato/stack_master/compare/v2.13.0...v2.13.1
+[#363]: https://github.com/envato/stack_master/pull/363
 
 ## [2.13.0] - 2021-02-10
 

features/apply_with_allowed_accounts.feature

@@ -5,14 +5,14 @@ Feature: Apply command with allowed accounts
       """
       stack_defaults:
         allowed_accounts:
-          - '11111111'
+          - '111111111111'
       stacks:
         us_east_1:
           myapp_vpc:
             template: myapp.rb
           myapp_db:
             template: myapp.rb
-            allowed_accounts: '22222222'
+            allowed_accounts: '222222222222'
           myapp_web:
             template: myapp.rb
             allowed_accounts: []
@@ -33,7 +33,7 @@ Feature: Apply command with allowed accounts
     Given I stub the following stack events:
       | stack_id | event_id | stack_name | logical_resource_id | resource_status | resource_type              | timestamp           |
       | 1        | 1        | myapp-vpc  | myapp-vpc           | CREATE_COMPLETE | AWS::CloudFormation::Stack | 2020-10-29 00:00:00 |
-    When I use the account "11111111"
+    When I use the account "111111111111"
     And I run `stack_master apply us-east-1 myapp-vpc`
     Then the output should match /2020-10-29 00:00:00 (\+|\-)[0-9]{4} myapp-vpc AWS::CloudFormation::Stack CREATE_COMPLETE/
     And the exit status should be 0
@@ -42,17 +42,17 @@ Feature: Apply command with allowed accounts
     Given I stub the following stack events:
       | stack_id | event_id | stack_name | logical_resource_id | resource_status | resource_type              | timestamp           |
       | 1        | 1        | myapp-db   | myapp-db            | CREATE_COMPLETE | AWS::CloudFormation::Stack | 2020-10-29 00:00:00 |
-    When I use the account "11111111"
+    When I use the account "111111111111"
     And I run `stack_master apply us-east-1 myapp-db`
     Then the output should contain all of these lines:
-      | Account '11111111' is not an allowed account. Allowed accounts are ["22222222"].|
+      | Account '111111111111' is not an allowed account. Allowed accounts are ["222222222222"].|
     And the exit status should be 1
 
   Scenario: Run apply with stack overriding allowed accounts to allow all accounts
     Given I stub the following stack events:
       | stack_id | event_id | stack_name | logical_resource_id | resource_status | resource_type              | timestamp           |
       | 1        | 1        | myapp-web  | myapp-web           | CREATE_COMPLETE | AWS::CloudFormation::Stack | 2020-10-29 00:00:00 |
-    When I use the account "33333333"
+    When I use the account "333333333333"
     And I run `stack_master apply us-east-1 myapp-web`
     Then the output should match /2020-10-29 00:00:00 (\+|\-)[0-9]{4} myapp-web AWS::CloudFormation::Stack CREATE_COMPLETE/
     And the exit status should be 0
@@ -61,7 +61,7 @@ Feature: Apply command with allowed accounts
     Given I stub the following stack events:
       | stack_id | event_id | stack_name  | logical_resource_id | resource_status | resource_type              | timestamp           |
       | 1        | 1        | myapp-cache | myapp-cache         | CREATE_COMPLETE | AWS::CloudFormation::Stack | 2020-10-29 00:00:00 |
-    When I use the account "44444444" with alias "my-account-alias"
+    When I use the account "444444444444" with alias "my-account-alias"
     And I run `stack_master apply us-east-1 myapp-cache`
     Then the output should match /2020-10-29 00:00:00 (\+|\-)[0-9]{4} myapp-cache AWS::CloudFormation::Stack CREATE_COMPLETE/
     And the exit status should be 0
@@ -70,8 +70,8 @@ Feature: Apply command with allowed accounts
     Given I stub the following stack events:
       | stack_id | event_id | stack_name  | logical_resource_id | resource_status | resource_type              | timestamp           |
       | 1        | 1        | myapp-cache | myapp-cache         | CREATE_COMPLETE | AWS::CloudFormation::Stack | 2020-10-29 00:00:00 |
-    When I use the account "11111111" with alias "an-account-alias"
+    When I use the account "111111111111" with alias "an-account-alias"
     And I run `stack_master apply us-east-1 myapp-cache`
     Then the output should contain all of these lines:
-      | Account '11111111' (an-account-alias) is not an allowed account. Allowed accounts are ["my-account-alias"].|
+      | Account '111111111111' (an-account-alias) is not an allowed account. Allowed accounts are ["my-account-alias"].|
     And the exit status should be 1

lib/stack_master/identity.rb

@@ -1,12 +1,17 @@
 module StackMaster
   class Identity
+    AllowedAccountAliasesError = Class.new(StandardError)
     MissingIamPermissionsError = Class.new(StandardError)
 
     def running_in_account?(accounts)
-      accounts.nil? ||
-        accounts.empty? ||
-        contains_account_id?(accounts) ||
-        contains_account_alias?(accounts)
+      return true if accounts.nil? || accounts.empty? || contains_account_id?(accounts)
+
+      # skip alias check (which makes an API call) if all values are account IDs
+      return false if accounts.all? { |account| account_id?(account) }
+
+      contains_account_alias?(accounts)
+    rescue MissingIamPermissionsError
+      raise AllowedAccountAliasesError, 'Failed to validate whether the current AWS account is allowed'
     end
 
     def account
@@ -40,5 +45,11 @@ def contains_account_id?(ids)
     def contains_account_alias?(aliases)
       account_aliases.any? { |account_alias| aliases.include?(account_alias) }
     end
+
+    def account_id?(id_or_alias)
+      # While it's not explicitly documented as prohibited, it cannot (currently) be possible to set an account alias of
+      # 12 digits, as that could cause one console sign-in URL to resolve to two separate accounts.
+      /^[0-9]{12}$/.match?(id_or_alias)
+    end
   end
 end

lib/stack_master/version.rb

@@ -1,3 +1,3 @@
 module StackMaster
-  VERSION = "2.13.0"
+  VERSION = "2.13.1"
 end

spec/stack_master/identity_spec.rb

@@ -10,7 +10,7 @@
   end
 
   describe '#running_in_account?' do
-    let(:account) { '1234567890' }
+    let(:account) { '123456789012' }
     let(:running_in_allowed_account) { identity.running_in_account?(allowed_accounts) }
 
     before do
@@ -42,11 +42,26 @@
     end
 
     context 'with no allowed account' do
-      let(:allowed_accounts) { ['9876543210'] }
+      let(:allowed_accounts) { ['210987654321'] }
 
       it 'returns false' do
         expect(running_in_allowed_account).to eq(false)
       end
+
+      context 'without list account aliases permissions' do
+        before do
+          allow(iam).to receive(:list_account_aliases).and_raise(
+            Aws::IAM::Errors.error_class('AccessDenied').new(
+              an_instance_of(Seahorse::Client::RequestContext),
+              'User: arn:aws:sts::123456789:assumed-role/my-role/123456789012 is not authorized to perform: iam:ListAccountAliases on resource: *'
+            )
+          )
+        end
+
+        it 'returns false' do
+          expect(running_in_allowed_account).to eq(false)
+        end
+      end
     end
 
     describe 'with account aliases' do
@@ -76,12 +91,29 @@
       end
 
       context 'with a combination of account id and alias' do
-        let(:allowed_accounts) { %w(1928374 allowed-account another-account) }
+        let(:allowed_accounts) { %w(192837471659 allowed-account another-account) }
 
         it 'returns true' do
           expect(running_in_allowed_account).to eq(true)
         end
       end
+
+      context 'without list account aliases permissions' do
+        let(:allowed_accounts) { ['an-account-alias'] }
+
+        before do
+          allow(iam).to receive(:list_account_aliases).and_raise(
+            Aws::IAM::Errors.error_class('AccessDenied').new(
+              an_instance_of(Seahorse::Client::RequestContext),
+              'User: arn:aws:sts::123456789:assumed-role/my-role/123456789012 is not authorized to perform: iam:ListAccountAliases on resource: *'
+            )
+          )
+        end
+
+        it 'raises the correct error' do
+          expect { running_in_allowed_account }.to raise_error(StackMaster::Identity::AllowedAccountAliasesError)
+        end
+      end
     end
   end