[#1130] Verify client assertions by their own alg; default id_token_signed_response_alg

[vharseko]

Fixes #1130 — both correctness bugs reported there, in OpenAMClientRegistration.

Bug 2 — client assertions were dispatched on id_token_signed_response_alg

verifyJwtIdentity() chose HMAC vs. asymmetric verification from the client's id_token_signed_response_alg — the algorithm of the ID tokens we issue — so a private_key_jwt client whose ID-token algorithm was HS256 had its RS256 assertion pushed through the shared-secret verifier (JwsSigningException: Unsupported Signing Algorithm, SHA256withRSA).

The method has four callers: two verify client-signed assertions (ClientCredentialsReader, JwtBearerGrantTypeHandler), two verify OpenAM-issued ID tokens (IdTokenInfo, OpenIdConnectSSOProvider). Dispatching on token_endpoint_auth_method as the issue suggests would break the latter two and legacy clients that never set the method.

• Client assertions (verifyJwtIdentity): the dispatch follows the JWS header of the presented JWT. HMAC → client secret only; RS/ES → the client's registered keys only (jwks / jwks_uri / x509, and an oct JWK is never tried against an asymmetric alg); none, or any alg outside the JwsAlgorithm enum (JwsHeader.getAlgorithm() is valueOf, so the RFC spelling "none" used to throw) → false. Neither branch can reach the other's key material.
• OpenAM-issued ID tokens (new OpenIdConnectClientRegistration.verifyIdTokenIdentity, used by IdTokenInfo and OpenIdConnectSSOProvider): the header alg must equal the client's id_token_signed_response_alg (a free-text attribute, upper-cased as StatefulTokenStore does when issuing; an unknown value is "not ours" — and IdTokenInfo's client-authentication gate, which parses the same attribute, now does the same instead of throwing), then the same verification runs. The algorithm — and so the key — is not the presenter's to pick, and IdTokenInfo's client-authentication gate and the verifier now reason about the same alg.
• Because any client_id can now be sent an assertion with any alg, what the client has registered decides between invalid_client and server_error: no userpassword / empty secret → false; no publicKeyLocation or an unknown value → false; a registered location with nothing behind it (jwks_uri without a URI — the state a console-, ssoadm- or /json/agents-created client is in, since AgentConfiguration.createAgent persists the schema default jwks_uri — x509 without a certificate, jwks without a set) → false; a registered key that cannot verify the header's alg or parse its signature (any JwsException — RSA key vs. ES256, a symmetric key served at jwks_uri, an RSA signature of another length than the registered modulus — or IllegalArgumentException from the handlers) → false, logged at message level with the client id and header alg. What remains server_error (an HTTP 400 with a logged stack trace) is what the client did not choose: failures to read the registration, to parse registered material, or to fetch the JWKS. At the base every one of these was server_error.
• Consequence of the design, stated so it reads as intended: a client configured for RS256 ID tokens that also has a userpassword now authenticates with an HS256 client_secret_jwt signed with that secret (at the base this was server_error). No new credential is accepted — the secret already authenticates the client via client_secret_basic/post — and token_endpoint_auth_method was never enforced.

Bug 1 — NPE when id_token_signed_response_alg was never persisted

AgentsRepo.getAgentAttrs() reads agent attributes with getAttributesWithoutDefaults(), so the schema default (HS256 in AgentService.xml) is not applied to a client whose creation path does not persist defaults — the realm-config REST endpoint (/json/realm-config/agents/OAuth2Client, SMS) and /frrest/oauth2/client; console, ssoadm and /json/agents go through AgentConfiguration.createAgent, which does. Such a client made getIDTokenSignedResponseAlgorithm() return null, which NPE'd in StatefulTokenStore.createOpenIDToken (toUpperCase()), IdTokenInfo and the old verifyJwtIdentity (JwsAlgorithm.valueOf(null)). It now falls back to HS256 — the default the schema, the console and dynamic registration (ID_TOKEN_SIGNED_RESPONSE_ALG_DEFAULT) already use, rather than the spec's RS256, to stay consistent with clients created through the console. Utils.getAttributeValueFromSet (getClientSecret()) is null-safe for the same population, which also gets StatefulTokenStore past line 278 for a client without userpassword; openam-uma's IdTokenClaimGatherer, the one consumer that did not guard, now does.

Not in this PR

The HMAC branch requires aud ∋ client_id (added for idtokeninfo in 4b1177e, where it is right); the asymmetric branch checks no aud value, and the callers add none of their own (ClientCredentialsReader checks aud ∋ token endpoint only when given one, TokenRevocationResource passes null, JwtBearerGrantTypeHandler checks nothing). A single aud policy cannot be applied here without breaking one side: RFC 7523 aud is the token endpoint for assertions (as ClientCredentialsReaderTest already builds them — a spec-compliant client_secret_jwt is rejected today), OIDC Core aud is the client_id for ID tokens. Now that the two use cases are separate methods, that split is a follow-up issue.

Tests

OpenAMClientRegistrationTest: default (empty set / null / explicit value); HS256 assertion verified with the secret under idTokenSignedResponseAlg=RS256 and rejected with a wrong secret; RS256 assertion verified against the client's JWKS under HS256 and rejected with a foreign key; wire "alg":"none" and "hs256", header without alg and enum-spelled "NONE" refused; secret null / empty → false; publicKeyLocation null / empty / unknown → false; each selector with nothing behind it → false; ES256 against an RSA JWK → false; RS256 signed with a 3072-bit key against a 2048-bit JWK → false; symmetric key behind jwks_uri (resolver seeded through ClientJwksResolverCache) → false; oct JWK + RS256 → false; verifyIdTokenIdentity accepts the configured alg (also hs256), rejects another, none and an unknown configured value. IdTokenInfoTest (new; openam-core test-jar for RealmTestHelper) and OpenIdConnectSSOProviderTest: a token that verifies only as a client assertion is rejected; IdTokenInfoTest also accepts a configured hs256 and rejects an unknown configured value as a bad request. IdTokenClaimGathererTest: public client, RS256 / HS256. OAuth2JwtTest: getSigningAlgorithm(). Full openam-oauth2 (655) and openam-uma (194) suites pass.

[maximthomas]

praise: Dispatching on the presented alg is the right fix — verification no longer trusts a field the client can leave blank — and the tests came first: all four new cases fail at the base commit.

• verifyJwtIdentity branches on JwsHeader.getAlgorithm(), and the JwsAlgorithmType.HMAC type check covers HS384/HS512 without listing them
• getIDTokenSignedResponseAlgorithm defaults null/empty to HS256, which closes the toUpperCase NPE at StatefulTokenStore:283 for REST/ssoadm-created clients
• verifyJwtBySharedSecret extracted with an explicit empty-secret guard; the RS256-by-JWKS path has a test

---

issue (blocking): A wire "alg":"none" — or any alg outside the JwsAlgorithm enum — is rejected by an uncaught IllegalArgumentException, not by the new guard.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:665-668, openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java:412-421

JwsHeader.getAlgorithm() in commons is getAlgorithmString() == null ? NONE : JwsAlgorithm.valueOf(s) — never null, case-sensitive. The RFC spelling "none" (likewise "hs256" or anything unknown) throws before line 666 runs, so the null arm is dead and the NONE arm fires only for a header literally spelled "NONE". That is what the builder emits (JwtHeader.setAlgorithm stores toString()), so the test pins a spelling that never occurs on the wire. On the token endpoint the exception becomes a 400 invalid_request (TokenEndpointResource:99-101); from IdTokenInfo:174 and OpenIdConnectSSOProvider:263 it escapes unhandled.

final JwsAlgorithm signatureAlgorithm;
try {
    signatureAlgorithm = jwt.getSignedJwt().getHeader().getAlgorithm();
} catch (IllegalArgumentException e) {   // "none", "hs256", anything not in the enum
    return false;
}
if (signatureAlgorithm.getAlgorithmType() == JwsAlgorithmType.NONE) {
    return false;
}

Pin: build the assertion as a string, not through the builder — Base64url.encode("{\"alg\":\"none\"}") + . + claims + . → OAuth2Jwt.create(jwtString) → verifyJwtIdentity is false; a second case with "alg":"hs256".

---

issue (blocking): The new empty-secret guard is unreachable for a client that has no userpassword attribute.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:693-696, :167-169, openam-oauth2/src/main/java/org/forgerock/openam/oauth2/Utils.java:115

getClientSecret() → Utils.getAttributeValueFromSet → values.iterator().next() with no guard: absent attribute → NPE, empty set → NoSuchElementException, both before StringUtils.isEmpty runs. The HMAC arm sits outside the asymmetric arm's catch, so the exception leaves verifyJwtIdentity raw. The description says a public client "now gets false instead of an NPE in the resolver"; for a client without the attribute the NPE only moves from SharedSecretOpenIdResolverImpl to Utils:115, and all three new verifyJwtIdentity* cases stub a non-empty secret, so nothing exercises the guard.

// Utils.getAttributeValueFromSet: null for absent or empty; the caller already guards
return CollectionUtils.getFirstItem(values);

Pin: given(amIdentity.getAttribute("userpassword")).willReturn(null) and .willReturn(Collections.<String>emptySet()), each with an HS256 assertion → false, no exception.

---

issue (blocking): Dispatching on the header makes getClientPublicKeySelector() hot for every client, and its set.iterator().next() has the same missing-default defect Bug 1 fixes.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:835, :671-683

At the base only RS/ES-configured clients reached the asymmetric branch; now any client_id does, on the sender's say-so. A client created via REST/ssoadm without publicKeyLocation — the population Bug 1 is about — gets null/empty from the same defaults-less AgentsRepo read, so one unauthenticated alg:RS256 assertion ends in NPE/NoSuchElementException → catch (Exception) → SERVER_ERROR 500 plus a logger.error stack trace per request, where the description promises "the client's registered public keys only" → false.

// getClientPublicKeySelector
final String selector = CollectionUtils.getFirstItem(set);
return StringUtils.isEmpty(selector) ? null : Client.PublicKeySelector.fromString(selector);

// verifyJwtIdentity, asymmetric arm
final Client.PublicKeySelector selector = getClientPublicKeySelector();
if (selector == null) {
    return false;   // nothing registered: invalid_client, not server_error
}

Pin: given(amIdentity.getAttribute(PUBLIC_KEY_SELECTOR)).willReturn(null) + an RS256 assertion → false. Or: give a selector with no key material (x509 without a certificate) the same false in byX509Key.

---

issue (non-blocking): IdTokenInfo and OpenIdConnectSSOProvider decide whether client authentication is required from the configured alg, while verification now follows the header alg.

openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java:162-176, openam-oauth2/src/main/java/org/forgerock/openidconnect/OpenIdConnectSSOProvider.java:263

For an RS256-configured client an HS256 id_token signed with the client secret passes the "no client auth needed" gate and then verifies by the secret (and yields a session in the SSO provider). It needs the secret, so no gain for an attacker, but the gate and the verifier now reason about different algs — the split this PR set out to remove. Reject a header alg that differs from the configured one in these two callers, or key the gate on the header too.

---

issue (non-blocking): The audience policy is now chosen by the sender: the HMAC branch requires aud ∋ client_id, the asymmetric branch checks no audience at all.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:702, byJWKs / byJWKsURI / byX509Key

Pre-existing asymmetry, but before this PR the client's configuration picked the branch; now the alg header does. Apply one aud check in both branches. RFC 7523 §3 makes aud the AS / token endpoint, not the client_id — the new HS256 test cements the non-standard value.

---

suggestion (non-blocking): "Neither branch can reach the other's key material" does not hold for byJWKs: an oct JWK is a SecretKey, and getSigningHandlerForKey hands it to newHmacSigningHandler.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:740-741

Not exploitable — an RS256 header against an HMAC handler does not verify — but nothing pins it. Pin: JWKS containing only an oct key + RS256 assertion → not true (today an OAuthProblemException; false once the blocking issue above lands). Or: drop the sentence from the description.

---

suggestion (non-blocking): Bug 1 is pinned at the getter only; the NPE sites the description cites have no regression test, and the caller tests mock verifyJwtIdentity.

openam-oauth2/src/test/java/org/forgerock/openam/oauth2/OpenAMClientRegistrationTest.java:355-361, openam-oauth2/src/test/java/org/forgerock/oauth2/core/ClientCredentialsReaderTest.java:124, openam-oauth2/src/test/java/org/forgerock/openidconnect/OpenIdConnectSSOProviderTest.java:205

A caller that stops calling verifyJwtIdentity stays green everywhere. One test through StatefulTokenStore (or IdTokenInfo) with idTokenSignedResponseAlg absent, asserting a signed token comes back, kills that mutant.

[vharseko]

Round 2 in 3dd4b80; PR description updated.

"alg":"none" / unknown alg → IAE — fixed. Confirmed against the bytecode: JwsHeader.getAlgorithm() is s == null ? NONE : JwsAlgorithm.valueOf(s) and JwsAlgorithm.toString() is the enum name, so the builder emitted "NONE" and the test pinned a spelling that never occurs on the wire. OAuth2Jwt.getSigningAlgorithm() now returns null for anything outside the enum and verifyJwtIdentity returns false; the null arm is live. Tests build the assertion as a raw compact serialisation with "none" and "hs256". Not a regression, for the record — at the base SignedJwt.verify() called the same getAlgorithm() and threw the same IAE — but the description claimed none → refused and it did not hold.

Empty-secret guard unreachable — fixed. Utils.getAttributeValueFromSet → CollectionUtils.getFirstItem(values); its only two callers are the getClientSecret() implementations, and every consumer of getClientSecret() (getIDTokenEncryptionKey, StatefulTokenStore:278, CheckSession:147) already guards with isEmpty. Side effect worth noting: StatefulTokenStore:278 NPE'd for a REST-created client without userpassword before ever reaching the toUpperCase() at 283, so this also covers that part of Bug 1's population. Tests: userpassword null and empty set, HS256 assertion → false.

getClientPublicKeySelector() hot for every client — fixed as suggested: getFirstItem + fromString (null for an unknown value too) and selector == null → false. Tests: selector null / empty / "not-a-selector" with an RS256 assertion → false.

IdTokenInfo / OpenIdConnectSSOProvider gate vs. verifier — fixed, in the registration rather than the two callers so the rule lives in one tested place: new OpenIdConnectClientRegistration.verifyIdTokenIdentity() requires the header alg to equal id_token_signed_response_alg, then runs the same verification. Both ID-token callers use it; verifyJwtIdentity is now only the client-assertion path, which matches its javadoc (“signed by this client”). Tests: match → true, another alg → false, wire none → false; OpenIdConnectSSOProviderTest rejects a token that verifies only as a client assertion.

aud policy chosen by the sender — recorded, not changed here. You are right about the origin: isIntendedForAudience(getClientId()) came in with idtokeninfo (4b1177e), where aud = client_id is correct, and it was inherited by the assertion path. ClientCredentialsReaderTest already builds assertions with aud = token endpoint, so a spec-compliant client_secret_jwt is rejected by the HMAC branch at the base as well. A single aud check in both branches cannot be done in this PR without breaking one side: aud ∋ client_id in the asymmetric branch breaks compliant private_key_jwt clients that work today; the token endpoint in the HMAC branch breaks IdTokenInfo / the SSO provider. With the two use cases now on separate methods the split is straightforward; I will open a follow-up issue for it (RFC 7523 aud for assertions, OIDC Core aud for ID tokens).

oct JWK → HMAC handler — pinned and closed: byJWKs returns false for a SecretKey. One detail: the fix for the selector would not have turned this into false on its own — HmacSigningHandler.verify(RS256) throws JwsSigningException inside the catch (Exception), so it stayed a server_error until the key-type check. The test JWK carries "alg":"HmacSHA256" because JWKLookup resolves an oct key's alg with JwsAlgorithm.getJwsAlgorithm(), which matches the JCA name; with the JWA "HS256" the commons parser throws before the guard is reached (pre-existing, not touched).

Bug 1 regression test through StatefulTokenStore — not added. StatefulTokenStoreTest mocks ClientRegistration, and the fix lives in OpenAMClientRegistration.getIDTokenSignedResponseAlgorithm(), so a store-level test cannot exercise it without wiring a real registration over a mocked AMIdentity into the store test; the getter is the unit where the change is. The mutant you describe (a caller that stops calling verifyJwtIdentity) is a pre-existing gap in the caller tests rather than something this PR introduces — happy to take it as a separate task.

[maximthomas]

praise: Every round-1 blocker is closed at the mechanism, not the symptom, and each closure is pinned by a test that fails at the base commit.

• OAuth2Jwt.getSigningAlgorithm() isolates the valueOf trap in one method; verifyJwtIdentityRejectsWireAlgNone / verifyJwtIdentityRejectsUnknownAlg build the assertion as a raw string, so the wire spelling "none" is what the test sends
• Utils.getAttributeValueFromSet → CollectionUtils.getFirstItem closes the absent-userpassword NPE at the source, and verifyJwtIdentityReturnsFalseForHmacAssertionWhenClientHasNoSecret covers null, empty set and empty string
• verifyIdTokenIdentity gives IdTokenInfo and the SSO provider a verifier that pins the header alg to the configured one — the round-1 "gate and verifier reason about different algs" point is gone
• byJWKs refuses an oct JWK; the three touched suites pass at head, 60/60

---

issue (blocking): A registered selector with no key material behind it still ends in server_error plus an error-level stack trace, and the same catch turns an alg/key-type mismatch into server_error too.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:671-687, :726-728, :766-768, :826-828, :834

The selector == null arm at :672-675 is half of round-1 Bug 3. The other half is the common shape: AgentService.xml:358-360 defaults publicKeyLocation to jwks_uri and AgentConfiguration.createAgent persists schema defaults, so a console- or ssoadm-created client carries a selector and no URI. For it byJWKsURI throws SERVER_ERROR inside the try, catch (Exception) → Utils.createException → logger.error with stack trace + HTTP 400 {"error":"server_error"} (OAuthError.SERVER_ERROR is a 400, not a 500 — round 1 had the number wrong, not the road). The same catch takes an ES256 header against an RSA JWK or certificate (RSASigningHandler.validateAlgorithm → IllegalArgumentException) and an oct JWK served at jwks_uri (the SecretKey guard is in byJWKs only; HmacSigningHandler → JwsSigningException). Since the header now picks the arm, one unauthenticated alg:RS256 assertion with any client_id does this, where the comment at :673-674 and the description ("fail closed instead of with a 500") promise false. Not a bypass: no path returns true.

// byJWKsURI — same shape in byJWKs (:725-728) and byX509Key (:826-831): nothing registered → false
final String url = CollectionUtils.getFirstItem(set);
if (StringUtils.isEmpty(url)) {
    return false;
}

// verifyJwtIdentity, asymmetric arm: a key that cannot verify this alg is invalid_client
} catch (JwsSigningException | IllegalArgumentException e) {   // alg/key-type mismatch, oct JWK behind jwks_uri
    return false;
} catch (Exception e) {                                        // IdRepo / SSO / JWKS fetch failures
    throw Utils.createException("Client Bearer Jwt Public key", e, logger);
}

Pin: three cases → false, no exception: PUBLIC_KEY_SELECTOR=jwks_uri with JWKS_URI absent; x509 with CLIENT_JWT_PUBLIC_KEY absent; ES256 assertion against an RSA JWK. Then the :673-674 comment and the description sentence become true.

---

issue (non-blocking): verifyIdTokenIdentity parses the configured alg with a strict valueOf on a free-text attribute.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:696, openam-server-only/src/main/resources/services/AgentService.xml:218-224

idTokenSignedResponseAlg is syntax="string" with no ChoiceValues. The issuer upper-cases it (StatefulTokenStore:283), the verifier does not, so a client saved as hs256 is issued tokens it cannot verify: IllegalArgumentException out of OpenIdConnectSSOProvider:263, which :233 does not translate. This PR gave the selector a fromString; the alg gets the same treatment.

final JwsAlgorithm configured;
try {
    configured = JwsAlgorithm.valueOf(getIDTokenSignedResponseAlgorithm().toUpperCase());   // as StatefulTokenStore:283
} catch (IllegalArgumentException e) {
    return false;
}
return idToken.getSigningAlgorithm() == configured && verifyJwtIdentity(idToken);

Pin: IDTOKEN_SIGNED_RESPONSE_ALG = "hs256" + an HS256 token → true; "not-an-alg" → false.

---

issue (non-blocking): IdTokenClaimGatherer still calls getClientSecret().getBytes() unguarded; the "every consumer guards" list misses it.

openam-uma/src/main/java/org/forgerock/openam/uma/IdTokenClaimGatherer.java:86-87

With getFirstItem a public client's secret is null here → NPE at :87, caught by neither arm at :102-107. Same outcome as base (the NPE moved one frame), but the getter's null contract is this PR's, and an RS256 id_token from that client is verifiable without a secret.

final String secret = clientRegistrationStore.get(authorizationApiToken.getClientId(), oAuth2Request).getClientSecret();
byte[] clientSecret = secret == null ? null : secret.getBytes(Utils.CHARSET);   // verify() only reads it on the HMAC road

---

suggestion (non-blocking): The new verifyIdTokenIdentity call in IdTokenInfo has no test; the mutant back to verifyJwtIdentity stays green.

openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java:174

There is no IdTokenInfo test class at head. The SSO provider caller is pinned (OpenIdConnectSSOProviderTest:229-230 distinguishes the two methods); this one is not, and it is the call that closed round-1's non-blocking #1.

Pin: a registration mock with verifyIdTokenIdentity → false, verifyJwtIdentity → true → BadRequestException("invalid id_token").

---

suggestion (non-blocking): The NONE half of the new guard is untested.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:664-666

No case builds a header without alg (which JwsHeader.getAlgorithm() maps to NONE) or with the literal "NONE"; deleting the NONE clause sends both into the asymmetric arm and no test flips.

// rawAssertion with header {"typ":"JWT"} — no alg — and once with "NONE"
assertThat(clientRegistration.verifyJwtIdentity(rawAssertionWithoutAlg(clientId))).isFalse();
assertThat(clientRegistration.verifyJwtIdentity(rawAssertion(clientId, "NONE"))).isFalse();

---

note (non-blocking): For the promised aud follow-up issue — the downstream callers of the asymmetric arm, traced at head, add no audience check of their own.

openam-oauth2/src/main/java/org/forgerock/oauth2/core/JwtBearerGrantTypeHandler.java:70 (no aud/iss check after verifyJwtIdentity), openam-oauth2/src/main/java/org/forgerock/openam/oauth2/ClientCredentialsReader.java:158 (aud ∋ endpoint only when endpoint != null), openam-oauth2/src/main/java/org/forgerock/openam/oauth2/rest/TokenRevocationResource.java:119 (passes null)

Recorded, not re-raised: with header dispatch the aud-less arm is reachable for every client with registered keys, not only RS/ES-configured ones.

---

thought (non-blocking): A client configured RS256 that also has a userpassword now authenticates with an HS256 client_secret_jwt; at base that was server_error.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:668-669

A consequence of the design, not a defect — no new credential is accepted, and token_endpoint_auth_method was never enforced. Worth one sentence in the description so it reads as intended.

[vharseko]

Round 3 in 6d58828; PR description updated (the ssoadm claim under Bug 1 and the 500 are corrected there too — see below).

Registered selector with no material / alg-key mismatch → server_error — fixed as suggested. Each of the three branches returns false when nothing is registered behind the selector (getFirstItem + isEmpty, so an empty string counts too), and the asymmetric arm catches JwsSigningException | IllegalArgumentException → false ahead of the catch (Exception), which keeps IdRepo/SSO and JWKS-fetch failures as server_error. Two things you were right about that I had wrong: OAuthError.SERVER_ERROR goes through the three-argument constructor, so it is a 400; and AgentConfiguration.createAgent (:444, getDefaultValues merged before createIdentity) persists the schema defaults, so ssoadm, the console and /json/agents all produce the "selector and no URI" shape — only the SMS realm-config endpoint and /frrest/oauth2/client skip defaults. The description said "realm-config REST endpoint or ssoadm" for Bug 1; that is now "realm-config REST endpoint or /frrest/oauth2/client". Tests: jwks_uri null / empty / "", x509 null / "", jwks null / ""; ES256 assertion against an RSA JWK (Not an RSA algorithm. at the base); and the symmetric-key-behind-jwks_uri case without an HTTP server — a SharedSecretOpenIdResolverImpl seeded through ClientJwksResolverCache.putIfAbsent under client_id|url, which reproduces HmacSigningHandler → Unsupported Signing Algorithm, SHA256withRSA at the base and is false now.

Strict valueOf on free-text idTokenSignedResponseAlg — fixed: toUpperCase() as StatefulTokenStore:283, IllegalArgumentException → false. Tests: hs256 + HS256 token → true, not-an-alg → false. (IdTokenInfo:162 keeps its strict valueOf for the client-auth gate — pre-existing, and it runs after the store has already issued with the upper-cased value; left alone.)

IdTokenClaimGatherer unguarded — fixed, and my "every consumer guards" was wrong: I had grepped openam-oauth2 only. secret == null ? null : secret.getBytes(...), and verify() returns false for an HMAC token when there is no secret rather than building a handler on null. Tests in IdTokenClaimGathererTest: public client + RS256 → requesting party id; public client + HS256 → null, no exception.

IdTokenInfo call untested — added IdTokenInfoTest. The original one was deleted in 2016 when setRealmOnRequest arrived; Realm.of() is covered by RealmTestHelper from the openam-core test-jar (new test-scope dependency in openam-oauth2/pom.xml, the same one openam-uma uses) and ServletUtils.getRequest() by mocked HttpRequest/ServletCall, as OAuth2RequestFactoryTest already does. Two cases: a token issued for the client comes back; verifyIdTokenIdentity → false with verifyJwtIdentity → true → BadRequestException("invalid id_token"). The mutant back to verifyJwtIdentity fails the second.

NONE half of the guard — tests added for a header without alg (→ NONE via getAlgorithm()) and the literal "NONE", both false. One honest note: after the blocking fix above, deleting the NONE clause no longer flips them — NONE would go down the asymmetric arm and end in false there as well (no selector, or validateAlgorithm → IAE → false). The clause is now belt-and-braces and the tests pin the contract rather than that line.

aud note — recorded for the follow-up; the three call sites you list are in the "Not in this PR" section of the description now, together with the client_secret_jwt-with-compliant-aud consequence.

HS256 client_secret_jwt for an RS256-configured client — one sentence added to the description, worded as a consequence of the design (no new credential; token_endpoint_auth_method never enforced).

[maximthomas]

praise: The round-2 fixes are real and measured, not just re-worded.

• All five hunks of this commit are pinned: reverting each one turns its named case red (catch (JwsSigningException | IllegalArgumentException), the three isEmpty → false guards, verifyIdTokenIdentity normalisation, the IdTokenInfo:174 call, the IdTokenClaimGatherer null-secret guard).
• Empty material behind every selector is false for null, empty set and "" alike (getFirstItem + isEmpty), each shape with its own case.
• verifyJwtIdentityReturnsFalseForSymmetricKeyBehindJwksUri seeds the resolver through ClientJwksResolverCache — the jwks_uri arm is covered without an HTTP server.
• The description now states the facts that were wrong before: SERVER_ERROR is a 400, AgentConfiguration.createAgent persists the schema defaults, and which endpoints skip them.

---

issue (blocking): The new catch misses JwsVerifyingException, so an RS256 assertion whose signature is not the registered RSA key's modulus length is still server_error with an error-level stack trace.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:689-695

RSASigningHandler.verify wraps the JDK's SignatureException("Bad signature length: got X but was expecting Y") in JwsVerifyingException — a sibling of JwsSigningException under JwsException, not a subclass — so it falls through to catch (Exception) → Utils.createException → logger.error(...) + server_error. Same on all three selectors (byJWKs, byX509Key, and byJWKsURI, whose JWKOpenIdResolverImpl.verifySignature has no catch around SignedJwt.verify). Measured at this head: registered 2048-bit JWK, assertion signed with a 3072-bit key → OAuthProblemException: server_error (400), Caused by: SignatureException: Bad signature length: got 384 but was expecting 256. That is any sender picking the error path with an arbitrary-length signature, and the realistic accident of a client rotating to a bigger key. The ECDSA handler wraps the same failure in JwsSigningException, so only the RSA path leaks. The description's "only failures to read the registration or to fetch the JWKS remain server_error" is false for this input.

import org.forgerock.json.jose.exceptions.JwsException;
...
} catch (JwsException | IllegalArgumentException e) {
    // Wrong key type, an alg the key cannot verify, or a signature the key cannot even
    // parse (RSA: wrong length): the client cannot be verified with what it registered.
    return false;
}

Pin: registered 2048-bit RSA JWK, assertion(clientId, signerWith3072BitKey, JwsAlgorithm.RS256, kid) → verifyJwtIdentity(...) is false. Red at this head, green with the catch widened.

---

issue (non-blocking): IdTokenInfo still parses the configured idTokenSignedResponseAlg with a strict valueOf, so a client configured hs256 gets server_error from /idtokeninfo for every token it was issued.

openam-oauth2/src/main/java/org/forgerock/openidconnect/restlet/IdTokenInfo.java:162

The reply says this parse "runs after the store has already issued with the upper-cased value" — but the value parsed here is the client's configured attribute, not the token's header, so the store's normalisation does not reach it. The IllegalArgumentException escapes validateIdToken(Representation)'s three catches and ExceptionHandler.handle(Throwable) turns it into a 400 server_error. Pre-existing; the same one-liner as verifyIdTokenIdentity closes it. Was leaving it a deliberate "pre-existing path stays"? If so, say so and this drops.

final JwsAlgorithm algorithm;
try {
    algorithm = JwsAlgorithm.valueOf(clientRegistration.getIDTokenSignedResponseAlgorithm().toUpperCase());
} catch (IllegalArgumentException e) {
    throw new BadRequestException("unsupported id_token_signed_response_alg");
}

---

suggestion (non-blocking): The → false arm logs nothing, so a client that stops authenticating after a key rotation leaves no server-side trace.

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/OpenAMClientRegistration.java:689-692

Round 2 objected to error per unauthenticated request, not to logging. One message-level line with the client id, the header alg and the exception is enough to tell "wrong key" from "no key".

} catch (JwsException | IllegalArgumentException e) {
    if (logger.messageEnabled()) {
        logger.message("Client {} assertion with alg {} cannot be verified with its registered key: {}",
                getClientId(), signatureAlgorithm, e.toString());
    }
    return false;
}

[vharseko]

Round 4 in cbe6fd9; PR description updated.

JwsVerifyingException past the catch — fixed: catch (JwsException | IllegalArgumentException). Confirmed in the 3.1.2 bytecode as you describe — RSASigningHandler.verify catches SignatureException and throws JwsVerifyingException, a sibling of JwsSigningException under JwsException, while ECDSASigningHandler throws JwsSigningException. Test: registered 2048-bit JWK, assertion signed with a 3072-bit key; at the previous head it failed with server_error, cause JwsVerifyingException: java.security.SignatureException: Bad signature length: got 384 but was expecting 256, now false. The description sentence you quoted is replaced: what stays server_error is what the client did not choose — reading the registration, parsing registered material, fetching the JWKS.

IdTokenInfo:162 strict valueOf — fixed, and not deliberate: my round-3 reply was wrong. The value parsed there is the client's configured attribute, so the store's upper-casing never reaches it; a client configured hs256 got server_error from /idtokeninfo for every token it was issued. Now toUpperCase() as the store does, IllegalArgumentException → BadRequestException("unsupported id_token_signed_response_alg"). Tests in IdTokenInfoTest: hs256 → the token comes back; not-an-alg → the bad request (at the previous head the first threw No enum constant ...hs256).

Logging on the → false arm — added as suggested: one logger.message line under messageEnabled() with the client id, the header alg and e.toString(), so "wrong key" and "no key" are distinguishable after a rotation without an error-level stack trace per unauthenticated request.