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

Feature/openid federation #10

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

Tommylans
Copy link
Member

No description provided.

@Tommylans Tommylans requested a review from TimoGlastra December 5, 2024 18:14
@Tommylans Tommylans force-pushed the feature/openid-federation branch from 090a081 to 2d13142 Compare December 5, 2024 18:15
@@ -22,7 +22,10 @@ export const vCredentialRequestJwtProofTypeHeader = v.pipe(
({ kid, jwk }) => jwk === undefined || kid === undefined,
`Both 'jwk' and 'kid' are defined. Only one is allowed`
),
v.check(({ trust_chain, kid }) => !trust_chain || !kid, `When 'trust_chain' is provided, 'kid' is required`)
v.check(
({ trust_chain, kid }) => !trust_chain || kid !== undefined,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious did you run into an issue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I did some work on it for supporting this property. I didn't push it with it but I had a array and a kid defined. But I still got this error so I found out that the validation is was not correct.

* Custom fetch implementation to use
*/
fetch?: Fetch
callbackContext: Pick<CallbackContext, 'fetch' | 'verifyJwt' | 'signJwt'>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the rest of the codebase i thinks it's just called callbacks

const result = await fetchWellKnownMetadata(wellKnownMetadataUrl, vCredentialIssuerMetadataWithDraftVersion, fetch)
let result: v.InferOutput<typeof vCredentialIssuerMetadataWithDraftVersion> | null = null

const entityConfiguration = await fetchEntityConfiguration({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking for a while whether the approach is nice and simple, or that it misses a few things.

What i'm not sure about:

  • we lose knowledge about whether federation is used
  • we lose knowledge on the verification result
  • metadata is dependent on the trust chain. We currently don't support metadata policies but since we will in the future i think it might be better to handle this in credo.

if (entityConfiguration) {
const credentialIssuerMetadata = await v.safeParseAsync(
vCredentialIssuerMetadataWithDraftVersion,
entityConfiguration.metadata?.openid_provider
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the issuer metadata called openid_provider?

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