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.

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

fix: tweak the format of the verifiable credential JWT #3614

Merged
merged 1 commit into from Aug 23, 2023

Conversation

hperl
Copy link
Contributor

@hperl hperl commented Aug 23, 2023

This PR implements the following tweaks to the format of the JWT:

  • The JWT should have an exp claim.
  • The JWT should have an nbf claim in addition to iat (OK if these are the same)
  • The top-level sub would have to be the did:jwk value. But it's OK to have the real sub if it's inside credentialSubject.
  • The aud claim should be deleted, since it represents the intended verifier, not the client to whom the VC is issued. (An important difference from normal OIDC is that these are different!) In the Webex case, it's OK, since the app issuing the VC is the same as the app verifying it. But that's not true in general.
  • The UserInfo claims (email, email_verified, updated_at) should be inside vc.credentialSubject.

For reference, an example JWT VC looks like this:

eyJhbGciOiJFUzI1NiIsImtpZCI6Imh5ZHJhLm9wZW5pZC5pZC10b2tlbiIsInR5cCI6IkpXVCJ9.eyJleHAiOjEuNjkyNzkzODg4ZSswOSwiaWF0IjoxLjY5Mjc5MDI4OGUrMDksImlzcyI6Imh0dHA6Ly8xMjcuMC4wLjE6NTkxMjMiLCJqdGkiOiJhZjYzOGE4My00ZjU3LTQ3YzQtYjdlMy0wOTIxY2I4NDIyYmEiLCJuYmYiOjEuNjkyNzkwMjg4ZSswOSwic3ViIjoiZGlkOmp3azpleUpyZEhraU9pSkZReUlzSW1OeWRpSTZJbEF0TWpVMklpd2lZV3huSWpvaVJWTXlOVFlpTENKNElqb2lTR3hGUVdkQ05tUmllbkowYUZSNU5HeGxiVWxqUlZSNlpFZGFkM0pQTXpSWVltdHVhRU4wVmpFd2F5SXNJbmtpT2lKaVlVdHZPWGMzUlZaWlowRjJlVnBZWDIxRWVtaEVhSGRWZFhOWVZ6TTNSa1JLTm0xVlRXaFlNa3BaSW4wIiwidmMiOnsiQGNvbnRleHQiOlsiaHR0cHM6Ly93d3cudzMub3JnLzIwMTgvY3JlZGVudGlhbHMvdjEiXSwiY3JlZGVudGlhbFN1YmplY3QiOnsiYmFyIjoiYmF6IiwiZW1haWwiOiJmb29AYmFyLmNvbSIsImlkIjoiZGlkOmp3azpleUpyZEhraU9pSkZReUlzSW1OeWRpSTZJbEF0TWpVMklpd2lZV3huSWpvaVJWTXlOVFlpTENKNElqb2lTR3hGUVdkQ05tUmllbkowYUZSNU5HeGxiVWxqUlZSNlpFZGFkM0pQTXpSWVltdHVhRU4wVmpFd2F5SXNJbmtpT2lKaVlVdHZPWGMzUlZaWlowRjJlVnBZWDIxRWVtaEVhSGRWZFhOWVZ6TTNSa1JLTm0xVlRXaFlNa3BaSW4wIiwic2lkIjoiMmNiOGQ2YjctZmU0Mi00MjM4LTg5ZGItOGE1M2YzYTQxZThlIiwic3ViIjoiYWVuZWFzLXJla2thcyJ9LCJ0eXBlIjpbIlZlcmlmaWFibGVDcmVkZW50aWFsIiwiVXNlckluZm9DcmVkZW50aWFsIl19fQ.q8KBPvqBTQgO5_mkoGYklPg-0GDm1hJo9xMtU2rrYLKUzRWPAMqdjEHuVztrA9dYhB_qXtIa8cylA19bD508pw

which decodes to the following claims:

{
  "exp": 1692793888,
  "iat": 1692790288,
  "iss": "http://127.0.0.1:59123",
  "jti": "af638a83-4f57-47c4-b7e3-0921cb8422ba",
  "nbf": 1692790288,
  "sub": "did:jwk:eyJrdHkiOiJFQyIsImNydiI6IlAtMjU2IiwiYWxnIjoiRVMyNTYiLCJ4IjoiSGxFQWdCNmRienJ0aFR5NGxlbUljRVR6ZEdad3JPMzRYYmtuaEN0VjEwayIsInkiOiJiYUtvOXc3RVZZZ0F2eVpYX21EemhEaHdVdXNYVzM3RkRKNm1VTWhYMkpZIn0",
  "vc": {
    "@context": [
      "https://www.w3.org/2018/credentials/v1"
    ],
    "credentialSubject": {
      "bar": "baz",
      "email": "foo@bar.com",
      "id": "did:jwk:eyJrdHkiOiJFQyIsImNydiI6IlAtMjU2IiwiYWxnIjoiRVMyNTYiLCJ4IjoiSGxFQWdCNmRienJ0aFR5NGxlbUljRVR6ZEdad3JPMzRYYmtuaEN0VjEwayIsInkiOiJiYUtvOXc3RVZZZ0F2eVpYX21EemhEaHdVdXNYVzM3RkRKNm1VTWhYMkpZIn0",
      "sid": "2cb8d6b7-fe42-4238-89db-8a53f3a41e8e",
      "sub": "aeneas-rekkas"
    },
    "type": [
      "VerifiableCredential",
      "UserInfoCredential"
    ]
  }
}

@hperl hperl requested a review from aeneasr as a code owner August 23, 2023 11:33
@hperl hperl requested a review from alnr August 23, 2023 11:33
@hperl hperl self-assigned this Aug 23, 2023
@hperl hperl force-pushed the hperl/fix-vc-format branch 2 times, most recently from 7b3271e to f539b83 Compare August 23, 2023 11:35
@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

Merging #3614 (d1922a5) into master (ad8a4ba) will decrease coverage by 0.05%.
Report is 2 commits behind head on master.
The diff coverage is 75.00%.

❗ Current head d1922a5 differs from pull request most recent head ef5429a. Consider uploading reports for the commit ef5429a to get more accurate results

@@            Coverage Diff             @@
##           master    #3614      +/-   ##
==========================================
- Coverage   76.22%   76.17%   -0.05%     
==========================================
  Files         132      133       +1     
  Lines       10009    10037      +28     
==========================================
+ Hits         7629     7646      +17     
- Misses       1860     1868       +8     
- Partials      520      523       +3     
Files Changed Coverage Δ
oauth2/verifiable_credentials.go 45.45% <45.45%> (ø)
oauth2/handler.go 67.94% <88.00%> (+0.52%) ⬆️

... and 2 files with indirect coverage changes

@aeneasr aeneasr merged commit 0176adc into master Aug 23, 2023
29 checks passed
@aeneasr aeneasr deleted the hperl/fix-vc-format branch August 23, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants