From 69f8e3f46cca39206042437f64b7306fc627e2d9 Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Fri, 11 Mar 2022 16:36:36 +0100
Subject: [PATCH] idp_oidc: fix profile suffix during sub generation (#62702)

---
 src/authentic2_idp_oidc/apps.py      |  4 +--
 src/authentic2_idp_oidc/utils.py     |  7 ++--
 tests/idp_oidc/test_user_profiles.py | 48 +++++++++++++++++++++++++++-
 3 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/src/authentic2_idp_oidc/apps.py b/src/authentic2_idp_oidc/apps.py
index 38e7077f5..cc83f337b 100644
--- a/src/authentic2_idp_oidc/apps.py
+++ b/src/authentic2_idp_oidc/apps.py
@@ -88,7 +88,7 @@ class AppConfig(django.apps.AppConfig):
         sub = smart_bytes(view.kwargs[lookup_url_kwarg])
         decrypted = utils.reverse_pairwise_sub(client, sub)
         if decrypted:
-            view.kwargs[lookup_url_kwarg] = uuid.UUID(bytes=decrypted.split(b'#')[0]).hex
+            view.kwargs[lookup_url_kwarg] = uuid.UUID(bytes=decrypted).hex
 
     def a2_hook_api_modify_serializer_after_validation(self, view, serializer):
         import uuid
@@ -111,7 +111,7 @@ class AppConfig(django.apps.AppConfig):
         for u in serializer.validated_data['known_uuids']:
             decrypted = utils.reverse_pairwise_sub(client, smart_bytes(u))
             if decrypted:
-                new_known_uuid = uuid.UUID(bytes=decrypted.split(b'#')[0]).hex
+                new_known_uuid = uuid.UUID(bytes=decrypted).hex
                 new_known_uuids.append(new_known_uuid)
                 uuid_map[new_known_uuid] = u
             else:
diff --git a/src/authentic2_idp_oidc/utils.py b/src/authentic2_idp_oidc/utils.py
index df3466e4e..01e9da7a1 100644
--- a/src/authentic2_idp_oidc/utils.py
+++ b/src/authentic2_idp_oidc/utils.py
@@ -175,16 +175,19 @@ def make_pairwise_reversible_sub_from_uuid(client, user_uuid, profile=None):
         sector_identifier,
     ]
     if profile and client.perform_sub_profile_substitution:
-        cipher_args[1] += b'#%s' % str(profile.id).encode()
+        # add user-chosen profile identifier information
+        cipher_args[1] += b'#profile-id:%s' % str(profile.id).encode()
     return crypto.aes_base64url_deterministic_encrypt(*cipher_args).decode('utf-8')
 
 
 def reverse_pairwise_sub(client, sub):
     sector_identifier = client.get_sector_identifier()
     try:
-        return crypto.aes_base64url_deterministic_decrypt(
+        reversed_id = crypto.aes_base64url_deterministic_decrypt(
             settings.SECRET_KEY.encode('utf-8'), sub, sector_identifier
         )
+        # strip any suffix from original 16-byte-long uuid
+        return reversed_id[:16]
     except crypto.DecryptionError:
         return None
 
diff --git a/tests/idp_oidc/test_user_profiles.py b/tests/idp_oidc/test_user_profiles.py
index 5e9c04534..1fee893f6 100644
--- a/tests/idp_oidc/test_user_profiles.py
+++ b/tests/idp_oidc/test_user_profiles.py
@@ -17,6 +17,7 @@
 import base64
 import json
 import urllib.parse
+from uuid import UUID
 
 import pytest
 from django.contrib.auth import get_user_model
@@ -29,7 +30,7 @@ from jwcrypto.jwt import JWT
 from authentic2.custom_user.models import Profile, ProfileType
 from authentic2.utils.misc import make_url
 from authentic2_idp_oidc.models import OIDCAccessToken, OIDCCode
-from authentic2_idp_oidc.utils import get_jwkset, make_sub
+from authentic2_idp_oidc.utils import get_jwkset, make_pairwise_sub, make_sub, reverse_pairwise_sub
 
 from .. import utils
 from .conftest import client_authentication_headers
@@ -334,3 +335,48 @@ def test_login_profile_reversible_sub(app, oidc_client, profile_user, profile_se
     claims = json.loads(jwt.claims)
     assert claims['sub'] != make_sub(oidc_client, profile_user)
     assert claims['sub'] == make_sub(oidc_client, profile_user, profile=profile_user.profiles.first())
+
+
+def test_full_sub_reversibility(app, oidc_client, profile_settings):
+    from django.utils.encoding import smart_bytes
+
+    oidc_client.idtoken_algo = oidc_client.ALGO_EC
+    oidc_client.activate_user_profiles = True
+    oidc_client.perform_sub_profile_substitution = True
+    oidc_client.identifier_policy = oidc_client.POLICY_PAIRWISE_REVERSIBLE
+    oidc_client.save()
+    profile_type_manager = ProfileType.objects.create(
+        name='One Manager Type',
+        slug='one-manager-type',
+    )
+    # first with no profiles
+    for i in range(10):
+        user = User.objects.create(
+            first_name='john-%s' % i,
+            last_name='doe',
+            email='john.doe.%s@example.org' % i,
+        )
+        sub = make_pairwise_sub(oidc_client, user, profile=None)
+        uuid = reverse_pairwise_sub(oidc_client, smart_bytes(sub))
+        assert uuid == UUID(user.uuid).bytes
+
+    # then adding user profile information
+    for i in range(100):
+        user = User.objects.create(
+            first_name='john-%s' % i,
+            last_name='doe',
+            email='john.doe.%s@example.org' % i,
+        )
+        profile = Profile.objects.create(
+            user=user,
+            profile_type=profile_type_manager,
+            identifier='manager %s' % i,
+            email='manager-%s@example.org' % i,
+        )
+        sub_with_profile = make_pairwise_sub(oidc_client, user, profile=profile)
+        sub_without_profile = make_pairwise_sub(oidc_client, user, profile=None)
+        uuid_with_profile = reverse_pairwise_sub(oidc_client, smart_bytes(sub_with_profile))
+        uuid_without_profile = reverse_pairwise_sub(oidc_client, smart_bytes(sub_without_profile))
+        assert sub_with_profile != sub_without_profile
+        assert uuid_with_profile == uuid_without_profile
+        assert uuid_with_profile == UUID(user.uuid).bytes
-- 
2.35.1

