Skip to content

[OID4VCI] Make sure events are properly used in OID4VCI endpoints #44946

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.

Sign up for GitHub

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

Jump to bottom
Open
forkimenjeckayang wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[OID4VCI] Make sure events are properly used in OID4VCI endpoints #44946

forkimenjeckayang wants to merge 2 commits into from

Conversation

@forkimenjeckayang
Copy link
Contributor

@forkimenjeckayang forkimenjeckayang commented Dec 16, 2025 •
edited by mposolda
Loading

Summary

Adds event tracking to all OID4VCI issuance endpoints to align with Keycloak's event system.

Changes

  • Added new event types: CREDENTIAL_REQUEST, CREDENTIAL_OFFER_REQUEST, and CREDENTIAL_NONCE_REQUEST (with error variants)
  • Implemented event firing in all OID4VCI endpoints:
    • POST /credential - tracks credential issuance requests
    • GET /credential-offer-uri - tracks offer URI generation
    • GET /credential-offer/{nonce} - tracks offer retrieval
    • POST /nonce - tracks nonce generation
  • Added event assertions to existing test classes to verify proper event firing

closes #44679

@forkimenjeckayang forkimenjeckayang requested review from a team as code owners December 16, 2025 13:32
@forkimenjeckayang forkimenjeckayang mentioned this pull request Dec 16, 2025
Open
@mposolda mposolda self-assigned this Dec 16, 2025
@mposolda mposolda added the area/oid4vc Issue related to OpenID for Verifiable Credentials label Dec 16, 2025
@keycloak-github-bot keycloak-github-bot bot mentioned this pull request Dec 16, 2025
Closed
mposolda
mposolda reviewed Dec 17, 2025
View reviewed changes
Copy link
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

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

@forkimenjeckayang Thanks for this PR!

I've added few minor comments inline. Besides that, is it please possible to test also some error scenarios a little bit? As it seems all the event tests are testing mostly successful flow scenarios.

It looks that in case of OID4VCAuthorizationCodeFlowTest , there are not yet any error scenarios tested (I am adding some in the PR #44954 ), but for some other automated tests, hopefully it is possible to test some "error events" ?

@mposolda
Copy link
Contributor

mposolda commented Dec 17, 2025

@forkimenjeckayang Small hint (not directly related to this PR): Can you please try to add some line with just something like:

closes #44679

in the PR descriptions? It might be added automatically if you use this in the commit message (which is preferred).

If you use something like:

## Closses
- **Issue**: https://github.com/keycloak/keycloak/issues/44679

It is not ideal as it does not automatically link the PR with the related issue.

@mposolda
Copy link
Contributor

mposolda commented Dec 18, 2025

@forkimenjeckayang I've merged the PR for "mandatory" claims #44954 . Is it possible that in this PR, you add also error event details in the OID4VCIssuerEndpoint.validateRequestedClaimsArePresent ? Hopefully it is possible to do in a way that also details about which mandatory claim is missing are available in the event details (maybe the reason detail, which is typically for the error causes).

Also should be possible to test it somehow in OID4VCAuthorizationCodeFlowTestBase.assertErrorCredentialResponse

@forkimenjeckayang
Copy link
Contributor Author

forkimenjeckayang commented Dec 18, 2025

@forkimenjeckayang I've merged the PR for "mandatory" claims #44954 . Is it possible that in this PR, you add also error event details in the OID4VCIssuerEndpoint.validateRequestedClaimsArePresent ? Hopefully it is possible to do in a way that also details about which mandatory claim is missing are available in the event details (maybe the reason detail, which is typically for the error causes).

Also should be possible to test it somehow in OID4VCAuthorizationCodeFlowTestBase.assertErrorCredentialResponse

Hello @mposolda , yes sure, I will address that. The PR should be ready again by tomorrow

@forkimenjeckayang
Copy link
Contributor Author

forkimenjeckayang commented Dec 22, 2025

Hello @mposolda , i've taken into consideration all you mentioned above.
Please have a look when you're chanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mposolda mposolda Awaiting requested review from mposolda

At least 1 approving review is required to merge this pull request.

Assignees

@mposolda mposolda

Labels

area/oid4vc Issue related to OpenID for Verifiable Credentials flaky-test team/cloud-native team/core-iam

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make sure events are properly used in OID4VCI endpoints

2 participants

@forkimenjeckayang @mposolda