Skip to content

Conversation

@forkimenjeckayang
Copy link
Collaborator

@forkimenjeckayang forkimenjeckayang commented Jun 18, 2025

Summary

This PR implements support for parsing and understanding authorization_details parameter at the Token Endpoint for OpenID4VCI (OpenID for Verifiable Credential Issuance).

Implementation Includes:

1. Authorization Details Processing

  • Added support for parsing authorization_details parameter in token requests
  • Supports both credential_configuration_id and format-based authorization details
  • Validates authorization details structure and prevents invalid combinations

2. Credential Identifier Generation

  • Generates unique credential identifiers based on authorization details
  • Format: {UUID}

3. Session-based Identifier Persistence

  • Stores credential identifiers in user session notes for persistence
  • Reuses identifiers for the same credential_configuration_id within the same session
  • Generates different identifiers for different sessions (session isolation)
  • Supports both PreAuthorizedCode and AuthorizationCode grant types

4. Response Enhancement

  • Returns authorization_details in token responses with generated credential identifiers
  • Maintains backward compatibility with existing token response format
  • Includes proper validation and error handling

stephane-segning

This comment was marked as off-topic.

stephane-segning

This comment was marked as off-topic.

stephane-segning

This comment was marked as off-topic.

stephane-segning

This comment was marked as off-topic.

stephane-segning

This comment was marked as off-topic.

stephane-segning

This comment was marked as off-topic.

stephane-segning

This comment was marked as resolved.

stephane-segning

This comment was marked as off-topic.

Copy link
Collaborator

@stephane-segning stephane-segning left a comment

Choose a reason for hiding this comment

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

Nice work. My remarks are mostly comments and worries.

  • Response Structure and Compatibility

    • Downstream clients expecting the previous token response format may break if they can’t handle the new authorization_details field.
    • Future changes to the AuthorizationDetailResponse structure could cause incompatibility
    • => Include comprehensive API changelogs and documentation updates & ensure strong versioning and feature toggling if needed.
  • Logical/Behavioral Regression

    • => Consider additional concurrency testing under load
  • Code Maintainability & Extensibility

  • Error Handling & Feedback

    • => Log internal context server-side, but give minimal info in the API response
  • Input Validation & Parsing

    • => Use strict Jackson settings (fail on unknown, max depth/size)
    • => Consider adding a maximum array/object depth or length to avoid DoS attacks 🤔

@Ogenbertrand Ogenbertrand self-requested a review June 20, 2025 09:47
@Ogenbertrand
Copy link
Collaborator

Ogenbertrand commented Jun 20, 2025

Permit me to continue with the review on monday, as my health doesn't allow me to continue with the review today.

Copy link
Collaborator

@Awambeng Awambeng left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the changes. I have two minor comments for you to take a look at

@Ogenbertrand
Copy link
Collaborator

Ogenbertrand commented Jun 26, 2025

I confirm that my changes has been resolve correctly, and i'll proceed by approving.
Thanks for the PR @forkimenjeckayang.

@IngridPuppet IngridPuppet force-pushed the main branch 2 times, most recently from bda0e2a to f99c912 Compare October 15, 2025 13:55
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.

Keycloak: Add support for parsing and understanding authorization_details at the Token Endpoint for OID4VCI

6 participants