From 7bc2ca826f6d9d543bbce3aee84809c4e64d8f4b Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu, 17 Mar 2022 15:34:48 +0100
Subject: [PATCH] idp_oidc: discard any extra scopes at profile selection
 (#62900)

---
 src/authentic2_idp_oidc/apps.py               | 25 +++++++
 .../authentic2_idp_oidc/authorization.html    | 13 +++-
 src/authentic2_idp_oidc/views.py              |  2 +
 tests/idp_oidc/test_user_profiles.py          | 66 ++++++++++++++++++-
 4 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/src/authentic2_idp_oidc/apps.py b/src/authentic2_idp_oidc/apps.py
index cc83f337b..c98aa7b17 100644
--- a/src/authentic2_idp_oidc/apps.py
+++ b/src/authentic2_idp_oidc/apps.py
@@ -14,10 +14,14 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import logging
+
 import django.apps
 from django.template.loader import render_to_string
 from django.utils.encoding import smart_bytes
 
+logger = logging.getLogger(__name__)
+
 
 class Plugin:
     def logout_list(self, request):
@@ -157,3 +161,24 @@ class AppConfig(django.apps.AppConfig):
         qs = qs.distinct()
 
         return qs
+
+    def a2_hook_idp_oidc_modify_user_info(self, client, user, scope_set, user_info, profile=None):
+        """Removes any extra scope and their associated claims whenever the user selects a profile"""
+        from authentic2_idp_oidc.models import OIDCClaim
+
+        extra_scopes = scope_set - {'openid', 'email', 'profile'}
+        if profile and extra_scopes:
+            logger.info(
+                'idp_oidc: profile of type %s chosen by user %s for client %s, discarding scopes %s',
+                profile.profile_type,
+                user,
+                client,
+                ' '.join(extra_scopes),
+            )
+            scope_set -= extra_scopes
+            extra_claims = []
+            for claim in OIDCClaim.objects.filter(client=client, name__in=user_info.keys()):
+                if set(claim.get_scopes()) & extra_scopes and not set(claim.get_scopes()) & scope_set:
+                    extra_claims.append(claim.name)
+            for claim in extra_claims:
+                user_info.pop(claim)
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 28f6d4790..2ab2785d9 100644
--- a/src/authentic2_idp_oidc/templates/authentic2_idp_oidc/authorization.html
+++ b/src/authentic2_idp_oidc/templates/authentic2_idp_oidc/authorization.html
@@ -23,8 +23,17 @@
       </ul>
     {% endif %}
     {% if needs_profile_validation %}
-     <div id="profile-validation">
-     <p>{% trans "You may authenticate as owner of the following juridical entity management profile, which may change the aforementioned information." %}</p>
+      <div id="profile-validation">
+      <p>{% trans "You may authenticate as owner of the following management profile, which may change the aforementioned information." %}</p>
+      {% if "email" in scopes %}
+      <p>{% blocktrans with client_name=client.name %}In this case, {{ client_name }} will collect your managenement profile email address instead of your personal email address.{% endblocktrans %}</p>
+      {% endif %}
+      {% if "profile" in scopes %}
+      <p>{% blocktrans with client_name=client.name %}{{ client_name }} will still collect your first name, your last name and your username.{% endblocktrans %}</p>
+      {% endif %}
+      {% if extra_scopes %}
+      {% blocktrans with client_name=client.name %}Additional personal info will not be sent to {{ client_name }}.{% endblocktrans %}
+      {% endif %}
       <div class="profile" id="profile-validation-none">
         <input type="radio" id="_none" name="profile-validation" value="" checked>
         <label for="_none">{% trans "Keep my default user profile." %}</label>
diff --git a/src/authentic2_idp_oidc/views.py b/src/authentic2_idp_oidc/views.py
index 79072b9a7..f2dcbdc10 100644
--- a/src/authentic2_idp_oidc/views.py
+++ b/src/authentic2_idp_oidc/views.py
@@ -307,6 +307,7 @@ def authorize_for_client(request, client, redirect_uri):
     if not scope:
         raise MissingParameter('scope')
     scopes = utils.scope_set(scope)
+    extra_scopes = scopes - {'openid', 'email', 'profile'}
     if 'openid' not in scopes:
         raise InvalidScope(_('Scope must contain "openid", received "%s"') % ', '.join(sorted(scopes)))
     if not is_scopes_allowed(scopes, client):
@@ -438,6 +439,7 @@ def authorize_for_client(request, client, redirect_uri):
                         'needs_scope_validation': needs_scope_validation,
                         'client': client,
                         'scopes': scopes - {'openid'},
+                        'extra_scopes': extra_scopes,
                     },
                 )
     if response_type == 'code':
diff --git a/tests/idp_oidc/test_user_profiles.py b/tests/idp_oidc/test_user_profiles.py
index 6ba77514d..a187188cf 100644
--- a/tests/idp_oidc/test_user_profiles.py
+++ b/tests/idp_oidc/test_user_profiles.py
@@ -30,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, OIDCAuthorization, OIDCCode
+from authentic2_idp_oidc.models import OIDCAccessToken, OIDCAuthorization, OIDCClaim, OIDCCode
 from authentic2_idp_oidc.utils import get_jwkset, make_pairwise_sub, make_sub, reverse_pairwise_sub
 
 from .. import utils
@@ -471,3 +471,67 @@ def test_do_no_ask_again(app, oidc_client, profile_user, profile_settings):
     )
     response = app.get(authorize_url)
     assert 'do_not_ask_again' not in response.text
+
+
+def test_scopes_minimization(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.scope = 'openid profile email custom1 custom2 custom3'
+    oidc_client.save()
+    redirect_uri = oidc_client.redirect_uris.split()[0]
+
+    oidcclaim_mapping = {
+        'custom1': 'first_name',
+        'custom2': 'last_name',
+        'custom3': 'is_active',
+    }
+    for claim, value in oidcclaim_mapping.items():
+        OIDCClaim.objects.create(
+            client=oidc_client,
+            name=claim,
+            scopes=claim,
+            value=value,
+        )
+
+    params = {
+        'client_id': oidc_client.client_id,
+        'scope': 'openid profile email custom1 custom2 custom3',
+        'redirect_uri': redirect_uri,
+        'state': 'xxx',
+        'nonce': 'yyy',
+        'login_hint': 'backoffice john@example.com',
+        'response_type': 'code',
+    }
+
+    assert not OIDCCode.objects.count()
+    assert not OIDCAuthorization.objects.count()
+
+    authorize_url = make_url('oidc-authorize', params=params)
+    utils.login(app, profile_user)
+    response = app.get(authorize_url)
+    response.form.set('profile-validation', profile_user.profiles.first().id)
+    response = response.form.submit('accept')
+    location = urllib.parse.urlparse(response['Location'])
+    query = urllib.parse.parse_qs(location.query)
+    code = query['code'][0]
+
+    token_url = make_url('oidc-token')
+    response = app.post(
+        token_url,
+        params={
+            'grant_type': 'authorization_code',
+            'code': code,
+            'redirect_uri': oidc_client.redirect_uris.split()[0],
+        },
+        headers=client_authentication_headers(oidc_client),
+    )
+    assert response.json['access_token']
+    id_token = response.json['id_token']
+    k = base64.b64encode(oidc_client.client_secret.encode('utf-8'))
+    key = JWK(kty='oct', k=force_text(k))
+    algs = ['HS256']
+    jwt = JWT(jwt=id_token, key=key, algs=algs)
+    claims = json.loads(jwt.claims)
+
+    assert not set(oidcclaim_mapping.keys()) & set(claims.keys())
-- 
2.35.1

