Feat/add bulk presigned url by jacob50231 · Pull Request #1344 · uc-cdis/fence

jacob50231 · 2026-04-15T15:59:36Z

Link to JIRA ticket if there is one:

New Features

Breaking Changes

Bug Fixes

Improvements

Dependency updates

Deployment changes

SpencerAxelrod · 2026-05-04T05:56:32Z

+    for access_id, object_ids in access_to_object_ids.items():
+        try:
+            result = bulk_get_signed_url_for_file(
+                file_ids=object_ids, requested_protocol=access_id


we need to pass passports in here and make sure they are supported.
ga4gh_passports=bulk_request.passports

And we should reject if a request tries to give both a bearer token and a passport. See how the single-object drs endpoint works as reference e.g.

if request.headers.get("Authorization") and bulk_request.passports: raise Forbidden("Sending both a bearer token and passports is not allowed")

Also unmock bulk_signed_url_for_file in tests and test both token and passports

SpencerAxelrod · 2026-05-04T05:59:58Z

+
+
+class BulkObjectAccessRequest(BaseModel):
+    passports: Optional[list[str]]


this has to be passports: Optional[list[str]] = None or else it will still require the field I think. Write a test to that doesn't pass a passports key just to make sure.

SpencerAxelrod · 2026-05-04T06:03:08Z

+            indexd_server = config.get("INDEXD") or config["BASE_URL"] + "/index"
+            url = indexd_server + "/index/"
+            try:
+                res = requests.get(url + file_id)


I think the main idea was to use a single POST to indexd's /bulk/documents endpoint here

SpencerAxelrod · 2026-05-04T06:32:54Z

+            unresolved_by_code[error_code].extend(object_ids)
+        except NotFound as e:
+            logger.debug(f"NotFound fetching signed URLs for {access_id}: {e}")
+            unresolved_by_code[404].extend(object_ids)


A request for [valid_guid, missing_guid] under the same access ID returns both as unresolved even though one could have been resolved.

use indexd's bulk documents endpoint and split returned/missing IDs into resolved and unresolved lists

SpencerAxelrod · 2026-05-04T06:38:12Z

    return {"url": signed_url}


+@enable_audit_logging


the function that runs within this decorator ( /fence/resources/audit/utils.py -> create_audit_log_for_request()) only will send logs for certain requests. We need to add support for the new endpoints to actually sned the logs to audit service

Still looking into this and would like a bit of clarification on what it usually looks like in our codebase. Should have pushed changes for the rest of the comments

https://github.com/uc-cdis/fence/blob/master/fence/resources/audit/utils.py#L66-L95 This segment of code checks the path and makes decisions on auditing based on it, so you'll need to update the handler here to account for it.

github-actions · 2026-05-05T20:56:58Z

Integration Tests

Test summary after running integration tests

filepath	passed	failed	skipped	SUBTOTAL
tests/test_drs_endpoint.py	4	0	20	24
tests/test_audit_service.py	3	0	3	6
tests/test_ras_authn.py	0	0	3	3
tests/test_data_upload.py	8	0	1	9
tests/test_dbgap.py	4	0	1	5
tests/test_user_login_activation.py	2	0	1	3
tests/test_oauth2.py	15	0	0	15
tests/test_centralized_auth.py	16	0	0	16
tests/test_presigned_url.py	7	1	0	8
tests/test_user_token.py	5	0	0	5
tests/test_fence_admin.py	2	0	0	2
tests/test_oidc_client.py	2	0	0	2
tests/test_google_data_access.py	1	0	0	1
tests/test_client_credentials.py	1	0	0	1
tests/test_register_user.py	2	0	0	2
TOTAL	72	1	29	102

Test summary after rerunning failed integration tests

filepath	failed	SUBTOTAL
tests/test_presigned_url.py	1	1
TOTAL	1	1

Please find the detailed integration test report here

Please find the detailed integration test report after rerunning failed tests here

Please find the Github Action logs here

k-burt-uch · 2026-04-27T19:39:11Z



+@blueprint.route("/download/bulk", methods=["POST"])
+def download_bulk_files():


Update the swagger / openapi yamls to include this endpoint.

k-burt-uch · 2026-05-11T16:06:04Z

    return {"url": signed_url}


+@enable_audit_logging


https://github.com/uc-cdis/fence/blob/master/fence/resources/audit/utils.py#L66-L95 This segment of code checks the path and makes decisions on auditing based on it, so you'll need to update the handler here to account for it.

github-actions · 2026-05-13T14:30:58Z

Integration Tests

filepath	SUBTOTAL
TOTAL	0

Please find the detailed integration test report here

Please find the Github Action logs here

github-actions · 2026-05-13T17:16:39Z

Integration Tests

Failed to Prepare CI environment.

The following pods are not in healthy state:
ohdsi-webapi-deployment-78c9c7b857-jslkw

Please find the Github Action logs here

github-actions · 2026-05-14T02:00:54Z

Integration Tests

filepath	error	SUBTOTAL
tests/test_aggregate_mds.py	1	1
tests/test_discoverypage.py	1	1
tests/test_etl.py	1	1
tests/test_pfb_export.py	1	1
tests/test_ras_authn.py	1	1
tests/test_study_viewer.py	1	1
TOTAL	6	6

Please find the detailed integration test report here

Please find the Github Action logs here

github-actions · 2026-05-14T19:27:42Z

Integration Tests

Test summary after running integration tests

filepath	passed	failed	SUBTOTAL
tests/test_presigned_url.py	7	1	8
TOTAL	7	1	8

Test summary after rerunning failed integration tests

filepath	failed	SUBTOTAL
tests/test_presigned_url.py	1	1
TOTAL	1	1

Please find the detailed integration test report here

Please find the detailed integration test report after rerunning failed tests here

Please find the Github Action logs here

github-actions · 2026-05-14T22:20:44Z

Integration Tests

Test summary after running integration tests

filepath	passed	failed	SUBTOTAL
tests/test_presigned_url.py	7	1	8
TOTAL	7	1	8

Test summary after rerunning failed integration tests

filepath	failed	SUBTOTAL
tests/test_presigned_url.py	1	1
TOTAL	1	1

Please find the detailed integration test report here

Please find the detailed integration test report after rerunning failed tests here

Please find the Github Action logs here

github-actions · 2026-05-15T07:36:58Z

Integration Tests

Test summary after running integration tests

filepath	passed	failed	SUBTOTAL
tests/test_presigned_url.py	7	1	8
TOTAL	7	1	8

Test summary after rerunning failed integration tests

filepath	failed	SUBTOTAL
tests/test_presigned_url.py	1	1
TOTAL	1	1

Please find the detailed integration test report here

Please find the detailed integration test report after rerunning failed tests here

Please find the Github Action logs here

…e into feat/add-bulk-presigned-url

github-actions · 2026-05-15T13:29:00Z

Integration Tests

filepath	passed	SUBTOTAL
tests/test_presigned_url.py	8	8
TOTAL	8	8

Please find the detailed integration test report here

Please find the Github Action logs here

github-actions · 2026-05-15T20:51:54Z

Integration Tests

Test summary after running integration tests

filepath	passed	failed	SUBTOTAL
tests/test_presigned_url.py	7	1	8
TOTAL	7	1	8

Test summary after rerunning failed integration tests

filepath	failed	SUBTOTAL
tests/test_presigned_url.py	1	1
TOTAL	1	1

Please find the detailed integration test report here

Please find the detailed integration test report after rerunning failed tests here

Please find the Github Action logs here

SpencerAxelrod · 2026-05-18T09:04:10Z

+
+            if file_authz:
+                authz_key = (
+                    tuple(file_authz) if isinstance(file_authz, list) else file_authz


I think we need tuple(sorted(file_authz)) so it's the same as the line 644

SpencerAxelrod · 2026-05-18T09:11:44Z

+        default=default_expires_in,
+    )
+
+    if ga4gh_passports and not config["GA4GH_PASSPORTS_TO_DRS_ENABLED"]:


we should move this check to the blueprint level before calling bulk_get_signed_url_for_file, ill put example there

SpencerAxelrod · 2026-05-18T09:14:45Z

+            "You cannot supply both GA4GH passports and a token "
+            "in the Authorization header of a request."
+        )
+


put the passports enabled check here:

if bulk_request.passports and not config["GA4GH_PASSPORTS_TO_DRS_ENABLED"]: raise NotSupported( "Using GA4GH Passports as a means of authentication and authorization " "is not supported by this instance of Gen3." )

SpencerAxelrod · 2026-05-18T09:37:14Z

+    auth_info = _get_auth_info_for_id_or_from_request(
+        sub_type=int, db_session=db_session
+    )
+    flask.g.audit_data = {


ok so I think there is a specific case here where if they authenticate with a Passport but no bearer token, _get_auth_info_for_id_or_from_request will set "username": auth_info["username"] for the audit log to be anonymous. So we should check for passports before here, and if they are using passports set the flask.g.audit_data username using users_from_passports.items()

MichaelLukowski added 30 commits October 31, 2025 10:58

adding support for vector download

adaab80

fix fstring escape

a18b512

add model to vec get

9255e13

change route for vector download

ad409e0

update vector return

98abb8f

add vec class

15a5cf8

add debug statement for vec

a5804e3

fix requests url

f9ec6b0

fix request protocol for vec signed url

98ccf74

adding vector upload

589d2e0

fix import and add logging

fcda7dd

fix function typo

9ff13a7

remove old code

5485623

update indexd post params

3638613

remove more old code

0b237b6

fix string in url

f0d09fd

fix string escape typo

6dcb43c

ensure md5 is not used for security

52ab7e9

add bulk download

68392f8

add bulk download

24ac110

bulk download with authz role caching

59e0234

add function definiton

1f2e60f

fix class call

f5bec59

clean up imports

4af5368

update function arguments

7220fe5

fix loop

1452601

actions of bulk to only support download

c34c074

actions of bulk to only support download

c132e40

fix escape quotes

8a548c5

no actions for bulk downloads

f403b36

SpencerAxelrod reviewed May 4, 2026

View reviewed changes

address spencer's comments

078026d

k-burt-uch requested changes May 11, 2026

View reviewed changes

audit, openapi, and some test fixes

f495534

PlanXCyborg added the TestPresignedURL label May 13, 2026

acl no authz case for auth flow

99c43a0

jacob50231 added 2 commits May 14, 2026 09:13

mock index_document instead of endpoint directly

9d923d8

One last unit test change to get past 500 (still other errors)

4ac1909

add guard to bulk path, implement UserError

57469f3

jacob50231 added 2 commits May 15, 2026 07:56

move acl-only else block

c96582c

Merge branch 'feat/add-bulk-presigned-url' of github.com:uc-cdis/fenc…

c15bee6

…e into feat/add-bulk-presigned-url

k-burt-uch added 2 commits May 15, 2026 15:04

Add unit tests for DRS 1.5 bulk presigned url

9bb0156

Add docs for pytests

cfe6f32

SpencerAxelrod reviewed May 18, 2026

View reviewed changes



		class BulkObjectAccessRequest(BaseModel):
		passports: Optional[list[str]]



		@blueprint.route("/download/bulk", methods=["POST"])
		def download_bulk_files():

Conversation

jacob50231 commented Apr 15, 2026

New Features

Breaking Changes

Bug Fixes

Improvements

Dependency updates

Deployment changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 5, 2026

Integration Tests

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 13, 2026

Integration Tests

Uh oh!

github-actions Bot commented May 13, 2026

Integration Tests

Uh oh!

github-actions Bot commented May 14, 2026

Integration Tests

Uh oh!

github-actions Bot commented May 14, 2026

Integration Tests

Uh oh!

github-actions Bot commented May 14, 2026

Integration Tests

Uh oh!

github-actions Bot commented May 15, 2026

Integration Tests

Uh oh!

github-actions Bot commented May 15, 2026

Integration Tests

Uh oh!

github-actions Bot commented May 15, 2026

Integration Tests

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants