Feat/verified identities - #102
Conversation
Add IdentityProofSigner in ans-sdk-crypto. It builds compact JWS control proofs that bind a verified identity to an agent key. Add the nimbus-jose-jwt dependency and unit tests. Signed-off-by: James Hateley <jhateley@godaddy.com>
Add IdentityService and IdentityClient in ans-sdk-registration for Verified-Identity management, with IdentityPaths for the endpoint paths. Add unit tests for the client, service, paths, and the registration client error paths. Signed-off-by: James Hateley <jhateley@godaddy.com>
Add transparency-log reads for verified identities (getAgentIdentities, getAgentIdentityHistory, and linked-agent lookups) and the v2 event schema models. Join verified identities onto agent badges and add TlLeafUncommitted error handling. Add unit tests and model coverage. Signed-off-by: James Hateley <jhateley@godaddy.com>
| JWSHeader header, byte[] signingInputBytes) { | ||
| try { | ||
| if (JWSAlgorithm.EdDSA.equals(algorithm)) { | ||
| // Ed25519 JCA signatures are already the raw R||S form JOSE expects, no transcoding needed. |
There was a problem hiding this comment.
Minor: misleading comment at line 158
// Ed25519 JCA signatures are already the raw R||S form JOSE expects, no transcoding needed.
R||S is ECDSA notation. Ed25519 produces a 64-byte signature that is not structured as R||S in the ECDSA sense. The intended meaning is correct (no DER transcoding needed, unlike ECDSA), but the wording will mislead anyone extending this to other EdDSA variants.
Should read something like: "Ed25519 JCA output is the raw 64-byte signature per RFC 8037 — no DER transcoding needed unlike ECDSA."
| private JWK toPublicJwk(JWSAlgorithm algorithm, PublicKey publicKey) { | ||
| try { | ||
| if (JWSAlgorithm.RS256.equals(algorithm)) { | ||
| return new RSAKey.Builder((RSAPublicKey) publicKey).build(); | ||
| } | ||
| if (JWSAlgorithm.ES256.equals(algorithm)) { | ||
| return new ECKey.Builder(Curve.P_256, (ECPublicKey) publicKey).build(); | ||
| } | ||
| // EdDSA: the raw 32-byte public key is the tail of the X.509 SubjectPublicKeyInfo encoding. | ||
| if (!(publicKey instanceof EdECPublicKey)) { | ||
| throw new IllegalArgumentException("publicKey does not match the private key algorithm"); | ||
| } |
There was a problem hiding this comment.
The private key rejects Ed448 at line 141, but toPublicJwk() doesn't apply the equivalent check to the public key.
The guard at line 186 is:
if (!(publicKey instanceof EdECPublicKey)) { ... }This accepts both Ed25519 and Ed448, since both implement EdECPublicKey. The subsequent extraction:
byte[] raw = Arrays.copyOfRange(encoded, encoded.length - ED25519_RAW_KEY_LEN, encoded.length);silently slices the last 32 bytes regardless of the actual key type. For Ed448 (71-byte encoding, 57-byte key), this cuts into the middle of the key material and builds a structurally valid but semantically wrong OctetKeyPair. No exception is thrown — the JWS is signed and returned.
The fix suggested by Claude is one check mirroring what resolveAlgorithm already does for the private key:
// after the instanceof check
EdECPublicKey edPublicKey = (EdECPublicKey) publicKey;
if (!ED25519.equals(edPublicKey.getParams().getName())) {
throw new IllegalArgumentException(
"EdEC public key must use Ed25519 curve, got: " + edPublicKey.getParams().getName());
}
byte[] encoded = edPublicKey.getEncoded();
if (encoded == null) {
throw new IllegalArgumentException("EdEC public key encoding is not available");
}The null check also addresses the getEncoded() NPE — the ClassCastException handler on line 192 won't catch a NullPointerException.
| @Test | ||
| void ed448Throws() throws Exception { | ||
| KeyPair kp = gen("Ed448"); | ||
| assertThatThrownBy(() -> signer.sign(SIGNING_INPUT, kp.getPrivate(), KID)) | ||
| .isInstanceOf(IllegalArgumentException.class); | ||
| } | ||
|
|
There was a problem hiding this comment.
Related with my previous comment: The Ed448-public-key bug has no test
ed448Throws() (line 116) only tests an Ed448 private key, which correctly fails in resolveAlgorithm. There's no test for Ed25519 private key + Ed448 public key:
@Test
void ed448PublicKeyWithEd25519PrivateThrows() throws Exception {
KeyPair ed25519 = gen("Ed25519");
PublicKey ed448Public = gen("Ed448").getPublic();
assertThatThrownBy(() -> signer.sign(SIGNING_INPUT, ed25519.getPrivate(), KID, ed448Public))
.isInstanceOf(IllegalArgumentException.class);
}This test would currently fail — the production code silently returns a JWS with a corrupted JWK.
| private void assertRoundTrip(KeyPair kp) throws Exception { | ||
| String jws = signer.sign(SIGNING_INPUT, kp.getPrivate(), KID, kp.getPublic()); | ||
| String[] parts = jws.split("\\."); | ||
| assertThat(parts).hasSize(3); | ||
| assertThat(parts[1]).isEqualTo(SIGNING_INPUT); | ||
|
|
||
| JWSObject parsed = JWSObject.parse(jws); | ||
| assertThat(parsed.getHeader().getKeyID()).isEqualTo(KID); | ||
| assertThat(parsed.getHeader().getJWK()).isNotNull(); | ||
| assertThat(parsed.getHeader().getJWK().isPrivate()).isFalse(); | ||
| assertThat(verifies(parts, kp.getPublic())).isTrue(); | ||
| } |
There was a problem hiding this comment.
Testing Gap: Round-trip tests never verify the embedded JWK
assertRoundTrip() (line 173) checks that a JWK is present and public-only, then verifies the signature using the original kp.getPublic() — it never uses the JWK from the header:
assertThat(parsed.getHeader().getJWK()).isNotNull();
assertThat(parsed.getHeader().getJWK().isPrivate()).isFalse();
assertThat(verifies(parts, kp.getPublic())).isTrue(); // ignores the embedded JWK entirelyThe signature is computed from the private key regardless of what ends up in the JWK field. So even if toPublicJwk() embedded completely wrong bytes, these assertions would still pass. A meaningful round-trip test for the embedded JWK path should verify the signature using the key extracted from the header:
// For Ed25519 round-trip, extract and use the embedded JWK
JWK embeddedJwk = parsed.getHeader().getJWK();
OctetKeyPair okp = (OctetKeyPair) embeddedJwk;
PublicKey recoveredPublic = okp.toPublicKey();
assertThat(verifies(parts, recoveredPublic)).isTrue();Without this, the JWK encoding logic is structurally untested — toPublicJwk could encode garbage and no test would catch it.
| mcpSdkVersion=1.1.0 | ||
| caffeineVersion=3.1.8 | ||
| cborVersion=4.5.4 | ||
| nimbusJoseVersion=10.0.2 |
There was a problem hiding this comment.
Curious whether we can update the version to use a more recent version? According to https://mvnrepository.com/artifact/com.nimbusds/nimbus-jose-jwt the latest release is 10.9.1
| static String identityPath(String identityId, String... segments) { | ||
| StringBuilder path = new StringBuilder(COLLECTION) | ||
| .append('/') | ||
| .append(identityId); | ||
| for (String segment : segments) { | ||
| path.append('/').append(segment); | ||
| } | ||
| return path.toString(); | ||
| } |
There was a problem hiding this comment.
Path parameters not percent-encoded (line 62)
identityId and agentId are raw-concatenated into URLs. A DID like did:web:example.com#key-1 causes URI.create() to throw IllegalArgumentException; a /-containing ID silently routes to the wrong path.
Failure scenario: An identity ID containing # (e.g., did:web:example.com#key-1) causes URI.create(baseUrl + path) to throw IllegalArgumentException at request-build time. An ID containing / silently routes to a different path, returning 404 or matching an unintended resource.
| /** Maximum number of agents that a single link request can carry. */ | ||
| private static final int MAX_LINK_AGENTS = 256; |
There was a problem hiding this comment.
Where is this 256 defined? Is it from some spec?
| private IdentityChallengeResponse parseChallenge(String body) { | ||
| IdentityChallengeResponse challenge = | ||
| httpClient.parseResponse(body, IdentityChallengeResponse.class); | ||
|
|
||
| if (challenge.getIdentityId() == null) { | ||
| throw new AnsServerException("Identity challenge response missing 'identityId'", 0, null); | ||
| } | ||
| if (challenge.getNonce() == null) { | ||
| throw new AnsServerException("Identity challenge response missing 'nonce'", 0, null); | ||
| } | ||
| return challenge; | ||
| } |
There was a problem hiding this comment.
parseChallenge silently returns nulls for missing @Nonnull fields (line 192)
Only identityId and nonce are validated; expiresAt, kind, value, status can be null even though annotated @Nonnull. Callers like challenge.getExpiresAt().isBefore(...) will NPE instead of receiving a proper AnsServerException.
Failure scenario: A server returns a 202 body omitting expiresAt. parseChallenge returns the IdentityChallengeResponse without error. Any caller that calls challenge.getExpiresAt().isBefore(Instant.now()) throws NullPointerException at the call site rather than the expected AnsServerException.
| IdentityDetails revoke(String identityId) { | ||
| HttpRequest httpRequest = httpClient.createRequestBuilder(IdentityPaths.revokePath(identityId)) | ||
| .POST(HttpRequest.BodyPublishers.noBody()) | ||
| .build(); | ||
|
|
||
| HttpResponse<String> response = httpClient.sendRequest(httpRequest); | ||
| return httpClient.parseResponse(response.body(), IdentityDetails.class); | ||
| } |
There was a problem hiding this comment.
Not sure whether this is an actual issue - revoke() sends Content-Type: application/json with an empty body
Line 137 createRequestBuilder() unconditionally sets the JSON content-type, but revoke() uses BodyPublishers.noBody() on line 138. Strict gateways or JSON-validating middleware will return 400/415, breaking all revocations.
Failure scenario: The revoke POST reaches the server with Content-Type: application/json and Content-Length: 0. Strict servers or JSON-validating middleware that require a parseable JSON body return 400 or 415, causing every revocation call to fail with AnsServerException('Unexpected error').
| /** | ||
| * Unit tests for {@link IdentityPaths}, the single source of RA Verified-Identity paths. | ||
| * | ||
| * <p>Each method is pinned to an exact string, so a typo in a path constant fails | ||
| * here at the source, not as an opaque WireMock stub miss. Every identity path sits | ||
| * under {@code /v2/ans/identities}.</p> | ||
| */ | ||
| class IdentityPathsTest { |
There was a problem hiding this comment.
Related with the previous comment:
No test with URL-special characters in IDs
All path tests use well-formed UUIDs. No test passes an ID containing #, /, or : (all plausible in DID identifiers).
A single test like:
assertThat(IdentityPaths.identityPath("did:web:example.com#key-1"))
.isEqualTo("/v2/ans/identities/did%3Aweb%3Aexample.com%23key-1");would immediately expose the missing percent-encoding from that finding.
Related issue
Fixes #86
Summary
This branch adds Verified Identity support to the ANS Java SDK. A Verified Identity is a first-class object with its own lifecycle, separate from agent registration. The branch covers three areas: identity management, control-proof signing, and transparency-log reads.
Identity management (ans-sdk-registration)
The new
IdentityClientgives access to the eight Registration Authority operations on the/v2/ans/identitiessurface: register, list, get details, rotate, verify control, revoke, link to agents, and unlink. An internalIdentityServicedoes the HTTP work. AnIdentityPathshelper builds the request paths.Register and rotate return a 202 challenge round. The identity is not sealed until the caller completes the challenge and submits a control proof to verify-control. A link request carries at most 256 agents. The client offers both synchronous and asynchronous (
CompletableFuture) call styles.Control-proof signing (ans-sdk-crypto)
The new
IdentityProofSignersigns the control-proof challenge as a compact JWS string, one per proven key. It supports the three algorithms the verifier implements: EdDSA (Ed25519), ES256 (ECDSA P-256), and RS256 (RSA 2048 or more). It reads the algorithm from the private key. It rejects key-agreement keys and curves with no verifier before it signs.The served signing input becomes the JWS payload without change, because the RA checks payload equality before signature. This work adds a dependency on Nimbus JOSE 10.0.2.
Transparency-log reads (ans-sdk-transparency)
TransparencyClientgains identity reads: get identity badge, identity audit, identity receipt, identity linked agents, agent identities, and agent identity history. Each read has an async variant.The agent badge now includes the joined verified identities. The badge caps its inline identity list at 25 entries. A caller pages the full set through the agent-identities read, which reports the total count. New models cover these responses:
AgentIdentitiesResponse,IdentityLinkedAgentsResponse, andLinkedAgentView.A new
TlLeafUncommittedExceptionmaps the retryable503 TL_LEAF_UNCOMMITTEDcondition. This condition means a leaf is committed but no signed checkpoint covers it yet. The exception carries theRetry-Afterdelay and reports itself as retryable.The branch also adds V2 schema handling for transparency-log events. It adds the V2 models
EventV2,AttestationsV2,CertificateInfoV2,DnsRecordV2,ProducerV2, andTransparencyLogV2.Testing
Unit tests added for all new code paths and coverage held > 90%.
E2E testing performed against locally running RA/TL.
AI assistance
Checklist
git commit -s) certifying the DCO