Allow org team members to see team and usage

Organisation team members will be ultimately interested in the detailed
usage of each service, but shouldn't necessarily have access to the
personal data of that services users.

So we should allow these organisation team members to navigate to live
services usage page from the organisation page. They may need to contact
the team so they should also be able to view the team members page.

So they'll then see just usage and team members pages.

If they are actually a team member of the service they're viewing, then
they'll see the full range of options as usual.

This commit implement the above by adding an extra flag to the
`user.has_permissions` decorator which allows certain pages to be marked
as viewable by an organisation user. The default (for all other existing
pages) is that organisation users don’t have permission.

app/main/views/dashboard.py

@@ -143,7 +143,7 @@ def template_usage(service_id):
 
 @main.route("/services/<service_id>/usage")
 @login_required
-@user_has_permissions('manage_service')
+@user_has_permissions('manage_service', allow_org_user=True)
 def usage(service_id):
     year, current_financial_year = requested_and_current_financial_year(request)
 

app/main/views/manage_users.py

@@ -31,7 +31,7 @@ from app.utils import is_gov_user, redact_mobile_number, user_has_permissions
 
 @main.route("/services/<service_id>/users")
 @login_required
-@user_has_permissions()
+@user_has_permissions(allow_org_user=True)
 def manage_users(service_id):
     return render_template(
         'views/manage-users.html',

app/models/user.py

@@ -174,7 +174,7 @@ class User(JSONModel, UserMixin):
     def platform_admin(self):
         return self._platform_admin and not session.get('disable_platform_admin_view', False)
 
-    def has_permissions(self, *permissions, restrict_admin_usage=False):
+    def has_permissions(self, *permissions, restrict_admin_usage=False, allow_org_user=False):
         unknown_permissions = set(permissions) - all_permissions
         if unknown_permissions:
             raise TypeError('{} are not valid permissions'.format(list(unknown_permissions)))
@@ -195,12 +195,19 @@ class User(JSONModel, UserMixin):
         if org_id:
             return self.belongs_to_organisation(org_id)
 
-        if not permissions:
-            return self.belongs_to_service(service_id)
+        if not permissions and self.belongs_to_service(service_id):
+            return True
 
-        return any(
+        if any(
             self.has_permission_for_service(service_id, permission)
             for permission in permissions
+        ):
+            return True
+
+        from app.models.service import Service
+
+        return allow_org_user and self.belongs_to_organisation(
+            Service.from_id(service_id).organisation_id
         )
 
     def has_permission_for_service(self, service_id, permission):

app/templates/main_nav.html

@@ -6,15 +6,17 @@
   {% if current_user.has_permissions('view_activity') %}
     <li><a href="{{ url_for('.service_dashboard', service_id=current_service.id) }}" {{ main_navigation.is_selected('dashboard') }}>Dashboard</a></li>
   {% endif %}
+  {% if current_user.has_permissions() %}
     <li><a href="{{ url_for('.choose_template', service_id=current_service.id) }}" {{ main_navigation.is_selected('templates') }}>Templates</a></li>
-  {% if not current_user.has_permissions('view_activity') %}
-    <li><a href="{{ url_for('.view_notifications', service_id=current_service.id, status='sending,delivered,failed') }}" {{ casework_navigation.is_selected('sent-messages') }}>Sent messages</a></li>
-    {% if current_service.has_jobs %}
-      <li><a href="{{ url_for('.view_jobs', service_id=current_service.id) }}" {{ casework_navigation.is_selected('uploaded-files') }}>Uploaded files</a></li>
+    {% if not current_user.has_permissions('view_activity') %}
+      <li><a href="{{ url_for('.view_notifications', service_id=current_service.id, status='sending,delivered,failed') }}" {{ casework_navigation.is_selected('sent-messages') }}>Sent messages</a></li>
+      {% if current_service.has_jobs %}
+        <li><a href="{{ url_for('.view_jobs', service_id=current_service.id) }}" {{ casework_navigation.is_selected('uploaded-files') }}>Uploaded files</a></li>
+      {% endif %}
     {% endif %}
   {% endif %}
     <li><a href="{{ url_for('.manage_users', service_id=current_service.id) }}" {{ main_navigation.is_selected('team-members') }}>Team members</a></li>
-  {% if current_user.has_permissions('manage_service') %}
+  {% if current_user.has_permissions('manage_service', allow_org_user=True) %}
     <li><a href="{{ url_for('.usage', service_id=current_service.id) }}" {{ main_navigation.is_selected('usage') }}>Usage</a></li>
   {% endif %}
   {% if current_user.has_permissions('manage_api_keys', 'manage_service') %}

tests/app/main/test_permissions.py

@@ -8,6 +8,13 @@ from app.models.roles_and_permissions import (
     translate_permissions_from_db_to_admin_roles,
 )
 from app.utils import user_has_permissions
+from tests import service_json
+from tests.conftest import (
+    ORGANISATION_ID,
+    ORGANISATION_TWO_ID,
+    SERVICE_ONE_ID,
+    SERVICE_TWO_ID,
+)
 
 
 def _test_permissions(
@@ -37,6 +44,7 @@ def _test_permissions(
 def test_user_has_permissions_on_endpoint_fail(
     client,
     mocker,
+    mock_get_service,
 ):
     user = _user_with_permissions()
     mocker.patch('app.user_api_client.get_user', return_value=user)
@@ -116,6 +124,7 @@ def test_platform_admin_user_can_not_access_page(
     client,
     platform_admin_user,
     mocker,
+    mock_get_service,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
     _test_permissions(
@@ -254,3 +263,109 @@ def test_translate_permissions_from_admin_roles_to_db():
     roles = ['send_messages', 'manage_templates', 'some_unknown_permission']
     db_perms = translate_permissions_from_admin_roles_to_db(roles)
     assert db_perms == {'send_texts', 'send_emails', 'send_letters', 'manage_templates', 'some_unknown_permission'}
+
+
+@pytest.mark.parametrize(
+    'user_services, user_organisations, expected_status, organisation_checked',
+    (
+        ([SERVICE_ONE_ID], [], 200, False),
+        ([SERVICE_ONE_ID, SERVICE_TWO_ID], [], 200, False),
+        ([], [ORGANISATION_ID], 200, True),
+        ([SERVICE_ONE_ID], [ORGANISATION_ID], 200, False),
+        ([], [], 403, True),
+        ([SERVICE_TWO_ID], [], 403, True),
+        ([SERVICE_TWO_ID], [ORGANISATION_ID], 200, True),
+        ([SERVICE_ONE_ID, SERVICE_TWO_ID], [ORGANISATION_ID], 200, False),
+        ([], [ORGANISATION_TWO_ID], 403, True),
+        ([], [ORGANISATION_ID, ORGANISATION_TWO_ID], 200, True),
+    )
+)
+def test_services_pages_that_org_users_are_allowed_to_see(
+    client_request,
+    mocker,
+    api_user_active,
+    mock_get_usage,
+    mock_get_billable_units,
+    mock_get_free_sms_fragment_limit,
+    mock_get_service,
+    mock_get_invites_for_service,
+    mock_get_users_by_service,
+    mock_get_template_folders,
+    mock_get_service_organisation,
+    mock_has_jobs,
+    user_services,
+    user_organisations,
+    expected_status,
+    organisation_checked,
+):
+    api_user_active['services'] = user_services
+    api_user_active['organisations'] = user_organisations
+    api_user_active['permissions'] = {
+        service_id: ['manage_service']
+        for service_id in user_services
+    }
+    service = service_json(
+        name='SERVICE WITH ORG',
+        id_=SERVICE_ONE_ID,
+        users=[api_user_active['id']],
+        organisation_id=ORGANISATION_ID,
+    )
+
+    mock_get_service = mocker.patch(
+        'app.notify_client.service_api_client.service_api_client.get_service',
+        return_value={'data': service}
+    )
+    client_request.login(
+        api_user_active,
+        service=service if SERVICE_ONE_ID in user_services else None,
+    )
+
+    endpoints = (
+        'main.usage',
+        'main.manage_users',
+    )
+
+    for endpoint in endpoints:
+        client_request.get(
+            endpoint,
+            service_id=SERVICE_ONE_ID,
+            _expected_status=expected_status,
+        )
+
+    assert mock_get_service.called is organisation_checked
+
+
+def test_service_navigation_for_org_user(
+    client_request,
+    mocker,
+    api_user_active,
+    mock_get_usage,
+    mock_get_billable_units,
+    mock_get_free_sms_fragment_limit,
+    mock_get_service,
+    mock_get_invites_for_service,
+    mock_get_users_by_service,
+    mock_get_service_organisation,
+):
+    api_user_active['services'] = []
+    api_user_active['organisations'] = [ORGANISATION_ID]
+    service = service_json(
+        id_=SERVICE_ONE_ID,
+        organisation_id=ORGANISATION_ID,
+    )
+    mocker.patch(
+        'app.service_api_client.get_service',
+        return_value={'data': service}
+    )
+    client_request.login(api_user_active, service=service)
+
+    page = client_request.get(
+        'main.usage',
+        service_id=SERVICE_ONE_ID,
+    )
+    assert [
+        item.text.strip() for item in page.select('nav.navigation a')
+    ] == [
+        'Team members',
+        'Usage',
+    ]

tests/conftest.py

@@ -691,6 +691,7 @@ def mock_update_service_raise_httperror_duplicate_name(mocker):
 SERVICE_ONE_ID = "596364a0-858e-42c8-9062-a8fe822260eb"
 SERVICE_TWO_ID = "147ad62a-2951-4fa1-9ca0-093cd1a52c52"
 ORGANISATION_ID = "c011fa40-4cbe-4524-b415-dde2f421bd9c"
+ORGANISATION_TWO_ID = "d9b5be73-0b36-4210-9d89-8f1a5c2fef26"
 TEMPLATE_ONE_ID = "b22d7d94-2197-4a7d-a8e7-fd5f9770bf48"
 USER_ONE_ID = "7b395b52-c6c1-469c-9d61-54166461c1ab"
 
@@ -1393,6 +1394,7 @@ def active_user_no_api_key_permission(fake_uuid):
         'auth_type': 'sms_auth',
         'organisations': [],
         'current_session_id': None,
+        'services': [SERVICE_ONE_ID],
     }
 
 
@@ -1415,6 +1417,8 @@ def active_user_no_settings_permission(fake_uuid):
         'platform_admin': False,
         'auth_type': 'sms_auth',
         'current_session_id': None,
+        'services': [SERVICE_ONE_ID],
+        'organisations': [],
     }
 
 
@@ -1432,6 +1436,7 @@ def api_user_locked(fake_uuid):
                  'organisations': [],
                  'current_session_id': None,
                  'platform_admin': False,
+                 'services': [],
                  }
     return user_data
 
@@ -1451,6 +1456,7 @@ def api_user_request_password_reset(fake_uuid):
                  'organisations': [],
                  'current_session_id': None,
                  'platform_admin': False,
+                 'services': [],
                  }
     return user_data
 
@@ -1470,6 +1476,7 @@ def api_user_changed_password(fake_uuid):
                  'organisations': [],
                  'current_session_id': None,
                  'platform_admin': False,
+                 'services': [],
                  }
     return user_data
 
@@ -2148,7 +2155,7 @@ def mock_no_inbound_number_for_service(mocker):
 
 @pytest.fixture(scope='function')
 def mock_has_permissions(mocker):
-    def _has_permission(*permissions, restrict_admin_usage=False):
+    def _has_permission(*permissions, restrict_admin_usage=False, allow_org_user=False):
         return True
 
     return mocker.patch(