Skip to content

Commit

Permalink
fix: txCode fixes #117
Browse files Browse the repository at this point in the history
  • Loading branch information
@nklomp
nklomp committed Jul 23, 2024
1 parent 41a5983 commit 7d17d13
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
2 changes: 1 addition & 1 deletion packages/issuer-rest/lib/OID4VCIServer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function buildVCIFromEnvironment<DIDDoc extends object>() {
export type ICreateCredentialOfferURIResponse = {
uri: string
userPin?: string
tsCode?: TxCode
txCode?: TxCode
}

export interface IGetCredentialOfferEndpointOpts extends ISingleEndpointOpts {
Expand Down
7 changes: 6 additions & 1 deletion packages/issuer/lib/VcIssuer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ export class VcIssuer<DIDDoc extends object> {
if (grants?.[PRE_AUTH_GRANT_LITERAL]) {
preAuthorizedCode = grants?.[PRE_AUTH_GRANT_LITERAL]?.[PRE_AUTH_CODE_LITERAL]
txCode = grants?.[PRE_AUTH_GRANT_LITERAL]?.tx_code

if (txCode !== undefined) {
if (!txCode?.length) {
txCode.length = opts.pinLength ?? 4
}
grants[PRE_AUTH_GRANT_LITERAL].tx_code = txCode
}
if (!preAuthorizedCode) {
Expand Down Expand Up @@ -187,7 +191,8 @@ export class VcIssuer<DIDDoc extends object> {
lastUpdatedAt,
status,
notification_id: v4(),
...(userPin && { userPin }),
...(userPin && { txCode: userPin }), // We used to use userPin according to older specs. We map these onto txCode now. If both are used, txCode in the end wins, even if they are different
...(txCode && { txCode }),

This comment has been minimized.

Sign in to view

Copy link
@cre8

cre8 Jul 25, 2024 •
edited
Loading

Contributor

@nklomp This line has to be removed since it will overwrite the value of the userPin in the line above.
We do not need the save txCode on the low level since it's value is in the grants and therefore stored in the object

This comment has been minimized.

Sign in to view

Copy link
@nklomp

nklomp Jul 25, 2024

Author Contributor

Not sure why. Both are only setting the txCode in the end. If no userPin is passed in and only a txCode it is set. If a userPin is sent in a txCode is set. If both are set, then the txCode wins, like the comment explains. Reason why both are there is for backwards compat with existing installations and versions

This comment has been minimized.

Sign in to view

Copy link
@cre8

cre8 Jul 25, 2024

Contributor

I think this is not correct:
when I go with the latest version (so with txcode and not userPin) this if statement gets triggered and sets the userPin: https://github.com/Sphereon-Opensource/OID4VC/blob/develop/packages/issuer/lib/VcIssuer.ts#L155

since userPin is now set, line 194 gets added: https://github.com/Sphereon-Opensource/OID4VC/blob/develop/packages/issuer/lib/VcIssuer.ts#L194
and because of txCode is set, it will get owerwritten in line 195.
So txCode has the value of the previous txCode of type TxCode with the object, not the generated userPin of type string.

And txCode got set in the if statement in line 126 because I defined it in the grant

So only when I patch the vcissuer by removing the overwrite, the pin validation will succeed: https://github.com/openwallet-foundation-labs/credhub/pull/89/files#diff-687dfa55346baf2a8fa8116e93f246b8696d9893f8d2ad075c529950fd85054d

I am adding the txCode here into the grant type: https://github.com/openwallet-foundation-labs/credhub/blob/main/apps/issuer-backend/src/app/issuer/issuer.service.ts#L128

I would unterstand the overwrite in case the userPin is included in the TxCode object:

OID4VC/packages/issuer-rest/README.md

Line 146 in 006d953

export interface TxCode {

...(opts.credentialDataSupplierInput && { credentialDataSupplierInput: opts.credentialDataSupplierInput }),
credentialOffer,
}
Expand Down
2 changes: 1 addition & 1 deletion packages/issuer/lib/functions/CredentialOfferUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export function createCredentialOfferObjectv1_0_11(
if (opts?.preAuthorizedCode) {
credential_offer.grants[PRE_AUTH_GRANT_LITERAL] = {
'pre-authorized_code': opts.preAuthorizedCode,
user_pin_required: opts.userPinRequired ? opts.userPinRequired : false,
user_pin_required: opts.userPinRequired ?? false,
}
} else if (!credential_offer.grants?.authorization_code?.issuer_state) {
credential_offer.grants = {
Expand Down

0 comments on commit 7d17d13

Please sign in to comment.