From c4b09e94d63b1ce2e10cc089a59144a21597c7a2 Mon Sep 17 00:00:00 2001
From: Art Lukyanchyk <artiom.lukyanchyk@hs-hannover.de>
Date: Wed, 7 Apr 2021 20:03:20 +0200
Subject: [PATCH] Despagettify hsh app compat and make it more straightforward

---
 README.md                        |  1 -
 ssoauth/app_settings/defaults.py |  3 +-
 ssoauth/auth_utils.py            | 47 ++++++++++++++------------------
 ssoauth/checks.py                | 11 ++++++++
 ssoauth/extras/__init__.py       |  0
 ssoauth/extras/hsh_compat.py     | 31 +++++++++++++++++++++
 ssoauth/urls.py                  |  2 +-
 ssoauth/views.py                 |  2 +-
 8 files changed, 65 insertions(+), 32 deletions(-)
 create mode 100644 ssoauth/extras/__init__.py
 create mode 100644 ssoauth/extras/hsh_compat.py

diff --git a/README.md b/README.md
index 1044ee2..c02d4f3 100644
--- a/README.md
+++ b/README.md
@@ -28,7 +28,6 @@ If you are not sure whether you need it: you don't need it.
 Use this only if you want an actual SSO with SAML2. For extra details see the default settings in `ssoauth.app_settings.defaults`. You might also need SSL, try using `nginx` for it.
 
 ```python
-""" settings/dev.py """
 import os, socket
 from django import urls
 
diff --git a/ssoauth/app_settings/defaults.py b/ssoauth/app_settings/defaults.py
index cdcb34b..4b9aad1 100644
--- a/ssoauth/app_settings/defaults.py
+++ b/ssoauth/app_settings/defaults.py
@@ -31,8 +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"
 
-GROUP_RESOLVER = "ssoauth.auth_utils.groups_from_group_names"  # 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
 
 PREDEFINED_GROUPS = {
@@ -80,6 +78,7 @@ ATTRIBUTE_SURNAME = "urn:oid:2.5.4.4"  # "sn"
 ATTRIBUTE_ACCOUNT_UUID = "UUID"  # custom stuff, this one has no OID
 ATTRIBUTE_GROUPS = "urn:oid:1.3.6.1.4.1.5923.1.5.1.1"  # "isMemberOf"
 
+GROUP_RESOLVER = None  # deprecated
 
 """
 This information will be published in this SP metadata...
diff --git a/ssoauth/auth_utils.py b/ssoauth/auth_utils.py
index f52a6eb..4ce6bd6 100644
--- a/ssoauth/auth_utils.py
+++ b/ssoauth/auth_utils.py
@@ -1,12 +1,12 @@
+from uuid import UUID
 from django.db import transaction
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Group
-from uuid import UUID
 from . import models
 from . import logger
 from . import app_settings
 from . import SUPERUSER_PERM_CODENAME, STAFF_PERM_CODENAME
-import importlib
+from .extras import hsh_compat
 
 
 def _validate_username(username):
@@ -96,20 +96,24 @@ def update_user_data(user, surname=None, forename=None, email=None):
     user.save()
 
 
-def set_user_groups(user, raw_values):
-    """ Updates groups for the user. """
-    # get the group resolver method
-    try:
-        resolver_path = app_settings.GROUP_RESOLVER.split(".")
-        assert len(resolver_path) >= 2, "need full path of the resolver method"
-        resolver_module_name = ".".join(resolver_path[:-1])
-        resolver_method_name = resolver_path[-1]
-        resolver_module = importlib.import_module(resolver_module_name)
-        resolver_method = getattr(resolver_module, resolver_method_name)
-    except Exception as e:
-        raise ImportError("Could not import {r}. {e.__class__.__name__}: {e}".format(r=app_settings.GROUP_RESOLVER, e=e))
-    # resolve the groups
-    groups = set(resolver_method(user, raw_values))
+def _groups_from_group_names(group_names):
+    """ This default group resolver returns list of Group objects based on group names """
+    groups = set(Group.objects.filter(name__in=group_names))
+    # log missing groups, might help to understand what is going on
+    found_group_names = {g.name for g in groups}
+    missing_group_names = set(group_names) - found_group_names
+    if missing_group_names:
+        n = len(missing_group_names)
+        logger.info("Received {n} names of nonexistent groups: {missing_group_names}".format(**locals()))
+    # done
+    return groups
+
+
+def set_user_groups(user, group_names):
+    if hsh_compat.HSH_APP_COMPAT_POSSIBLE:
+        logger.info("Resolving groups names for {user} directly from hsh app.".format(user=user))
+        group_names = hsh_compat.get_group_names_for_user(user)
+    groups = set(_groups_from_group_names(group_names))
     # update user groups
     if set(user.groups.all()) != groups:
         user.groups.set(groups)
@@ -119,17 +123,6 @@ def set_user_groups(user, raw_values):
     logger.info("User {user} is member of: {groups}".format(user=user, groups=[str(g) for g in groups]))
 
 
-def groups_from_group_names(user, saml2_raw_values):
-    """ This default group resolver returns list of Group objects based on group names as raw_values """
-    groups = set(Group.objects.filter(name__in=saml2_raw_values))
-    # perform some sanity checks
-    group_names_redundant = set(saml2_raw_values) - {g.name for g in groups}
-    if group_names_redundant:
-        logger.warning("Received redundant group names (clean up the configs): {0}".format(group_names_redundant))
-    # done
-    return groups
-
-
 def cleanup_direct_permissions(user):
     if user.user_permissions.exists():
         logger.critical("Who attached permissions directly to {user} ?!?!".format(**locals()))
diff --git a/ssoauth/checks.py b/ssoauth/checks.py
index 87b38dd..8963b8f 100644
--- a/ssoauth/checks.py
+++ b/ssoauth/checks.py
@@ -136,3 +136,14 @@ def production_entity_id(app_configs, **kwargs):
         ))
     return errors
 
+
+@register(Tags.compatibility)
+def group_resolver_deprecated(app_configs, **kwargs):
+    errors = list()
+    if app_settings.GROUP_RESOLVER is not None:
+        errors.append(Warning(
+            "GROUP_RESOLVER is deprecated. If hsh app is present, groups are resolved from it automatically instead of the received SAML group names.",
+            obj=conf.settings,
+        ))
+    return errors
+
diff --git a/ssoauth/extras/__init__.py b/ssoauth/extras/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/ssoauth/extras/hsh_compat.py b/ssoauth/extras/hsh_compat.py
new file mode 100644
index 0000000..d66398d
--- /dev/null
+++ b/ssoauth/extras/hsh_compat.py
@@ -0,0 +1,31 @@
+"""
+Compatibility features with the `hsh` app.
+"""
+
+from .. import logger
+
+
+try:
+    from hsh.models import Account
+    HSH_APP_COMPAT_POSSIBLE = True
+except ImportError:
+    HSH_APP_COMPAT_POSSIBLE = False
+
+
+ALLOWED_AUTH_PROVIDERS = ["fh-h.de"]
+GROUP_NAME_LOOKUP = dict(name__istartswith="WEB_")
+
+
+def get_group_names_for_user(user):
+    """
+    Resolve group names for the user directly from hsh app.
+    It's better to get groups directly from the database, instead of trusting AD data received via SAML2.
+    """
+    try:
+        hsh_account = Account.objects.get(username=user.username, auth_provider__in=ALLOWED_AUTH_PROVIDERS)
+    except (Account.DoesNotExist, Account.MultipleObjectsReturned,) as e:
+        logger.error("hsh.Account not found for {user}. {e.__class__.__name__}: {e}".format(user=user, e=e))
+        return set()
+    hsh_groups = {hsh_account.auth_groups.filter(**GROUP_NAME_LOOKUP)}
+    return {g.name for g in hsh_groups}
+
diff --git a/ssoauth/urls.py b/ssoauth/urls.py
index 550dc82..a395560 100644
--- a/ssoauth/urls.py
+++ b/ssoauth/urls.py
@@ -15,5 +15,5 @@ urlpatterns = (
 if conf.settings.DEBUG:
     # add the dev view
     urlpatterns += (
-        url(r"^dev/?$", views.DevView.as_view(), name="sso-dev"),
+        url(r"^sso-dev/?$", views.DevView.as_view(), name="sso-dev"),
     )
diff --git a/ssoauth/views.py b/ssoauth/views.py
index b71db1c..fb7d970 100644
--- a/ssoauth/views.py
+++ b/ssoauth/views.py
@@ -204,7 +204,7 @@ class ACSAuthNView(SAMLMixin, View):
         )
         auth_utils.set_user_groups(
             user=user,
-            raw_values=get_attr(app_settings.ATTRIBUTE_GROUPS, nullable=True, multivalued=True) or list()
+            group_names=get_attr(app_settings.ATTRIBUTE_GROUPS, nullable=True, multivalued=True) or list()
         )
         # other necessary steps
         auth_utils.cleanup_direct_permissions(user=user)  # in case somebody was acting funny
-- 
GitLab