Merge pull request #3470 from alphagov/enforce-service-permissions

Enforce correct naming of service permissions

app/models/service.py

@@ -39,7 +39,6 @@ class Service(JSONModel):
         'letter_contact_block',
         'message_limit',
         'name',
-        'permissions',
         'prefix_sms',
         'research_mode',
         'service_callback_api',
@@ -58,16 +57,24 @@ class Service(JSONModel):
         'letter',
     )
 
-    def __init__(self, _dict):
-
-        super().__init__(_dict)
-        if 'permissions' not in self._dict:
-            self.permissions = {'email', 'sms', 'letter'}
+    ALL_PERMISSIONS = TEMPLATE_TYPES + (
+        'edit_folder_permissions',
+        'email_auth',
+        'inbound_sms',
+        'international_letters',
+        'international_sms',
+        'upload_document',
+        'upload_letters',
+    )
 
     @classmethod
     def from_id(cls, service_id):
         return cls(service_api_client.get_service(service_id)['data'])
 
+    @property
+    def permissions(self):
+        return self._dict.get('permissions', self.TEMPLATE_TYPES)
+
     def update(self, **kwargs):
         return service_api_client.update_service(self.id, **kwargs)
 
@@ -106,6 +113,8 @@ class Service(JSONModel):
         return not self.trial_mode
 
     def has_permission(self, permission):
+        if permission not in self.ALL_PERMISSIONS:
+            raise KeyError(f'{permission} is not a service permission')
         return permission in self.permissions
 
     def get_page_of_jobs(self, page):

tests/app/models/test_service.py

@@ -1,5 +1,7 @@
 import uuid
 
+import pytest
+
 from app.models.organisation import Organisation
 from app.models.service import Service
 from app.models.user import User
@@ -247,3 +249,9 @@ def test_service_without_organisation_doesnt_need_org_api(mocker, service_one):
 
     assert mock_redis_get.called is False
     assert mock_get_organisation.called is False
+
+
+def test_bad_permission_raises(service_one):
+    with pytest.raises(KeyError) as e:
+        Service(service_one).has_permission('foo')
+    assert str(e.value) == "'foo is not a service permission'"