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.
This commit is contained in:
parent
466604122e
commit
b439593203
|
|
@ -33,9 +33,8 @@ class FcAuthenticator(BaseAuthenticator):
|
|||
def name(self):
|
||||
return gettext_noop('FranceConnect')
|
||||
|
||||
@property
|
||||
def popup(self):
|
||||
return app_settings.popup
|
||||
def id(self):
|
||||
return 'fc'
|
||||
|
||||
def login(self, request, *args, **kwargs):
|
||||
if 'nofc' in request.GET:
|
||||
|
|
@ -43,10 +42,10 @@ class FcAuthenticator(BaseAuthenticator):
|
|||
fc_user_info = request.session.get('fc_user_info')
|
||||
context = kwargs.pop('context', {}).copy()
|
||||
params = {}
|
||||
if self.popup:
|
||||
if app_settings.popup:
|
||||
params['popup'] = ''
|
||||
context.update({
|
||||
'popup': self.popup,
|
||||
'popup': app_settings.popup,
|
||||
'about_url': app_settings.about_url,
|
||||
'fc_user_info': fc_user_info,
|
||||
})
|
||||
|
|
@ -79,14 +78,14 @@ class FcAuthenticator(BaseAuthenticator):
|
|||
params = {
|
||||
'next': account_path,
|
||||
}
|
||||
if self.popup:
|
||||
if app_settings.popup:
|
||||
params['popup'] = ''
|
||||
link_url = a2_utils.make_url('fc-login-or-link',
|
||||
params=params)
|
||||
|
||||
context = kwargs.pop('context', {}).copy()
|
||||
context.update({
|
||||
'popup': self.popup,
|
||||
'popup': app_settings.popup,
|
||||
'unlink': unlink,
|
||||
'about_url': app_settings.about_url,
|
||||
'link_url': link_url,
|
||||
|
|
@ -101,13 +100,13 @@ class FcAuthenticator(BaseAuthenticator):
|
|||
params = {
|
||||
'registration': '',
|
||||
}
|
||||
if self.popup:
|
||||
if app_settings.popup:
|
||||
params['popup'] = ''
|
||||
context.update({
|
||||
'login_url': a2_utils.make_url('fc-login-or-link',
|
||||
keep_params=True, params=params,
|
||||
request=request),
|
||||
'popup': self.popup,
|
||||
'popup': app_settings.popup,
|
||||
'about_url': app_settings.about_url,
|
||||
})
|
||||
return TemplateResponse(request, 'authentic2_auth_fc/registration.html', context)
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ import logging
|
|||
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.contrib.auth.backends import ModelBackend
|
||||
from django.core.exceptions import PermissionDenied, MultipleObjectsReturned
|
||||
from django.db import IntegrityError
|
||||
|
||||
from authentic2.a2_rbac.utils import get_default_ou
|
||||
from authentic2 import hooks
|
||||
|
|
@ -33,24 +35,41 @@ class FcBackend(ModelBackend):
|
|||
user_info = kwargs.get('user_info')
|
||||
user = None
|
||||
try:
|
||||
fc_account = models.FcAccount.objects.get(sub=sub, user__is_active=True)
|
||||
logger.debug(u'existing user %s using sub %s', fc_account.user, sub)
|
||||
user = fc_account.user
|
||||
try:
|
||||
account = models.FcAccount.objects.select_related().get(sub=sub)
|
||||
except MultipleObjectsReturned:
|
||||
account = models.FcAccount.objects.select_related().get(sub=sub, order=0)
|
||||
except models.FcAccount.DoesNotExist:
|
||||
logger.debug(u'user with the sub %s does not exist.', sub)
|
||||
else:
|
||||
user = account.user
|
||||
logger.debug(u'found user %s with sub %s', user, sub)
|
||||
if not user.is_active:
|
||||
logger.info(u'user %s login refused, it is inactive', user)
|
||||
raise PermissionDenied
|
||||
if user_info:
|
||||
if not user and app_settings.create:
|
||||
User = get_user_model()
|
||||
user = User.objects.create(ou=get_default_ou())
|
||||
fc_account = models.FcAccount.objects.create(
|
||||
user=user,
|
||||
sub=sub,
|
||||
token=json.dumps(kwargs['token']))
|
||||
logger.debug(u'user creation enabled with fc_account (sub : %s - token : %s)',
|
||||
sub, json.dumps(kwargs['token']))
|
||||
hooks.call_hooks('event', name='fc-create', user=user, sub=sub)
|
||||
try:
|
||||
models.FcAccount.objects.create(
|
||||
user=user,
|
||||
sub=sub,
|
||||
order=0,
|
||||
token=json.dumps(kwargs['token']))
|
||||
except IntegrityError:
|
||||
# uniqueness check failed, as the user is new, it can only means that the sub is not unique
|
||||
# let's try again
|
||||
user.delete()
|
||||
return self.authenticate(sub, **kwargs)
|
||||
else:
|
||||
logger.debug(u'user creation enabled with fc_account (sub : %s - token : %s)',
|
||||
sub, json.dumps(kwargs['token']))
|
||||
hooks.call_hooks('event', name='fc-create', user=user, sub=sub)
|
||||
|
||||
if not user:
|
||||
return None
|
||||
|
||||
logger.debug(u'updated (given_name : %s - family_name : %s)', user_info['given_name'],
|
||||
user_info['family_name'])
|
||||
user.first_name = user_info['given_name']
|
||||
|
|
|
|||
|
|
@ -0,0 +1,60 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# Generated by Django 1.11.20 on 2019-06-12 21:27
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from collections import Counter
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
def set_fcaccount_order(apps, schema_editor):
|
||||
FcAccount = apps.get_model('authentic2_auth_fc', 'FcAccount')
|
||||
c = Counter()
|
||||
FcAccount.objects.update(order=0)
|
||||
user_ids = (
|
||||
FcAccount.objects
|
||||
.values('user_id')
|
||||
.annotate(total=models.Count('id'))
|
||||
.filter(total__gt=1)
|
||||
.values_list('user_id', flat=True)
|
||||
)
|
||||
subs = (
|
||||
FcAccount.objects
|
||||
.values_list('sub')
|
||||
.annotate(total=models.Count('id'))
|
||||
.filter(total__gt=1)
|
||||
.values_list('sub', flat=True)
|
||||
)
|
||||
|
||||
for account in FcAccount.objects.filter(models.Q(user_id__in=user_ids) | models.Q(sub__in=subs)):
|
||||
order = max(c[account.user_id], c[account.sub])
|
||||
if account.order != order:
|
||||
account.order = order
|
||||
account.save()
|
||||
order += 1
|
||||
c[account.user_id] = order
|
||||
c[account.sub] = order
|
||||
assert FcAccount.objects.filter(order__isnull=True).count() == 0
|
||||
|
||||
|
||||
def noop(apps, schema_editor):
|
||||
pass
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
# Disembiguate sub and user_id columns using an integer order column, then
|
||||
# force sub/order and user_id/order pairs unique to be unique in order
|
||||
# to move to a model where user and sub are unique without breaking on existing data
|
||||
|
||||
dependencies = [
|
||||
('authentic2_auth_fc', '0002_auto_20200416_1439'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='fcaccount',
|
||||
name='order',
|
||||
field=models.PositiveIntegerField(null=True, verbose_name='order'),
|
||||
),
|
||||
migrations.RunPython(set_fcaccount_order, noop),
|
||||
]
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# Generated by Django 1.11.20 on 2019-06-12 21:27
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
dependencies = [
|
||||
('authentic2_auth_fc', '0003_fcaccount_order1'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AlterField(
|
||||
model_name='fcaccount',
|
||||
name='order',
|
||||
field=models.PositiveIntegerField(default=0, verbose_name='order'),
|
||||
),
|
||||
migrations.AlterUniqueTogether(
|
||||
name='fcaccount',
|
||||
unique_together=set([('sub', 'order'), ('user', 'order')]),
|
||||
),
|
||||
]
|
||||
|
|
@ -98,6 +98,9 @@ class FcAccount(models.Model):
|
|||
sub = models.TextField(
|
||||
verbose_name=_('sub'),
|
||||
db_index=True)
|
||||
order = models.PositiveIntegerField(
|
||||
verbose_name=_('order'),
|
||||
default=0)
|
||||
token = models.TextField(verbose_name=_('access token'))
|
||||
user_info = models.TextField(verbose_name=_('access token'), blank=True, null=True)
|
||||
|
||||
|
|
@ -119,3 +122,9 @@ class FcAccount(models.Model):
|
|||
if 'family_name' in user_info:
|
||||
display_name.append(user_info['family_name'])
|
||||
return ' '.join(display_name)
|
||||
|
||||
class Meta:
|
||||
unique_together = [
|
||||
('sub', 'order'),
|
||||
('user', 'order'),
|
||||
]
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ from requests_oauthlib import OAuth2Session
|
|||
|
||||
|
||||
import django
|
||||
from django.db import IntegrityError
|
||||
from django.views.generic import View, FormView
|
||||
from django.http import HttpResponseRedirect, Http404
|
||||
from django.contrib.auth import REDIRECT_FIELD_NAME, get_user_model
|
||||
|
|
@ -162,6 +163,7 @@ class FcOAuthSessionViewMixin(LoggerMixin):
|
|||
redirect_field_name = REDIRECT_FIELD_NAME
|
||||
in_popup = False
|
||||
token = None
|
||||
user_info = None
|
||||
|
||||
def get_in_popup(self):
|
||||
return self.in_popup
|
||||
|
|
@ -324,6 +326,13 @@ class FcOAuthSessionViewMixin(LoggerMixin):
|
|||
scopes = list(set(scopes) | set(request.GET['fd_scopes'].split()))
|
||||
return ask_authorization(request, scopes, self.logger)
|
||||
|
||||
@property
|
||||
def fc_display_name(self):
|
||||
'''Human representation of the current FC account'''
|
||||
if not self.user_info:
|
||||
return u''
|
||||
return u'{0} {1}'.format(self.user_info['family_name'], self.user_info['given_name'])
|
||||
|
||||
|
||||
class PopupViewMixin(object):
|
||||
def get_in_popup(self):
|
||||
|
|
@ -337,11 +346,22 @@ class LoginOrLinkView(PopupViewMixin, FcOAuthSessionViewMixin, View):
|
|||
'''
|
||||
|
||||
def update_user_info(self):
|
||||
self.fc_account.token = json.dumps(self.token)
|
||||
self.fc_account.user_info = json.dumps(self.user_info)
|
||||
self.fc_account.save()
|
||||
self.fc_account.save(update_fields=['token', 'user_info'])
|
||||
utils.apply_user_info_mappings(self.fc_account.user, self.user_info)
|
||||
self.logger.debug('updating user_info %s', self.fc_account.user_info)
|
||||
|
||||
def uniqueness_check_failed(self, request):
|
||||
if models.FcAccount.objects.filter(user=request.user, order=0).count():
|
||||
messages.error(request,
|
||||
_('Your account is already linked to a FranceConnect account'))
|
||||
else:
|
||||
messages.error(request,
|
||||
_('The FranceConnect account {} is already'
|
||||
' linked with another account.').format(self.fc_display_name))
|
||||
return self.redirect(request)
|
||||
|
||||
def get(self, request, *args, **kwargs):
|
||||
registration = True if 'registration' in request.GET else False
|
||||
'''Request an access grant code and associate it to the current user'''
|
||||
|
|
@ -349,48 +369,21 @@ class LoginOrLinkView(PopupViewMixin, FcOAuthSessionViewMixin, View):
|
|||
if request.user.is_authenticated:
|
||||
# Prevent to add a link with an FC account already linked with another user.
|
||||
try:
|
||||
fc_account = models.FcAccount.objects.get(sub=self.sub, user__is_active=True)
|
||||
if fc_account.user is not request.user:
|
||||
msg = 'Attempt to link FC account {} already linked with user {}'
|
||||
self.logger.info(msg.format(self.sub, fc_account.user))
|
||||
messages.error(request,
|
||||
_('The FranceConnect account {} is already'
|
||||
' linked with another account.').format(fc_account))
|
||||
return self.redirect(request)
|
||||
except models.FcAccount.DoesNotExist:
|
||||
pass
|
||||
self.fc_account, created = models.FcAccount.objects.get_or_create(
|
||||
sub=self.sub, user=request.user, order=0,
|
||||
defaults={'token': json.dumps(self.token)})
|
||||
except IntegrityError:
|
||||
# unique index check failed, find why.
|
||||
return self.uniqueness_check_failed(request)
|
||||
|
||||
# Prevent to add a link to an user which is already linked to an FC account
|
||||
if request.user.fc_accounts.exists():
|
||||
self.logger.warning(u'cannot link to sub %s, account is already linked to an '
|
||||
u'FC account', self.sub)
|
||||
messages.error(request,
|
||||
_('Your account is already linked to a FranceConnect account'))
|
||||
return self.redirect(request)
|
||||
|
||||
json_token = json.dumps(self.token)
|
||||
self.fc_account, created = models.FcAccount.objects.get_or_create(
|
||||
defaults={'token': json_token},
|
||||
sub=self.sub, user=request.user)
|
||||
if created:
|
||||
self.logger.info('fc link created sub %s', self.sub)
|
||||
self.update_user_info()
|
||||
data = utils.get_mapped_attributes_flat(request)
|
||||
if 'email' in data:
|
||||
messages.info(request,
|
||||
_('Your FranceConnect account {} with '
|
||||
'email {} has been linked.').format(self.fc_account,
|
||||
data['email']))
|
||||
else:
|
||||
messages.info(request, _('Your FranceConnect account {} '
|
||||
'has been linked.').format(self.fc_account))
|
||||
hooks.call_hooks('event', name='fc-link', user=request.user, sub=self.sub,
|
||||
request=request)
|
||||
messages.info(request,
|
||||
_('Your FranceConnect account {} has been linked.').format(self.fc_display_name))
|
||||
hooks.call_hooks('event', name='fc-link', user=request.user, sub=self.sub, request=request)
|
||||
else:
|
||||
self.fc_account.token = json_token
|
||||
self.fc_account.save()
|
||||
self.update_user_info()
|
||||
messages.info(request, _('Your local account has been updated.'))
|
||||
self.update_user_info()
|
||||
return self.redirect(request)
|
||||
|
||||
default_ou = get_default_ou()
|
||||
|
|
@ -400,6 +393,8 @@ class LoginOrLinkView(PopupViewMixin, FcOAuthSessionViewMixin, View):
|
|||
sub=self.sub,
|
||||
user_info=self.user_info,
|
||||
token=self.token)
|
||||
if user:
|
||||
self.fc_account = user.fc_accounts.get(order=0)
|
||||
if not user and self.user_info.get('email') and email_is_unique:
|
||||
email = self.user_info['email']
|
||||
User = get_user_model()
|
||||
|
|
@ -417,9 +412,14 @@ class LoginOrLinkView(PopupViewMixin, FcOAuthSessionViewMixin, View):
|
|||
user = qs[0]
|
||||
# but does he have already a link to an FC account ?
|
||||
if not user.fc_accounts.exists():
|
||||
fc_account, created = models.FcAccount.objects.get_or_create(
|
||||
defaults={'token': json.dumps(self.token)},
|
||||
sub=self.sub, user=user)
|
||||
try:
|
||||
self.fc_account, created = models.FcAccount.objects.get_or_create(
|
||||
defaults={'token': json.dumps(self.token)},
|
||||
sub=self.sub, user=user, order=0)
|
||||
except IntegrityError:
|
||||
# unique index check failed, find why.
|
||||
return self.uniqueness_check_failed(request)
|
||||
|
||||
if created:
|
||||
self.logger.info(u'fc link created sub %s user %s', self.sub, user)
|
||||
hooks.call_hooks('event', name='fc-link', user=user, sub=self.sub,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
# authentic2-auth-fc - authentic2 authentication for FranceConnect
|
||||
# Copyright (C) 2019 Entr'ouvert
|
||||
# authentic2 - authentic2 authentication for FranceConnect
|
||||
# Copyright (C) 2020 Entr'ouvert
|
||||
#
|
||||
# This program is free software: you can redistribute it and/or modify it
|
||||
# under the terms of the GNU Affero General Public License as published
|
||||
|
|
|
|||
|
|
@ -0,0 +1,43 @@
|
|||
# authentic2 - authentic2 authentication for FranceConnect
|
||||
# Copyright (C) 2020 Entr'ouvert
|
||||
#
|
||||
# This program is free software: you can redistribute it and/or modify it
|
||||
# under the terms of the GNU Affero General Public License as published
|
||||
# by the Free Software Foundation, either version 3 of the License, or
|
||||
# (at your option) any later version.
|
||||
#
|
||||
# This program is distributed in the hope that it will be useful,
|
||||
# but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
# GNU Affero General Public License for more details.
|
||||
#
|
||||
# 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/>.
|
||||
|
||||
|
||||
def test_migration_0003_0004_fcaccount_order(migration):
|
||||
migrate_from = [('authentic2_auth_fc', '0002_auto_20200416_1439')]
|
||||
# it's a two-parts migration, as it contains data and schema changes.
|
||||
migrate_to = [('authentic2_auth_fc', '0004_fcaccount_order2')]
|
||||
|
||||
old_apps = migration.before(migrate_from, at_end=False)
|
||||
|
||||
User = old_apps.get_model('custom_user', 'User')
|
||||
FcAccount = old_apps.get_model('authentic2_auth_fc', 'FcAccount')
|
||||
user1 = User.objects.create(username='user1')
|
||||
user2 = User.objects.create(username='user2')
|
||||
user3 = User.objects.create(username='user3')
|
||||
FcAccount.objects.create(user=user1, sub='sub1')
|
||||
FcAccount.objects.create(user=user1, sub='sub2')
|
||||
FcAccount.objects.create(user=user2, sub='sub2')
|
||||
FcAccount.objects.create(user=user2, sub='sub3')
|
||||
FcAccount.objects.create(user=user3, sub='sub3')
|
||||
assert len(set(FcAccount.objects.values_list('user_id', flat=True))) == 3
|
||||
assert len(set(FcAccount.objects.values_list('sub', flat=True))) == 3
|
||||
assert FcAccount.objects.count() == 5
|
||||
|
||||
# execute migration
|
||||
new_apps = migration.apply(migrate_to)
|
||||
FcAccount = new_apps.get_model('authentic2_auth_fc', 'FcAccount')
|
||||
assert len(set(FcAccount.objects.values_list('user_id', 'order'))) == 5
|
||||
assert len(set(FcAccount.objects.values_list('sub', 'order'))) == 5
|
||||
Loading…
Reference in New Issue