feat: OpenID Federation for the verifier <-> holder #27
Conversation
Signed-off-by: Tom Lanser <[email protected]>
Signed-off-by: Tom Lanser <[email protected]>
Tommylans
force-pushed
the
feature/openid-federation-verfier
branch
from
310e1a6
to
bcaed4d
Compare
Signed-off-by: Tom Lanser <[email protected]>
There was a problem hiding this comment.
There's two things I'm missing:
- actual verification of the JWT to be verified (see comment)
- the federation metadata taking precedence of the non-federation metadata. Is that still todo? So the
client_metadataMUST be ignored when federation is used, and only the federation metadata should be used.
| const openidProvider = await this.getOpenIdProvider(agentContext, { | ||
| federation: options.federation, | ||
| }) | ||
|
|
There was a problem hiding this comment.
This is ok for now, but was also discussing with NF and i think we probably need to have some more control over 'trusted' entities in Credo.
So extend the x509 callbacks with global 'verificationContext` and you can either provide it to the call, or we use the global static config, or we use the dynamic callback. And you can either provide trusted federations, trusted x509s, or trusted dids / did methods.
(just rambling here 😄 )
There was a problem hiding this comment.
Yeah, I think we should think a bit more about this of what a good structure would be.
packages/openid4vc/src/openid4vc-issuer/OpenId4VcIssuerModuleConfig.ts
Outdated
Show resolved
Hide resolved
| const { key } = await config.keyCallback(agentContext, { | ||
| verifierId: verifier.verifierId, | ||
| }) |
There was a problem hiding this comment.
I'm not sure what this is? Is it the key used for the entity? I think that can just be a key generated by Credo right on the spot? or we should store it in some sort of FederationEntityRecord that we create if it doesn't exist yet?
| const jwk = getJwkFromKey(key) | ||
| const alg = jwk.supportedSignatureAlgorithms[0] | ||
| const kid = 'key-1' |
There was a problem hiding this comment.
can an federation put restrictions on which key types are used for signing the entity configurations?
| @@ -0,0 +1,9 @@ | |||
| import type { AgentContext, Key } from '@credo-ts/core' | |||
|
|
|||
| // TODO: Not really sure about this type yet but it's a start. | |||
There was a problem hiding this comment.
see my other comment ,do we really need this callback?
packages/openid4vc/src/openid4vc-issuer/router/federationEndpoint.ts
Outdated
Show resolved
Hide resolved
| organization_name: issuerDisplay.organization_name, | ||
| logo_uri: issuerDisplay.logo_uri, |
There was a problem hiding this comment.
organization_name and logo_uri are different in te issuer display. I think name and logo.uri. I think we can just reuse them instead of requiring duplicate parameters?
| const issuanceBaseUrl = `${baseUrl}/oid4vci` | ||
| const verificationBaseUrl = `${baseUrl}/oid4vp` | ||
|
|
||
| describe('OpenId4Vc', () => { |
There was a problem hiding this comment.
can we also add a negative test (non-trusted federatino)?
packages/openid4vc/src/openid4vc-verifier/__tests__/openid4vc-verifier.test.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Tom Lanser <[email protected]>
Signed-off-by: Tom Lanser <[email protected]>
Signed-off-by: Tom Lanser <[email protected]>
Signed-off-by: Tom Lanser <[email protected]>
The main branch isn't updated so the pr looks really big. So if we do that it will become more clear.
Not everything is finished yet. But it would be really nice to have this merged quite soon so we can start on the implementation for the wallet.
For the issuance it's a bit more unclear of what's need to happen will do some more investigation quickly.
Also the policies are not merged yet in the openid federation. But is planned but the merge strategy is a bit complex so the main goal was to have the structure working for now