Various fixes (#95)

* Fix example_setup with docker

* xml validation local_metadata in form

* refresh metadata on changed field content

* update version number for fix release

* select_related in admin

CHANGELOG.md

@@ -1,8 +1,6 @@
 # Changelog
 
-## [0.7.0] - 2020-04-xx
-
-*In development, *
+## [0.7.2] - 2020-04-14
 
 Many thanks for major contributions (especially on the testing front where the project was lacking) from [Amertz08](https://github.com/Amertz08) and [askvortsov1](https://github.com/askvortsov1)
 
@@ -12,7 +10,7 @@ Many thanks for major contributions (especially on the testing front where the p
 - Django 3.0 is added to the tests matrix. We currently are doing Python 3.6, 3.7, 3.8 and Django 2.2, 3.0.
 
 ### Removed
-- Python 3.5
+- Dropped Python 3.5.
 - Django 2.0 and 2.1 as they are no longer officially supported Django versions.
 
 

djangosaml2idp/__init__.py

@@ -1 +1 @@
-__version__ = '0.7.0'
+__version__ = '0.7.2'

djangosaml2idp/admin.py

@@ -31,6 +31,4 @@ class ServiceProviderAdmin(admin.ModelAdmin):
 class PersistentIdAdmin(admin.ModelAdmin):
     list_filter = ['sp', ]
     list_display = ['user', 'sp', 'persistent_id']
-
-    def get_queryset(self, request):
-        return super().get_queryset(request).select_related('user', 'sp')
+    select_related = ['user', 'sp']

djangosaml2idp/forms.py

@@ -6,6 +6,7 @@
 
 from .models import ServiceProvider
 from .processors import instantiate_processor, validate_processor_path
+from .utils import validate_metadata
 
 boolean_form_select_choices = ((None, _('--------')), (True, _('Yes')), (False, _('No')))
 
@@ -39,6 +40,11 @@ def clean__processor(self):
         validate_processor_path(value)
         return value
 
+    def clean_local_metadata(self):
+        value = self.cleaned_data['local_metadata']
+        validate_metadata(value)
+        return value
+
     def clean(self):
         cleaned_data = super().clean()
 

djangosaml2idp/migrations/0001_initial.py

@@ -18,7 +18,7 @@ class Migration(migrations.Migration):
                 ('dt_created', models.DateTimeField(auto_now_add=True, verbose_name='Created at')),
                 ('dt_updated', models.DateTimeField(auto_now=True, null=True, verbose_name='Updated at')),
                 ('entity_id', models.CharField(max_length=255, unique=True, verbose_name='Entity ID')),
-                ('pretty_name', models.CharField(blank=True, help_text='For display purposes, can be empty', max_length=256, verbose_name='Pretty Name')),
+                ('pretty_name', models.CharField(blank=True, help_text='For display purposes, can be empty', max_length=255, verbose_name='Pretty Name')),
                 ('description', models.TextField(blank=True, verbose_name='Description')),
                 ('metadata_expiration_dt', models.DateTimeField(verbose_name='Metadata valid until')),
                 ('remote_metadata_url', models.CharField(blank=True, help_text='If set, metadata will be fetched upon saving into the local metadata xml field, and automatically be refreshed after the expiration timestamp.', max_length=512, verbose_name='Remote metadata URL')),

djangosaml2idp/models.py

@@ -51,11 +51,25 @@ class ServiceProvider(models.Model):
     remote_metadata_url = models.CharField(verbose_name='Remote metadata URL', max_length=512, blank=True, help_text='If set, metadata will be fetched upon saving into the local metadata xml field, and automatically be refreshed after the expiration timestamp.')
     local_metadata = models.TextField(verbose_name='Local Metadata XML', blank=True, help_text='XML containing the metadata')
 
+    @classmethod
+    def from_db(cls, db, field_names, values):
+        instance = super().from_db(db, field_names, values)
+        instance._loaded_db_values = dict(zip(field_names, values))
+        return instance
+
+    def field_value_changed(self, field_name: str) -> bool:
+        ''' Returns whether the current value of a field is changed vs what was loaded from the db. '''
+        current_value = getattr(self, field_name)
+        return current_value != getattr(self, '_loaded_db_values', {}).get(field_name, current_value)
+
     def _should_refresh(self) -> bool:
         ''' Returns whether or not a refresh operation is necessary.
         '''
-        # - Data was not fetched ever before, so local_metadata is empty
-        if not self.local_metadata:
+        # - Data was not fetched ever before, so local_metadata is empty, or local_metadata has been changed from what it was in the db before
+        if not self.local_metadata or self.field_value_changed('local_metadata'):
+            return True
+        # - The remote url has been updated
+        if self.field_value_changed('remote_metadata_url'):
             return True
         # - The expiration timestamp is not set, or it is expired
         if not self.metadata_expiration_dt or now() > self.metadata_expiration_dt:
@@ -104,7 +118,7 @@ def refresh_metadata(self, force_refresh: bool = False) -> bool:
         if self.remote_metadata_url:
             return self._refresh_from_remote()
 
-        if (not self.metadata_expiration_dt) or (now() > self.metadata_expiration_dt):
+        if force_refresh or (not self.metadata_expiration_dt) or (now() > self.metadata_expiration_dt) or self.field_value_changed('local_metadata'):
             return self._refresh_from_local()
 
         raise Exception('Uncaught case of refresh_metadata')

example_setup/docker-compose.yml

@@ -20,7 +20,7 @@ services:
     stdin_open: true
     tty: true
     volumes:
-      - ./idp:/app
-    working_dir: /app
+      - ..:/app
+    working_dir: /app/example_setup/idp
     ports:
       - "9000:9000"

example_setup/idp/Dockerfile

@@ -1,4 +1,4 @@
-FROM python:3.6.6-alpine3.7
+FROM python:3.8-alpine
 
 RUN apk add --update \
     build-base libffi-dev openssl-dev \

example_setup/idp/requirements.txt

@@ -1,4 +1,4 @@
-django<3.0
+django~=3.0
 pysaml2==5.0.0
 arrow
 pytz

example_setup/sp/Dockerfile

@@ -1,4 +1,4 @@
-FROM python:3.6.6-alpine3.7
+FROM python:3.8-alpine
 
 RUN apk add --update \
     build-base libffi-dev openssl-dev \

example_setup/sp/requirements.txt

@@ -1,5 +1,5 @@
-django<3.0
-djangosaml2==0.17.2
+django~=3.0
+djangosaml2==0.18.1
 pysaml2==5.0.0
 arrow
 pytz

tests/test_models.py

@@ -1,5 +1,6 @@
 import json
 from datetime import timedelta
+from unittest import mock
 
 import arrow
 import pytest
@@ -10,6 +11,8 @@
 from djangosaml2idp.idp import IDP
 from djangosaml2idp.models import DEFAULT_ATTRIBUTE_MAPPING, ServiceProvider
 
+from .testing_utilities import mocked_requests_get
+
 future_dt = arrow.get().shift(days=30)
 expired_dt = arrow.get().shift(days=-30)
 
@@ -90,6 +93,27 @@ def test_refresh_meta_data_returns_false_on_model_state(self):
         )
         assert instance.refresh_metadata() is False
 
+    @pytest.mark.django_db
+    def test_should_refresh_on_changed_local_metadata(self, sp_metadata_xml):
+        ServiceProvider.objects.create(entity_id='entity-id', local_metadata=sp_metadata_xml)
+        instance = ServiceProvider.objects.get(entity_id='entity-id')
+        # By default, no refresh necessary upon loading
+        assert instance._should_refresh() is False
+        # After modifying the local_metadata, refresh is necessary
+        instance.local_metadata = EXPIRED_XML
+        assert instance._should_refresh() is True
+
+    @pytest.mark.django_db
+    @mock.patch('requests.get', side_effect=mocked_requests_get)
+    def test_should_refresh_on_changed_remote_metadata_url(self, mock_get, sp_metadata_xml):
+        ServiceProvider.objects.create(entity_id='entity-id', remote_metadata_url='https://ok', local_metadata=sp_metadata_xml)
+        instance = ServiceProvider.objects.get(entity_id='entity-id')
+        # By default, no refresh necessary upon loading
+        assert instance._should_refresh() is False
+        # After modifying the remote_metadata_url, refresh is necessary
+        instance.remote_metadata_url = 'https://new-ok'
+        assert instance._should_refresh() is True
+
     @pytest.mark.parametrize(
         "instance",
         (