Merge branch 'mfa-hotfixes' into release/2.022.24

Author: jnm

.github/workflows/pytest.yml

@@ -56,6 +56,7 @@ jobs:
       run: >-
         sudo DEBIAN_FRONTEND=noninteractive apt-get -y install
         gdal-bin gettext libproj-dev postgresql-client ffmpeg
+        gcc libc-dev
     - name: Install Python dependencies
       run: pip-sync dependencies/pip/dev_requirements.txt
     - name: Install JavaScript dependencies

jsapp/js/components/account/accountSidebar.tsx

@@ -83,16 +83,19 @@ export default class AccountSidebar extends React.Component<
           </bem.FormSidebar__label>
           }
 
-          <bem.FormSidebar__label
-            m={{selected: this.isSecuritySelected()}}
-            href={'#' + ROUTES.SECURITY}
-            disabled={ !(envStore.isReady && envStore.data.mfa_enabled) }
-          >
-            <Icon name='lock-alt' size='xl'/>
-            <bem.FormSidebar__labelText>
-              {t('Security')}
-            </bem.FormSidebar__labelText>
-          </bem.FormSidebar__label>
+          { /* hide "Security" entirely if nothing there is available */
+            envStore.isReady && envStore.data.mfa_enabled &&
+            <bem.FormSidebar__label
+              m={{selected: this.isSecuritySelected()}}
+              href={'#' + ROUTES.SECURITY}
+              disabled={ !(envStore.isReady && envStore.data.mfa_enabled) }
+            >
+              <Icon name='lock-alt' size='xl'/>
+              <bem.FormSidebar__labelText>
+                {t('Security')}
+              </bem.FormSidebar__labelText>
+            </bem.FormSidebar__label>
+          }
         </bem.FormSidebar>
       );
     }

kobo/apps/mfa/__init__.py

@@ -8,6 +8,5 @@ class MfaAppConfig(AppConfig):
 
     def ready(self):
         # Makes sure all signal handlers are connected
-        # Uncomment the lines below if you need signals
-        # from kobo.apps.mfa import signals
+        from kobo.apps.mfa import signals
         super().ready()

kobo/apps/mfa/admin.py

@@ -2,10 +2,13 @@
 from django.contrib import admin
 
 from .models import (
-    MFAMethod,
-    KoboMFAMethod,
-    KoboMFAMethodAdmin,
+    TrenchMFAMethod,
+    MfaAvailableToUser,
+    MfaAvailableToUserAdmin,
+    MfaMethod,
+    MfaMethodAdmin,
 )
 
-admin.site.unregister(MFAMethod)
-admin.site.register(KoboMFAMethod, KoboMFAMethodAdmin)
+admin.site.unregister(TrenchMFAMethod)
+admin.site.register(MfaMethod, MfaMethodAdmin)
+admin.site.register(MfaAvailableToUser, MfaAvailableToUserAdmin)

kobo/apps/mfa/forms.py

@@ -10,7 +10,7 @@
 )
 
 
-class MFALoginForm(AuthenticationForm):
+class MfaLoginForm(AuthenticationForm):
     """
     Authenticating users.
     If 2FA is activated, first step (of two) of the login process.
@@ -42,7 +42,7 @@ def get_ephemeral_token(self):
         return self.ephemeral_token_cache
 
 
-class MFATokenForm(forms.Form):
+class MfaTokenForm(forms.Form):
     """
     Validate 2FA token.
     Second (and last) step of login process when MFA is activated.

kobo/apps/mfa/migrations/0002_add_mfa_available_to_user_model.py

@@ -0,0 +1,31 @@
+# Generated by Django 2.2.27 on 2022-07-05 18:51
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('mfa', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='kobomfamethod',
+            options={'verbose_name': 'MFA Method', 'verbose_name_plural': 'MFA Methods'},
+        ),
+        migrations.CreateModel(
+            name='MfaAvailableToUser',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
+            ],
+            options={
+                'verbose_name': 'per-user availability',
+                'verbose_name_plural': 'per-user availabilities',
+            },
+        ),
+    ]

kobo/apps/mfa/migrations/0003_rename_kobo_mfa_method_model.py

@@ -0,0 +1,20 @@
+# Generated by Django 2.2.27 on 2022-07-05 19:19
+
+from django.conf import settings
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('trench', '0003_auto_20190213_2330'),
+        ('mfa', '0002_add_mfa_available_to_user_model'),
+    ]
+
+    operations = [
+        migrations.RenameModel(
+            old_name='KoboMFAMethod',
+            new_name='MfaMethod',
+        ),
+    ]

kobo/apps/mfa/models.py

@@ -1,15 +1,41 @@
 # coding: utf-8
 from django.conf import settings
+from django.contrib import admin
 from django.db import models
 from django.utils.timezone import now
-from trench.admin import MFAMethod, MFAMethodAdmin
+from trench.admin import (
+    MFAMethod as TrenchMFAMethod,
+    MFAMethodAdmin as TrenchMFAMethodAdmin,
+)
 
 from kpi.deployment_backends.kc_access.shadow_models import (
     KobocatUserProfile,
 )
 
 
-class KoboMFAMethod(MFAMethod):
+class MfaAvailableToUser(models.Model):
+
+    class Meta:
+        verbose_name = 'per-user availability'
+        verbose_name_plural = 'per-user availabilities'
+    user = models.ForeignKey('auth.User', on_delete=models.CASCADE)
+
+    def __str__(self):
+        # Used to display the user-friendly representation of MfaAvailableToUser
+        # objects, especially in Django Admin interface.
+        return f'MFA available to user {self.user.username}'
+
+
+class MfaAvailableToUserAdmin(admin.ModelAdmin):
+
+    search_fields = ('user__username',)
+    autocomplete_fields = ['user']
+    # To customize list columns, uncomment line below to use instead of string
+    # representation of `MfaAvailableToUser` objects
+    # list_display = ('user',)
+
+
+class MfaMethod(TrenchMFAMethod):
     """
     Extend DjangoTrench model to add created, modified and last disabled date
     """
@@ -65,6 +91,6 @@ def delete(self, using=None, keep_parents=False):
             )
 
 
-class KoboMFAMethodAdmin(MFAMethodAdmin):
+class MfaMethodAdmin(TrenchMFAMethodAdmin):
 
     search_fields = ['user__username']

kobo/apps/mfa/serializers.py

@@ -1,9 +1,29 @@
 # coding: utf-8
 from rest_framework import serializers
-from trench.utils import get_mfa_model
+from trench.serializers import RequestMFAMethodActivationSerializer
+from trench.utils import create_secret, get_mfa_model
 
 
-class UserMFAMethodSerializer(serializers.ModelSerializer):
+class ActivateMfaMethodSerializer(RequestMFAMethodActivationSerializer):
+     def create(self, validated_data):
+        """
+        When the client requests creation of a new MFA method, *always* create
+        a new secret. The default django-trench behavior reuses old secrets,
+        meaning that without this override, a given user would always see the
+        same QR code no matter how many times they enabled and disabled MFA.
+        That default behavior would create a security weakness if a user lost
+        their original device, because deactivating MFA and reconfiguring it on
+        a new device would not prevent the lost device from continuing to
+        generate valid TOTPs.
+        """
+        mfa_method, created = super().create(validated_data)
+        if not created:
+            mfa_method.secret = create_secret()
+            mfa_method.save()
+        return mfa_method, created
+
+
+class UserMfaMethodSerializer(serializers.ModelSerializer):
     """
     Exposes user's MFA methods and their created, modified and disabled dates
     """

kobo/apps/mfa/signals.py

@@ -0,0 +1,28 @@
+# coding: utf-8
+from django.db.models.signals import pre_delete
+from django.dispatch import receiver
+
+from .models import MfaAvailableToUser, MfaMethod
+
+
+@receiver(pre_delete, sender=MfaAvailableToUser)
+def deactivate_mfa_method_for_user(**kwargs):
+    """
+    Deactivate MFA methods for user on delete (and bulk delete)
+    """
+    # We need to use a signal (instead of adding this code to
+    # `MfaAvailableToUser.delete()`) because of bulk deletes which do not
+    # call `.delete()`.
+
+    mfa_available_to_user = kwargs['instance']
+    # Deactivate any MFA methods user could have already created
+    try:
+        # Use `.get()` + `.save()` (from model `MfaMethod`) instead of
+        # `.update()` to run some logic inside `.save()`. It makes an extra
+        # query to DB but avoid duplicated code.
+        mfa_method = MfaMethod.objects.get(user=mfa_available_to_user.user)
+    except MfaMethod.DoesNotExist:
+        pass
+    else:
+        mfa_method.is_active = False
+        mfa_method.save(update_fields=['is_active'])