Deprecate UserIdHashType

There seems to be no reason for the mapping between the nameid-format
and the hash-type. Hashing data for no reason seems to be causing
problems, so this behaviour will be removed, starting by deprecating
this relation between the nameid-format and the "hash type".

Also see:
https://lists.sunet.se/pipermail/idpy-discuss/2018-September/000274.html

Signed-off-by: Ivan Kanakarakis <[email protected]>

Author: c00kiemon5ter

src/satosa/deprecated.py

@@ -0,0 +1,93 @@
+import warnings as _warnings
+from enum import Enum
+
+from saml2.saml import NAMEID_FORMAT_TRANSIENT
+from saml2.saml import NAMEID_FORMAT_PERSISTENT
+from saml2.saml import NAMEID_FORMAT_EMAILADDRESS
+from saml2.saml import NAMEID_FORMAT_UNSPECIFIED
+
+
+_warnings.simplefilter("default")
+
+
+class UserIdHashType(Enum):
+    """
+    All different user id hash types
+    """
+
+    transient = 1
+    persistent = 2
+    pairwise = 3
+    public = 4
+    emailaddress = 5
+    unspecified = 6
+
+    def __getattr__(self, name):
+        msg = "UserIdHashType is deprecated and will be removed."
+        _warnings.warn(msg, DeprecationWarning)
+        return self.__getattribute__(name)
+
+    @classmethod
+    def from_string(cls, str):
+        msg = "UserIdHashType is deprecated and will be removed."
+        _warnings.warn(msg, DeprecationWarning)
+        try:
+            return getattr(cls, str)
+        except AttributeError:
+            raise ValueError("Unknown hash type '{}'".format(str))
+
+
+def saml_name_id_format_to_hash_type(name_format):
+    """
+    Translate pySAML2 name format to satosa format
+
+    :type name_format: str
+    :rtype: satosa.internal_data.UserIdHashType
+    :param name_format: SAML2 name format
+    :return: satosa format
+    """
+    msg = "saml_name_id_format_to_hash_type is deprecated and will be removed."
+    _warnings.warn(msg, DeprecationWarning)
+
+    name_id_format_to_hash_type = {
+        NAMEID_FORMAT_TRANSIENT: UserIdHashType.transient,
+        NAMEID_FORMAT_PERSISTENT: UserIdHashType.persistent,
+        NAMEID_FORMAT_EMAILADDRESS: UserIdHashType.emailaddress,
+        NAMEID_FORMAT_UNSPECIFIED: UserIdHashType.unspecified,
+    }
+
+    return name_id_format_to_hash_type.get(
+        name_format, UserIdHashType.transient
+    )
+
+
+def hash_type_to_saml_name_id_format(hash_type):
+    """
+    Translate satosa format to pySAML2 name format
+
+    :type hash_type: satosa.internal_data.UserIdHashType
+    :rtype: str
+    :param hash_type: satosa format
+    :return: pySAML2 name format
+    """
+    msg = "hash_type_to_saml_name_id_format is deprecated and will be removed."
+    _warnings.warn(msg, DeprecationWarning)
+
+    hash_type_to_name_id_format = {
+        UserIdHashType.transient: NAMEID_FORMAT_TRANSIENT,
+        UserIdHashType.persistent: NAMEID_FORMAT_PERSISTENT,
+        UserIdHashType.emailaddress: NAMEID_FORMAT_EMAILADDRESS,
+        UserIdHashType.unspecified: NAMEID_FORMAT_UNSPECIFIED,
+    }
+
+    return hash_type_to_name_id_format.get(hash_type, NAMEID_FORMAT_PERSISTENT)
+
+
+def oidc_subject_type_to_hash_type(subject_type):
+    msg = "oidc_subject_type_to_hash_type is deprecated and will be removed."
+    _warnings.warn(msg, DeprecationWarning)
+
+    if subject_type == "public":
+        return UserIdHashType.public
+
+    return UserIdHashType.pairwise

src/satosa/frontends/openid_connect.py

@@ -22,21 +22,16 @@
 
 from .base import FrontendModule
 from ..internal_data import InternalRequest
-from ..internal_data import UserIdHashType
 from ..logging_util import satosa_logging
 from ..response import BadRequest, Created
 from ..response import SeeOther, Response
 from ..response import Unauthorized
 from ..util import rndstr
 
-logger = logging.getLogger(__name__)
-
+from satosa.deprecated import oidc_subject_type_to_hash_type
 
-def oidc_subject_type_to_hash_type(subject_type):
-    if subject_type == "public":
-        return UserIdHashType.public
 
-    return UserIdHashType.pairwise
+logger = logging.getLogger(__name__)
 
 
 class OpenIDConnectFrontend(FrontendModule):
@@ -140,12 +135,12 @@ def handle_backend_error(self, exception):
         """
         auth_req = self._get_authn_request_from_state(exception.state)
         # If the client sent us a state parameter, we should reflect it back according to the spec
-        if 'state' in auth_req: 
+        if 'state' in auth_req:
             error_resp = AuthorizationErrorResponse(error="access_denied",
                                                     error_description=exception.message,
                                                     state=auth_req['state'])
-        else:                                           
-            error_resp = AuthorizationErrorResponse(error="access_denied", 
+        else:
+            error_resp = AuthorizationErrorResponse(error="access_denied",
                                                     error_description=exception.message)
         satosa_logging(logger, logging.DEBUG, exception.message, exception.state)
         return SeeOther(error_resp.request(auth_req["redirect_uri"], should_fragment_encode(auth_req)))
@@ -286,7 +281,7 @@ def _handle_authn_request(self, context):
 
         client_id = authn_req["client_id"]
         context.state[self.name] = {"oidc_request": request}
-        hash_type = oidc_subject_type_to_hash_type(self.provider.clients[client_id].get("subject_type", "pairwise"))
+        hash_type = self.provider.clients[client_id].get("subject_type", "pairwise")
         client_name = self.provider.clients[client_id].get("client_name")
         if client_name:
             # TODO should process client names for all languages, see OIDC Registration, Section 2.1

src/satosa/frontends/saml2.py

@@ -27,57 +27,32 @@
 from satosa.base import SAMLBaseModule
 from satosa.context import Context
 from .base import FrontendModule
-from ..internal_data import InternalRequest, UserIdHashType
+from ..internal_data import InternalRequest
 from ..logging_util import satosa_logging
 from ..response import Response
 from ..response import ServiceError
 from ..saml_util import make_saml_response
 import satosa.util as util
 
-
-logger = logging.getLogger(__name__)
+from satosa.deprecated import saml_name_id_format_to_hash_type
+from satosa.deprecated import hash_type_to_saml_name_id_format
 
 
-def saml_name_id_format_to_hash_type(name_format):
-    """
-    Translate pySAML2 name format to satosa format
-
-    :type name_format: str
-    :rtype: satosa.internal_data.UserIdHashType
-    :param name_format: SAML2 name format
-    :return: satosa format
-    """
-    name_id_format_to_hash_type = {
-        NAMEID_FORMAT_TRANSIENT:    UserIdHashType.transient,
-        NAMEID_FORMAT_PERSISTENT:   UserIdHashType.persistent,
-        NAMEID_FORMAT_EMAILADDRESS: UserIdHashType.emailaddress,
-        NAMEID_FORMAT_UNSPECIFIED:  UserIdHashType.unspecified,
-    }
-
-    return name_id_format_to_hash_type.get(
-            name_format,
-            UserIdHashType.transient)
+logger = logging.getLogger(__name__)
 
 
-def hash_type_to_saml_name_id_format(hash_type):
-    """
-    Translate satosa format to pySAML2 name format
+subject_type_map = {
+    NAMEID_FORMAT_TRANSIENT: NAMEID_FORMAT_TRANSIENT,
+    NAMEID_FORMAT_PERSISTENT: NAMEID_FORMAT_PERSISTENT,
+    NAMEID_FORMAT_EMAILADDRESS: NAMEID_FORMAT_EMAILADDRESS,
+    NAMEID_FORMAT_UNSPECIFIED: NAMEID_FORMAT_UNSPECIFIED,
+    "public": NAMEID_FORMAT_PERSISTENT,
+    "pairwise": NAMEID_FORMAT_TRANSIENT,
+}
 
-    :type hash_type: satosa.internal_data.UserIdHashType
-    :rtype: str
-    :param hash_type: satosa format
-    :return: pySAML2 name format
-    """
-    hash_type_to_name_id_format = {
-        UserIdHashType.transient:    NAMEID_FORMAT_TRANSIENT,
-        UserIdHashType.persistent:   NAMEID_FORMAT_PERSISTENT,
-        UserIdHashType.emailaddress: NAMEID_FORMAT_EMAILADDRESS,
-        UserIdHashType.unspecified:  NAMEID_FORMAT_UNSPECIFIED,
-    }
 
-    return hash_type_to_name_id_format.get(
-            hash_type,
-            NAMEID_FORMAT_PERSISTENT)
+def subject_type_to_saml_nameid_format(subject_type):
+    return subject_type_map.get(subject_type, NAMEID_FORMAT_PERSISTENT)
 
 
 class SAMLFrontend(FrontendModule, SAMLBaseModule):
@@ -224,14 +199,14 @@ def _handle_authn_request(self, context, binding_in, idp):
                                                            context.request.get("RelayState"))
 
         if authn_req.name_id_policy and authn_req.name_id_policy.format:
-            name_format = saml_name_id_format_to_hash_type(authn_req.name_id_policy.format)
+            name_format = authn_req.name_id_policy.format
         else:
             # default to name id format from metadata, or just transient name id
             name_format_from_metadata = idp.metadata[requester]["spsso_descriptor"][0].get("name_id_format")
             if name_format_from_metadata:
-                name_format = saml_name_id_format_to_hash_type(name_format_from_metadata[0]["text"])
+                name_format = name_format_from_metadata[0]["text"]
             else:
-                name_format = UserIdHashType.transient
+                name_format = NAMEID_FORMAT_TRANSIENT
 
         requester_name = self._get_sp_display_name(idp, requester)
         internal_req = InternalRequest(name_format, requester, requester_name)
@@ -322,11 +297,16 @@ def _handle_authn_response(self, context, internal_response, idp):
             for k in attributes_to_remove:
                 ava.pop(k, None)
 
-        name_id = NameID(text=internal_response.user_id,
-                         format=hash_type_to_saml_name_id_format(
-                             internal_response.user_id_hash_type),
-                         sp_name_qualifier=None,
-                         name_qualifier=None)
+        nameid_value = internal_response.user_id
+        nameid_format = subject_type_to_saml_nameid_format(
+            internal_response.user_id_hash_type
+        )
+        name_id = NameID(
+            text=nameid_value,
+            format=nameid_format,
+            sp_name_qualifier=None,
+            name_qualifier=None,
+        )
 
         dbgmsg = "returning attributes %s" % json.dumps(ava)
         satosa_logging(logger, logging.DEBUG, dbgmsg, context.state)

src/satosa/internal_data.py

@@ -3,28 +3,14 @@
 for converting from SAML/OAuth/OpenID connect to the internal representation.
 """
 import datetime
-from enum import Enum
-
-import satosa.util as util
 
+from saml2.saml import NAMEID_FORMAT_TRANSIENT
+from saml2.saml import NAMEID_FORMAT_PERSISTENT
+from saml2.saml import NAMEID_FORMAT_EMAILADDRESS
+from saml2.saml import NAMEID_FORMAT_UNSPECIFIED
 
-class UserIdHashType(Enum):
-    """
-    All different user id hash types
-    """
-    transient = 1
-    persistent = 2
-    pairwise = 3
-    public = 4
-    emailaddress = 5
-    unspecified = 6
-
-    @classmethod
-    def from_string(cls, str):
-        try:
-            return getattr(cls, str)
-        except AttributeError:
-            raise ValueError("Unknown hash type '{}'".format(str))
+import satosa.util as util
+from satosa.deprecated import UserIdHashType
 
 
 class UserIdHasher(object):
@@ -44,7 +30,7 @@ def save_state(internal_request, state):
         :param state: The current state
         """
         state_data = {
-            "hash_type": internal_request.user_id_hash_type.name
+            "hash_type": internal_request.user_id_hash_type
         }
         state[UserIdHasher.STATE_KEY] = state_data
 
@@ -63,7 +49,7 @@ def hash_data(salt, value):
     @staticmethod
     def hash_type(state):
         state_data = state[UserIdHasher.STATE_KEY]
-        hash_type = UserIdHashType.from_string(state_data["hash_type"])
+        hash_type = state_data["hash_type"]
         return hash_type
 
     @staticmethod
@@ -85,12 +71,12 @@ def hash_id(salt, user_id, requester, state):
         :return: the internal_response containing the hashed user ID
         """
         hash_type_to_format = {
-            UserIdHashType.transient:    '{id}{req}{time}',
-            UserIdHashType.persistent:   '{id}{req}',
-            UserIdHashType.pairwise:     '{id}{req}',
-            UserIdHashType.public:       '{id}',
-            UserIdHashType.emailaddress: '{id}',
-            UserIdHashType.unspecified:  '{id}',
+            NAMEID_FORMAT_TRANSIENT:    '{id}{req}{time}',
+            NAMEID_FORMAT_PERSISTENT:   '{id}{req}',
+            "pairwise":                 '{id}{req}',
+            "public":                   '{id}',
+            NAMEID_FORMAT_EMAILADDRESS: '{id}',
+            NAMEID_FORMAT_UNSPECIFIED:  '{id}',
         }
 
         format_args = {
@@ -110,8 +96,8 @@ def hash_id(salt, user_id, requester, state):
         hasher = (
             (lambda salt, value: value)
             if hash_type in [
-                UserIdHashType.emailaddress,
-                UserIdHashType.unspecified,
+                NAMEID_FORMAT_EMAILADDRESS,
+                NAMEID_FORMAT_UNSPECIFIED,
             ]
             else UserIdHasher.hash_data)
         return hasher(salt, user_id)
@@ -179,7 +165,7 @@ def __init__(self, user_id_hash_type, requester, requester_name=None):
         :param user_id_hash_type:
         :param requester: identifier of the requester
 
-        :type user_id_hash_type: UserIdHashType
+        :type user_id_hash_type: str
         :type requester: str
         """
         self.user_id_hash_type = user_id_hash_type
@@ -197,7 +183,7 @@ class InternalResponse(InternalData):
 
     :type _user_id: str
     :type attributes: dict[str, str]
-    :type user_id_hash_type: UserIdHashType
+    :type user_id_hash_type: str
     :type auth_info: AuthenticationInformation
     """
 
@@ -221,7 +207,7 @@ def from_dict(int_resp_dict):
         auth_info = AuthenticationInformation.from_dict(int_resp_dict["auth_info"])
         internal_response = InternalResponse(auth_info=auth_info)
         if "hash_type" in int_resp_dict:
-            internal_response.user_id_hash_type = UserIdHashType.from_string(int_resp_dict["hash_type"])
+            internal_response.user_id_hash_type = int_resp_dict["hash_type"]
         internal_response.attributes = int_resp_dict["attr"]
         internal_response.user_id = int_resp_dict["usr_id"]
         internal_response.requester = int_resp_dict["to"]
@@ -238,5 +224,5 @@ def to_dict(self):
                  "to": self.requester,
                  "auth_info": self.auth_info.to_dict()}
         if self.user_id_hash_type:
-            _dict["hash_type"] = self.user_id_hash_type.name
+            _dict["hash_type"] = self.user_id_hash_type
         return _dict