Skip to content

release: structures 3.8.0 - #28

Merged
NickPadilla merged 23 commits into
mainfrom
develop
Oct 10, 2026
Merged

NickPadilla merged 23 commits into
mainfrom
develop

Conversation

@NickPadilla

Copy link
Copy Markdown
Member

OIDC logins no longer hang when an identity provider stops answering, conflicting writes answer 409 instead of 500, and the UI shows which tenant you're signed into.

  • OIDC JWKS resilience (fix: OIDC JWKS fetching: timeouts, outage-tolerant caching, issuer allow-list #25): a stalled identity provider could hang authentication for 15+ minutes, because JWKS fetches had no timeout and reused pooled connections that a NAT or load balancer had silently dropped.
    • Every discovery and JWKS fetch has a connect timeout and an end-to-end timeout, and connections aren't pooled.
    • A token's iss must equal the authority of an enabled provider. Otherwise it fails with Issuer not allowed: <iss> before anything is fetched.
    • Through an IdP outage the last fetched keys keep working by default. jwks-max-staleness can cap that.
    • Concurrent lookups share one fetch, and unknown kids refresh at most once per cooldown.
    • New oidc-security-service.jwks-* settings, also in Helm.
    • /app-config.override.json now serves only the fields the UI reads.
    • Fixed a Helm bug: an unset rolesClaimPath rendered as "" and rejected every token.
  • Version conflicts answer 409 (fix: 409 for version conflicts, and show the signed-in tenant #26): a write that lost an optimistic locking race (@Version) reached callers as a 500, with the Elasticsearch host and request URI in the message.
    • Save, update, bulk save and bulk update now fail with the new VersionConflictException: REST answers 409, the TS client rejects with a message starting Version conflict writing <Structure>, and Java callers can catch the type.
    • The message carries Elasticsearch's reason, without the host or URI, and says what to do. On versioned structures: re-read for the current version. On streams: the id already exists. Without @Version, two racing updates conflict too, and the message says to send the request again. Structures doesn't retry those itself, since that would apply the change over the other write silently.
    • Bulk calls answer 409 when every failed item was a conflict, with the count of items not written and every reason listed. The items that didn't conflict are written.
    • The OpenAPI spec documents the 409s.
  • Tenant in the header (fix: 409 for version conflicts, and show the signed-in tenant #26): the UI shows the tenant the server resolved for the session, and the avatar menu shows the user and tenant.
  • 3.8.0 (build: version 3.8.0, with docker-compose, KinD and Helm pointed at it #27): a minor version, for the new OIDC settings and the new public exception. docker-compose, the KinD values and the Helm chart (appVersion, image tags, chart 1.2.0) point at 3.8.0.

Upgrade notes

  • Check every OIDC provider's authority before deploying. It must match the iss claim in that provider's tokens exactly, scheme, host, port and path included. A mismatch now fails every login from that provider with Issuer not allowed.
  • Callers that caught 500 on a version conflict now get 409 over REST. Over STOMP the message changes: it still contains version conflict, but now starts with Version conflict writing <Structure> instead of the raw Elasticsearch text.
  • Bulk calls with only version conflicts answer 409 where they answered 400. Mixed failures still answer 400.
  • No npm publish. The JS libraries didn't change; the UI change ships in the server image.

Known gaps, recorded in docs/performance/ELASTICSEARCH_WORKLOAD.md

  • Bulk items rejected under load (429) or with a 5xx still make the call answer 400.
  • Bulk callers get no structured list of failed items, only the reasons in the message.
  • Other low-level Elasticsearch errors on a single write (429, 503) still answer 500 with the host and URI in the message.

Testing

🤖 Generated with Claude Code

NickPadilla and others added 23 commits October 7, 2026 16:05
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Authentication could hang for 15+ minutes when an OIDC provider stopped
answering. The JWKS client had no response timeout and reused pooled
connections, so a request written to a connection silently dropped by a
NAT or load balancer waited on TCP retransmission before failing.

- Only fetch for issuers that match an enabled provider's authority;
  tokens from other issuers are rejected before any network call
- Connect and end-to-end timeouts on every fetch, configurable, and no
  connection pooling for these rare fetches
- Failed fetches, empty key sets and discovery documents without a
  jwks_uri are not cached; concurrent lookups share one fetch
- Cache the whole key set; an unknown kid refetches at most once per
  refresh cooldown, so rotation works without a fetch per bad token
- Match .local on the host only for the insecure dev client
- Optional per-provider jwks-uri for when the authority's public URL is
  not reachable from the cluster

JwksResilienceTest covers each case against a fake IdP.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-ups on the JWKS fetch changes:

- Key sets and discovery documents are refreshed with Caffeine's
  refreshAfterWrite instead of expiring. Lookups are served the cached
  document during a refresh, and a failed refresh keeps it, so a token
  with a made up kid, or an expired discovery entry, during an IdP
  outage no longer breaks valid logins
- An unknown kid refreshes via cache.refresh(), deduplicated, and the
  cooldown also limits failed refreshes
- jwksUri is @JsonIgnore, so an in-cluster IdP URL is not published in
  the unauthenticated frontend configuration
- Issuer to provider matching lives in one place,
  OidcSecurityServiceProperties.findEnabledProviders
- Build the JWK parser once; drop the warn that duplicated Caffeine's
  own log of failed loads
- Shut down each fake IdP's executor in JwksResilienceTest

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The frontend configuration endpoint is unauthenticated and serialized
the whole OidcSecurityServiceProperties, so backend-only settings
(audience, required roles, roles claim path, allowAnyDomain, jwksUri,
tenantIdFieldName and the JWKS fetch settings) were published with it.

It now serves OidcFrontendConfiguration, an allow-list of the fields the
frontend reads, for enabled providers only. A new setting is not sent
unless it is added there, so the @JsonIgnore annotations are dropped.
No frontend change is needed: it never read the removed fields.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…owed issuers

Second review follow-ups:

- Tokens with an unknown kid within the cooldown of a refresh now use
  that refresh, so a burst of tokens signed with a rotated key all wait
  on one fetch instead of all but the first being rejected
- A failed refresh keeps the cached copy until the next refresh is due,
  instead of every lookup refetching; the 24 h key set staleness bound
  is checked in getKey since that resets Caffeine's write time
- After a fetch fails with nothing cached, lookups fail with its error
  for jwks-retry-backoff (5 s) before fetching again
- A token whose issuer is not the authority of an enabled provider,
  unknown or disabled, fails with AuthenticationException "Issuer not
  allowed: <iss>". OidcSecurityService completes with the original
  error rather than a CompletionException, so the gateway reports it
- Disabled providers are sent to the frontend again, enabled false,
  since its basic auth toggle counts configured providers
- Log rejected unknown key ids; Helm and README cover jwksUri and the
  fetch settings; fix the refreshAfterWrite javadoc; small cleanups

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- tenantIdFieldName, and per provider allowAnyDomain, frontEndRoles and
  additionalScopes, each rendered only when set
- rolesClaimPath is rendered only when set too. Left empty it rendered
  as "", which OidcSecurityService took as a roles path, so every token
  was rejected with "No roles found in claims". A blank path now counts
  as unset in OidcSecurityService as well
- values.yaml documents each option, and that provider metadata cannot
  be set this way: Spring cannot bind map entries from environment
  variables under the oidc-security-service prefix

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Send the frontend only the endpoint keys of provider metadata, which
  the backend also merges into every Participant; none sends null, as
  the frontend treats any metadata object as endpoint overrides
- A failed refresh for an unknown kid reports the fetch error instead
  of "key not found"
- Check key set staleness before the lookup and compare by value, so a
  failed refresh's new future cannot bypass the 24 h bound and an idle
  deployment does not fetch twice
- Rejections of unverified tokens log at warn at most once a minute,
  with the token's claims shortened and control characters replaced
- Drop the Accept header, as before, so strict JWKS endpoints are not
  refused; trust-all dev TLS also covers *.localhost
- Start unknown-kid refreshes outside ConcurrentHashMap.compute; one
  shared CompletionException unwrap
- Helm refuses JWKS durations without a unit, since Spring would read
  a bare number as milliseconds

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Compared with other JWKS clients, the 24 h staleness cap sat on the
strict side: a network problem lasting a day rejected tokens that were
still valid. Follow Microsoft.IdentityModel and go-oidc instead.

- The last fetched keys are used for as long as an outage lasts; the
  new jwks-max-staleness caps that for deployments that prefer to fail
  closed. A failed refresh logs the age of the keys it keeps
- jwks-refresh-interval (1 h) makes the routine refresh configurable
- The unknown kid cooldown counts only from refreshes unknown key ids
  started, so a rotation just after a routine fetch is picked up
- Tests for an outage with and without the cap, a failed routine
  refresh retried after the interval, keys published ahead of use, and
  a rotation right after a fetch
- Docs: behaviour per situation, comparison with other libraries, and
  rotating Keycloak keys without rejected logins; README and Helm cover
  the new settings

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Evict stale key sets with computeIfPresent: asMap().remove(key,
  value) reads the entry, which started the refresh the eviction is
  meant to avoid, so a down IdP was asked twice per lookup
- A refresh an unknown kid starts is always used, so a zero cooldown no
  longer rejects every rotated key; fetch durations are validated at
  startup instead of failing at the first lookup
- Rate-limit logging of failed JWT validation, which anyone can trigger
  with a forged signature, through the same RejectionLog as the JWKS
  rejections; log JWKS keys that cannot be parsed at warn
- Skip null provider metadata values instead of failing the frontend
  configuration endpoint
- Helm accepts durations in any case, as Spring does
- Correct the refreshAfterWrite javadoc on what a failed refresh does

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The discovery document was the last fetch on a hardcoded interval
(24 h). It now refreshes on jwks-refresh-interval like key sets, so one
setting controls how often Structures refreshes from the IdP. It still
has no maximum age: a failed refresh keeps the cached document.

Tests cover one lookup refreshing both, and a failed discovery refresh
leaving key lookups working.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A review of the JWT -> discovery document -> JWKS key flow, and of the
docs against the code:

- RejectionLog sanitizes every string it logs. JJWT copies the token's
  alg header into its error message, so a forged alg could put a
  newline into the log. One rate-limit window is shared by all
  rejection logs
- jwks-max-staleness must be longer than jwks-refresh-interval; shorter,
  healthy keys were evicted before a refresh was due, so logins waited
  on a fetch every time
- An unknown kid refresh that throws while starting completes its
  shared future, so lookups waiting on it cannot hang; drop the
  unreachable null branch
- A successful load clears the previous refresh error, so it is not
  reported for a later missing key; the refresh failure is logged with
  the rejection
- Docs: staleness counted from the last successful fetch, lazy refresh
  on idle pods, a changed jwks_uri loading without fallback, what
  clients see, and the components section and tuning notes

Tests: forged alg in the log, a changed jwks_uri used and failing,
discovery retry backoff, a lookup not waiting on either refresh, the
staleness/interval check, and an exact unknown kid fetch count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Found in the KinD smoke test: for a discovery document, which has no
age, the warning read "Keeping the cached OIDC discovery document for
<issuer> refreshing it failed".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix: OIDC JWKS fetching: timeouts, outage-tolerant caching, issuer allow-list
A write that lost an optimistic locking race reached callers as a 500.
The async client hands a 409 back as the low level ResponseException,
since the transport only asks the low level client to pass 400-405
through. Its message also named the Elasticsearch host and request URI,
which STOMP callers saw.

Save, update, bulkSave and bulkUpdate now turn a 409 into the new
VersionConflictException, with the reason read from the response body.
The REST failure handler maps it to 409, and the OpenAPI spec documents
409 on the write operations of versioned and stream structures. A bulk
request fails with a conflict only when every failed item conflicted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Shared structures only show the data of the session's tenant, so a user
signed into the wrong tenant saw an empty or unexpected view with no
clue why. The header now shows the tenant the server resolved for the
session, and the avatar menu shows the user and tenant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A 409 also happens without a version field, when another write changes
an entity while an update is applied. Elasticsearch is still not asked
to retry it, since that would apply the partial update over the other
write without the caller knowing; the message says to send the request
again instead of to read a version the entity does not have. The
OpenAPI spec lists 409 on update and bulk update for every Structure.

The message always says "version conflict", even when no reason could
be read. A bulk request is a conflict only when it has conflicts and
nothing else failed, and the message counts the items not written.

The tenant badge tooltip now just says "Your tenant".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Found in the review of PR #26: a bulk call with items Elasticsearch
rejected under load (429) or failed with a 5xx answers 400, so clients
treat it as bad data and don't retry. Noted in the write throttling
plan, with the status each kind of failure should map to.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Stream structures only create, so their conflicts now say the id
  already exists instead of telling the caller to re-read a version.
- A bulk conflict without item reasons still says "version conflict",
  and the reasons and the advice no longer run together.
- Bulk errors list at most 20 reasons plus "and N more", and a set
  finds duplicates, so a large call no longer builds a quadratic,
  multi-megabyte message.
- The reason is parsed from the response bytes, since Elasticsearch's
  compatibility content type names no charset and the string fallback
  was ISO-8859-1.

Also records, in the write throttling doc, that non-409 errors from
the low level client still reach callers with the Elasticsearch host
and URI in the message. That fix is held for its own PR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The 20-reason cap from the last commit meant callers could not tell
which items past the first 20 failed. They find failed items by
reading the document ids out of the reasons, so every distinct reason
is listed again, as before this branch. A set still drops duplicates.

The throttling doc records the redesign: telling bulk callers which
items failed with a structured result instead of a message to parse.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Over STOMP: a save without a version for an existing id, a bulk update
where one of three items is stale (the other two are written), and a
bulk save of existing ids. Over REST: a bulk update with a stale item
answers 409. The STOMP cases match the message from its start, which
shows the client gets the exception's own message, not a wrapper's.

Run against KinD with this branch's image: 11 of 11 pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-tenant-badge

fix: 409 for version conflicts, and show the signed-in tenant
A minor version: #25 adds public configuration (the oidc-security-service
jwks-* settings, also in Helm), and #26 adds the public
VersionConflictException and answers 409 where callers got 500.
The Helm chart moves to 1.2.0 for the new OIDC options.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
build: version 3.8.0, with docker-compose, KinD and Helm pointed at it
@NickPadilla
NickPadilla merged commit c338552 into main Oct 10, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant