Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

verifiable consent of visa usage #40

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

verifiable consent of visa usage #40

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e36ca3e
verifiable consent of visa usage
dvoet Feb 14, 2020
e5dc9ad
add version info
dvoet Feb 14, 2020
a0fbb0b
address PR comments
dvoet Mar 10, 2020
9debd79
minor language updates
dvoet Mar 30, 2020
303994e
Merge branch 'master' into master
dvoet Mar 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion AAI/AAIConnectProfile.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

| Version | Date | Editor | Notes |
|---------|---------|--------------------------------------------|-------------------------|
| 1.0.3 | 2020-04 | Douglas Voet | Add audiences to embedded tokens |
| 1.0.2 | 2020-02 | David Bernick | Clarify risk scenarios |
| 1.0.1 | 2019-10 | David Bernick | Clarify that non-GA4GH claims are allowed in tokens |
| 1.0.0 | 2019-10 | Approved by GA4GH Steering Committee | |
Expand Down Expand Up @@ -397,6 +398,12 @@ the Broker.
presented and/or transformed without misrepresenting the original intent,
except for accommodating for `exp` timestamps to be represented as
indicated above.

3. An Embedded Token Issuer MAY include the `aud` claim to identify the
[Brokers](#term-broker) as the intended audience as specified by
[RFC 7523 Section 3](https://tools.ietf.org/html/rfc7523#section-3).
When an `aud` claim is specified, the values within the `aud` claim MUST
match the `iss` claim of access tokens issued by consented Brokers.

#### Conformance for Claim Clearinghouses (consuming Access Tokens to give access to data)

Expand Down Expand Up @@ -429,6 +436,11 @@ the Broker.
any other Broker involved in the propagation of the claims to
also be trusted if the Claim Clearinghouse needs to restrict its
trust model).
2. If an Embedded Token contains an `aud` claim, Clearinghouse MUST check
that one of the Embedded Token's `aud` entries
[matches](https://tools.ietf.org/html/rfc3986#section-6.2.1) the Broker's
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why broker's? The "aud" claim means "audience", so it should match the clearing house's client id, not the broker's issuer id. The audience ("the recipients processing the JWT" as per RFC 7519#4.1.3) is the clearing house, not the broker.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the PR summary for what I am trying to achieve.

In addition to allowing the aud claim to contain brokers we could say it must also contain clearing houses then the clearing house must verify that it is in the list itself as well as the broker. Essentially the visa issuer would specify all the parties, brokers and clearing houses, who are allowed to use the JWT. This spec would then go above and beyond the JWT RFC by saying the clearing house much check that the broker is in the aud claim.

If aud is not the right claim to use then I would propose adding a new claim to the spec and registering that with IANA.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining a new claim seems to be a better way to me.

`iss` claim (i.e. a Broker's access token `iss` claim must match the `aud`
claim within its Embedded Tokens if the Embedded Token aud claim is provided).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aud -> aud


3. MUST check `exp` to ensure the token has not expired.

Expand Down Expand Up @@ -660,7 +672,7 @@ where:
4. The payload claims MAY contain at least one GA4GH Claim
(`<ga4gh-spec-claims>`).

5. The payload claims MUST NOT include `aud`.
5. The payload claims MAY include `aud` to list approved brokers

##### Embedded Document Token Format

Expand Down Expand Up @@ -706,6 +718,8 @@ Issuer.
- `<ga4gh-spec-claims>`: OPTIONAL. One or more GA4GH Claims MAY be
provided. See [Authorization/Claims](#authorizationclaims) for an
example.

- MAY contain `aud` to list approved brokers

#### Authorization/Claims

Expand Down