diff --git a/.gitignore b/.gitignore index 2aaec7a..875e8f2 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,5 @@ *.pyc build src/tests/bundles/overlays/local-charm-overlay.yaml.j2 +interfaces +layers diff --git a/src/README.md b/src/README.md index f7fda9f..f40aa89 100644 --- a/src/README.md +++ b/src/README.md @@ -47,6 +47,7 @@ In a bundle: idp-name: 'samltest' protocol-name: 'mapped' user-facing-name: "samltest.id' + nameid-formats="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" resources: idp-metadata: "./idp-metadata.xml" sp-signing-keyinfo: "./sp-keyinfo.xml" diff --git a/src/lib/charm/openstack/keystone_saml_mellon.py b/src/lib/charm/openstack/keystone_saml_mellon.py index 94fc428..72b250d 100644 --- a/src/lib/charm/openstack/keystone_saml_mellon.py +++ b/src/lib/charm/openstack/keystone_saml_mellon.py @@ -16,10 +16,8 @@ import charmhelpers.core as core import charmhelpers.core.host as ch_host import charmhelpers.core.hookenv as hookenv -import charmhelpers.core.unitdata as unitdata import charmhelpers.contrib.openstack.templating as os_templating -import charmhelpers.contrib.openstack.utils as os_utils import charms_openstack.charm import charms_openstack.adapters @@ -31,14 +29,6 @@ from lxml import etree from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization -# release detection is done via keystone package given that -# openstack-origin is not present in the subordinate charm -# see https://github.com/juju/charm-helpers/issues/83 -from charms_openstack.charm.core import ( - register_os_release_selector -) -OPENSTACK_RELEASE_KEY = 'charmers.openstack-release-version' - CONFIGS = (IDP_METADATA, SP_METADATA, SP_PRIVATE_KEY, SP_LOCATION_CONFIG,) = [ os.path.join('/etc/apache2/mellon', @@ -49,21 +39,6 @@ CONFIGS = (IDP_METADATA, SP_METADATA, SP_PRIVATE_KEY, 'sp-location.{}.conf']] -@register_os_release_selector -def select_release(): - """Determine the release based on the keystone package version. - - Note that this function caches the release after the first install so - that it doesn't need to keep going and getting it from the package - information. - """ - release_version = unitdata.kv().get(OPENSTACK_RELEASE_KEY, None) - if release_version is None: - release_version = os_utils.os_release('keystone') - unitdata.kv().set(OPENSTACK_RELEASE_KEY, release_version) - return release_version - - class KeystoneSAMLMellonConfigurationAdapter( charms_openstack.adapters.ConfigurationAdapter): @@ -101,24 +76,6 @@ class KeystoneSAMLMellonConfigurationAdapter( def sp_location_config(self): return SP_LOCATION_CONFIG - @property - def keystone_host(self): - return unitdata.kv().get('hostname') - - @property - def keystone_port(self): - return unitdata.kv().get('port') - - @property - def tls_enabled(self): - return unitdata.kv().get('tls-enabled') - - @property - def keystone_base_url(self): - scheme = 'https' if self.tls_enabled else 'http' - return ('{}://{}:{}'.format(scheme, self.keystone_host, - self.keystone_port)) - @property def sp_idp_path(self): return ('/v3/OS-FEDERATION/identity_providers/{}' @@ -158,21 +115,6 @@ class KeystoneSAMLMellonConfigurationAdapter( def sp_logout_path(self): return '{}/logout'.format(self.mellon_endpoint_path) - @property - def sp_auth_url(self): - return '{}{}'.format(self.keystone_base_url, - self.sp_auth_path) - - @property - def sp_logout_url(self): - return '{}{}'.format(self.keystone_base_url, - self.sp_logout_path) - - @property - def sp_post_response_url(self): - return '{}{}'.format(self.keystone_base_url, - self.sp_post_response_path) - @property def mellon_subject_confirmation_data_address_check(self): return ('On' if self.subject_confirmation_data_address_check @@ -256,6 +198,13 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm): # First release supported release = 'mitaka' + release_pkg = 'keystone-common' + + # Required relations + required_relations = [ + 'keystone-fid-service-provider', + 'websso-fid-service-provider'] + # List of packages to install for this charm packages = ['libapache2-mod-auth-mellon'] @@ -275,6 +224,13 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm): # ownership. group = 'www-data' + restart_map = { + IDP_METADATA: [], + SP_METADATA: [], + SP_PRIVATE_KEY: [], + SP_LOCATION_CONFIG: [], + } + def configuration_complete(self): """Determine whether sufficient configuration has been provided via charm config options and resources. @@ -292,21 +248,22 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm): return all(required_config.values()) - def assess_status(self): - """Determine the current application status for the charm""" - hookenv.application_version_set(self.application_version) + def custom_assess_status_check(self): + """Custom asses status. + + Check the configuration is complete. + """ if not self.configuration_complete(): errors = [ '{}: {}'.format(k, v) - for k, v in self.options.validation_errors.items() if v] + for k, v in self.options.validation_errors.items()] status_msg = 'Configuration is incomplete. {}'.format( ','.join(errors)) - hookenv.status_set('blocked', status_msg) - else: - hookenv.status_set('active', - 'Unit is ready') + return 'blocked', status_msg + # Nothing to report + return None, None - def render_config(self): + def render_config(self, *args): """ Render Service Provider configuration file to be used by Apache and provided to idP out of band to establish mutual trust. @@ -323,14 +280,21 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm): # ensure that a directory we need is there ch_host.mkdir('/etc/apache2/mellon', perms=dperms, owner=owner, group=group) + self.render_configs(self.string_templates.keys()) + # For now the template name does not match + # basename(file_path/file_name). This is necessary to enable multiple + # instantiations of keystone-saml-mellon using service_name() in the + # file names. So not using self.render_with_interfaces(args) + # TODO: Make a mapping mechanism between target and source templates + # in charms.openstack core.templating.render( source='mellon-sp-metadata.xml', template_loader=os_templating.get_loader( 'templates/', self.release), target=self.options.sp_metadata_file, - context=self.adapters_instance, + context=self.adapters_class(args, charm_instance=self), owner=owner, group=group, perms=fileperms @@ -341,14 +305,14 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm): template_loader=os_templating.get_loader( 'templates/', self.release), target=self.options.sp_location_config, - context=self.adapters_instance, + context=self.adapters_class(args, charm_instance=self), owner=owner, group=group, perms=fileperms ) def remove_config(self): - for f in CONFIGS: + for f in self.restart_map.keys(): if os.path.exists(f): os.unlink(f) diff --git a/src/reactive/keystone_saml_mellon_handlers.py b/src/reactive/keystone_saml_mellon_handlers.py index 05fe788..bcc61cf 100644 --- a/src/reactive/keystone_saml_mellon_handlers.py +++ b/src/reactive/keystone_saml_mellon_handlers.py @@ -13,51 +13,20 @@ # See the License for the specific language governing permissions and # limitations under the License. -import uuid +import charms_openstack.bus +charms_openstack.bus.discover() -# import to trigger openstack charm metaclass init -import charm.openstack.keystone_saml_mellon # noqa import charms_openstack.charm as charm import charms.reactive as reactive -import charms.reactive.flags as flags - -import charmhelpers.core.unitdata as unitdata - -from charms.reactive.relations import ( - endpoint_from_flag, -) charm.use_defaults( 'charm.installed', - 'update-status') - -# if config has been changed we need to re-evaluate flags -# config.changed is set and cleared (atexit) in layer-basic -flags.register_trigger(when='config.changed', - clear_flag='config.rendered') -flags.register_trigger(when='upgraded', clear_flag='config.rendered') -flags.register_trigger(when='config.changed', - clear_flag='config.complete') -flags.register_trigger( - when='endpoint.keystone-fid-service-provider.changed', - clear_flag='keystone-data.complete' -) + 'update-status', + 'upgrade-charm') -@reactive.hook('upgrade-charm') -def default_upgrade_charm(): - """Default handler for the 'upgrade-charm' hook. - This calls the charm.singleton.upgrade_charm() function as a default. - """ - reactive.set_state('upgraded') - - -# clear the upgraded state once config.rendered is set again -flags.register_trigger(when='config.rendered', clear_flag='upgraded') - - -@reactive.when_not('endpoint.keystone-fid-service-provider.joined') +@reactive.when_not('keystone-fid-service-provider.connected') def keystone_departed(): """ Service restart should be handled on the keystone side @@ -67,69 +36,34 @@ def keystone_departed(): charm_instance.remove_config() -@reactive.when('endpoint.keystone-fid-service-provider.joined') -@reactive.when_not('config.complete') -def config_changed(): +@reactive.when('keystone-fid-service-provider.connected') +def publish_sp_fid(fid_sp): with charm.provide_charm_instance() as charm_instance: - if charm_instance.configuration_complete(): - flags.set_flag('config.complete') - - -@reactive.when('endpoint.keystone-fid-service-provider.joined') -@reactive.when_not('keystone-data.complete') -def keystone_data_changed(fid_sp): - primary_data = fid_sp.all_joined_units[0].received - if primary_data: - hostname = primary_data.get('hostname') - port = primary_data.get('port') - tls_enabled = primary_data.get('tls-enabled') - # a basic check on the fact that keystone provided us with - # hostname and port information - if hostname and port: - # save hostname and port data in local storage for future - # use - in case config is incomplete but a relation is - # we need to store this across charm hook invocations - unitdb = unitdata.kv() - unitdb.set('hostname', hostname) - unitdb.set('port', port) - unitdb.set('tls-enabled', tls_enabled) - flags.set_flag('keystone-data.complete') - - -@reactive.when('endpoint.keystone-fid-service-provider.joined') -@reactive.when('config.complete') -@reactive.when('keystone-data.complete') -@reactive.when_not('config.rendered') -def render_config(): - # don't always have a relation context - obtain from the flag - fid_sp = endpoint_from_flag( - 'endpoint.keystone-fid-service-provider.joined') - with charm.provide_charm_instance() as charm_instance: - charm_instance.render_config() - flags.set_flag('config.rendered') - # Trigger keystone restart. The relation is container-scoped - # so a per-unit db of a remote unit will only contain a nonce - # of a single subordinate - restart_nonce = str(uuid.uuid4()) - fid_sp.publish(restart_nonce, - charm_instance.options.protocol_name, + fid_sp.publish(charm_instance.options.protocol_name, charm_instance.options.remote_id_attribute) -@reactive.when('endpoint.websso-fid-service-provider.joined') -@reactive.when('config.complete') -@reactive.when('keystone-data.complete') -@reactive.when('config.rendered') -def configure_websso(): - # don't always have a relation context - obtain from the flag - websso_fid_sp = endpoint_from_flag( - 'endpoint.websso-fid-service-provider.joined') +@reactive.when('keystone-fid-service-provider.available') +def render_config(fid_sp): with charm.provide_charm_instance() as charm_instance: - # publish config options for all remote units of a given rel - options = charm_instance.options - websso_fid_sp.publish(options.protocol_name, - options.idp_name, - options.user_facing_name) + if charm_instance.configuration_complete(): + charm_instance.render_config(fid_sp) + # Trigger keystone restart. The relation is container-scoped + # so a per-unit db of a remote unit will only contain a nonce + # of a single subordinate + if reactive.any_file_changed(charm_instance.restart_map.keys()): + fid_sp.request_restart() + + +@reactive.when('websso-fid-service-provider.connected') +def configure_websso(websso_fid_sp): + with charm.provide_charm_instance() as charm_instance: + if charm_instance.configuration_complete(): + # publish config options for all remote units of a given rel + options = charm_instance.options + websso_fid_sp.publish(options.protocol_name, + options.idp_name, + options.user_facing_name) @reactive.when_not('always.run') diff --git a/src/templates/mellon-sp-metadata.xml b/src/templates/mellon-sp-metadata.xml index 9472497..6dd6356 100644 --- a/src/templates/mellon-sp-metadata.xml +++ b/src/templates/mellon-sp-metadata.xml @@ -1,5 +1,5 @@ {% endif %} - + {% for format in options.supported_nameid_formats -%} {{ format }} {% endfor -%} - + diff --git a/unit_tests/test_keystone_saml_mellon_handlers.py b/unit_tests/test_keystone_saml_mellon_handlers.py index ef9387a..aa8974c 100644 --- a/unit_tests/test_keystone_saml_mellon_handlers.py +++ b/unit_tests/test_keystone_saml_mellon_handlers.py @@ -34,26 +34,16 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks): 'default_upgrade_charm': ('upgrade-charm',), }, 'when': { + 'publish_sp_fid': ( + 'keystone-fid-service-provider.connected',), 'render_config': ( - 'endpoint.keystone-fid-service-provider.joined', - 'config.complete', - 'keystone-data.complete',), - 'config_changed': ( - 'endpoint.keystone-fid-service-provider.joined',), - 'keystone_data_changed': ( - 'endpoint.keystone-fid-service-provider.joined',), + 'keystone-fid-service-provider.available',), 'configure_websso': ( - 'endpoint.websso-fid-service-provider.joined', - 'config.complete', - 'keystone-data.complete', - 'config.rendered',), + 'websso-fid-service-provider.connected',), }, 'when_not': { - 'config_changed': ('config.complete',), 'keystone_departed': ( - 'endpoint.keystone-fid-service-provider.joined',), - 'keystone_data_changed': ('keystone-data.complete',), - 'render_config': ('config.rendered',), + 'keystone-fid-service-provider.connected',), 'assess_status': ('always.run',), }, } @@ -75,21 +65,10 @@ class TestKeystoneSAMLMellonHandlers(test_utils.PatchHelper): self.keystone_saml_mellon_charm) self.provide_charm_instance().__exit__.return_value = None - self.patch_object(handlers, 'flags') - - self.uuid = 'uuid-uuid' - self.patch_object(handlers.uuid, 'uuid4') - self.uuid4.return_value = self.uuid - - self.patch_object(handlers, 'unitdata', + self.patch_object(handlers.reactive, 'any_file_changed', new=mock.MagicMock()) - self.kv = mock.MagicMock() - self.unitdata.kv.return_value = self.kv - self.patch_object(handlers, 'endpoint_from_flag', - new=mock.MagicMock()) self.endpoint = mock.MagicMock() - self.endpoint_from_flag.return_value = self.endpoint self.protocol_name = "mapped" self.remote_id_attribute = "https://samltest.id" @@ -116,33 +95,29 @@ class TestKeystoneSAMLMellonHandlers(test_utils.PatchHelper): handlers.keystone_departed() self.keystone_saml_mellon_charm.remove_config.assert_called_once_with() - def test_keystone_data_changed(self): - kv_set_calls = [ - mock.call("tls-enabled", True), - mock.call("port", "5000"), - mock.call("hostname", "keystone-0"), - ] - - handlers.keystone_data_changed(self.endpoint) - - self.kv.set.has_calls(kv_set_calls) - self.flags.set_flag.assert_called_once_with('keystone-data.complete') + def test_publish_sp_fid(self): + handlers.publish_sp_fid(self.endpoint) + self.endpoint.publish.assert_called_once_with( + self.protocol_name, self.remote_id_attribute) def test_render_config(self): - handlers.render_config() - self.keystone_saml_mellon_charm.render_config.assert_called_once_with() - self.flags.set_flag.assert_called_once_with('config.rendered') - self.endpoint.publish.assert_called_once_with( - self.uuid, self.protocol_name, self.remote_id_attribute) + # No restart + self.any_file_changed.return_value = False + (self.keystone_saml_mellon_charm + .configuration_complete.return_value) = True - def test_config_changed(self): - handlers.config_changed() - (self.keystone_saml_mellon_charm.configuration_complete - .return_value) = True - self.flags.set_flag.assert_called_once_with('config.complete') + handlers.render_config(self.endpoint) + self.keystone_saml_mellon_charm.render_config.assert_called_once_with( + self.endpoint) + self.endpoint.request_restart.assert_not_called() + + # Restart + self.any_file_changed.return_value = True + handlers.render_config(self.endpoint) + self.endpoint.request_restart.assert_called_once_with() def test_configure_websso(self): - handlers.configure_websso() + handlers.configure_websso(self.endpoint) self.endpoint.publish.assert_called_once_with( self.protocol_name, self.idp_name, self.user_facing_name) diff --git a/unit_tests/test_lib_charm_openstack_keystone_saml_mellon.py b/unit_tests/test_lib_charm_openstack_keystone_saml_mellon.py index cf67e77..be12dec 100644 --- a/unit_tests/test_lib_charm_openstack_keystone_saml_mellon.py +++ b/unit_tests/test_lib_charm_openstack_keystone_saml_mellon.py @@ -45,13 +45,7 @@ class Helper(test_utils.PatchHelper): self.patch_release( keystone_saml_mellon.KeystoneSAMLMellonCharm.release) - self.patch_object(keystone_saml_mellon, 'unitdata', - new=mock.MagicMock()) - self.kv = mock.MagicMock() - self.unitdata.kv.return_value = self.kv - - self.patch_object(keystone_saml_mellon.os_utils, 'os_release', - new=mock.MagicMock()) + self.endpoint = mock.MagicMock() self.idp_name = "samltest" self.protocol_name = "mapped" @@ -100,19 +94,6 @@ class Helper(test_utils.PatchHelper): self.open.return_value = self.fileobj -class TestKeystoneSAMLMellonUtils(Helper): - - def test_select_release(self): - self.kv.get.return_value = 'mitaka' - self.assertEqual( - keystone_saml_mellon.select_release(), 'mitaka') - - self.kv.get.return_value = None - self.os_release.return_value = 'rocky' - self.assertEqual( - keystone_saml_mellon.select_release(), 'rocky') - - class TestKeystoneSAMLMellonConfigurationAdapter(Helper): def setUp(self): @@ -120,12 +101,13 @@ class TestKeystoneSAMLMellonConfigurationAdapter(Helper): self.hostname = "keystone-sp.local" self.port = "5000" self.tls_enabled = True - self.unitdata_data = { + self.endpoint_data = { "hostname": self.hostname, "port": self.port, "tls-enabled": self.tls_enabled, } - self.kv.get.side_effect = FakeConfig(self.unitdata_data) + self.endpoint.all_joined_units.received.get.side_effect = ( + FakeConfig(self.endpoint_data)) self.base_url = "https://{}:{}".format(self.hostname, self.port) def test_validation_errors(self): @@ -153,22 +135,6 @@ class TestKeystoneSAMLMellonConfigurationAdapter(Helper): self.assertEqual( ksmca.sp_private_key_file, keystone_saml_mellon.SP_PRIVATE_KEY) - def test_keystone_host(self): - ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() - self.assertEqual(ksmca.keystone_host, self.hostname) - - def test_keystone_port(self): - ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() - self.assertEqual(ksmca.keystone_port, self.port) - - def test_keystone_tls_enabled(self): - ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() - self.assertEqual(ksmca.tls_enabled, self.tls_enabled) - - def test_keystone_base_url(self): - ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() - self.assertEqual(ksmca.keystone_base_url, self.base_url) - def test_sp_idp_path(self): ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() self.assertEqual( @@ -210,25 +176,6 @@ class TestKeystoneSAMLMellonConfigurationAdapter(Helper): ksmca.sp_logout_path, '{}/logout'.format(ksmca.mellon_endpoint_path)) - def test_sp_auth_url(self): - ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() - self.assertEqual( - ksmca.sp_auth_url, - '{}{}'.format(ksmca.keystone_base_url, ksmca.sp_auth_path)) - - def test_sp_logout_url(self): - ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() - self.assertEqual( - ksmca.sp_logout_url, - '{}{}'.format(ksmca.keystone_base_url, ksmca.sp_logout_path)) - - def test_sp_post_response_url(self): - ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() - self.assertEqual( - ksmca.sp_post_response_url, - '{}{}'.format(ksmca.keystone_base_url, - ksmca.sp_post_response_path)) - def test_mellon_subject_confirmation_data_address_check(self): ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter() self.assertEqual( @@ -338,19 +285,20 @@ class TestKeystoneSAMLMellonCharm(Helper): self.sp_signing_keyinfo.__bool__.return_value = False self.assertFalse(ksm.configuration_complete()) - def test_assess_status(self): + def test_custom_assess_status_check(self): ksm = keystone_saml_mellon.KeystoneSAMLMellonCharm() - ksm.assess_status() - self.application_version_set.asert_called_once_with() - self.status_set.assert_called_once_with("active", "Unit is ready") + self.assertEqual( + ksm.custom_assess_status_check(), + (None, None)) # One option not ready self.status_set.reset_mock() self.sp_signing_keyinfo.__bool__.return_value = False ksm.options._validation_errors = {"idp-metadata": "malformed"} - ksm.assess_status() - self.status_set.assert_called_once_with( - "blocked", "Configuration is incomplete. idp-metadata: malformed") + self.assertEqual( + ksm.custom_assess_status_check(), + ("blocked", + "Configuration is incomplete. idp-metadata: malformed")) def test_render_config(self): ksm = keystone_saml_mellon.KeystoneSAMLMellonCharm()