Project

General

Profile

0001-auth_fc-make-user-and-sub-relatively-unique-19959.patch

Benjamin Dauvergne, 30 Oct 2019 09:00 PM

Download (24 KB)

View differences:

Subject: [PATCH] auth_fc: make user and sub relatively unique (#19959)

As we are not sure they are unique in all deployments, we make them
unique relative to a new order integer field. New federations with
FranceConnect with be forced to have the column order to be 0, making
them unique.
 src/authentic2_auth_fc/backends.py            | 39 ++++++---
 .../migrations/0002_fcaccount_order1.py       | 59 +++++++++++++
 .../migrations/0003_fcaccount_order2.py       | 26 ++++++
 src/authentic2_auth_fc/models.py              |  9 ++
 src/authentic2_auth_fc/views.py               | 86 +++++++++----------
 tests/auth_fc/test_auth_fc.py                 | 28 ++++++
 tests/conftest.py                             | 53 ++++++++++++
 tests/test_idp_oidc.py                        | 29 +++----
 8 files changed, 259 insertions(+), 70 deletions(-)
 create mode 100644 src/authentic2_auth_fc/migrations/0002_fcaccount_order1.py
 create mode 100644 src/authentic2_auth_fc/migrations/0003_fcaccount_order2.py
src/authentic2_auth_fc/backends.py
19 19

  
20 20
from django.contrib.auth import get_user_model
21 21
from django.contrib.auth.backends import ModelBackend
22
from django.core.exceptions import PermissionDenied, MultipleObjectsReturned
23
from django.db import IntegrityError
22 24

  
23 25
from authentic2.a2_rbac.utils import get_default_ou
24 26
from authentic2 import hooks
......
33 35
        user_info = kwargs.get('user_info')
34 36
        user = None
35 37
        try:
36
            fc_account = models.FcAccount.objects.get(sub=sub, user__is_active=True)
37
            logger.debug(u'existing user %s using sub %s', fc_account.user, sub)
38
            user = fc_account.user
38
            try:
39
                account = models.FcAccount.objects.select_related().get(sub=sub)
40
            except MultipleObjectsReturned:
41
                account = models.FcAccount.objects.select_related().get(sub=sub, order=0)
39 42
        except models.FcAccount.DoesNotExist:
40 43
            logger.debug(u'user with the sub %s does not exist.', sub)
44
        else:
45
            user = account.user
46
            logger.debug(u'found user %s with sub %s', user, sub)
47
            if not user.is_active:
48
                logger.info(u'user %s login refused, it is inactive', user)
49
                raise PermissionDenied
41 50
        if user_info:
42 51
            if not user and app_settings.create:
43 52
                User = get_user_model()
44 53
                user = User.objects.create(ou=get_default_ou())
45
                fc_account = models.FcAccount.objects.create(
46
                    user=user,
47
                    sub=sub,
48
                    token=json.dumps(kwargs['token']))
49
                logger.debug(u'user creation enabled with fc_account (sub : %s - token : %s)',
50
                             sub, json.dumps(kwargs['token']))
51
                hooks.call_hooks('event', name='fc-create', user=user, sub=sub)
54
                try:
55
                    models.FcAccount.objects.create(
56
                        user=user,
57
                        sub=sub,
58
                        order=0,
59
                        token=json.dumps(kwargs['token']))
60
                except IntegrityError:
61
                    # uniqueness check failed, as the user is new, it can only means that the sub is not unique
62
                    # let's try again
63
                    user.delete()
64
                    return self.authenticate(sub, **kwargs)
65
                else:
66
                    logger.debug(u'user creation enabled with fc_account (sub : %s - token : %s)',
67
                                 sub, json.dumps(kwargs['token']))
68
                    hooks.call_hooks('event', name='fc-create', user=user, sub=sub)
69

  
52 70
            if not user:
53 71
                return None
72

  
54 73
            logger.debug(u'updated (given_name : %s - family_name : %s)', user_info['given_name'],
55 74
                         user_info['family_name'])
56 75
            user.first_name = user_info['given_name']
src/authentic2_auth_fc/migrations/0002_fcaccount_order1.py
1
# -*- coding: utf-8 -*-
2
# Generated by Django 1.11.20 on 2019-06-12 21:27
3
from __future__ import unicode_literals
4

  
5
from collections import Counter
6

  
7
from django.db import migrations, models
8

  
9

  
10
def set_fcaccount_order(apps, schema_editor):
11
    FcAccount = apps.get_model('authentic2_auth_fc', 'FcAccount')
12
    c = Counter()
13
    FcAccount.objects.update(order=0)
14
    user_ids = (
15
        FcAccount.objects
16
        .values('user_id')
17
        .annotate(total=models.Count('id'))
18
        .filter(total__gt=1)
19
        .values_list('user_id', flat=True)
20
    )
21
    subs = (
22
        FcAccount.objects
23
        .values_list('sub')
24
        .annotate(total=models.Count('id'))
25
        .filter(total__gt=1)
26
        .values_list('sub', flat=True)
27
    )
28

  
29
    for account in FcAccount.objects.filter(models.Q(user_id__in=user_ids) | models.Q(sub__in=subs)):
30
        order = max(c[account.user_id], c[account.sub])
31
        if account.order != order:
32
            account.order = order
33
            account.save()
34
        c[account.user_id] = order + 1
35
        c[account.sub] = order + 1
36
    assert FcAccount.objects.filter(order__isnull=True).count() == 0
37

  
38

  
39
def noop(apps, schema_editor):
40
    pass
41

  
42

  
43
class Migration(migrations.Migration):
44
    # Disembiguate sub and user_id columns using an integer order column, then
45
    # force sub/order and user_id/order pairs unique to be unique in order
46
    # to move to a model where user and sub are unique without breaking on existing data
47

  
48
    dependencies = [
49
        ('authentic2_auth_fc', '0001_initial'),
50
    ]
51

  
52
    operations = [
53
        migrations.AddField(
54
            model_name='fcaccount',
55
            name='order',
56
            field=models.IntegerField(null=True, verbose_name='order'),
57
        ),
58
        migrations.RunPython(set_fcaccount_order, noop),
59
    ]
src/authentic2_auth_fc/migrations/0003_fcaccount_order2.py
1
# -*- coding: utf-8 -*-
2
# Generated by Django 1.11.20 on 2019-06-13 09:44
3
from __future__ import unicode_literals
4

  
5
from django.conf import settings
6
from django.db import migrations, models
7

  
8

  
9
class Migration(migrations.Migration):
10

  
11
    dependencies = [
12
        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
13
        ('authentic2_auth_fc', '0002_fcaccount_order1'),
14
    ]
15

  
16
    operations = [
17
        migrations.AlterField(
18
            model_name='fcaccount',
19
            name='order',
20
            field=models.PositiveIntegerField(default=0, verbose_name='order'),
21
        ),
22
        migrations.AlterUniqueTogether(
23
            name='fcaccount',
24
            unique_together=set([('sub', 'order'), ('user', 'order')]),
25
        ),
26
    ]
src/authentic2_auth_fc/models.py
87 87
    sub = models.TextField(
88 88
        verbose_name=_('sub'),
89 89
        db_index=True)
90
    order = models.PositiveIntegerField(
91
        verbose_name=_('order'),
92
        default=0)
90 93
    token = models.TextField(verbose_name=_('access token'))
91 94
    user_info = models.TextField(verbose_name=_('access token'), blank=True, null=True)
92 95

  
......
108 111
        if 'family_name' in user_info:
109 112
            display_name.append(user_info['family_name'])
110 113
        return ' '.join(display_name)
114

  
115
    class Meta:
116
        unique_together = [
117
            ('sub', 'order'),
118
            ('user', 'order'),
119
        ]
src/authentic2_auth_fc/views.py
24 24

  
25 25

  
26 26
import django
27
from django.db import IntegrityError
27 28
from django.views.generic import View, FormView
28 29
from django.http import HttpResponseRedirect, Http404
29 30
from django.contrib.auth import REDIRECT_FIELD_NAME, get_user_model
......
163 164
    redirect_field_name = REDIRECT_FIELD_NAME
164 165
    in_popup = False
165 166
    token = None
167
    user_info = None
166 168

  
167 169
    def get_in_popup(self):
168 170
        return self.in_popup
......
332 334
                self.scopes.extend(scopes)
333 335
            return ask_authorization(request, self.get_scopes(), self.logger)
334 336

  
337
    @property
338
    def fc_display_name(self):
339
        '''Human representation of the current FC account'''
340
        if not self.user_info:
341
            return u''
342
        return u'{0} {1}'.format(self.user_info['family_name'], self.user_info['given_name'])
343

  
335 344

  
336 345
class PopupViewMixin(object):
337 346
    def get_in_popup(self):
......
345 354
    '''
346 355

  
347 356
    def update_user_info(self):
357
        self.fc_account.token = json.dumps(self.token)
348 358
        self.fc_account.user_info = json.dumps(self.user_info)
349
        self.fc_account.save()
359
        self.fc_account.save(update_fields=['token', 'user_info'])
350 360
        utils.apply_user_info_mappings(self.fc_account.user, self.user_info)
351 361
        self.logger.debug('updating user_info %s', self.fc_account.user_info)
352 362

  
363
    def uniqueness_check_failed(self, request):
364
        if models.FcAccount.objects.filter(user=request.user, order=0).count():
365
            messages.error(request,
366
                           _('Your account is already linked to a FranceConnect account'))
367
        else:
368
            messages.error(request,
369
                           _('The FranceConnect account {} is already'
370
                             ' linked with another account.').format(self.fc_display_name))
371
        return self.redirect(request)
372

  
353 373
    def get(self, request, *args, **kwargs):
354 374
        registration = True if 'registration' in request.GET else False
355 375
        '''Request an access grant code and associate it to the current user'''
356 376
        self.service_slug = request.GET.get(constants.SERVICE_FIELD_NAME)
357 377
        if request.user.is_authenticated():
358
            # Prevent to add a link with an FC account already linked with another user.
359 378
            try:
360
                fc_account = models.FcAccount.objects.get(sub=self.sub, user__is_active=True)
361
                if fc_account.user is not request.user:
362
                    msg = 'Attempt to link FC account {} already linked with user {}'
363
                    self.logger.info(msg.format(self.sub, fc_account.user))
364
                    messages.error(request,
365
                                   _('The FranceConnect account {} is already'
366
                                     ' linked with another account.').format(fc_account))
367
                    return self.redirect(request)
368
            except models.FcAccount.DoesNotExist:
369
                pass
370

  
371
            # Prevent to add a link to an user which is already linked to an FC account
372
            if request.user.fc_accounts.exists():
373
                self.logger.warning(u'cannot link to sub %s, account is already linked to an '
374
                                    u'FC account', self.sub)
375
                messages.error(request,
376
                               _('Your account is already linked to a FranceConnect account'))
377
                return self.redirect(request)
379
                self.fc_account, created = models.FcAccount.objects.get_or_create(
380
                    sub=self.sub, user=request.user, order=0,
381
                    defaults={'token': json.dumps(self.token)})
382
            except IntegrityError:
383
                # unique index check failed, find why.
384
                return self.uniqueness_check_failed(request)
378 385

  
379
            json_token = json.dumps(self.token)
380
            self.fc_account, created = models.FcAccount.objects.get_or_create(
381
                defaults={'token': json_token},
382
                sub=self.sub, user=request.user)
383 386
            if created:
384 387
                self.logger.info('fc link created sub %s', self.sub)
385
                self.update_user_info()
386
                data = utils.get_mapped_attributes_flat(request)
387
                if 'email' in data:
388
                    messages.info(request,
389
                                  _('Your FranceConnect account {} with '
390
                                    'email {} has been linked.').format(self.fc_account,
391
                                                                        data['email']))
392
                else:
393
                    messages.info(request, _('Your FranceConnect account {} '
394
                                             'has been linked.').format(self.fc_account))
395
                hooks.call_hooks('event', name='fc-link', user=request.user, sub=self.sub,
396
                                 request=request)
388
                messages.info(request,
389
                              _('Your FranceConnect account {} has been linked.').format(self.fc_display_name))
390
                hooks.call_hooks('event', name='fc-link', user=request.user, sub=self.sub, request=request)
397 391
            else:
398
                self.fc_account.token = json_token
399
                self.fc_account.save()
400
                self.update_user_info()
401 392
                messages.info(request, _('Your local account has been updated.'))
393
            self.update_user_info()
402 394
            return self.redirect(request)
403 395

  
404 396
        default_ou = get_default_ou()
......
408 400
            sub=self.sub,
409 401
            user_info=self.user_info,
410 402
            token=self.token)
403
        if user:
404
            self.fc_account = user.fc_accounts.get(order=0)
411 405
        if not user and self.user_info.get('email') and email_is_unique:
412 406
            email = self.user_info['email']
413 407
            User = get_user_model()
......
425 419
                    user = qs[0]
426 420
                    # but does he have already a link to an FC account ?
427 421
                    if not user.fc_accounts.exists():
428
                        fc_account, created = models.FcAccount.objects.get_or_create(
429
                            defaults={'token': json.dumps(self.token)},
430
                            sub=self.sub, user=user)
422
                        try:
423
                            self.fc_account, created = models.FcAccount.objects.get_or_create(
424
                                defaults={'token': json.dumps(self.token)},
425
                                sub=self.sub, user=user, order=0)
426
                        except IntegrityError:
427
                            # unique index check failed, find why.
428
                            return self.uniqueness_check_failed(request)
429

  
431 430
                        if created:
432 431
                            self.logger.info(u'fc link created sub %s user %s', self.sub, user)
433 432
                            hooks.call_hooks('event', name='fc-link', user=user, sub=self.sub,
......
447 446
                        return self.redirect(request)
448 447
        if user:
449 448
            a2_utils.login(request, user, 'france-connect', service_slug=self.service_slug)
450
            self.fc_account = models.FcAccount.objects.get(sub=self.sub, user=user)
451
            self.fc_account.token = json.dumps(self.token)
452
            self.fc_account.save(update_fields=['token'])
453 449
            self.update_user_info()
454 450
            self.logger.info('logged in using fc sub %s', self.sub)
455 451
            return self.redirect(request)
tests/auth_fc/test_auth_fc.py
589 589
    assert path(response['Location']) == '/accounts/'
590 590
    response = response.follow()
591 591
    assert len(response.pyquery('[href*="password/change"]')) > 0
592

  
593

  
594
def test_migration_0002_0003(migration):
595
    migrate_from = [('authentic2_auth_fc', '0001_initial')]
596
    # it's a two-parts migration, as it contains data and schema changes.
597
    migrate_to = [('authentic2_auth_fc', '0003_fcaccount_order2')]
598

  
599
    old_apps = migration.before(migrate_from, at_end=False)
600

  
601
    User = old_apps.get_model('custom_user', 'User')
602
    FcAccount = old_apps.get_model('authentic2_auth_fc', 'FcAccount')
603
    user1 = User.objects.create(username='user1')
604
    user2 = User.objects.create(username='user2')
605
    user3 = User.objects.create(username='user3')
606
    FcAccount.objects.create(user=user1, sub='sub1')
607
    FcAccount.objects.create(user=user1, sub='sub2')
608
    FcAccount.objects.create(user=user2, sub='sub2')
609
    FcAccount.objects.create(user=user2, sub='sub3')
610
    FcAccount.objects.create(user=user3, sub='sub3')
611
    assert len(set(FcAccount.objects.values_list('user_id', flat=True))) == 3
612
    assert len(set(FcAccount.objects.values_list('sub', flat=True))) == 3
613
    assert FcAccount.objects.count() == 5
614

  
615
    # execute migration
616
    new_apps = migration.apply(migrate_to)
617
    FcAccount = new_apps.get_model('authentic2_auth_fc', 'FcAccount')
618
    assert len(set(FcAccount.objects.values_list('user_id', 'order'))) == 5
619
    assert len(set(FcAccount.objects.values_list('sub', 'order'))) == 5
tests/conftest.py
21 21

  
22 22
import django_webtest
23 23

  
24
import django
24 25
from django.contrib.auth import get_user_model
25 26
from django_rbac.utils import get_ou_model, get_role_model
26 27
from django.conf import settings
27 28
from django.utils.six.moves.urllib import parse as urlparse
29
from django.db.migrations.executor import MigrationExecutor
30
from django.db import connection
31
from django.core.management import call_command
28 32

  
29 33
from pytest_django.migrations import DisableMigrations
30 34

  
......
384 388
        ou=get_default_ou(),
385 389
        slug='service',
386 390
        name='Service')
391

  
392

  
393
@pytest.fixture()
394
def migration(request, transactional_db):
395
    # see https://gist.github.com/asfaltboy/b3e6f9b5d95af8ba2cc46f2ba6eae5e2
396
    if django.VERSION < (1, 9):
397
        pytest.skip('migration fixture only works with Django 1.9')
398
    """
399
    This fixture returns a helper object to test Django data migrations.
400
    The fixture returns an object with two methods;
401
     - `before` to initialize db to the state before the migration under test
402
     - `after` to execute the migration and bring db to the state after the migration
403
    The methods return `old_apps` and `new_apps` respectively; these can
404
    be used to initiate the ORM models as in the migrations themselves.
405
    For example:
406
        def test_foo_set_to_bar(migration):
407
            old_apps = migration.before([('my_app', '0001_inital')])
408
            Foo = old_apps.get_model('my_app', 'foo')
409
            Foo.objects.create(bar=False)
410
            assert Foo.objects.count() == 1
411
            assert Foo.objects.filter(bar=False).count() == Foo.objects.count()
412
            # executing migration
413
            new_apps = migration.apply([('my_app', '0002_set_foo_bar')])
414
            Foo = new_apps.get_model('my_app', 'foo')
415

  
416
            assert Foo.objects.filter(bar=False).count() == 0
417
            assert Foo.objects.filter(bar=True).count() == Foo.objects.count()
418
    Based on: https://gist.github.com/blueyed/4fb0a807104551f103e6
419
    """
420
    class Migrator(object):
421
        def before(self, targets, at_end=True):
422
            """ Specify app and starting migration names as in:
423
                before([('app', '0001_before')]) => app/migrations/0001_before.py
424
            """
425
            executor = MigrationExecutor(connection)
426
            executor.migrate(targets)
427
            executor.loader.build_graph()
428
            return executor._create_project_state(with_applied_migrations=True).apps
429

  
430
        def apply(self, targets):
431
            """ Migrate forwards to the "targets" migration """
432
            executor = MigrationExecutor(connection)
433
            executor.migrate(targets)
434
            executor.loader.build_graph()
435
            return executor._create_project_state(with_applied_migrations=True).apps
436

  
437
    yield Migrator()
438

  
439
    call_command('migrate', verbosity=0)
tests/test_idp_oidc.py
28 28
from django.core.urlresolvers import reverse
29 29
from django.core.files import File
30 30
from django.db import connection
31
from django.db.migrations.executor import MigrationExecutor
32 31
from django.utils.timezone import now
33 32
from django.contrib.auth import get_user_model
34 33
from django.utils.six.moves.urllib import parse as urlparse
......
918 917
    assert hooks.event[2]['kwargs']['service'] == 'client'
919 918

  
920 919

  
921
def test_oidclient_claims_data_migration():
922
    executor = MigrationExecutor(connection)
920
def test_oidclient_claims_data_migration(migration):
923 921
    app = 'authentic2_idp_oidc'
924 922
    migrate_from = [(app, '0009_auto_20180313_1156')]
925 923
    migrate_to = [(app, '0010_oidcclaim')]
926
    executor.migrate(migrate_from)
927
    executor.loader.build_graph()
928 924

  
929
    old_apps = executor.loader.project_state(migrate_from).apps
925
    old_apps = migration.before(migrate_from)
930 926
    OIDCClient = old_apps.get_model('authentic2_idp_oidc', 'OIDCClient')
927

  
931 928
    client = OIDCClient(name='test', slug='test', redirect_uris='https://example.net/')
932 929
    client.save()
933 930

  
934
    executor.migrate(migrate_to)
935
    executor.loader.build_graph()
931
    new_apps = migration.apply(migrate_to)
932
    OIDCClient = new_apps.get_model('authentic2_idp_oidc', 'OIDCClient')
933

  
936 934
    client = OIDCClient.objects.first()
937 935
    assert OIDCClaim.objects.filter(client=client.id).count() == 5
938 936

  
939 937

  
940
def test_oidclient_preferred_username_as_identifier_data_migration():
941
    executor = MigrationExecutor(connection)
938
def test_oidclient_preferred_username_as_identifier_data_migration(migration):
942 939
    app = 'authentic2_idp_oidc'
943 940
    migrate_from = [(app, '0010_oidcclaim')]
944 941
    migrate_to = [(app, '0011_auto_20180808_1546')]
945
    executor.migrate(migrate_from)
946
    executor.loader.build_graph()
947
    old_apps = executor.loader.project_state(migrate_from).apps
942

  
943
    old_apps = migration.before(migrate_from)
948 944
    OIDCClient = old_apps.get_model('authentic2_idp_oidc', 'OIDCClient')
949 945
    OIDCClaim = old_apps.get_model('authentic2_idp_oidc', 'OIDCClaim')
946

  
950 947
    client1 = OIDCClient.objects.create(name='test', slug='test', redirect_uris='https://example.net/')
951 948
    client2 = OIDCClient.objects.create(name='test1', slug='test1', redirect_uris='https://example.net/')
952 949
    client3 = OIDCClient.objects.create(name='test2', slug='test2', redirect_uris='https://example.net/')
......
964 961
            continue
965 962
        OIDCClaim.objects.create(client=client, name='email', value='django_user_email', scopes='email')
966 963
        OIDCClaim.objects.create(client=client, name='email_verified', value='django_user_email_verified', scopes='email')
967
    executor.migrate(migrate_to)
968
    executor.loader.build_graph()
964

  
965
    new_apps = migration.apply(migrate_to)
966
    OIDCClient = new_apps.get_model('authentic2_idp_oidc', 'OIDCClient')
967

  
969 968
    client = OIDCClient.objects.first()
970 969
    for client in OIDCClient.objects.all():
971 970
        claims = client.oidcclaim_set.all()
972
-