chore: coverage improved, better handling of unknown idp entity id

Author: peppelinux

djangosaml2/tests/__init__.py

@@ -40,7 +40,9 @@
                                saml2_from_httpredirect_request)
 from djangosaml2.views import (EchoAttributesView, finish_logout)
 from saml2.config import SPConfig
-from saml2.s_utils import decode_base64_and_inflate, deflate_and_base64_encode
+from saml2.s_utils import (decode_base64_and_inflate,
+                           deflate_and_base64_encode,
+                           UnknownSystemEntity)
 
 from .auth_response import auth_response
 from .utils import SAMLPostFormParser
@@ -114,8 +116,9 @@ def test_get_idp_sso_supported_bindings_unknown_idp(self):
             idp_hosts=['idp.example.com'],
             metadata_file='remote_metadata_one_idp.xml',
         )
-        self.assertEqual(get_idp_sso_supported_bindings(
-            idp_entity_id='random'), [])
+
+        with self.assertRaises(UnknownSystemEntity):
+            get_idp_sso_supported_bindings(idp_entity_id='random')
 
     def test_get_idp_sso_supported_bindings_no_idps(self):
         settings.SAML_CONFIG = conf.create_conf(
@@ -189,6 +192,16 @@ def test_no_redirect(self):
 
         self.assertEqual(params['RelayState'], [settings.LOGIN_REDIRECT_URL, ])
 
+    def test_unknown_idp(self):
+        # monkey patch SAML configuration
+        settings.SAML_CONFIG = conf.create_conf(
+            sp_host='sp.example.com',
+            metadata_file='remote_metadata_three_idps.xml',
+        )
+
+        response = self.client.get(reverse('saml2_login')+'?idp=https://unknown.org')
+        self.assertEqual(response.status_code, 403)
+
     def test_login_one_idp(self):
         # monkey patch SAML configuration
         settings.SAML_CONFIG = conf.create_conf(

djangosaml2/utils.py

@@ -68,8 +68,9 @@ def get_idp_sso_supported_bindings(idp_entity_id: Optional[str] = None, config:
     try:
         return list(meta.service(idp_entity_id, 'idpsso_descriptor', 'single_sign_on_service').keys())
     except UnknownSystemEntity:
-        return []
-
+        raise UnknownSystemEntity
+    except Exception as e:
+        logger.error(f"get_idp_sso_supported_bindings failed with: {e}")
 
 def get_location(http_info):
     """Extract the redirect URL from a pysaml2 http_info object"""

djangosaml2/views.py

@@ -125,6 +125,13 @@ def get_next_path(self, request: HttpRequest) -> str:
         next_path = validate_referral_url(request, next_path)
         return next_path
 
+    def unknown_idp(self, request, idp):
+        msg = (f'Error: IdP EntityID {idp} was not found in metadata')
+        logger.error(msg)
+        return HttpResponse(
+            msg.format('Please contact technical support.'), status=403
+        )
+
     def get(self, request, *args, **kwargs):
         logger.debug('Login process started')
         next_path = self.get_next_path(request)
@@ -149,10 +156,10 @@ def get(self, request, *args, **kwargs):
 
         try:
             conf = self.get_sp_config(request)
-        except SourceNotFound as excp:
-            msg = ('Error, IdP EntityID was not found in metadata: {}')
-            logger.exception(msg.format(excp))
-            return HttpResponse(msg.format('Please contact technical support.'), status=500)
+        except SourceNotFound as excp:  # pragma: no cover
+            # this is deprecated and it's here only for the doubts that something
+            # would happen the day after I'll remove it! :)
+            return self.unknown_idp(request, idp='unknown')
 
         # is a embedded wayf or DiscoveryService needed?
         configured_idps = available_idps(conf)
@@ -186,9 +193,9 @@ def get(self, request, *args, **kwargs):
                 })
 
         # is the first one, otherwise next logger message will print None
-        if not configured_idps:
+        if not configured_idps: # pragma: no cover
             raise IdPConfigurationMissing(
-                ('IdP configuration is missing or its metadata is expired.'))
+                ('IdP is missing or its metadata is expired.'))
         if selected_idp is None:
             selected_idp = list(configured_idps.keys())[0]
 
@@ -202,15 +209,17 @@ def get(self, request, *args, **kwargs):
             )
             sso_kwargs['scoping'] = idp_scoping
 
-
         # choose a binding to try first
         binding = getattr(settings, 'SAML_DEFAULT_BINDING',
                           saml2.BINDING_HTTP_POST)
         logger.debug(f'Trying binding {binding} for IDP {selected_idp}')
 
         # ensure our selected binding is supported by the IDP
-        supported_bindings = get_idp_sso_supported_bindings(
-            selected_idp, config=conf)
+        try:
+            supported_bindings = get_idp_sso_supported_bindings(
+                selected_idp, config=conf)
+        except saml2.s_utils.UnknownSystemEntity:
+            return self.unknown_idp(request, selected_idp)
 
         if binding not in supported_bindings:
             logger.debug(
@@ -223,17 +232,17 @@ def get(self, request, *args, **kwargs):
                     f'trying {saml2.BINDING_HTTP_REDIRECT}',
                 )
                 binding = saml2.BINDING_HTTP_REDIRECT
-            else:
+            else: # pragma: no cover
                 logger.warning(
                     f'IDP {selected_idp} does not support {binding} '
                     f'trying {saml2.BINDING_HTTP_POST}',
                 )
                 binding = saml2.BINDING_HTTP_POST
             # if switched binding still not supported, give up
-            if binding not in supported_bindings:
+            if binding not in supported_bindings: # pragma: no cover
                 raise UnsupportedBinding(
                     f'IDP {selected_idp} does not support '
-                    f'{saml2.BINDING_HTTP_POST} and {saml2.BINDING_HTTP_REDIRECT}'
+                    f'{saml2.BINDING_HTTP_POST} or {saml2.BINDING_HTTP_REDIRECT}'
                 )
 
         client = Saml2Client(conf)