Merge pull request #144 from geops/master

Fix for POST requests failing with 500

Author: Roland Hedberg

src/s2repoze/plugins/sp.py

@@ -1,6 +1,6 @@
 #
-""" 
-A plugin that allows you to use SAML2 SSO as authentication 
+"""
+A plugin that allows you to use SAML2 SSO as authentication
 and SAML2 attribute aggregations as metadata collector in your
 WSGI application.
 
@@ -49,46 +49,14 @@
 
 
 def construct_came_from(environ):
-    """ The URL that the user used when the process where interupted 
+    """ The URL that the user used when the process where interupted
     for single-sign-on processing. """
-    
-    came_from = environ.get("PATH_INFO") 
+
+    came_from = environ.get("PATH_INFO")
     qstr = environ.get("QUERY_STRING", "")
     if qstr:
         came_from += '?' + qstr
     return came_from
-    
-
-def cgi_field_storage_to_dict(field_storage):
-    """Get a plain dictionary, rather than the '.value' system used by the
-    cgi module."""
-    
-    params = {}
-    for key in field_storage.keys():
-        try:
-            params[key] = field_storage[key].value
-        except AttributeError:
-            if isinstance(field_storage[key], basestring):
-                params[key] = field_storage[key]
-                
-    return params
-
-
-def get_body(environ):
-    length = int(environ["CONTENT_LENGTH"])
-    try:
-        body = environ["wsgi.input"].read(length)
-    except Exception, excp:
-        logger.exception("Exception while reading post: %s" % (excp,))
-        raise
-
-    # restore what I might have upset
-    from StringIO import StringIO
-    environ['wsgi.input'] = StringIO(body)
-    environ['s2repoze.body'] = body
-
-    return body
-
 
 def exception_trace(tag, exc, log):
     message = traceback.format_exception(*sys.exc_info())
@@ -113,7 +81,7 @@ def __call__(self, environ, start_response):
 class SAML2Plugin(object):
 
     implements(IChallenger, IIdentifier, IAuthenticator, IMetadataProvider)
-    
+
     def __init__(self, rememberer_name, config, saml_client, wayf, cache,
                  sid_store=None, discovery="", idp_query_param="",
                  sid_store_cert=None,):
@@ -158,27 +126,24 @@ def forget(self, environ, identity):
     def _get_post(self, environ):
         """
         Get the posted information
-    
+
         :param environ: A dictionary with environment variables
         """
-    
-        post_env = environ.copy()
-        post_env['QUERY_STRING'] = ''
-    
-        _ = get_body(environ)
-        
+
+        body= ''
         try:
-            post = cgi.FieldStorage(
-                fp=environ['wsgi.input'],
-                environ=post_env,
-                keep_blank_values=True
-            )
-        except Exception, excp:
-            logger.debug("Exception (II): %s" % (excp,))
-            raise
-    
+            length= int(environ.get('CONTENT_LENGTH', '0'))
+        except ValueError:
+            length= 0
+        if length!=0:
+            body = environ['wsgi.input'].read(length)  # get the POST variables
+            environ['s2repoze.body'] = body            # store the request body for later use by pysaml2
+            environ['wsgi.input'] = StringIO(body)     # restore the request body as a stream so that everything seems untouched
+
+        post = parse_qs(body)                          # parse the POST fields into a dict
+
         logger.debug('identify post: %s' % (post,))
-    
+
         return post
 
     def _wayf_redirect(self, came_from):
@@ -190,8 +155,8 @@ def _wayf_redirect(self, came_from):
 
     #noinspection PyUnusedLocal
     def _pick_idp(self, environ, came_from):
-        """ 
-        If more than one idp and if none is selected, I have to do wayf or 
+        """
+        If more than one idp and if none is selected, I have to do wayf or
         disco
         """
 
@@ -230,7 +195,7 @@ def _pick_idp(self, environ, came_from):
                     detail='unknown ECP version')
 
         idps = self.metadata.with_descriptor("idpsso")
-        
+
         logger.info("IdP URL: %s" % idps)
 
         idp_entity_id = query = None
@@ -290,7 +255,7 @@ def _pick_idp(self, environ, came_from):
 
         logger.info("Chosen IdP: '%s'" % idp_entity_id)
         return 0, idp_entity_id
-        
+
     #### IChallenger ####
     #noinspection PyUnusedLocal
     def challenge(self, environ, _status, _app_headers, _forget_headers):
@@ -320,7 +285,7 @@ def challenge(self, environ, _status, _app_headers, _forget_headers):
         came_from = construct_came_from(environ)
         environ["myapp.came_from"] = came_from
         logger.debug("[sp.challenge] RelayState >> '%s'" % came_from)
-        
+
         # Am I part of a virtual organization or more than one ?
         try:
             vorg_name = environ["myapp.vo"]
@@ -329,7 +294,7 @@ def challenge(self, environ, _status, _app_headers, _forget_headers):
                 vorg_name = _cli.vorg._name
             except AttributeError:
                 vorg_name = ""
-            
+
         logger.info("[sp.challenge] VO: %s" % vorg_name)
 
         # If more than one idp and if none is selected, I have to do wayf
@@ -373,7 +338,7 @@ def challenge(self, environ, _status, _app_headers, _forget_headers):
                     req_id, msg_str = _cli.create_authn_request(
                         dest, vorg=vorg_name, sign=_cli.authn_requests_signed,
                         message_id=_sid, extensions=extensions)
-                    _sid = req_id 
+                    _sid = req_id
                 else:
                     req_id, req = _cli.create_authn_request(
                         dest, vorg=vorg_name, sign=False, extensions=extensions)
@@ -423,7 +388,7 @@ def _construct_identity(self, session_info):
         logger.debug("Identity: %s" % identity)
 
         return identity
-        
+
     def _eval_authn_response(self, environ, post, binding=BINDING_HTTP_POST):
         logger.info("Got AuthN response, checking..")
         logger.info("Outstanding: %s" % (self.outstanding_queries,))
@@ -432,18 +397,18 @@ def _eval_authn_response(self, environ, post, binding=BINDING_HTTP_POST):
             # Evaluate the response, returns a AuthnResponse instance
             try:
                 authresp = self.saml_client.parse_authn_request_response(
-                    post["SAMLResponse"], binding, self.outstanding_queries,
+                    post["SAMLResponse"][0], binding, self.outstanding_queries,
                     self.outstanding_certs)
 
             except Exception, excp:
                 logger.exception("Exception: %s" % (excp,))
                 raise
-                
+
             session_info = authresp.session_info()
         except TypeError, excp:
             logger.exception("Exception: %s" % (excp,))
             return None
-                                        
+
         if session_info["came_from"]:
             logger.debug("came_from << %s" % session_info["came_from"])
             try:
@@ -478,13 +443,13 @@ def identify(self, environ):
                         "SAMLResponse" not in query and "SAMLRequest" not in query:
             logger.debug('[identify] get or empty post')
             return None
-        
+
         # if logger:
         #     logger.info("ENVIRON: %s" % environ)
         #     logger.info("self: %s" % (self.__dict__,))
-        
+
         uri = environ.get('REQUEST_URI', construct_url(environ))
-        
+
         logger.debug('[sp.identify] uri: %s' % (uri,))
 
         query = parse_dict_querystring(environ)
@@ -495,15 +460,13 @@ def identify(self, environ):
             binding = BINDING_HTTP_REDIRECT
         else:
             post = self._get_post(environ)
-            if post.list is None:
-                post.list = []
             binding = BINDING_HTTP_POST
 
         try:
             logger.debug('[sp.identify] post keys: %s' % (post.keys(),))
         except (TypeError, IndexError):
             pass
-            
+
         try:
             path_info = environ['PATH_INFO']
             logout = False
@@ -514,7 +477,7 @@ def identify(self, environ):
                 print("logout request received")
                 try:
                     response = self.saml_client.handle_logout_request(
-                        post["SAMLRequest"],
+                        post["SAMLRequest"][0],
                         self.saml_client.users.subjects()[0], binding)
                     environ['samlsp.pending'] = self._handle_logout(response)
                     return {}
@@ -536,7 +499,7 @@ def identify(self, environ):
                 try:
                     if logout:
                         response = self.saml_client.parse_logout_request_response(
-                            post["SAMLResponse"], binding)
+                            post["SAMLResponse"][0], binding)
                         if response:
                             action = self.saml_client.handle_logout_response(
                                 response)
@@ -552,7 +515,7 @@ def identify(self, environ):
                             return {}
                     else:
                         session_info = self._eval_authn_response(
-                            environ, cgi_field_storage_to_dict(post),
+                            environ, post,
                             binding=binding)
                 except Exception, err:
                     environ["s2repoze.saml_error"] = err
@@ -572,8 +535,8 @@ def identify(self, environ):
                 exception_trace("sp.identity", exc, logger)
                 environ["post.fieldstorage"] = post
                 return {}
-            
-        if session_info:        
+
+        if session_info:
             environ["s2repoze.sessioninfo"] = session_info
             return self._construct_identity(session_info)
         else:
@@ -596,12 +559,12 @@ def add_metadata(self, environ, identity):
             logger.debug("Issuers: %s" % _cli.users.sources(name_id))
         except KeyError:
             pass
-            
+
         if "user" not in identity:
             identity["user"] = {}
         try:
             (ava, _) = _cli.users.get_identity(name_id)
-            #now = time.gmtime()        
+            #now = time.gmtime()
             logger.debug("[add_metadata] adds: %s" % ava)
             identity["user"].update(ava)
         except KeyError:
@@ -625,7 +588,7 @@ def add_metadata(self, environ, identity):
         if not identity["user"]:
             # remove cookie and demand re-authentication
             pass
-        
+
     # used 2 times : one to get the ticket, the other to validate it
     @staticmethod
     def _service_url(environ, qstr=None):
@@ -635,7 +598,7 @@ def _service_url(environ, qstr=None):
             url = construct_url(environ)
         return url
 
-    #### IAuthenticatorPlugin #### 
+    #### IAuthenticatorPlugin ####
     #noinspection PyUnusedLocal
     def authenticate(self, environ, identity=None):
         if identity:
@@ -672,7 +635,7 @@ def make_plugin(remember_name=None,  # plugin for remember
                 discovery="",
                 idp_query_param=""
                 ):
-    
+
     if saml_conf is "":
         raise ValueError(
             'must include saml_conf in configuration')