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

expect transformCredentialInput to move top-level props of W3C credentials inside vc #73

Closed
joshbax189 opened this issue Apr 20, 2021 · 5 comments · Fixed by #75
Closed
Assignees
Labels
bug Something isn't working

Comments

@joshbax189
Copy link
Contributor

Based on my reading of the W3C spec, it seems as though top-level optional props like evidence and credentialSubject should be copied inside the vc claim in the resulting JWT encoded credential.

However, when you pass transformCredentialInput an arg of type CredentialPayload they are copied to the top level of the JWT:

const cred = {
  '@context': ['https://www.w3.org/2018/credentials/v1'],
  credentialSubject: {
    id: 'did:ethr:0x435df3eda57154cf8cf7926079881f2912f54db4',
    bar: 'baz',
  },
  evidence: { foo: 'bar' },
  type: ['VerifiableCredential'],
};

transformCredentialInput(cred);

Results in

{
  vc: {
    credentialSubject: { bar: 'baz' },
    '@context': [ 'https://www.w3.org/2018/credentials/v1' ],
    type: [ 'VerifiableCredential' ]
  },
  evidence: { foo: 'bar' },
  sub: 'did:ethr:0x435df3eda57154cf8cf7926079881f2912f54db4'
}

Expect

{
  vc: {
    credentialSubject: { bar: 'baz' },
    '@context': [ 'https://www.w3.org/2018/credentials/v1' ],
    type: [ 'VerifiableCredential' ],
    evidence: { foo: 'bar' }
  },
  sub: 'did:ethr:0x435df3eda57154cf8cf7926079881f2912f54db4'
}

A workaround is to move the props inside a manually constructed vc object. But then it's unclear whether all of the props should be manually transformed to match the JwtCredentialPayload type or left for the transformation function to handle.

@mirceanis
Copy link
Member

Excellent question.
I suppose that answering it would make the spec a lot clearer (and maybe also the implementation).

@awoie do you have an opinion here?

@joshbax189
Copy link
Contributor Author

Also, normalizeCredential has the inverse problem:

normalizeCredential({
      vc: {
          credentialSubject: { bar: 'baz' },
          '@context': [ 'https://www w3 org/2018/credentials/v1' ],
          type: [ 'VerifiableCredential' ],
          evidence: { foo: 'bar' }
        },
      sub: 'did:ethr:0x435df3eda57154cf8cf7926079881f2912f54db4'
    })

Result

{
  proof: {},
  vc: { evidence: { foo: 'bar' } },
  credentialSubject: {
    bar: 'baz',
    id: 'did:ethr:0x435df3eda57154cf8cf7926079881f2912f54db4'
  },
  issuer: {},
  type: [ 'VerifiableCredential' ],
  '@context': [ 'https://www.w3.org/2018/credentials/v1' ]
}

@mirceanis
Copy link
Member

yes, and the same for the presentation transforms.

Also it loosely relates to #64 since it relates to JWT-JSON payload conversion.

@rado0x54 rado0x54 added the bug Something isn't working label May 7, 2021
@rado0x54 rado0x54 self-assigned this May 10, 2021
@rado0x54
Copy link
Contributor

Hey, I explicitly fixed the code to handle 'credentialStatus', 'evidence' and 'termsOfUse' correctly. A more general refactor of these conversion functions should be addressed in #64

mirceanis pushed a commit that referenced this issue May 28, 2021
* Explicitly map `credentialStatus`, `termsOfUse` and `evidence` between between JWT and W3C VC representations.

* tests: extended test coverage

fixes #73
uport-automation-bot pushed a commit that referenced this issue May 28, 2021
## [2.1.2](2.1.1...2.1.2) (2021-05-28)

### Bug Fixes

* mapp additional W3C spec fields to and from JWT VC ([#75](#75)) ([e0482dc](e0482dc)), closes [#73](#73)
@uport-automation-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 2.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants