From b0b8fb7d5c1459357ce85ce345909b10d793393d Mon Sep 17 00:00:00 2001 From: Josue Kouka Date: Wed, 8 Aug 2018 17:17:26 +0200 Subject: [PATCH] idp oidc: set user identifier as preferred username claim (#23900) --- .../attributes_ng/sources/django_user.py | 2 +- src/authentic2_idp_oidc/admin.py | 2 +- .../migrations/0011_auto_20180808_1546.py | 39 ++++++++++++++ tests/test_idp_oidc.py | 54 +++++++++++++++++-- 4 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 src/authentic2_idp_oidc/migrations/0011_auto_20180808_1546.py diff --git a/src/authentic2/attributes_ng/sources/django_user.py b/src/authentic2/attributes_ng/sources/django_user.py index e1083d30..4a075c54 100644 --- a/src/authentic2/attributes_ng/sources/django_user.py +++ b/src/authentic2/attributes_ng/sources/django_user.py @@ -76,7 +76,7 @@ def get_attributes(instance, ctx): if user.username: splitted = user.username.rsplit('@', 1) ctx['django_user_domain'] = splitted[1] if '@' in user.username else '' - ctx['django_user_identifier'] = splitted[0] if '@' in user.username else '' + ctx['django_user_identifier'] = splitted[0] ctx['django_user_full_name'] = user.get_full_name() Role = get_role_model() roles = Role.objects.for_user(user) diff --git a/src/authentic2_idp_oidc/admin.py b/src/authentic2_idp_oidc/admin.py index aee197cd..924299d6 100644 --- a/src/authentic2_idp_oidc/admin.py +++ b/src/authentic2_idp_oidc/admin.py @@ -32,7 +32,7 @@ class OIDCClaimInlineAdmin(admin.TabularInline): # values on the GET (display of the creation form) if request.method == 'GET' and not obj: initial.extend([ - {'name': 'preferred_username', 'value': 'django_user_username', 'scopes': 'profile'}, + {'name': 'preferred_username', 'value': 'django_user_identifier', 'scopes': 'profile'}, {'name': 'given_name', 'value': 'django_user_first_name', 'scopes': 'profile'}, {'name': 'family_name', 'value': 'django_user_last_name', 'scopes': 'profile'}, {'name': 'email', 'value': 'django_user_email', 'scopes': 'email'}, diff --git a/src/authentic2_idp_oidc/migrations/0011_auto_20180808_1546.py b/src/authentic2_idp_oidc/migrations/0011_auto_20180808_1546.py new file mode 100644 index 00000000..bbf1820e --- /dev/null +++ b/src/authentic2_idp_oidc/migrations/0011_auto_20180808_1546.py @@ -0,0 +1,39 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations + + +OLD_DEFAULT_CLAIMS_MAPPING = { + 'email': 'django_user_email', 'email_verified': 'django_user_email_verified', + 'family_name': 'django_user_last_name', 'given_name': 'django_user_first_name', + 'preferred_username': 'django_user_username'} + + +def set_oidcclient_default_preferred_username_as_identifier(apps, schema_editor): + OIDCClient = apps.get_model('authentic2_idp_oidc', 'OIDCClient') + OIDCClaim = apps.get_model('authentic2_idp_oidc', 'OIDCClaim') + for oidcclient in OIDCClient.objects.all(): + claims = oidcclient.oidcclaim_set.values_list('name', 'value') + # check if default config + if set(OLD_DEFAULT_CLAIMS_MAPPING.items()).symmetric_difference(claims): + continue + pref_username_claim = OIDCClaim.objects.get(name='preferred_username', client=oidcclient) + if pref_username_claim.value != 'django_user_identifier': + pref_username_claim.value = 'django_user_identifier' + pref_username_claim.save() + + +def unset_oidcclient_default_preferred_username_as_identifier(apps, schema_editor): + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ('authentic2_idp_oidc', '0010_oidcclaim'), + ] + + operations = [ + migrations.RunPython(set_oidcclient_default_preferred_username_as_identifier, unset_oidcclient_default_preferred_username_as_identifier) + ] diff --git a/tests/test_idp_oidc.py b/tests/test_idp_oidc.py index 2f77cde9..6a14525b 100644 --- a/tests/test_idp_oidc.py +++ b/tests/test_idp_oidc.py @@ -11,9 +11,12 @@ from jwcrypto.jwk import JWKSet, JWK import utils from django.core.urlresolvers import reverse +from django.db import connection +from django.db.migrations.executor import MigrationExecutor from django.utils.timezone import now from django.contrib.auth import get_user_model + User = get_user_model() from authentic2_idp_oidc.models import OIDCClient, OIDCAuthorization, OIDCCode, OIDCAccessToken, OIDCClaim @@ -856,9 +859,6 @@ def test_registration_service_slug(oidc_settings, app, simple_oidc_client, simpl def test_oidclient_claims_data_migration(): - from django.db import connection - from django.db.migrations.executor import MigrationExecutor - executor = MigrationExecutor(connection) app = 'authentic2_idp_oidc' migrate_from = [(app, '0009_auto_20180313_1156')] @@ -877,6 +877,54 @@ def test_oidclient_claims_data_migration(): assert OIDCClaim.objects.filter(client=client.id).count() == 5 +def test_oidclient_preferred_username_as_identifier_data_migration(): + executor = MigrationExecutor(connection) + app = 'authentic2_idp_oidc' + migrate_from = [(app, '0010_oidcclaim')] + migrate_to = [(app, '0011_auto_20180808_1546')] + executor.migrate(migrate_from) + executor.loader.build_graph() + old_apps = executor.loader.project_state(migrate_from).apps + OIDCClient = old_apps.get_model('authentic2_idp_oidc', 'OIDCClient') + OIDCClaim = old_apps.get_model('authentic2_idp_oidc', 'OIDCClaim') + client1 = OIDCClient.objects.create(name='test', slug='test', redirect_uris='https://example.net/') + client2 = OIDCClient.objects.create(name='test1', slug='test1', redirect_uris='https://example.net/') + client3 = OIDCClient.objects.create(name='test2', slug='test2', redirect_uris='https://example.net/') + client4 = OIDCClient.objects.create(name='test3', slug='test3', redirect_uris='https://example.net/') + for client in (client1, client2, client3, client4): + if client.name == 'test1': + continue + if client.name == 'test3': + OIDCClaim.objects.create(client=client, name='preferred_username', value='django_user_full_name', scopes='profile') + else: + OIDCClaim.objects.create(client=client, name='preferred_username', value='django_user_username', scopes='profile') + OIDCClaim.objects.create(client=client, name='given_name', value='django_user_first_name', scopes='profile') + OIDCClaim.objects.create(client=client, name='family_name', value='django_user_last_name', scopes='profile') + if client.name == 'test2': + continue + OIDCClaim.objects.create(client=client, name='email', value='django_user_email', scopes='email') + OIDCClaim.objects.create(client=client, name='email_verified', value='django_user_email_verified', scopes='email') + executor.migrate(migrate_to) + executor.loader.build_graph() + client = OIDCClient.objects.first() + for client in OIDCClient.objects.all(): + claims = client.oidcclaim_set.all() + if client.name == 'test': + assert claims.count() == 5 + assert sorted(claims.values_list('name', flat=True)) == [u'email', u'email_verified', u'family_name', u'given_name', u'preferred_username'] + assert sorted(claims.values_list('value', flat=True)) == [u'django_user_email', u'django_user_email_verified', u'django_user_first_name', u'django_user_identifier', u'django_user_last_name'] + elif client.name == 'test2': + assert claims.count() == 3 + assert sorted(claims.values_list('name', flat=True)) == [u'family_name', u'given_name', u'preferred_username'] + assert sorted(claims.values_list('value', flat=True)) == [u'django_user_first_name', u'django_user_last_name', u'django_user_username'] + elif client.name == 'test3': + assert claims.count() == 5 + assert sorted(claims.values_list('name', flat=True)) == [u'email', u'email_verified', u'family_name', u'given_name', u'preferred_username'] + assert sorted(claims.values_list('value', flat=True)) == [u'django_user_email', u'django_user_email_verified', u'django_user_first_name', u'django_user_full_name', u'django_user_last_name'] + else: + assert claims.count() == 0 + + def test_api_synchronization(app, oidc_client): oidc_client.has_api_access = True oidc_client.save() -- 2.18.0