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: key id for jwt and sdjwt #1420

Merged
merged 11 commits into from
Oct 31, 2024
Merged

fix: key id for jwt and sdjwt #1420

merged 11 commits into from
Oct 31, 2024

Conversation

mineme0110
Copy link
Contributor

@mineme0110 mineme0110 commented Oct 28, 2024

Description:

The key ID (kid) should be used to select the correct key when multiple keys with the same purpose, such as ED25519 and SECP256K1, are available for JWT issuance. For SDJWT, only ED25519 is supported for now. This change will enable the agent to issue JWTs in both formats, with the header containing the appropriate kid value

Checklist:

  • My PR follows the contribution guidelines of this project
  • My PR is free of third-party dependencies that don't comply with the Allowlist
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked the PR title to follow the conventional commit specification

@mineme0110 mineme0110 requested a review from a team as a code owner October 28, 2024 14:53
@mineme0110 mineme0110 changed the title fix:key id for jwt and sdjwt fix: key id for jwt and sdjwt Oct 28, 2024
Copy link
Contributor

github-actions bot commented Oct 28, 2024

Unit Test Results

103 files  ±0  103 suites  ±0   19m 52s ⏱️ -18s
877 tests +1  869 ✅ +1  8 💤 ±0  0 ❌ ±0 
884 runs  +1  876 ✅ +1  8 💤 ±0  0 ❌ ±0 

Results for commit e0ad6ae. ± Comparison against base commit 228f07b.

♻️ This comment has been updated with latest results.

@coveralls
Copy link

coveralls commented Oct 28, 2024

Coverage Status

coverage: 48.539% (+0.1%) from 48.438%
when pulling e0ad6ae on feat/keyid-jwt-sdjwt
into 228f07b on main.

@@ -120,7 +120,13 @@ object CredentialServiceError {
StatusCode.NotFound,
s"A key with the given purpose was not found in the DID: did=${did.toString}, purpose=${verificationRelationship.name}"
)

final case class MultipleKeysWithSamePurposeFoundInDID(
Copy link
Member

Choose a reason for hiding this comment

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

@mineme0110, is it possible to keep two keys (secp256 and de25519) with the same purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is possible to have multiple keys with same purpose as per the spec

Copy link
Member

@yshyn-iohk yshyn-iohk left a comment

Choose a reason for hiding this comment

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

Good job, @mineme0110!

mineme0110 and others added 7 commits October 30, 2024 10:12
Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: Hyperledger Bot <[email protected]>
Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: Hyperledger Bot <[email protected]>
Copy link
Contributor

github-actions bot commented Oct 30, 2024

Integration Test Results

20 files  ±0  20 suites  ±0   3s ⏱️ ±0s
53 tests +1  53 ✅ +1  0 💤 ±0  0 ❌ ±0 
94 runs  +1  94 ✅ +1  0 💤 ±0  0 ❌ ±0 

Results for commit e0ad6ae. ± Comparison against base commit 228f07b.

♻️ This comment has been updated with latest results.

Signed-off-by: mineme0110 <[email protected]>

scalafmt

Signed-off-by: mineme0110 <[email protected]>

Updated unit test to support kid

Signed-off-by: mineme0110 <[email protected]>

OIDC feature doesnt have kid support in JWT

Signed-off-by: mineme0110 <[email protected]>

style: apply linters automatic fixes (#1425)

Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: Hyperledger Bot <[email protected]>

Add new keyid test for JWT with ED25519 signed credential

Signed-off-by: mineme0110 <[email protected]>
@mineme0110 mineme0110 merged commit 5830a7e into main Oct 31, 2024
14 checks passed
@mineme0110 mineme0110 deleted the feat/keyid-jwt-sdjwt branch October 31, 2024 11:41
bytewizard42i pushed a commit to bytewizard42i/identus-cloud-agent that referenced this pull request Nov 13, 2024
# [1.40.0](hyperledger/identus-cloud-agent@cloud-agent-v1.39.0...cloud-agent-v1.40.0) (2024-11-05)

### Bug Fixes

* Add key_id missing field ([hyperledger#1403](hyperledger#1403)) ([cbd1a03](hyperledger@cbd1a03))
* adjust Kotlin and TypeScript HTTP client to use the `schemaId` f… ([hyperledger#1388](hyperledger#1388)) ([c2da492](hyperledger@c2da492))
* cannot reuse the same credential-offer in oid4vci ([hyperledger#1361](hyperledger#1361)) ([6a0a3ea](hyperledger@6a0a3ea))
* handle unsupported PIURI found in DIDComm messages accordingly ([hyperledger#1399](hyperledger#1399)) ([9b64793](hyperledger@9b64793))
* key id for jwt and sdjwt ([hyperledger#1420](hyperledger#1420)) ([5830a7e](hyperledger@5830a7e))
* oas to use any schema for json ast node ([hyperledger#1372](hyperledger#1372)) ([95d328e](hyperledger@95d328e))
* oid4vci endpoints error statuses and negative input validation ([hyperledger#1384](hyperledger#1384)) ([65cc9a7](hyperledger@65cc9a7))
* Preserve Presentation Format ([hyperledger#1363](hyperledger#1363)) ([c18385c](hyperledger@c18385c))
* return 404 when create credConfig on non-existing issuer ([hyperledger#1379](hyperledger#1379)) ([e532ba6](hyperledger@e532ba6))

### Features

* Add KID to the credential-offers API - ATL-7704 ([hyperledger#1320](hyperledger#1320)) ([56200cf](hyperledger@56200cf))
* add presentation-exchange endpoints ([hyperledger#1365](hyperledger#1365)) ([49f7ab3](hyperledger@49f7ab3))
* ATL-6983 ZIO Stream Kafka PoC in background jobs ([hyperledger#1339](hyperledger#1339)) ([19ab426](hyperledger@19ab426))
* Default Backend API to Array Of Credential Schema ([hyperledger#1366](hyperledger#1366)) ([693dcc4](hyperledger@693dcc4))
* Default Object As Issuer ([hyperledger#1349](hyperledger#1349)) ([d29eebb](hyperledger@d29eebb))
* Implement prism anoncreds method for schemas and credential definitions ([hyperledger#1385](hyperledger#1385)) ([fbee055](hyperledger@fbee055))
* Issuer Replace Either By Union Type ([hyperledger#1374](hyperledger#1374)) ([8fc2fe3](hyperledger@8fc2fe3))
* presentation_submission validation logic ([hyperledger#1332](hyperledger#1332)) ([f80b3c3](hyperledger@f80b3c3))
* Support Array Of Credential Schema ([hyperledger#1351](hyperledger#1351)) ([948e314](hyperledger@948e314))
* Test JWT OBJECT as Issuer ([hyperledger#1343](hyperledger#1343)) ([7208d95](hyperledger@7208d95))
* VC support for Array of credential Status ([hyperledger#1383](hyperledger#1383)) ([ad946cf](hyperledger@ad946cf))
* VCVerification API support ARRAY or OBJECT as Credential Sc… ([hyperledger#1355](hyperledger#1355)) ([91cb4e7](hyperledger@91cb4e7))

 [skip ci]

Signed-off-by: Hyperledger Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants