Merge pull request #334 from MAKENTNU/dev

Deployment

.github/workflows/python.yml

@@ -29,7 +29,7 @@ jobs:
 
       - name: Install System Dependencies
         run: |
-          sudo apt install python-dev libldap2-dev libsasl2-dev libssl-dev
+          sudo apt update && sudo apt install python-dev libldap2-dev libsasl2-dev libssl-dev
 
       - name: Install Python Dependencies
         run: |

announcements/migrations/0003_use_timezone_aware_default.py

@@ -0,0 +1,19 @@
+# Generated by Django 3.0.11 on 2020-11-10 16:21
+
+from django.db import migrations, models
+import django.utils.timezone
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('announcements', '0002_help_text_and_naming'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='announcement',
+            name='display_from',
+            field=models.DateTimeField(default=django.utils.timezone.localtime, help_text='The date from which the announcement will be shown.', verbose_name='Display from'),
+        ),
+    ]

announcements/models.py

@@ -10,8 +10,8 @@ class AnnouncementManager(models.Manager):
 
     def valid(self):
         """Finds all announcements that are currently valid"""
-        return self.filter(display_from__lte=timezone.localtime().now()).filter(
-            Q(display_to__isnull=True) | Q(display_to__gt=timezone.localtime().now()))
+        return self.filter(display_from__lte=timezone.localtime()).filter(
+            Q(display_to__isnull=True) | Q(display_to__gt=timezone.localtime()))
 
     def valid_site_wide(self):
         """Finds all currently valid announcements that should be displayed site-wide"""
@@ -42,7 +42,7 @@ class AnnouncementType(models.TextChoices):
     content = MultiLingualTextField(max_length=256, verbose_name=_("Content"))
     link = models.CharField(max_length=2048, verbose_name=_("Link"), blank=True, null=True,
                             help_text=_("An optional link to an information page."))
-    display_from = models.DateTimeField(default=timezone.localtime().now, verbose_name=_("Display from"),
+    display_from = models.DateTimeField(default=timezone.localtime, verbose_name=_("Display from"),
                                         help_text=_("The date from which the announcement will be shown."))
     display_to = models.DateTimeField(blank=True, null=True, verbose_name=_("Display to"),
                                       help_text=_("The announcement will be shown until this date. If none is given, it"

dataporten/tests.py

@@ -1,9 +1,29 @@
+import uuid
+from typing import Tuple
+
 from django.contrib.auth import get_user
+from django.http import HttpRequest
 from django.test import TestCase
 from django.urls import reverse
+from social_django.models import UserSocialAuth
 
 from dataporten.social import DataportenOAuth2
 from users.models import User
+from util.test_utils import mock_module_attrs
+from . import views
+
+
+def mock_complete(*args, **kwargs):
+    return "mock response"
+
+
+ldap_data_return_value = {
+    'username': "",
+}
+
+
+def mock_get_user_details_from_email(*args, **kwargs):
+    return ldap_data_return_value
 
 
 class ViewTestCase(TestCase):
@@ -17,6 +37,87 @@ def test_logout(self):
         self.client.get(reverse('logout'))
         self.assertFalse(get_user(self.client).is_authenticated)
 
+    @mock_module_attrs({
+        (views, 'complete'): mock_complete,
+        (views, 'get_user_details_from_email'): mock_get_user_details_from_email,
+    })
+    def test_login_wrapper(self):
+        user1 = self.create_social_user(
+            "user1", "email1", ("", ""), ldap_full_name="", social_data_fullname="Name Nameson",
+        )
+        ldap_data_return_value['username'] = user1.username
+
+        def assert_original_user1_values():
+            self.assert_expected_values_after_login(
+                user1, expected_username="user1", expected_full_name="Name Nameson", expected_ldap_full_name="Name Nameson",
+            )
+
+        fixed_num_queries = 1  # number of queries that are always executed (currently only `user.social_auth`)
+        with self.assertNumQueries(2 + fixed_num_queries):
+            assert_original_user1_values()
+        # All combinations of missing name fields should result in the same values
+        user1.first_name, user1.last_name = "", ""
+        user1.save()
+        with self.assertNumQueries(1 + fixed_num_queries):
+            assert_original_user1_values()
+        user1.ldap_full_name = ""
+        user1.save()
+        with self.assertNumQueries(1 + fixed_num_queries):
+            assert_original_user1_values()
+
+        # Changing first_name or last_name should prevent updating any of them on login
+        user1.first_name = "New Name"
+        user1.save()
+        with self.assertNumQueries(0 + fixed_num_queries):
+            self.assert_expected_values_after_login(
+                user1, expected_username="user1", expected_full_name="New Name Nameson", expected_ldap_full_name="Name Nameson",
+            )
+
+        # When the user's full name and ldap_full_name are equal, they should both be set to social_data['fullname']
+        user2 = self.create_social_user(
+            "user2", "email2", ("Name", "Nameson"), ldap_full_name="Name Nameson", social_data_fullname="New LDAP Name",
+        )
+        ldap_data_return_value['username'] = user2.username
+        with self.assertNumQueries(2 + fixed_num_queries):
+            self.assert_expected_values_after_login(
+                user2, expected_username="user2", expected_full_name="New LDAP Name", expected_ldap_full_name="New LDAP Name",
+            )
+
+        user3 = self.create_social_user(
+            "user3", "email3", ("Name", "Nameson"), ldap_full_name="Name Nameson", social_data_fullname="Name Nameson",
+        )
+        # When the user's username differs from the LDAP data's username, the former should change to match
+        ldap_data_return_value['username'] = "ldap_username3"
+        self.assert_expected_values_after_login(
+            user3, expected_username="ldap_username3", expected_full_name="Name Nameson", expected_ldap_full_name="Name Nameson",
+        )
+        # When the user's username differs from the local-part of the email (and the LDAP data doesn't contain anything),
+        # the username should change to match
+        ldap_data_return_value.pop('username')
+        self.assert_expected_values_after_login(
+            user3, expected_username="email3", expected_full_name="Name Nameson", expected_ldap_full_name="Name Nameson",
+        )
+
+    @staticmethod
+    def create_social_user(username, email_username, first_and_last_name: Tuple[str, str],
+                           *, ldap_full_name, social_data_fullname):
+        first_name, last_name = first_and_last_name
+        user = User.objects.create_user(
+            username=username, email=f"{email_username}@example.com",
+            first_name=first_name, last_name=last_name, ldap_full_name=ldap_full_name,
+        )
+        UserSocialAuth.objects.create(user=user, extra_data={'fullname': social_data_fullname}, uid=uuid.uuid4())
+        return user
+
+    def assert_expected_values_after_login(self, user, *, expected_username, expected_full_name, expected_ldap_full_name):
+        request = HttpRequest()
+        request.user = user
+        response = views.login_wrapper(request, None)
+        self.assertEqual(response, mock_complete())
+        self.assertEqual(user.username, expected_username)
+        self.assertEqual(user.get_full_name(), expected_full_name)
+        self.assertEqual(user.ldap_full_name, expected_ldap_full_name)
+
 
 class DataportenTestCase(TestCase):
 

dataporten/views.py

@@ -2,11 +2,16 @@
 
 from django.conf import settings
 from django.contrib.auth import logout
-from django.http import HttpResponseRedirect, HttpResponseForbidden
+from django.http import HttpResponseForbidden, HttpResponseRedirect
 from django.views import View
-from social_django.views import complete
+from social_django import views as social_views
 
-from dataporten.ldap_utils import get_user_details_from_email
+from users.models import User
+from . import ldap_utils
+
+# Assign these functions to module-level variables, to facilitate testing (through monkey patching)
+complete = social_views.complete
+get_user_details_from_email = ldap_utils.get_user_details_from_email
 
 
 class Logout(View):
@@ -28,23 +33,43 @@ def login_wrapper(request, backend, *args, **kwargs):
     try:
         response = complete(request, backend, *args, **kwargs)
     except Exception as e:
-        logging.exception("Authentication through Dataporten failed.", exc_info=e)
+        logging.getLogger('django.request').exception("Authentication through Dataporten failed.", exc_info=e)
         return HttpResponseForbidden()
 
-    user = request.user
-    data = user.social_auth.first().extra_data
+    user: User = request.user
+    social_data = user.social_auth.first().extra_data
 
-    # Update the full name of the user
-    user.first_name = ' '.join(data['fullname'].split()[:-1])
-    user.last_name = data['fullname'].split()[-1]
+    # If any of the user's names have not been set...
+    if (not user.get_full_name() or not user.ldap_full_name
+            # ...or if the user has not set a different name after account creation:
+            or user.get_full_name().strip() == user.ldap_full_name.strip()):
+        _update_full_name_if_different(user, social_data)
+        _update_ldap_full_name_if_different(user, social_data)
 
     # Try to retrieve username from NTNUs LDAP server. Otherwise use the first part of the email as the username
     ldap_data = get_user_details_from_email(user.email, use_cached=False)
-    if ldap_data:
-        user.username = ldap_data["username"]
-    else:
-        user.username = user.email.split('@')[0]
-
-    user.save()
+    _update_username_if_different(user, ldap_data)
 
     return response
+
+
+def _update_full_name_if_different(user: User, social_data: dict):
+    split_ldap_name = social_data['fullname'].split()
+    old_full_name = user.get_full_name()
+    user.first_name = " ".join(split_ldap_name[:-1])
+    user.last_name = split_ldap_name[-1]
+    if user.get_full_name() != old_full_name:
+        user.save()
+
+
+def _update_ldap_full_name_if_different(user: User, social_data: dict):
+    if user.ldap_full_name != social_data['fullname']:
+        user.ldap_full_name = social_data['fullname']
+        user.save()
+
+
+def _update_username_if_different(user: User, ldap_data: dict):
+    potentially_new_username = ldap_data['username'] if ldap_data else user.email.split("@")[0]
+    if user.username != potentially_new_username:
+        user.username = potentially_new_username
+        user.save()

internal/admin.py

@@ -1,5 +1,13 @@
 from django.contrib import admin
 
-from internal.models import Member
+from internal.models import Member, Secret
+from web.multilingual.database import MultiLingualFieldAdmin
+
+
+class SecretAdmin(MultiLingualFieldAdmin):
+    list_display = ('title', 'last_modified')
+    search_fields = ('title', 'content')
+
 
 admin.site.register(Member)
+admin.site.register(Secret, SecretAdmin)

internal/forms.py

@@ -1,9 +1,10 @@
 from django.forms import ModelForm, TextInput
 from django.utils.translation import gettext_lazy as _
+from django import forms
 
 import card.utils
 from card.forms import CardNumberField
-from internal.models import Member, SystemAccess
+from internal.models import Member, SystemAccess, Secret
 from users.models import User
 from web.widgets import SemanticSearchableChoiceInput, SemanticDateInput, SemanticMultipleSelectInput
 
@@ -83,3 +84,9 @@ def __init__(self, **kwargs):
         self.fields["name"].disabled = True
         self.fields["member"].disabled = True
         self.fields["member"].label_from_instance = lambda member: member.user.get_full_name()
+
+
+class SecretsForm(forms.ModelForm):
+    class Meta:
+        model = Secret
+        fields = "__all__"

internal/migrations/0006_secret.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.0.10 on 2020-11-12 15:25
+
+from django.db import migrations, models
+import web.multilingual.database
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('internal', '0005_remove_member_card_number'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='Secret',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('title', web.multilingual.database.MultiLingualTextField(max_length=100, unique=True, verbose_name='Title')),
+                ('content', web.multilingual.database.MultiLingualRichTextUploadingField(verbose_name='Description')),
+                ('last_modified', models.DateTimeField(auto_now=True)),
+            ],
+        ),
+    ]

internal/models.py

@@ -11,6 +11,8 @@
 from internal.util import date_to_term
 from users.models import User
 
+from web.multilingual.database import MultiLingualRichTextUploadingField, MultiLingualTextField
+
 
 class Member(models.Model):
     class Meta:
@@ -168,3 +170,16 @@ def change_url(self):
 
     def should_be_changed(self):
         return self.name != self.WEBSITE
+
+
+class Secret(models.Model):
+    title = MultiLingualTextField(
+        max_length=100,
+        unique=True,
+        verbose_name=_("Title"),
+    )
+    content = MultiLingualRichTextUploadingField(verbose_name=_("Description"))
+    last_modified = models.DateTimeField(auto_now=True)
+
+    def __str__(self):
+        return str(self.title)

internal/static/internal/css/secrets.css

@@ -0,0 +1,3 @@
+.hidden {
+    display: none !important; /* Overrides Fomantic UI */
+}

internal/static/internal/js/secrets.js

@@ -0,0 +1,13 @@
+const SECRET_SHOW_DURATION_SECONDS = 10;
+
+$(".secret-button").click(function () {
+    const secretButton = $(this);
+    const secretId = secretButton.data("secret-id");
+    const secret = $(`#${secretId}`);
+    secret.removeClass("hidden");
+    secretButton.addClass("hidden");
+    setTimeout(() => {
+        secret.addClass("hidden");
+        secretButton.removeClass("hidden");
+    }, SECRET_SHOW_DURATION_SECONDS * 1000);
+});

internal/templates/internal/header.html

@@ -27,6 +27,10 @@
             <div class="text">{% trans "Members" %}</div>
             <div class="make_bg_blue bubble-background"></div>
         </a>
+         <a class="item" href="{% url "secrets" %}">
+            <div class="text">{% trans "Secrets" %}</div>
+            <div class="make_bg_blue bubble-background"></div>
+        </a>
     </nav>
 
     <div id="side-nav" class="ui huge secondary inverted menu">