From 8de04d936faba5ef3808fdbcd512d1f35c0ef63d Mon Sep 17 00:00:00 2001 From: Clemens Wolff Date: Wed, 25 Dec 2019 23:10:05 -0500 Subject: [PATCH] Add endpoint to list clients (#268) * Nest client metadata files in folders * Remove fallback code for missing owner metadata * Refactor from TextStorage to ObjectStorage * Fix listing of files that contain a dot * Add method to list domains * Add endpoint to list domains --- opwen_email_server/actions.py | 13 +++++ opwen_email_server/constants/events.py | 1 + opwen_email_server/integration/azure.py | 2 +- opwen_email_server/integration/connexion.py | 3 ++ opwen_email_server/services/auth.py | 54 +++++++++---------- opwen_email_server/services/storage.py | 37 ++++++++++--- .../swagger/client-register.yaml | 24 +++++++++ .../opwen_email_server/services/test_auth.py | 18 +++---- .../services/test_storage.py | 14 ++++- tests/opwen_email_server/test_actions.py | 20 +++++++ 10 files changed, 138 insertions(+), 48 deletions(-) diff --git a/opwen_email_server/actions.py b/opwen_email_server/actions.py index 2dac8a39..236a125c 100644 --- a/opwen_email_server/actions.py +++ b/opwen_email_server/actions.py @@ -362,6 +362,19 @@ def _action(self, client, **auth_args): # type: ignore return 'accepted', 201 +class ListClients(_Action): + def __init__(self, auth: AzureAuth): + self._auth = auth + + def _action(self, **auth_args): # type: ignore + clients = [{'domain': domain} for domain in self._auth.domains()] + + self.log_event(events.CLIENTS_FETCHED) # noqa: E501 # yapf: disable + return { + 'clients': clients, + } + + class GetClient(_Action): def __init__(self, auth: AzureAuth, client_storage: AzureObjectsStorage): self._auth = auth diff --git a/opwen_email_server/constants/events.py b/opwen_email_server/constants/events.py index 0b2a7f06..8a075a07 100644 --- a/opwen_email_server/constants/events.py +++ b/opwen_email_server/constants/events.py @@ -2,6 +2,7 @@ CLIENT_DELETED = 'client_deleted' # type: Final CLIENT_FETCHED = 'client_fetched' # type: Final +CLIENTS_FETCHED = 'clients_fetched' # type: Final CLIENT_CREATED = 'client_created' # type: Final NEW_CLIENT_REGISTERED = 'new_client_registered' # type: Final UNREGISTERED_CLIENT = 'unregistered_client' # type: Final diff --git a/opwen_email_server/integration/azure.py b/opwen_email_server/integration/azure.py index 9a63bb7b..625ae1b6 100644 --- a/opwen_email_server/integration/azure.py +++ b/opwen_email_server/integration/azure.py @@ -13,7 +13,7 @@ @singleton def get_auth() -> AzureAuth: - return AzureAuth(storage=AzureTextStorage( + return AzureAuth(storage=AzureObjectStorage( account=config.TABLES_ACCOUNT, key=config.TABLES_KEY, host=config.TABLES_HOST, diff --git a/opwen_email_server/integration/connexion.py b/opwen_email_server/integration/connexion.py index 53f1e0c3..be4a43d3 100644 --- a/opwen_email_server/integration/connexion.py +++ b/opwen_email_server/integration/connexion.py @@ -4,6 +4,7 @@ from opwen_email_server.actions import DeleteClient from opwen_email_server.actions import DownloadClientEmails from opwen_email_server.actions import GetClient +from opwen_email_server.actions import ListClients from opwen_email_server.actions import Ping from opwen_email_server.actions import ReceiveInboundEmail from opwen_email_server.actions import UploadClientEmails @@ -44,6 +45,8 @@ task=register_client.delay, ) +client_list = ListClients(auth=get_auth()) + client_get = GetClient( auth=get_auth(), client_storage=get_client_storage(), diff --git a/opwen_email_server/services/auth.py b/opwen_email_server/services/auth.py index 47df646c..a896ab67 100644 --- a/opwen_email_server/services/auth.py +++ b/opwen_email_server/services/auth.py @@ -1,4 +1,3 @@ -from json import JSONDecodeError from typing import Callable from typing import Dict from typing import Iterable @@ -11,10 +10,8 @@ from opwen_email_server.constants import events from opwen_email_server.constants import github -from opwen_email_server.services.storage import AzureTextStorage +from opwen_email_server.services.storage import AzureObjectStorage from opwen_email_server.utils.log import LogMixin -from opwen_email_server.utils.serialization import from_json -from opwen_email_server.utils.serialization import to_json class AnyOfBasicAuth(LogMixin): @@ -135,56 +132,59 @@ def _fetch_team_members(self, access_token: str) -> Iterable[str]: class AzureAuth(LogMixin): - def __init__(self, storage: AzureTextStorage) -> None: + def __init__(self, storage: AzureObjectStorage) -> None: self._storage = storage def insert(self, client_id: str, domain: str, owner: str): - self._storage.store_text(client_id, domain) - self._storage.store_text(domain, to_json({'client_id': client_id, 'owner': owner})) + auth = {'client_id': client_id, 'owner': owner, 'domain': domain} + self._storage.store_object(self._client_id_file(client_id), auth) + self._storage.store_object(self._domain_file(domain), auth) self.log_info('Registered client %s at domain %s', client_id, domain) def is_owner(self, domain: str, username: str) -> bool: try: - raw_auth = self._storage.fetch_text(domain) + auth = self._storage.fetch_object(self._domain_file(domain)) except ObjectDoesNotExistError: self.log_warning('Unrecognized domain %s', domain) return False - try: - auth = from_json(raw_auth) - except JSONDecodeError: - # fallback for clients registered before November 2019 - self.log_warning('Unable to lookup owner for domain %s', domain) - return False - return auth.get('owner') == username def delete(self, client_id: str, domain: str) -> bool: - self._storage.delete(domain) - self._storage.delete(client_id) + self._storage.delete(self._domain_file(domain)) + self._storage.delete(self._client_id_file(client_id)) return True def client_id_for(self, domain: str) -> Optional[str]: try: - raw_auth = self._storage.fetch_text(domain) + auth = self._storage.fetch_object(self._domain_file(domain)) except ObjectDoesNotExistError: self.log_warning('Unrecognized domain %s', domain) return None - else: - try: - client_id = from_json(raw_auth)['client_id'] - except JSONDecodeError: - # fallback for clients registered before November 2019 - client_id = raw_auth - self.log_debug('Domain %s has client %s', domain, client_id) - return client_id + + client_id = auth['client_id'] + + self.log_debug('Domain %s has client %s', domain, client_id) + return client_id def domain_for(self, client_id: str) -> Optional[str]: try: - domain = self._storage.fetch_text(client_id) + auth = self._storage.fetch_object(self._client_id_file(client_id)) except ObjectDoesNotExistError: self.log_warning('Unrecognized client %s', client_id) return None else: + domain = auth['domain'] self.log_debug('Client %s has domain %s', client_id, domain) return domain + + def domains(self) -> Iterable[str]: + return self._storage.iter(self._domain_file('')) + + @classmethod + def _domain_file(cls, domain: str) -> str: + return f'domain/{domain}' + + @classmethod + def _client_id_file(cls, client_id: str) -> str: + return f'client_id/{client_id}' diff --git a/opwen_email_server/services/storage.py b/opwen_email_server/services/storage.py index 7ebc7a96..2f82d84d 100644 --- a/opwen_email_server/services/storage.py +++ b/opwen_email_server/services/storage.py @@ -21,7 +21,6 @@ from xtarfile.xtarfile import SUPPORTED_FORMATS from opwen_email_server.utils.log import LogMixin -from opwen_email_server.utils.path import get_extension from opwen_email_server.utils.serialization import from_msgpack_bytes from opwen_email_server.utils.serialization import gunzip_bytes from opwen_email_server.utils.serialization import gzip_bytes @@ -67,6 +66,10 @@ def _client(self) -> Container: container = self._driver.get_container(self._container) return container + @property + def _generated_suffix(self) -> str: + return '' + def access_info(self) -> AccessInfo: return AccessInfo( account=self._account, @@ -87,10 +90,25 @@ def delete(self, resource_id: str): resource.delete() self.log_debug('deleted %s', resource_id) - def iter(self) -> Iterator[str]: - for resource in self._client.list_objects(): - extension = get_extension(resource.name) - resource_id = resource.name.replace(extension, '') + def iter(self, prefix: Optional[str] = None) -> Iterator[str]: + try: + # noinspection PyArgumentList + resources = self._driver.iterate_container_objects(self._client, prefix) + except TypeError: + resources = self._driver.iterate_container_objects(self._client) + + for resource in resources: + resource_id = resource.name + + if prefix is not None: + if not resource_id.startswith(prefix): + continue + else: + resource_id = resource_id[len(prefix):] + + if resource_id.endswith(self._generated_suffix): + resource_id = resource_id[:-len(self._generated_suffix)] + yield resource_id self.log_debug('listed %s', resource_id) @@ -135,10 +153,13 @@ def delete(self, resource_id: str): super().delete(filename) def _to_filename(self, resource_id: str) -> str: - extension = f'.{self._extension}.{self._compression}' - if resource_id.endswith(extension): + if resource_id.endswith(self._generated_suffix): return resource_id - return f'{resource_id}{extension}' + return f'{resource_id}{self._generated_suffix}' + + @property + def _generated_suffix(self) -> str: + return f'.{self._extension}.{self._compression}' @property def _extension(self) -> str: diff --git a/opwen_email_server/swagger/client-register.yaml b/opwen_email_server/swagger/client-register.yaml index 96509cff..6d49a7e6 100644 --- a/opwen_email_server/swagger/client-register.yaml +++ b/opwen_email_server/swagger/client-register.yaml @@ -10,6 +10,19 @@ paths: '/': + get: + operationId: opwen_email_server.integration.connexion.client_list + summary: Endpoint to list all registered Lokole clients. + produces: + - application/json + responses: + 200: + description: Information about the clients. + schema: + $ref: '#/definitions/RegistrationInfos' + security: + - basic: [] + post: operationId: opwen_email_server.integration.connexion.client_create summary: Endpoint where Lokole clients register themselves. @@ -100,6 +113,17 @@ definitions: required: - domain + RegistrationInfos: + type: object + properties: + clients: + description: Domains of all registered clients. + type: array + items: + $ref: '#/definitions/RegistrationInfo' + required: + - clients + RegisteredClient: type: object properties: diff --git a/tests/opwen_email_server/services/test_auth.py b/tests/opwen_email_server/services/test_auth.py index aec5f75e..c0502618 100644 --- a/tests/opwen_email_server/services/test_auth.py +++ b/tests/opwen_email_server/services/test_auth.py @@ -10,7 +10,7 @@ from opwen_email_server.services.auth import AnyOfBasicAuth from opwen_email_server.services.auth import BasicAuth from opwen_email_server.services.auth import GithubBasicAuth -from opwen_email_server.services.storage import AzureTextStorage +from opwen_email_server.services.storage import AzureObjectStorage from tests.opwen_email_server.helpers import MockResponses @@ -170,7 +170,7 @@ def test_with_correct_password(self): class AzureAuthTests(TestCase): def setUp(self): self._folder = mkdtemp() - self._storage = AzureTextStorage( + self._storage = AzureObjectStorage( account=self._folder, key='key', container='auth', @@ -191,17 +191,13 @@ def test_inserts_and_retrieves_client(self): self.assertFalse(self._auth.is_owner('domain', 'unknown-user')) self.assertFalse(self._auth.is_owner('unknown-domain', 'owner')) + def test_lists_domains(self): + self._auth.insert('client1', 'domain1', 'owner1') + self._auth.insert('client2', 'domain2', 'owner2') + self.assertEqual(sorted(self._auth.domains()), sorted(['domain1', 'domain2'])) + def test_deletes_client(self): self._auth.insert('client', 'domain', 'owner') self.assertIsNotNone(self._auth.domain_for('client')) self._auth.delete('client', 'domain') self.assertIsNone(self._auth.domain_for('client')) - - def test_inserts_and_retrieves_client_backwards_compatibility_pre_november_2019(self): - # emulate pre november 2019 version of self._auth.insert - self._storage.store_text('client', 'domain') - self._storage.store_text('domain', 'client') - - self.assertEqual(self._auth.domain_for('client'), 'domain') - self.assertEqual(self._auth.client_id_for('domain'), 'client') - self.assertFalse(self._auth.is_owner('domain', 'owner')) diff --git a/tests/opwen_email_server/services/test_storage.py b/tests/opwen_email_server/services/test_storage.py index 92b08968..933fa8b8 100644 --- a/tests/opwen_email_server/services/test_storage.py +++ b/tests/opwen_email_server/services/test_storage.py @@ -45,11 +45,23 @@ def test_stores_fetches_and_deletes_text(self): def test_list(self): self._storage.store_text('resource1', 'a') self._storage.store_text('resource2.txt.gz', 'b') - self.assertEqual(sorted(self._storage.iter()), sorted(['resource1', 'resource2'])) + self._storage.store_text('pa.th/to/re.sou.rce.txt.gz', 'b') + self.assertEqual(sorted(self._storage.iter()), sorted(['resource1', 'resource2', 'pa.th/to/re.sou.rce'])) self._storage.delete('resource2') + self._storage.delete('pa.th/to/re.sou.rce') self.assertEqual(sorted(self._storage.iter()), sorted(['resource1'])) + def test_list_with_prefix(self): + self._storage.store_text('one/a', 'a') + self._storage.store_text('one/b.txt.gz', 'b') + self._storage.store_text('two/c.txt.gz', 'c') + self._storage.store_text('two/d', 'd') + self._storage.store_text('two/e', 'e') + self._storage.store_text('f', 'f') + self.assertEqual(sorted(self._storage.iter('one/')), sorted(['a', 'b'])) + self.assertEqual(sorted(self._storage.iter('two/')), sorted(['c', 'd', 'e'])) + def test_ensure_exists(self): self.assertFalse(isdir(join(self._folder, self._container))) self._storage.ensure_exists() diff --git a/tests/opwen_email_server/test_actions.py b/tests/opwen_email_server/test_actions.py index 572e3be7..79a8ca94 100644 --- a/tests/opwen_email_server/test_actions.py +++ b/tests/opwen_email_server/test_actions.py @@ -544,6 +544,26 @@ def _execute_action(self, *args, **kwargs): return action(*args, **kwargs) +class ListClientsTests(TestCase): + def setUp(self): + self.auth = Mock() + + def test_200(self): + domains = ['1.test.com', '2.test.com'] + + self.auth.domains.return_value = domains + + response = self._execute_action() + + self.assertEqual(response['clients'], [{'domain': '1.test.com'}, {'domain': '2.test.com'}]) + self.auth.domains.assert_called_once() + + def _execute_action(self, *args, **kwargs): + action = actions.ListClients(auth=self.auth) + + return action(*args, **kwargs) + + class GetClientTests(TestCase): def setUp(self): self.auth = Mock()