diff --git a/gui/src/components/archive/ArchiveBrowser.js b/gui/src/components/archive/ArchiveBrowser.js index f784fa98fde1902943eaebd9342dfb4dc714a82f..16c273fcbcc9346438b820b73f15c80dfd89553e 100644 --- a/gui/src/components/archive/ArchiveBrowser.js +++ b/gui/src/components/archive/ArchiveBrowser.js @@ -165,7 +165,7 @@ function ArchiveConfigForm({searchOptions, data}) { </Tooltip> {entryId && <Download tooltip="download the archive" - url={`entries/${entryId}/archive/download?set_browser_download_headers=true`} + url={`entries/${entryId}/archive/download?ignore_mime_type=true`} component={IconButton} > <DownloadIcon /> diff --git a/gui/src/components/archive/FileBrowser.js b/gui/src/components/archive/FileBrowser.js index d5bac321f5679bb27536d68db291d6183e37c566..17aaca49aa41674ef628ebc8f3c1fb32b6119da9 100644 --- a/gui/src/components/archive/FileBrowser.js +++ b/gui/src/components/archive/FileBrowser.js @@ -183,7 +183,7 @@ function RawDirectoryContent({uploadId, path, title, highlightedItem, editable}) return <Content key={path}><Typography>loading ...</Typography></Content> } else { // Data loaded - const downloadUrl = `uploads/${uploadId}/raw/${encodedPath}?compress=true&set_browser_download_headers=true` + const downloadUrl = `uploads/${uploadId}/raw/${encodedPath}?compress=true` return ( <Dropzone disabled={!editable} @@ -360,7 +360,7 @@ function RawFileContent({uploadId, path, data, editable}) { const { api } = useApi() const [openConfirmDeleteFileDialog, setOpenConfirmDeleteFileDialog] = useState(false) const encodedPath = path.split('/').map(segment => encodeURIComponent(segment)).join('/') - const downloadUrl = `uploads/${uploadId}/raw/${encodedPath}?set_browser_download_headers=true` + const downloadUrl = `uploads/${uploadId}/raw/${encodedPath}?ignore_mime_type=true` const allNorthTools = useTools() const applicableNorthTools = useMemo(() => { const fileExtension = path.split('.').pop().toLowerCase() diff --git a/gui/src/components/archive/FilePreview.js b/gui/src/components/archive/FilePreview.js index 2f69b1826cc803f9e081b54d9862d72e9f2afc5c..8b659c16fc2534af2eff5844b526eeda442cb825 100644 --- a/gui/src/components/archive/FilePreview.js +++ b/gui/src/components/archive/FilePreview.js @@ -261,7 +261,7 @@ function FilePreviewText({uploadId, path}) { offset: contents?.length || 0, length: 16 * 1024, decompress: true, - set_browser_download_headers: true + ignore_mime_type: true }, {transformResponse: []}) .then(contents => { diff --git a/gui/src/components/uploads/UploadOverview.js b/gui/src/components/uploads/UploadOverview.js index 0b53f71d1ef50678d0d9d42201638e6846407f3c..6fb71b603075bf9c4054e284f3896b41a10b8363 100644 --- a/gui/src/components/uploads/UploadOverview.js +++ b/gui/src/components/uploads/UploadOverview.js @@ -413,7 +413,7 @@ function UploadOverview(props) { <EditMembersDialog disabled={!isWriter}/> <Download component={IconButton} tooltip="Download files" - url={`uploads/${uploadId}/raw/?compress=true&set_browser_download_headers=true`} + url={`uploads/${uploadId}/raw/?compress=true`} data-testid='upload-download-action' > <DownloadIcon /> diff --git a/nomad/app/v1/routers/auth.py b/nomad/app/v1/routers/auth.py index 303c7a5e413dab7baebce9fbca6cb900bd2c8c1e..8e7a0b5c6ea662c8d3f8b55f31b46cc3c0d1d9aa 100644 --- a/nomad/app/v1/routers/auth.py +++ b/nomad/app/v1/routers/auth.py @@ -70,11 +70,11 @@ def create_user_dependency( if basic_auth_allowed: user = _get_user_basic_auth(kwargs.get('form_data')) if not user and bearer_token_auth_allowed: - user = _get_user_bearer_token_auth(kwargs.get('bearer_token'), kwargs.get('request')) + user = _get_user_bearer_token_auth(kwargs.get('bearer_token')) if not user and upload_token_auth_allowed: user = _get_user_upload_token_auth(kwargs.get('token')) if not user and signature_token_auth_allowed: - user = _get_user_signature_token_auth(kwargs.get('signature_token')) + user = _get_user_signature_token_auth(kwargs.get('signature_token'), kwargs.get('request')) if required and not user: raise HTTPException( @@ -111,11 +111,6 @@ def create_user_dependency( annotation=str, default=Depends(oauth2_scheme), kind=Parameter.KEYWORD_ONLY)) - new_parameters.append( - Parameter( - name='request', - annotation=Request, - kind=Parameter.KEYWORD_ONLY)) if upload_token_auth_allowed: new_parameters.append( Parameter( @@ -134,6 +129,11 @@ def create_user_dependency( None, description='Signature token used to sign download urls.'), kind=Parameter.KEYWORD_ONLY)) + new_parameters.append( + Parameter( + name='request', + annotation=Request, + kind=Parameter.KEYWORD_ONLY)) # Create a wrapper around user_dependency, and set the signature on it @wraps(user_dependency) @@ -165,20 +165,11 @@ def _get_user_basic_auth(form_data: OAuth2PasswordRequestForm) -> User: return None -def _get_user_bearer_token_auth(bearer_token: str, request: Request) -> User: +def _get_user_bearer_token_auth(bearer_token: str) -> User: ''' Verifies bearer_token (throwing exception if illegal value provided) and returns the corresponding user object, or None, if no bearer_token provided. ''' - if not bearer_token and request: - auth_cookie = request.cookies.get('Authorization') - if auth_cookie: - try: - auth_cookie = requests.utils.unquote(auth_cookie) # type: ignore - if auth_cookie.startswith('Bearer '): - bearer_token = auth_cookie[7:] - except Exception: - pass if bearer_token: try: user = cast(datamodel.User, infrastructure.keycloak.tokenauth(bearer_token)) @@ -218,7 +209,7 @@ def _get_user_upload_token_auth(upload_token: str) -> User: return None -def _get_user_signature_token_auth(signature_token: str) -> User: +def _get_user_signature_token_auth(signature_token: str, request: Request) -> User: ''' Verifies the signature token (throwing exception if illegal value provided) and returns the corresponding user object, or None, if no upload_token provided. @@ -239,6 +230,21 @@ def _get_user_signature_token_auth(signature_token: str) -> User: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail='Invalid token.') + elif request: + auth_cookie = request.cookies.get('Authorization') + if auth_cookie: + try: + auth_cookie = requests.utils.unquote(auth_cookie) # type: ignore + if auth_cookie.startswith('Bearer '): + cookie_bearer_token = auth_cookie[7:] + user = cast(datamodel.User, infrastructure.keycloak.tokenauth(cookie_bearer_token)) + return user + except infrastructure.KeycloakError as e: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail=str(e), headers={'WWW-Authenticate': 'Bearer'}) + except Exception: + pass return None diff --git a/nomad/app/v1/routers/entries.py b/nomad/app/v1/routers/entries.py index 5dc84b0405f3ccd1c4b8396af5c531e0387e90bc..5686413b26e7180bb8eb4861b106eebbeb1c0365 100644 --- a/nomad/app/v1/routers/entries.py +++ b/nomad/app/v1/routers/entries.py @@ -1100,20 +1100,21 @@ async def get_entry_archive( responses=create_responses(_bad_id_response, _archive_download_response)) async def get_entry_archive_download( entry_id: str = Path(..., description='The unique entry id of the entry to retrieve archive data from.'), - set_browser_download_headers: bool = QueryParameter( + ignore_mime_type: bool = QueryParameter( False, description=strip(''' - Sets the response headers to signal to browsers to download the data.''')), + Sets the mime type specified in the response headers to `application/octet-stream` + instead of the actual mime type (i.e. `application/json`).''')), user: User = Depends(create_user_dependency(signature_token_auth_allowed=True))): ''' Returns the full archive for the given `entry_id`. ''' response = answer_entry_archive_request(dict(entry_id=entry_id), required='*', user=user) archive = response['data']['archive'] - if set_browser_download_headers: - return StreamingResponse( - io.BytesIO(json.dumps(archive, indent=2).encode()), - headers=browser_download_headers(filename=f'{entry_id}.json')) - return archive + return StreamingResponse( + io.BytesIO(json.dumps(archive, indent=2).encode()), + headers=browser_download_headers( + filename=f'{entry_id}.json', + media_type='application/octet-stream' if ignore_mime_type else 'application/json')) @router.post( diff --git a/nomad/app/v1/routers/uploads.py b/nomad/app/v1/routers/uploads.py index b335e0bbd93dc1f79f8da2d98643a3bd893f6963..1d9a9dc8b01f1b0148a9764d5e90b306817811b3 100644 --- a/nomad/app/v1/routers/uploads.py +++ b/nomad/app/v1/routers/uploads.py @@ -621,7 +621,7 @@ async def get_upload_rawdir_path( description=strip(''' If the fields `entry_id` and `parser_name` should be populated for all encountered mainfiles.''')), - user: User = Depends(create_user_dependency(required=False, signature_token_auth_allowed=True))): + user: User = Depends(create_user_dependency(required=False))): ''' For the upload specified by `upload_id`, gets the raw file or directory metadata located at the given `path`. The response will either contain a `file_metadata` or @@ -735,10 +735,11 @@ async def get_upload_raw_path( Set if compressed files should be decompressed before streaming the content (that is: if there are compressed files *within* the raw files). Note, only some compression formats are supported.''')), - set_browser_download_headers: bool = FastApiQuery( + ignore_mime_type: bool = FastApiQuery( False, description=strip(''' - Sets the response headers to signal to browsers to download the data.''')), + Sets the mime type specified in the response headers to `application/octet-stream` + instead of the actual mime type.''')), user: User = Depends(create_user_dependency(required=False, signature_token_auth_allowed=True))): ''' For the upload specified by `upload_id`, gets the raw file or directory content located @@ -783,17 +784,15 @@ async def get_upload_raw_path( raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=strip(''' Invalid length provided. Should be greater than 0, or -1 if the remainder of the file should be read.''')) - if set_browser_download_headers or not (offset == 0 and length == -1): + if ignore_mime_type or not (offset == 0 and length == -1): media_type = 'application/octet-stream' else: media_type = upload_files.raw_file_mime_type(path) content = create_download_stream_raw_file( upload_files, path, offset, length, decompress) - if set_browser_download_headers: - return StreamingResponse(content, headers=browser_download_headers( - filename=os.path.basename(path), - media_type=media_type)) - return StreamingResponse(content, media_type=media_type) + return StreamingResponse(content, headers=browser_download_headers( + filename=os.path.basename(path) + ('.zip' if files_params.compress else ''), + media_type=media_type)) else: # Directory if not files_params.compress: @@ -806,11 +805,9 @@ async def get_upload_raw_path( download_item, upload_files, re_pattern=files_params.re_pattern, recursive=True, create_manifest_file=False, compress=True) - if set_browser_download_headers: - return StreamingResponse(content, headers=browser_download_headers( - (upload.upload_id if not path else os.path.basename(path.rstrip('/'))) + '.zip', - media_type='application/zip')) - return StreamingResponse(content, media_type='application/zip') + return StreamingResponse(content, headers=browser_download_headers( + (upload.upload_id if not path else os.path.basename(path.rstrip('/'))) + '.zip', + media_type='application/zip')) except Exception as e: logger.error('exception while streaming download', exc_info=e) upload_files.close() diff --git a/tests/app/v1/routers/common.py b/tests/app/v1/routers/common.py index 9867113c8164a7adb3ad55db8c140d312439b3a4..a14cf37cec07cd154c476d486f3fb74a94e114ce 100644 --- a/tests/app/v1/routers/common.py +++ b/tests/app/v1/routers/common.py @@ -751,6 +751,16 @@ def assert_pagination(pagination, pagination_response, data, order_by=None, orde assert_url_query_args(prev_page_url, page=page - 1, page_after_value=None) +def assert_browser_download_headers(response, media_type: str, filename: str): + if media_type: + assert response.headers['Content-Type'].split(';')[0] == media_type.split(';')[0] + content_disposition = response.headers['Content-Disposition'] + assert 'attatchment;' in content_disposition + if filename: + filename_in_header = content_disposition.split('filename="')[1][:-1] + assert filename_in_header == filename + + def perform_metadata_test( client, endpoint: str, owner=None, headers={}, status_code=200, total=None, http_method='get', **kwargs): diff --git a/tests/app/v1/routers/test_entries.py b/tests/app/v1/routers/test_entries.py index e5a547e260a2c4eb72e1bff33a4e467f2393df7d..d0698f849637ceaf7c9cae20ceff6b4a6a13f673 100644 --- a/tests/app/v1/routers/test_entries.py +++ b/tests/app/v1/routers/test_entries.py @@ -30,7 +30,7 @@ from tests.test_files import example_mainfile_contents, append_raw_files # pyli from .common import ( aggregation_exclude_from_search_test_parameters, assert_response, assert_base_metadata_response, - assert_metadata_response, assert_required, assert_aggregations, assert_pagination, + assert_metadata_response, assert_required, assert_aggregations, assert_pagination, assert_browser_download_headers, perform_metadata_test, post_query_test_parameters, get_query_test_parameters, perform_owner_test, owner_test_parameters, pagination_test_parameters, aggregation_test_parameters) @@ -204,6 +204,7 @@ def assert_raw_zip_file( manifest_keys = ['entry_id', 'upload_id', 'mainfile'] assert len(response.content) > 0 + assert_browser_download_headers(response, 'application/zip', 'raw_files.zip') with zipfile.ZipFile(io.BytesIO(response.content)) as zip_file: with zip_file.open('manifest.json', 'r') as f: manifest = json.load(f) @@ -267,6 +268,7 @@ def assert_archive_zip_file(response, total: int = -1, compressed: bool = False) manifest_keys = ['entry_id', 'upload_id', 'path', 'parser_name'] assert len(response.content) > 0 + assert_browser_download_headers(response, 'application/zip', 'archives.zip') with zipfile.ZipFile(io.BytesIO(response.content)) as zip_file: assert zip_file.testzip() is None with zip_file.open('manifest.json', 'r') as f: @@ -644,16 +646,25 @@ def test_entry_archive(client, example_data, test_auth_dict, user, entry_id, sta assert_archive_response(response.json()) -@pytest.mark.parametrize('user, entry_id, status_code', [ - pytest.param(None, 'id_01', 200, id='id'), - pytest.param('test_user', 'id_child_entries_child1', 200, id='child-entry'), - pytest.param(None, 'id_02', 404, id='404-not-visible'), - pytest.param(None, 'doesnotexist', 404, id='404-does-not-exist')]) -def test_entry_archive_download(client, example_data, test_auth_dict, user, entry_id, status_code): +@pytest.mark.parametrize('user, entry_id, ignore_mime_type, status_code', [ + pytest.param(None, 'id_01', False, 200, id='id'), + pytest.param(None, 'id_01', True, 200, id='id'), + pytest.param('test_user', 'id_child_entries_child1', False, 200, id='child-entry'), + pytest.param('test_user', 'id_child_entries_child1', True, 200, id='child-entry'), + pytest.param(None, 'id_02', True, 404, id='404-not-visible'), + pytest.param(None, 'doesnotexist', False, 404, id='404-does-not-exist')]) +def test_entry_archive_download( + client, example_data, test_auth_dict, user, entry_id, ignore_mime_type, status_code): user_auth, _ = test_auth_dict[user] - response = client.get('entries/%s/archive/download' % entry_id, headers=user_auth) + response = client.get( + f'entries/{entry_id}/archive/download' + ('?ignore_mime_type=true' if ignore_mime_type else ''), + headers=user_auth) assert_response(response, status_code) if status_code == 200: + assert_browser_download_headers( + response, + media_type='application/octet-stream' if ignore_mime_type else 'application/json', + filename=entry_id + '.json') archive = response.json() assert 'metadata' in archive assert 'run' in archive diff --git a/tests/app/v1/routers/test_uploads.py b/tests/app/v1/routers/test_uploads.py index fbf5bc99666ecc47a0b3ea08a9fde4015edde39d..c07c5a8e771c5d4e6215f162f77a13efe2cbd08c 100644 --- a/tests/app/v1/routers/test_uploads.py +++ b/tests/app/v1/routers/test_uploads.py @@ -33,7 +33,7 @@ from tests.test_files import ( from tests.test_search import assert_search_upload from tests.processing.test_edit_metadata import ( assert_metadata_edited, all_coauthor_metadata, all_admin_metadata) -from tests.app.v1.routers.common import assert_response +from tests.app.v1.routers.common import assert_response, assert_browser_download_headers from nomad import config, files, infrastructure from nomad.processing import Upload, Entry, ProcessStatus from nomad.files import UploadFiles, StagingUploadFiles, PublicUploadFiles @@ -683,8 +683,8 @@ def test_get_upload_entry( 200, 'text/plain; charset=utf-8', 'content', id='unpublished-file'), pytest.param(dict( user='test_user', upload_id='id_unpublished', path='test_content/id_unpublished_1/1.aux', - set_browser_download_headers=True), - 200, 'application/octet-stream', 'content', id='unpublished-file-set_browser_download_headers'), + ignore_mime_type=True), + 200, 'application/octet-stream', 'content', id='unpublished-file-ignore_mime_type'), pytest.param(dict( user='other_test_user', upload_id='id_unpublished', path='test_content/id_unpublished_1/1.aux'), 401, None, None, id='unpublished-file-unauthorized'), @@ -696,8 +696,8 @@ def test_get_upload_entry( 200, 'text/plain; charset=utf-8', 'method', id='published-file'), pytest.param(dict( user='test_user', upload_id='id_published', path='test_content/subdir/test_entry_01/mainfile.json', - set_browser_download_headers=True), - 200, 'application/octet-stream', 'method', id='published-file-set_browser_download_headers'), + ignore_mime_type=True), + 200, 'application/octet-stream', 'method', id='published-file-ignore_mime_type'), pytest.param(dict( user='admin_user', upload_id='id_published', path='test_content/subdir/test_entry_01/1.aux'), 200, 'text/plain; charset=utf-8', 'content', id='published-file-admin-auth'), @@ -783,10 +783,10 @@ def test_get_upload_raw_path( re_pattern = args.get('re_pattern', None) offset = args.get('offset', None) length = args.get('length', None) - set_browser_download_headers = args.get('set_browser_download_headers', None) + ignore_mime_type = args.get('ignore_mime_type', None) user_auth, __token = test_auth_dict[user] query_args = dict( - set_browser_download_headers=set_browser_download_headers, + ignore_mime_type=ignore_mime_type, compress=compress, re_pattern=re_pattern, offset=offset, @@ -798,8 +798,12 @@ def test_get_upload_raw_path( assert_response(response, expected_status_code) if expected_status_code == 200: - mime_type = response.headers.get('content-type') - assert mime_type == expected_mime_type + mime_type = response.headers.get('Content-Type') + if not path: + expected_filename = upload_id + '.zip' + else: + expected_filename = os.path.basename(path.rstrip('/')) + ('.zip' if mime_type == 'application/zip' else '') + assert_browser_download_headers(response, expected_mime_type, expected_filename) if mime_type == 'application/zip': if expected_content: with zipfile.ZipFile(io.BytesIO(response.content)) as zip_file: