Add authorisation process for sign ups and allow access to EventDetail for basic users (#399)

* CHANGE: First pass at opening up RIGS #233

Whilst it makes it something of a misnomer, the intent is to make the 'view_event' perm a permission to view event details like client/price. I don't see the point in giving everyone 'view_event' and adding a new 'view_event_detail'...Open to arguments the other way.

* CHANGE: New user signups now require admin approval

Given that I intend to reveal much more data to new users this seems necessary...

* CHORE: Fix CI

* FIX: Legacy Profiles are now auto-approved correctly

* Add testing of approval mechanism

This fixes the other functional tests failing because the user cannot login without being approved.

* Superusers bypass approval check

This should fix the remainder of the tests

* Prevent unapproved users logging in through embeds

Test suite doing its job...!

* FIX: Require login on events and event embeds again

Little too far to the open side there Arona... Whooooooops!

* FIX: Use has_oembed decorator for events

* FIX: Re-prevent basic seeing reversion

This is to prevent financials/client data leaking when changed. Hopefully can show them a filtered version in future.

* FIX: Remove mitigation for #264

Someone quietly fixed it, it appears

* FEAT: Add admin email notif when an account is activated and awaiting approval

No async or time-since shenanigans yet!

* FIX: Whoops, undo accidental whitespace change

* FEAT: Add a fifteen min cooldown between emails to admins

Probably not the right way to go about it...but it does work!

TODO: How to handle cooldown-emailing shared mailbox addresses?

* FIX: Remove event modal history deadlink for basic users

Also removes some links on the RIGS homepage that will deadlink for them

* FIX: Wrong perms syntax for history pages

* CHORE: Squash migrations

* FIX: Use a setting for cooldown

* FIX: Minor code improvements

PyRIGS/settings.py

@@ -12,6 +12,7 @@ https://docs.djangoproject.com/en/1.7/ref/settings/
 import os
 import raven
 import secrets
+import datetime
 
 BASE_DIR = os.path.dirname(os.path.dirname(__file__))
 
@@ -44,9 +45,9 @@ if not DEBUG:
 
 INTERNAL_IPS = ['127.0.0.1']
 
-ADMINS = (
+ADMINS = [('Tom Price', 'tomtom5152@gmail.com'), ('IT Manager', 'it@nottinghamtec.co.uk'), ('Arona Jones', 'arona.jones@nottinghamtec.co.uk')]
-    ('Tom Price', 'tomtom5152@gmail.com')
+if DEBUG:
-)
+    ADMINS.append(('Testing Superuser', 'superuser@example.com'))
 
 # Application definition
 
@@ -182,6 +183,8 @@ if not DEBUG or EMAILER_TEST:
 else:
     EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'
 
+EMAIL_COOLDOWN = datetime.timedelta(minutes=15)
+
 # Internationalization
 # https://docs.djangoproject.com/en/1.7/topics/i18n/
 

RIGS/admin.py

@@ -22,13 +22,22 @@ admin.site.register(models.Invoice)
 admin.site.register(models.Payment)
 
 
+def approve_user(modeladmin, request, queryset):
+    queryset.update(is_approved=True)
+
+
+approve_user.short_description = "Approve selected users"
+
+
 @admin.register(models.Profile)
 class ProfileAdmin(UserAdmin):
+    # Don't know how to add 'is_approved' whilst preserving the default list...
+    list_filter = ('is_approved', 'is_active', 'is_staff', 'is_superuser', 'groups')
     fieldsets = (
         (None, {'fields': ('username', 'password')}),
         (_('Personal info'), {
             'fields': ('first_name', 'last_name', 'email', 'initials', 'phone')}),
-        (_('Permissions'), {'fields': ('is_active', 'is_staff', 'is_superuser',
+        (_('Permissions'), {'fields': ('is_approved', 'is_active', 'is_staff', 'is_superuser',
                                        'groups', 'user_permissions')}),
         (_('Important dates'), {
             'fields': ('last_login', 'date_joined')}),
@@ -41,6 +50,7 @@ class ProfileAdmin(UserAdmin):
     )
     form = forms.ProfileChangeForm
     add_form = forms.ProfileCreationForm
+    actions = [approve_user]
 
 
 class AssociateAdmin(VersionAdmin):

RIGS/forms.py

@@ -2,8 +2,10 @@ from django import forms
 from django.utils import formats
 from django.conf import settings
 from django.core import serializers
+from django.core.mail import EmailMessage, EmailMultiAlternatives
 from django.contrib.auth.forms import UserCreationForm, UserChangeForm, AuthenticationForm, PasswordResetForm
 from registration.forms import RegistrationFormUniqueEmail
+from django.contrib.auth.forms import AuthenticationForm
 from captcha.fields import ReCaptchaField
 import simplejson
 
@@ -33,8 +35,16 @@ class ProfileRegistrationFormUniqueEmail(RegistrationFormUniqueEmail):
         return self.cleaned_data['initials']
 
 
+class CheckApprovedForm(AuthenticationForm):
+    def confirm_login_allowed(self, user):
+        if user.is_approved or user.is_superuser:
+            return AuthenticationForm.confirm_login_allowed(self, user)
+        else:
+            raise forms.ValidationError("Your account hasn't been approved by an administrator yet. Please check back in a few minutes!")
+
+
 # Embedded Login form - remove the autofocus
-class EmbeddedAuthenticationForm(AuthenticationForm):
+class EmbeddedAuthenticationForm(CheckApprovedForm):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.fields['username'].widget.attrs.pop('autofocus', None)

RIGS/migrations/0036_profile_is_approved.py

@@ -0,0 +1,23 @@
+# Generated by Django 2.0.13 on 2020-01-10 14:52
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('RIGS', '0035_auto_20191124_1319'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='profile',
+            name='is_approved',
+            field=models.BooleanField(default=False),
+        ),
+        migrations.AddField(
+            model_name='profile',
+            name='last_emailed',
+            field=models.DateTimeField(blank=True, null=True),
+        ),
+    ]

RIGS/migrations/0037_approve_legacy.py

@@ -0,0 +1,19 @@
+# Generated by Django 2.0.13 on 2020-01-11 18:29
+# This migration ensures that legacy Profiles from before approvals were implemented are automatically approved 
+from django.db import migrations
+
+def approve_legacy(apps, schema_editor):
+    Profile = apps.get_model('RIGS', 'Profile')
+    for person in Profile.objects.all():
+        person.is_approved = True
+        person.save()
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('RIGS', '0036_profile_is_approved'),
+    ]
+
+    operations = [
+        migrations.RunPython(approve_legacy)
+    ]

RIGS/models.py

@@ -27,6 +27,8 @@ class Profile(AbstractUser):
     initials = models.CharField(max_length=5, unique=True, null=True, blank=False)
     phone = models.CharField(max_length=13, null=True, blank=True)
     api_key = models.CharField(max_length=40, blank=True, editable=False, null=True)
+    is_approved = models.BooleanField(default=False)
+    last_emailed = models.DateTimeField(blank=True, null=True)  # Currently only populated by the admin approval email. TODO: Populate it each time we send any email, might need that...
 
     @classmethod
     def make_api_key(cls):
@@ -53,6 +55,14 @@ class Profile(AbstractUser):
     def latest_events(self):
         return self.event_mic.order_by('-start_date').select_related('person', 'organisation', 'venue', 'mic')
 
+    @classmethod
+    def admins(cls):
+        return Profile.objects.filter(email__in=[y for x in settings.ADMINS for y in x])
+
+    @classmethod
+    def users_awaiting_approval_count(cls):
+        return Profile.objects.filter(models.Q(is_approved=False)).count()
+
     def __str__(self):
         return self.name
 

RIGS/signals.py

@@ -1,3 +1,4 @@
+import datetime
 import re
 import urllib.request
 import urllib.error
@@ -10,6 +11,9 @@ from django.conf import settings
 from django.contrib.staticfiles.storage import staticfiles_storage
 from django.core.mail import EmailMessage, EmailMultiAlternatives
 from django.template.loader import get_template
+from django.urls import reverse
+from django.utils import timezone
+from registration.signals import user_activated
 from premailer import Premailer
 from z3c.rml import rml2pdf
 
@@ -102,3 +106,35 @@ def on_revision_commit(sender, instance, created, **kwargs):
 
 
 post_save.connect(on_revision_commit, sender=models.EventAuthorisation)
+
+
+def send_admin_awaiting_approval_email(user, request, **kwargs):
+    # Bit more controlled than just emailing all superusers
+    for admin in models.Profile.admins():
+        # Check we've ever emailed them before and if so, if cooldown has passed.
+        if admin.last_emailed is None or admin.last_emailed + settings.EMAIL_COOLDOWN <= timezone.now():
+            context = {
+                'request': request,
+                'link_suffix': reverse("admin:RIGS_profile_changelist") + '?is_approved__exact=0',
+                'number_of_users': models.Profile.users_awaiting_approval_count(),
+                'to_name': admin.first_name
+            }
+
+            email = EmailMultiAlternatives(
+                "%s new users awaiting approval on RIGS" % (context['number_of_users']),
+                get_template("RIGS/admin_awaiting_approval.txt").render(context),
+                to=[admin.email],
+                reply_to=[user.email],
+            )
+            css = staticfiles_storage.path('css/email.css')
+            html = Premailer(get_template("RIGS/admin_awaiting_approval.html").render(context),
+                             external_styles=css).transform()
+            email.attach_alternative(html, 'text/html')
+            email.send()
+
+            # Update last sent
+            admin.last_emailed = timezone.now()
+            admin.save()
+
+
+user_activated.connect(send_admin_awaiting_approval_email)

RIGS/templates/RIGS/admin_awaiting_approval.html

@@ -0,0 +1,9 @@
+{% extends 'base_client_email.html' %}
+
+{% block content %}
+    <p>Hi {{ to_name|default_if_none:"Administrator" }},</p>
+
+    <p>{{ number_of_users|default_if_none:"Some" }} new users are awaiting administrator approval on RIGS. Click <a href="{{ request.scheme }}://{{ request.get_host }}{{ link_suffix }}">here</a> to  approve them.</p>
+
+    <p>TEC PA &amp; Lighting</p>
+{% endblock %}

RIGS/templates/RIGS/admin_awaiting_approval.txt

@@ -0,0 +1,5 @@
+Hi {{ to_name|default_if_none:"Administrator" }},
+
+{{ number_of_users|default_if_none:"Some" }} new users are awaiting administrator approval on RIGS. Use this link to approve them: {{ request.scheme }}://{{ request.get_host }}/{{ link_suffix }}
+
+TEC PA & Lighting

RIGS/templates/RIGS/event_detail.html

@@ -10,12 +10,14 @@
                 | {{ object.name }} {% if event.dry_hire %}<span class="badge">Dry Hire</span>{% endif %}
             </h1>
     </div>
+        {% if perms.RIGS.view_event %}
         <div class="col-sm-12 text-right">
             {% include 'RIGS/event_detail_buttons.html' %}
         </div>
+        {% endif %}
 
     {% endif %}
-    {% if object.is_rig %}
+    {% if object.is_rig and perms.RIGS.view_event %}
         {# only need contact details for a rig #}
         <div class="col-sm-12 col-md-6 col-lg-5">
             <div class="panel panel-default">
@@ -72,7 +74,7 @@
             {% endif %}
         </div>
     {% endif %}
-    <div class="col-sm-12 {% if event.is_rig %}col-md-6 col-lg-7{% endif %}">
+    <div class="col-sm-12 {% if event.is_rig and perms.RIGS.view_event %}col-md-6 col-lg-7{% endif %}">
         <div class="panel panel-info">
             <div class="panel-heading">Event Info</div>
             <div class="panel-body">
@@ -147,7 +149,7 @@
                         <dd>{{ object.collector }}</dd>
                     {% endif %}
 
-                    {% if event.is_rig and not event.internal %}
+                    {% if event.is_rig and not event.internal and perms.RIGS.view_event %}
                         <dd>&nbsp;</dd>
                         <dt>PO</dt>
                         <dd>{{ object.purchase_order }}</dd>
@@ -156,7 +158,7 @@
             </div>
         </div>
     </div>
-    {% if event.is_rig and event.internal %}
+    {% if event.is_rig and event.internal and perms.RIGS.view_event %}
     <div class="col-sm-12">
         <div class="panel panel-default
             {% if object.authorised %}
@@ -212,7 +214,7 @@
         </div>
     <div>
     {% endif %}
-    {% if not request.is_ajax %}
+    {% if not request.is_ajax and perms.RIGS.view_event %}
     <div class="col-sm-12 text-right">
         {% include 'RIGS/event_detail_buttons.html' %}
     </div>
@@ -222,21 +224,23 @@
             <div class="panel panel-default">
                 <div class="panel-heading">Event Details</div>
                 <div class="panel-body">
+                    {% if perms.RIGS.view_event %}
                     <div class="well well-sm">
                         <h4>Notes</h4>
                         <div class="dont-break-out">{{ event.notes|linebreaksbr }}</div>
                     </div>
+                    {% endif %}
                     {% include 'RIGS/item_table.html' %}
                 </div>
             </div>
         </div>
-        {% if not request.is_ajax %}
+        {% if not request.is_ajax and perms.RIGS.view_event %}
         <div class="col-sm-12 text-right">
             {% include 'RIGS/event_detail_buttons.html' %}
         </div>
         {% endif %}
     {% endif %}
-    {% if not request.is_ajax %}
+    {% if not request.is_ajax and perms.RIGS.view_event %}
         <div class="col-sm-12 text-right">
             <div>
                 <a href="{% url 'event_history' object.pk %}" title="View Revision History">
@@ -251,12 +255,16 @@
 {% if request.is_ajax %}
     {% block footer %}
         <div class="row">
+            {% if perms.RIGS.view_event %}
             <div class="col-sm-10 align-left">
                 <a href="{% url 'event_history' object.pk %}" title="View Revision History">
                     Last edited at {{ object.last_edited_at|default:'never' }} by {{ object.last_edited_by.name|default:'nobody' }}
                 </a>
             </div>
             <div class="col-sm-2">
+            {% else %}
+            <div class="col-sm-12">
+            {% endif %}
                 <div class="pull-right">
                     <a href="{% url 'event_detail' object.pk %}" class="btn btn-primary">Open Event Page <span
                             class="glyphicon glyphicon-eye"></span></a>

RIGS/templates/RIGS/event_embed.html

@@ -6,7 +6,7 @@
 <div class="row">
     <div class="col-sm-12">
         <a href="/">
-            <span class="source"> R<small>ig</small> I<small>nformation</small> G<small>athering</small> S<small>ystem</small></span> 
+            <span class="source"> R<small>ig</small> I<small>nformation</small> G<small>athering</small> S<small>ystem</small></span>
         </a>
     </div>
 
@@ -20,9 +20,9 @@
                 <span class="glyphicon glyphicon-exclamation-sign"></span>
             {% endif %}
         </span>
-        
+
         <h3>
-            <a {% if perms.RIGS.view_event %}href="{% url 'event_detail' object.pk %}"{% endif %}>
+            <a href="{% url 'event_detail' object.pk %}">
                 {% if object.is_rig %}N{{ object.pk|stringformat:"05d" }}{% else %}{{ object.pk }}{% endif %}
                 | {{ object.name }} </a>
                 {% if object.venue %}
@@ -72,7 +72,7 @@
                 </p>
             </div>
             <div class="col-xs-6">
-                
+
                 {% if object.meet_at %}
                     <p>
                         <strong>Crew meet:</strong>
@@ -97,7 +97,7 @@
             {{ object.description|linebreaksbr }}
             </p>
         {% endif %}
-            
+
         </table>
     </div>
 </div>

RIGS/templates/RIGS/event_table.html

@@ -33,7 +33,7 @@
                 </td>
                 <td>
                     <h4>
-                        <a {% if perms.RIGS.view_event %}href="{% url 'event_detail' event.pk %}" {% endif %}>
+                        <a href="{% url 'event_detail' event.pk %}">
                             {{ event.name }}
                         </a>
                         {% if event.venue %}

RIGS/templates/RIGS/index.html

@@ -11,7 +11,7 @@
     </div>
     <div class="row">
     <div class="col-sm-{% if perms.RIGS.view_event %}6{% else %}12{% endif %}">
-        
+
         <div class="panel panel-default">
             <div class="panel-heading">
                  <h4 class="list-group-item-heading">Quick Links</h4>
@@ -26,10 +26,11 @@
 
                     <a class="list-group-item" href="https://forum.nottinghamtec.co.uk" target="_blank"><span class="glyphicon glyphicon-link"></span> TEC Forum</a>
                     <a class="list-group-item" href="//members.nottinghamtec.co.uk/wiki" target="_blank"><span class="glyphicon glyphicon-link"></span> TEC Wiki</a>
+                    {% if perms.RIGS.view_event %}
                     <a class="list-group-item" href="http://members.nottinghamtec.co.uk/wiki/images/2/22/Event_Risk_Assesment.pdf" target="_blank"><span class="glyphicon glyphicon-link"></span> Pre-Event Risk Assessment</a>
                     <a class="list-group-item" href="//members.nottinghamtec.co.uk/price" target="_blank"><span class="glyphicon glyphicon-link"></span> Price List</a>
                     <a class="list-group-item" href="https://goo.gl/forms/jdPWov8PCNPoXtbn2" target="_blank"><span class="glyphicon glyphicon-link"></span> Subhire Insurance Form</a>
-                
+                    {% endif %}
             </div>
         </div>
 
@@ -73,7 +74,7 @@
     </div>
     {% if perms.RIGS.view_event %}
     <div class="col-sm-6">
-        {% include 'RIGS/activity_feed.html' %}    
+        {% include 'RIGS/activity_feed.html' %}
     </div>
     {% endif %}
 </div>

RIGS/templates/RIGS/item_row.html

@@ -6,17 +6,21 @@
             <em class="description">{{item.description|linebreaksbr}}</em>
         </div>
 	</td>
+    {% if perms.RIGS.view_event %}
 	<td>£&nbsp;<span class="cost">{{item.cost|floatformat:2}}</span></td>
+    {% endif %}
 	<td class="quantity">{{item.quantity}}</td>
+    {% if perms.RIGS.view_event %}
 	<td>£&nbsp;<span class="sub-total" data-subtotal="{{item.total_cost}}">{{item.total_cost|floatformat:2}}</span></td>
+    {% endif %}
 	{% if edit %}
 	<td class="vert-align text-right">
-		<button type="button" class="item-edit btn btn-xs btn-default" 
+		<button type="button" class="item-edit btn btn-xs btn-default"
 				data-pk="{{item.pk}}"
 				data-toggle="modal" data-target="#itemModal">
 			<span class="glyphicon glyphicon-edit"></span>
 		</button>
-		<button type="button" class="item-delete btn btn-xs btn-danger" 
+		<button type="button" class="item-delete btn btn-xs btn-danger"
 				data-pk="{{item.pk}}">
 			<span class="glyphicon glyphicon-remove"></span>
 		</button>

RIGS/templates/RIGS/item_table.html

@@ -3,9 +3,13 @@
 		<thead>
 			<tr>
 				<td>Item</td>
+                {% if perms.RIGS.view_event %}
 				<td>Price</td>
+                {% endif %}
 				<td>Quantity</td>
+                {% if perms.RIGS.view_event %}
 				<td>Sub-total</td>
+                {% endif %}
 				{% if edit %}
 				<td class="text-right">
 					<button type="button" class="btn btn-default btn-xs item-add"
@@ -22,6 +26,7 @@
 				{% include 'RIGS/item_row.html' %}
 			{% endfor %}
 		</tbody>
+        {% if perms.RIGS.view_event %}
 		<tfoot>
 			<tr>
 				<td rowspan="3" colspan="2"></td>
@@ -43,6 +48,7 @@
 				<td colspan="2">£ <span id="total">{{object.total|default:0|floatformat:2}}</span></td>
 			</tr>
 		</tfoot>
+        {% endif %}
 	</table>
 </div>
 <table class="hidden invisible">

RIGS/test_functional.py

@@ -141,18 +141,41 @@ class UserRegistrationTest(LiveServerTestCase):
         self.assertEqual(password.get_attribute('placeholder'), 'Password')
         self.assertEqual(password.get_attribute('type'), 'password')
 
+        # Expected to fail as not approved
         username.send_keys('TestUsername')
         password.send_keys('correcthorsebatterystaple')
         self.browser.execute_script(
             "return function() {jQuery('#g-recaptcha-response').val('PASSED'); return 0}()")
         password.send_keys(Keys.ENTER)
 
+        # Test approval
+        profileObject = models.Profile.objects.all()[0]
+        self.assertFalse(profileObject.is_approved)
+
+        # Read what the error is
+        alert = self.browser.find_element_by_css_selector(
+            'div.alert-danger').text
+        self.assertIn("approved", alert)
+
+        # Approve the user so we can proceed
+        profileObject.is_approved = True
+        profileObject.save()
+
+        # Retry login
+        self.browser.get(self.live_server_url + '/user/login')
+        username = self.browser.find_element_by_id('id_username')
+        username.send_keys('TestUsername')
+        password = self.browser.find_element_by_id('id_password')
+        password.send_keys('correcthorsebatterystaple')
+        self.browser.execute_script(
+            "return function() {jQuery('#g-recaptcha-response').val('PASSED'); return 0}()")
+        password.send_keys(Keys.ENTER)
+
         # Check we are logged in
         udd = self.browser.find_element_by_class_name('navbar').text
         self.assertIn('Hi John', udd)
 
         # Check all the data actually got saved
-        profileObject = models.Profile.objects.all()[0]
         self.assertEqual(profileObject.username, 'TestUsername')
         self.assertEqual(profileObject.first_name, 'John')
         self.assertEqual(profileObject.last_name, 'Smith')

RIGS/urls.py

@@ -6,7 +6,7 @@ from RIGS import models, views, rigboard, finance, ical, versioning, forms
 from django.views.generic import RedirectView
 from django.views.decorators.clickjacking import xframe_options_exempt
 
-from PyRIGS.decorators import permission_required_with_403
+from PyRIGS.decorators import permission_required_with_403, has_oembed
 from PyRIGS.decorators import api_key_required
 
 urlpatterns = [
@@ -87,8 +87,7 @@ urlpatterns = [
         permission_required_with_403('RIGS.view_event')(versioning.ActivityFeed.as_view()),
         name='activity_feed'),
 
-    url(r'^event/(?P<pk>\d+)/$',
+    url(r'^event/(?P<pk>\d+)/$', has_oembed(oembed_view="event_oembed")(
-        permission_required_with_403('RIGS.view_event', oembed_view="event_oembed")(
             rigboard.EventDetail.as_view()),
         name='event_detail'),
     url(r'^event/(?P<pk>\d+)/embed/$',

RIGS/views.py

@@ -41,7 +41,7 @@ def login(request, **kwargs):
     else:
         from django.contrib.auth.views import login
 
-        return login(request)
+        return login(request, authentication_form=forms.CheckApprovedForm)
 
 
 # This view should be exempt from requiring CSRF token.

templates/registration/activation_complete.html

@@ -5,6 +5,6 @@
 {% block content %}
 <div class="alert alert-success">
 	<h2>Activation Complete</h2>
-	<p>You user account is now fully registered. Enjoy RIGS</p>
+	<p>Your user account is now awaiting administrator approval. Won't be long!</p>
 </div>
-{% endblock %}
+{% endblock %}