From b391b4e98668e4593bece6f8126822665f74d614 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 12:14:37 +1200 Subject: [PATCH 01/24] Tests! --- .../parameter_resolvers/sso_group_id_spec.rb | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 spec/stack_master/parameter_resolvers/sso_group_id_spec.rb diff --git a/spec/stack_master/parameter_resolvers/sso_group_id_spec.rb b/spec/stack_master/parameter_resolvers/sso_group_id_spec.rb new file mode 100644 index 00000000..7644f287 --- /dev/null +++ b/spec/stack_master/parameter_resolvers/sso_group_id_spec.rb @@ -0,0 +1,38 @@ +RSpec.describe StackMaster::ParameterResolvers::SsoGroupId do + describe "#resolve" do + let(:identity_store_id) { 'd-12345678' } + let(:sso_group_id) { '64e804c8-8091-7093-3da9-123456789012' } + let(:sso_group_name) { 'Okta-App-AWS-Group-Admin' } + let(:region) { 'us-east-1' } + + let(:config) { instance_double('Config', sso_identity_store_id: identity_store_id) } + let(:stack_definition) { instance_double('StackDefinition', region: region) } + let(:finder) { instance_double(StackMaster::SsoGroupIdFinder) } + + subject(:resolver) { described_class.new(config, stack_definition) } + + before do + allow(StackMaster::SsoGroupIdFinder).to receive(:new).with(region).and_return(finder) + allow(finder).to receive(:find).with(sso_group_name, identity_store_id).and_return(sso_group_id) + end + + context 'when given an SSO group name' do + it "finds the sso group id" do + expect(resolver.resolve(sso_group_name)).to eq sso_group_id + end + end + + context 'when sso_identity_store_id is missing' do + let(:config) { instance_double('Config', sso_identity_store_id: nil) } + + it 'raises an InvalidParameter error' do + expect { + described_class.new(config, stack_definition) + }.to raise_error( + StackMaster::ParameterResolvers::SsoGroupId::InvalidParameter, + /sso_identity_store_id must be set/ + ) + end + end + end +end From 8edf38274fe7447aeec961915b5393a3bdeb715d Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 12:38:17 +1200 Subject: [PATCH 02/24] Add in resolver code and integrate into main app --- lib/stack_master.rb | 3 ++ lib/stack_master/config.rb | 2 ++ .../parameter_resolvers/sso_group_id.rb | 22 ++++++++++++ lib/stack_master/sso_group_id_finder.rb | 35 +++++++++++++++++++ lib/stack_master/version.rb | 2 +- stack_master.gemspec | 1 + 6 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 lib/stack_master/parameter_resolvers/sso_group_id.rb create mode 100644 lib/stack_master/sso_group_id_finder.rb diff --git a/lib/stack_master.rb b/lib/stack_master.rb index bdc16996..4234330d 100644 --- a/lib/stack_master.rb +++ b/lib/stack_master.rb @@ -4,6 +4,7 @@ require 'aws-sdk-cloudformation' require 'aws-sdk-ec2' require 'aws-sdk-ecr' +require 'aws-sdk-identitystore' require 'aws-sdk-s3' require 'aws-sdk-sns' require 'aws-sdk-ssm' @@ -33,6 +34,7 @@ module StackMaster autoload :StackStatus, 'stack_master/stack_status' autoload :SnsTopicFinder, 'stack_master/sns_topic_finder' autoload :SecurityGroupFinder, 'stack_master/security_group_finder' + autoload :SsoGroupIdFinder, 'stack_master/sso_group_id_finder' autoload :ParameterLoader, 'stack_master/parameter_loader' autoload :ParameterResolver, 'stack_master/parameter_resolver' autoload :RoleAssumer, 'stack_master/role_assumer' @@ -84,6 +86,7 @@ module ParameterResolvers autoload :Ejson, 'stack_master/parameter_resolvers/ejson' autoload :SnsTopicName, 'stack_master/parameter_resolvers/sns_topic_name' autoload :SecurityGroup, 'stack_master/parameter_resolvers/security_group' + autoload :SsoGroupId, 'stack_master/parameter_resolvers/sso_group_id' autoload :LatestAmiByTags, 'stack_master/parameter_resolvers/latest_ami_by_tags' autoload :LatestAmi, 'stack_master/parameter_resolvers/latest_ami' autoload :Env, 'stack_master/parameter_resolvers/env' diff --git a/lib/stack_master/config.rb b/lib/stack_master/config.rb index 14cbc05a..e12d81aa 100644 --- a/lib/stack_master/config.rb +++ b/lib/stack_master/config.rb @@ -17,6 +17,7 @@ def self.load!(config_file = 'stack_master.yml') attr_accessor :stacks, :base_dir, :template_dir, + :sso_identity_store_id, :parameters_dir, :stack_defaults, :region_defaults, @@ -41,6 +42,7 @@ def initialize(config, base_dir) @base_dir = base_dir @template_dir = config.fetch('template_dir', nil) @parameters_dir = config.fetch('parameters_dir', nil) + @sso_identity_store_id = config.fetch('sso_identity_store_id',nil) @stack_defaults = config.fetch('stack_defaults', {}) @region_aliases = Utils.underscore_keys_to_hyphen(config.fetch('region_aliases', {})) @region_to_aliases = @region_aliases.inject({}) do |hash, (key, value)| diff --git a/lib/stack_master/parameter_resolvers/sso_group_id.rb b/lib/stack_master/parameter_resolvers/sso_group_id.rb new file mode 100644 index 00000000..521e4a3c --- /dev/null +++ b/lib/stack_master/parameter_resolvers/sso_group_id.rb @@ -0,0 +1,22 @@ +module StackMaster + module ParameterResolvers + class SsoGroupId < Resolver + InvalidParameter = Class.new(StandardError) + + def initialize(config, stack_definition) + @config = config + @stack_definition = stack_definition + raise InvalidParameter, "sso_identity_store_id must be set in stack_master.yml when using sso_group_id resolver" unless @config.sso_identity_store_id + end + + def resolve(value) + sso_group_id_finder.find(value, @config.sso_identity_store_id) + end + + private + def sso_group_id_finder + StackMaster::SsoGroupIdFinder.new(@stack_definition.region) + end + end + end +end diff --git a/lib/stack_master/sso_group_id_finder.rb b/lib/stack_master/sso_group_id_finder.rb new file mode 100644 index 00000000..16fd2aa2 --- /dev/null +++ b/lib/stack_master/sso_group_id_finder.rb @@ -0,0 +1,35 @@ +module StackMaster + class SsoGroupIdFinder + SSOGroupNotFound = Class.new(StandardError) + SSOIdentityStoreInvalid = Class.new(StandardError) + + def initialize(region) + @client = Aws::IdentityStore::Client.new({ region: region }) + end + + def find(reference, identity_store_id) + raise ArgumentError, 'SSO Group Name must be a non-empty string' unless reference.is_a?(String) && !reference.empty? + + next_token = nil + all_sso_groups = [] + begin + loop do + response = @client.list_groups({ + identity_store_id: identity_store_id, + next_token: next_token, + max_results: 50 + }) + + matching_group = response.groups.find { |group| group.display_name == reference } + return matching_group.group_id if matching_group + break unless response.next_token + next_token = response.next_token + end + rescue Aws::IdentityStore::Errors::ServiceError => e + puts "Error calling ListGroups: #{e.message}" + end + + raise SSOGroupNotFound, "No group with name #{reference} found" + end + end +end diff --git a/lib/stack_master/version.rb b/lib/stack_master/version.rb index 1fba44ac..74e1149d 100644 --- a/lib/stack_master/version.rb +++ b/lib/stack_master/version.rb @@ -1,3 +1,3 @@ module StackMaster - VERSION = "2.16.0" + VERSION = "2.17.8" end diff --git a/stack_master.gemspec b/stack_master.gemspec index 5d803c09..00f60a28 100644 --- a/stack_master.gemspec +++ b/stack_master.gemspec @@ -40,6 +40,7 @@ Gem::Specification.new do |spec| spec.add_dependency "aws-sdk-acm", "~> 1" spec.add_dependency "aws-sdk-cloudformation", "~> 1" spec.add_dependency "aws-sdk-ec2", "~> 1" + spec.add_dependency "aws-sdk-identitystore", "~> 1" spec.add_dependency "aws-sdk-s3", "~> 1" spec.add_dependency "aws-sdk-sns", "~> 1" spec.add_dependency "aws-sdk-ssm", "~> 1" From 380ff8243685dbd976cce77226826d02915fae1e Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 12:40:07 +1200 Subject: [PATCH 03/24] linting --- lib/stack_master/sso_group_id_finder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stack_master/sso_group_id_finder.rb b/lib/stack_master/sso_group_id_finder.rb index 16fd2aa2..56910552 100644 --- a/lib/stack_master/sso_group_id_finder.rb +++ b/lib/stack_master/sso_group_id_finder.rb @@ -29,7 +29,7 @@ def find(reference, identity_store_id) puts "Error calling ListGroups: #{e.message}" end - raise SSOGroupNotFound, "No group with name #{reference} found" + raise SSOGroupNotFound, "No group with name #{reference} found" end end end From 97feb2f2fa1c141755fa4b3707e3d0b133531b55 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 12:51:09 +1200 Subject: [PATCH 04/24] Add some documentation too --- README.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/README.md b/README.md index 04974de8..26557c40 100644 --- a/README.md +++ b/README.md @@ -416,6 +416,24 @@ ssh_sg: - WebAccessSecurityGroup ``` +### AWS IIC/SSO Group IDs + +Looks up AWS Identity Center group name in the configured Identity Store and returns the ID suitable for use in AWS IIC assignments. +It is likely that account and role will need to be specified to do the lookup. + +In stack_master.yml + +```yaml +sso_identity_store_id: `d-12345678` +``` + +In the parameter file itself + +```yaml +GroupId: + sso_group_id: 'SSO Group Name' +``` + ### SNS Topic Looks up an SNS topic by name and returns the ARN. From d0c6d62847f9c4adc2fd5641e317acb4424d0148 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 13:39:01 +1200 Subject: [PATCH 05/24] PR Feedback fixes Remove unused variables and exceptions Harmonise capitalisation of Sso --- lib/stack_master/sso_group_id_finder.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/stack_master/sso_group_id_finder.rb b/lib/stack_master/sso_group_id_finder.rb index 56910552..f7291fb9 100644 --- a/lib/stack_master/sso_group_id_finder.rb +++ b/lib/stack_master/sso_group_id_finder.rb @@ -1,7 +1,6 @@ module StackMaster class SsoGroupIdFinder - SSOGroupNotFound = Class.new(StandardError) - SSOIdentityStoreInvalid = Class.new(StandardError) + SsoGroupNotFound = Class.new(StandardError) def initialize(region) @client = Aws::IdentityStore::Client.new({ region: region }) @@ -11,7 +10,6 @@ def find(reference, identity_store_id) raise ArgumentError, 'SSO Group Name must be a non-empty string' unless reference.is_a?(String) && !reference.empty? next_token = nil - all_sso_groups = [] begin loop do response = @client.list_groups({ @@ -29,7 +27,7 @@ def find(reference, identity_store_id) puts "Error calling ListGroups: #{e.message}" end - raise SSOGroupNotFound, "No group with name #{reference} found" + raise SsoGroupNotFound, "No group with name #{reference} found" end end end From 6de910d4c291f8893a530d4047f8cbbaa1c9b7c0 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:12:20 +1200 Subject: [PATCH 06/24] Add tests for the finder class too --- spec/stack_master/sso_group_id_finder_spec.rb | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 spec/stack_master/sso_group_id_finder_spec.rb diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb new file mode 100644 index 00000000..720fb871 --- /dev/null +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -0,0 +1,96 @@ +RSpec.describe StackMaster::SsoGroupIdFinder do + let(:region) { 'us-east-1' } + let(:identity_store_id) { 'd-12345678' } + let(:group_name) { 'AdminGroup' } + let(:group_id) { 'abc-123-group-id' } + + let(:aws_client) { instance_double(Aws::IdentityStore::Client) } + + subject(:finder) { described_class.new(region) } + + before do + allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) + end + + context 'when the group is found on the first page' do + let(:response) do + double(groups: [double(display_name: group_name, group_id: group_id)], next_token: nil) + end + + it 'returns the group ID' do + expect(aws_client).to receive(:list_groups).with( + identity_store_id: identity_store_id, + next_token: nil, + max_results: 50 + ).and_return(response) + + result = finder.find(group_name, identity_store_id) + expect(result).to eq(group_id) + end + end + + context 'when the group is found on the second page' do + let(:page_1) do + double(groups: [double(display_name: 'OtherGroup', group_id: 'zzz')], next_token: 'page-2') + end + + let(:page_2) do + double(groups: [double(display_name: group_name, group_id: group_id)], next_token: nil) + end + + it 'returns the group ID after paging' do + expect(aws_client).to receive(:list_groups).with( + identity_store_id: identity_store_id, + next_token: nil, + max_results: 50 + ).and_return(page_1) + + expect(aws_client).to receive(:list_groups).with( + identity_store_id: identity_store_id, + next_token: 'page-2', + max_results: 50 + ).and_return(page_2) + + result = finder.find(group_name, identity_store_id) + expect(result).to eq(group_id) + end + end + + context 'when the group is not found' do + let(:response) do + double(groups: [double(display_name: 'AnotherGroup', group_id: 'zzz')], next_token: nil) + end + + it 'raises SsoGroupNotFound' do + expect(aws_client).to receive(:list_groups).and_return(response) + + expect { + finder.find(group_name, identity_store_id) + }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound, /No group with name #{group_name} found/) + end + end + + context 'when reference is empty or not a string' do + it 'raises ArgumentError for nil' do + expect { + finder.find(nil, identity_store_id) + }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) + end + + it 'raises ArgumentError for empty string' do + expect { + finder.find('', identity_store_id) + }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) + end + end + + context 'when AWS service error occurs' do + it 'rescues and raises SsoGroupNotFound' do + allow(aws_client).to receive(:list_groups).and_raise(Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS failure")) + + expect { + finder.find(group_name, identity_store_id) + }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) + end + end +end From 6fd6c5a4abded2b1e1a607f89d0d7dc160c97b33 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:26:09 +1200 Subject: [PATCH 07/24] Make ruby3.0+ compatible --- spec/stack_master/sso_group_id_finder_spec.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index 720fb871..728046ed 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -6,10 +6,10 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } - subject(:finder) { described_class.new(region) } - - before do - allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) + subject(:finder) do + # Ruby 3+ keyword args fix: make sure new accepts keyword args + allow(Aws::IdentityStore::Client).to receive(:new).with(hash_including(region: region)).and_return(aws_client) + described_class.new(region) end context 'when the group is found on the first page' do @@ -86,7 +86,8 @@ context 'when AWS service error occurs' do it 'rescues and raises SsoGroupNotFound' do - allow(aws_client).to receive(:list_groups).and_raise(Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS failure")) + error = Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS failure") + allow(aws_client).to receive(:list_groups).and_raise(error) expect { finder.find(group_name, identity_store_id) @@ -94,3 +95,4 @@ end end end + From eabc0664c23c07b7374e56d5c16498bd87a662a1 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:41:06 +1200 Subject: [PATCH 08/24] Handle positional args properly --- spec/stack_master/sso_group_id_finder_spec.rb | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index 728046ed..ecf5505a 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -7,8 +7,7 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } subject(:finder) do - # Ruby 3+ keyword args fix: make sure new accepts keyword args - allow(Aws::IdentityStore::Client).to receive(:new).with(hash_including(region: region)).and_return(aws_client) + allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) described_class.new(region) end @@ -56,13 +55,27 @@ end end - context 'when the group is not found' do - let(:response) do - double(groups: [double(display_name: 'AnotherGroup', group_id: 'zzz')], next_token: nil) + context 'when the group is not found after paging' do + let(:page_1) do + double(groups: [double(display_name: 'WrongGroup', group_id: 'aaa')], next_token: 'page-2') + end + + let(:page_2) do + double(groups: [double(display_name: 'AnotherWrongGroup', group_id: 'bbb')], next_token: nil) end it 'raises SsoGroupNotFound' do - expect(aws_client).to receive(:list_groups).and_return(response) + expect(aws_client).to receive(:list_groups).with( + identity_store_id: identity_store_id, + next_token: nil, + max_results: 50 + ).and_return(page_1) + + expect(aws_client).to receive(:list_groups).with( + identity_store_id: identity_store_id, + next_token: 'page-2', + max_results: 50 + ).and_return(page_2) expect { finder.find(group_name, identity_store_id) @@ -70,7 +83,7 @@ end end - context 'when reference is empty or not a string' do + context 'when the reference is invalid' do it 'raises ArgumentError for nil' do expect { finder.find(nil, identity_store_id) @@ -82,17 +95,23 @@ finder.find('', identity_store_id) }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) end + + it 'raises ArgumentError for non-string type' do + expect { + finder.find(12345, identity_store_id) + }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) + end end - context 'when AWS service error occurs' do - it 'rescues and raises SsoGroupNotFound' do - error = Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS failure") - allow(aws_client).to receive(:list_groups).and_raise(error) + context 'when AWS raises a service error' do + let(:aws_error) { Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS error") } + + it 'raises SsoGroupNotFound' do + allow(aws_client).to receive(:list_groups).and_raise(aws_error) expect { finder.find(group_name, identity_store_id) - }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) + }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound, /No group with name #{group_name} found/) end end end - From 8280eb39ad84f980de33f4bf84f08ec7cb332389 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:48:14 +1200 Subject: [PATCH 09/24] Another attempt to fix ruby3 incompatibilities --- spec/stack_master/sso_group_id_finder_spec.rb | 69 +++++++++++-------- 1 file changed, 41 insertions(+), 28 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index ecf5505a..96ad15f1 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -1,3 +1,5 @@ +require 'ostruct' + RSpec.describe StackMaster::SsoGroupIdFinder do let(:region) { 'us-east-1' } let(:identity_store_id) { 'd-12345678' } @@ -7,37 +9,46 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } subject(:finder) do - allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) + # Avoid Ruby 3.x keyword arg stubbing issues + allow(Aws::IdentityStore::Client).to receive(:new).and_return(aws_client) described_class.new(region) end context 'when the group is found on the first page' do let(:response) do - double(groups: [double(display_name: group_name, group_id: group_id)], next_token: nil) + OpenStruct.new( + groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], + next_token: nil + ) end - it 'returns the group ID' do + it 'returns the group ID immediately' do expect(aws_client).to receive(:list_groups).with( identity_store_id: identity_store_id, next_token: nil, max_results: 50 ).and_return(response) - result = finder.find(group_name, identity_store_id) - expect(result).to eq(group_id) + expect(finder.find(group_name, identity_store_id)).to eq(group_id) end end context 'when the group is found on the second page' do let(:page_1) do - double(groups: [double(display_name: 'OtherGroup', group_id: 'zzz')], next_token: 'page-2') + OpenStruct.new( + groups: [OpenStruct.new(display_name: 'SomeOtherGroup', group_id: 'wrong-id')], + next_token: 'next-token' + ) end let(:page_2) do - double(groups: [double(display_name: group_name, group_id: group_id)], next_token: nil) + OpenStruct.new( + groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], + next_token: nil + ) end - it 'returns the group ID after paging' do + it 'finds the group after paging' do expect(aws_client).to receive(:list_groups).with( identity_store_id: identity_store_id, next_token: nil, @@ -46,22 +57,27 @@ expect(aws_client).to receive(:list_groups).with( identity_store_id: identity_store_id, - next_token: 'page-2', + next_token: 'next-token', max_results: 50 ).and_return(page_2) - result = finder.find(group_name, identity_store_id) - expect(result).to eq(group_id) + expect(finder.find(group_name, identity_store_id)).to eq(group_id) end end - context 'when the group is not found after paging' do + context 'when the group is not found after all pages' do let(:page_1) do - double(groups: [double(display_name: 'WrongGroup', group_id: 'aaa')], next_token: 'page-2') + OpenStruct.new( + groups: [OpenStruct.new(display_name: 'Wrong1', group_id: 'id1')], + next_token: 'token-2' + ) end let(:page_2) do - double(groups: [double(display_name: 'AnotherWrongGroup', group_id: 'bbb')], next_token: nil) + OpenStruct.new( + groups: [OpenStruct.new(display_name: 'Wrong2', group_id: 'id2')], + next_token: nil + ) end it 'raises SsoGroupNotFound' do @@ -73,7 +89,7 @@ expect(aws_client).to receive(:list_groups).with( identity_store_id: identity_store_id, - next_token: 'page-2', + next_token: 'token-2', max_results: 50 ).and_return(page_2) @@ -83,35 +99,32 @@ end end - context 'when the reference is invalid' do + context 'when group name is invalid' do it 'raises ArgumentError for nil' do expect { finder.find(nil, identity_store_id) - }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) + }.to raise_error(ArgumentError, /must be a non-empty string/) end it 'raises ArgumentError for empty string' do expect { finder.find('', identity_store_id) - }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) - end - - it 'raises ArgumentError for non-string type' do - expect { - finder.find(12345, identity_store_id) - }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) + }.to raise_error(ArgumentError, /must be a non-empty string/) end end context 'when AWS raises a service error' do - let(:aws_error) { Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS error") } + it 'prints an error and raises SsoGroupNotFound' do + error = Aws::IdentityStore::Errors::ServiceError.new( + Seahorse::Client::RequestContext.new, + 'Simulated AWS error' + ) - it 'raises SsoGroupNotFound' do - allow(aws_client).to receive(:list_groups).and_raise(aws_error) + allow(aws_client).to receive(:list_groups).and_raise(error) expect { finder.find(group_name, identity_store_id) - }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound, /No group with name #{group_name} found/) + }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) end end end From 47d2b64abf64bd9092a689ab4dde150f6426d757 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:50:25 +1200 Subject: [PATCH 10/24] Revert "Another attempt to fix ruby3 incompatibilities" This reverts commit 8280eb39ad84f980de33f4bf84f08ec7cb332389. --- spec/stack_master/sso_group_id_finder_spec.rb | 69 ++++++++----------- 1 file changed, 28 insertions(+), 41 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index 96ad15f1..ecf5505a 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -1,5 +1,3 @@ -require 'ostruct' - RSpec.describe StackMaster::SsoGroupIdFinder do let(:region) { 'us-east-1' } let(:identity_store_id) { 'd-12345678' } @@ -9,46 +7,37 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } subject(:finder) do - # Avoid Ruby 3.x keyword arg stubbing issues - allow(Aws::IdentityStore::Client).to receive(:new).and_return(aws_client) + allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) described_class.new(region) end context 'when the group is found on the first page' do let(:response) do - OpenStruct.new( - groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], - next_token: nil - ) + double(groups: [double(display_name: group_name, group_id: group_id)], next_token: nil) end - it 'returns the group ID immediately' do + it 'returns the group ID' do expect(aws_client).to receive(:list_groups).with( identity_store_id: identity_store_id, next_token: nil, max_results: 50 ).and_return(response) - expect(finder.find(group_name, identity_store_id)).to eq(group_id) + result = finder.find(group_name, identity_store_id) + expect(result).to eq(group_id) end end context 'when the group is found on the second page' do let(:page_1) do - OpenStruct.new( - groups: [OpenStruct.new(display_name: 'SomeOtherGroup', group_id: 'wrong-id')], - next_token: 'next-token' - ) + double(groups: [double(display_name: 'OtherGroup', group_id: 'zzz')], next_token: 'page-2') end let(:page_2) do - OpenStruct.new( - groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], - next_token: nil - ) + double(groups: [double(display_name: group_name, group_id: group_id)], next_token: nil) end - it 'finds the group after paging' do + it 'returns the group ID after paging' do expect(aws_client).to receive(:list_groups).with( identity_store_id: identity_store_id, next_token: nil, @@ -57,27 +46,22 @@ expect(aws_client).to receive(:list_groups).with( identity_store_id: identity_store_id, - next_token: 'next-token', + next_token: 'page-2', max_results: 50 ).and_return(page_2) - expect(finder.find(group_name, identity_store_id)).to eq(group_id) + result = finder.find(group_name, identity_store_id) + expect(result).to eq(group_id) end end - context 'when the group is not found after all pages' do + context 'when the group is not found after paging' do let(:page_1) do - OpenStruct.new( - groups: [OpenStruct.new(display_name: 'Wrong1', group_id: 'id1')], - next_token: 'token-2' - ) + double(groups: [double(display_name: 'WrongGroup', group_id: 'aaa')], next_token: 'page-2') end let(:page_2) do - OpenStruct.new( - groups: [OpenStruct.new(display_name: 'Wrong2', group_id: 'id2')], - next_token: nil - ) + double(groups: [double(display_name: 'AnotherWrongGroup', group_id: 'bbb')], next_token: nil) end it 'raises SsoGroupNotFound' do @@ -89,7 +73,7 @@ expect(aws_client).to receive(:list_groups).with( identity_store_id: identity_store_id, - next_token: 'token-2', + next_token: 'page-2', max_results: 50 ).and_return(page_2) @@ -99,32 +83,35 @@ end end - context 'when group name is invalid' do + context 'when the reference is invalid' do it 'raises ArgumentError for nil' do expect { finder.find(nil, identity_store_id) - }.to raise_error(ArgumentError, /must be a non-empty string/) + }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) end it 'raises ArgumentError for empty string' do expect { finder.find('', identity_store_id) - }.to raise_error(ArgumentError, /must be a non-empty string/) + }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) + end + + it 'raises ArgumentError for non-string type' do + expect { + finder.find(12345, identity_store_id) + }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) end end context 'when AWS raises a service error' do - it 'prints an error and raises SsoGroupNotFound' do - error = Aws::IdentityStore::Errors::ServiceError.new( - Seahorse::Client::RequestContext.new, - 'Simulated AWS error' - ) + let(:aws_error) { Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS error") } - allow(aws_client).to receive(:list_groups).and_raise(error) + it 'raises SsoGroupNotFound' do + allow(aws_client).to receive(:list_groups).and_raise(aws_error) expect { finder.find(group_name, identity_store_id) - }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) + }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound, /No group with name #{group_name} found/) end end end From ba77d210e357312a17c928ddc43ac35fc4b4491a Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:50:50 +1200 Subject: [PATCH 11/24] Revert "Handle positional args properly" This reverts commit eabc0664c23c07b7374e56d5c16498bd87a662a1. --- spec/stack_master/sso_group_id_finder_spec.rb | 45 ++++++------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index ecf5505a..728046ed 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -7,7 +7,8 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } subject(:finder) do - allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) + # Ruby 3+ keyword args fix: make sure new accepts keyword args + allow(Aws::IdentityStore::Client).to receive(:new).with(hash_including(region: region)).and_return(aws_client) described_class.new(region) end @@ -55,27 +56,13 @@ end end - context 'when the group is not found after paging' do - let(:page_1) do - double(groups: [double(display_name: 'WrongGroup', group_id: 'aaa')], next_token: 'page-2') - end - - let(:page_2) do - double(groups: [double(display_name: 'AnotherWrongGroup', group_id: 'bbb')], next_token: nil) + context 'when the group is not found' do + let(:response) do + double(groups: [double(display_name: 'AnotherGroup', group_id: 'zzz')], next_token: nil) end it 'raises SsoGroupNotFound' do - expect(aws_client).to receive(:list_groups).with( - identity_store_id: identity_store_id, - next_token: nil, - max_results: 50 - ).and_return(page_1) - - expect(aws_client).to receive(:list_groups).with( - identity_store_id: identity_store_id, - next_token: 'page-2', - max_results: 50 - ).and_return(page_2) + expect(aws_client).to receive(:list_groups).and_return(response) expect { finder.find(group_name, identity_store_id) @@ -83,7 +70,7 @@ end end - context 'when the reference is invalid' do + context 'when reference is empty or not a string' do it 'raises ArgumentError for nil' do expect { finder.find(nil, identity_store_id) @@ -95,23 +82,17 @@ finder.find('', identity_store_id) }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) end - - it 'raises ArgumentError for non-string type' do - expect { - finder.find(12345, identity_store_id) - }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) - end end - context 'when AWS raises a service error' do - let(:aws_error) { Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS error") } - - it 'raises SsoGroupNotFound' do - allow(aws_client).to receive(:list_groups).and_raise(aws_error) + context 'when AWS service error occurs' do + it 'rescues and raises SsoGroupNotFound' do + error = Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS failure") + allow(aws_client).to receive(:list_groups).and_raise(error) expect { finder.find(group_name, identity_store_id) - }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound, /No group with name #{group_name} found/) + }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) end end end + From 40d4e7e388785b964120b340ef00096bb298c845 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:53:41 +1200 Subject: [PATCH 12/24] Let people know which directory was searched --- lib/stack_master/sso_group_id_finder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stack_master/sso_group_id_finder.rb b/lib/stack_master/sso_group_id_finder.rb index f7291fb9..1d012582 100644 --- a/lib/stack_master/sso_group_id_finder.rb +++ b/lib/stack_master/sso_group_id_finder.rb @@ -27,7 +27,7 @@ def find(reference, identity_store_id) puts "Error calling ListGroups: #{e.message}" end - raise SsoGroupNotFound, "No group with name #{reference} found" + raise SsoGroupNotFound, "No group with name #{reference} found in identity store #{identity_store_id}" end end end From f51bee82b36543a6237f03bfbcb26fadac4f68c1 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:56:31 +1200 Subject: [PATCH 13/24] Pass hash, not named arguments --- spec/stack_master/sso_group_id_finder_spec.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index 728046ed..c3a22bb2 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -7,8 +7,7 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } subject(:finder) do - # Ruby 3+ keyword args fix: make sure new accepts keyword args - allow(Aws::IdentityStore::Client).to receive(:new).with(hash_including(region: region)).and_return(aws_client) + allow(Aws::IdentityStore::Client).to receive(:new).with({ region: region }).and_return(aws_client) described_class.new(region) end @@ -39,16 +38,16 @@ end it 'returns the group ID after paging' do - expect(aws_client).to receive(:list_groups).with( + expect(aws_client).to receive(:list_groups).with({ identity_store_id: identity_store_id, next_token: nil, - max_results: 50 + max_results: 50} ).and_return(page_1) - expect(aws_client).to receive(:list_groups).with( + expect(aws_client).to receive(:list_groups).with({ identity_store_id: identity_store_id, next_token: 'page-2', - max_results: 50 + max_results: 50} ).and_return(page_2) result = finder.find(group_name, identity_store_id) From 3bf111c0e2d5ca4dc9f31d135293876f22d9df01 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 14:57:42 +1200 Subject: [PATCH 14/24] Pass hash, not named arguments (in all the places, not just some) --- spec/stack_master/sso_group_id_finder_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index c3a22bb2..6ae17b99 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -17,10 +17,10 @@ end it 'returns the group ID' do - expect(aws_client).to receive(:list_groups).with( + expect(aws_client).to receive(:list_groups).with({ identity_store_id: identity_store_id, next_token: nil, - max_results: 50 + max_results: 50} ).and_return(response) result = finder.find(group_name, identity_store_id) From 6f79b20232ce9ece8594cf123fbff95c866704d2 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 15:03:30 +1200 Subject: [PATCH 15/24] Use a before block --- spec/stack_master/sso_group_id_finder_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index 6ae17b99..f8b0242e 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -6,8 +6,11 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } - subject(:finder) do + begin do allow(Aws::IdentityStore::Client).to receive(:new).with({ region: region }).and_return(aws_client) + end + + subject(:finder) do described_class.new(region) end From 40255802deacf9f50fb66ea90843ad7c91d1b008 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 15:05:22 +1200 Subject: [PATCH 16/24] Umm, itchy tab key finger --- spec/stack_master/sso_group_id_finder_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index f8b0242e..8eec2277 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -6,7 +6,7 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } - begin do + before do allow(Aws::IdentityStore::Client).to receive(:new).with({ region: region }).and_return(aws_client) end From f55d90506d8ce6f4ae7494cb9d4991fb0cf2903c Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 15:54:53 +1200 Subject: [PATCH 17/24] Update docs and test for new way of specifying identity-store-id --- README.md | 16 +-- .../parameter_resolvers/sso_group_id_spec.rb | 58 ++++---- spec/stack_master/sso_group_id_finder_spec.rb | 124 +++++++++++------- 3 files changed, 122 insertions(+), 76 deletions(-) diff --git a/README.md b/README.md index 26557c40..e6cc62a1 100644 --- a/README.md +++ b/README.md @@ -419,19 +419,19 @@ ssh_sg: ### AWS IIC/SSO Group IDs Looks up AWS Identity Center group name in the configured Identity Store and returns the ID suitable for use in AWS IIC assignments. -It is likely that account and role will need to be specified to do the lookup. - -In stack_master.yml +It is likely that account and role will need to be specified to do the lookup, the region specification is optional it defaults to stack region. ```yaml -sso_identity_store_id: `d-12345678` +GroupId: + sso_group_id: '[region:]identity-store-id/SSO Group Name' ``` -In the parameter file itself - +e.g. ```yaml -GroupId: - sso_group_id: 'SSO Group Name' +GroupIdNotInStackRegion: + sso_group_id: 'us-east-1:d-123456df8:Okta-App-AWS-FooBar' +GroupIdInStackRegion: + sso_group_id: 'd-123456df8:Okta-App-AWS-FooBar' ``` ### SNS Topic diff --git a/spec/stack_master/parameter_resolvers/sso_group_id_spec.rb b/spec/stack_master/parameter_resolvers/sso_group_id_spec.rb index 7644f287..abc7d518 100644 --- a/spec/stack_master/parameter_resolvers/sso_group_id_spec.rb +++ b/spec/stack_master/parameter_resolvers/sso_group_id_spec.rb @@ -1,38 +1,50 @@ +require 'spec_helper' + RSpec.describe StackMaster::ParameterResolvers::SsoGroupId do - describe "#resolve" do - let(:identity_store_id) { 'd-12345678' } - let(:sso_group_id) { '64e804c8-8091-7093-3da9-123456789012' } - let(:sso_group_name) { 'Okta-App-AWS-Group-Admin' } - let(:region) { 'us-east-1' } + let(:config) { instance_double('Config') } + let(:stack_definition) { instance_double('StackDefinition', region: 'us-east-1') } + + subject(:resolver) { described_class.new(config, stack_definition) } - let(:config) { instance_double('Config', sso_identity_store_id: identity_store_id) } - let(:stack_definition) { instance_double('StackDefinition', region: region) } - let(:finder) { instance_double(StackMaster::SsoGroupIdFinder) } + let(:group_reference) { 'us-east-1:d-12345678/AdminGroup' } + let(:resolved_group_id) { 'abc-123-group-id' } + let(:finder) { instance_double(StackMaster::SsoGroupIdFinder) } - subject(:resolver) { described_class.new(config, stack_definition) } + before do + allow(StackMaster::SsoGroupIdFinder).to receive(:new).and_return(finder) + end - before do - allow(StackMaster::SsoGroupIdFinder).to receive(:new).with(region).and_return(finder) - allow(finder).to receive(:find).with(sso_group_name, identity_store_id).and_return(sso_group_id) + describe '#resolve' do + context 'when group is found' do + it 'returns the resolved group ID' do + expect(finder).to receive(:find).with(group_reference).and_return(resolved_group_id) + + result = resolver.resolve(group_reference) + expect(result).to eq(resolved_group_id) + end end - context 'when given an SSO group name' do - it "finds the sso group id" do - expect(resolver.resolve(sso_group_name)).to eq sso_group_id + context 'when SsoGroupIdFinder raises an error' do + it 'propagates the SsoGroupNotFound error' do + allow(finder).to receive(:find).and_raise(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) + + expect { + resolver.resolve(group_reference) + }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) end end - context 'when sso_identity_store_id is missing' do - let(:config) { instance_double('Config', sso_identity_store_id: nil) } + context 'with invalid input' do + let(:invalid_reference) { 'not/a/valid/reference' } + + it 'raises ArgumentError from SsoGroupIdFinder' do + allow(finder).to receive(:find).and_raise(ArgumentError) - it 'raises an InvalidParameter error' do expect { - described_class.new(config, stack_definition) - }.to raise_error( - StackMaster::ParameterResolvers::SsoGroupId::InvalidParameter, - /sso_identity_store_id must be set/ - ) + resolver.resolve(invalid_reference) + }.to raise_error(ArgumentError) end end end end + diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index 8eec2277..27bbfc54 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -1,100 +1,134 @@ +require 'ostruct' + RSpec.describe StackMaster::SsoGroupIdFinder do - let(:region) { 'us-east-1' } let(:identity_store_id) { 'd-12345678' } let(:group_name) { 'AdminGroup' } let(:group_id) { 'abc-123-group-id' } + let(:region) { 'us-east-1' } + let(:reference) { "#{region}:#{identity_store_id}/#{group_name}" } let(:aws_client) { instance_double(Aws::IdentityStore::Client) } - before do - allow(Aws::IdentityStore::Client).to receive(:new).with({ region: region }).and_return(aws_client) - end - subject(:finder) do - described_class.new(region) + allow(Aws::IdentityStore::Client).to receive(:new).and_return(aws_client) + described_class.new end - context 'when the group is found on the first page' do - let(:response) do - double(groups: [double(display_name: group_name, group_id: group_id)], next_token: nil) - end + before do + # Stub StackMaster.cloud_formation_driver.region + allow(StackMaster).to receive(:cloud_formation_driver).and_return(double(region: region)) + end + context 'when group is found on first page' do it 'returns the group ID' do + page = OpenStruct.new( + groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], + next_token: nil + ) + expect(aws_client).to receive(:list_groups).with({ identity_store_id: identity_store_id, next_token: nil, max_results: 50} - ).and_return(response) + ).and_return(page) - result = finder.find(group_name, identity_store_id) - expect(result).to eq(group_id) + expect(finder.find(reference)).to eq(group_id) end end - context 'when the group is found on the second page' do - let(:page_1) do - double(groups: [double(display_name: 'OtherGroup', group_id: 'zzz')], next_token: 'page-2') - end + context 'when region is omitted' do + let(:reference) { "#{identity_store_id}/#{group_name}" } + + it 'uses region from StackMaster.cloud_formation_driver' do + page = OpenStruct.new( + groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], + next_token: nil + ) + + expect(aws_client).to receive(:list_groups).with({ + identity_store_id: identity_store_id, + next_token: nil, + max_results: 50} + ).and_return(page) - let(:page_2) do - double(groups: [double(display_name: group_name, group_id: group_id)], next_token: nil) + expect(finder.find(reference)).to eq(group_id) end + end + + context 'when group is found on second page' do + it 'paginates and returns the group ID' do + page1 = OpenStruct.new( + groups: [OpenStruct.new(display_name: 'OtherGroup', group_id: 'wrong')], + next_token: 'next123' + ) + + page2 = OpenStruct.new( + groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], + next_token: nil + ) - it 'returns the group ID after paging' do expect(aws_client).to receive(:list_groups).with({ identity_store_id: identity_store_id, next_token: nil, max_results: 50} - ).and_return(page_1) + ).and_return(page1) expect(aws_client).to receive(:list_groups).with({ identity_store_id: identity_store_id, - next_token: 'page-2', + next_token: 'next123', max_results: 50} - ).and_return(page_2) + ).and_return(page2) - result = finder.find(group_name, identity_store_id) - expect(result).to eq(group_id) + expect(finder.find(reference)).to eq(group_id) end end - context 'when the group is not found' do - let(:response) do - double(groups: [double(display_name: 'AnotherGroup', group_id: 'zzz')], next_token: nil) - end - + context 'when no matching group is found' do it 'raises SsoGroupNotFound' do - expect(aws_client).to receive(:list_groups).and_return(response) + page = OpenStruct.new( + groups: [OpenStruct.new(display_name: 'WrongGroup', group_id: 'x')], + next_token: nil + ) + + expect(aws_client).to receive(:list_groups).and_return(page) expect { - finder.find(group_name, identity_store_id) + finder.find(reference) }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound, /No group with name #{group_name} found/) end end - context 'when reference is empty or not a string' do - it 'raises ArgumentError for nil' do + context 'when input format is invalid' do + it 'raises ArgumentError for blank string' do expect { - finder.find(nil, identity_store_id) - }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) + finder.find('') + }.to raise_error(ArgumentError, /Sso group lookup parameter must be/) end - it 'raises ArgumentError for empty string' do + it 'raises ArgumentError for missing slash' do expect { - finder.find('', identity_store_id) - }.to raise_error(ArgumentError, /SSO Group Name must be a non-empty string/) + finder.find('region:storeid-and-no-group') + }.to raise_error(ArgumentError) + end + + it 'raises ArgumentError for non-string input' do + expect { + finder.find(12345) + }.to raise_error(ArgumentError) end end - context 'when AWS service error occurs' do - it 'rescues and raises SsoGroupNotFound' do - error = Aws::IdentityStore::Errors::ServiceError.new(nil, "AWS failure") - allow(aws_client).to receive(:list_groups).and_raise(error) + context 'when AWS service raises an error' do + it 'logs and raises SsoGroupNotFound' do + aws_error = Aws::IdentityStore::Errors::ServiceError.new( + Seahorse::Client::RequestContext.new, 'AWS failure' + ) + + allow(aws_client).to receive(:list_groups).and_raise(aws_error) expect { - finder.find(group_name, identity_store_id) + finder.find(reference) }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) end end end - From f702341b34676c2bbe81ddfa40ef47edd88e8787 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 15:59:28 +1200 Subject: [PATCH 18/24] Remove the top level sso_identity_store_id attribute --- lib/stack_master/config.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/stack_master/config.rb b/lib/stack_master/config.rb index e12d81aa..14cbc05a 100644 --- a/lib/stack_master/config.rb +++ b/lib/stack_master/config.rb @@ -17,7 +17,6 @@ def self.load!(config_file = 'stack_master.yml') attr_accessor :stacks, :base_dir, :template_dir, - :sso_identity_store_id, :parameters_dir, :stack_defaults, :region_defaults, @@ -42,7 +41,6 @@ def initialize(config, base_dir) @base_dir = base_dir @template_dir = config.fetch('template_dir', nil) @parameters_dir = config.fetch('parameters_dir', nil) - @sso_identity_store_id = config.fetch('sso_identity_store_id',nil) @stack_defaults = config.fetch('stack_defaults', {}) @region_aliases = Utils.underscore_keys_to_hyphen(config.fetch('region_aliases', {})) @region_to_aliases = @region_aliases.inject({}) do |hash, (key, value)| From af6e6bdda9546da80a9842e30963a5eb9b453b8f Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 16:00:14 +1200 Subject: [PATCH 19/24] Use the same format as stack_output does to specify the region,identity store and group name --- .../parameter_resolvers/sso_group_id.rb | 5 ++--- lib/stack_master/sso_group_id_finder.rb | 21 +++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/stack_master/parameter_resolvers/sso_group_id.rb b/lib/stack_master/parameter_resolvers/sso_group_id.rb index 521e4a3c..2f9ebc44 100644 --- a/lib/stack_master/parameter_resolvers/sso_group_id.rb +++ b/lib/stack_master/parameter_resolvers/sso_group_id.rb @@ -6,16 +6,15 @@ class SsoGroupId < Resolver def initialize(config, stack_definition) @config = config @stack_definition = stack_definition - raise InvalidParameter, "sso_identity_store_id must be set in stack_master.yml when using sso_group_id resolver" unless @config.sso_identity_store_id end def resolve(value) - sso_group_id_finder.find(value, @config.sso_identity_store_id) + sso_group_id_finder.find(value) end private def sso_group_id_finder - StackMaster::SsoGroupIdFinder.new(@stack_definition.region) + StackMaster::SsoGroupIdFinder.new() end end end diff --git a/lib/stack_master/sso_group_id_finder.rb b/lib/stack_master/sso_group_id_finder.rb index 1d012582..55ffae23 100644 --- a/lib/stack_master/sso_group_id_finder.rb +++ b/lib/stack_master/sso_group_id_finder.rb @@ -2,23 +2,26 @@ module StackMaster class SsoGroupIdFinder SsoGroupNotFound = Class.new(StandardError) - def initialize(region) - @client = Aws::IdentityStore::Client.new({ region: region }) - end + def find(reference) + output_regex = %r{(?:(?[^:]+):)?(?[^:/]+)/(?.+)} + + if !reference.is_a?(String) || !(match = output_regex.match(reference)) + raise ArgumentError, 'Sso group lookup parameter must be in the form of [region:]identity-store-id/group_name' + end - def find(reference, identity_store_id) - raise ArgumentError, 'SSO Group Name must be a non-empty string' unless reference.is_a?(String) && !reference.empty? + region = match[:region] || StackMaster.cloud_formation_driver.region + client = Aws::IdentityStore::Client.new({ region: region }) next_token = nil begin loop do - response = @client.list_groups({ - identity_store_id: identity_store_id, + response = client.list_groups({ + identity_store_id: match[:identity_store_id], next_token: next_token, max_results: 50 }) - matching_group = response.groups.find { |group| group.display_name == reference } + matching_group = response.groups.find { |group| group.display_name == match[:group_name] } return matching_group.group_id if matching_group break unless response.next_token next_token = response.next_token @@ -27,7 +30,7 @@ def find(reference, identity_store_id) puts "Error calling ListGroups: #{e.message}" end - raise SsoGroupNotFound, "No group with name #{reference} found in identity store #{identity_store_id}" + raise SsoGroupNotFound, "No group with name #{match[:group_name]} found in identity store #{match[:identity_store_id]} in #{region}" end end end From 45d27f09844a3812c3a0843045020b3ebafc5308 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 16:52:34 +1200 Subject: [PATCH 20/24] Use much more efficient method of finding group --- lib/stack_master/sso_group_id_finder.rb | 25 ++- lib/stack_master/version.rb | 2 +- spec/stack_master/sso_group_id_finder_spec.rb | 163 +++++++----------- 3 files changed, 74 insertions(+), 116 deletions(-) diff --git a/lib/stack_master/sso_group_id_finder.rb b/lib/stack_master/sso_group_id_finder.rb index 55ffae23..c2a32434 100644 --- a/lib/stack_master/sso_group_id_finder.rb +++ b/lib/stack_master/sso_group_id_finder.rb @@ -12,22 +12,19 @@ def find(reference) region = match[:region] || StackMaster.cloud_formation_driver.region client = Aws::IdentityStore::Client.new({ region: region }) - next_token = nil begin - loop do - response = client.list_groups({ - identity_store_id: match[:identity_store_id], - next_token: next_token, - max_results: 50 - }) - - matching_group = response.groups.find { |group| group.display_name == match[:group_name] } - return matching_group.group_id if matching_group - break unless response.next_token - next_token = response.next_token - end + response = client.get_group_id({ + identity_store_id: match[:identity_store_id], + alternate_identifier: { + unique_attribute: { + attribute_path: 'displayName', + attribute_value: match[:group_name], + }, + }, + }) + return response.group_id rescue Aws::IdentityStore::Errors::ServiceError => e - puts "Error calling ListGroups: #{e.message}" + puts "Error calling GetGroupId: #{e.message}" end raise SsoGroupNotFound, "No group with name #{match[:group_name]} found in identity store #{match[:identity_store_id]} in #{region}" diff --git a/lib/stack_master/version.rb b/lib/stack_master/version.rb index 74e1149d..92d0bac8 100644 --- a/lib/stack_master/version.rb +++ b/lib/stack_master/version.rb @@ -1,3 +1,3 @@ module StackMaster - VERSION = "2.17.8" + VERSION = "2.17.0" end diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index 27bbfc54..e85db988 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -1,134 +1,95 @@ -require 'ostruct' +require 'spec_helper' RSpec.describe StackMaster::SsoGroupIdFinder do - let(:identity_store_id) { 'd-12345678' } let(:group_name) { 'AdminGroup' } - let(:group_id) { 'abc-123-group-id' } + let(:identity_store_id) { 'd-12345678' } let(:region) { 'us-east-1' } let(:reference) { "#{region}:#{identity_store_id}/#{group_name}" } - let(:aws_client) { instance_double(Aws::IdentityStore::Client) } subject(:finder) do - allow(Aws::IdentityStore::Client).to receive(:new).and_return(aws_client) + allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) described_class.new end before do - # Stub StackMaster.cloud_formation_driver.region allow(StackMaster).to receive(:cloud_formation_driver).and_return(double(region: region)) end - context 'when group is found on first page' do - it 'returns the group ID' do - page = OpenStruct.new( - groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], - next_token: nil - ) - - expect(aws_client).to receive(:list_groups).with({ - identity_store_id: identity_store_id, - next_token: nil, - max_results: 50} - ).and_return(page) - - expect(finder.find(reference)).to eq(group_id) + describe '#find' do + context 'when the group is found successfully' do + it 'returns the group ID' do + group_id = 'abc-123-group-id' + + response = double(group_id: group_id) + expect(aws_client).to receive(:get_group_id).with( + identity_store_id: identity_store_id, + alternate_identifier: { + unique_attribute: { + attribute_path: 'displayName', + attribute_value: group_name + } + } + ).and_return(response) + + expect(finder.find(reference)).to eq(group_id) + end end - end - - context 'when region is omitted' do - let(:reference) { "#{identity_store_id}/#{group_name}" } - - it 'uses region from StackMaster.cloud_formation_driver' do - page = OpenStruct.new( - groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], - next_token: nil - ) - expect(aws_client).to receive(:list_groups).with({ - identity_store_id: identity_store_id, - next_token: nil, - max_results: 50} - ).and_return(page) + context 'when the group is not found' do + it 'raises SsoGroupNotFound' do + error = Aws::IdentityStore::Errors::ResourceNotFoundException.new( + Seahorse::Client::RequestContext.new, + "Group not found" + ) - expect(finder.find(reference)).to eq(group_id) - end - end + expect(aws_client).to receive(:get_group_id).and_raise(error) - context 'when group is found on second page' do - it 'paginates and returns the group ID' do - page1 = OpenStruct.new( - groups: [OpenStruct.new(display_name: 'OtherGroup', group_id: 'wrong')], - next_token: 'next123' - ) - - page2 = OpenStruct.new( - groups: [OpenStruct.new(display_name: group_name, group_id: group_id)], - next_token: nil - ) - - expect(aws_client).to receive(:list_groups).with({ - identity_store_id: identity_store_id, - next_token: nil, - max_results: 50} - ).and_return(page1) - - expect(aws_client).to receive(:list_groups).with({ - identity_store_id: identity_store_id, - next_token: 'next123', - max_results: 50} - ).and_return(page2) - - expect(finder.find(reference)).to eq(group_id) + expect { + finder.find(reference) + }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound, /No group with name #{group_name} found/) + end end - end - context 'when no matching group is found' do - it 'raises SsoGroupNotFound' do - page = OpenStruct.new( - groups: [OpenStruct.new(display_name: 'WrongGroup', group_id: 'x')], - next_token: nil - ) + context 'when region is not provided in reference' do + let(:reference_without_region) { "#{identity_store_id}/#{group_name}" } - expect(aws_client).to receive(:list_groups).and_return(page) + it 'uses the fallback region from cloud_formation_driver' do + allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) - expect { - finder.find(reference) - }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound, /No group with name #{group_name} found/) - end - end + group_id = 'fallback-region-group-id' + response = double(group_id: group_id) - context 'when input format is invalid' do - it 'raises ArgumentError for blank string' do - expect { - finder.find('') - }.to raise_error(ArgumentError, /Sso group lookup parameter must be/) - end + expect(aws_client).to receive(:get_group_id).with( + identity_store_id: identity_store_id, + alternate_identifier: { + unique_attribute: { + attribute_path: 'displayName', + attribute_value: group_name + } + } + ).and_return(response) - it 'raises ArgumentError for missing slash' do - expect { - finder.find('region:storeid-and-no-group') - }.to raise_error(ArgumentError) + expect(finder.find(reference_without_region)).to eq(group_id) + end end - it 'raises ArgumentError for non-string input' do - expect { - finder.find(12345) - }.to raise_error(ArgumentError) + context 'when input is not a string' do + it 'raises ArgumentError' do + expect { + finder.find(123) + }.to raise_error(ArgumentError, /Sso group lookup parameter must be in the form/) + end end - end - - context 'when AWS service raises an error' do - it 'logs and raises SsoGroupNotFound' do - aws_error = Aws::IdentityStore::Errors::ServiceError.new( - Seahorse::Client::RequestContext.new, 'AWS failure' - ) - allow(aws_client).to receive(:list_groups).and_raise(aws_error) + context 'when input is an invalid string' do + it 'raises ArgumentError' do + invalid_reference = 'badformat' - expect { - finder.find(reference) - }.to raise_error(StackMaster::SsoGroupIdFinder::SsoGroupNotFound) + expect { + finder.find(invalid_reference) + }.to raise_error(ArgumentError, /Sso group lookup parameter must be in the form/) + end end end end From af64e1d7fac8e0238934b8bf60d41da456fe9602 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 22:03:57 +1200 Subject: [PATCH 21/24] hash not keywords again --- spec/stack_master/sso_group_id_finder_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index e85db988..675a676d 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -22,7 +22,7 @@ group_id = 'abc-123-group-id' response = double(group_id: group_id) - expect(aws_client).to receive(:get_group_id).with( + expect(aws_client).to receive(:get_group_id).with({ identity_store_id: identity_store_id, alternate_identifier: { unique_attribute: { @@ -30,7 +30,7 @@ attribute_value: group_name } } - ).and_return(response) + }).and_return(response) expect(finder.find(reference)).to eq(group_id) end @@ -60,7 +60,7 @@ group_id = 'fallback-region-group-id' response = double(group_id: group_id) - expect(aws_client).to receive(:get_group_id).with( + expect(aws_client).to receive(:get_group_id).with({ identity_store_id: identity_store_id, alternate_identifier: { unique_attribute: { @@ -68,7 +68,7 @@ attribute_value: group_name } } - ).and_return(response) + }).and_return(response) expect(finder.find(reference_without_region)).to eq(group_id) end From 43f67e461e0b934673c7704ba709ebf61cef82fe Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Thu, 10 Jul 2025 22:06:22 +1200 Subject: [PATCH 22/24] And again --- spec/stack_master/sso_group_id_finder_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/stack_master/sso_group_id_finder_spec.rb b/spec/stack_master/sso_group_id_finder_spec.rb index 675a676d..7a34db74 100644 --- a/spec/stack_master/sso_group_id_finder_spec.rb +++ b/spec/stack_master/sso_group_id_finder_spec.rb @@ -8,7 +8,7 @@ let(:aws_client) { instance_double(Aws::IdentityStore::Client) } subject(:finder) do - allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) + allow(Aws::IdentityStore::Client).to receive(:new).with({region: region}).and_return(aws_client) described_class.new end @@ -55,7 +55,7 @@ let(:reference_without_region) { "#{identity_store_id}/#{group_name}" } it 'uses the fallback region from cloud_formation_driver' do - allow(Aws::IdentityStore::Client).to receive(:new).with(region: region).and_return(aws_client) + allow(Aws::IdentityStore::Client).to receive(:new).with({region: region}).and_return(aws_client) group_id = 'fallback-region-group-id' response = double(group_id: group_id) From f8b6f675d7ed7d7c4ce3b301ada19ce3dbac71dd Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Fri, 11 Jul 2025 01:10:33 +1200 Subject: [PATCH 23/24] Update with PR details --- CHANGELOG.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7adb867c..2f60ae25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,18 @@ The format is based on [Keep a Changelog], and this project adheres to ## [Unreleased] -[Unreleased]: https://github.com/envato/stack_master/compare/v2.16.0...HEAD +[Unreleased]: https://github.com/envato/stack_master/compare/v2.17.0...HEAD + +## [2.17.0] - 2025-07-11 + +### Added + +- Add a parameter resolver for AWS SSO/IIC mapping group display name to id ([#390]) + +```yaml +group_id: + sso_group_id: "us-east-1:d-123456bf8/SSO Group Display Name"" +``` ## [2.16.0] - 2024-08-01 From d6cb2ae5663e7b6b01060940a27ec6d8fc199324 Mon Sep 17 00:00:00 2001 From: Andrew Humphrey Date: Fri, 11 Jul 2025 01:36:10 +1200 Subject: [PATCH 24/24] Remove double double quote --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f60ae25..219a3d9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ The format is based on [Keep a Changelog], and this project adheres to ```yaml group_id: - sso_group_id: "us-east-1:d-123456bf8/SSO Group Display Name"" + sso_group_id: "us-east-1:d-123456bf8/SSO Group Display Name" ``` ## [2.16.0] - 2024-08-01