diff --git a/README.md b/README.md index 14424467..92940b43 100644 --- a/README.md +++ b/README.md @@ -603,9 +603,9 @@ The path for `install_dir` will be joined with `bin/kcadm.sh` to produce the ful The keycloak_required_action type can be used to define actions a user must perform during the authentication process. A user will not be able to complete the authentication process until these actions are complete. For instance, change a one-time password, accept T&C, etc. -The name for an action is `$alias on $realm`. +The name for an action is `$provider_id on $realm`. -**Important**: actions from puppet config and from a server are matched based on a combination of alias and realm, so edition of aliases is not supported. +**Important**: The keycloak rest api documentation uses the term `alias` which will be filled with the value of `provider_id` in this module. ```puppet # Minimal example diff --git a/lib/puppet/provider/keycloak_required_action/kcadm.rb b/lib/puppet/provider/keycloak_required_action/kcadm.rb index 631dddb0..8d9a78f6 100644 --- a/lib/puppet/provider/keycloak_required_action/kcadm.rb +++ b/lib/puppet/provider/keycloak_required_action/kcadm.rb @@ -11,7 +11,7 @@ def self.prefetch(resources) action_providers = instances resources.each_key do |name| provider = action_providers.find do |c| - c.alias == resources[name][:alias] && c.realm == resources[name][:realm] + c.provider_id == resources[name][:provider_id] && c.realm == resources[name][:realm] end if provider resources[name].provider = provider @@ -34,7 +34,6 @@ def self.instances required_actions.each do |a| action = { ensure: :present, - alias: a['alias'], display_name: a['name'], realm: realm, enabled: a['enabled'], @@ -61,7 +60,6 @@ def self.instances unregistered_actions.each do |a| action = { ensure: :absent, - alias: a['providerId'], display_name: a['name'], realm: realm, enabled: false, @@ -105,18 +103,17 @@ def create # Asigning property_flush to is needed to make the flush method to # configure properties of the required action after the registration. @property_flush = resource.to_hash - @property_hash[:alias] = resource[:provider_id] # Initially it's equal to the provider id until configuration is applied to it @property_hash[:ensure] = :present end def destroy Puppet.debug('Keycloak required action: destroy') begin - kcadm('delete', "authentication/required-actions/#{@property_hash[:alias]}", resource[:realm]) + kcadm('delete', "authentication/required-actions/#{@property_hash[:provider_id]}", resource[:realm]) rescue StandardError => e raise Puppet::Error, "kcadm deletion of required action failed\nError message: #{e.message}" end - Puppet.info("Keycloak: deregistered required action #{@property_hash[:alias]} for #{resource[:realm]}") + Puppet.info("Keycloak: deregistered required action #{@property_hash[:provider_id]} for #{resource[:realm]}") @property_hash.clear end @@ -130,7 +127,7 @@ def flush begin t = Tempfile.new('keycloak_required_action_configure') - t.write(JSON.pretty_generate(alias: resource[:alias], + t.write(JSON.pretty_generate(alias: resource[:provider_id], name: resource[:display_name] || @property_hash[:display_name], enabled: resource[:enabled], priority: resource[:priority], @@ -138,8 +135,8 @@ def flush defaultAction: resource[:default])) t.close Puppet.debug(IO.read(t.path)) - kcadm('update', "authentication/required-actions/#{@property_hash[:alias]}", resource[:realm], t.path) - Puppet.info("Keycloak: configured required action #{@property_hash[:alias]} (provider #{resource[:provider_id]}) for #{resource[:realm]}") + kcadm('update', "authentication/required-actions/#{@property_hash[:provider_id]}", resource[:realm], t.path) + Puppet.info("Keycloak: configured required action #{@property_hash[:display_name]} (provider #{resource[:provider_id]}) for #{resource[:realm]}") rescue StandardError => e raise Puppet::Error, "kcadm configuration of required action failed\nError message: #{e.message}" end @@ -150,7 +147,6 @@ def flush def to_keycloak_representation(resource) { - alias: resource[:alias], name: resource[:display_name], realm: resource[:realm], providerId: resource[:provider_id], diff --git a/lib/puppet/type/keycloak_required_action.rb b/lib/puppet/type/keycloak_required_action.rb index 5a8892f4..00ef18c7 100644 --- a/lib/puppet/type/keycloak_required_action.rb +++ b/lib/puppet/type/keycloak_required_action.rb @@ -9,7 +9,6 @@ @example Enable Webauthn Register and make it default keycloak_required_action { 'webauthn-register on master': ensure => present, - alias => 'webauthn-register', provider_id => 'webauthn-register', display_name => 'Webauthn Register', default => true, @@ -40,16 +39,9 @@ desc 'realm' end - newparam(:alias, namevar: true) do - desc 'Alias.' - end - - newparam(:provider_id) do - desc 'providerId of the required action. Default to `alias`' + newparam(:provider_id, namevar: true) do + desc 'providerId of the required action.' munge { |v| v.to_s } - defaultto do - @resource[:alias] - end end newproperty(:display_name) do @@ -107,7 +99,7 @@ def self.title_patterns %r{^((\S+) on (\S+))$}, [ [:name], - [:alias], + [:provider_id], [:realm] ] ], @@ -122,7 +114,7 @@ def self.title_patterns validate do required_properties = [ - :alias, + :provider_id, :realm ] required_properties.each do |property| diff --git a/spec/acceptance/10_required_action_spec.rb b/spec/acceptance/10_required_action_spec.rb index f9fa45ad..4e010060 100644 --- a/spec/acceptance/10_required_action_spec.rb +++ b/spec/acceptance/10_required_action_spec.rb @@ -87,4 +87,31 @@ class { 'keycloak': } end end end + + context 'when required action with multiple realms' do + it 'runs successfully' do + pp = <<-PUPPET_PP + class { 'keycloak': } + keycloak_realm { 'test': ensure => 'present' } + keycloak_realm { 'test2': ensure => 'present' } + keycloak_required_action { 'webauthn-register on test': + ensure => 'present', + display_name => 'Webauthn Register', + default => true, + enabled => true, + priority => 200, + } + keycloak_required_action { 'webauthn-register on test2': + ensure => 'present', + display_name => 'Webauthn Register', + default => true, + enabled => true, + priority => 200, + } + PUPPET_PP + + apply_manifest(pp, catch_failures: true) + apply_manifest(pp, catch_changes: true) + end + end end diff --git a/spec/unit/puppet/provider/keycloak_required_action/kcadm_spec.rb b/spec/unit/puppet/provider/keycloak_required_action/kcadm_spec.rb index b8eb07f6..45c947a2 100644 --- a/spec/unit/puppet/provider/keycloak_required_action/kcadm_spec.rb +++ b/spec/unit/puppet/provider/keycloak_required_action/kcadm_spec.rb @@ -9,7 +9,6 @@ let(:resource) do type.new(name: 'foo', realm: 'test', - alias: 'somealias', provider_id: 'webauthn-register') end @@ -50,11 +49,9 @@ describe 'destroy' do it 'deregisters a required action' do - # It suppoed to use whatever came from api and was matched by provider id - # But not what developer provided - resource.provider.instance_variable_set(:@property_hash, alias: 'otheralias') + resource.provider.instance_variable_set(:@property_hash, resource.to_hash) - expect(resource.provider).to receive(:kcadm).with('delete', 'authentication/required-actions/otheralias', 'test') + expect(resource.provider).to receive(:kcadm).with('delete', 'authentication/required-actions/webauthn-register', 'test') resource.provider.destroy @@ -77,7 +74,7 @@ temp = Tempfile.new('keycloak_required_action_configure') allow(Tempfile).to receive(:new).with('keycloak_required_action_configure').and_return(temp) - expect(resource.provider).to receive(:kcadm).with('update', 'authentication/required-actions/somealias', 'test', temp.path) + expect(resource.provider).to receive(:kcadm).with('update', 'authentication/required-actions/webauthn-register', 'test', temp.path) resource.provider.display_name = 'something' resource.provider.flush @@ -86,11 +83,11 @@ # If developer does not specify the display name, the api would use the name # that is initially returned from unregistered-required-actions it 'uses display_name from current state if none specified explicitly' do - resource.provider.instance_variable_set(:@property_hash, display_name: 'display name', alias: 'somealias') + resource.provider.instance_variable_set(:@property_hash, display_name: 'display name', provider_id: 'webauthn-register') temp = Tempfile.new('keycloak_required_action_configure') allow(Tempfile).to receive(:new).with('keycloak_required_action_configure').and_return(temp) - expect(resource.provider).to receive(:kcadm).with('update', 'authentication/required-actions/somealias', 'test', temp.path) + expect(resource.provider).to receive(:kcadm).with('update', 'authentication/required-actions/webauthn-register', 'test', temp.path) resource.provider.priority = 1000 resource.provider.flush @@ -106,7 +103,7 @@ temp = Tempfile.new('keycloak_required_action_configure') allow(Tempfile).to receive(:new).with('keycloak_required_action_configure').and_return(temp) - expect(resource.provider).to receive(:kcadm).with('update', 'authentication/required-actions/somealias', 'test', temp.path) + expect(resource.provider).to receive(:kcadm).with('update', 'authentication/required-actions/webauthn-register', 'test', temp.path) resource.provider.priority = 200 resource.provider.flush @@ -115,18 +112,5 @@ json = JSON.parse(data) expect(json['name']).to eq('something') end - - it 'always uses alias from the current state to make edits' do - resource[:display_name] = 'newalias' - resource.provider.instance_variable_set(:@property_hash, alias: 'current') - - temp = Tempfile.new('keycloak_required_action_configure') - allow(Tempfile).to receive(:new).with('keycloak_required_action_configure').and_return(temp) - - expect(resource.provider).to receive(:kcadm).with('update', 'authentication/required-actions/current', 'test', temp.path) - - resource.provider.priority = 200 - resource.provider.flush - end end end diff --git a/spec/unit/puppet/type/keycloak_required_action_spec.rb b/spec/unit/puppet/type/keycloak_required_action_spec.rb index 912f9751..cb2fc643 100644 --- a/spec/unit/puppet/type/keycloak_required_action_spec.rb +++ b/spec/unit/puppet/type/keycloak_required_action_spec.rb @@ -7,7 +7,6 @@ { name: 'foo', realm: 'test', - alias: 'something', provider_id: 'some-provider' } end @@ -25,15 +24,9 @@ }.not_to raise_error end - it 'has alias default to provider_id' do - config.delete(:provider_id) - expect(resource[:provider_id]).to eq('something') - end - it 'handles componsite name' do component = described_class.new(name: 'foo on test') expect(component[:name]).to eq('foo on test') - expect(component[:alias]).to eq('foo') expect(component[:provider_id]).to eq('foo') expect(component[:realm]).to eq('test') end @@ -49,8 +42,7 @@ :realm, :name, :display_name, - :provider_id, - :alias + :provider_id ].each do |p| it "accepts a #{p}" do config[p] = 'foo' @@ -163,16 +155,9 @@ expect { resource }.to raise_error(%r{must have a realm defined}) end - it 'requires alias' do - config.delete(:provider_id) - config.delete(:alias) - expect { resource }.to raise_error(%r{must have a alias defined}) - end - - it 'does not require provider_id for absent' do + it 'requires provider_id' do config.delete(:provider_id) - config[:ensure] = 'absent' - expect { resource }.not_to raise_error + expect { resource }.to raise_error(%r{must have a provider_id defined}) end end end