[Reliability] KeycloakAdminService.GetRoleMembersAsync news a per-call HttpClient (leak + inconsistent with the rest of the class) #435

New Issue

2026-07-06T03:18:28Z

spikerj commented

2026-07-06 03:18:28 +00:00

Problem

SpikerSoft.Business/Services/KeycloakAdminService.cs is a primary-constructor class that already injects HttpClient httpClient, and every method uses it with a per-request HttpRequestMessage carrying the Authorization header (e.g. GetAdminTokenAsync, AssignRealmRoleAsync, the availability checks — all httpClient.SendAsync(...)).

GetRoleMembersAsync (line ~444) is the lone outlier:

using var client = new HttpClient();
client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);

This news + disposes an HttpClient on every call (socket/TIME_WAIT leak, same family as #411/#433/#434) and mutates DefaultRequestHeaders — the anti-pattern the rest of the class deliberately avoids.

Fix

Use the injected httpClient with a per-request HttpRequestMessage + Authorization header, matching every sibling method. No ctor change; URL construction unchanged.

Acceptance

No new HttpClient() in KeycloakAdminService; GetRoleMembersAsync uses the injected client.
Returns the same role members (unit-testable via the existing Mock<HttpMessageHandler> harness — the stub sits on the injected client, so the test only passes once the injected client is used).

Refs #411 (resource-leak availability bombs).

## Problem `SpikerSoft.Business/Services/KeycloakAdminService.cs` is a primary-constructor class that **already injects `HttpClient httpClient`**, and every method uses it with a per-request `HttpRequestMessage` carrying the `Authorization` header (e.g. `GetAdminTokenAsync`, `AssignRealmRoleAsync`, the availability checks — all `httpClient.SendAsync(...)`). `GetRoleMembersAsync` (line ~444) is the lone outlier: ```csharp using var client = new HttpClient(); client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); ``` This news + disposes an `HttpClient` on every call (socket/`TIME_WAIT` leak, same family as #411/#433/#434) and mutates `DefaultRequestHeaders` — the anti-pattern the rest of the class deliberately avoids. ## Fix Use the injected `httpClient` with a per-request `HttpRequestMessage` + `Authorization` header, matching every sibling method. No ctor change; URL construction unchanged. ## Acceptance - No `new HttpClient()` in `KeycloakAdminService`; `GetRoleMembersAsync` uses the injected client. - Returns the same role members (unit-testable via the existing `Mock<HttpMessageHandler>` harness — the stub sits on the injected client, so the test only passes once the injected client is used). Refs #411 (resource-leak availability bombs).

spikerj commented

2026-07-06 03:27:33 +00:00

Resolved in spikersoft-backend PR #114 (merged to master). GetRoleMembersAsync now uses the injected HttpClient with a per-request Authorization header, matching every sibling method — no more per-call new HttpClient(). Guarded by KeycloakAdminServiceRoleMembersTests (stub on the injected client) + 21 existing Keycloak tests. This cleared the last non-benign new HttpClient() in the backend. Closing.

Resolved in spikersoft-backend PR #114 (merged to `master`). `GetRoleMembersAsync` now uses the injected `HttpClient` with a per-request `Authorization` header, matching every sibling method — no more per-call `new HttpClient()`. Guarded by `KeycloakAdminServiceRoleMembersTests` (stub on the injected client) + 21 existing Keycloak tests. This cleared the last non-benign `new HttpClient()` in the backend. Closing.

spikerj closed this issue

2026-07-06 03:27:34 +00:00

spikerj referenced this issue

2026-07-20 18:08:00 +00:00

Child's own profile drops 5 granted permissions — ProfileMapper.ToDto maps only 8 of 13 FeaturePermissions fields (child sees "Blog posting isn't turned on" despite parent grant) #736

spikerj referenced this issue

2026-07-21 03:00:40 +00:00

ProfileMapper.ToDto omits 5 permission fields — child's own blogPosting etc. always read as defaults #759

Sign in to join this conversation.