fix(auth): #543 whitespace-only configured key fails closed in the three pre-existing gRPC auth interceptors
TelemetryStreamAuthInterceptor, ConfigServeAuthInterceptor and LocalDbSyncAuthInterceptor all decided "is a key configured?" with string.IsNullOrEmpty. A whitespace-only configured value therefore passed that guard and fell through to the constant-time comparison — where it is satisfied by a whitespace-only bearer token. The gate would be open to anyone who guessed a blank credential. This backports the check the two NEW mesh interceptors already use: MeshCommandAuthInterceptor (and its central-side clone) validate with string.IsNullOrWhiteSpace, so the four/five planes now answer the same way to the same misconfiguration. Nothing else changes — the denial status codes (PermissionDenied / Unauthenticated / PermissionDenied respectively) and every denial message are untouched, and no key material is logged. No deployment configures a whitespace key: the docker-dev rig and production both set real keys, so this is a fail-closed hardening of an unreachable-today misconfiguration rather than a live exposure. Tests: each of the three test files gains a ..._WithBlankKeyConfigured_IsDenied fact mirroring MeshCommandAuthInterceptorTests.ExecuteMethod_WithBlankPskConfigured_IsDenied (configured key " ", presented "Bearer "), verified red before the fix. ConfigServeAuthInterceptor and LocalDbSyncAuthInterceptor also gain the HasExactlyOnePublicConstructor pin their two siblings already had — Grpc.AspNetCore silently stops invoking an interceptor with more than one public constructor, which would disable auth without failing anything. Claude-Session: https://claude.ai/code/session_014WNM4vjoVksyyBraTXSZE1
This commit is contained in:
@@ -110,7 +110,9 @@ public sealed class ConfigServeAuthInterceptor : Interceptor
|
||||
return;
|
||||
|
||||
var expected = _options.Value.ApiKey;
|
||||
if (string.IsNullOrEmpty(expected))
|
||||
// Whitespace-only counts as unconfigured: an all-blank expected key would otherwise be
|
||||
// satisfied by an all-blank bearer token, which is no gate at all.
|
||||
if (string.IsNullOrWhiteSpace(expected))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"Rejected a config-serve call to {Method}: no ConfigServe:ApiKey is configured, so the " +
|
||||
|
||||
@@ -115,7 +115,9 @@ public sealed class LocalDbSyncAuthInterceptor : Interceptor
|
||||
return;
|
||||
|
||||
var expected = _options.Value.ApiKey;
|
||||
if (string.IsNullOrEmpty(expected))
|
||||
// Whitespace-only counts as unconfigured: an all-blank expected key would otherwise be
|
||||
// satisfied by an all-blank bearer token, which is no gate at all.
|
||||
if (string.IsNullOrWhiteSpace(expected))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"Rejected a LocalDb sync call to {Method}: no LocalDb:Replication:ApiKey is configured, " +
|
||||
|
||||
@@ -117,7 +117,9 @@ public sealed class TelemetryStreamAuthInterceptor : Interceptor
|
||||
return;
|
||||
|
||||
var expected = _options.Value.ApiKey;
|
||||
if (string.IsNullOrEmpty(expected))
|
||||
// Whitespace-only counts as unconfigured: an all-blank expected key would otherwise be
|
||||
// satisfied by an all-blank bearer token, which is no gate at all.
|
||||
if (string.IsNullOrWhiteSpace(expected))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"Rejected a telemetry stream call to {Method}: no Telemetry:ApiKey is configured, so " +
|
||||
|
||||
+23
@@ -65,6 +65,19 @@ public sealed class ConfigServeAuthInterceptorTests
|
||||
ex.StatusCode.ShouldBe(StatusCode.Unauthenticated);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task FetchMethod_WithBlankKeyConfigured_IsDenied()
|
||||
{
|
||||
// Blank is distinct from unset in configuration (a stray-whitespace appsettings value) but
|
||||
// must be treated identically here: an all-blank expected key would be satisfied by an
|
||||
// all-blank bearer. Pinned identically on MeshCommandAuthInterceptor.
|
||||
var interceptor = CreateInterceptor(" ");
|
||||
var context = CreateContext(FetchMethod, "Bearer ");
|
||||
|
||||
var ex = await Should.ThrowAsync<RpcException>(() => Invoke(interceptor, context));
|
||||
ex.StatusCode.ShouldBe(StatusCode.Unauthenticated);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task FetchMethod_WithNoBearerToken_IsDenied()
|
||||
{
|
||||
@@ -121,6 +134,16 @@ public sealed class ConfigServeAuthInterceptorTests
|
||||
ex.StatusCode.ShouldBe(StatusCode.Unauthenticated);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HasExactlyOnePublicConstructor()
|
||||
{
|
||||
// Grpc.AspNetCore silently stops invoking an interceptor with more than one public ctor —
|
||||
// this pins the shape so a well-meaning overload doesn't quietly disable auth in production.
|
||||
var publicCtors = typeof(ConfigServeAuthInterceptor).GetConstructors();
|
||||
|
||||
publicCtors.Length.ShouldBe(1);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Minimal <see cref="ServerCallContext"/> carrying just a method name and request headers —
|
||||
/// the only two things the interceptor reads. Hand-rolled because
|
||||
|
||||
+23
@@ -99,6 +99,19 @@ public sealed class LocalDbSyncAuthInterceptorTests
|
||||
ex.StatusCode.ShouldBe(StatusCode.PermissionDenied);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SyncMethod_WithBlankKeyConfigured_IsDenied()
|
||||
{
|
||||
// Blank is distinct from unset in configuration (a stray-whitespace appsettings value) but
|
||||
// must be treated identically here: an all-blank expected key would be satisfied by an
|
||||
// all-blank bearer. Pinned identically on MeshCommandAuthInterceptor.
|
||||
var interceptor = CreateInterceptor(" ");
|
||||
var context = CreateContext(SyncMethod, "Bearer ");
|
||||
|
||||
var ex = await Should.ThrowAsync<RpcException>(() => Invoke(interceptor, context));
|
||||
ex.StatusCode.ShouldBe(StatusCode.PermissionDenied);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SyncMethod_WithNoBearerToken_IsDenied()
|
||||
{
|
||||
@@ -152,6 +165,16 @@ public sealed class LocalDbSyncAuthInterceptorTests
|
||||
ex.StatusCode.ShouldBe(StatusCode.PermissionDenied);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HasExactlyOnePublicConstructor()
|
||||
{
|
||||
// Grpc.AspNetCore silently stops invoking an interceptor with more than one public ctor —
|
||||
// this pins the shape so a well-meaning overload doesn't quietly disable auth in production.
|
||||
var publicCtors = typeof(LocalDbSyncAuthInterceptor).GetConstructors();
|
||||
|
||||
publicCtors.Length.ShouldBe(1);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task DuplexStreaming_IsGated_BecauseThatIsHowSyncActuallyRuns()
|
||||
{
|
||||
|
||||
+13
@@ -65,6 +65,19 @@ public sealed class TelemetryStreamAuthInterceptorTests
|
||||
ex.StatusCode.ShouldBe(StatusCode.PermissionDenied);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SubscribeMethod_WithBlankKeyConfigured_IsDenied()
|
||||
{
|
||||
// Blank is distinct from unset in configuration (a stray-whitespace appsettings value) but
|
||||
// must be treated identically here: an all-blank expected key would be satisfied by an
|
||||
// all-blank bearer. Pinned identically on MeshCommandAuthInterceptor.
|
||||
var interceptor = CreateInterceptor(" ");
|
||||
var context = CreateContext(SubscribeMethod, "Bearer ");
|
||||
|
||||
var ex = await Should.ThrowAsync<RpcException>(() => Invoke(interceptor, context));
|
||||
ex.StatusCode.ShouldBe(StatusCode.PermissionDenied);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SubscribeMethod_WithNoBearerToken_IsDenied()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user