PR#58: Allow SAML SP metadata to be viewed and edited - ipsilon

ipsilon

#58 Allow SAML SP metadata to be viewed and edited

Closed 8 years ago by puiterwijk. Opened 8 years ago by jdennis.

jdennis/ipsilon ticket-130 into master

Allow SAML SP metadata to be viewed and edited

John Dennis • 8 years ago

f9269d3

ipsilon/providers/saml2/admin.py

file modified

+11 -1

		`@@ -11,6 +11,8 @@`
		`from ipsilon.providers.saml2.provider import ServiceProvider`
		`from ipsilon.providers.saml2.provider import ServiceProviderCreator`
		`from ipsilon.providers.saml2.provider import InvalidProviderId`
		`+ from ipsilon.providers.saml2.provider import validate_sp_metadata`
		`+ from ipsilon.providers.saml2.provider import InvalidProviderMetadata`
		`from copy import deepcopy`
		`import requests`
		`import logging`
		`@@ -227,7 +229,8 @@`
		`'Allowed NameIDs', 'Attribute Mapping',`
		`'Allowed Attributes', 'Description',`
		`'Service Provider link',`
		`- 'Visible in Portal', 'Image File']:`
		`+ 'Visible in Portal', 'Image File',`
		`+ 'Metadata']:`
		`if not self.user.is_admin:`
		`raise UnauthorizedUser(`
		`"Unauthorized to set %s" % key`
		`@@ -277,7 +280,14 @@`
		`raise InvalidValueFormat(`
		`'Invalid Image file format'`
		`)`
		`+ elif name == 'Metadata':`
		`+ validate_sp_metadata(value)`
		`+ self.sp.metadata = value`

		`+ except InvalidProviderMetadata, e:`
		`+ message = str(e)`
		`+ message_type = ADMIN_STATUS_WARN`
		`+ return self.root_with_msg(message, message_type)`
		`except InvalidValueFormat, e:`
		`message = str(e)`
		`message_type = ADMIN_STATUS_WARN`

ipsilon/providers/saml2/provider.py

file modified

+52 -9

		`@@ -15,12 +15,20 @@`

		`class InvalidProviderId(ProviderException):`

		`- def __init__(self, code):`
		`- message = 'Invalid Provider ID: %s' % code`
		`+ def __init__(self, msg):`
		`+ message = 'Invalid Provider ID: %s' % msg`
		`super(InvalidProviderId, self).__init__(message)`
		`self.debug(message)`


		`+ class InvalidProviderMetadata(ProviderException):`
		`+`
		`+ def __init__(self, msg):`
		`+ message = 'Invalid Provider Metadata: %s' % msg`
		`+ super(InvalidProviderMetadata, self).__init__(message)`
		`+ self.debug(message)`
		`+`
		`+`
		`class NameIdNotAllowed(Exception):`

		`def __init__(self, nid):`
		`@@ -32,6 +40,30 @@`
		`return repr(self.message)`


		`+ def validate_sp_metadata(metadata):`
		`+ '''Validate SP metadata`
		`+`
		`+ Attempt to load the metadata into Lasso and verify it loads.`
		`+ Assure only 1 provider is included in the metadata and return`
		`+ it's id. If not valid raise an exception.`
		`+`
		`+ Note, loading the metadata into Lasso is a weak check, basically`
		`+ all that Lasso does is to parse the XML, it doesn't verify the`
		`+ contents until first use.`
		`+ '''`
		`+ test = lasso.Server()`
		`+ try:`
		`+ test.addProviderFromBuffer(lasso.PROVIDER_ROLE_SP, metadata)`
		`+ except Exception as e: # pylint: disable=broad-except`
		`+ raise InvalidProviderMetadata(str(e))`
		`+ newsps = test.get_providers()`
		`+ if len(newsps) != 1:`
		`+ raise InvalidProviderMetadata("Metadata must contain one Provider")`
		`+`
		`+ spid = newsps.keys()[0]`
		`+ return spid`
		`+`
		`+`
		`class ServiceProviderConfig(ConfigHelper):`
		`def __init__(self):`
		`super(ServiceProviderConfig, self).__init__()`
		`@@ -69,6 +101,11 @@`
		`' accepted.',`
		`self.name),`
		`pconfig.String(`
		`+ 'Metadata',`
		`+ "Service Provider's metadata",`
		`+ self.metadata,`
		`+ multiline=True),`
		`+ pconfig.String(`
		`'Description',`
		`'A description of the SP to show on the Portal.',`
		`self.description),`
		`@@ -124,6 +161,17 @@`
		`self._staging['name'] = value`

		`@property`
		`+ def metadata(self):`
		`+ if 'metadata' in self._properties:`
		`+ return self._properties['metadata']`
		`+ else:`
		`+ return ''`
		`+`
		`+ @metadata.setter`
		`+ def metadata(self, value):`
		`+ self._staging['metadata'] = value`
		`+`
		`+ @property`
		`def description(self):`
		`return self._properties.get('description', '')`

		`@@ -306,13 +354,7 @@`
		`raise InvalidProviderId("Name must contain only "`
		`"numbers and letters")`

		`- test = lasso.Server()`
		`- test.addProviderFromBuffer(lasso.PROVIDER_ROLE_SP, metabuf)`
		`- newsps = test.get_providers()`
		`- if len(newsps) != 1:`
		`- raise InvalidProviderId("Metadata must contain one Provider")`
		`-`
		`- spid = newsps.keys()[0]`
		`+ spid = validate_sp_metadata(metabuf)`
		`data = self.cfg.get_data(name='id', value=spid)`
		`if len(data) != 0:`
		`raise InvalidProviderId("Provider Already Exists")`
		`@@ -349,6 +391,7 @@`
		`self.sessionfactory = sessionfactory`

		`def add_provider(self, sp):`
		`+ validate_sp_metadata(sp['metadata'])`
		`self.server.addProviderFromBuffer(lasso.PROVIDER_ROLE_SP,`
		`sp['metadata'])`
		`self.debug('Added SP %s' % sp['name'])`

ipsilon/util/config.py

file modified

+3 -1

		`@@ -148,9 +148,11 @@`

		`class String(Option):`

		`- def __init__(self, name, description, default_value=None, readonly=False):`
		`+ def __init__(self, name, description, default_value=None, readonly=False,`
		`+ multiline=False):`
		`super(String, self).__init__(name, description, readonly=readonly)`
		`self._default_value = str(default_value)`
		`+ self.multiline = multiline`

		`def set_value(self, value):`
		`self._assigned_value = str(value)`

templates/admin/option_config.html

file modified

+3 -1

		`@@ -76,7 +76,9 @@`
		`<label class="col-sm-2" for="{{ v.name }}">{{ v.name }}:</label>`
		`<div class="col-sm-10">`
		`{% set value = v.get_value() -%}`
		`- {% if v.__class__.__name__ in ['String', 'Template'] -%}`
		`+ {% if v.__class__.__name__ == 'String' and v.multiline -%}`
		`+ <textarea class="form-control" name="{{ v.name }}" rows="10" cols="80"}">{{value}}</textarea>`
		`+ {% elif v.__class__.__name__ in ['String', 'Template'] -%}`
		`<input type="text" class="form-control" name="{{ v.name }}"`
		`{%- if value %}`
		`value="{{ value }}"`

no initial comment

rcritten commented 8 years ago

The new metadata should be loaded into a temporary variable to ensure it is still valid. For example I added a space in an XML attribute name and it saved ok but then the SP was unusable.

e.g.

test = lasso.Server()

test.addProviderFromBuffer(lasso.PROVIDER_ROLE_SP, metabuf)

(and then probably some validation that something has loaded or a try/except around it).

jdennis commented 8 years ago

Commit has been amended to include a validation check. Previously other places where metadata was being loaded did not validate the metadata, this has also been corrected.

Note, the validation function is weak, it does call addProviderFromBuffer and check for errors but that Lasso function only weakly validates the metadata, basically all it does is parse the XML. At some point we might want better metadata validation but at least now it's localized in one place and called everywhere so improving the validation should be straight forward.

rcritten commented 8 years ago

ACK. The validation is about the best we can do for now.