Move new style service configuration to MAMA_CAS_SERVICES

mama_cas/cas.py

@@ -1,7 +1,5 @@
 import logging
-import warnings
 
-from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth import logout
 from django.utils.module_loading import import_string
@@ -98,18 +96,9 @@ def get_attributes(user, service):
     callback functions.
     """
     attributes = {}
-
-    callbacks = list(getattr(settings, 'MAMA_CAS_ATTRIBUTE_CALLBACKS', []))
-    if callbacks:
-        warnings.warn(
-            'The MAMA_CAS_ATTRIBUTE_CALLBACKS setting is deprecated. Service callbacks '
-            'should be configured using MAMA_CAS_VALID_SERVICES.', DeprecationWarning)
-    callbacks.extend(get_callbacks(service))
-
-    for path in callbacks:
+    for path in get_callbacks(service):
         callback = import_string(path)
         attributes.update(callback(user, service))
-
     return attributes
 
 
@@ -121,11 +110,7 @@ def logout_user(request):
         ProxyTicket.objects.consume_tickets(request.user)
         ProxyGrantingTicket.objects.consume_tickets(request.user)
 
-        if getattr(settings, 'MAMA_CAS_ENABLE_SINGLE_SIGN_OUT', True):
-            warnings.warn(
-                'The MAMA_CAS_ENABLE_SINGLE_SIGN_OUT setting is deprecated. SLO '
-                'should be configured using MAMA_CAS_VALID_SERVICES.', DeprecationWarning)
-            ServiceTicket.objects.request_sign_out(request.user)
+        ServiceTicket.objects.request_sign_out(request.user)
 
         logger.info("Single sign-on session ended for %s" % request.user)
         logout(request)

mama_cas/services/__init__.py

@@ -1,3 +1,7 @@
+import re
+import warnings
+
+from django.conf import settings
 from django.utils.module_loading import import_string
 
 
@@ -20,10 +24,15 @@ def _is_allowed(attr, *args):
 
 
 def get_callbacks(service):
-    callbacks = []
+    callbacks = list(getattr(settings, 'MAMA_CAS_ATTRIBUTE_CALLBACKS', []))
+    if callbacks:
+        warnings.warn(
+            'The MAMA_CAS_ATTRIBUTE_CALLBACKS setting is deprecated. Service callbacks '
+            'should be configured using MAMA_CAS_SERVICES.', DeprecationWarning)
+
     for backend in _get_backends():
         try:
-            callbacks = callbacks + backend.get_callbacks(service)
+            callbacks.extend(backend.get_callbacks(service))
         except AttributeError:
             raise NotImplementedError("%s does not implement get_callbacks()" % backend)
     return callbacks
@@ -39,7 +48,14 @@ def get_logout_url(service):
 
 
 def logout_allowed(service):
-    return _is_allowed('logout_allowed', service)
+    if getattr(settings, 'MAMA_CAS_SERVICES', {}):
+        return _is_allowed('logout_allowed', service)
+
+    if getattr(settings, 'MAMA_CAS_ENABLE_SINGLE_SIGN_OUT', False):
+        warnings.warn(
+            'The MAMA_CAS_ENABLE_SINGLE_SIGN_OUT setting is deprecated. SLO '
+            'should be configured using MAMA_CAS_SERVICES.', DeprecationWarning)
+        return True
 
 
 def proxy_allowed(service):
@@ -51,4 +67,16 @@ def proxy_callback_allowed(service, pgturl):
 
 
 def service_allowed(service):
-    return _is_allowed('service_allowed', service)
+    if getattr(settings, 'MAMA_CAS_SERVICES', {}):
+        return _is_allowed('service_allowed', service)
+
+    valid_services = getattr(settings, 'MAMA_CAS_VALID_SERVICES', ())
+    if not valid_services:
+        return True
+    warnings.warn(
+        'The MAMA_CAS_VALID_SERVICES setting is deprecated. Services '
+        'should be configured using MAMA_CAS_SERVICES.', DeprecationWarning)
+    for identifier in [re.compile(s) for s in valid_services]:
+        if identifier.match(service):
+            return True
+    return False

mama_cas/services/backends.py

@@ -1,9 +1,7 @@
 import re
-import warnings
 
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
-from django.utils import six
 from django.utils.functional import cached_property
 
 
@@ -17,21 +15,14 @@ class ServiceConfig(object):
     def services(self):
         services = []
 
-        for service in getattr(settings, 'MAMA_CAS_VALID_SERVICES', []):
-            if isinstance(service, six.string_types):
-                warnings.warn(
-                    'Service URL configuration is changing. Check the documentation '
-                    'for the MAMA_CAS_VALID_SERVICES setting.', DeprecationWarning)
-                match = re.compile(service)
-                service = {'SERVICE': service}
-            else:
-                service = service.copy()
-                try:
-                    match = re.compile(service['SERVICE'])
-                except KeyError:
-                    raise ImproperlyConfigured(
-                        'Missing SERVICE key for service configuration. '
-                        'Check your MAMA_CAS_VALID_SERVICES setting.')
+        for service in getattr(settings, 'MAMA_CAS_SERVICES', []):
+            service = service.copy()
+            try:
+                match = re.compile(service['SERVICE'])
+            except KeyError:
+                raise ImproperlyConfigured(
+                    'Missing SERVICE key for service configuration. '
+                    'Check your MAMA_CAS_VALID_SERVICES setting.')
 
             service['MATCH'] = match
             # TODO For transitional backwards compatibility, this defaults to True.
@@ -54,7 +45,10 @@ def get_service(self, s):
         return {}
 
     def get_config(self, service, setting):
-        """Access the configuration for a given service and setting."""
+        """
+        Access the configuration for a given service and setting. If the
+        service is not found, return a default value.
+        """
         try:
             return self.get_service(service)[setting]
         except KeyError:
@@ -83,17 +77,13 @@ def proxy_allowed(self, service):
         return services.get_config(service, 'PROXY_ALLOW')
 
     def proxy_callback_allowed(self, service, pgturl):
-        """
-        """
         try:
             return services.get_config(service, 'PROXY_PATTERN').match(pgturl)
         except AttributeError:
             # TODO For transitional backwards compatibility, check against valid services
             return self.service_allowed(pgturl)
 
     def service_allowed(self, service):
-        """
-        """
         if not service:
             return False
         return services.is_valid(service)

mama_cas/tests/settings.py

@@ -50,7 +50,7 @@
     'mama_cas',
 )
 
-MAMA_CAS_VALID_SERVICES = [
+MAMA_CAS_SERVICES = [
     {
         'SERVICE': 'https?://.+\.example\.com',
         'PROXY_ALLOW': True,

mama_cas/tests/test_services.py

@@ -46,7 +46,7 @@ def test_logout_allowed(self):
         self.assertFalse(logout_allowed('http://example.com'))
         self.assertFalse(logout_allowed('http://www.example.org'))
 
-    @modify_settings(MAMA_CAS_VALID_SERVICES={
+    @modify_settings(MAMA_CAS_SERVICES={
         'append': [{'SERVICE': 'http://example\.org/proxy'}]
     })
     def test_proxy_allowed(self):
@@ -71,6 +71,7 @@ def test_proxy_callback_allowed(self):
         self.assertFalse(proxy_callback_allowed('http://example.org', 'http://example.org'))
 
     @override_settings(MAMA_CAS_VALID_SERVICES=('http://.*\.example\.com',))
+    @override_settings(MAMA_CAS_SERVICES=[])
     def test_service_allowed_tuple(self):
         """
         When valid services are configured, ``service_allowed()``
@@ -90,23 +91,24 @@ def test_service_allowed(self):
         self.assertFalse(service_allowed('http://www.example.org'))
 
     @override_settings(MAMA_CAS_VALID_SERVICES=())
+    @override_settings(MAMA_CAS_SERVICES=[])
     def test_empty_valid_services_tuple(self):
         """
         When no valid services are configured,
         ``service_allowed()`` should return ``True``.
         """
         self.assertTrue(service_allowed('http://www.example.com'))
 
-    @override_settings(MAMA_CAS_VALID_SERVICES=[])
-    def test_empty_valid_services(self):
+    @override_settings(MAMA_CAS_SERVICES=[])
+    def test_empty_services(self):
         """
         When no valid services are configured,
         ``service_allowed()`` should return ``True``.
         """
         self.assertTrue(service_allowed('http://www.example.com'))
 
-    @override_settings(MAMA_CAS_VALID_SERVICES=[{}])
-    def test_invalid_valid_services(self):
+    @override_settings(MAMA_CAS_SERVICES=[{}])
+    def test_invalid_services(self):
         """
         When invalid services are configured, ``service_allowed()``
         should raise ``ImproperlyConfigured``.