-
Notifications
You must be signed in to change notification settings - Fork 238
feat: Add support for JWT VC Issuer Metadata #2456
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
base: main
Are you sure you want to change the base?
feat: Add support for JWT VC Issuer Metadata #2456
Conversation
🦋 Changeset detectedLatest commit: 11a773a The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
b52fd78
to
4b523bd
Compare
return { | ||
method: 'jwk', | ||
jwk: result.jwk, | ||
issuer: result.issuer, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking whether we want to call the method something else than jwk
. Since with JWK we really only verify the binding that the credential is issued by a JWK, but with the jwt vc issuer metadata, you also verify the domain of the issuer where the issuer metadata is hosted.
Should we thus call this maybe well-known-issuer-metadata
or well-known-issuer
? I'm open to other names, if we can come up with something simple that does capture the origin of the key (like with x5c and did)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other options:
- 'jwt-vc-issuer-metadata'
- 'jwt-vc'
const operation = { operation: 'verify', algorithm: options.algorithm } as const | ||
const kms = | ||
backend || typeof options.key !== 'string' | ||
backend || typeof options.key.keyId !== 'string' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah thanks for cathing this 👍
export const JwksSchema = z.object({ | ||
keys: z.array(vJwk), | ||
}) | ||
|
||
export const SdJwtVcIssuerMetadataSchema = z.union([ | ||
z.object({ | ||
issuer: z.string().url(), | ||
jwks_uri: z.string().url(), | ||
jwks: z.never().optional(), | ||
}), | ||
z.object({ | ||
issuer: z.string().url(), | ||
jwks_uri: z.never().optional(), | ||
jwks: JwksSchema, | ||
}), | ||
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably makes sense to also move to zod gradually for other modules, but are these validation schemas also used somewhere? Or are they just used for types? I think it would be good to use this in resolveSigningPublicJwkFromJwtVcIssuerMetadata
.
You can use the zParseWithErrorHandling
util method on the returned JSON payload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are used at resolveSigningPublicJwkFromJwtVcIssuerMetadata
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They just use the types as a generic to the fetch method, but they are not actually used for validation. So we don't get runtime validation, only build time validation, and we cas the .json()
to the provided type.
// Configure endpoints | ||
configureIssuerMetadataEndpoint(endpointRouter) | ||
configureJwksEndpoint(endpointRouter, this.config) | ||
configureJwtVcIssuerMetadataEndpoint(endpointRouter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While i understand to add it to the openid4vc router, it does tightly couple the credential format with the issuance protocol. We could add this to the openid4vc module, so it could just register some additional routes, but we would need a new record type for it in that case, and now we can just use the openid4vc issuer record.
I think for this is fine, and we can always make it more flexible later on 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm after thinking about this a bit more, I'm not so sure, since it would mean the jwt-vc-issuer would use the same url as the openid4vc-issuer. I'd expect you'd more often use an url like example.com
over example.com/oid4vci/<uuid>
as the value for iss
.
Wouldn't it make more sense to just host the issuer metadata separately for now? We have the same for did:web, where you can create the did document in Credo, but you need to host it yourself on your webserver on the well konwn path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can support both use cases where we can host our own endpoint and have issuers host on their own servers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this the same thing for the other well-known endpoints?
We host oauth and openid well-known endpoints.
It's the same logic here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if someone wants to issue a jwt vc over didcomm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We host oauth and openid well-known endpoints.
oauth and openid well-known endpoints are inherently tied to openid4vci. You can issue a credenetial over oid4vci without these metadata files. For jwt-vc-issuer metadata it's different. There's no inherent relation between the two, except that you can choose to issue an SD-JWT VC credential over openid4vci (but as @rmlearney-digicatapult, it could theoretically also be done over DIDComm for example)
import type { OpenId4VcIssuanceRequest } from './requestContext' | ||
|
||
export function configureJwtVcIssuerMetadataEndpoint(router: Router) { | ||
router.get('/.well-known/jwt-vc-issuer', async (_request: OpenId4VcIssuanceRequest, response: Response, next) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't be hosted at the correct path looking at the spec. It will be hosted at <url>/oid4vci/<uuid>/.well-known/jwt-vc-issuer
, while it should be hosted at <url>/.well-known/jwt-vc-issuer/oid4vci/<uuid>
We have this issue for other documents as well: #2221
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's the same issue for other documents, but I think we should leave it here until we figure out how to host all the documents correctly.
Plus, I've added support for both URLs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @GianfrancoMS, thanks a lot for this PR! It's a great addition.
I'm thinking whether we should get the support into sd-jwt-vc merged, and hold off on the oid4vci integration and just require it to be hosted separately for now. It's quite easy to add this, and I'm not convinced yet that we should bind this to the oid4vci record yet. Currently the issuance method (did/x5c) is separate from the oid4vci protocol.
@TimoGlastra I tried to generate the migrations for drizzle, but got an error, so I'm not including those migrations in this PR. Hopefully you can help me with that 🙏 |
Signed-off-by: gianfrancoms <gianfrancomonsal@gmail.com>
Signed-off-by: Timo Glastra <timo@animo.id>
Signed-off-by: gianfrancoms <gianfrancomonsal@gmail.com>
Signed-off-by: gianfrancoms <gianfrancomonsal@gmail.com>
42bef4d
to
11a773a
Compare
Add support for JWT VC Issuer Metadata
https://www.ietf.org/archive/id/draft-ietf-oauth-sd-jwt-vc-03.html#name-jwt-vc-issuer-metadata