DT-2991: GA4GH Passport User Info API by rushtong · Pull Request #2237 · DataBiosphere/consent

rushtong · 2024-01-26T16:15:32Z

Addresses

https://broadworkbench.atlassian.net/browse/DT-2991

Summary

This PR represents our first effort to generate a compliant GA4GH passport implementation. This PR implements the following features:

Authenticated userinfo endpoint that provides a JSON Passport. This follows the implementation requirements specified in the AAI OpenID spec
Passport consisting a list of visas:
- ControlledAccessGrants Visa per dataset the user has current access to.
- AffiliationAndRole Visa describing the user's affiliation
- ResearcherStatus Visa describing the user's status

What this PR does NOT cover

This work should be slated for future implementation efforts.

Provide a passport scoped access token
- Requires an additional scope that we do not currently support: ga4gh_passport_v1
Provide a signed JWT
- Requires public and private keys
Complete the currently unimplemented claim types:
- AcceptedTermsAndPolicies
- LinkedIdentities
ResearcherStatus: We need to provide a URL Claim to describe a researcher

Have you read CONTRIBUTING.md lately? If not, do that first.

Label PR with a Jira ticket number and include a link to the ticket
Label PR with a security risk modifier [no, low, medium, high]
PR describes scope of changes
Get a minimum of one thumbs worth of review, preferably two if enough team members are available
Get PO sign-off for all non-trivial UI or workflow changes
Verify all tests go green
Test this change deployed correctly and works on dev environment after deployment

rushtong · 2025-03-19T15:08:35Z

@dependabot show ignore conditions

# Conflicts: # src/main/java/org/broadinstitute/consent/http/ConsentApplication.java

# Conflicts: # src/main/resources/assets/api-docs.yaml

Copilot feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

otchet-broad · 2026-03-02T20:03:14Z

+            .map(LibraryCard::getCreateDate)
+            .orElse(user.getCreateDate());
+    // GA4GH requires Unix timestamps in seconds; Date#getTime() returns milliseconds.
+    return assertedDate.getTime() / 1000L;


What's the tolerance for being off by a second? Technically speaking we could be rounding up. If it's inconsequential, I'm not concerned. If it is, we might want to convert these Date objects to something like Instant and use the built in functions to convert to epoch seconds.

We don't have a hard requirement for that level of precision. I would be amenable to a more centralized function to ensure we're always following the same pattern.

otchet-broad · 2026-03-02T20:14:09Z

+  }
+
+  @Override
+  public Long asserted() {


What does this date represent? How will a consumer use it?

This represents the seconds since the visa assertion is made: https://ga4gh.github.io/data-security/ga4gh-passport#visa-object-claims

otchet-broad · 2026-03-02T20:16:14Z

+
+  protected List<Visa> buildControlledAccessGrants(
+      String userSubjectId, List<ApprovedDataset> approvedDatasets) {
+    return approvedDatasets.stream()


Do we need to be more selective about the filter here? Don't we want the ApprovedDataset with the longest runway?

That's a good point. I should sort them by the expiration and then filter out duplicates so we get the ones that extend the furthest into the future.

After digging into this some more, it doesn't matter which of the ApprovedDataset entities we return, only that we return one of them. IOW, if the user has access to two ApprovedDataset objects, one that expires tomorrow, one that expires next year, they have the same underlying dataset identifier.

It's true that they'll have the same identifier, but I'm thinking about how we communicate when access to that dataset is no longer authorized. I haven't gotten through enough of the spec yet to see how that unwinds (or if it's even supported). What is clear to me is that the expiration (exp) of the Visa shouldn't be past when the we would remove access (when access is planned to expire or the user's LC is revoked). Fun stuff we'll be able to work through in future PRs. :-)

What is clear to me is that the expiration (exp) of the Visa shouldn't be past when the we would remove access (when access is planned to expire or the user's LC is revoked)

Of these two conditions, we have control over the planned expiration, but not future LC revocation. I can see improvements on the expiration front at least.

otchet-broad · 2026-03-03T02:28:50Z

+    description: The Issued-At timestamp
+  exp:
+    type: number
+    description: The Expiration timestamp


We might want to be clearer that making an access decision after the exp is reached is unsupported.

ControlledAccessGrants is just one type of visa which is what that language would be applicable to. I think it would be confusing with respect to the other visa types that exist, such as ResearcherStatus or AffiliationAndRole.

ControlledAccessGrants is just one type of visa which is what that language would be applicable to. I think it would be confusing with respect to the other visa types that exist, such as ResearcherStatus or AffiliationAndRole.

I have to correct myself. exp applies equally to all Visas, and in the case of ControlledAccessGrants it does NOT refer to the expiration of the user's access to the dataset. It applies only to the state of the Visa such that the visa will always expire 1 hour from issuance.

otchet-broad · 2026-03-03T02:32:49Z

As I'm coming up to speed on the GA4GH Passport spec, I'm wondering if we should be creating durable logs for every passport we issue so that we can trace (with high fidelity) all of the clients that have requested these objects and know their expiration so that we can:

Inform overseers and data custodians about potential usage in other systems.
Maintain a clear audit trail for issuance.
Monitor for abuse.

rushtong · 2026-03-03T13:20:36Z

As I'm coming up to speed on the GA4GH Passport spec, I'm wondering if we should be creating durable logs for every passport we issue so that we can trace (with high fidelity) all of the clients that have requested these objects and know their expiration so that we can:

Inform overseers and data custodians about potential usage in other systems.

Maintain a clear audit trail for issuance.

Monitor for abuse.

Absolutely, these are great features. There are a number of other things we need to do to be fully compliant as noted in the PR description so I intend that this be iterated upon over time. Currently, there are no clients that will be using this in the near (1-3 month at a minimum) future.

fboulnois

As a first approach, this looks reasonable! 👍

otchet-broad · 2026-03-04T14:27:19Z

Thanks for the harmonization around dates. I think that's going to be helpful in the future.

I'm wondering if the resource should be updated so that it's not PermitAll, but rather Admin and maybe Service Account for now. I think there are Expiration and Revocation cases that are important from a compliance standpoint that are missing from this that make me concerned we'll communicate a controlled access grant without having these other mechanisms in place. https://github.com/ga4gh-duri/ga4gh-duri.github.io/blob/master/researcher_ids/ga4gh_passport_v1.md#overview

Would that be an okay next step on this?

rushtong · 2026-03-04T14:46:22Z

Thanks for the harmonization around dates. I think that's going to be helpful in the future.

I'm wondering if the resource should be updated so that it's not PermitAll, but rather Admin and maybe Service Account for now. I think there are Expiration and Revocation cases that are important from a compliance standpoint that are missing from this that make me concerned we'll communicate a controlled access grant without having these other mechanisms in place. https://github.com/ga4gh-duri/ga4gh-duri.github.io/blob/master/researcher_ids/ga4gh_passport_v1.md#overview

Would that be an okay next step on this?

Sure thing. I'd prefer to limit it to Admin for now rather than SA since they are not an intended audience, general researchers are the current target. For comparison, this API is doing essentially the same thing that https://consent.dsde-prod.broadinstitute.org/#/User/get_api_user_me_researcher_datasets does ... retrieving the same entity for an authenticated user. That API is also @PermitAll.

sonarqubecloud · 2026-03-04T15:04:03Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

otchet-broad

👍
Nice work!

rushtong added 2 commits January 26, 2024 11:14

feat: passport api

007bde8

cleanup

1425497

rushtong changed the title ~~Gr passports~~ WIP: Passports Jan 26, 2024

rushtong added 6 commits March 19, 2024 10:55

Merge branch 'develop' into gr-passports

00a9a3e

Merge branch 'refs/heads/develop' into gr-passports

e5af226

Merge branch 'refs/heads/develop' into gr-passports

a3dd2cc

Merge branch 'refs/heads/develop' into gr-passports

2188f94

Merge branch 'refs/heads/develop' into gr-passports

e3ba844

feat: issuer http -> https

ca2847a

This comment was marked as outdated.

Sign in to view

rushtong added 10 commits August 1, 2025 07:25

Merge branch 'develop' into gr-passports

9995c21

merge fixes

c661989

address some sonar warnings

c2e919e

use subject id for visa sub values

a1c8da0

add researcher status; minor refactoring

07c6067

Merge branch 'develop' into gr-passports

221795a

fix asserted calculation

44dd1b5

minor refactoring

4bc7001

accepted terms and policies WIP

50e8069

Merge branch 'develop' into gr-passports

6c870db

This comment was marked as outdated.

Sign in to view

rushtong added 2 commits September 30, 2025 08:46

Merge branch 'develop' into gr-passports

b1986ae

# Conflicts: # src/main/java/org/broadinstitute/consent/http/ConsentApplication.java

merge fix

b09f69b

This comment was marked as outdated.

Sign in to view

rushtong added 5 commits February 26, 2026 12:42

Merge branch 'develop' into gr-passports

9062c5a

# Conflicts: # src/main/java/org/broadinstitute/consent/http/ConsentApplication.java

refactoring after merge

398062e

Merge branch 'develop' into gr-passports

b8cf3c4

# Conflicts: # src/main/resources/assets/api-docs.yaml

align with AffiliationAndRole

dcd0586

rm unused

a168d2b

rushtong and others added 5 commits February 27, 2026 10:20

Apply suggestions from code review

2cee1a6

Copilot feedback Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

copilot feedback: ensure all iat and exp values are epoch seconds

1adc117

copilot feedback: handle null user case; remove extraneous logging

1ffc3f5

copilot feedback: handle null userstatusinfo case

dd6c3aa

copilot feedback: handle null dataset identifier case

25411bd

rushtong requested a review from Copilot February 27, 2026 17:52

Copilot started reviewing on behalf of rushtong February 27, 2026 17:53 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

rushtong and others added 5 commits February 27, 2026 13:37

copilot feedback: types

85e1329

test cleanup

20199a2

Apply suggestions from code review

f703f0a

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge remote-tracking branch 'origin/gr-passports' into gr-passports

d8627bb

ensure asserted is not null per spec.

3e4316b

otchet-broad reviewed Mar 2, 2026

View reviewed changes

otchet-broad reviewed Mar 3, 2026

View reviewed changes

rushtong added 3 commits March 3, 2026 07:52

Merge branch 'develop' into gr-passports

8ddaaa1

PR feedback: consolidate date/calendar/instant handling for consistency

f11b4ee

doc: remove stray comment

c97170d

rushtong added the help wanted label Mar 3, 2026

fboulnois approved these changes Mar 3, 2026

View reviewed changes

rushtong requested a review from otchet-broad March 4, 2026 13:33

feat: limit to admins for initial trial

8421ee3

otchet-broad approved these changes Mar 4, 2026

View reviewed changes

rushtong merged commit e02bfa4 into develop Mar 4, 2026
14 checks passed

rushtong deleted the gr-passports branch March 4, 2026 15:35

Conversation

rushtong commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Addresses

Summary

What this PR does NOT cover

Uh oh!

rushtong commented Mar 19, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rushtong Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

otchet-broad Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

otchet-broad commented Mar 3, 2026

Uh oh!

rushtong commented Mar 3, 2026

Uh oh!

fboulnois left a comment

Choose a reason for hiding this comment

Uh oh!

otchet-broad commented Mar 4, 2026

Uh oh!

rushtong commented Mar 4, 2026

Uh oh!

sonarqubecloud Bot commented Mar 4, 2026

Quality Gate passed

Uh oh!

otchet-broad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rushtong commented Jan 26, 2024 •

edited

Loading

rushtong Mar 2, 2026 •

edited

Loading

otchet-broad Mar 4, 2026 •

edited

Loading