fix: Close #103 Filter running time entry by owner_id

Author: EliuX

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 <JWT>
+
+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 "<JWT>"
+```
+
+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

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}

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))

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']

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")

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)
 

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

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)

time_tracker_api/security.py

@@ -25,7 +25,7 @@
 }
 
 iss_claim_pattern = re.compile(
-    r"securityioet.b2clogin.com/(?P<tenant_id>[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<tenant_id>[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:

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: