From 3646c123d16458a08c074efd0d89ba76801771b0 Mon Sep 17 00:00:00 2001 From: EliuX Date: Thu, 23 Apr 2020 15:54:40 -0500 Subject: [PATCH] fix: Close #103 Filter running time entry by owner_id --- README.md | 35 +++++++++++++++++-- commons/data_access_layer/cosmos_db.py | 16 +++++---- .../data_access_layer/cosmos_db_test.py | 20 +++++++++-- tests/commons/data_access_layer/sql_test.py | 4 +-- tests/conftest.py | 2 +- tests/time_tracker_api/security_test.py | 9 +++-- .../time_entries/time_entries_model_test.py | 13 ++++--- .../time_entries_namespace_test.py | 8 +++-- time_tracker_api/security.py | 2 +- .../time_entries/time_entries_model.py | 32 +++++++++++------ 10 files changed, 106 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index c49b99f3..000fb54b 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,31 @@ automatically [pip](https://pip.pypa.io/en/stable/) as well. - Open `http://127.0.0.1:5000/` in a browser. You will find in the presented UI a link to the swagger.json with the definition of the api. +### Security +In this API we are requiring authenticated users using JWT. To do so, we are using the library +[PyJWT](https://pypi.org/project/PyJWT/), so in every request to the API we expect a header `Authorization` with a format +like: + +>Bearer + +In the Swagger UI, you will now see a new button called "Authorize": +![image](https://user-images.githubusercontent.com/6514740/80011459-841f7580-8491-11ea-9c23-5bfb8822afe6.png) + +when you click it then you will be notified that you must enter the content of the Authorization header, as mentioned +before: +![image](https://user-images.githubusercontent.com/6514740/80011702-d95b8700-8491-11ea-973a-8aaf3cdadb00.png) + +Click "Authorize" and then close that dialog. From that moment forward you will not have to do it anymore because the +Swagger UI will use that JWT in every call, e.g. +![image](https://user-images.githubusercontent.com/6514740/80011852-0e67d980-8492-11ea-9dd3-2b1efeaa57d8.png) + +If you want to check out the data (claims) that your JWT contains, you can also use the CLI of +[PyJWT](https://pypi.org/project/PyJWT/): +``` +pyjwt decode --no-verify "" +``` + +Bear in mind that this API is not in charge of verifying the authenticity of the JWT, but the API Management. ### Important notes Due to the used technology and particularities on the implementation of this API, it is important that you respect the @@ -164,13 +189,17 @@ python cli.py gen_swagger_json -f ~/Downloads/swagger.json ## Semantic versioning ### Style -We use [angular commit message style](https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#commits) as the standard commit message style. +We use [angular commit message style](https://github.com/angular/angular.js/blob/master/DEVELOPERS.md#commits) as the +standard commit message style. ### Release -1. The release is automatically done by the [TimeTracker CI](https://dev.azure.com/IOET-DevOps/TimeTracker-API/_build?definitionId=1&_a=summary) although can also be done manually. The variable `GH_TOKEN` is required to post releases to Github. The `GH_TOKEN` can be generated following [these steps](https://help.github.com/es/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line). +1. The release is automatically done by the [TimeTracker CI](https://dev.azure.com/IOET-DevOps/TimeTracker-API/_build?definitionId=1&_a=summary) +although can also be done manually. The variable `GH_TOKEN` is required to post releases to Github. The `GH_TOKEN` can +be generated following [these steps](https://help.github.com/es/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line). 2. We use the command `semantic-release publish` after a successful PR to make a release. Check the library -[python-semantic-release](https://python-semantic-release.readthedocs.io/en/latest/commands.html#publish) for details of underlying operations. +[python-semantic-release](https://python-semantic-release.readthedocs.io/en/latest/commands.html#publish) for details of +underlying operations. ## Run as docker container 1. Build image diff --git a/commons/data_access_layer/cosmos_db.py b/commons/data_access_layer/cosmos_db.py index f8f6c391..f064b175 100644 --- a/commons/data_access_layer/cosmos_db.py +++ b/commons/data_access_layer/cosmos_db.py @@ -37,7 +37,7 @@ def from_flask_config(cls, app: Flask): raise EnvironmentError("DATABASE_MASTER_KEY is not defined in the environment") client = cosmos_client.CosmosClient(account_uri, {'masterKey': master_key}, - user_agent="CosmosDBDotnetQuickstart", + user_agent="TimeTrackerAPI", user_agent_overwrite=True) else: client = cosmos_client.CosmosClient.from_connection_string(db_uri) @@ -108,7 +108,7 @@ def create_sql_condition_for_visibility(visible_only: bool, container_name='c') def create_sql_where_conditions(conditions: dict, container_name='c') -> str: where_conditions = [] for k in conditions.keys(): - where_conditions.append('{c}.{var} = @{var}'.format(c=container_name, var=k)) + where_conditions.append(f'{container_name}.{k} = @{k}') if len(where_conditions) > 0: return "AND {where_conditions_clause}".format( @@ -117,14 +117,15 @@ def create_sql_where_conditions(conditions: dict, container_name='c') -> str: return "" @staticmethod - def append_conditions_values(params: list, conditions: dict) -> dict: + def generate_condition_values(conditions: dict) -> dict: + result = [] for k, v in conditions.items(): - params.append({ + result.append({ "name": "@%s" % k, "value": v }) - return params + return result @staticmethod def check_visibility(item, throw_not_found_if_deleted): @@ -152,11 +153,12 @@ def find_all(self, partition_key_value: str, conditions: dict = {}, max_count=No visible_only=True, mapper: Callable = None): # TODO Use the tenant_id param and change container alias max_count = self.get_page_size_or(max_count) - params = self.append_conditions_values([ + params = [ {"name": "@partition_key_value", "value": partition_key_value}, {"name": "@offset", "value": offset}, {"name": "@max_count", "value": max_count}, - ], conditions) + ] + params.extend(self.generate_condition_values(conditions)) result = self.container.query_items(query=""" SELECT * FROM c WHERE c.{partition_key_attribute}=@partition_key_value {conditions_clause} {visibility_condition} {order_clause} diff --git a/tests/commons/data_access_layer/cosmos_db_test.py b/tests/commons/data_access_layer/cosmos_db_test.py index b19cb5c6..571311bc 100644 --- a/tests/commons/data_access_layer/cosmos_db_test.py +++ b/tests/commons/data_access_layer/cosmos_db_test.py @@ -1,4 +1,5 @@ from dataclasses import dataclass +from datetime import timedelta from typing import Callable import pytest @@ -7,7 +8,8 @@ from flask_restplus._http import HTTPStatus from pytest import fail -from commons.data_access_layer.cosmos_db import CosmosDBRepository, CosmosDBModel, CustomError +from commons.data_access_layer.cosmos_db import CosmosDBRepository, CosmosDBModel, CustomError, current_datetime, \ + datetime_str fake = Faker() Faker.seed() @@ -557,7 +559,7 @@ def test_repository_create_sql_where_conditions_with_no_values(cosmos_db_reposit def test_repository_append_conditions_values(cosmos_db_repository: CosmosDBRepository): - result = cosmos_db_repository.append_conditions_values([], {'owner_id': 'mark', 'customer_id': 'ioet'}) + result = cosmos_db_repository.generate_condition_values({'owner_id': 'mark', 'customer_id': 'ioet'}) assert result is not None assert result == [{'name': '@owner_id', 'value': 'mark'}, @@ -586,3 +588,17 @@ def raise_bad_request_if_name_diff_the_one_from_sample_item(data: dict): except Exception as e: assert e.code == HTTPStatus.BAD_REQUEST assert e.description == "Anything" + + +def test_datetime_str_comparison(): + now = current_datetime() + now_str = datetime_str(now) + + assert now_str > datetime_str(now - timedelta(microseconds=1)) + assert now_str < datetime_str(now + timedelta(microseconds=1)) + + assert now_str > datetime_str(now - timedelta(seconds=1)) + assert now_str < datetime_str(now + timedelta(seconds=1)) + + assert now_str > datetime_str(now - timedelta(days=1)) + assert now_str < datetime_str(now + timedelta(days=1)) diff --git a/tests/commons/data_access_layer/sql_test.py b/tests/commons/data_access_layer/sql_test.py index 26f992b8..0c602f15 100644 --- a/tests/commons/data_access_layer/sql_test.py +++ b/tests/commons/data_access_layer/sql_test.py @@ -56,10 +56,10 @@ def test_find_all_that_contains_property_with_string_case_insensitive(sql_reposi existing_elements_registry.append(new_element) search_snow_result = sql_repository.find_all_contain_str('name', 'Snow') - assert len(search_snow_result) == 2 + assert 2 == len(search_snow_result) search_jon_result = sql_repository.find_all_contain_str('name', 'Jon') - assert len(search_jon_result) == 1 + assert 1 == len(search_jon_result) search_ram_result = sql_repository.find_all_contain_str('name', fake_name) assert search_ram_result[0].name == new_element['name'] diff --git a/tests/conftest.py b/tests/conftest.py index 0f1feb23..97ca2862 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -167,7 +167,7 @@ def running_time_entry(time_entry_repository: TimeEntryCosmosDBRepository, def valid_jwt(app: Flask, tenant_id: str, owner_id: str) -> str: expiration_time = datetime.utcnow() + timedelta(seconds=3600) return jwt.encode({ - "iss": "https://securityioet.b2clogin.com/%s/v2.0/" % tenant_id, + "iss": "https://ioetec.b2clogin.com/%s/v2.0/" % tenant_id, "oid": owner_id, 'exp': expiration_time }, key=get_or_generate_dev_secret_key()).decode("UTF-8") diff --git a/tests/time_tracker_api/security_test.py b/tests/time_tracker_api/security_test.py index 54ffe5e8..7b62eb45 100644 --- a/tests/time_tracker_api/security_test.py +++ b/tests/time_tracker_api/security_test.py @@ -1,3 +1,5 @@ +import pytest + from time_tracker_api.security import parse_jwt, parse_tenant_id_from_iss_claim @@ -14,8 +16,11 @@ def test_parse_jwt_with_invalid_input(): assert result is None -def test_parse_tenant_id_from_iss_claim_with_valid_input(): - valid_iss_claim = "https://securityioet.b2clogin.com/b21c4e98-c4bf-420f-9d76-e51c2515c7a4/v2.0/" +@pytest.mark.parametrize( + 'domain_prefix', ['securityioet', 'ioetec', 'anything-else'] +) +def test_parse_tenant_id_from_iss_claim_with_valid_input(domain_prefix): + valid_iss_claim = f'https://{domain_prefix}.b2clogin.com/b21c4e98-c4bf-420f-9d76-e51c2515c7a4/v2.0/' result = parse_tenant_id_from_iss_claim(valid_iss_claim) diff --git a/tests/time_tracker_api/time_entries/time_entries_model_test.py b/tests/time_tracker_api/time_entries/time_entries_model_test.py index 43bd998c..80505625 100644 --- a/tests/time_tracker_api/time_entries/time_entries_model_test.py +++ b/tests/time_tracker_api/time_entries/time_entries_model_test.py @@ -81,16 +81,21 @@ def test_find_interception_should_ignore_id_of_existing_item(owner_id: str, def test_find_running_should_return_running_time_entry(running_time_entry, + owner_id: str, time_entry_repository: TimeEntryCosmosDBRepository): - found_time_entry = time_entry_repository.find_running(partition_key_value=running_time_entry.tenant_id) + found_time_entry = time_entry_repository.find_running(partition_key_value=running_time_entry.tenant_id, + owner_id=owner_id) - assert found_time_entry is not None - assert found_time_entry.id == running_time_entry.id + assert found_time_entry is not None + assert found_time_entry.id == running_time_entry.id + assert found_time_entry.owner_id == running_time_entry.owner_id def test_find_running_should_not_find_any_item(tenant_id: str, + owner_id: str, time_entry_repository: TimeEntryCosmosDBRepository): try: - time_entry_repository.find_running(partition_key_value=tenant_id) + time_entry_repository.find_running(partition_key_value=tenant_id, + owner_id=owner_id) except Exception as e: assert type(e) is StopIteration diff --git a/tests/time_tracker_api/time_entries/time_entries_namespace_test.py b/tests/time_tracker_api/time_entries/time_entries_namespace_test.py index f3fe83bb..5ddc9741 100644 --- a/tests/time_tracker_api/time_entries/time_entries_namespace_test.py +++ b/tests/time_tracker_api/time_entries/time_entries_namespace_test.py @@ -390,6 +390,7 @@ def test_restart_time_entry_with_id_with_invalid_format(client: FlaskClient, def test_get_running_should_call_find_running(client: FlaskClient, mocker: MockFixture, valid_header: dict, + owner_id: str, tenant_id: str): from time_tracker_api.time_entries.time_entries_namespace import time_entries_dao repository_update_mock = mocker.patch.object(time_entries_dao.repository, @@ -402,12 +403,14 @@ def test_get_running_should_call_find_running(client: FlaskClient, assert HTTPStatus.OK == response.status_code assert json.loads(response.data) is not None - repository_update_mock.assert_called_once_with(partition_key_value=tenant_id) + repository_update_mock.assert_called_once_with(partition_key_value=tenant_id, + owner_id=owner_id) def test_get_running_should_return_not_found_if_find_running_throws_StopIteration(client: FlaskClient, mocker: MockFixture, valid_header: dict, + owner_id: str, tenant_id: str): from time_tracker_api.time_entries.time_entries_namespace import time_entries_dao repository_update_mock = mocker.patch.object(time_entries_dao.repository, @@ -419,4 +422,5 @@ def test_get_running_should_return_not_found_if_find_running_throws_StopIteratio follow_redirects=True) assert HTTPStatus.NOT_FOUND == response.status_code - repository_update_mock.assert_called_once_with(partition_key_value=tenant_id) + repository_update_mock.assert_called_once_with(partition_key_value=tenant_id, + owner_id=owner_id) diff --git a/time_tracker_api/security.py b/time_tracker_api/security.py index d0c382ec..754671e6 100644 --- a/time_tracker_api/security.py +++ b/time_tracker_api/security.py @@ -25,7 +25,7 @@ } iss_claim_pattern = re.compile( - r"securityioet.b2clogin.com/(?P[0-9a-f]{8}\-[0-9a-f]{4}\-4[0-9a-f]{3}\-[89ab][0-9a-f]{3}\-[0-9a-f]{12})") + r"(.*).b2clogin.com/(?P[0-9a-f]{8}\-[0-9a-f]{4}\-4[0-9a-f]{3}\-[89ab][0-9a-f]{3}\-[0-9a-f]{12})") def current_user_id() -> str: diff --git a/time_tracker_api/time_entries/time_entries_model.py b/time_tracker_api/time_entries/time_entries_model.py index cfecfc79..2ba04889 100644 --- a/time_tracker_api/time_entries/time_entries_model.py +++ b/time_tracker_api/time_entries/time_entries_model.py @@ -88,20 +88,22 @@ def on_update(self, updated_item_data: dict): def find_interception_with_date_range(self, start_date, end_date, owner_id, partition_key_value, ignore_id=None, visible_only=True, mapper: Callable = None): - conditions = {"owner_id": owner_id} - params = self.append_conditions_values([ - {"name": "@partition_key_value", "value": partition_key_value}, + conditions = { + "owner_id": owner_id, + "tenant_id": partition_key_value, + } + params = [ {"name": "@start_date", "value": start_date}, {"name": "@end_date", "value": end_date or current_datetime_str()}, {"name": "@ignore_id", "value": ignore_id}, - ], conditions) + ] + params.extend(self.generate_condition_values(conditions)) result = self.container.query_items( query=""" - SELECT * FROM c WHERE c.tenant_id=@partition_key_value - AND ((c.start_date BETWEEN @start_date AND @end_date) OR (c.end_date BETWEEN @start_date AND @end_date)) + SELECT * FROM c WHERE ((c.start_date BETWEEN @start_date AND @end_date) + OR (c.end_date BETWEEN @start_date AND @end_date)) {conditions_clause} {ignore_id_condition} {visibility_condition} {order_clause} - """.format(partition_key_attribute=self.partition_key_attribute, - ignore_id_condition=self.create_sql_ignore_id_condition(ignore_id), + """.format(ignore_id_condition=self.create_sql_ignore_id_condition(ignore_id), visibility_condition=self.create_sql_condition_for_visibility(visible_only), conditions_clause=self.create_sql_where_conditions(conditions), order_clause=self.create_sql_order_clause()), @@ -111,15 +113,22 @@ def find_interception_with_date_range(self, start_date, end_date, owner_id, part function_mapper = self.get_mapper_or_dict(mapper) return list(map(function_mapper, result)) - def find_running(self, partition_key_value: str, mapper: Callable = None): + def find_running(self, partition_key_value: str, owner_id: str, mapper: Callable = None): + conditions = { + "owner_id": owner_id, + "tenant_id": partition_key_value, + } result = self.container.query_items( query=""" SELECT * from c - WHERE (NOT IS_DEFINED(c.end_date) OR c.end_date = null) {visibility_condition} + WHERE (NOT IS_DEFINED(c.end_date) OR c.end_date = null) + {conditions_clause} {visibility_condition} OFFSET 0 LIMIT 1 """.format( visibility_condition=self.create_sql_condition_for_visibility(True), + conditions_clause=self.create_sql_where_conditions(conditions), ), + parameters=self.generate_condition_values(conditions), partition_key=partition_key_value, max_item_count=1) @@ -183,7 +192,8 @@ def delete(self, id): peeker=self.check_whether_current_user_owns_item) def find_running(self): - return self.repository.find_running(partition_key_value=self.partition_key_value) + return self.repository.find_running(partition_key_value=self.partition_key_value, + owner_id=self.current_user_id()) def create_dao() -> TimeEntriesDao: