From a19db9cdf9dc30af7fbd00096c8028c2fed1853c Mon Sep 17 00:00:00 2001 From: Niels Hamers Date: Tue, 13 Apr 2021 14:43:10 +0200 Subject: [PATCH 1/7] EP-3769: implement download result with signed url's and optional expiration timestamp --- openeo_driver/errors.py | 11 +++ openeo_driver/views.py | 58 ++++++++++++-- tests/test_views.py | 168 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 231 insertions(+), 6 deletions(-) diff --git a/openeo_driver/errors.py b/openeo_driver/errors.py index b81be47d..82bda7ca 100644 --- a/openeo_driver/errors.py +++ b/openeo_driver/errors.py @@ -215,6 +215,17 @@ def __init__(self, filename: str): super().__init__(message=self.message.format(file=filename)) +class FileExpiredException(OpenEOApiException): + status_code = 410 + code = 'FileExpired' + message = "File '{file}' has expired." + _description = 'The requested file does not exist anymore.' + _tags = ['File Management'] + + def __init__(self, filename: str): + super().__init__(message=self.message.format(file=filename)) + + class FileContentInvalidException(OpenEOApiException): status_code = 400 code = 'FileContentInvalid' diff --git a/openeo_driver/views.py b/openeo_driver/views.py index b488cdbe..98e84e16 100644 --- a/openeo_driver/views.py +++ b/openeo_driver/views.py @@ -1,12 +1,16 @@ +import base64 import copy import datetime import functools import logging import os import re -import uuid +import time +import typing from collections import namedtuple, defaultdict -from typing import Callable, Tuple, List +import uuid +from hashlib import md5 +from typing import Callable, Tuple, List, Union import flask import flask_cors @@ -24,7 +28,8 @@ from openeo_driver.datacube import DriverDataCube from openeo_driver.delayed_vector import DelayedVector from openeo_driver.errors import OpenEOApiException, ProcessGraphMissingException, ServiceNotFoundException, \ - FilePathInvalidException, ProcessGraphNotFoundException, FeatureUnsupportedException + FilePathInvalidException, ProcessGraphNotFoundException, FeatureUnsupportedException, CredentialsInvalidException, \ + FileExpiredException from openeo_driver.processes import DEFAULT_NAMESPACE from openeo_driver.save_result import SaveResult, get_temp_file from openeo_driver.users import HttpAuthHandler, User @@ -591,13 +596,34 @@ def list_job_results(job_id, user: User): job_info = backend_implementation.batch_jobs.get_job_info(job_id, user.user_id) results = backend_implementation.batch_jobs.get_results(job_id=job_id, user_id=user.user_id) + def base64_user_id() -> str: + return base64.urlsafe_b64encode(user.user_id.encode()).decode() + + def secure_token(filename, expires) -> str: + return _compute_secure_token(job_id, user.user_id, filename, expires) + + def expiration_timestamp() -> Union[str, None]: + if 'SIGNED_URL_EXPIRATION' in os.environ: + return str(time.time() + int(os.environ['SIGNED_URL_EXPIRATION'])) + else: + return None + + def download_url(filename) -> str: + if os.getenv("SIGNED_URL", "FALSE").upper() == "TRUE": + expires = expiration_timestamp() + return url_for('.download_job_result_signed', job_id=job_id, user_base64=base64_user_id(), + secure_key=secure_token(filename, expires), filename=filename, + expires=expires, _external=True) + else: + return url_for('.download_job_result', job_id=job_id, filename=filename, _external=True) + def asset_object(filename: str, asset_metadata: dict) -> dict: bands = asset_metadata.get("bands") nodata = asset_metadata.get("nodata") return dict_no_none(**{ "title": asset_metadata.get("title",filename), #there has to be title - "href": url_for('.download_job_result', job_id=job_id, filename=filename, _external=True), + "href": download_url(filename), "type": asset_metadata.get("media_type"), "eo:bands": [dict_no_none(**{"name": band.name, "center_wavelength": band.wavelength_um}) for band in bands] if bands else None, @@ -645,8 +671,7 @@ def asset_object(filename: str, asset_metadata: dict) -> dict: else: result = { "links": [ - {"href": url_for('.download_job_result', job_id=job_id, filename=filename, _external=True)} - for filename in results.keys() + {"href": download_url(filename)} for filename in results.keys() ] } @@ -702,6 +727,27 @@ def download_job_result(job_id, filename, user: User): return send_from_directory(output_dir, filename, mimetype=results[filename].get("media_type")) +@api_endpoint +@openeo_bp.route('/jobs//results///', methods=['GET']) +def download_job_result_signed(job_id, user_base64, secure_key, filename): + expires = request.args.get('expires') + user_id = base64.urlsafe_b64decode(user_base64).decode() + if secure_key != _compute_secure_token(job_id, user_id, filename, expires): + raise CredentialsInvalidException() + if expires and int(expires) < time.time(): + raise FileExpiredException(str(filename)) + results = backend_implementation.batch_jobs.get_results(job_id=job_id, user_id=user_id) + if filename not in results.keys(): + raise FilePathInvalidException(str(filename) + ' not in ' + str(list(results.keys()))) + output_dir = results[filename]["output_dir"] + return send_from_directory(output_dir, filename, mimetype=results[filename].get("media_type")) + + +def _compute_secure_token(job_id, user_id, filename, expiration_timestamp): + token_key = job_id + user_id + filename + str(expiration_timestamp) + os.environ['SIGNED_URL_SECRET'] + return md5(token_key.encode()).hexdigest() + + @api_endpoint @openeo_bp.route('/jobs//logs', methods=['GET']) @auth_handler.requires_bearer_auth diff --git a/tests/test_views.py b/tests/test_views.py index e7c87057..b8134154 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -864,6 +864,138 @@ def test_get_job_results_100(self, api100): 'type': 'Feature' } + @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) + def test_get_job_results_signed_040(self, api040): + with self._fresh_job_registry(next_job_id='job-370'): + dummy_backend.DummyBatchJobs._update_status( + job_id='07024ee9-7847-4b8a-b260-6c879a2b3cdc', user_id=TEST_USER, status='finished') + resp = api040.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results', headers=self.AUTH_HEADER) + assert resp.assert_status_code(200).json == { + 'links': [ + { + 'href': 'http://oeo.net/openeo/0.4.0/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/50afb0cad129e61d415278c4ffcd8a83/output.tiff' + } + ] + } + + @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) + def test_get_job_results_signed_100(self, api100): + with self._fresh_job_registry(next_job_id='job-372'): + dummy_backend.DummyBatchJobs._update_status( + job_id='07024ee9-7847-4b8a-b260-6c879a2b3cdc', user_id=TEST_USER, status='finished') + resp = api100.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results', headers=self.AUTH_HEADER) + assert resp.assert_status_code(200).json == { + 'assets': { + 'output.tiff': { + 'roles': ['data'], + 'title': 'output.tiff', + 'href': 'http://oeo.net/openeo/1.0.0/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/50afb0cad129e61d415278c4ffcd8a83/output.tiff', + 'type': 'image/tiff; application=geotiff', + 'eo:bands': [{ + 'name': 'NDVI', + 'center_wavelength': 1.23 + }], + 'file:nodata': [123] + } + }, + 'geometry': None, + 'id': '07024ee9-7847-4b8a-b260-6c879a2b3cdc', + 'links': [ + { + 'rel': 'self', + 'href': 'http://oeo.net/openeo/1.0.0/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results', + 'type': 'application/json' + }, + { + 'rel': 'card4l-document', + 'href': 'http://ceos.org/ard/files/PFS/SR/v5.0/CARD4L_Product_Family_Specification_Surface_Reflectance-v5.0.pdf', + 'type': 'application/pdf' + } + ], + 'properties': { + 'created': '2017-01-01T09:32:12Z', + 'datetime': None, + 'card4l:processing_chain': {'process_graph': {'foo': {'process_id': 'foo', 'arguments': {}}}}, + 'card4l:specification': 'SR', + 'card4l:specification_version': '5.0', + 'processing:facility': 'VITO - SPARK', + 'processing:software': 'openeo-geotrellis-0.0.1' + }, + 'stac_extensions': ['processing', + 'card4l-eo', + 'https://stac-extensions.github.io/file/v1.0.0/schema.json', + 'eo'], + 'stac_version': '0.9.0', + 'type': 'Feature' + } + + @mock.patch('time.time', mock.MagicMock(return_value=1234)) + @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) + def test_get_job_results_signed_with_expiration_040(self, api040): + with self._fresh_job_registry(next_job_id='job-371'): + dummy_backend.DummyBatchJobs._update_status( + job_id='07024ee9-7847-4b8a-b260-6c879a2b3cdc', user_id=TEST_USER, status='finished') + resp = api040.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results', headers=self.AUTH_HEADER) + assert resp.assert_status_code(200).json == { + 'links': [ + { + 'href': 'http://oeo.net/openeo/0.4.0/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/fd0ca65e29c6d223da05b2e73a875683/output.tiff?expires=2234' + } + ] + } + + @mock.patch('time.time', mock.MagicMock(return_value=1234)) + @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) + def test_get_job_results_signed_with_expiration_100(self, api100): + with self._fresh_job_registry(next_job_id='job-373'): + dummy_backend.DummyBatchJobs._update_status( + job_id='07024ee9-7847-4b8a-b260-6c879a2b3cdc', user_id=TEST_USER, status='finished') + resp = api100.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results', headers=self.AUTH_HEADER) + assert resp.assert_status_code(200).json == { + 'assets': { + 'output.tiff': { + 'roles': ['data'], + 'title': 'output.tiff', + 'href': 'http://oeo.net/openeo/1.0.0/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/fd0ca65e29c6d223da05b2e73a875683/output.tiff?expires=2234', + 'type': 'image/tiff; application=geotiff', + 'eo:bands': [{ + 'name': 'NDVI', + 'center_wavelength': 1.23 + }], + 'file:nodata': [123] + } + }, + 'geometry': None, + 'id': '07024ee9-7847-4b8a-b260-6c879a2b3cdc', + 'links': [ + { + 'rel': 'self', + 'href': 'http://oeo.net/openeo/1.0.0/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results', + 'type': 'application/json' + }, + { + 'rel': 'card4l-document', + 'href': 'http://ceos.org/ard/files/PFS/SR/v5.0/CARD4L_Product_Family_Specification_Surface_Reflectance-v5.0.pdf', + 'type': 'application/pdf' + } + ], + 'properties': { + 'created': '2017-01-01T09:32:12Z', + 'datetime': None, + 'card4l:processing_chain': {'process_graph': {'foo': {'process_id': 'foo', 'arguments': {}}}}, + 'card4l:specification': 'SR', + 'card4l:specification_version': '5.0', + 'processing:facility': 'VITO - SPARK', + 'processing:software': 'openeo-geotrellis-0.0.1' + }, + 'stac_extensions': ['processing', + 'card4l-eo', + 'https://stac-extensions.github.io/file/v1.0.0/schema.json', + 'eo'], + 'stac_version': '0.9.0', + 'type': 'Feature' + } + def test_get_job_results_invalid_job(self, api): api.get('/jobs/deadbeef-f00/results', headers=self.AUTH_HEADER).assert_error(404, "JobNotFound") @@ -881,6 +1013,42 @@ def test_download_result(self, api, tmp_path): assert resp.assert_status_code(200).data == b"tiffdata" assert resp.headers["Content-Type"] == "image/tiff; application=geotiff" + @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) + def test_download_result_signed(self, api, tmp_path): + output_root = Path(tmp_path) + with mock.patch.object(dummy_backend.DummyBatchJobs, '_output_root', return_value=output_root): + output = output_root / '07024ee9-7847-4b8a-b260-6c879a2b3cdc' / 'output.tiff' + output.parent.mkdir(parents=True) + with output.open('wb') as f: + f.write(b'tiffdata') + resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/50afb0cad129e61d415278c4ffcd8a83/output.tiff') + assert resp.assert_status_code(200).data == b'tiffdata' + assert resp.headers['Content-Type'] == 'image/tiff; application=geotiff' + + @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) + def test_download_result_signed_invalid(self, api): + resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/test123/output.tiff') + assert resp.assert_error(403, 'CredentialsInvalid') + + @mock.patch('time.time', mock.MagicMock(return_value=1234)) + @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) + def test_download_result_signed_with_expiration(self, api, tmp_path): + output_root = Path(tmp_path) + with mock.patch.object(dummy_backend.DummyBatchJobs, '_output_root', return_value=output_root): + output = output_root / '07024ee9-7847-4b8a-b260-6c879a2b3cdc' / 'output.tiff' + output.parent.mkdir(parents=True) + with output.open('wb') as f: + f.write(b'tiffdata') + resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/fd0ca65e29c6d223da05b2e73a875683/output.tiff?expires=2234') + assert resp.assert_status_code(200).data == b'tiffdata' + assert resp.headers['Content-Type'] == 'image/tiff; application=geotiff' + + @mock.patch('time.time', mock.MagicMock(return_value=3456)) + @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) + def test_download_result_signed_with_expiration_invalid(self, api, tmp_path): + resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/fd0ca65e29c6d223da05b2e73a875683/output.tiff?expires=2234') + assert resp.assert_error(410, 'FileExpired') + def test_get_batch_job_logs(self, api): resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/logs', headers=self.AUTH_HEADER) assert resp.assert_status_code(200).json == { From 48cc1c4805eebc971b628f87e3b146abfaec719a Mon Sep 17 00:00:00 2001 From: Niels Hamers Date: Wed, 14 Apr 2021 14:59:47 +0200 Subject: [PATCH 2/7] EP-3769: address comments made in pull request --- openeo_driver/server.py | 4 ++++ openeo_driver/views.py | 19 +++++++++++-------- tests/test_views.py | 16 ++++++++-------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/openeo_driver/server.py b/openeo_driver/server.py index 4cc492fa..e67c1763 100644 --- a/openeo_driver/server.py +++ b/openeo_driver/server.py @@ -1,4 +1,5 @@ import logging +import os from typing import Union import gunicorn.app.base @@ -21,6 +22,9 @@ def run(title: str, description: str, deploy_metadata: Union[dict, None], backen app.config['OPENEO_DESCRIPTION'] = description app.config['OPENEO_BACKEND_DEPLOY_METADATA'] = deploy_metadata app.config['MAX_CONTENT_LENGTH'] = 1024 * 1024 # bytes + app.config['SIGNED_URL'] = os.getenv('SIGNED_URL') + app.config['SIGNED_URL_SECRET'] = os.getenv('SIGNED_URL_SECRET') + app.config['SIGNED_URL_EXPIRATION'] = os.getenv('SIGNED_URL_EXPIRATION') app.logger.info('App info logging enabled!') app.logger.debug('App debug logging enabled!') diff --git a/openeo_driver/views.py b/openeo_driver/views.py index 98e84e16..670e9cdc 100644 --- a/openeo_driver/views.py +++ b/openeo_driver/views.py @@ -602,14 +602,12 @@ def base64_user_id() -> str: def secure_token(filename, expires) -> str: return _compute_secure_token(job_id, user.user_id, filename, expires) - def expiration_timestamp() -> Union[str, None]: - if 'SIGNED_URL_EXPIRATION' in os.environ: - return str(time.time() + int(os.environ['SIGNED_URL_EXPIRATION'])) - else: - return None + def expiration_timestamp() -> Union[int, None]: + expiration = current_app.config.get('SIGNED_URL_EXPIRATION') + return time.time() + int(expiration) if expiration else None def download_url(filename) -> str: - if os.getenv("SIGNED_URL", "FALSE").upper() == "TRUE": + if smart_bool(current_app.config.get('SIGNED_URL')): expires = expiration_timestamp() return url_for('.download_job_result_signed', job_id=job_id, user_base64=base64_user_id(), secure_key=secure_token(filename, expires), filename=filename, @@ -735,7 +733,12 @@ def download_job_result_signed(job_id, user_base64, secure_key, filename): if secure_key != _compute_secure_token(job_id, user_id, filename, expires): raise CredentialsInvalidException() if expires and int(expires) < time.time(): - raise FileExpiredException(str(filename)) + #TODO: use dedicated exception (https://github.com/Open-EO/openeo-api/issues/379) + raise OpenEOApiException( + status_code=410, + code="FileExpired", + message="File '{file}' has expired".format(file=filename) + ) results = backend_implementation.batch_jobs.get_results(job_id=job_id, user_id=user_id) if filename not in results.keys(): raise FilePathInvalidException(str(filename) + ' not in ' + str(list(results.keys()))) @@ -744,7 +747,7 @@ def download_job_result_signed(job_id, user_base64, secure_key, filename): def _compute_secure_token(job_id, user_id, filename, expiration_timestamp): - token_key = job_id + user_id + filename + str(expiration_timestamp) + os.environ['SIGNED_URL_SECRET'] + token_key = job_id + user_id + filename + str(expiration_timestamp) + current_app.config.get('SIGNED_URL_SECRET') return md5(token_key.encode()).hexdigest() diff --git a/tests/test_views.py b/tests/test_views.py index b8134154..8ed87486 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -864,7 +864,7 @@ def test_get_job_results_100(self, api100): 'type': 'Feature' } - @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) + @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) def test_get_job_results_signed_040(self, api040): with self._fresh_job_registry(next_job_id='job-370'): dummy_backend.DummyBatchJobs._update_status( @@ -878,7 +878,7 @@ def test_get_job_results_signed_040(self, api040): ] } - @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) + @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) def test_get_job_results_signed_100(self, api100): with self._fresh_job_registry(next_job_id='job-372'): dummy_backend.DummyBatchJobs._update_status( @@ -930,7 +930,7 @@ def test_get_job_results_signed_100(self, api100): } @mock.patch('time.time', mock.MagicMock(return_value=1234)) - @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) + @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) def test_get_job_results_signed_with_expiration_040(self, api040): with self._fresh_job_registry(next_job_id='job-371'): dummy_backend.DummyBatchJobs._update_status( @@ -945,7 +945,7 @@ def test_get_job_results_signed_with_expiration_040(self, api040): } @mock.patch('time.time', mock.MagicMock(return_value=1234)) - @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) + @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) def test_get_job_results_signed_with_expiration_100(self, api100): with self._fresh_job_registry(next_job_id='job-373'): dummy_backend.DummyBatchJobs._update_status( @@ -1013,7 +1013,7 @@ def test_download_result(self, api, tmp_path): assert resp.assert_status_code(200).data == b"tiffdata" assert resp.headers["Content-Type"] == "image/tiff; application=geotiff" - @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) + @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) def test_download_result_signed(self, api, tmp_path): output_root = Path(tmp_path) with mock.patch.object(dummy_backend.DummyBatchJobs, '_output_root', return_value=output_root): @@ -1025,13 +1025,13 @@ def test_download_result_signed(self, api, tmp_path): assert resp.assert_status_code(200).data == b'tiffdata' assert resp.headers['Content-Type'] == 'image/tiff; application=geotiff' - @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) + @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#'}) def test_download_result_signed_invalid(self, api): resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/test123/output.tiff') assert resp.assert_error(403, 'CredentialsInvalid') @mock.patch('time.time', mock.MagicMock(return_value=1234)) - @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) + @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) def test_download_result_signed_with_expiration(self, api, tmp_path): output_root = Path(tmp_path) with mock.patch.object(dummy_backend.DummyBatchJobs, '_output_root', return_value=output_root): @@ -1044,7 +1044,7 @@ def test_download_result_signed_with_expiration(self, api, tmp_path): assert resp.headers['Content-Type'] == 'image/tiff; application=geotiff' @mock.patch('time.time', mock.MagicMock(return_value=3456)) - @mock.patch.dict(os.environ, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) + @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) def test_download_result_signed_with_expiration_invalid(self, api, tmp_path): resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/fd0ca65e29c6d223da05b2e73a875683/output.tiff?expires=2234') assert resp.assert_error(410, 'FileExpired') From 64062f316f640197da16f5c330a9ed01583471dd Mon Sep 17 00:00:00 2001 From: Niels Hamers Date: Wed, 14 Apr 2021 15:02:25 +0200 Subject: [PATCH 3/7] EP-3769: use LinkExpired instead of FileExpired --- openeo_driver/views.py | 4 ++-- tests/test_views.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openeo_driver/views.py b/openeo_driver/views.py index 670e9cdc..8334396c 100644 --- a/openeo_driver/views.py +++ b/openeo_driver/views.py @@ -736,8 +736,8 @@ def download_job_result_signed(job_id, user_base64, secure_key, filename): #TODO: use dedicated exception (https://github.com/Open-EO/openeo-api/issues/379) raise OpenEOApiException( status_code=410, - code="FileExpired", - message="File '{file}' has expired".format(file=filename) + code="LinkExpired", + message="Link to file '{file}' has expired".format(file=filename) ) results = backend_implementation.batch_jobs.get_results(job_id=job_id, user_id=user_id) if filename not in results.keys(): diff --git a/tests/test_views.py b/tests/test_views.py index 8ed87486..2a11d366 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1047,7 +1047,7 @@ def test_download_result_signed_with_expiration(self, api, tmp_path): @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) def test_download_result_signed_with_expiration_invalid(self, api, tmp_path): resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/fd0ca65e29c6d223da05b2e73a875683/output.tiff?expires=2234') - assert resp.assert_error(410, 'FileExpired') + assert resp.assert_error(410, 'LinkExpired') def test_get_batch_job_logs(self, api): resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/logs', headers=self.AUTH_HEADER) From ea699c42ac1d402d8a5132fadfef076b441e8bac Mon Sep 17 00:00:00 2001 From: Niels Hamers Date: Wed, 14 Apr 2021 15:09:47 +0200 Subject: [PATCH 4/7] EP-3769: remove FileExpiredException from errors.py --- openeo_driver/errors.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/openeo_driver/errors.py b/openeo_driver/errors.py index 82bda7ca..b81be47d 100644 --- a/openeo_driver/errors.py +++ b/openeo_driver/errors.py @@ -215,17 +215,6 @@ def __init__(self, filename: str): super().__init__(message=self.message.format(file=filename)) -class FileExpiredException(OpenEOApiException): - status_code = 410 - code = 'FileExpired' - message = "File '{file}' has expired." - _description = 'The requested file does not exist anymore.' - _tags = ['File Management'] - - def __init__(self, filename: str): - super().__init__(message=self.message.format(file=filename)) - - class FileContentInvalidException(OpenEOApiException): status_code = 400 code = 'FileContentInvalid' From 6e4386f5c982abcc72d303036c290aa4135bea08 Mon Sep 17 00:00:00 2001 From: Niels Hamers Date: Wed, 14 Apr 2021 15:12:53 +0200 Subject: [PATCH 5/7] EP-3769: remove FileExpiredException import --- openeo_driver/views.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openeo_driver/views.py b/openeo_driver/views.py index 8334396c..75f9fc98 100644 --- a/openeo_driver/views.py +++ b/openeo_driver/views.py @@ -28,8 +28,7 @@ from openeo_driver.datacube import DriverDataCube from openeo_driver.delayed_vector import DelayedVector from openeo_driver.errors import OpenEOApiException, ProcessGraphMissingException, ServiceNotFoundException, \ - FilePathInvalidException, ProcessGraphNotFoundException, FeatureUnsupportedException, CredentialsInvalidException, \ - FileExpiredException + FilePathInvalidException, ProcessGraphNotFoundException, FeatureUnsupportedException, CredentialsInvalidException from openeo_driver.processes import DEFAULT_NAMESPACE from openeo_driver.save_result import SaveResult, get_temp_file from openeo_driver.users import HttpAuthHandler, User From d62f0c079784de6daa0e93fffed6313362028e28 Mon Sep 17 00:00:00 2001 From: Niels Hamers Date: Thu, 15 Apr 2021 16:17:59 +0200 Subject: [PATCH 6/7] EP-3769: use dedicated exception for expired links --- openeo_driver/errors.py | 8 ++++++++ openeo_driver/specs/openeo-api/0.4 | 2 +- openeo_driver/specs/openeo-api/1.0 | 2 +- openeo_driver/views.py | 10 +++------- tests/test_views.py | 2 +- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/openeo_driver/errors.py b/openeo_driver/errors.py index b81be47d..6376e401 100644 --- a/openeo_driver/errors.py +++ b/openeo_driver/errors.py @@ -458,6 +458,14 @@ def __init__(self, process: str, parameter: str): super().__init__(message=self.message.format(process=process, parameter=parameter)) +class ResultLinkExpiredException(OpenEOApiException): + status_code = 410 + code = 'ResultLinkExpired' + message = 'The link to the batch job result has expired. Please request the results again.' + _description = 'The signed URLs for batch job results have expired. Please send a request to `GET /jobs/{job_id}/results` to refresh the links.' + _tags: ['Batch Jobs'] + + class ServiceConfigUnsupportedException(OpenEOApiException): status_code = 400 code = 'ServiceConfigUnsupported' diff --git a/openeo_driver/specs/openeo-api/0.4 b/openeo_driver/specs/openeo-api/0.4 index c0c5e8dd..7086a792 160000 --- a/openeo_driver/specs/openeo-api/0.4 +++ b/openeo_driver/specs/openeo-api/0.4 @@ -1 +1 @@ -Subproject commit c0c5e8ddebe57d074df458be6604a4e480b0ef59 +Subproject commit 7086a79235253aecac4430614f56742766d90ff4 diff --git a/openeo_driver/specs/openeo-api/1.0 b/openeo_driver/specs/openeo-api/1.0 index 81dd248b..7086a792 160000 --- a/openeo_driver/specs/openeo-api/1.0 +++ b/openeo_driver/specs/openeo-api/1.0 @@ -1 +1 @@ -Subproject commit 81dd248b9e8b8caeedbdd96c6a9dd8f3f56d0eb7 +Subproject commit 7086a79235253aecac4430614f56742766d90ff4 diff --git a/openeo_driver/views.py b/openeo_driver/views.py index 75f9fc98..ee2e97ea 100644 --- a/openeo_driver/views.py +++ b/openeo_driver/views.py @@ -28,7 +28,8 @@ from openeo_driver.datacube import DriverDataCube from openeo_driver.delayed_vector import DelayedVector from openeo_driver.errors import OpenEOApiException, ProcessGraphMissingException, ServiceNotFoundException, \ - FilePathInvalidException, ProcessGraphNotFoundException, FeatureUnsupportedException, CredentialsInvalidException + FilePathInvalidException, ProcessGraphNotFoundException, FeatureUnsupportedException, CredentialsInvalidException, \ + ResultLinkExpiredException from openeo_driver.processes import DEFAULT_NAMESPACE from openeo_driver.save_result import SaveResult, get_temp_file from openeo_driver.users import HttpAuthHandler, User @@ -732,12 +733,7 @@ def download_job_result_signed(job_id, user_base64, secure_key, filename): if secure_key != _compute_secure_token(job_id, user_id, filename, expires): raise CredentialsInvalidException() if expires and int(expires) < time.time(): - #TODO: use dedicated exception (https://github.com/Open-EO/openeo-api/issues/379) - raise OpenEOApiException( - status_code=410, - code="LinkExpired", - message="Link to file '{file}' has expired".format(file=filename) - ) + raise ResultLinkExpiredException() results = backend_implementation.batch_jobs.get_results(job_id=job_id, user_id=user_id) if filename not in results.keys(): raise FilePathInvalidException(str(filename) + ' not in ' + str(list(results.keys()))) diff --git a/tests/test_views.py b/tests/test_views.py index 2a11d366..5c0b5380 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1047,7 +1047,7 @@ def test_download_result_signed_with_expiration(self, api, tmp_path): @mock.patch.dict(app.config, {'SIGNED_URL': 'TRUE', 'SIGNED_URL_SECRET': '123&@#', 'SIGNED_URL_EXPIRATION': '1000'}) def test_download_result_signed_with_expiration_invalid(self, api, tmp_path): resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/results/TXIuVGVzdA%3D%3D/fd0ca65e29c6d223da05b2e73a875683/output.tiff?expires=2234') - assert resp.assert_error(410, 'LinkExpired') + assert resp.assert_error(410, 'ResultLinkExpired') def test_get_batch_job_logs(self, api): resp = api.get('/jobs/07024ee9-7847-4b8a-b260-6c879a2b3cdc/logs', headers=self.AUTH_HEADER) From 7663fbdf5c654e1cca23d46b6e838fe5615910ba Mon Sep 17 00:00:00 2001 From: Niels Hamers Date: Fri, 16 Apr 2021 09:45:12 +0200 Subject: [PATCH 7/7] EP-3769: fix tags of ResultLinkExpiredException --- openeo_driver/errors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openeo_driver/errors.py b/openeo_driver/errors.py index 6376e401..e58d63aa 100644 --- a/openeo_driver/errors.py +++ b/openeo_driver/errors.py @@ -463,7 +463,7 @@ class ResultLinkExpiredException(OpenEOApiException): code = 'ResultLinkExpired' message = 'The link to the batch job result has expired. Please request the results again.' _description = 'The signed URLs for batch job results have expired. Please send a request to `GET /jobs/{job_id}/results` to refresh the links.' - _tags: ['Batch Jobs'] + _tags = ['Batch Jobs'] class ServiceConfigUnsupportedException(OpenEOApiException):