Project

General

Profile

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

Benjamin Dauvergne, 14 Jun 2019 04:00 PM

Download (20.8 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                             | 55 ++++++++++++
 7 files changed, 247 insertions(+), 55 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
......
162 163
    redirect_field_name = REDIRECT_FIELD_NAME
163 164
    in_popup = False
164 165
    token = None
166
    user_info = None
165 167

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

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

  
334 343

  
335 344
class PopupViewMixin(object):
336 345
    def get_in_popup(self):
......
344 353
    '''
345 354

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

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

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

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

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

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

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

  
592

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

  
598
    old_apps = migration.before(migrate_from)
599

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

  
614
    # execute migration
615
    new_apps = migration.apply(migrate_to)
616
    FcAccount = new_apps.get_model('authentic2_auth_fc', 'FcAccount')
617
    assert len(set(FcAccount.objects.values_list('user_id', 'order'))) == 5
618
    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

  
......
360 364
@pytest.fixture
361 365
def media(settings, tmpdir):
362 366
    settings.MEDIA_ROOT = str(tmpdir.mkdir('media'))
367

  
368

  
369
@pytest.fixture()
370
def migration(request, transactional_db):
371
    # see https://gist.github.com/asfaltboy/b3e6f9b5d95af8ba2cc46f2ba6eae5e2
372
    if django.VERSION < (1, 9):
373
        pytest.skip('migration fixture only works with Django 1.9')
374
    """
375
    This fixture returns a helper object to test Django data migrations.
376
    The fixture returns an object with two methods;
377
     - `before` to initialize db to the state before the migration under test
378
     - `after` to execute the migration and bring db to the state after the migration
379
    The methods return `old_apps` and `new_apps` respectively; these can
380
    be used to initiate the ORM models as in the migrations themselves.
381
    For example:
382
        def test_foo_set_to_bar(migration):
383
            old_apps = migration.before([('my_app', '0001_inital')])
384
            Foo = old_apps.get_model('my_app', 'foo')
385
            Foo.objects.create(bar=False)
386
            assert Foo.objects.count() == 1
387
            assert Foo.objects.filter(bar=False).count() == Foo.objects.count()
388
            # executing migration
389
            new_apps = migration.apply([('my_app', '0002_set_foo_bar')])
390
            Foo = new_apps.get_model('my_app', 'foo')
391

  
392
            assert Foo.objects.filter(bar=False).count() == 0
393
            assert Foo.objects.filter(bar=True).count() == Foo.objects.count()
394
    Based on: https://gist.github.com/blueyed/4fb0a807104551f103e6
395
    """
396
    class Migrator(object):
397
        def before(self, targets):
398
            """ Specify app and starting migration names as in:
399
                before([('app', '0001_before')]) => app/migrations/0001_before.py
400
            """
401
            self.executor = MigrationExecutor(connection)
402
            # prepare state of db to before the migration ("migrate_from" state)
403
            self._old_apps = self.executor.migrate(targets).apps
404
            return self._old_apps
405

  
406
        def apply(self, targets):
407
            """ Migrate forwards to the "targets" migration """
408
            self.executor.loader.build_graph()  # reload.
409
            self._new_apps = self.executor.migrate(targets).apps
410
            return self._new_apps
411

  
412
    # ensure to migrate forward migrated apps all the way after test
413
    def migrate_to_end():
414
        call_command('migrate', verbosity=0)
415
    request.addfinalizer(migrate_to_end)
416

  
417
    return Migrator()
363
-