-
Notifications
You must be signed in to change notification settings - Fork 204
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
feat: dpop support #1966
feat: dpop support #1966
Changes from all commits
a3fe108
88e2ee5
8765c0b
cb3e749
c49466b
c599fa9
7a656c8
f250d57
7b13e97
e55bb2f
004eb51
bc92691
7273ca1
f2d73af
33ed706
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
"@credo-ts/core": patch | ||
"@credo-ts/openid4vc": patch | ||
--- | ||
|
||
Add support for Demonstrating Proof of Possesion (DPoP) when receiving credentials using OpenID4VCI |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,72 +1,81 @@ | ||
import type { | ||
OpenId4VciAcceptCredentialOfferOptions, | ||
OpenId4VciAuthCodeFlowOptions, | ||
OpenId4VciProofOfPossessionRequirements, | ||
OpenId4VciCredentialBindingResolver, | ||
OpenId4VciResolvedCredentialOffer, | ||
OpenId4VciCredentialResponse, | ||
OpenId4VciNotificationEvent, | ||
OpenId4VciProofOfPossessionRequirements, | ||
OpenId4VciResolvedAuthorizationRequest, | ||
OpenId4VciResolvedAuthorizationRequestWithCode, | ||
OpenId4VciResolvedCredentialOffer, | ||
OpenId4VciSupportedCredentialFormats, | ||
OpenId4VciCredentialResponse, | ||
OpenId4VciNotificationEvent, | ||
OpenId4VciAcceptCredentialOfferOptions, | ||
OpenId4VciTokenRequestOptions, | ||
} from './OpenId4VciHolderServiceOptions' | ||
import type { | ||
OpenId4VciCredentialConfigurationSupported, | ||
OpenId4VciCredentialSupported, | ||
OpenId4VciIssuerMetadata, | ||
} from '../shared' | ||
import type { AgentContext, JwaSignatureAlgorithm, Key, JwkJson } from '@credo-ts/core' | ||
import type { AgentContext, JwaSignatureAlgorithm, JwkJson, Key } from '@credo-ts/core' | ||
import type { | ||
AccessTokenResponse, | ||
CredentialResponse, | ||
Jwt, | ||
OpenIDResponse, | ||
AuthorizationDetails, | ||
AuthorizationDetailsJwtVcJson, | ||
AuthorizationDetailsJwtVcJsonLdAndLdpVc, | ||
AuthorizationDetailsSdJwtVc, | ||
CredentialResponse, | ||
Jwt, | ||
OpenIDResponse, | ||
} from '@sphereon/oid4vci-common' | ||
|
||
import { | ||
SdJwtVcApi, | ||
getJwkFromJson, | ||
DidsApi, | ||
CredoError, | ||
DidsApi, | ||
Hasher, | ||
InjectionSymbols, | ||
JsonEncoder, | ||
Jwk, | ||
JwsService, | ||
Logger, | ||
SdJwtVcApi, | ||
SignatureSuiteRegistry, | ||
TypedArrayEncoder, | ||
W3cCredentialService, | ||
W3cJsonLdVerifiableCredential, | ||
W3cJwtVerifiableCredential, | ||
getJwkClassFromJwaSignatureAlgorithm, | ||
getJwkFromJson, | ||
getJwkFromKey, | ||
getKeyFromVerificationMethod, | ||
getSupportedVerificationMethodTypesFromKeyType, | ||
inject, | ||
injectable, | ||
parseDid, | ||
} from '@credo-ts/core' | ||
import { CreateDPoPClientOpts, CreateDPoPJwtPayloadProps, SigningAlgo } from '@sphereon/oid4vc-common' | ||
import { | ||
AccessTokenClient, | ||
CredentialRequestClientBuilder, | ||
ProofOfPossessionBuilder, | ||
OpenID4VCIClient, | ||
OpenID4VCIClientStateV1_0_13, | ||
OpenID4VCIClientV1_0_11, | ||
OpenID4VCIClientV1_0_13, | ||
OpenID4VCIClientStateV1_0_13, | ||
ProofOfPossessionBuilder, | ||
} from '@sphereon/oid4vci-client' | ||
import { CodeChallengeMethod, OpenId4VCIVersion, PARMode, post } from '@sphereon/oid4vci-common' | ||
import { | ||
CodeChallengeMethod, | ||
DPoPResponseParams, | ||
EndpointMetadataResult, | ||
OpenId4VCIVersion, | ||
PARMode, | ||
post, | ||
} from '@sphereon/oid4vci-common' | ||
|
||
import { OpenId4VciCredentialFormatProfile } from '../shared' | ||
import { getTypesFromCredentialSupported, getOfferedCredentials } from '../shared/issuerMetadataUtils' | ||
import { getSupportedJwaSignatureAlgorithms, isCredentialOfferV1Draft13 } from '../shared/utils' | ||
import { getOfferedCredentials, getTypesFromCredentialSupported } from '../shared/issuerMetadataUtils' | ||
import { getCreateJwtCallback, getSupportedJwaSignatureAlgorithms, isCredentialOfferV1Draft13 } from '../shared/utils' | ||
|
||
import { openId4VciSupportedCredentialFormats, OpenId4VciNotificationMetadata } from './OpenId4VciHolderServiceOptions' | ||
import { OpenId4VciNotificationMetadata, openId4VciSupportedCredentialFormats } from './OpenId4VciHolderServiceOptions' | ||
|
||
@injectable() | ||
export class OpenId4VciHolderService { | ||
|
@@ -260,14 +269,69 @@ export class OpenId4VciHolderService { | |
} | ||
} | ||
|
||
private async getCreateDpopOptions( | ||
agentContext: AgentContext, | ||
metadata: Pick<EndpointMetadataResult, 'authorizationServerMetadata'> & { | ||
credentialIssuerMetadata: OpenId4VciIssuerMetadata | ||
}, | ||
resourceRequestOptions?: { | ||
jwk: Jwk | ||
jwtPayloadProps: Omit<CreateDPoPJwtPayloadProps, 'htu' | 'htm'> | ||
} | ||
) { | ||
const dpopSigningAlgValuesSupported = | ||
metadata.authorizationServerMetadata?.dpop_signing_alg_values_supported ?? | ||
metadata.credentialIssuerMetadata.dpop_signing_alg_values_supported | ||
|
||
if (!dpopSigningAlgValuesSupported) return undefined | ||
|
||
const alg = dpopSigningAlgValuesSupported.find((alg) => getJwkClassFromJwaSignatureAlgorithm(alg)) | ||
|
||
let jwk: Jwk | ||
if (resourceRequestOptions) { | ||
jwk = resourceRequestOptions.jwk | ||
} else { | ||
const JwkClass = alg ? getJwkClassFromJwaSignatureAlgorithm(alg) : undefined | ||
|
||
if (!JwkClass) { | ||
throw new CredoError( | ||
`No supported dpop signature algorithms found in dpop_signing_alg_values_supported '${dpopSigningAlgValuesSupported.join( | ||
', ' | ||
)}'` | ||
) | ||
} | ||
|
||
const key = await agentContext.wallet.createKey({ keyType: JwkClass.keyType }) | ||
jwk = getJwkFromKey(key) | ||
} | ||
|
||
const createDPoPOpts: CreateDPoPClientOpts = { | ||
jwtIssuer: { alg: alg as unknown as SigningAlgo, jwk: jwk.toJson() }, | ||
dPoPSigningAlgValuesSupported: dpopSigningAlgValuesSupported, | ||
jwtPayloadProps: resourceRequestOptions?.jwtPayloadProps ?? {}, | ||
createJwtCallback: getCreateJwtCallback(agentContext), | ||
} | ||
return createDPoPOpts | ||
} | ||
|
||
public async requestAccessToken(agentContext: AgentContext, options: OpenId4VciTokenRequestOptions) { | ||
const { resolvedCredentialOffer, txCode, resolvedAuthorizationRequest, code } = options | ||
const { metadata, credentialOfferRequestWithBaseUrl } = resolvedCredentialOffer | ||
|
||
// acquire the access token | ||
let accessTokenResponse: OpenIDResponse<AccessTokenResponse> | ||
let accessTokenResponse: OpenIDResponse<AccessTokenResponse, DPoPResponseParams> | ||
|
||
const accessTokenClient = new AccessTokenClient() | ||
|
||
const createDPoPOpts = await this.getCreateDpopOptions(agentContext, metadata) | ||
|
||
let dpopJwk: Jwk | undefined | ||
if (createDPoPOpts) { | ||
if (!createDPoPOpts.jwtIssuer.jwk.kty) { | ||
throw new CredoError('Missing required key type (kty) in the jwk.') | ||
} | ||
Comment on lines
+330
to
+332
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this needed? In theory the jwk should have this property, so I'm not sure why we check for this specific property |
||
dpopJwk = getJwkFromJson(createDPoPOpts.jwtIssuer.jwk as JwkJson) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why first transform from JWK class to json and then back to jwk, shouldn't the createDpopOptions just return the instance? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm. I don't think so because we create the options which are passed to sphereons library and there we don't have the jwk class |
||
} | ||
if (resolvedAuthorizationRequest) { | ||
const { codeVerifier, redirectUri } = resolvedAuthorizationRequest | ||
accessTokenResponse = await accessTokenClient.acquireAccessToken({ | ||
|
@@ -277,12 +341,14 @@ export class OpenId4VciHolderService { | |
code, | ||
codeVerifier, | ||
redirectUri, | ||
createDPoPOpts, | ||
}) | ||
} else { | ||
accessTokenResponse = await accessTokenClient.acquireAccessToken({ | ||
metadata: metadata, | ||
credentialOffer: { credential_offer: credentialOfferRequestWithBaseUrl.credential_offer }, | ||
pin: txCode, | ||
createDPoPOpts, | ||
}) | ||
} | ||
|
||
|
@@ -294,7 +360,10 @@ export class OpenId4VciHolderService { | |
|
||
this.logger.debug('Requested OpenId4VCI Access Token.') | ||
|
||
return accessTokenResponse.successBody | ||
return { | ||
...accessTokenResponse.successBody, | ||
...(dpopJwk && { dpop: { jwk: dpopJwk, nonce: accessTokenResponse.params?.dpop?.dpopNonce } }), | ||
} | ||
} | ||
|
||
public async acceptCredentialOffer( | ||
|
@@ -305,6 +374,7 @@ export class OpenId4VciHolderService { | |
resolvedAuthorizationRequestWithCode?: OpenId4VciResolvedAuthorizationRequestWithCode | ||
accessToken?: string | ||
cNonce?: string | ||
dpop?: { jwk: Jwk; nonce?: string } | ||
clientId?: string | ||
} | ||
) { | ||
|
@@ -318,7 +388,9 @@ export class OpenId4VciHolderService { | |
return [] | ||
} | ||
|
||
this.logger.info(`Accepting the following credential offers '${credentialsToRequest}'`) | ||
this.logger.info( | ||
`Accepting the following credential offers '${credentialsToRequest ? credentialsToRequest.join(', ') : 'all'}` | ||
) | ||
|
||
const supportedJwaSignatureAlgorithms = getSupportedJwaSignatureAlgorithms(agentContext) | ||
|
||
|
@@ -345,7 +417,11 @@ export class OpenId4VciHolderService { | |
} as OpenId4VciTokenRequestOptions | ||
|
||
const tokenResponse = options.accessToken | ||
? { access_token: options.accessToken, c_nonce: options.cNonce } | ||
? { | ||
access_token: options.accessToken, | ||
c_nonce: options.cNonce, | ||
dpop: options.dpop, | ||
} | ||
: await this.requestAccessToken(agentContext, tokenRequestOptions) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the service we can already change it (only api we need to worry about breaking changes) |
||
|
||
const receivedCredentials: Array<OpenId4VciCredentialResponse> = [] | ||
|
@@ -413,10 +489,19 @@ export class OpenId4VciHolderService { | |
.withToken(tokenResponse.access_token) | ||
|
||
const credentialRequestClient = credentialRequestBuilder.build() | ||
|
||
const createDpopOpts = tokenResponse.dpop | ||
? await this.getCreateDpopOptions(agentContext, metadata, { | ||
jwk: tokenResponse.dpop.jwk, | ||
jwtPayloadProps: { accessToken: tokenResponse.access_token, nonce: tokenResponse.dpop?.nonce }, | ||
}) | ||
: undefined | ||
|
||
const credentialResponse = await credentialRequestClient.acquireCredentialsUsingProof({ | ||
proofInput: proofOfPossession, | ||
credentialTypes: getTypesFromCredentialSupported(offeredCredentialConfiguration), | ||
format: offeredCredentialConfiguration.format, | ||
createDPoPOpts: createDpopOpts, | ||
}) | ||
|
||
newCNonce = credentialResponse.successBody?.c_nonce | ||
|
@@ -611,7 +696,7 @@ export class OpenId4VciHolderService { | |
|
||
private async handleCredentialResponse( | ||
agentContext: AgentContext, | ||
credentialResponse: OpenIDResponse<CredentialResponse>, | ||
credentialResponse: OpenIDResponse<CredentialResponse, DPoPResponseParams>, | ||
options: { | ||
verifyCredentialStatus: boolean | ||
credentialIssuerMetadata: OpenId4VciIssuerMetadata | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can change the service interface just fine, so let's change the service return value to:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
? what should the response contain? the whole response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it would be a start at solving #1965, which was requested. Returning the response means better extensibility as you can extract the headers, etc..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the PR where I implemented the tokenRequestfunctionality, I had that behaviour, and you made me change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay then i misunderstood the previous implementation. What i don't want is requiring the whole access token response as input to the request credentials method.
I want to have separation between the required fields for credo to work, and additional params you may need for extension. And as I said in the previous PR, i'm not a fan of mixing the successBody with extra params we extracted from e.g. the header.