use Inheritance for Factors instead of JSONField

passbook/admin/templates/administration/factor/list.html

@@ -13,9 +13,17 @@
     <h1>{% trans "Factors" %}</h1>
     <span>{% trans "Factors required for a user to successfully authenticate." %}</span>
     <hr>
-   <a href="{% url 'passbook_admin:factor-create' %}" class="btn btn-primary">
+    <div class="dropdown">
+        <button class="btn btn-primary dropdown-toggle" type="button" id="createDropdown" data-toggle="dropdown">
             {% trans 'Create...' %}
-   </a>
+            <span class="caret"></span>
+        </button>
+        <ul class="dropdown-menu" role="menu" aria-labelledby="createDropdown">
+            {% for type, name in types.items %}
+            <li role="presentation"><a role="menuitem" tabindex="-1" href="{% url 'passbook_admin:factor-create' %}?type={{ type }}">{{ name }}</a></li>
+            {% endfor %}
+        </ul>
+    </div>
     <hr>
     <table class="table table-striped table-bordered">
         <thead>
@@ -35,11 +43,14 @@
                 <td>{{ factor.order }}</td>
                 <td>{{ factor.enabled }}</td>
                 <td>
-            <a class="btn btn-default btn-sm" href="{% url 'passbook_admin:factor-update' pk=factor.pk %}?back={{ request.get_full_path }}">{% trans 'Edit' %}</a>
+                    <a class="btn btn-default btn-sm"
-            <a class="btn btn-default btn-sm" href="{% url 'passbook_admin:factor-delete' pk=factor.pk %}?back={{ request.get_full_path }}">{% trans 'Delete' %}</a>
+                        href="{% url 'passbook_admin:factor-update' pk=factor.pk %}?back={{ request.get_full_path }}">{% trans 'Edit' %}</a>
+                    <a class="btn btn-default btn-sm"
+                        href="{% url 'passbook_admin:factor-delete' pk=factor.pk %}?back={{ request.get_full_path }}">{% trans 'Delete' %}</a>
                     {% get_links factor as links %}
                     {% for name, href in links.items %}
-              <a class="btn btn-default btn-sm" href="{{ href }}?back={{ request.get_full_path }}">{% trans name %}</a>
+                    <a class="btn btn-default btn-sm"
+                        href="{{ href }}?back={{ request.get_full_path }}">{% trans name %}</a>
                     {% endfor %}
                 </td>
             </tr>

passbook/admin/templates/administration/source/list.html

@@ -16,7 +16,8 @@
         </button>
         <ul class="dropdown-menu" role="menu" aria-labelledby="createDropdown">
             {% for type, name in types.items %}
-      <li role="presentation"><a role="menuitem" tabindex="-1" href="{% url 'passbook_admin:source-create' %}?type={{ type }}">{{ name }}</a></li>
+            <li role="presentation"><a role="menuitem" tabindex="-1"
+                    href="{% url 'passbook_admin:source-create' %}?type={{ type }}">{{ name }}</a></li>
             {% endfor %}
         </ul>
     </div>
@@ -35,11 +36,14 @@
                 <td>{{ source.name }}</td>
                 <td>{{ source|fieldtype }}</td>
                 <td>
-            <a class="btn btn-default btn-sm" href="{% url 'passbook_admin:source-update' pk=source.uuid %}?back={{ request.get_full_path }}">{% trans 'Edit' %}</a>
+                    <a class="btn btn-default btn-sm"
-            <a class="btn btn-default btn-sm" href="{% url 'passbook_admin:source-delete' pk=source.uuid %}?back={{ request.get_full_path }}">{% trans 'Delete' %}</a>
+                        href="{% url 'passbook_admin:source-update' pk=source.uuid %}?back={{ request.get_full_path }}">{% trans 'Edit' %}</a>
+                    <a class="btn btn-default btn-sm"
+                        href="{% url 'passbook_admin:source-delete' pk=source.uuid %}?back={{ request.get_full_path }}">{% trans 'Delete' %}</a>
                     {% get_links source as links %}
                     {% for name, href in links %}
-              <a class="btn btn-default btn-sm" href="{{ href }}?back={{ request.get_full_path }}">{% trans name %}</a>
+                    <a class="btn btn-default btn-sm"
+                        href="{{ href }}?back={{ request.get_full_path }}">{% trans name %}</a>
                     {% endfor %}
                 </td>
             </tr>

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()

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"""
 

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(),
+        }

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',),
+        ),
+    ]

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')

passbook/core/auth/factor_manager.py

@@ -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()

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"""
 

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 PasswordFactor(FormView, AuthenticationFactor):
-class AuthenticationBackendFactor(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):

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')}))

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 DummyFactor, PasswordFactor
-from passbook.core.models import Factor
-from passbook.lib.utils.reflection import class_to_path
 
+GENERAL_FIELDS = ['name', 'slug', 'order', 'policies', 'enabled']
 
-def get_factors():
+class PasswordFactorForm(forms.ModelForm):
-    """Return list of factors for Select Widget"""
+    """Form to create/edit Password Factors"""
-    for factor in MANAGER.all:
-        yield (class_to_path(factor), factor.__name__)
-
-class FactorForm(forms.ModelForm):
-    """Form to create/edit Factors"""
 
     class Meta:
 
-        model = Factor
+        model = PasswordFactor
-        fields = ['name', 'slug', 'order', 'policies', 'type', 'enabled', 'arguments']
+        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(),
         }

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',
+        ),
+    ]

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'},
+        ),
+    ]

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