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

feat: allow any credentialStatus for oa v4 #315

Open
wants to merge 1 commit into
base: beta
Choose a base branch
from

Conversation

nghaninn
Copy link

@nghaninn nghaninn commented Oct 4, 2024

Summary

What is the background of this pull request?

  • To remove restriction for credetialStatus v4

Changes

What are the changes made in this pull request?

  • Allowing any type for credentialStatus in oa v4.

Issues

What are the related issues or stories?

  • TradeTrust will be utilising oa v4. However we will require customisation to allow other type value for credentialStatus.
  • Note: No @context schema validation in place for external type.

Happy to have a discussion for any suggestion.

Removing restriction for credetialStatus
@nghaninn nghaninn marked this pull request as ready for review October 7, 2024 02:34
@HJunyuan
Copy link
Member

HJunyuan commented Oct 7, 2024

Hi @nghaninn ,

Thanks for the suggestion. May we clarify what are some other type values of credentialStatus you are looking at?

This is because adding z.object({ type: z.string() }).passthrough() might be too permissive, overriding our first 2 definitions (OscpResponderRevocation, RevocationStoreRevocation).

As a result, an object like this would pass incorrectly:

"credentialStatus": {
  // id: "MISSING_ID",
  type: "OpenAttestationOcspResponder"
}

The credentialStatus field is intentionally strict to ensure that all OA documents will be issued in an expected manner. OA schema should be an intentionally narrowed subset of the W3C VC data model so the OA framework knows exactly how to handle these documents end-to-end (from issuance, to verification and to rendering). Adding a generic definition would dilute the OA definition and might lead to ambiguity upon verification.

@nghaninn
Copy link
Author

nghaninn commented Oct 7, 2024

Hi @HJunyuan.

We wanted to add a new type for TransferableRecords, such that the object will be like

    credentialStatus: {
      type: "TransferableRecords",
      id: "0xb1767E3B31A286b051684CdBe0447FBd483D71A7",
      tokenNetwork: {
        chain: "Amoy",
        chainId: 80002
      },
      tokenRegistry: "0xb1767E3B31A286b051684CdBe0447FBd483D71A7"
    },

Understand the strict enforcement. Will the below worth within the constraint? with additional @context

export const TransferableRecords = z.object({
  id: z.string().url().describe("Address of smart contract "),
  type: z.literal("TransferableRecords"),
  tokenNetwork: z.object({
    chainId: z.number(),
    chain: z.string(),
  }).passthrough(),
  tokenRegistry: z.string(),
}).passthrough();

...

    credentialStatus: z.discriminatedUnion("type", [OscpResponderRevocation, RevocationStoreRevocation, TransferableRecords]).optional(),

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.

2 participants