From cfd454eeed1a0cdc977da9c82c0e67e771642514 Mon Sep 17 00:00:00 2001 From: Monty Dawson <wgd23@cam.ac.uk> Date: Mon, 14 Mar 2022 16:03:12 +0000 Subject: [PATCH] Add infrastucture for testing --- .coveragerc | 6 + .editorconfig | 10 + .gitlab-ci.yml | 93 +++++ Dockerfile | 13 + README.md | 3 + apigatewayauth/permissions.py | 20 +- apigatewayauth/permissions_spec.py | 18 - apigatewayauth/tests/__init__.py | 0 apigatewayauth/tests/mocks/__init__.py | 1 + apigatewayauth/tests/mocks/apps.py | 6 + .../tests/mocks/migrations/0001_initial.py | 20 ++ .../tests/mocks/migrations/__init__.py | 0 apigatewayauth/tests/mocks/mock_ibis.py | 6 + apigatewayauth/tests/mocks/models.py | 18 + .../tests/mocks/permissions_spec.yml | 18 + .../tests/mocks/test_model_migrations.py | 20 ++ apigatewayauth/tests/test_apigateway_auth.py | 133 +++++++ apigatewayauth/tests/test_permissions.py | 339 ++++++++++++++++++ apigatewayauth/tests/test_permissions_spec.py | 90 +++++ docker-compose.yml | 21 ++ runtests.py | 35 ++ setup.py | 39 ++ test.sh | 12 + tox.ini | 72 ++++ 24 files changed, 973 insertions(+), 20 deletions(-) create mode 100644 .coveragerc create mode 100644 .editorconfig create mode 100644 .gitlab-ci.yml create mode 100644 Dockerfile create mode 100644 README.md create mode 100644 apigatewayauth/tests/__init__.py create mode 100644 apigatewayauth/tests/mocks/__init__.py create mode 100644 apigatewayauth/tests/mocks/apps.py create mode 100644 apigatewayauth/tests/mocks/migrations/0001_initial.py create mode 100644 apigatewayauth/tests/mocks/migrations/__init__.py create mode 100644 apigatewayauth/tests/mocks/mock_ibis.py create mode 100644 apigatewayauth/tests/mocks/models.py create mode 100644 apigatewayauth/tests/mocks/permissions_spec.yml create mode 100644 apigatewayauth/tests/mocks/test_model_migrations.py create mode 100644 apigatewayauth/tests/test_apigateway_auth.py create mode 100644 apigatewayauth/tests/test_permissions.py create mode 100644 apigatewayauth/tests/test_permissions_spec.py create mode 100644 docker-compose.yml create mode 100644 runtests.py create mode 100644 setup.py create mode 100755 test.sh create mode 100644 tox.ini diff --git a/.coveragerc b/.coveragerc new file mode 100644 index 0000000..03d0b4b --- /dev/null +++ b/.coveragerc @@ -0,0 +1,6 @@ +[run] +omit= + setup.py + runtests.py + apigatewayauth/tests/* + .tox/* diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..7e14540 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,10 @@ +root = true + +[*] +end_of_line = lf +insert_final_newline = true + +# 4 space indentation +[*.py] +indent_style = space +indent_size = 4 \ No newline at end of file diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml new file mode 100644 index 0000000..78b875c --- /dev/null +++ b/.gitlab-ci.yml @@ -0,0 +1,93 @@ +# CI configuration which tests against supported Django and Python versions. +# +# GitLab CI does nto support Matrix builds in the traditional sense. Instead we +# build up a matrix of test jobs using inheritance via "extends". +# +# See also: https://gitlab.com/gitlab-org/gitlab-ce/issues/19199 + +include: + # Bring in the AutoDevOps template from GitLab. + # It can be viewed at: + # https://gitlab.com/gitlab-org/gitlab-ee/blob/master/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml + - template: Auto-DevOps.gitlab-ci.yml + + # Support uploading to PyPI + - project: 'uis/devops/continuous-delivery/ci-templates' + ref: '1.7.2' + file: '/pypi-release.yml' + +variables: + # We use matrix testing (below) rather than the Auto-DevOps "test" job. + TEST_DISABLED: "true" + # we don't have an application for DAST to run against, so disable it + DAST_DISABLED: "true" + +# Test code coverage +coverage: + extends: .test + variables: + TOX_ENVLIST: coverage + PYTHON_VERSION: "3.9" + + # Look for the summary line output from coverage's text report. The + # parentheses are used to indicate which portion of the report contains the + # coverage percentage. + coverage: '/^TOTAL\s+\d+\s+\d+\s+(\d+)%$/' + +# Check for PEP8 violations +flake8: + extends: .test + variables: + TOX_ENVLIST: flake8 + PYTHON_VERSION: "3.9" + +# Template jobs which run against given django versions +python38-django32: + extends: .py38 + variables: + TOX_DJANGO_FRAGMENT: "django32" + +python39-django32: + extends: .py39 + variables: + TOX_DJANGO_FRAGMENT: "django32" + +python310-django32: + extends: .py310 + variables: + TOX_DJANGO_FRAGMENT: "django32" + +# Template jobs which run tests in various Python versions. +.py38: + extends: .test + variables: + PYTHON_VERSION: "3.8" + TOX_PY_FRAGMENT: "py38" + +.py39: + extends: .test + variables: + PYTHON_VERSION: "3.9" + TOX_PY_FRAGMENT: "py39" + +.py310: + extends: .test + variables: + PYTHON_VERSION: "3.10" + TOX_PY_FRAGMENT: "py310" + +# Base test template job. +.test: + stage: test + + image: python:${PYTHON_VERSION} + + script: + - pip install tox + - tox -e ${TOX_ENVLIST} + + variables: + PYTHON_VERSION: replace-with-python-version + TOX_PY_FRAGMENT: replace-with-pyXY + TOX_DJANGO_FRAGMENT: replace-with-djangoXY + TOX_ENVLIST: "$TOX_PY_FRAGMENT-$TOX_DJANGO_FRAGMENT" diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..62a25d5 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,13 @@ +# This Dockerfile is just used for testing purposes and therefore builds in tox +# to the image. + +FROM python:3.10 + +WORKDIR /usr/src/app + +ADD . . + +RUN pip install --upgrade pip +RUN pip install tox +RUN pip install -r requirements.txt +RUN python setup.py sdist bdist_wheel diff --git a/README.md b/README.md new file mode 100644 index 0000000..0687d64 --- /dev/null +++ b/README.md @@ -0,0 +1,3 @@ +# Django API Gateway Authentication + +Todo - complete \ No newline at end of file diff --git a/apigatewayauth/permissions.py b/apigatewayauth/permissions.py index 88b2817..1c60d83 100644 --- a/apigatewayauth/permissions.py +++ b/apigatewayauth/permissions.py @@ -1,6 +1,6 @@ from typing import Set from django.core.cache import cache -from rest_framework import permissions +from rest_framework import permissions, request from identitylib.identifiers import IdentifierSchemes from ucamlookup.utils import get_connection @@ -9,7 +9,9 @@ from ucamlookup.ibisclient import PersonMethods, IbisException from logging import getLogger from .api_gateway_auth import APIGatewayAuthenticationDetails -from .permissions_spec import get_groups_with_permission, get_principals_with_permission +from .permissions_spec import ( + get_groups_with_permission, get_permission_spec, get_principals_with_permission +) LOG = getLogger(__name__) @@ -160,3 +162,17 @@ def SpecifiedPermission(permission: str): return is_in_group return HasSpecifiedPermission + + +def get_permissions_for_request(req: request.Request): + """ + Returns a list of permissions which the request's principal has been granted. + The permissions will be key any of the keys from the permissions spec, where the + principal is included within the principals or groups sections. + + """ + + return [ + permission_name for permission_name in get_permission_spec().keys() if + SpecifiedPermission(permission_name)().has_permission(req, None) + ] if isinstance(req.auth, APIGatewayAuthenticationDetails) else [] diff --git a/apigatewayauth/permissions_spec.py b/apigatewayauth/permissions_spec.py index 9a45147..fc780a5 100644 --- a/apigatewayauth/permissions_spec.py +++ b/apigatewayauth/permissions_spec.py @@ -1,14 +1,10 @@ from typing import List, Dict, Set from django.core.cache import cache from django.conf import settings -from rest_framework.request import Request from yaml import safe_load from geddit import geddit from identitylib.identifiers import Identifier -from .api_gateway_auth import APIGatewayAuthenticationDetails -from .permissions import SpecifiedPermission - PERMISSIONS_CACHE_KEY = '__PERMISSION_CACHE__' @@ -59,17 +55,3 @@ def get_groups_with_permission(permission_name: str) -> Set[Identifier]: get_permission_spec().get(permission_name, {}).get('groups', []), ) ) - - -def get_permissions_for_request(request: Request): - """ - Returns a list of permissions which the request's principal has been granted. - The permissions will be key any of the keys from the permissions spec, where the - principal is included within the principals or groups sections. - - """ - - return [ - permission_name for permission_name in get_permission_spec().keys() if - SpecifiedPermission(permission_name)().has_permission(request, None) - ] if isinstance(request.auth, APIGatewayAuthenticationDetails) else [] diff --git a/apigatewayauth/tests/__init__.py b/apigatewayauth/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/apigatewayauth/tests/mocks/__init__.py b/apigatewayauth/tests/mocks/__init__.py new file mode 100644 index 0000000..aa0ac70 --- /dev/null +++ b/apigatewayauth/tests/mocks/__init__.py @@ -0,0 +1 @@ +default_app_config = 'apigatewayauth.tests.mocks.apps.MockAPIGatewayAuthConfig' diff --git a/apigatewayauth/tests/mocks/apps.py b/apigatewayauth/tests/mocks/apps.py new file mode 100644 index 0000000..5cab59a --- /dev/null +++ b/apigatewayauth/tests/mocks/apps.py @@ -0,0 +1,6 @@ +from django.apps import AppConfig + + +class MockAPIGatewayAuthConfig(AppConfig): + name = 'apigatewayauth.tests.mocks' + verbose_name = 'Mock testing app' diff --git a/apigatewayauth/tests/mocks/migrations/0001_initial.py b/apigatewayauth/tests/mocks/migrations/0001_initial.py new file mode 100644 index 0000000..4e87be7 --- /dev/null +++ b/apigatewayauth/tests/mocks/migrations/0001_initial.py @@ -0,0 +1,20 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='TestModel', + fields=[ + ('name', models.TextField(primary_key=True, serialize=False, verbose_name='Name')), + ('isAdmin', models.BooleanField(verbose_name='Is Admin')), + ('principal_identifier', models.TextField(verbose_name='Principal identifier')), + ], + ), + ] diff --git a/apigatewayauth/tests/mocks/migrations/__init__.py b/apigatewayauth/tests/mocks/migrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/apigatewayauth/tests/mocks/mock_ibis.py b/apigatewayauth/tests/mocks/mock_ibis.py new file mode 100644 index 0000000..0ed200a --- /dev/null +++ b/apigatewayauth/tests/mocks/mock_ibis.py @@ -0,0 +1,6 @@ +from dataclasses import dataclass + + +@dataclass +class MockIbisGroup: + groupid: str diff --git a/apigatewayauth/tests/mocks/models.py b/apigatewayauth/tests/mocks/models.py new file mode 100644 index 0000000..00fe919 --- /dev/null +++ b/apigatewayauth/tests/mocks/models.py @@ -0,0 +1,18 @@ +from django.db import models + + +class TestModel(models.Model): + """ + A test model to allow us to test functionality which requires a model instance to be passed in. + + """ + + @staticmethod + def get_queryset_for_principal(principal_identifier): + return TestModel.objects.filter( + principal_identifier__iexact=principal_identifier.value, + ) + + name = models.TextField('Name', 'name', primary_key=True) + is_admin = models.BooleanField('Is Admin', 'isAdmin') + principal_identifier = models.TextField('Principal identifier') diff --git a/apigatewayauth/tests/mocks/permissions_spec.yml b/apigatewayauth/tests/mocks/permissions_spec.yml new file mode 100644 index 0000000..6b8ca08 --- /dev/null +++ b/apigatewayauth/tests/mocks/permissions_spec.yml @@ -0,0 +1,18 @@ +# A map of permission names to identifiers of the people, services or groups that are granted +# this permission. + +# This is purely for testing purposes + + +ABSTRACT_DATA_READER: + principals: + - 1234@application.api.apps.cam.ac.uk + - 16aa8901-7d92-405c-8568-6b2c5adff5ac@apigee-staging.test.api.gcp.uis.cam.ac.uk + groups: + - 105217@groups.lookup.cam.ac.uk + - 105215@groups.lookup.cam.ac.uk + - 105216@groups.lookup.cam.ac.uk + +THOUGHT_CREATOR: + principals: + - 1234@application.api.apps.cam.ac.uk \ No newline at end of file diff --git a/apigatewayauth/tests/mocks/test_model_migrations.py b/apigatewayauth/tests/mocks/test_model_migrations.py new file mode 100644 index 0000000..4e87be7 --- /dev/null +++ b/apigatewayauth/tests/mocks/test_model_migrations.py @@ -0,0 +1,20 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ] + + operations = [ + migrations.CreateModel( + name='TestModel', + fields=[ + ('name', models.TextField(primary_key=True, serialize=False, verbose_name='Name')), + ('isAdmin', models.BooleanField(verbose_name='Is Admin')), + ('principal_identifier', models.TextField(verbose_name='Principal identifier')), + ], + ), + ] diff --git a/apigatewayauth/tests/test_apigateway_auth.py b/apigatewayauth/tests/test_apigateway_auth.py new file mode 100644 index 0000000..3e36ff2 --- /dev/null +++ b/apigatewayauth/tests/test_apigateway_auth.py @@ -0,0 +1,133 @@ +from django.test import TestCase +from identitylib.identifiers import Identifier, IdentifierSchemes +from rest_framework.test import APIRequestFactory +from rest_framework.exceptions import AuthenticationFailed + +from apigatewayauth.api_gateway_auth import ( + APIGatewayAuthentication, APIGatewayAuthenticationDetails +) + + +class APIGatewayAuthTestCase(TestCase): + + def setUp(self): + super().setUp() + + self.request_factory = APIRequestFactory() + self.auth = APIGatewayAuthentication() + + def request_with_headers(self, headers={}): + parsed_headers = { + f'HTTP_{key.upper().replace("-", "_")}': value + for key, value in headers.items() + } + return self.request_factory.get('/', **parsed_headers) + + def test_bails_early_without_api_org(self): + self.assertIsNone( + self.auth.authenticate(self.request_with_headers({"Accept": "application/json"})) + ) + + def test_throws_without_auth_details(self): + with self.assertRaisesMessage( + AuthenticationFailed, 'Could not authenticate using x-api-* headers' + ): + self.auth.authenticate(self.request_with_headers({"x-api-org-name": "test"})) + + def test_throws_without_principal_identifier(self): + with self.assertRaisesMessage( + AuthenticationFailed, 'Could not authenticate using x-api-* headers' + ): + self.auth.authenticate(self.request_with_headers({ + "x-api-org-name": "test", + "x-api-developer-app-class": "public" + })) + + def test_throws_with_bad_principal_identifier(self): + with self.assertRaisesMessage( + AuthenticationFailed, 'Invalid principal identifier' + ): + self.auth.authenticate(self.request_with_headers({ + "x-api-org-name": "test", + "x-api-developer-app-class": "public", + "x-api-oauth2-user": "Monty Dawson" + })) + + def test_can_use_any_identifier_scheme_in_principal_identifier(self): + for scheme in IdentifierSchemes.get_registered_schemes(): + _, auth = self.auth.authenticate(self.request_with_headers({ + "x-api-org-name": "test", + "x-api-developer-app-class": "public", + "x-api-oauth2-user": str(Identifier("1000", scheme)) + })) + self.assertEqual(auth.principal_identifier, Identifier("1000", scheme)) + + def test_throws_with_unknown_identifier_type(self): + with self.assertRaisesMessage( + AuthenticationFailed, 'Invalid principal identifier' + ): + self.auth.authenticate(self.request_with_headers({ + "x-api-org-name": "test", + "x-api-developer-app-class": "public", + "x-api-oauth2-user": 'wgd23@gmail.com' + })) + + def test_returns_client_details_for_valid_auth(self): + user, auth = self.auth.authenticate(self.request_with_headers({ + "x-api-org-name": "test", + "x-api-developer-app-class": "public", + "x-api-oauth2-user": str(Identifier('a123', IdentifierSchemes.CRSID)) + })) + self.assertIsNone(user) + + self.assertEqual( + auth, + APIGatewayAuthenticationDetails( + Identifier('a123', IdentifierSchemes.CRSID), + set(), + None, + None, + ) + ) + + def test_will_pass_through_scopes(self): + _, auth = self.auth.authenticate(self.request_with_headers({ + "x-api-org-name": "test", + "x-api-developer-app-class": "public", + "x-api-oauth2-user": str(Identifier('a123', IdentifierSchemes.CRSID)), + "x-api-oauth2-scope": ( + "https://api.apps.cam.ac.uk/a.readonly https://api.apps.cam.ac.uk/b" + ) + })) + + self.assertEqual( + auth, + APIGatewayAuthenticationDetails( + Identifier('a123', IdentifierSchemes.CRSID), + set(['https://api.apps.cam.ac.uk/a.readonly', 'https://api.apps.cam.ac.uk/b']), + None, + None, + ), + ) + + def test_will_pass_through_app_and_client_ids(self): + _, auth = self.auth.authenticate(self.request_with_headers({ + "x-api-org-name": "test", + "x-api-developer-app-class": "confidential", + "x-api-oauth2-user": str(Identifier('a123', IdentifierSchemes.CRSID)), + "x-api-oauth2-scope": ( + "https://api.apps.cam.ac.uk/a.readonly https://api.apps.cam.ac.uk/b" + ), + "x-api-developer-app-id": "app-uuid-mock", + "x-api-oauth2-client-id": "client-id-uuid-mock", + })) + + self.assertEqual( + auth, + APIGatewayAuthenticationDetails( + Identifier('a123', IdentifierSchemes.CRSID), + set(['https://api.apps.cam.ac.uk/a.readonly', 'https://api.apps.cam.ac.uk/b']), + 'app-uuid-mock', + 'client-id-uuid-mock' + ) + ) diff --git a/apigatewayauth/tests/test_permissions.py b/apigatewayauth/tests/test_permissions.py new file mode 100644 index 0000000..6c53499 --- /dev/null +++ b/apigatewayauth/tests/test_permissions.py @@ -0,0 +1,339 @@ +from unittest.mock import MagicMock, patch +from django.test import TestCase +from identitylib.identifiers import Identifier, IdentifierSchemes +from rest_framework.test import APIRequestFactory +from ucamlookup.ibisclient import IbisException + +from .mocks.mock_ibis import MockIbisGroup +from .mocks.models import TestModel + +from apigatewayauth.api_gateway_auth import APIGatewayAuthenticationDetails + +from apigatewayauth.permissions import ( + Disallowed, + HasAnyScope, + SpecifiedPermission, + IsResourceOwningPrincipal +) + + +class PermissionsTestCase(TestCase): + + def setUp(self): + super().setUp() + + self.view = MagicMock() + self.object = MagicMock() + + self.client_auth_details = APIGatewayAuthenticationDetails( + Identifier('abc', IdentifierSchemes.CRSID), set() + ) + + def request_with_auth(self, auth): + request = APIRequestFactory().get('/') + setattr(request, 'auth', auth) + return request + + def test_disallowed_permission_always_disallows(self): + permission = Disallowed() + + for auth in [None, self.client_auth_details]: + self.assertFalse(permission.has_permission(self.request_with_auth(auth), self.view)) + self.assertFalse( + permission.has_object_permission( + self.request_with_auth(auth), self.view, self.object + ) + ) + + def test_is_resource_owning_principal_returns_true_when_authenticated(self): + permission = IsResourceOwningPrincipal() + + self.assertFalse(permission.has_permission(self.request_with_auth(None), self.view)) + self.assertFalse( + permission.has_object_permission( + self.request_with_auth(None), self.view, self.object + ) + ) + + public_client_request = self.request_with_auth(self.client_auth_details) + self.assertTrue(permission.has_permission(public_client_request, self.view)) + # the request should be flagged as needing limiting + self.assertTrue( + getattr(public_client_request, 'should_limit_to_resource_owning_principal') + ) + + def test_is_resource_owning_principal_users_has_object_permission(self): + permission = IsResourceOwningPrincipal() + + # should be false as the object passed in doesn't have a 'is_owned_by' property + self.assertFalse( + permission.has_object_permission( + self.request_with_auth(self.client_auth_details), + self.view, + {} + ) + ) + + mock_object = MagicMock() + mock_object.is_owned_by.return_value = False + + self.assertFalse( + permission.has_object_permission( + self.request_with_auth(self.client_auth_details), + self.view, + mock_object + ) + ) + mock_object.is_owned_by.assert_called_with( + self.client_auth_details.principal_identifier + ) + + mock_object.is_owned_by.return_value = True + self.assertTrue( + permission.has_object_permission( + self.request_with_auth(self.client_auth_details), + self.view, + mock_object + ) + ) + + def test_get_queryset_for_principal_bails_for_unhandled_request(self): + # bails early if request doesn't require filtering + self.assertQuerysetEqual( + IsResourceOwningPrincipal.get_queryset_for_principal( + self.request_with_auth(self.client_auth_details), + TestModel + ), + TestModel.objects.all() + ) + + def test_get_queryset_for_principal_returns_filtered_queryset(self): + permission = IsResourceOwningPrincipal() + + request = self.request_with_auth(self.client_auth_details) + + # pass through `has_permission` to mark `should_limit_to_resource_owning_principal` + # on the requests + self.assertTrue(permission.has_permission(request, self.view)) + + self.assertQuerysetEqual( + IsResourceOwningPrincipal.get_queryset_for_principal(request, TestModel), + TestModel.get_queryset_for_principal( + self.client_auth_details.principal_identifier + ) + ) + + def test_has_any_scope(self): + permission = HasAnyScope('a', 'b.readonly')() + + for auth in [None, self.client_auth_details]: + self.assertFalse(permission.has_permission(self.request_with_auth(auth), self.view)) + self.assertFalse( + permission.has_object_permission( + self.request_with_auth(auth), self.view, self.object + ) + ) + + self.assertTrue( + permission.has_permission( + self.request_with_auth( + APIGatewayAuthenticationDetails( + Identifier('abc', IdentifierSchemes.CRSID), + set(['b.readonly']) + ) + ), + self.view, + ) + ) + + self.assertFalse( + permission.has_object_permission( + self.request_with_auth( + APIGatewayAuthenticationDetails( + Identifier('abc', IdentifierSchemes.CRSID), + set(['abcd']) + ) + ), + self.view, + self.object, + ) + ) + + @patch('apigatewayauth.permissions.get_principals_with_permission') + def test_has_specified_permission_return_false_if_not_matching(self, get_identities_mock): + get_identities_mock.return_value = set([ + Identifier('1000', IdentifierSchemes.STAFF_NUMBER), + Identifier('abc123', IdentifierSchemes.CRSID) + ]) + permission = SpecifiedPermission('READ_BOOKS')() + + self.assertFalse( + permission.has_permission(self.request_with_auth(self.client_auth_details), self.view) + ) + get_identities_mock.assert_called_with('READ_BOOKS') + + @patch('apigatewayauth.permissions.get_principals_with_permission') + def test_has_specified_permission_return_true_if_matching(self, get_identities_mock): + get_identities_mock.return_value = set([ + Identifier('1000', IdentifierSchemes.STAFF_NUMBER), + self.client_auth_details.principal_identifier + ]) + permission = SpecifiedPermission('READ_BOOKS')() + + self.assertTrue( + permission.has_permission(self.request_with_auth(self.client_auth_details), self.view) + ) + get_identities_mock.assert_called_with('READ_BOOKS') + + @patch('apigatewayauth.permissions.get_principals_with_permission') + @patch('apigatewayauth.permissions.get_groups_with_permission') + @patch('apigatewayauth.permissions.PersonMethods.getGroups') + def test_does_not_query_lookup_groups_for_non_crsid_principal( + self, + get_groups_mock, + get_groups_with_permission_mock, + get_identities_mock, + ): + get_identities_mock.return_value = set([ + Identifier('abc123', IdentifierSchemes.CRSID), + Identifier('abc333', IdentifierSchemes.CRSID) + ]) + get_groups_with_permission_mock.return_value = set([ + Identifier('1000', IdentifierSchemes.LOOKUP_GROUP), + ]) + permission = SpecifiedPermission('READ_MAGAZINES')() + + auth_details = APIGatewayAuthenticationDetails( + Identifier('1', IdentifierSchemes.STAFF_NUMBER), + set([]) + ) + + self.assertFalse( + permission.has_permission(self.request_with_auth(auth_details), self.view) + ) + get_identities_mock.assert_called_with('READ_MAGAZINES') + + # should not have queried for groups as our principal does not have a crsid identifier + get_groups_mock.assert_not_called() + + @patch('apigatewayauth.permissions.get_principals_with_permission') + @patch('apigatewayauth.permissions.get_groups_with_permission') + @patch('apigatewayauth.permissions.PersonMethods.getGroups') + def test_does_not_query_lookup_groups_if_no_lookup_groups_in_permissions_spec( + self, + get_groups_mock, + get_groups_with_permission_mock, + get_identities_mock, + ): + get_identities_mock.return_value = set([ + Identifier('1000', IdentifierSchemes.STAFF_NUMBER), + Identifier('bb123', IdentifierSchemes.CRSID) + ]) + get_groups_with_permission_mock.return_value = set() + permission = SpecifiedPermission('READ_PAMPHLETS')() + + self.assertFalse( + permission.has_permission(self.request_with_auth(self.client_auth_details), self.view) + ) + get_identities_mock.assert_called_with('READ_PAMPHLETS') + + # should not have queried for groups as we do not have a lookup group in our + # allowed identities list + get_groups_mock.assert_not_called() + + @patch('apigatewayauth.permissions.get_principals_with_permission') + @patch('apigatewayauth.permissions.get_groups_with_permission') + @patch('apigatewayauth.permissions.PersonMethods.getGroups') + def test_will_return_true_if_principal_is_in_specified_lookup_group( + self, + get_groups_mock, + get_groups_with_permission_mock, + get_identities_mock, + ): + get_identities_mock.return_value = set([ + Identifier('cc123', IdentifierSchemes.CRSID) + ]) + get_groups_with_permission_mock.return_value = set([ + Identifier('1001', IdentifierSchemes.LOOKUP_GROUP) + ]) + permission = SpecifiedPermission('READ_THE_NEWS')() + get_groups_mock.return_value = [MockIbisGroup('1001'), MockIbisGroup('1002')] + + self.assertTrue( + permission.has_permission(self.request_with_auth(self.client_auth_details), self.view) + ) + get_identities_mock.assert_called_with('READ_THE_NEWS') + + # should have queried Lookup for the principal's groups + get_groups_mock.assert_called_with( + scheme='crsid', identifier=self.client_auth_details.principal_identifier.value + ) + + # querying again returns true without going to Lookup due to caching the result + get_groups_mock.reset_mock() + self.assertTrue( + permission.has_permission(self.request_with_auth(self.client_auth_details), self.view) + ) + get_groups_mock.assert_not_called() + + @patch('apigatewayauth.permissions.get_principals_with_permission') + @patch('apigatewayauth.permissions.get_groups_with_permission') + @patch('apigatewayauth.permissions.PersonMethods.getGroups') + def test_will_return_false_if_principal_is_not_in_specified_lookup_group( + self, + get_groups_mock, + get_groups_with_permission_mock, + get_identities_mock, + ): + get_identities_mock.return_value = set([ + Identifier('dd123', IdentifierSchemes.CRSID) + ]) + get_groups_with_permission_mock.return_value = set([ + Identifier('10101', IdentifierSchemes.LOOKUP_GROUP) + ]) + permission = SpecifiedPermission('READ_THE_ROOM')() + get_groups_mock.return_value = [MockIbisGroup('1001'), MockIbisGroup('1002')] + + self.assertFalse( + permission.has_permission(self.request_with_auth(self.client_auth_details), self.view) + ) + get_identities_mock.assert_called_with('READ_THE_ROOM') + + # should have queried Lookup for the principal's groups + get_groups_mock.assert_called_with( + scheme='crsid', identifier=self.client_auth_details.principal_identifier.value + ) + + @patch('apigatewayauth.permissions.get_principals_with_permission') + @patch('apigatewayauth.permissions.get_groups_with_permission') + @patch('apigatewayauth.permissions.PersonMethods.getGroups') + def test_will_return_false_if_unable_to_read_lookup_groups( + self, + get_groups_mock, + get_groups_with_permission_mock, + get_identities_mock, + ): + get_identities_mock.return_value = set([ + Identifier('ee123', IdentifierSchemes.CRSID), + ]) + get_groups_with_permission_mock.return_value = set([ + Identifier('2000', IdentifierSchemes.LOOKUP_GROUP), + ]) + permission = SpecifiedPermission('READ_THE_FUTURE')() + + def throw_ibis_error(*args, **kwargs): + error = ValueError('failed') + setattr(error, 'message', 'failed') # ibis exception needs a 'message' attribute + raise IbisException(error) + + get_groups_mock.side_effect = throw_ibis_error + + self.assertFalse( + permission.has_permission(self.request_with_auth(self.client_auth_details), self.view) + ) + get_identities_mock.assert_called_with('READ_THE_FUTURE') + + # should have queried Lookup for the principal's groups + get_groups_mock.assert_called_with( + scheme='crsid', identifier=self.client_auth_details.principal_identifier.value + ) diff --git a/apigatewayauth/tests/test_permissions_spec.py b/apigatewayauth/tests/test_permissions_spec.py new file mode 100644 index 0000000..4888c50 --- /dev/null +++ b/apigatewayauth/tests/test_permissions_spec.py @@ -0,0 +1,90 @@ +from unittest.mock import patch +from django.test import TestCase +from django.conf import settings +from django.core.cache import cache +from identitylib.identifiers import Identifier, IdentifierSchemes + +from apigatewayauth.permissions_spec import ( + get_permission_spec, get_principals_with_permission, get_groups_with_permission +) + + +class PermissionSpecTestCase(TestCase): + + def setUp(self): + super().setUp() + cache.clear() # clear the cache between tests + + @patch('apigatewayauth.permissions_spec.geddit') + def test_will_return_parsed_permissions_spec_with_cache(self, geddit_mock): + geddit_mock.return_value = """ + CARD_DATA_READERS: + principals: + - abc123@v1.person.identifiers.cam.ac.uk + - 1234@application.api.apps.cam.ac.uk + groups: + - 1001@groups.lookup.cam.ac.uk + """ + + expected_permission_spec = { + "CARD_DATA_READERS": { + "principals": [ + str(Identifier('abc123', IdentifierSchemes.CRSID)), + str(Identifier('1234', IdentifierSchemes.API_GATEWAY_APPLICATION)), + ], + "groups": [ + str(Identifier('1001', IdentifierSchemes.LOOKUP_GROUP)), + ], + } + } + self.assertEqual(get_permission_spec(), expected_permission_spec) + geddit_mock.assert_called_with(settings.PERMISSIONS_SPECIFICATION_URL) + + geddit_mock.reset_mock() + + # ensure that getting the permissions spec again does not use geddit as the cached + # result is used + self.assertEqual(get_permission_spec(), expected_permission_spec) + geddit_mock.assert_not_called() + + @patch('apigatewayauth.permissions_spec.geddit') + def test_can_query_specific_permission(self, geddit_mock): + geddit_mock.return_value = """ + CARD_DATA_READERS: + principals: + - 1234@application.api.apps.cam.ac.uk + - abc123@v1.person.identifiers.cam.ac.uk + groups: + - 1001@groups.lookup.cam.ac.uk + CARD_DATA_WRITERS: + principals: + - abc234@v1.person.identifiers.cam.ac.uk + """ + + self.assertEqual( + get_principals_with_permission('CARD_DATA_READERS'), { + Identifier('1234', IdentifierSchemes.API_GATEWAY_APPLICATION), + Identifier('abc123', IdentifierSchemes.CRSID) + } + ) + geddit_mock.assert_called_with(settings.PERMISSIONS_SPECIFICATION_URL) + geddit_mock.reset_mock() + + self.assertEqual( + get_groups_with_permission('CARD_DATA_READERS'), { + Identifier('1001', IdentifierSchemes.LOOKUP_GROUP) + } + ) + # should not be called as we have cached the spec + geddit_mock.assert_not_called() + + self.assertEqual( + get_principals_with_permission('CARD_DATA_WRITERS'), { + Identifier('abc234', IdentifierSchemes.CRSID) + } + ) + + self.assertEqual(get_groups_with_permission('CARD_DATA_WRITERS'), set()) + + self.assertEqual(get_groups_with_permission('CARD_DATA_ADMINS'), set()) + self.assertEqual(get_principals_with_permission('CARD_DATA_ADMINS'), set()) diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..48a2ff0 --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,21 @@ +# docker-compose file for running tox tests locally +version: '3.2' +services: + tox: + build: + context: . + dockerfile: ./Dockerfile + entrypoint: [] + command: ["tox", "-e", "coverage,flake8"] + environment: + - TOXINI_WORK_DIR=/tmp/tox-data/work + - TOXINI_ARTEFACT_DIR=/tmp/tox-data/artefacts + - TOXINI_COVERAGE_FILE=/tmp/tox-coverage + volumes: + - tox-data:/tmp/tox-data + - ./:/usr/src/app:ro + +volumes: + # A persistent volume for tox to store its stuff. This allows caching of + # virtualenvs between runs. + tox-data: diff --git a/runtests.py b/runtests.py new file mode 100644 index 0000000..856181b --- /dev/null +++ b/runtests.py @@ -0,0 +1,35 @@ +import logging +import os +import sys +import django +from django.conf import settings +from django.test.runner import DiscoverRunner +from django.db import DEFAULT_DB_ALIAS + + +DIRNAME = os.path.dirname(os.path.realpath(__file__)) + +settings.configure( + DEBUG=False, + SECRET_KEY='placeholder', + DATABASES={DEFAULT_DB_ALIAS: {'ENGINE': 'django.db.backends.sqlite3', 'NAME': '/tmp/test.db'}}, + TIME_ZONE='Europe/London', + USE_TZ=True, + INSTALLED_APPS=('apigatewayauth', 'apigatewayauth.tests.mocks'), + MIDDLEWARE_CLASSES=(), + MIDDLEWARE=(), + TEMPLATES=[], + # point to our mock permissions spec + PERMISSIONS_SPECIFICATION_URL=( + os.path.join(DIRNAME, 'apigatewayauth/tests/mocks/permissions_spec.yml') + ) +) + +django.setup() + +logging.basicConfig() +test_runner = DiscoverRunner() + +failures = test_runner.run_tests(['apigatewayauth']) +if failures: + sys.exit(1) diff --git a/setup.py b/setup.py new file mode 100644 index 0000000..2753f1f --- /dev/null +++ b/setup.py @@ -0,0 +1,39 @@ +import os +from setuptools import setup, find_packages + + +def load_requirements(file: str): + """ + Load requirements file and return non-empty, non-comment lines with leading and trailing + whitespace stripped. + """ + with open(os.path.join(os.path.dirname(__file__), file)) as f: + return [ + line.strip() for line in f + if line.strip() != '' and not line.strip().startswith('#') + ] + + +setup( + name='django-ucam-apigatewayauth', + description='A Django module allow auth based on the headers passed from the API Gateway', + long_description=open('README.md').read(), + long_description_content_type='text/markdown', + url='https://gitlab.developers.cam.ac.uk/uis/devops/django/api-gateway-auth', + version='0.0.1', + license='MIT', + author='DevOps Division, University Information Services, University of Cambridge', + author_email='devops@uis.cam.ac.uk', + packages=find_packages(), + include_package_data=True, + install_requires=['django>=3.2', 'ibisclient~=1.3'], + classifiers=[ + 'Development Status :: 3 - Alpha ', + 'Environment :: Web Environment', + 'Framework :: Django', + 'Intended Audience :: Developers', + 'License :: OSI Approved :: MIT License', + 'Operating System :: OS Independent', + 'Programming Language :: Python', + ], +) diff --git a/test.sh b/test.sh new file mode 100755 index 0000000..1a33311 --- /dev/null +++ b/test.sh @@ -0,0 +1,12 @@ +# +# Wrapper script to run tox. Arguments are passed directly to tox. + +# Exit on failure +set -e + +# Change to this script's directory +cd "$( dirname "${BASH_SOURCE[0]}")" + +# Execute tox runner, logging command used +set -x +exec docker-compose run --rm tox $@ \ No newline at end of file diff --git a/tox.ini b/tox.ini new file mode 100644 index 0000000..c82e284 --- /dev/null +++ b/tox.ini @@ -0,0 +1,72 @@ +# Tox runner configuration +# +# The following optional environment variables can change behaviour. See the +# comments where they are used for more information. +# +# - TOXINI_ARTEFACT_DIR +# - TOXINI_FLAKE8_VERSION +# - TOXINI_WORK_DIR +# +[tox] +# Envs which should be run by default. This will execute a matrix of tests +envlist = + py{38,39,310}-django32 + coverage + flake8 +# Allow overriding toxworkdir via environment variable +toxworkdir={env:TOXINI_WORK_DIR:{toxinidir}/.tox} +# Do not attempt to create .egg-info directories in the application root as it +# is mounted as a read-only volume. +skipsdist=true + +# The "_vars" section is ignored by tox but we place some useful shared +# variables in it to avoid needless repetition. +[_vars] +# Where to write build artefacts. We default to the "build" directory in the +# tox.ini file's directory. Override with the TOXINI_ARTEFACT_DIR environment +# variable. +build_root={env:TOXINI_ARTEFACT_DIR:{toxinidir}/build} + +[testenv] +setenv= +# Override the coverage dtaa file location since the application root is +# mounted read-only. + COVERAGE_FILE={env:TOXINI_COVERAGE_FILE:{toxinidir}/.coverage} +# Additional dependencies +deps= + . + mock + coverage + -rrequirements.txt + django22: Django>=2.2,<3.0 + django32: Django>=3.2,<3.3 +# Specify the default environment. +commands= + coverage run --source={toxinidir} ./runtests.py + +# Check for PEP8 violations +[testenv:flake8] +basepython=python3 +deps= + -rrequirements.txt +# We specify a specific version of flake8 to avoid introducing "false" +# regressions when new checks are introduced. The version of flake8 used may +# be overridden via the TOXINI_FLAKE8_VERSION environment variable. + mock + flake8=={env:TOXINI_FLAKE8_VERSION:4.0.1} +commands= + flake8 --version + flake8 . + +# Check for PEP8 violations +[testenv:coverage] +basepython=python3 +deps= + -rrequirements.txt + mock + coverage +commands= + coverage run --source={toxinidir} ./runtests.py + coverage html --directory {[_vars]build_root}/htmlcov/ + coverage report + coverage xml -o {env:COVERAGE_XML_FILE:{[_vars]build_root}/coverage.xml} -- GitLab