Error reporting clarified: CRITICAL level for unexpected conditions in saml2test; ERROR and lower for conditions in the test target; various documentation improvements

Author: rhoerbe

doc/sp_test/internal.rst

@@ -24,7 +24,7 @@ These files should be stored outside the saml2test package to have a clean separ
 Client (sp_test/__init__.py)
 .........................
 Its life cycle is responsible for following activites:
- - read config files and command line argumants
+ - read config files and command line argumants (the test driver's config is "json_config")
  - initialize the test driver IDP
  - initialize a Conversation
  - start the Conversion with .do_sequence_and_tests()
@@ -33,54 +33,55 @@ Its life cycle is responsible for following activites:
 Conversation (sp_test/base.py)
 ..............................
 
-operation (oper)
+Operation (oper)
 ................
-  - comprises an id, name, sequence and tests
-  - names oper in
+  - Comprises an id, name, sequence and tests
   - Example: 'sp-00': {"name": 'Basic Login test', "sequence": [(Login, AuthnRequest, AuthnResponse, None)], "tests": {"pre": [], "post": []}
-  - Names a use case in STHREP
 
 OPERATIONS
 ..........
   - set of operations provided in sp_test
   - can be listed with the -l command line option
 
-sequence
+Sequence
 ........
   - A list of flows
-  - Example: see "sequence" item in operation dictionary
+  - Example: see "sequence" item in operation dict
 
-test (in the context of an operation)
+Test (in the context of an operation)
 ....
-  - class to be executed as part of an operation, either before ("pre") or after ("post") the sequence
+  - class to be executed as part of an operation, either before ("pre") or after ("post") the sequence or inbetween a SAML request and response ("mid").
+    There are standard tests with the Request class (VerifyAuthnRequest) and operation-specific tests.
+  - Example for an operation-specific "mid" test: VerifyIfRequestIsSigned
+  - A test may be specified together with an argument as a tupel
 
-flow
+Flow
 ....
-  - A tupel of classes that together implement an SAML request-response pair between IDP and SP (and possible a discovery service). A class can be derived from Request, Response or (other), Check or Operation.
-  - A flow for a solicited authentication consists of 4 classes:
-    flow[0]: Operation (Handling a login flow such as discovery or WAYF - not implemented yet)
-    flow[1]: Request (process the authentication request)
-    flow[2]: Response (send the authentication response)
-    flow[3]: Check (optional - can be None. E.g. check the response if a correct error status was raised when sending a broken response)
+  * A tupel of classes that together implement an SAML request-response pair between IDP and SP (and possible other actors, such as a discovery service or IDP-proxy). A class can be derived from Request, Response (or other), Check or Operation.
+  * A flow for a solicited authentication consists of 4 classes:
+
+    * flow[0]: Operation (Handling a login flow such as discovery or WAYF - not implemented yet)
+    * flow[1]: Request (process the authentication request)
+    * flow[2]: Response (send the authentication response)
+    * flow[3]: Check (optional - can be None. E.g. check the response if a correct error status was raised when sending a broken response)
 
 Check (and subclasses)
 .....
   - an optional class that is executed on receiving the SP's HTTP response(s) after the SAML response. If there are redirects it will be called for each response.
   - writes a structured test report to conv.test_output
   - It can check for expected errors, which do not cause an exception but in contrary are reported as success
 
-interaction
+Interaction
 ...........
   - An interaction automates a human interaction. It searches a response from a test target for some constants, and if
     there is a match, it will create a response suitable response.
 
-(2) Simplefied Flow
+(2) Simplyfied Flow
 :::::::::::::::::::
 
-The following pseudocdoe is an extract showing an overview of what is executed
-for test sp-00:
+The following pseudocode is an extract showing an overview of what is executed
+for test sp-00::
 
-::
     do_sequence_and_test(self, oper, test):
         self.test_sequence(tests["pre"])  # currently no tests defined for sp_test
         for flow in oper:
@@ -97,8 +98,8 @@ for test sp-00:
         else:
             self.handle_result()
 
-    send_idp_response(req, resp):
-        self.test_sequence(req.tests["post"])   # execute "post"-tests (request has "VerifyContent"-test built in; others from config)
+    send_idp_response(req_flow, resp_flow):
+        self.test_sequence(req_flow.tests["mid"])   # execute "mid"-tests (request has "VerifyContent"-test built in; others from config)
         # this line stands for a part that is a bit more involved .. see source
 
         args.update(resp._response_args)    # set userid, identity
@@ -129,3 +130,20 @@ for test sp-00:
             _txt = self.last_response.content
             if self.last_response.status_code >= 400:
                 raise FatalError("Did not expected error")
+
+(3) Status Reporting
+::::::::::::::::::::
+The proper reporting of results is at the core of saml2test. Several conditions
+must be considered:
+
+#. An operation that was successful because the test target reports OK (e.g. HTTP 200)
+#. An operation that was successful because the test target reports NOK as expected, e.g. because of an invalid signature - HTTP 500 could be the correct response
+#. An errror in SAML2Test
+#. An errror in configuration of SAML2Test
+
+Status values are defined in saml2test.check like this:
+INFORMATION = 0, OK = 1, WARNING = 2, ERROR = 3, CRITICAL = 4, INTERACTION = 5
+
+
+There are 2 targets to write output to:
+* Test_ouput is written to conv.test_ouput during the execution of the flows.

setup.py

@@ -26,7 +26,7 @@
     author = "Roland Hedberg",
     author_email = "[email protected]",
     license="Apache 2.0",
-    packages=["idp_test", "idp_test/package", "saml2test", "sp_test"],
+    packages=["idp_test", "idp_test/package", "saml2test", "sp_test", "utility"],
     package_dir = {"": "src"},
     classifiers = [
         "Development Status :: 4 - Beta",

src/saml2test/check.py

@@ -9,8 +9,8 @@
 INFORMATION = 0
 OK = 1
 WARNING = 2
-ERROR = 3
-CRITICAL = 4
+ERROR = 3   # an error condition in the test target
+CRITICAL = 4 # an error condition in the test driver
 INTERACTION = 5
 
 STATUSCODE = ["INFORMATION", "OK", "WARNING", "ERROR", "CRITICAL",
@@ -124,8 +124,8 @@ def _func(self, conv):
 
 class VerifyBadRequestResponse(ExpectedError):
     """
-    Verifies that the OP returned a 400 Bad Request response containing a
-    Error message.
+    Verifies that the test target returned a 400 Bad Request response
+    containing a an error message.
     """
     cid = "verify-bad-request-response"
     msg = "OP error"
@@ -138,7 +138,7 @@ def _func(self, conv):
             pass
         else:
             self._message = "Expected a 400 error message"
-            self._status = CRITICAL
+            self._status = ERROR
 
         return res
 
@@ -191,22 +191,22 @@ class Other(CriticalError):
     msg = "Other error"
 
 
-class CheckHTTPResponse(CriticalError):
+class CheckSpHttpResponseOK(Error):
     """
-    Checks that the HTTP response status is within the 200 or 300 range
+    Checks that the SP's HTTP response status is within the 200 or 300 range
     """
-    cid = "check-http-response"
-    msg = "OP error"
+    cid = "check-sp-http-response-ok"
+    msg = "SP error OK"
 
     def _func(self, conv):
         _response = conv.last_response
-        _content = conv.last_content
+        _content = conv.last_response.content
 
         res = {}
         if _response.status_code >= 400:
             self._status = self.status
             self._message = self.msg
-            res["content"] = _content
+            #res["content"] = _content   #too big + charset converstion needed
             res["url"] = conv.position
             res["http_status"] = _response.status_code
 

src/sp_test/__init__.py

@@ -74,8 +74,8 @@ def __init__(self, operations, check_factory):
             help="Path to the configuration file for the IdP")
         self._parser.add_argument("-t", dest="testpackage",
                                   help="Module describing tests")
-        self._parser.add_argument('-v', dest='verbose', action='store_true',
-                                  help="Print runtime information")
+        #self._parser.add_argument('-v', dest='verbose', action='store_true',
+        #                          help="Print runtime information") # unsused
         self._parser.add_argument("-Y", dest="pysamllog", action='store_true',
                                   help="Print PySAML2 logs")
         self._parser.add_argument("oper", nargs="?", help="Which test to run")

src/sp_test/base.py

@@ -37,13 +37,13 @@
 class Conversation():
     def __init__(self, instance, config, interaction, json_config,
                  check_factory, entity_id, msg_factory=None,
-                 features=None, verbose=False, constraints=None,
-                 expect_exception=None):
+                 features=None, constraints=None,  # verbose=False,
+                 expect_exception=None, commandlineargs=None):
         self.instance = instance
         self._config = config
         self.test_output = []
         self.features = features
-        self.verbose = verbose
+        #self.verbose = verbose  # removed (not used)
         self.check_factory = check_factory
         self.msg_factory = msg_factory
         self.expect_exception = expect_exception
@@ -70,7 +70,7 @@ def __init__(self, instance, config, interaction, json_config,
         self.position = ""
         self.response = None
         self.oper = None
-        self.idp_constraints = constraints
+        self.msg_constraints = constraints
         self.json_config = json_config
         self.start_page = json_config["start_page"]
 
@@ -122,7 +122,7 @@ def test_sequence(self, sequence):
                 kwargs = {}
             self.do_check(test, **kwargs)
             if test == ExpectedError:
-                return False
+                return False  # TODO: return value is unused
         return True
 
     def my_endpoints(self):
@@ -167,11 +167,6 @@ def handle_result(self, response=None):
             elif isinstance(response(), Check):
                 self.do_check(response)
             else:
-                # rhoerbe: I guess that this branch is never used, therefore
-                # I am proposing this exception:
-                #raise FatalError("can't use " + response.__class__.__name__ +
-                #                 ", because it is not a subclass of 'Check'")
-                #
                 # A HTTP redirect or HTTP Post
                 if 300 < self.last_response.status_code <= 303:
                     self._redirect(self.last_response)
@@ -241,11 +236,11 @@ def _redirect(self, _response):
                     break
         return url
 
-    def send_idp_response(self, req, resp):
+    def send_idp_response(self, req_flow, resp_flow):
         """
-        :param req: The expected request
-        :param resp: The response type to be used
-        :return: A response
+        :param req_flow: The flow to check the request
+        :param resp_flow: The flow to prepare the response
+        :return: The SP's HTTP response on receiving the SAML response
         """
         # make sure I got the request I expected
         assert isinstance(self.saml_request.message, req._class)
@@ -257,37 +252,41 @@ def send_idp_response(self, req, resp):
 
         # Pick information from the request that should be in the response
         args = self.instance.response_args(self.saml_request.message,
-                                           [resp._binding])
-        _mods = list(resp.__mro__[:])
+                                           [resp_flow._binding])
+        _mods = list(resp_flow.__mro__[:])
         _mods.reverse()
         for m in _mods:
             try:
                 args.update(self.json_config["args"][m.__name__])
             except KeyError:
                 pass
 
-        args.update(resp._response_args)
+        args.update(resp_flow._response_args)
 
         for param in ["identity", "userid"]:
             if param in self.json_config:
                 args[param] = self.json_config[param]
 
-        if resp == ErrorResponse:
+        if resp_flow == ErrorResponse:
             func = getattr(self.instance, "create_error_response")
         else:
-            _op = camel2underscore.sub(r'_\1', req._class.c_tag).lower()
+            _op = camel2underscore.sub(r'_\1', req_flow._class.c_tag).lower()
             func = getattr(self.instance, "create_%s_response" % _op)
 
         # get from config which parts shall be signed
         sign = []
         for styp in ["sign_assertion", "sign_response"]:
             if styp in args:
-                if args[styp].lower() == "always":
-                    sign.append(styp)
-                del args[styp]
+                try:
+                    if args[styp].lower() == "always":
+                        sign.append(styp)
+                    del args[styp]
+                except (AttributeError, TypeError):
+                    raise AssertionError('config parameters "sign_assertion", '
+                                         '"sign_response" must be of type string')
 
         response = func(**args)
-        response = resp(self).pre_processing(response)
+        response = resp_flow(self).pre_processing(response)
         # and now for signing
         if sign:
             to_sign = []
@@ -315,21 +314,21 @@ def send_idp_response(self, req, resp):
             response = signed_instance_factory(response, self.instance.sec,
                                                to_sign)
 
-        info = self.instance.apply_binding(resp._binding, response,
+        info = self.instance.apply_binding(resp_flow._binding, response,
                                            args["destination"],
                                            self.relay_state,
-                                           "SAMLResponse", resp._sign)
+                                           "SAMLResponse", resp_flow._sign)
 
-        if resp._binding == BINDING_HTTP_REDIRECT:
+        if resp_flow._binding == BINDING_HTTP_REDIRECT:
             url = None
             for param, value in info["headers"]:
                 if param == "Location":
                     url = value
                     break
             self.last_response = self.instance.send(url)
-        elif resp._binding == BINDING_HTTP_POST:
-            resp = base64.b64encode("%s" % response)
-            info["data"] = urllib.urlencode({"SAMLResponse": resp,
+        elif resp_flow._binding == BINDING_HTTP_POST:
+            resp_flow = base64.b64encode("%s" % response)
+            info["data"] = urllib.urlencode({"SAMLResponse": resp_flow,
                                              "RelayState": self.relay_state})
             info["method"] = "POST"
             info["headers"] = {
@@ -343,7 +342,7 @@ def do_flow(self, flow):
         Solicited or 'un-solicited' flows.
 
         Solicited always starts with the Web client accessing a page.
-        Un-solicited starts with the IDP sending something.
+        Un-solicited starts with the IDP sending a SAMl Response.
         """
         if len(flow) >= 3:
             self.wb_send_GET_startpage()
@@ -373,7 +372,7 @@ def do_sequence_and_tests(self, oper, tests=None):
                 break
             except FatalError:
                 raise
-            except Exception:
+            except Exception as err:
                 #self.err_check("exception", err)
                 raise