#58 Allow SAML SP metadata to be viewed and edited
Closed 5 years ago by puiterwijk. Opened 5 years ago by jdennis.
jdennis/ipsilon ticket-130  into  master

@@ -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

@@ -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'])

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)

@@ -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

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).

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.

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

This has been merged, thanks.