From 9c2cfd7db45254f9191865c72be562a433daa2aa Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 24 Feb 2019 22:39:09 +0100 Subject: [PATCH] use Inheritance for Factors instead of JSONField --- .../templates/administration/factor/list.html | 81 ++++++++++-------- .../templates/administration/source/list.html | 82 ++++++++++--------- passbook/admin/views/factors.py | 39 +++++++-- passbook/captcha_factor/factor.py | 2 - passbook/captcha_factor/forms.py | 17 ++++ .../captcha_factor/migrations/0001_initial.py | 29 +++++++ .../captcha_factor/migrations/__init__.py | 0 passbook/captcha_factor/models.py | 23 ++++++ passbook/core/auth/factor_manager.py | 25 ------ passbook/core/auth/factors/dummy.py | 2 - .../auth/factors/{backend.py => password.py} | 8 +- passbook/core/forms/authentication.py | 10 ++- passbook/core/forms/factors.py | 31 ++++--- .../migrations/0009_auto_20190224_0950.py | 44 ++++++++++ .../migrations/0010_auto_20190224_1016.py | 21 +++++ passbook/core/models.py | 38 ++++++++- 16 files changed, 319 insertions(+), 133 deletions(-) create mode 100644 passbook/captcha_factor/migrations/0001_initial.py create mode 100644 passbook/captcha_factor/migrations/__init__.py create mode 100644 passbook/captcha_factor/models.py delete mode 100644 passbook/core/auth/factor_manager.py rename passbook/core/auth/factors/{backend.py => password.py} (87%) create mode 100644 passbook/core/migrations/0009_auto_20190224_0950.py create mode 100644 passbook/core/migrations/0010_auto_20190224_1016.py diff --git a/passbook/admin/templates/administration/factor/list.html b/passbook/admin/templates/administration/factor/list.html index bcedc7a10..bb918adb6 100644 --- a/passbook/admin/templates/administration/factor/list.html +++ b/passbook/admin/templates/administration/factor/list.html @@ -10,41 +10,52 @@ {% block content %}
-

{% trans "Factors" %}

- {% trans "Factors required for a user to successfully authenticate." %} -
- - {% trans 'Create...' %} - -
- - - - - - - - - - - - {% for factor in object_list %} - - - - - - - - {% endfor %} - -
{% trans 'Name' %}{% trans 'Type' %}{% trans 'Order' %}{% trans 'Enabled?' %}
{{ factor.name }} ({{ factor.slug }}){{ factor.type }}{{ factor.order }}{{ factor.enabled }} - {% trans 'Edit' %} - {% trans 'Delete' %} - {% get_links factor as links %} - {% for name, href in links.items %} - {% trans name %} +

{% trans "Factors" %}

+ {% trans "Factors required for a user to successfully authenticate." %} +
+
+ +
+ +
+
+ + + + + + + + + + + + {% for factor in object_list %} + + + + + + + + {% endfor %} + +
{% trans 'Name' %}{% trans 'Type' %}{% trans 'Order' %}{% trans 'Enabled?' %}
{{ factor.name }} ({{ factor.slug }}){{ factor.type }}{{ factor.order }}{{ factor.enabled }} + {% trans 'Edit' %} + {% trans 'Delete' %} + {% get_links factor as links %} + {% for name, href in links.items %} + {% trans name %} + {% endfor %} +
{% endblock %} diff --git a/passbook/admin/templates/administration/source/list.html b/passbook/admin/templates/administration/source/list.html index d227bf1ad..0676c36db 100644 --- a/passbook/admin/templates/administration/source/list.html +++ b/passbook/admin/templates/administration/source/list.html @@ -6,45 +6,49 @@ {% block content %}
-

{% trans "Sources" %}

- {% trans "External Sources which can be used to get Identities into passbook, for example Social Providers like Twiter and GitHub or Enterprise Providers like ADFS and LDAP." %} -
-
- - -
-
- - - - - - - - - - {% for source in object_list %} - - - - - - {% endfor %} - -
{% trans 'Name' %}{% trans 'Class' %}
{{ source.name }}{{ source|fieldtype }} - {% trans 'Edit' %} - {% trans 'Delete' %} - {% get_links source as links %} - {% for name, href in links %} - {% trans name %} +

{% trans "Sources" %}

+ {% trans "External Sources which can be used to get Identities into passbook, for example Social Providers like Twiter and GitHub or Enterprise Providers like ADFS and LDAP." %} +
+
+ +
+ +
+
+ + + + + + + + + + {% for source in object_list %} + + + + + + {% endfor %} + +
{% trans 'Name' %}{% trans 'Class' %}
{{ source.name }}{{ source|fieldtype }} + {% trans 'Edit' %} + {% trans 'Delete' %} + {% get_links source as links %} + {% for name, href in links %} + {% trans name %} + {% endfor %} +
{% endblock %} diff --git a/passbook/admin/views/factors.py b/passbook/admin/views/factors.py index 82409816d..4dcab93b3 100644 --- a/passbook/admin/views/factors.py +++ b/passbook/admin/views/factors.py @@ -1,14 +1,20 @@ """passbook Factor administration""" from django.contrib.messages.views import SuccessMessageMixin +from django.http import Http404 from django.urls import reverse_lazy from django.utils.translation import ugettext as _ from django.views.generic import CreateView, DeleteView, ListView, UpdateView from passbook.admin.mixins import AdminRequiredMixin -from passbook.core.forms.factors import FactorForm from passbook.core.models import Factor +from passbook.lib.utils.reflection import path_to_class +def all_subclasses(cls): + """Recursively return all subclassess of cls""" + return set(cls.__subclasses__()).union( + [s for c in cls.__subclasses__() for s in all_subclasses(c)]) + class FactorListView(AdminRequiredMixin, ListView): """Show list of all factors""" @@ -18,17 +24,32 @@ class FactorListView(AdminRequiredMixin, ListView): def get_context_data(self, **kwargs): kwargs['types'] = { - x.__name__: x._meta.verbose_name for x in Factor.__subclasses__()} + x.__name__: x._meta.verbose_name for x in all_subclasses(Factor)} return super().get_context_data(**kwargs) + def get_queryset(self): + return super().get_queryset().select_subclasses() class FactorCreateView(SuccessMessageMixin, AdminRequiredMixin, CreateView): """Create new Factor""" - template_name = 'generic/create.html' + template_name = 'generic/create_inheritance.html' success_url = reverse_lazy('passbook_admin:factors') success_message = _('Successfully created Factor') - form_class = FactorForm + + def get_context_data(self, **kwargs): + kwargs = super().get_context_data(**kwargs) + source_type = self.request.GET.get('type') + model = next(x for x in all_subclasses(Factor) if x.__name__ == source_type) + kwargs['type'] = model._meta.verbose_name + return kwargs + + def get_form_class(self): + source_type = self.request.GET.get('type') + model = next(x for x in all_subclasses(Factor) if x.__name__ == source_type) + if not model: + raise Http404 + return path_to_class(model.form) class FactorUpdateView(SuccessMessageMixin, AdminRequiredMixin, UpdateView): @@ -38,8 +59,13 @@ class FactorUpdateView(SuccessMessageMixin, AdminRequiredMixin, UpdateView): template_name = 'generic/update.html' success_url = reverse_lazy('passbook_admin:factors') success_message = _('Successfully updated Factor') - form_class = FactorForm + def get_form_class(self): + source_type = self.request.GET.get('type') + model = next(x for x in all_subclasses(Factor) if x.__name__ == source_type) + if not model: + raise Http404 + return path_to_class(model.form) class FactorDeleteView(SuccessMessageMixin, AdminRequiredMixin, DeleteView): """Delete factor""" @@ -48,3 +74,6 @@ class FactorDeleteView(SuccessMessageMixin, AdminRequiredMixin, DeleteView): template_name = 'generic/delete.html' success_url = reverse_lazy('passbook_admin:factors') success_message = _('Successfully updated Factor') + + def get_object(self, queryset=None): + return Factor.objects.filter(pk=self.kwargs.get('pk')).select_subclasses().first() diff --git a/passbook/captcha_factor/factor.py b/passbook/captcha_factor/factor.py index 82e76f1a0..a16222995 100644 --- a/passbook/captcha_factor/factor.py +++ b/passbook/captcha_factor/factor.py @@ -4,10 +4,8 @@ from django.views.generic import FormView from passbook.captcha_factor.forms import CaptchaForm from passbook.core.auth.factor import AuthenticationFactor -from passbook.core.auth.factor_manager import MANAGER -@MANAGER.factor() class CaptchaFactor(FormView, AuthenticationFactor): """Simple captcha checker, logic is handeled in django-captcha module""" diff --git a/passbook/captcha_factor/forms.py b/passbook/captcha_factor/forms.py index ba1c53760..ed48968ff 100644 --- a/passbook/captcha_factor/forms.py +++ b/passbook/captcha_factor/forms.py @@ -2,8 +2,25 @@ from captcha.fields import ReCaptchaField from django import forms +from passbook.captcha_factor.models import CaptchaFactor +from passbook.core.forms.factors import GENERAL_FIELDS + class CaptchaForm(forms.Form): """passbook captcha factor form""" captcha = ReCaptchaField() + +class CaptchaFactorForm(forms.ModelForm): + """Form to edit CaptchaFactor Instance""" + + class Meta: + + model = CaptchaFactor + fields = GENERAL_FIELDS + ['public_key', 'private_key'] + widgets = { + 'name': forms.TextInput(), + 'order': forms.NumberInput(), + 'public_key': forms.TextInput(), + 'private_key': forms.TextInput(), + } diff --git a/passbook/captcha_factor/migrations/0001_initial.py b/passbook/captcha_factor/migrations/0001_initial.py new file mode 100644 index 000000000..97d2be499 --- /dev/null +++ b/passbook/captcha_factor/migrations/0001_initial.py @@ -0,0 +1,29 @@ +# Generated by Django 2.1.7 on 2019-02-24 21:35 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('passbook_core', '0010_auto_20190224_1016'), + ] + + operations = [ + migrations.CreateModel( + name='CaptchaFactor', + fields=[ + ('factor_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='passbook_core.Factor')), + ('public_key', models.TextField()), + ('private_key', models.TextField()), + ], + options={ + 'verbose_name': 'Captcha Factor', + 'verbose_name_plural': 'Captcha Factors', + }, + bases=('passbook_core.factor',), + ), + ] diff --git a/passbook/captcha_factor/migrations/__init__.py b/passbook/captcha_factor/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/passbook/captcha_factor/models.py b/passbook/captcha_factor/models.py new file mode 100644 index 000000000..cb0319ac2 --- /dev/null +++ b/passbook/captcha_factor/models.py @@ -0,0 +1,23 @@ +"""passbook captcha factor""" +from django.db import models +from django.utils.translation import gettext as _ + +from passbook.core.models import Factor + + +class CaptchaFactor(Factor): + """Captcha Factor instance""" + + public_key = models.TextField() + private_key = models.TextField() + + type = 'passbook.captcha_factor.factor.CaptchaFactor' + form = 'passbook.captcha_factor.forms.CaptchaFactorForm' + + def __str__(self): + return "Captcha Factor %s" % self.slug + + class Meta: + + verbose_name = _('Captcha Factor') + verbose_name_plural = _('Captcha Factors') diff --git a/passbook/core/auth/factor_manager.py b/passbook/core/auth/factor_manager.py deleted file mode 100644 index ecde60724..000000000 --- a/passbook/core/auth/factor_manager.py +++ /dev/null @@ -1,25 +0,0 @@ -"""Authentication Factor Manager""" -from logging import getLogger - -LOGGER = getLogger(__name__) - -class AuthenticationFactorManager: - """Manager to hold all Factors.""" - - __factors = [] - - def factor(self): - """Class decorator to register classes inline.""" - def inner_wrapper(cls): - self.__factors.append(cls) - LOGGER.debug("Registered factor '%s'", cls.__name__) - return cls - return inner_wrapper - - @property - def all(self): - """Get list of all registered factors""" - return self.__factors - - -MANAGER = AuthenticationFactorManager() diff --git a/passbook/core/auth/factors/dummy.py b/passbook/core/auth/factors/dummy.py index 64c5f17f4..593916409 100644 --- a/passbook/core/auth/factors/dummy.py +++ b/passbook/core/auth/factors/dummy.py @@ -2,12 +2,10 @@ from logging import getLogger from passbook.core.auth.factor import AuthenticationFactor -from passbook.core.auth.factor_manager import MANAGER LOGGER = getLogger(__name__) -@MANAGER.factor() class DummyFactor(AuthenticationFactor): """Dummy factor for testing with multiple factors""" diff --git a/passbook/core/auth/factors/backend.py b/passbook/core/auth/factors/password.py similarity index 87% rename from passbook/core/auth/factors/backend.py rename to passbook/core/auth/factors/password.py index 1bff589cd..3ea1c4689 100644 --- a/passbook/core/auth/factors/backend.py +++ b/passbook/core/auth/factors/password.py @@ -8,19 +8,17 @@ from django.utils.translation import gettext as _ from django.views.generic import FormView from passbook.core.auth.factor import AuthenticationFactor -from passbook.core.auth.factor_manager import MANAGER from passbook.core.auth.view import AuthenticationView -from passbook.core.forms.authentication import AuthenticationBackendFactorForm +from passbook.core.forms.authentication import PasswordFactorForm from passbook.lib.config import CONFIG LOGGER = getLogger(__name__) -@MANAGER.factor() -class AuthenticationBackendFactor(FormView, AuthenticationFactor): +class PasswordFactor(FormView, AuthenticationFactor): """Authentication factor which authenticates against django's AuthBackend""" - form_class = AuthenticationBackendFactorForm + form_class = PasswordFactorForm template_name = 'login/factors/backend.html' def form_valid(self, form): diff --git a/passbook/core/forms/authentication.py b/passbook/core/forms/authentication.py index e49fcc587..b10c9c837 100644 --- a/passbook/core/forms/authentication.py +++ b/passbook/core/forms/authentication.py @@ -29,10 +29,6 @@ class LoginForm(forms.Form): validate_email(self.cleaned_data.get('uid_field')) return self.cleaned_data.get('uid_field') -class AuthenticationBackendFactorForm(forms.Form): - """Password authentication form""" - - password = forms.CharField(widget=forms.PasswordInput(attrs={'placeholder': _('Password')})) class SignUpForm(forms.Form): """SignUp Form""" @@ -86,3 +82,9 @@ class SignUpForm(forms.Form): # TODO: Password policy? Via Plugin? via Policy? # return check_password(self) return self.cleaned_data.get('password_repeat') + + +class PasswordFactorForm(forms.Form): + """Password authentication form""" + + password = forms.CharField(widget=forms.PasswordInput(attrs={'placeholder': _('Password')})) diff --git a/passbook/core/forms/factors.py b/passbook/core/forms/factors.py index 4a3b1edb8..692066e57 100644 --- a/passbook/core/forms/factors.py +++ b/passbook/core/forms/factors.py @@ -1,25 +1,30 @@ """passbook administration forms""" from django import forms -from passbook.core.auth.factor_manager import MANAGER -from passbook.core.models import Factor -from passbook.lib.utils.reflection import class_to_path +from passbook.core.models import DummyFactor, PasswordFactor +GENERAL_FIELDS = ['name', 'slug', 'order', 'policies', 'enabled'] -def get_factors(): - """Return list of factors for Select Widget""" - for factor in MANAGER.all: - yield (class_to_path(factor), factor.__name__) - -class FactorForm(forms.ModelForm): - """Form to create/edit Factors""" +class PasswordFactorForm(forms.ModelForm): + """Form to create/edit Password Factors""" class Meta: - model = Factor - fields = ['name', 'slug', 'order', 'policies', 'type', 'enabled', 'arguments'] + model = PasswordFactor + fields = GENERAL_FIELDS + ['backends'] + widgets = { + 'name': forms.TextInput(), + 'order': forms.NumberInput(), + } + +class DummyFactorForm(forms.ModelForm): + """Form to create/edit Dummy Factor""" + + class Meta: + + model = DummyFactor + fields = GENERAL_FIELDS widgets = { - 'type': forms.Select(choices=get_factors()), 'name': forms.TextInput(), 'order': forms.NumberInput(), } diff --git a/passbook/core/migrations/0009_auto_20190224_0950.py b/passbook/core/migrations/0009_auto_20190224_0950.py new file mode 100644 index 000000000..6e0e39e7f --- /dev/null +++ b/passbook/core/migrations/0009_auto_20190224_0950.py @@ -0,0 +1,44 @@ +# Generated by Django 2.1.7 on 2019-02-24 09:50 + +import django.contrib.postgres.fields +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('passbook_core', '0008_auto_20190221_1516'), + ] + + operations = [ + migrations.CreateModel( + name='DummyFactor', + fields=[ + ('factor_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='passbook_core.Factor')), + ], + options={ + 'abstract': False, + }, + bases=('passbook_core.factor',), + ), + migrations.CreateModel( + name='PasswordFactor', + fields=[ + ('factor_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='passbook_core.Factor')), + ('backends', django.contrib.postgres.fields.ArrayField(base_field=models.TextField(), size=None)), + ], + options={ + 'abstract': False, + }, + bases=('passbook_core.factor',), + ), + migrations.RemoveField( + model_name='factor', + name='arguments', + ), + migrations.RemoveField( + model_name='factor', + name='type', + ), + ] diff --git a/passbook/core/migrations/0010_auto_20190224_1016.py b/passbook/core/migrations/0010_auto_20190224_1016.py new file mode 100644 index 000000000..b0da3563b --- /dev/null +++ b/passbook/core/migrations/0010_auto_20190224_1016.py @@ -0,0 +1,21 @@ +# Generated by Django 2.1.7 on 2019-02-24 10:16 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('passbook_core', '0009_auto_20190224_0950'), + ] + + operations = [ + migrations.AlterModelOptions( + name='dummyfactor', + options={'verbose_name': 'Dummy Factor', 'verbose_name_plural': 'Dummy Factors'}, + ), + migrations.AlterModelOptions( + name='passwordfactor', + options={'verbose_name': 'Password Factor', 'verbose_name_plural': 'Password Factors'}, + ), + ] diff --git a/passbook/core/models.py b/passbook/core/models.py index eca304028..1c0866a69 100644 --- a/passbook/core/models.py +++ b/passbook/core/models.py @@ -6,7 +6,7 @@ from time import sleep from uuid import uuid4 from django.contrib.auth.models import AbstractUser -from django.contrib.postgres.fields import JSONField +from django.contrib.postgres.fields import ArrayField from django.db import models from django.urls import reverse_lazy from django.utils.translation import gettext as _ @@ -68,13 +68,45 @@ class Factor(PolicyModel): name = models.TextField() slug = models.SlugField(unique=True) order = models.IntegerField() - type = models.TextField(unique=True) enabled = models.BooleanField(default=True) - arguments = JSONField(default=dict, blank=True) + + objects = InheritanceManager() + type = '' + form = '' def __str__(self): return "Factor %s" % self.slug +class PasswordFactor(Factor): + """Password-based Django-backend Authentication Factor""" + + backends = ArrayField(models.TextField()) + + type = 'passbook.core.auth.factors.password.PasswordFactor' + form = 'passbook.core.forms.factors.PasswordFactorForm' + + def __str__(self): + return "Password Factor %s" % self.slug + + class Meta: + + verbose_name = _('Password Factor') + verbose_name_plural = _('Password Factors') + +class DummyFactor(Factor): + """Dummy factor, mostly used to debug""" + + type = 'passbook.core.auth.factors.dummy.DummyFactor' + form = 'passbook.core.forms.factors.DummyFactorForm' + + def __str__(self): + return "Dummy Factor %s" % self.slug + + class Meta: + + verbose_name = _('Dummy Factor') + verbose_name_plural = _('Dummy Factors') + class Application(PolicyModel): """Every Application which uses passbook for authentication/identification/authorization needs an Application record. Other authentication types can subclass this Model to