Skip to content

Commit a6a7dc1

Browse files
committed
Addressed comments
1 parent a5b0aca commit a6a7dc1

16 files changed

Lines changed: 254 additions & 187 deletions

File tree

Makefile

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ get_env: ## Get secrets needed for integration tests from vault
5353
$(call write_secret,SDS_AAI_CLIENT_ID,sd-submit/secrets,sds_aai_id) \
5454
$(call write_secret,SDS_AAI_CLIENT_SECRET,sd-submit/secrets,sds_aai_secret) \
5555
$(call write_secret,SDS_AAI_URL,sd-submit/secrets,sds_aai_url) \
56-
$(call write_secret,LS_AAI_CLIENT_ID,sd-submit/secrets,ls_aai_id) \
57-
$(call write_secret,LS_AAI_CLIENT_SECRET,sd-submit/secrets,ls_aai_secret) \
58-
$(call write_secret,LS_AAI_URL,sd-submit/secrets,ls_aai_url) \
5956
$(call write_secret,KEYSTONE_ENDPOINT,sd-submit/secrets,pouta_host) \
6057
$(call write_secret,NBIS_JWT_PUBLIC_KEY,sd-submit/secrets,nbis_jwt_public_key) \
6158
$(call write_integration_test_secret,DATACITE_API,sd-submit/datacite_test,DOI_API) \

docker-compose.yml

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,6 @@ services:
7575
start_period: 30s
7676
environment:
7777
- "DEPLOYMENT=NBIS"
78-
- "OIDC_CLIENT_SECRET=${LS_AAI_CLIENT_SECRET:-${OIDC_CLIENT_SECRET:?OIDC_CLIENT_SECRET must be defined}}"
79-
- "OIDC_CLIENT_ID=${LS_AAI_CLIENT_ID:-${OIDC_CLIENT_ID:?OIDC_CLIENT_ID must be defined}}"
80-
- "OIDC_URL=${LS_AAI_URL:-${OIDC_URL:?OIDC_URL must be defined}}"
8178
- "BASE_URL=http://sd-submit-api-nbis:5431"
8279
- "OIDC_REDIRECT_URL=${OIDC_REDIRECT_URL}"
8380
- "OIDC_SECURE_COOKIE=${OIDC_SECURE_COOKIE}"
@@ -101,9 +98,9 @@ services:
10198
- "S3_ENDPOINT=${S3_INBOX_ENDPOINT:?S3_INBOX_ENDPOINT must be defined}"
10299
- "ADMIN_URL=${SDA_API_URL}"
103100
- "ADMIN_TOKEN=${ADMIN_TOKEN}"
104-
- "BP_PUBLIC_C4GH_KEY=${C4GH_RECIPIENT_PUBLIC_KEY:?C4GH_RECIPIENT_PUBLIC_KEY must be defined}"
105-
- "SENDER_SECRET_KEY=${C4GH_SENDER_SECRET_KEY:?C4GH_SENDER_SECRET_KEY must be defined}"
106-
- "SECRET_KEY_PASSPHRASE=${C4GH_SECRET_KEY_PASSPHRASE:?C4GH_SECRET_KEY_PASSPHRASE must be defined}"
101+
- "CRYPT4GH_PUBLIC_KEY=${C4GH_RECIPIENT_PUBLIC_KEY:?C4GH_RECIPIENT_PUBLIC_KEY must be defined}"
102+
- "CRYPT4GH_PRIVATE_KEY=${C4GH_SENDER_SECRET_KEY:?C4GH_SENDER_SECRET_KEY must be defined}"
103+
- "CRYPT4GH_PRIVATE_KEY_PASSPHRASE=${C4GH_SECRET_KEY_PASSPHRASE:?C4GH_SECRET_KEY_PASSPHRASE must be defined}"
107104
- "ALLOW_UNSAFE=${ALLOW_UNSAFE}"
108105

109106
mock-oauth2:

metadata_backend/api/handlers/publish.py

Lines changed: 3 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -15,34 +15,16 @@
1515
from ..models.models import File, Registration, SubmissionId
1616
from ..models.submission import Rems, Submission, SubmissionMetadata, SubmissionWorkflow
1717
from ..processors.xml.bigpicture import (
18-
BP_ANNOTATION_OBJECT_TYPE,
19-
BP_DATASET_OBJECT_TYPE,
20-
BP_IMAGE_OBJECT_TYPE,
21-
BP_OBSERVATION_OBJECT_TYPE,
22-
BP_OBSERVER_OBJECT_TYPE,
2318
BP_POLICY_OBJECT_TYPE,
24-
BP_SAMPLE_OBJECT_TYPES,
25-
BP_STAINING_OBJECT_TYPE,
2619
BP_XML_OBJECT_CONFIG,
2720
)
2821
from ..processors.xml.processors import XmlObjectProcessor
22+
from ..services.bigpicture import upload_bp_metadata_xmls
2923
from ..services.datacite import DataciteService
30-
from ..services.file import S3InboxSDAService
3124
from ..services.submission.bigpicture import is_clinical_policy
3225
from .restapi import RESTAPIHandler
3326
from .submission import SubmissionAPIHandler
3427

35-
BP_METADATA_FILES: tuple[tuple[str | tuple[str, ...], str], ...] = (
36-
(BP_DATASET_OBJECT_TYPE, "dataset.xml.c4gh"),
37-
(BP_POLICY_OBJECT_TYPE, "policy.xml.c4gh"),
38-
(BP_IMAGE_OBJECT_TYPE, "image.xml.c4gh"),
39-
(BP_ANNOTATION_OBJECT_TYPE, "annotation.xml.c4gh"),
40-
(BP_OBSERVATION_OBJECT_TYPE, "observation.xml.c4gh"),
41-
(BP_OBSERVER_OBJECT_TYPE, "observer.xml.c4gh"),
42-
(tuple(BP_SAMPLE_OBJECT_TYPES), "sample.xml.c4gh"),
43-
(BP_STAINING_OBJECT_TYPE, "staining.xml.c4gh"),
44-
)
45-
4628

4729
class PublishAPIHandler(RESTAPIHandler):
4830
"""Publish API handler."""
@@ -161,7 +143,7 @@ async def _publish_rems(self, submission: Submission, rems: Rems, registration:
161143
# Create REMS resource.
162144
if not registration.remsResourceId:
163145
if submission.workflow == SubmissionWorkflow.BP:
164-
# Use BigPicture dataset id as the REMS resource id.
146+
# Use Bigpicture dataset id as the REMS resource id.
165147
resid = submission.submissionId
166148
elif registration.doi is not None:
167149
# Use DOI as the REMS resource id.
@@ -331,7 +313,7 @@ async def publish_submission(
331313
jwt = self._services.auth._get_bearer_token(headers)
332314
if not jwt:
333315
raise UserException("Missing OIDC access token in Authorization bearer header for SDA inbox upload.")
334-
await self._upload_bp_metadata_xmls(submission_id, user.user_id, jwt)
316+
await upload_bp_metadata_xmls(self._services, submission_id, user.user_id, jwt)
335317

336318
# Update submission status to published.
337319
await submission_service.publish(submission_id)
@@ -406,32 +388,6 @@ async def _register_submission(self, submission: Submission, datacite: DataCiteM
406388
if self._handlers.metax is not None:
407389
await self._publish_metax(registration)
408390

409-
async def _upload_bp_metadata_xmls(self, submission_id: str, user_id: str, jwt: str) -> None:
410-
"""Upload encrypted Bigpicture metadata XML files to SDA inbox."""
411-
file_provider = self._services.file_provider
412-
if not isinstance(file_provider, S3InboxSDAService):
413-
raise SystemException("Bigpicture metadata upload requires SDA inbox file provider service.")
414-
415-
bucket = user_id.replace("@", "_") # SDA inbox bucket name is the user id with @ replaced by underscore
416-
prefix = f"DATASET_{submission_id}/METADATA"
417-
for object_type, filename in BP_METADATA_FILES:
418-
xml = None
419-
async for xml_doc in self._services.object.get_xml_documents(submission_id, object_type):
420-
xml = xml_doc
421-
break
422-
if xml is None:
423-
continue
424-
425-
object_key = f"{prefix}/{filename}"
426-
await file_provider._add_file_to_bucket(
427-
bucket_name=bucket,
428-
object_key=object_key,
429-
access_key=user_id,
430-
secret_key=user_id,
431-
session_token=jwt,
432-
body=xml.encode("utf-8"),
433-
)
434-
435391
@staticmethod
436392
def _create_registration(submission_id: str, title: str, description: str, doi: str) -> Registration:
437393
"""Create submission registration.

metadata_backend/api/handlers/restapi.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class RESTAPIServiceHandlers(BaseModel):
4949
ror: RorServiceHandler | None
5050
rems: RemsServiceHandler
5151
keystone: KeystoneServiceHandler | None
52-
auth: AuthServiceHandler
52+
auth: AuthServiceHandler | None
5353
admin: AdminServiceHandler | None = None
5454
database: HealthHandler
5555

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
"""Bigpicture API services."""
2+
3+
from ..exceptions import SystemException
4+
from ..handlers.restapi import RESTAPIServices
5+
from ..processors.xml.bigpicture import (
6+
BP_ANNOTATION_OBJECT_TYPE,
7+
BP_DATASET_OBJECT_TYPE,
8+
BP_IMAGE_OBJECT_TYPE,
9+
BP_LANDING_PAGE_OBJECT_TYPE,
10+
BP_OBSERVATION_OBJECT_TYPE,
11+
BP_OBSERVER_OBJECT_TYPE,
12+
BP_ORGANISATION_OBJECT_TYPE,
13+
BP_POLICY_OBJECT_TYPE,
14+
BP_REMS_OBJECT_TYPE,
15+
BP_SAMPLE_OBJECT_TYPES,
16+
BP_STAINING_OBJECT_TYPE,
17+
)
18+
from .file import S3InboxSDAService
19+
20+
BP_METADATA_FILES: tuple[tuple[str | tuple[str, ...], str], ...] = (
21+
(BP_DATASET_OBJECT_TYPE, "dataset.xml.c4gh"),
22+
(BP_POLICY_OBJECT_TYPE, "policy.xml.c4gh"),
23+
(BP_IMAGE_OBJECT_TYPE, "image.xml.c4gh"),
24+
(BP_ANNOTATION_OBJECT_TYPE, "annotation.xml.c4gh"),
25+
(BP_OBSERVATION_OBJECT_TYPE, "observation.xml.c4gh"),
26+
(BP_OBSERVER_OBJECT_TYPE, "observer.xml.c4gh"),
27+
(tuple(BP_SAMPLE_OBJECT_TYPES), "sample.xml.c4gh"),
28+
(BP_STAINING_OBJECT_TYPE, "staining.xml.c4gh"),
29+
)
30+
31+
BP_LANDING_PAGE: tuple[tuple[str, str], ...] = ((BP_LANDING_PAGE_OBJECT_TYPE, "landing_page.xml.c4gh"),)
32+
33+
BP_PRIVATE_METDATA_FILES: tuple[tuple[str, str], ...] = (
34+
(BP_ORGANISATION_OBJECT_TYPE, "organisation.xml.c4gh"),
35+
(BP_REMS_OBJECT_TYPE, "rems.xml.c4gh"),
36+
)
37+
38+
39+
async def _upload_xml_documents(
40+
services: RESTAPIServices,
41+
file_provider: S3InboxSDAService,
42+
*,
43+
bucket_name: str,
44+
prefix: str,
45+
object_files: tuple[tuple[str | tuple[str, ...], str], ...],
46+
user_id: str,
47+
jwt: str,
48+
submission_id: str,
49+
) -> None:
50+
for object_type, filename in object_files:
51+
xml = None
52+
async for xml_doc in services.object.get_xml_documents(submission_id, object_type):
53+
xml = xml_doc
54+
break
55+
if xml is None:
56+
continue
57+
58+
object_key = f"{prefix}/{filename}"
59+
await file_provider._add_file_to_bucket(
60+
bucket_name=bucket_name,
61+
object_key=object_key,
62+
access_key=user_id,
63+
secret_key=user_id,
64+
session_token=jwt,
65+
body=xml.encode("utf-8"),
66+
)
67+
68+
69+
async def upload_bp_metadata_xmls(services: RESTAPIServices, submission_id: str, user_id: str, jwt: str) -> None:
70+
"""Upload encrypted Bigpicture metadata XML files to SDA inbox."""
71+
file_provider = services.file_provider
72+
if not isinstance(file_provider, S3InboxSDAService):
73+
raise SystemException("Bigpicture metadata upload requires SDA inbox file provider service.")
74+
75+
bucket = user_id.replace("@", "_") # SDA inbox bucket name is the user id with @ replaced by underscore
76+
77+
# Metadata XML files
78+
await _upload_xml_documents(
79+
services,
80+
file_provider,
81+
bucket_name=bucket,
82+
prefix=f"DATASET_{submission_id}/METADATA",
83+
object_files=BP_METADATA_FILES,
84+
user_id=user_id,
85+
jwt=jwt,
86+
submission_id=submission_id,
87+
)
88+
89+
# Landing page XML file
90+
await _upload_xml_documents(
91+
services,
92+
file_provider,
93+
bucket_name=bucket,
94+
prefix=f"DATASET_{submission_id}/LANDING_PAGE",
95+
object_files=BP_LANDING_PAGE,
96+
user_id=user_id,
97+
jwt=jwt,
98+
submission_id=submission_id,
99+
)
100+
101+
# Private metadata XML files
102+
# TODO(improve): Add datacite.xml to private metadata files once datacite.xml is available
103+
await _upload_xml_documents(
104+
services,
105+
file_provider,
106+
bucket_name=bucket,
107+
prefix=f"DATASET_{submission_id}/PRIVATE",
108+
object_files=BP_PRIVATE_METDATA_FILES,
109+
user_id=user_id,
110+
jwt=jwt,
111+
submission_id=submission_id,
112+
)

metadata_backend/api/services/file.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,8 +442,8 @@ async def _load_crypt4gh_keys(self) -> tuple[object, object]:
442442
"""Load Crypt4GH sender secret and recipient public keys from env variables."""
443443
conf = c4gh_config()
444444
try:
445-
sender_key_pem = base64.b64decode(conf.SENDER_SECRET_KEY).decode("utf-8")
446-
recipient_key_pem = base64.b64decode(conf.BP_PUBLIC_C4GH_KEY).decode("utf-8")
445+
sender_key_pem = base64.b64decode(conf.CRYPT4GH_PRIVATE_KEY).decode("utf-8")
446+
recipient_key_pem = base64.b64decode(conf.CRYPT4GH_PUBLIC_KEY).decode("utf-8")
447447
except (binascii.Error, UnicodeDecodeError) as ex:
448448
raise SystemException("Invalid base64 in C4GH key environment variables.") from ex
449449

@@ -458,7 +458,7 @@ async def _load_crypt4gh_keys(self) -> tuple[object, object]:
458458
if private_data.startswith(c4gh.MAGIC_WORD):
459459
private_stream.seek(len(c4gh.MAGIC_WORD))
460460

461-
sender_secret_key = c4gh.parse_private_key(private_stream, lambda: conf.SECRET_KEY_PASSPHRASE)
461+
sender_secret_key = c4gh.parse_private_key(private_stream, lambda: conf.CRYPT4GH_PRIVATE_KEY_PASSPHRASE)
462462
recipient_public_key = public_data
463463
return sender_secret_key, recipient_public_key
464464
except Exception as ex:

metadata_backend/api/services/submission/bigpicture.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def is_clinical_policy(policy_processor: XmlObjectProcessor) -> bool:
7878

7979

8080
class BigpictureObjectSubmissionService(ObjectSubmissionService):
81-
"""Service for processing BigPicture submissions."""
81+
"""Service for processing Bigpicture submissions."""
8282

8383
def __init__(
8484
self,
@@ -140,7 +140,7 @@ def _create_processor(objects: list[ObjectSubmission]) -> tuple[XmlStringDocumen
140140
if datacite_object:
141141
datacite = read_datacite_xml(datacite_object.document)
142142

143-
# Create processor for BigPicture XMLs.
143+
# Create processor for Bigpicture XMLs.
144144
processor = XmlStringDocumentsProcessor(BP_XML_OBJECT_CONFIG, [o.document for o in bp_objects])
145145
return processor, datacite
146146

metadata_backend/conf/c4gh.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ class Crypt4GHConfig(BaseSettings):
99

1010
model_config = {"extra": "allow"} # Allow creation using the constructor.
1111

12-
SENDER_SECRET_KEY: str = Field(description="Base64 encoded private Crypt4GH key")
13-
SECRET_KEY_PASSPHRASE: str = Field(description="Secret key passphrase")
14-
BP_PUBLIC_C4GH_KEY: str = Field(description="Base64 encoded public Crypt4GH key for Bigpicture")
12+
CRYPT4GH_PRIVATE_KEY: str = Field(description="Base64 encoded private Crypt4GH key")
13+
CRYPT4GH_PRIVATE_KEY_PASSPHRASE: str = Field(description="Secret key passphrase")
14+
CRYPT4GH_PUBLIC_KEY: str = Field(description="Base64 encoded public Crypt4GH key for Bigpicture")
1515

1616

1717
def c4gh_config() -> Crypt4GHConfig:

metadata_backend/server.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,12 +183,14 @@ async def _shutdown() -> None:
183183
pid_handler = None
184184
admin_handler = None
185185
keystone_handler = None
186+
auth_handler = None
186187

187188
if config.DEPLOYMENT == DEPLOYMENT_CSC:
188189
metax_handler = _create_handler(MetaxServiceHandler())
189190
ror_handler = _create_handler(RorServiceHandler())
190191
pid_handler = _create_handler(PIDServiceHandler(metax_handler))
191192
keystone_handler = _create_handler(KeystoneServiceHandler())
193+
auth_handler = _create_handler(AuthServiceHandler())
192194

193195
if config.DEPLOYMENT == DEPLOYMENT_NBIS:
194196
datacite_handler = _create_handler(DataciteServiceHandler(metax_handler))
@@ -200,7 +202,6 @@ async def _shutdown() -> None:
200202
)
201203

202204
rems_handler = _create_handler(RemsServiceHandler())
203-
auth_handler = _create_handler(AuthServiceHandler())
204205

205206
# Provide services for FastAPI routes.
206207
services = RESTAPIServices(

tests/integration/conftest.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
get_submission,
3535
)
3636
from tests.utils import (
37-
BigPictureObjectNames,
37+
BigpictureObjectNames,
3838
bp_submission_documents,
3939
bp_update_documents,
4040
get_test_es256_keypair,
@@ -143,20 +143,20 @@ async def _update(
143143
class SubmissionCallableBigpicture(Protocol):
144144
def __call__(
145145
self, is_datacite: bool
146-
) -> Awaitable[tuple[Submission, BigPictureObjectNames]]: ... # submission names, object names
146+
) -> Awaitable[tuple[Submission, BigpictureObjectNames]]: ... # submission names, object names
147147

148148

149149
class SubmissionUpdateCallableBigpicture(Protocol):
150150
def __call__(
151-
self, submission_id: str, submission_name: str, object_names: BigPictureObjectNames, is_datacite: bool
151+
self, submission_id: str, submission_name: str, object_names: BigpictureObjectNames, is_datacite: bool
152152
) -> Awaitable[Submission]: ...
153153

154154

155155
@pytest.fixture
156156
async def bp_submission(nbis_client: aiohttp.ClientSession, project_id: str) -> SubmissionCallableBigpicture:
157157
"""Create Bigpicture submission using the /submit endpoint."""
158158

159-
async def _create(is_datacite: bool = False) -> tuple[Submission, BigPictureObjectNames]: # noqa
159+
async def _create(is_datacite: bool = False) -> tuple[Submission, BigpictureObjectNames]: # noqa
160160
submission_name, object_names, files = bp_submission_documents(is_datacite=is_datacite)
161161

162162
# Post submission.
@@ -178,7 +178,7 @@ async def bp_submission_update(
178178
"""
179179

180180
async def _update(
181-
submission_id: str, submission_name: str, object_names: BigPictureObjectNames, is_datacite: bool
181+
submission_id: str, submission_name: str, object_names: BigpictureObjectNames, is_datacite: bool
182182
) -> Submission: # noqa
183183
_, _, files = bp_update_documents(submission_name, object_names, is_datacite)
184184

0 commit comments

Comments
 (0)