From bdc2fa1b4d0d53c80deaa4be15a67e39e107b8fa Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu, 17 Mar 2022 12:05:07 +0100
Subject: [PATCH] idp_oidc: discard profile selection from 'do not ask again'
 logic (#62889)

---
 .../authentic2_idp_oidc/authorization.html    | 13 ++++--
 src/authentic2_idp_oidc/views.py              |  8 ----
 tests/idp_oidc/test_user_profiles.py          | 45 ++++++++++++++++++-
 3 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/src/authentic2_idp_oidc/templates/authentic2_idp_oidc/authorization.html b/src/authentic2_idp_oidc/templates/authentic2_idp_oidc/authorization.html
index 4e06ae167..66845a3d3 100644
--- a/src/authentic2_idp_oidc/templates/authentic2_idp_oidc/authorization.html
+++ b/src/authentic2_idp_oidc/templates/authentic2_idp_oidc/authorization.html
@@ -35,20 +35,25 @@
           <label for="{{ profile.id }}">
           {% with identifier=profile.identifier type_name=profile.profile_type.name %}
             {% trans "Unknown entity" as fallback_identifier %}
+            <div>
             {% blocktrans %}Profile of type {{ type_name }}: {% endblocktrans %}
-            {% firstof identifier fallback_identifier %}<br />
+            {% firstof identifier fallback_identifier %}
+            </div>
           {% endwith %}
           {% if profile.email %}
-            {% blocktrans with email=profile.email %}Email address: {{ email }}{% endblocktrans %}<br />
+            <div>
+            {% blocktrans with email=profile.email %}Email address: {{ email }}{% endblocktrans %}
+            </div>
           {% endif %}</label>
         </div>
       {% endfor %}
      </div>
-    {% endif %}
-    {% csrf_token %}
+    {% else %}
     <p class="a2-oidc-authorization-form--do-not-ask-again">
       <label for="id_do_not_ask_again"><input id="id_do_not_ask_again" type="checkbox" name="do_not_ask_again" value="on"/><span>{% trans "Do not ask again" %}</span></label>
     </p>
+    {% endif %}
+    {% csrf_token %}
     <button name="accept">{% trans "Accept" %}</button>
     <button name="refuse">{% trans "Refuse" %}</button>
   </form>
diff --git a/src/authentic2_idp_oidc/views.py b/src/authentic2_idp_oidc/views.py
index 5a96601fc..2fe5584a8 100644
--- a/src/authentic2_idp_oidc/views.py
+++ b/src/authentic2_idp_oidc/views.py
@@ -383,21 +383,14 @@ def authorize_for_client(request, client, redirect_uri):
         else:
             qs = qs.filter(expired__gte=iat)
         authorized_scopes = set()
-        authorized_profile = None
         for authorization in qs:
             authorized_scopes |= authorization.scope_set()
-            # load first authorized profile
-            if not authorized_profile and authorization.profile:
-                authorized_profile = authorization.profile
         if (
             request.user.profiles.count()
-            and not authorized_profile
             and client.activate_user_profiles
             and a2_app_settings.A2_USER_PROFILE_MANAGEMENT
         ):
             needs_profile_validation = True
-        else:
-            profile = authorized_profile
         if (authorized_scopes & scopes) < scopes:
             needs_scope_validation = True
         if needs_scope_validation or needs_profile_validation:
@@ -421,7 +414,6 @@ def authorize_for_client(request, client, redirect_uri):
                                 pk_to_deletes.append(authorization.pk)
                         auth_manager.create(
                             user=request.user,
-                            profile=profile,
                             scopes=' '.join(sorted(scopes)),
                             expired=iat + datetime.timedelta(days=365),
                         )
diff --git a/tests/idp_oidc/test_user_profiles.py b/tests/idp_oidc/test_user_profiles.py
index 1fee893f6..d9477fdf6 100644
--- a/tests/idp_oidc/test_user_profiles.py
+++ b/tests/idp_oidc/test_user_profiles.py
@@ -29,7 +29,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.models import OIDCAccessToken, OIDCAuthorization, OIDCCode
 from authentic2_idp_oidc.utils import get_jwkset, make_pairwise_sub, make_sub, reverse_pairwise_sub
 
 from .. import utils
@@ -380,3 +380,46 @@ def test_full_sub_reversibility(app, oidc_client, profile_settings):
         assert sub_with_profile != sub_without_profile
         assert uuid_with_profile == uuid_without_profile
         assert uuid_with_profile == UUID(user.uuid).bytes
+
+
+def test_do_no_ask_again(app, oidc_client, profile_user, profile_settings):
+    oidc_client.idtoken_algo = oidc_client.ALGO_HMAC
+    oidc_client.activate_user_profiles = True
+    oidc_client.perform_sub_profile_substitution = True
+    oidc_client.save()
+    redirect_uri = oidc_client.redirect_uris.split()[0]
+    params = {
+        'client_id': oidc_client.client_id,
+        'scope': 'openid profile email',
+        'redirect_uri': redirect_uri,
+        'state': 'xxx',
+        'nonce': 'yyy',
+        'login_hint': 'backoffice john@example.com',
+        'response_type': 'code',
+    }
+
+    authorize_url = make_url('oidc-authorize', params=params)
+    utils.login(app, profile_user)
+
+    # attempt with profile, 'do not ask again' checkbox absent
+    response = app.get(authorize_url)
+    assert 'do_not_ask_again' not in response.text
+
+    # when no profile is set, fallback to usual 'do not ask again' logic
+    Profile.objects.filter(user=profile_user).delete()
+    response = app.get(authorize_url)
+    assert 'do_not_ask_again' in response.text
+    assert OIDCAuthorization.objects.filter(user=profile_user).count() == 0
+    response.form['do_not_ask_again'] = True
+    response.form.submit('accept')
+
+    # when an authorization has been defined yet the user posesses profiles,
+    # the authorization form is still displayed
+    assert OIDCAuthorization.objects.filter(user=profile_user).count() == 1
+    Profile.objects.create(
+        user=profile_user,
+        profile_type=ProfileType.objects.first(),
+        email='entity123@example.org',
+    )
+    response = app.get(authorize_url)
+    assert 'do_not_ask_again' not in response.text
-- 
2.35.1

