Include txCode in the OpenID4VCIClient

[cre8]

When parsing the uri, it would help to get the information from txCode to inform the user he needs to provide some values.

@nklomp I think it was included here: https://github.com/Sphereon-Opensource/OID4VCI/blob/40ad1ded998168e2703daeddc3a3afc78c9b19a1/packages/client/lib/CredentialOfferClient.ts#L76

Any reasons to not include it again? :)

[cre8]

It seems to be available via client.credentialOffer.credential_offer.grants?.[ GrantTypes.PRE_AUTHORIZED_CODE ]?.tx_code,

But with the outcomment txCode the CredentialOfferRequestWithBaseUrl can lead to confusion because there the txCode is marked as optional, but it's value is never set

[nklomp]

Yeah I am working on it. It needed way more changes then initially estimates. Should have it fixed tonight

[nklomp]

Fixed in v0.14.0

[cre8]

@nklomp I think it is not fixed :( When I am printing our the credental_offer and the request I get this:

{
  grants: {
    'urn:ietf:params:oauth:grant-type:pre-authorized_code': {
      'pre-authorized_code': '2e08465f-d04a-4d64-a75c-f8d8c6cfc154',
      tx_code: [Object]
    }
  },
  credential_configuration_ids: [ 'Identity' ],
  credential_issuer: 'http://localhost:3001'
}
{
  user_pin: '155195',
  grant_type: 'urn:ietf:params:oauth:grant-type:pre-authorized_code',
  'pre-authorized_code': '2e08465f-d04a-4d64-a75c-f8d8c6cfc154'
}

I am confused why now the user_pin is set since this was not before in the version when using v13

[cre8]

It seems the request is build incorrect, see: https://github.com/Sphereon-Opensource/OID4VC/blob/develop/packages/client/lib/AccessTokenClient.ts#L107

Any reason to set it to user_pin or was it maybe a merge conflict? :)

[cre8]

It seems that in the sessionsManager the value is still set as userPin, while the interface is designed to use txCode:

    const session: CredentialOfferSession = {
      preAuthorizedCode,
      issuerState,
      createdAt,
      lastUpdatedAt,
      status,
      notification_id: v4(),
      ...(userPin && { userPin }),
      ...(opts.credentialDataSupplierInput && { credentialDataSupplierInput: opts.credentialDataSupplierInput }),
      credentialOffer,
    }

I don't feel good to use userPin and txCode in the code, I suggest it should be refactored to only use txCode.

The small fix would be to update the check to:

if (request.tx_code !== credentialOfferSession.userPin) {

when we want to treat the object still as userPin for internal management

[nklomp]

No we cannot change to only use txCode in this version. That will be part of the next refactor, where we drop support for everything older. This would break any wallet interacting with older issuers

Met vriendelijke groet,

Niels Klomp

CTO

[cid:bc2925bb-1e2e-423b-91ca-431f31493eba]

J.M. Keynesplein 10 1066 EP Amsterdam

+31 (0)852 736513

Op woensdagmiddag niet aanwezig

@.?anonymous&ep=signature> Maak een afspraak voor een gesprek met @.?anonymous&ep=signature>

---

Van: Mirko Mollik @.> Verzonden: dinsdag 16 juli 2024 11:50 Aan: Sphereon-Opensource/OID4VC @.> CC: Niels Klomp @.>; Mention @.> Onderwerp: Re: [Sphereon-Opensource/OID4VC] Include txCode in the OpenID4VCIClient (Issue #117)

It seems that in the sessionsManager the value is still set as userPin, while the interface is designed to use txCode:

const session: CredentialOfferSession = {
  preAuthorizedCode,
  issuerState,
  createdAt,
  lastUpdatedAt,
  status,
  notification_id: v4(),
  ...(userPin && { userPin }),
  ...(opts.credentialDataSupplierInput && { credentialDataSupplierInput: opts.credentialDataSupplierInput }),
  credentialOffer,
}

I don't feel good to use userPin and txCode in the code, I suggest it should be refactored to only use txCode.

The small fix would be to update the check to:

if (request.tx_code !== credentialOfferSession.userPin) {

when we want to treat the object still as userPin for internal management

— Reply to this email directly, view it on GitHubhttps://github.com/Sphereon-Opensource/OID4VC/issues/117#issuecomment-2230480434, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADH4BSE72ZBFIERNN3FJIL3ZMTUEXAVCNFSM6AAAAABJ4TUWZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZQGQ4DANBTGQ. You are receiving this because you were mentioned.Message ID: @.***>

[cre8]

Okay, can we at least then patch the comparison line? Right now the function will fail all the time. I can open a PR for this (it will not break the old approaches)

[nklomp]

Sue, but it will need another set of eyes, to see whether we didn't miss other areas