From df14af5322b232a8e3b2e54476386d2cc55bb16a Mon Sep 17 00:00:00 2001 From: Matheus Lima Date: Mon, 18 Mar 2019 16:34:34 -0300 Subject: [PATCH] Implemented etag support on the endpoint `GET /device/{uuid}` --- api/public/public_api/endpoints/device.py | 4 ++ api/public/tests/features/get_device.feature | 13 +++- api/public/tests/features/steps/get_device.py | 68 ++++++++++++++++++- shared/selene/api/blueprint.py | 6 ++ shared/selene/api/public_endpoint.py | 27 +++++++- shared/selene/util/not_modified.py | 3 + 6 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 shared/selene/util/not_modified.py diff --git a/api/public/public_api/endpoints/device.py b/api/public/public_api/endpoints/device.py index 1f1b4473..1f72e44a 100644 --- a/api/public/public_api/endpoints/device.py +++ b/api/public/public_api/endpoints/device.py @@ -23,6 +23,8 @@ class DeviceEndpoint(PublicEndpoint): def get(self, device_id): self._authenticate(device_id) + etag_key = 'device.etag:{uuid}'.format(uuid=device_id) + self._validate_etag(etag_key) with get_db_connection(self.config['DB_CONNECTION_POOL']) as db: device = DeviceRepository(db).get_device_by_id(device_id) if device: @@ -35,6 +37,8 @@ class DeviceEndpoint(PublicEndpoint): device['user'] = dict(uuid=device['account_id']) del device['account_id'] response = device, HTTPStatus.OK + + self._add_etag(etag_key) else: response = '', HTTPStatus.NO_CONTENT return response diff --git a/api/public/tests/features/get_device.feature b/api/public/tests/features/get_device.feature index 8aa11ac2..78696e51 100644 --- a/api/public/tests/features/get_device.feature +++ b/api/public/tests/features/get_device.feature @@ -16,4 +16,15 @@ Feature: Get device's information Scenario: Update device information When the device is updated And device is retrieved - Then the information should be updated \ No newline at end of file + Then the information should be updated + + Scenario: Get a not modified device using etag + When device is retrieved + And try to fetch a device using a valid etag + Then 304 status code should be returned by the device endpoint + + Scenario: Get a device using an expired etag + Given an etag expired by selene ui + When try to fetch a device using an expired etag + Then should return status 200 + And a new etag \ No newline at end of file diff --git a/api/public/tests/features/steps/get_device.py b/api/public/tests/features/steps/get_device.py index 277e8c9b..36777819 100644 --- a/api/public/tests/features/steps/get_device.py +++ b/api/public/tests/features/steps/get_device.py @@ -2,8 +2,10 @@ import json import uuid from http import HTTPStatus -from behave import when, then -from hamcrest import assert_that, equal_to, has_key +from behave import when, then, given +from hamcrest import assert_that, equal_to, has_key, not_none, is_not + +from selene.util.cache import SeleneCache new_fields = dict( platform='mycroft_mark_1', @@ -21,6 +23,7 @@ def get_device(context): '/v1/device/{uuid}'.format(uuid=device_id), headers=headers ) + context.device_etag = context.get_device_response.headers.get('ETag') @then('a valid device should be returned') @@ -86,3 +89,64 @@ def validate_update(context): assert_that(device['coreVersion'], equal_to(new_fields['coreVersion'])) assert_that(device['enclosureVersion'], equal_to(new_fields['enclosureVersion'])) assert_that(device['platform'], equal_to(new_fields['platform'])) + + +@when('try to fetch a device using a valid etag') +def get_device_using_etag(context): + etag = context.device_etag + assert_that(etag, not_none()) + access_token = context.device_login['accessToken'] + device_uuid = context.device_login['uuid'] + headers = { + 'Authorization': 'Bearer {token}'.format(token=access_token), + 'If-None-Match': etag + } + context.response_using_etag = context.client.get( + '/v1/device/{uuid}'.format(uuid=device_uuid), + headers=headers + ) + + +@then('304 status code should be returned by the device endpoint') +def validate_etag(context): + response = context.response_using_etag + assert_that(response.status_code, equal_to(HTTPStatus.NOT_MODIFIED)) + + +@given('an etag expired by selene ui') +def expire_etag(context): + context.device_etag = '123' + new_etag = '456' + device_id = context.device_login['uuid'] + cache: SeleneCache = context.client_config['SELENE_CACHE'] + cache.set('device.etag:{uuid}'.format(uuid=device_id), new_etag) + + +@when('try to fetch a device using an expired etag') +def fetch_device_expired_etag(context): + etag = context.device_etag + assert_that(etag, not_none()) + access_token = context.device_login['accessToken'] + device_uuid = context.device_login['uuid'] + headers = { + 'Authorization': 'Bearer {token}'.format(token=access_token), + 'If-None-Match': etag + } + context.response_using_invalid_etag = context.client.get( + '/v1/device/{uuid}'.format(uuid=device_uuid), + headers=headers + ) + + +@then('should return status 200') +def validate_status_code(context): + response = context.response_using_invalid_etag + assert_that(response.status_code, equal_to(HTTPStatus.OK)) + + +@then('a new etag') +def validate_new_etag(context): + etag = context.device_etag + response = context.response_using_invalid_etag + etag_from_response = response.headers.get('ETag') + assert_that(etag, is_not(etag_from_response)) diff --git a/shared/selene/api/blueprint.py b/shared/selene/api/blueprint.py index 93045bce..a34d33d5 100644 --- a/shared/selene/api/blueprint.py +++ b/shared/selene/api/blueprint.py @@ -4,6 +4,7 @@ from flask import Blueprint from schematics.exceptions import DataError from selene.util.auth import AuthenticationError +from selene.util.not_modified import NotModifiedError selene_api = Blueprint('selene_api', __name__) @@ -16,3 +17,8 @@ def handle_data_error(error): @selene_api.app_errorhandler(AuthenticationError) def handle_data_error(error): return dict(error=str(error)), HTTPStatus.UNAUTHORIZED + + +@selene_api.app_errorhandler(NotModifiedError) +def handle_not_modified(error): + return '', HTTPStatus.NOT_MODIFIED diff --git a/shared/selene/api/public_endpoint.py b/shared/selene/api/public_endpoint.py index ca4adbcf..a08db2ad 100644 --- a/shared/selene/api/public_endpoint.py +++ b/shared/selene/api/public_endpoint.py @@ -1,11 +1,14 @@ import hashlib import json +import random +import string import uuid -from flask import current_app, request +from flask import current_app, request, Response, after_this_request from flask.views import MethodView from selene.util.auth import AuthenticationError +from selene.util.not_modified import NotModifiedError from ..util.cache import SeleneCache ONE_DAY = 86400 @@ -58,6 +61,8 @@ def generate_device_login(device_id: str, cache: SeleneCache) -> dict: class PublicEndpoint(MethodView): """Abstract class for all endpoints used by Mycroft devices""" + etag_chars = string.ascii_letters + string.digits + def __init__(self): self.config: dict = current_app.config self.request = request @@ -81,3 +86,23 @@ class PublicEndpoint(MethodView): device_authenticated = True if not device_authenticated: raise AuthenticationError('device not authorized') + + def _add_etag(self, key): + """Add a etag header to the response. We try to get the etag from the cache using the given key. + If the cache has the etag, we use it, otherwise we generate a etag, store it and add it to the response""" + etag = self.cache.get(key) + if etag is None: + etag = ''.join(random.choice(self.etag_chars) for _ in range(32)) + self.cache.set(key, etag) + + @after_this_request + def set_etag_header(response: Response): + response.headers['ETag'] = etag + return response + + def _validate_etag(self, key): + etag_from_request = self.request.headers.get('If-None-Match') + if etag_from_request is not None: + etag_from_cache = self.cache.get(key) + if etag_from_cache is not None and etag_from_request == etag_from_cache.decode('utf-8'): + raise NotModifiedError() diff --git a/shared/selene/util/not_modified.py b/shared/selene/util/not_modified.py new file mode 100644 index 00000000..fe2dba89 --- /dev/null +++ b/shared/selene/util/not_modified.py @@ -0,0 +1,3 @@ + +class NotModifiedError(Exception): + pass