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

fix(openid4vc): several fixes and improvements #1795

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export {
isDid,
asArray,
equalsIgnoreOrder,
DateTransformer,
} from './utils'
export * from './logger'
export * from './error'
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ export * from './objectEquality'
export * from './MessageValidator'
export * from './did'
export * from './array'
export { DateTransformer } from './transformers'
1 change: 1 addition & 0 deletions packages/core/src/utils/transformers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export function MetadataTransformer() {
*/
export function DateTransformer() {
return Transform(({ value, type }) => {
if (value === undefined) return undefined
TimoGlastra marked this conversation as resolved.
Show resolved Hide resolved
if (type === TransformationType.CLASS_TO_PLAIN) {
return value.toISOString()
}
Expand Down
4 changes: 2 additions & 2 deletions packages/openid4vc/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
"dependencies": {
"@credo-ts/core": "0.5.0",
"@sphereon/did-auth-siop": "0.6.2",
"@sphereon/oid4vci-client": "^0.10.1",
"@sphereon/oid4vci-client": "^0.10.2",
"@sphereon/oid4vci-common": "^0.10.1",
"@sphereon/oid4vci-issuer": "^0.10.1",
"@sphereon/oid4vci-issuer": "^0.10.2",
"@sphereon/ssi-types": "^0.18.1",
"rxjs": "^7.8.0"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@ export class OpenId4VcIssuerApi {
return this.openId4VcIssuerService.getAllIssuers(this.agentContext)
}

/**
* @deprecated use {@link getIssuerByIssuerId} instead.
* @todo remove in 0.6
*/
public async getByIssuerId(issuerId: string) {
return this.getIssuerByIssuerId(issuerId)
}

public async getIssuerByIssuerId(issuerId: string) {
return this.openId4VcIssuerService.getIssuerByIssuerId(this.agentContext, issuerId)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,19 @@ export class OpenId4VcIssuerService {
utils.uuid(),
])

const { uri } = await vcIssuer.createCredentialOfferURI({
let { uri } = await vcIssuer.createCredentialOfferURI({
grants: await this.getGrantsFromConfig(agentContext, preAuthorizedCodeFlowConfig),
credentials: offeredCredentials,
credentialOfferUri: hostedCredentialOfferUri,
baseUri: options.baseUri,
credentialDataSupplierInput: options.issuanceMetadata,
})

// FIXME: https://github.com/Sphereon-Opensource/OID4VCI/issues/102
if (uri.includes(hostedCredentialOfferUri)) {
uri = uri.replace(hostedCredentialOfferUri, encodeURIComponent(hostedCredentialOfferUri))
}

const issuanceSession = await this.openId4VcIssuanceSessionRepository.getSingleByQuery(agentContext, {
credentialOfferUri: hostedCredentialOfferUri,
})
Expand Down Expand Up @@ -158,6 +164,18 @@ export class OpenId4VcIssuerService {

const issuer = await this.getIssuerByIssuerId(agentContext, options.issuanceSession.issuerId)

const cNonce = getCNonceFromCredentialRequest(credentialRequest)
if (issuanceSession.cNonce !== cNonce) {
throw new CredoError('The cNonce in the credential request does not match the cNonce in the issuance session.')
}

if (!issuanceSession.cNonceExpiresAt) {
throw new CredoError('Missing required cNonceExpiresAt in the issuance session. Assuming cNonce is not valid')
}
if (Date.now() > issuanceSession.cNonceExpiresAt.getTime()) {
throw new CredoError('The cNonce has expired.')
}

const vcIssuer = this.getVcIssuer(agentContext, issuer)
const credentialResponse = await vcIssuer.issueCredential({
credentialRequest,
Expand All @@ -166,21 +184,31 @@ export class OpenId4VcIssuerService {
// This can just be combined with signing callback right?
credentialDataSupplier: this.getCredentialDataSupplier(agentContext, { ...options, issuer }),
credentialDataSupplierInput: issuanceSession.issuanceMetadata,
newCNonce: undefined,
responseCNonce: undefined,
})

const updatedIssuanceSession = await this.openId4VcIssuanceSessionRepository.getById(
agentContext,
issuanceSession.id
)

if (!credentialResponse.credential) {
throw new CredoError('No credential found in the issueCredentialResponse.')
updatedIssuanceSession.state = OpenId4VcIssuanceSessionState.Error
updatedIssuanceSession.errorMessage = 'No credential found in the issueCredentialResponse.'
await this.openId4VcIssuanceSessionRepository.update(agentContext, updatedIssuanceSession)
throw new CredoError(updatedIssuanceSession.errorMessage)
}

if (credentialResponse.acceptance_token) {
throw new CredoError('Acceptance token not yet supported.')
updatedIssuanceSession.state = OpenId4VcIssuanceSessionState.Error
updatedIssuanceSession.errorMessage = 'Acceptance token not yet supported.'
await this.openId4VcIssuanceSessionRepository.update(agentContext, updatedIssuanceSession)
throw new CredoError(updatedIssuanceSession.errorMessage)
}

return {
credentialResponse,
issuanceSession: await this.openId4VcIssuanceSessionRepository.getById(agentContext, issuanceSession.id),
issuanceSession: updatedIssuanceSession,
}
}

Expand Down Expand Up @@ -469,6 +497,7 @@ export class OpenId4VcIssuerService {
agentContext: AgentContext,
options: OpenId4VciCreateCredentialResponseOptions & {
issuer: OpenId4VcIssuerRecord
issuanceSession: OpenId4VcIssuanceSessionRecord
}
): CredentialDataSupplier => {
return async (args: CredentialDataSupplierArgs) => {
Expand Down Expand Up @@ -497,6 +526,7 @@ export class OpenId4VcIssuerService {
this.openId4VcIssuerConfig.credentialEndpoint.credentialRequestToCredentialMapper
const signOptions = await mapper({
agentContext,
issuanceSession: options.issuanceSession,
holderBinding,

credentialOffer,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { OpenId4VcIssuanceSessionRecord } from './repository'
import type {
OpenId4VcCredentialHolderBinding,
OpenId4VciCredentialOffer,
Expand Down Expand Up @@ -37,6 +38,14 @@ export interface OpenId4VciCreateCredentialOfferOptions {
baseUri?: string

preAuthorizedCodeFlowConfig: OpenId4VciPreAuthorizedCodeFlowConfig

/**
* Metadata about the issuance, that will be stored in the issuance session record and
* passed to the credential request to credential mapper. This can be used to e.g. store an
* user identifier so user data can be fetched in the credential mapper, or the actual credential
* data.
*/
issuanceMetadata?: Record<string, unknown>
TimoGlastra marked this conversation as resolved.
Show resolved Hide resolved
}

export interface OpenId4VciCreateCredentialResponseOptions {
Expand All @@ -60,6 +69,12 @@ export interface OpenId4VciCreateCredentialResponseOptions {
export type OpenId4VciCredentialRequestToCredentialMapper = (options: {
agentContext: AgentContext

/**
* The issuance session associated with the credential request. You can extract the
* issuance metadata from this record if passed in the offer creation method.
*/
issuanceSession: OpenId4VcIssuanceSessionRecord

/**
* The credential request received from the wallet
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import { agentDependencies } from '../../../../node/src'
import { OpenId4VciCredentialFormatProfile } from '../../shared'
import { OpenId4VcIssuanceSessionState } from '../OpenId4VcIssuanceSessionState'
import { OpenId4VcIssuerModule } from '../OpenId4VcIssuerModule'
import { OpenId4VcIssuerService } from '../OpenId4VcIssuerService'
import { OpenId4VcIssuanceSessionRepository } from '../repository'

const openBadgeCredential = {
Expand Down Expand Up @@ -288,12 +287,13 @@ describe('OpenId4VcIssuer', () => {

const issuanceSessionRepository = issuer.context.dependencyManager.resolve(OpenId4VcIssuanceSessionRepository)
result.issuanceSession.cNonce = '1234'
result.issuanceSession.cNonceExpiresAt = new Date(Date.now() + 30000) // 30 seconds
await issuanceSessionRepository.update(issuer.context, result.issuanceSession)

expect(result).toMatchObject({
credentialOffer: expect.stringMatching(
new RegExp(
`^openid-credential-offer://\\?credential_offer_uri=https://openid4vc-issuer.com/${openId4VcIssuer.issuerId}/offers/.*$`
`^openid-credential-offer://\\?credential_offer_uri=https%3A%2F%2Fopenid4vc-issuer.com%2F${openId4VcIssuer.issuerId}%2Foffers%2F.*$`
)
),
issuanceSession: {
Expand Down Expand Up @@ -346,7 +346,7 @@ describe('OpenId4VcIssuer', () => {

expect(credentialResponse).toEqual({
c_nonce: expect.any(String),
c_nonce_expires_in: 300000,
c_nonce_expires_in: 300,
credential: expect.any(String),
format: 'vc+sd-jwt',
})
Expand All @@ -368,11 +368,15 @@ describe('OpenId4VcIssuer', () => {
preAuthorizedCode,
userPinRequired: false,
},
issuanceMetadata: {
myIssuance: 'metadata',
},
})

const issuanceSessionRepository = issuer.context.dependencyManager.resolve(OpenId4VcIssuanceSessionRepository)
// We need to update the state, as it is checked and we're skipping the access token step
result.issuanceSession.cNonce = '1234'
result.issuanceSession.cNonceExpiresAt = new Date(Date.now() + 30000) // 30 seconds
result.issuanceSession.state = OpenId4VcIssuanceSessionState.AccessTokenCreated
await issuanceSessionRepository.update(issuer.context, result.issuanceSession)

Expand All @@ -381,16 +385,23 @@ describe('OpenId4VcIssuer', () => {
const issuerMetadata = await issuer.modules.openId4VcIssuer.getIssuerMetadata(openId4VcIssuer.issuerId)
const { credentialResponse } = await issuer.modules.openId4VcIssuer.createCredentialResponse({
issuanceSessionId: result.issuanceSession.id,
credentialRequestToCredentialMapper: () => ({
format: 'jwt_vc',
credential: new W3cCredential({
type: openBadgeCredential.types,
issuer: new W3cIssuer({ id: issuerDid }),
credentialSubject: new W3cCredentialSubject({ id: holderDid }),
issuanceDate: w3cDate(Date.now()),
}),
verificationMethod: issuerVerificationMethod.id,
}),
credentialRequestToCredentialMapper: ({ issuanceSession }) => {
expect(issuanceSession.id).toEqual(result.issuanceSession.id)
expect(issuanceSession.issuanceMetadata).toEqual({
myIssuance: 'metadata',
})

return {
format: 'jwt_vc',
credential: new W3cCredential({
type: openBadgeCredential.types,
issuer: new W3cIssuer({ id: issuerDid }),
credentialSubject: new W3cCredentialSubject({ id: holderDid }),
issuanceDate: w3cDate(Date.now()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to your PR, but looking at all the W3cXXX classes, it might be worth considering to make the W3cDate a class as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah maybe, or we can make it a date instance and do the transformation for the user

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to address this in a separate PR, as it will require quite some changes across the codebase 😊

}),
verificationMethod: issuerVerificationMethod.id,
}
},
credentialRequest: await createCredentialRequest(holder.context, {
credentialSupported: openBadgeCredential,
issuerMetadata,
Expand All @@ -401,7 +412,7 @@ describe('OpenId4VcIssuer', () => {

expect(credentialResponse).toEqual({
c_nonce: expect.any(String),
c_nonce_expires_in: 300000,
c_nonce_expires_in: 300,
credential: expect.any(String),
format: 'jwt_vc_json',
})
Expand Down Expand Up @@ -444,6 +455,7 @@ describe('OpenId4VcIssuer', () => {
// We need to update the state, as it is checked and we're skipping the access token step
result.issuanceSession.state = OpenId4VcIssuanceSessionState.AccessTokenCreated
result.issuanceSession.cNonce = '1234'
result.issuanceSession.cNonceExpiresAt = new Date(Date.now() + 30000) // 30 seconds
await issuanceSessionRepository.update(issuer.context, result.issuanceSession)

const issuerMetadata = await issuer.modules.openId4VcIssuer.getIssuerMetadata(openId4VcIssuer.issuerId)
Expand Down Expand Up @@ -478,6 +490,7 @@ describe('OpenId4VcIssuer', () => {
const issuanceSessionRepository = issuer.context.dependencyManager.resolve(OpenId4VcIssuanceSessionRepository)
// We need to update the state, as it is checked and we're skipping the access token step
result.issuanceSession.cNonce = '1234'
result.issuanceSession.cNonceExpiresAt = new Date(Date.now() + 30000) // 30 seconds
result.issuanceSession.state = OpenId4VcIssuanceSessionState.AccessTokenCreated
await issuanceSessionRepository.update(issuer.context, result.issuanceSession)

Expand All @@ -504,7 +517,7 @@ describe('OpenId4VcIssuer', () => {

expect(credentialResponse).toEqual({
c_nonce: expect.any(String),
c_nonce_expires_in: 300000,
c_nonce_expires_in: 300,
credential: expect.any(String),
format: 'jwt_vc_json-ld',
})
Expand Down Expand Up @@ -532,6 +545,7 @@ describe('OpenId4VcIssuer', () => {
// We need to update the state, as it is checked and we're skipping the access token step
result.issuanceSession.state = OpenId4VcIssuanceSessionState.AccessTokenCreated
result.issuanceSession.cNonce = '1234'
result.issuanceSession.cNonceExpiresAt = new Date(Date.now() + 30000) // 30 seconds
await issuanceSessionRepository.update(issuer.context, result.issuanceSession)

const issuerMetadata = await issuer.modules.openId4VcIssuer.getIssuerMetadata(openId4VcIssuer.issuerId)
Expand Down Expand Up @@ -569,7 +583,7 @@ describe('OpenId4VcIssuer', () => {

expect(credentialOffer).toMatch(
new RegExp(
`^openid-credential-offer://\\?credential_offer_uri=https://openid4vc-issuer.com/${openId4VcIssuer.issuerId}/offers/.*$`
`^openid-credential-offer://\\?credential_offer_uri=https%3A%2F%2Fopenid4vc-issuer.com%2F${openId4VcIssuer.issuerId}%2Foffers%2F.*$`
)
)
})
Expand All @@ -588,6 +602,7 @@ describe('OpenId4VcIssuer', () => {

const issuanceSessionRepository = issuer.context.dependencyManager.resolve(OpenId4VcIssuanceSessionRepository)
result.issuanceSession.cNonce = '1234'
result.issuanceSession.cNonceExpiresAt = new Date(Date.now() + 30000) // 30 seconds
await issuanceSessionRepository.update(issuer.context, result.issuanceSession)

expect(result.issuanceSession.credentialOfferPayload?.credentials).toEqual([
Expand Down Expand Up @@ -629,7 +644,7 @@ describe('OpenId4VcIssuer', () => {

expect(credentialResponse).toEqual({
c_nonce: expect.any(String),
c_nonce_expires_in: 300000,
c_nonce_expires_in: 300,
credential: expect.any(String),
format: 'jwt_vc_json',
})
Expand All @@ -653,7 +668,7 @@ describe('OpenId4VcIssuer', () => {

expect(credentialResponse2).toEqual({
c_nonce: expect.any(String),
c_nonce_expires_in: 300000,
c_nonce_expires_in: 300,
credential: expect.any(String),
format: 'jwt_vc_json',
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@ import type { CNonceState, IStateManager } from '@sphereon/oid4vci-common'

import { CredoError } from '@credo-ts/core'

import { OpenId4VcIssuerModuleConfig } from '../OpenId4VcIssuerModuleConfig'

import { OpenId4VcIssuanceSessionRepository } from './OpenId4VcIssuanceSessionRepository'

export class OpenId4VcCNonceStateManager implements IStateManager<CNonceState> {
private openId4VcIssuanceSessionRepository: OpenId4VcIssuanceSessionRepository
private openId4VcIssuerModuleConfig: OpenId4VcIssuerModuleConfig

public constructor(private agentContext: AgentContext, private issuerId: string) {
this.openId4VcIssuanceSessionRepository = agentContext.dependencyManager.resolve(OpenId4VcIssuanceSessionRepository)
this.openId4VcIssuerModuleConfig = agentContext.dependencyManager.resolve(OpenId4VcIssuerModuleConfig)
}

public async set(cNonce: string, stateValue: CNonceState): Promise<void> {
Expand All @@ -29,7 +33,17 @@ export class OpenId4VcCNonceStateManager implements IStateManager<CNonceState> {
preAuthorizedCode: stateValue.preAuthorizedCode,
})

// cNonce already matches, no need to update
if (record.cNonce === stateValue.cNonce) {
return
}

const expiresAtDate = new Date(
Date.now() + this.openId4VcIssuerModuleConfig.accessTokenEndpoint.cNonceExpiresInSeconds * 1000
)

record.cNonce = stateValue.cNonce
record.cNonceExpiresAt = expiresAtDate
await this.openId4VcIssuanceSessionRepository.update(this.agentContext, record)
}

Expand Down Expand Up @@ -74,6 +88,7 @@ export class OpenId4VcCNonceStateManager implements IStateManager<CNonceState> {
// We only remove the cNonce from the record, we don't want to remove
// the whole issuance session.
record.cNonce = undefined
record.cNonceExpiresAt = undefined
await this.openId4VcIssuanceSessionRepository.update(this.agentContext, record)
return true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export class OpenId4VcCredentialOfferSessionStateManager implements IStateManage
const state = await this.get(preAuthorizedCode)

if (!state) {
throw new CredoError(`No cNonce state found for id ${preAuthorizedCode}`)
throw new CredoError(`No credential offer state found for id ${preAuthorizedCode}`)
}

return state
Expand Down
Loading
Loading