Skip to content

fix(sso): bound Microsoft Graph calls from the Entra ID authenticator - #3221

Open
marevol wants to merge 1 commit into
masterfrom
entraid-graph-timeout
Open

fix(sso): bound Microsoft Graph calls from the Entra ID authenticator#3221
marevol wants to merge 1 commit into
masterfrom
entraid-graph-timeout

Conversation

@marevol

@marevol marevol commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Problem

None of the four Microsoft Graph calls in EntraIdAuthenticator set a timeout.

curl4j's CurlRequest defaults both to -1:

protected int connectTimeout = -1;
protected int readTimeout = -1;
...
if (connectTimeout >= 0) { connection.setConnectTimeout(connectTimeout); }
if (readTimeout   >= 0) { connection.setReadTimeout(readTimeout); }

so setConnectTimeout / setReadTimeout are never called, which for
HttpURLConnection means wait forever. Fess sets no JVM-wide default
(sun.net.client.defaultReadTimeout etc.) either.

Where that hurts:

  • processDirectMemberOf() runs synchronously on the login request thread (called from the
    EntraIdUser constructor). An unresponsive Graph endpoint blocks that thread indefinitely.
  • The other three run on the TimeoutManager pool. That pool is
    ThreadPoolExecutor(nThreads, nThreads, ..., new LinkedBlockingQueue<>(nThreads), new CallerRunsPolicy())
    with nThreads = availableProcessors() / 2 (min 1), so once the queue fills, work runs on the
    caller — the single CoreLib-TimeoutManager scheduler thread.

Fix

  • graphConnectTimeout (10s) and graphReadTimeout (30s), with setters so they can be tuned
    through LastaDi like the other knobs on this component.
  • A createGraphRequest(CurlRequest, String accessToken) helper that applies the timeouts plus
    the Authorization and Accept headers, which all four sites repeated verbatim.

Only timeouts and the header de-duplication — no change to what is requested or how responses are
parsed.

Tests

EntraIdAuthenticator's unit test class, 2 new tests:

  • test_graphTimeouts_haveBoundedDefaults — guards against a non-positive default silently
    restoring the unbounded behaviour
  • test_createGraphRequest_stopsWaitingOnAnUnresponsiveEndpoint — binds a loopback ServerSocket
    that accepts and never answers, sets the read timeout to 300ms, and asserts the call gives up
    well under the 30s default

Falsification: with setGraphReadTimeout stubbed to ignore its argument, the second test fails
with took 30014ms instead of passing — so it distinguishes "timeout applied" from "default used"
as well as from "no timeout at all".

Tests run: 133, Failures: 0, Errors: 0, Skipped: 0

(the whole org.codelibs.fess.sso package)

None of the four Graph calls set a timeout. curl4j's CurlRequest defaults both
connectTimeout and readTimeout to -1 and only calls setConnectTimeout /
setReadTimeout when the value is >= 0, so every call waited forever, and Fess
sets no JVM-wide default either.

processDirectMemberOf() runs synchronously on the login request thread, so an
unresponsive Graph endpoint blocked that thread indefinitely. The other three
run on the TimeoutManager pool, whose queue is only as deep as its thread count
and whose saturation policy is CallerRunsPolicy, so a backlog of unbounded calls
ends up executing on the single scheduler thread.

Add graphConnectTimeout (10s) and graphReadTimeout (30s) with setters, and route
all four call sites through a createGraphRequest() helper that applies them
along with the Authorization and Accept headers those sites each repeated.
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