From 2ecf0c833c60551bb526460ea19715e5fc9ebb20 Mon Sep 17 00:00:00 2001
From: Art Lukyanchyk <artiom.lukyanchyk@hs-hannover.de>
Date: Thu, 31 Jan 2019 16:42:02 +0100
Subject: [PATCH] Improve the SAML2 attribute mapping

---
 ssoauth/app_settings/defaults.py |  9 +++++-
 ssoauth/views.py                 | 52 +++++++++++++++-----------------
 2 files changed, 33 insertions(+), 28 deletions(-)

diff --git a/ssoauth/app_settings/defaults.py b/ssoauth/app_settings/defaults.py
index 3e13c80..a0ab768 100644
--- a/ssoauth/app_settings/defaults.py
+++ b/ssoauth/app_settings/defaults.py
@@ -31,7 +31,6 @@ SSO_REQUIRED_OUTSIDE_MANAGE_PY = True  # enabled to ensure that production (that
 SP_SLS_ENABLED = False  # single log out creates too many problems, so it is disabled for now
 SP_SLS_X_FRAME_OPTIONS = None  # in case you encounter problems with SLS view not allowed inside of an iframe, e.g. "ALLOW-FROM idp-test.it.hs-hannover.de idp.hs-hannover.de"
 
-GROUPS_SAML_ATTRIBUTE = "IDMGroups"  # this SAML attribute is expected to contain list of groups for a user
 GROUP_RESOLVER = "ssoauth.auth_utils.groups_from_saml2_dn_list"  # in case you want to override how groups are resolved for users
 
 LOGIN_PERM_CODENAME = None  # None or str; value "can_log_in" will require this permission for users to log in
@@ -79,6 +78,14 @@ STAFF_PERM_CODENAME = "staff"
 
 PRETEND_AUTH_BACKEND = django_settings.AUTHENTICATION_BACKENDS[0]  # pretend to be this backend; django does not expect that it is possible to log in without an authentication backend
 
+# the block below defines from which SAML2 attributes this SP receives user data
+ATTRIBUTE_USERNAME = "urn:oid:0.9.2342.19200300.100.1.1"  # "uid"
+ATTRIBUTE_EMAIL = "urn:oid:0.9.2342.19200300.100.1.3"  # "mail"
+ATTRIBUTE_FORENAME = "urn:oid:2.5.4.42"  # "givenName"
+ATTRIBUTE_SURNAME = "urn:oid:2.5.4.4"  # "sn"
+ATTRIBUTE_ACCOUNT_UUID = "UUID"  # our custom stuff, this one has no OID
+ATTRIBUTE_GROUPS = "urn:oid:1.3.6.1.4.1.5923.1.5.1.1"  # "isMemberOf"
+
 
 """
 This information will be published in this SP metadata...
diff --git a/ssoauth/views.py b/ssoauth/views.py
index 88b7503..f8f08ce 100644
--- a/ssoauth/views.py
+++ b/ssoauth/views.py
@@ -21,17 +21,6 @@ from django.utils import timezone
 from datetime import timedelta
 
 
-ATTRIBUTE_MAPPING = dict(
-    # https://commons.lbl.gov/display/IDMgmt/Attribute+Definitions
-    username="urn:oid:0.9.2342.19200300.100.1.1",
-    email="urn:oid:0.9.2342.19200300.100.1.3",
-    forename="urn:oid:2.5.4.42",
-    surname="urn:oid:2.5.4.4",
-    uuid="UUID",
-    groups=app_settings.GROUPS_SAML_ATTRIBUTE,
-)
-
-
 class SAMLMixin:
     """ Merges Django runtime info and settings for OneLogin toolkit """
 
@@ -181,35 +170,42 @@ class ACSAuthNView(SAMLMixin, View):
 
     def log_in_user(self, request, auth):
 
-        def get_attr(attribute_name, nullable=False, multivalued=False):
-            attribute_id = ATTRIBUTE_MAPPING[attribute_name]
-            values = auth.get_attributes().get(attribute_id, list())
-            assert nullable or values, "Haven't received any {0}".format(attribute_name)
+        def get_attr(attribute, nullable=False, multivalued=False):
+            values = auth.get_attributes().get(attribute, list())
+            logger.critical(attribute)
+            logger.warning(values)
+            if not values and not nullable:
+                raise ValueError("Received no value(s) for {0}".format(attribute))
             if multivalued:
                 return values
             else:
-                assert len(values) <= 1, "Received too many {0}: {1}".format(attribute_name, values)
-                return values[0] if values else None
+                if len(values) == 0:
+                    return None
+                if len(values) == 1:
+                    return values[0]
+                else:
+                    raise ValueError("Received {1} values for {0}, didn't expect that many.".format(attribute, len(values)))
 
         logger.debug("Synchronizing user using SAML2 data: {}".format(auth.get_attributes()))
         # get the user
         user = auth_utils.get_or_create_user(
-            uuid=get_attr("uuid"),
-            username=get_attr("username"),
+            uuid=get_attr(app_settings.ATTRIBUTE_ACCOUNT_UUID),
+            username=get_attr(app_settings.ATTRIBUTE_USERNAME),
         )
         # update user data
         auth_utils.update_user_data(
             user=user,
-            surname=get_attr("surname", nullable=True),
-            forename=get_attr("forename", nullable=True),
-            email=get_attr("email", nullable=True),
+            surname=get_attr(app_settings.ATTRIBUTE_SURNAME, nullable=True),
+            forename=get_attr(app_settings.ATTRIBUTE_FORENAME, nullable=True),
+            email=get_attr(app_settings.ATTRIBUTE_EMAIL, nullable=True),
         )
         auth_utils.set_user_groups(
             user=user,
-            saml2_groups=get_attr("groups", nullable=True, multivalued=True) or list()
+            saml2_groups=get_attr(app_settings.ATTRIBUTE_GROUPS, nullable=True, multivalued=True) or list()
         )
-        auth_utils.cleanup_direct_permissions(user=user)
-        auth_utils.set_user_compat_flags(user=user)
+        # other necessary steps
+        auth_utils.cleanup_direct_permissions(user=user)  # in case somebody was acting funny
+        auth_utils.set_user_compat_flags(user=user)  # superuser, staff
         user.backend = app_settings.PRETEND_AUTH_BACKEND
         request.user = user
         if user.is_active:
@@ -301,7 +297,8 @@ class DevView(FormView):
         return self.request.GET.get(REDIRECT_FIELD_NAME, None)
 
     def get_context_data(self, **kwargs):
-        assert conf.settings.DEBUG
+        if not conf.settings.DEBUG:
+            raise exceptions.PermissionDenied
         context = super().get_context_data(**kwargs)
         user = self.request.user
         groups = list(user.groups.all()) if user.is_authenticated else list()
@@ -322,7 +319,8 @@ class DevView(FormView):
         return context
 
     def form_valid(self, form):
-        assert conf.settings.DEBUG
+        if not conf.settings.DEBUG:
+            raise exceptions.PermissionDenied
         log_in_as_username = form.cleaned_data["username"]
         toggle_group = form.cleaned_data["toggle_group"]
         local_logout = bool(self.request.POST.get("local-logout", None))
-- 
GitLab