From 14fda3f80b478f3cf0bb3c04eee5ae3bfc9d24fd Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Thu, 26 Feb 2026 16:58:06 -0800 Subject: [PATCH 1/4] Addressed comments --- src/Core/Resolvers/MsSqlQueryExecutor.cs | 133 ++++++++++- .../UnitTests/SqlQueryExecutorUnitTests.cs | 206 ++++++++++++++++++ 2 files changed, 335 insertions(+), 4 deletions(-) diff --git a/src/Core/Resolvers/MsSqlQueryExecutor.cs b/src/Core/Resolvers/MsSqlQueryExecutor.cs index a6684144c1..688c1fa189 100644 --- a/src/Core/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Core/Resolvers/MsSqlQueryExecutor.cs @@ -12,6 +12,7 @@ using Azure.DataApiBuilder.Core.Authorization; using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; +using Azure.DataApiBuilder.Product; using Azure.DataApiBuilder.Service.Exceptions; using Azure.Identity; using Microsoft.AspNetCore.Http; @@ -69,6 +70,13 @@ public override IDictionary ConnectionStringB /// private Dictionary _dataSourceUserDelegatedAuth; + /// + /// DatasourceName to base Application Name for OBO per-user pooling. + /// Only populated for data sources with user-delegated-auth enabled. + /// Used as a prefix when constructing user-specific Application Names. + /// + private Dictionary _dataSourceBaseAppName; + /// /// Optional OBO token provider for user-delegated authentication. /// @@ -94,6 +102,7 @@ public MsSqlQueryExecutor( _dataSourceAccessTokenUsage = new Dictionary(); _dataSourceToSessionContextUsage = new Dictionary(); _dataSourceUserDelegatedAuth = new Dictionary(); + _dataSourceBaseAppName = new Dictionary(); _accessTokensFromConfiguration = runtimeConfigProvider.ManagedIdentityAccessToken; _runtimeConfigProvider = runtimeConfigProvider; _oboTokenProvider = oboTokenProvider; @@ -114,9 +123,11 @@ public override SqlConnection CreateConnection(string dataSourceName) throw new DataApiBuilderException("Query execution failed. Could not find datasource to execute query against", HttpStatusCode.BadRequest, DataApiBuilderException.SubStatusCodes.DataSourceNotFound); } + string connectionString = GetConnectionStringForCurrentUser(dataSourceName); + SqlConnection conn = new() { - ConnectionString = ConnectionStringBuilders[dataSourceName].ConnectionString, + ConnectionString = connectionString, }; // Extract info message from SQLConnection @@ -150,6 +161,116 @@ public override SqlConnection CreateConnection(string dataSourceName) return conn; } + /// + /// Gets the connection string for the current user. For OBO-enabled data sources, + /// this returns a connection string with a user-specific Application Name to isolate + /// connection pools per user identity. + /// + /// The name of the data source. + /// The connection string to use for the current request. + private string GetConnectionStringForCurrentUser(string dataSourceName) + { + string baseConnectionString = ConnectionStringBuilders[dataSourceName].ConnectionString; + + // Per-user pooling is automatic when OBO is enabled. + // _dataSourceBaseAppName is only populated for data sources with user-delegated-auth enabled. + if (!_dataSourceBaseAppName.TryGetValue(dataSourceName, out string? baseAppName)) + { + // OBO not enabled for this data source, use the standard connection string + return baseConnectionString; + } + + // Extract user pool key from current HTTP context (prefers oid, falls back to sub) + string? poolKeyHash = GetUserPoolKeyHash(dataSourceName); + if (string.IsNullOrEmpty(poolKeyHash)) + { + // For OBO-enabled data sources, we must have a user context for actual requests. + // Null poolKeyHash is only acceptable during startup/metadata phase when there's no HttpContext. + // If we have an HttpContext with a User but missing required claims, fail-safe to prevent + // potential cross-user connection pool contamination. + if (HttpContextAccessor?.HttpContext?.User?.Identity?.IsAuthenticated == true) + { + throw new DataApiBuilderException( + message: "User-delegated authentication requires 'iss' and user identifier (oid/sub) claims for connection pool isolation.", + statusCode: System.Net.HttpStatusCode.Unauthorized, + subStatusCode: DataApiBuilderException.SubStatusCodes.OboAuthenticationFailure); + } + + // No user context (startup/metadata phase), use base connection string + return baseConnectionString; + } + + // Create a user-specific connection string by appending to the existing Application Name. + // baseAppName preserves the customer's original Application Name. + // We append |obo:{hash} to create a separate connection pool for each unique user. + // Format: {existingAppName}|obo:{hash} + SqlConnectionStringBuilder userBuilder = new(baseConnectionString) + { + ApplicationName = $"{baseAppName}|obo:{poolKeyHash}", + Pooling = true + }; + + return userBuilder.ConnectionString; + } + + /// + /// Generates a pool key hash from the current user's claims for OBO per-user pooling. + /// Uses iss|(oid||sub) to ensure each unique user identity gets its own connection pool. + /// Prefers 'oid' (stable GUID) but falls back to 'sub' for guest/B2B users. + /// + /// The data source name for logging purposes. + /// A URL-safe Base64-encoded hash, or null if no user context is available. + private string? GetUserPoolKeyHash(string dataSourceName) + { + if (HttpContextAccessor?.HttpContext?.User is null) + { + return null; + } + + ClaimsPrincipal user = HttpContextAccessor.HttpContext.User; + + // Extract issuer claim - required for tenant isolation and connection pool security. + // The "iss" claim must be present along with a user identifier (oid/sub) for per-user pooling. + // Callers are responsible for enforcing fail-safe behavior when claims are missing. + string? iss = user.FindFirst("iss")?.Value; + + // Prefer oid (stable GUID), fall back to sub for guest/B2B users + string? userKey = user.FindFirst("oid")?.Value + ?? user.FindFirst("http://schemas.microsoft.com/identity/claims/objectidentifier")?.Value + ?? user.FindFirst("sub")?.Value + ?? user.FindFirst(ClaimTypes.NameIdentifier)?.Value; + + if (string.IsNullOrEmpty(iss) || string.IsNullOrEmpty(userKey)) + { + // Cannot create a pool key without both claims + QueryExecutorLogger.LogDebug( + "Cannot create per-user pool key for data source {DataSourceName}: missing {MissingClaim} claim.", + dataSourceName, + string.IsNullOrEmpty(iss) ? "iss" : "user identifier (oid/sub)"); + return null; + } + + // Create the pool key as iss|userKey and hash it to keep connection string small + string poolKey = $"{iss}|{userKey}"; + return HashPoolKey(poolKey); + } + + /// + /// Hashes the pool key using SHA512 to create a compact, URL-safe identifier. + /// This keeps the Application Name reasonably short while ensuring uniqueness. + /// + /// The pool key to hash (format: iss|oid or iss|sub). + /// A URL-safe Base64-encoded hash of the key. + private static string HashPoolKey(string key) + { + using var sha = System.Security.Cryptography.SHA512.Create(); + byte[] bytes = sha.ComputeHash(System.Text.Encoding.UTF8.GetBytes(key)); + return Convert.ToBase64String(bytes) + .TrimEnd('=') + .Replace('+', '-') + .Replace('/', '_'); + } + /// /// Configure during construction or a hot-reload scenario. /// @@ -177,9 +298,13 @@ private void ConfigureMsSqlQueryExecutor() { _dataSourceUserDelegatedAuth[dataSourceName] = dataSource.UserDelegatedAuth!; - // Disable connection pooling for OBO connections since each connection - // uses a user-specific token and cannot be shared across users - builder.Pooling = false; + // Per-user pooling: Keep pooling enabled but store the base Application Name. + // At connection time, we'll append the user's iss:sub hash to create isolated pools per user. + // This is automatic for OBO to prevent connection exhaustion while ensuring pool isolation. + // Note: ApplicationName is typically already set by RuntimeConfigLoader (e.g., "CustomerApp,dab_oss_2.0.0") + // but we use GetDataApiBuilderUserAgent() as fallback for consistency. + _dataSourceBaseAppName[dataSourceName] = builder.ApplicationName ?? ProductInfo.GetDataApiBuilderUserAgent(); + builder.Pooling = true; } } } diff --git a/src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs b/src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs index 573ac4c8f4..67315dc736 100644 --- a/src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs @@ -671,6 +671,212 @@ public void ValidateStreamingLogicForEmptyCellsAsync() Assert.AreEqual(availableSize, (int)runtimeConfig.MaxResponseSizeMB() * 1024 * 1024); } + #region Per-User Connection Pooling Tests + + /// + /// Creates MsSqlQueryExecutor with the specified configuration for per-user connection pooling tests. + /// + /// The connection string to use. + /// Whether to enable user-delegated-auth (OBO). + /// The HttpContextAccessor mock to use. + /// A tuple containing the query executor and runtime config provider. + private static (MsSqlQueryExecutor QueryExecutor, RuntimeConfigProvider Provider) CreateQueryExecutorForPoolingTest( + string connectionString, + bool enableObo, + Mock httpContextAccessor) + { + DataSource dataSource = new( + DatabaseType: DatabaseType.MSSQL, + ConnectionString: connectionString, + Options: null) + { + UserDelegatedAuth = enableObo + ? new UserDelegatedAuthOptions( + Enabled: true, + Provider: "EntraId", + DatabaseAudience: "https://database.windows.net") + : null + }; + + RuntimeConfig mockConfig = new( + Schema: "", + DataSource: dataSource, + Runtime: new( + Rest: new(), + GraphQL: new(), + Mcp: new(), + Host: new(null, null) + ), + Entities: new(new Dictionary())); + + MockFileSystem fileSystem = new(); + fileSystem.AddFile(FileSystemRuntimeConfigLoader.DEFAULT_CONFIG_FILE_NAME, new MockFileData(mockConfig.ToJson())); + FileSystemRuntimeConfigLoader loader = new(fileSystem); + RuntimeConfigProvider provider = new(loader); + + Mock>> queryExecutorLogger = new(); + DbExceptionParser dbExceptionParser = new MsSqlDbExceptionParser(provider); + + MsSqlQueryExecutor queryExecutor = new(provider, dbExceptionParser, queryExecutorLogger.Object, httpContextAccessor.Object); + return (queryExecutor, provider); + } + + /// + /// Creates an HttpContextAccessor mock with the specified user claims. + /// + /// The issuer claim value, or empty string for no context. + /// The oid claim value, or empty string for no context. + /// A configured HttpContextAccessor mock. + private static Mock CreateHttpContextAccessorWithClaims(string issuer, string objectId) + { + Mock httpContextAccessor = new(); + + if (string.IsNullOrEmpty(issuer) && string.IsNullOrEmpty(objectId)) + { + httpContextAccessor.Setup(x => x.HttpContext).Returns(value: null); + } + else + { + DefaultHttpContext context = new(); + System.Security.Claims.ClaimsIdentity identity = new("TestAuth"); + if (!string.IsNullOrEmpty(issuer)) + { + identity.AddClaim(new System.Security.Claims.Claim("iss", issuer)); + } + + if (!string.IsNullOrEmpty(objectId)) + { + identity.AddClaim(new System.Security.Claims.Claim("oid", objectId)); + } + + context.User = new System.Security.Claims.ClaimsPrincipal(identity); + httpContextAccessor.Setup(x => x.HttpContext).Returns(context); + } + + return httpContextAccessor; + } + + /// + /// Test that pooling remains enabled regardless of whether user-delegated-auth is configured. + /// When OBO is enabled, per-user pooling is automatic. When disabled, pooling stays as configured. + /// + [DataTestMethod, TestCategory(TestCategory.MSSQL)] + [DataRow(true, DisplayName = "OBO enabled - per-user pooling is automatic")] + [DataRow(false, DisplayName = "OBO disabled - pooling not modified")] + public void TestPoolingBehaviorWithAndWithoutUserDelegatedAuth(bool enableUserDelegatedAuth) + { + // Arrange & Act + Mock httpContextAccessor = new(); + (MsSqlQueryExecutor queryExecutor, RuntimeConfigProvider provider) = CreateQueryExecutorForPoolingTest( + connectionString: "Server=localhost;Database=test;Pooling=true;", + enableObo: enableUserDelegatedAuth, + httpContextAccessor: httpContextAccessor); + + SqlConnectionStringBuilder connBuilder = new(queryExecutor.ConnectionStringBuilders[provider.GetConfig().DefaultDataSourceName].ConnectionString); + + // Assert - pooling should be enabled in both cases + string expectedMessage = enableUserDelegatedAuth + ? "Pooling should be enabled when user-delegated-auth is configured (per-user pooling is automatic)" + : "Pooling should remain as configured when user-delegated-auth is not used"; + Assert.IsTrue(connBuilder.Pooling, expectedMessage); + } + + /// + /// Test that when OBO is enabled and user claims are present, CreateConnection returns + /// a connection string with a user-specific Application Name containing the pool hash. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public void TestOboWithUserClaims_ConnectionStringHasUserSpecificAppName() + { + // Arrange & Act + Mock httpContextAccessor = CreateHttpContextAccessorWithClaims( + issuer: "https://login.microsoftonline.com/tenant-id/v2.0", + objectId: "user-object-id-12345"); + + (MsSqlQueryExecutor queryExecutor, RuntimeConfigProvider provider) = CreateQueryExecutorForPoolingTest( + connectionString: "Server=localhost;Database=test;Application Name=TestApp;", + enableObo: true, + httpContextAccessor: httpContextAccessor); + + SqlConnection conn = queryExecutor.CreateConnection(provider.GetConfig().DefaultDataSourceName); + SqlConnectionStringBuilder connBuilder = new(conn.ConnectionString); + + // Assert - Application Name should contain the base name plus |obo: and a hash + // Note: The actual format includes version suffix, e.g., "TestApp,dab_oss_2.0.0|obo:{hash}" + Assert.IsTrue(connBuilder.ApplicationName.Contains("|obo:"), + $"Application Name should contain '|obo:' but was '{connBuilder.ApplicationName}'"); + Assert.IsTrue(connBuilder.ApplicationName.StartsWith("TestApp"), + $"Application Name should start with 'TestApp' but was '{connBuilder.ApplicationName}'"); + Assert.IsTrue(connBuilder.Pooling, "Pooling should be enabled"); + } + + /// + /// Test that different users get different pool hashes (different Application Names). + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public void TestObo_DifferentUsersGetDifferentPoolHashes() + { + // Arrange & Act - User 1 + Mock httpContextAccessor1 = CreateHttpContextAccessorWithClaims( + issuer: "https://login.microsoftonline.com/tenant-id/v2.0", + objectId: "user1-oid-aaaa"); + + (MsSqlQueryExecutor queryExecutor1, RuntimeConfigProvider provider) = CreateQueryExecutorForPoolingTest( + connectionString: "Server=localhost;Database=test;Application Name=DAB;", + enableObo: true, + httpContextAccessor: httpContextAccessor1); + + SqlConnection conn1 = queryExecutor1.CreateConnection(provider.GetConfig().DefaultDataSourceName); + SqlConnectionStringBuilder connBuilder1 = new(conn1.ConnectionString); + + // Arrange & Act - User 2 + Mock httpContextAccessor2 = CreateHttpContextAccessorWithClaims( + issuer: "https://login.microsoftonline.com/tenant-id/v2.0", + objectId: "user2-oid-bbbb"); + + (MsSqlQueryExecutor queryExecutor2, _) = CreateQueryExecutorForPoolingTest( + connectionString: "Server=localhost;Database=test;Application Name=DAB;", + enableObo: true, + httpContextAccessor: httpContextAccessor2); + + SqlConnection conn2 = queryExecutor2.CreateConnection(provider.GetConfig().DefaultDataSourceName); + SqlConnectionStringBuilder connBuilder2 = new(conn2.ConnectionString); + + // Assert - both should have |obo: in Application Name but different hashes + // Note: The actual format includes version suffix, e.g., "DAB,dab_oss_2.0.0|obo:{hash}" + Assert.IsTrue(connBuilder1.ApplicationName.Contains("|obo:"), "User 1 should have OBO pool prefix"); + Assert.IsTrue(connBuilder2.ApplicationName.Contains("|obo:"), "User 2 should have OBO pool prefix"); + Assert.AreNotEqual(connBuilder1.ApplicationName, connBuilder2.ApplicationName, + "Different users should have different Application Names (different pool hashes)"); + } + + /// + /// Test that when no user context is present (e.g., startup), connection string uses base Application Name. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public void TestOboNoUserContext_UsesBaseConnectionString() + { + // Arrange & Act + Mock httpContextAccessor = CreateHttpContextAccessorWithClaims(issuer: string.Empty, objectId: string.Empty); + + (MsSqlQueryExecutor queryExecutor, RuntimeConfigProvider provider) = CreateQueryExecutorForPoolingTest( + connectionString: "Server=localhost;Database=test;Application Name=BaseApp;", + enableObo: true, + httpContextAccessor: httpContextAccessor); + + SqlConnection conn = queryExecutor.CreateConnection(provider.GetConfig().DefaultDataSourceName); + SqlConnectionStringBuilder connBuilder = new(conn.ConnectionString); + + // Assert - without user context, should use base Application Name (no |obo: suffix) + // Note: The actual format includes version suffix, e.g., "BaseApp,dab_oss_2.0.0" + Assert.IsTrue(connBuilder.ApplicationName.StartsWith("BaseApp"), + $"Without user context, Application Name should start with 'BaseApp' but was '{connBuilder.ApplicationName}'"); + Assert.IsFalse(connBuilder.ApplicationName.Contains("|obo:"), + $"Without user context, Application Name should not contain '|obo:' but was '{connBuilder.ApplicationName}'"); + } + + #endregion + [TestCleanup] public void CleanupAfterEachTest() { From f3f6bb646d605ac328a205fbf4b2952452a6294c Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Thu, 26 Feb 2026 17:07:50 -0800 Subject: [PATCH 2/4] Update src/Core/Resolvers/MsSqlQueryExecutor.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Core/Resolvers/MsSqlQueryExecutor.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Core/Resolvers/MsSqlQueryExecutor.cs b/src/Core/Resolvers/MsSqlQueryExecutor.cs index 688c1fa189..cf803297fe 100644 --- a/src/Core/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Core/Resolvers/MsSqlQueryExecutor.cs @@ -204,9 +204,20 @@ private string GetConnectionStringForCurrentUser(string dataSourceName) // baseAppName preserves the customer's original Application Name. // We append |obo:{hash} to create a separate connection pool for each unique user. // Format: {existingAppName}|obo:{hash} + // SQL Server limits Application Name to 128 characters. To avoid SQL Server silently + // truncating the value (which could cut off part of the hash and compromise per-user + // pooling), we ensure the full |obo:{hash} suffix is preserved and, if necessary, + // truncate only the baseAppName portion. + string oboSuffix = $"|obo:{poolKeyHash}"; + const int maxApplicationNameLength = 128; + int allowedBaseAppNameLength = Math.Max(0, maxApplicationNameLength - oboSuffix.Length); + string effectiveBaseAppName = baseAppName.Length > allowedBaseAppNameLength + ? baseAppName[..allowedBaseAppNameLength] + : baseAppName; + SqlConnectionStringBuilder userBuilder = new(baseConnectionString) { - ApplicationName = $"{baseAppName}|obo:{poolKeyHash}", + ApplicationName = $"{effectiveBaseAppName}{oboSuffix}", Pooling = true }; From 639d0034d8f973712c148e18ea02fd0f7e76f704 Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Thu, 26 Feb 2026 17:10:16 -0800 Subject: [PATCH 3/4] Update src/Core/Resolvers/MsSqlQueryExecutor.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Core/Resolvers/MsSqlQueryExecutor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Resolvers/MsSqlQueryExecutor.cs b/src/Core/Resolvers/MsSqlQueryExecutor.cs index cf803297fe..532a17541f 100644 --- a/src/Core/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Core/Resolvers/MsSqlQueryExecutor.cs @@ -310,7 +310,7 @@ private void ConfigureMsSqlQueryExecutor() _dataSourceUserDelegatedAuth[dataSourceName] = dataSource.UserDelegatedAuth!; // Per-user pooling: Keep pooling enabled but store the base Application Name. - // At connection time, we'll append the user's iss:sub hash to create isolated pools per user. + // At connection time, we'll append the user's iss|oid (or iss|sub) hash to create isolated pools per user. // This is automatic for OBO to prevent connection exhaustion while ensuring pool isolation. // Note: ApplicationName is typically already set by RuntimeConfigLoader (e.g., "CustomerApp,dab_oss_2.0.0") // but we use GetDataApiBuilderUserAgent() as fallback for consistency. From cb5123ec6642227dd9a834bdf57a5051f1f68d2a Mon Sep 17 00:00:00 2001 From: Anusha Kolan Date: Fri, 27 Feb 2026 11:53:59 -0800 Subject: [PATCH 4/4] Addressed comments, added new test cases. --- src/Core/Resolvers/MsSqlQueryExecutor.cs | 12 +- .../UnitTests/SqlQueryExecutorUnitTests.cs | 103 +++++++++++++++++- 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/src/Core/Resolvers/MsSqlQueryExecutor.cs b/src/Core/Resolvers/MsSqlQueryExecutor.cs index 532a17541f..7621d66533 100644 --- a/src/Core/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Core/Resolvers/MsSqlQueryExecutor.cs @@ -235,6 +235,9 @@ private string GetConnectionStringForCurrentUser(string dataSourceName) { if (HttpContextAccessor?.HttpContext?.User is null) { + QueryExecutorLogger.LogDebug( + "Cannot create per-user pool key for data source {DataSourceName}: no HTTP context or user available.", + dataSourceName); return null; } @@ -245,7 +248,14 @@ private string GetConnectionStringForCurrentUser(string dataSourceName) // Callers are responsible for enforcing fail-safe behavior when claims are missing. string? iss = user.FindFirst("iss")?.Value; - // Prefer oid (stable GUID), fall back to sub for guest/B2B users + // User identifier claim resolution (in priority order): + // 1. "oid" - Short claim name for object ID, used in Entra ID v2.0 tokens + // 2. Full URI form - "http://schemas.microsoft.com/identity/claims/objectidentifier" + // Used in Entra ID v1.0 tokens and some SAML-based flows + // 3. "sub" - Subject claim, unique per user per application. Used as fallback for + // guest/B2B users where oid may not be present or stable across tenants + // 4. ClaimTypes.NameIdentifier - .NET standard claim type (maps to various underlying claims) + // Acts as a last-resort fallback for non-Entra identity providers string? userKey = user.FindFirst("oid")?.Value ?? user.FindFirst("http://schemas.microsoft.com/identity/claims/objectidentifier")?.Value ?? user.FindFirst("sub")?.Value diff --git a/src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs b/src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs index 67315dc736..827a1927f5 100644 --- a/src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs +++ b/src/Service.Tests/UnitTests/SqlQueryExecutorUnitTests.cs @@ -781,6 +781,30 @@ public void TestPoolingBehaviorWithAndWithoutUserDelegatedAuth(bool enableUserDe Assert.IsTrue(connBuilder.Pooling, expectedMessage); } + /// + /// Test that when OBO is enabled, connection pooling is forcibly enabled even if + /// the original connection string has Pooling=false. Per-user pooling is required + /// for OBO to prevent connection exhaustion, so DAB intentionally overrides this setting. + /// + [TestMethod, TestCategory(TestCategory.MSSQL)] + public void TestOboEnabled_ForciblyEnablesPooling_EvenWhenConnectionStringDisablesIt() + { + // Arrange - connection string explicitly disables pooling + Mock httpContextAccessor = new(); + (MsSqlQueryExecutor queryExecutor, RuntimeConfigProvider provider) = CreateQueryExecutorForPoolingTest( + connectionString: "Server=localhost;Database=test;Pooling=false;", + enableObo: true, + httpContextAccessor: httpContextAccessor); + + // Act - check the stored connection string builder + SqlConnectionStringBuilder connBuilder = new( + queryExecutor.ConnectionStringBuilders[provider.GetConfig().DefaultDataSourceName].ConnectionString); + + // Assert - pooling should be forcibly enabled despite Pooling=false in original connection string + Assert.IsTrue(connBuilder.Pooling, + "OBO requires per-user pooling, so Pooling should be forcibly enabled even when connection string specifies Pooling=false"); + } + /// /// Test that when OBO is enabled and user claims are present, CreateConnection returns /// a connection string with a user-specific Application Name containing the pool hash. @@ -834,12 +858,12 @@ public void TestObo_DifferentUsersGetDifferentPoolHashes() issuer: "https://login.microsoftonline.com/tenant-id/v2.0", objectId: "user2-oid-bbbb"); - (MsSqlQueryExecutor queryExecutor2, _) = CreateQueryExecutorForPoolingTest( + (MsSqlQueryExecutor queryExecutor2, RuntimeConfigProvider provider2) = CreateQueryExecutorForPoolingTest( connectionString: "Server=localhost;Database=test;Application Name=DAB;", enableObo: true, httpContextAccessor: httpContextAccessor2); - SqlConnection conn2 = queryExecutor2.CreateConnection(provider.GetConfig().DefaultDataSourceName); + SqlConnection conn2 = queryExecutor2.CreateConnection(provider2.GetConfig().DefaultDataSourceName); SqlConnectionStringBuilder connBuilder2 = new(conn2.ConnectionString); // Assert - both should have |obo: in Application Name but different hashes @@ -875,6 +899,81 @@ public void TestOboNoUserContext_UsesBaseConnectionString() $"Without user context, Application Name should not contain '|obo:' but was '{connBuilder.ApplicationName}'"); } + /// + /// Test that when OBO is enabled and a user is authenticated but missing required claims + /// (iss or oid/sub), CreateConnection throws DataApiBuilderException with OboAuthenticationFailure. + /// This fail-safe behavior prevents cross-user connection pool contamination. + /// + [DataTestMethod, TestCategory(TestCategory.MSSQL)] + [DataRow("https://login.microsoftonline.com/tenant/v2.0", null, "oid/sub", + DisplayName = "Authenticated user with iss but missing oid/sub throws OboAuthenticationFailure")] + [DataRow(null, "user-object-id", "iss", + DisplayName = "Authenticated user with oid but missing iss throws OboAuthenticationFailure")] + [DataRow(null, null, "iss and oid/sub", + DisplayName = "Authenticated user with no claims throws OboAuthenticationFailure")] + public void TestOboEnabled_AuthenticatedUserMissingClaims_ThrowsException( + string? issuer, + string? objectId, + string missingClaimDescription) + { + // Arrange - Create an authenticated HttpContext with incomplete claims + Mock httpContextAccessor = CreateHttpContextAccessorWithAuthenticatedUserMissingClaims( + issuer: issuer, + objectId: objectId); + + (MsSqlQueryExecutor queryExecutor, RuntimeConfigProvider provider) = CreateQueryExecutorForPoolingTest( + connectionString: "Server=localhost;Database=test;Application Name=TestApp;", + enableObo: true, + httpContextAccessor: httpContextAccessor); + + // Act & Assert - CreateConnection should throw DataApiBuilderException + DataApiBuilderException exception = Assert.ThrowsException(() => + { + queryExecutor.CreateConnection(provider.GetConfig().DefaultDataSourceName); + }); + + Assert.AreEqual(HttpStatusCode.Unauthorized, exception.StatusCode, + $"Expected Unauthorized status code when missing {missingClaimDescription}"); + Assert.AreEqual(DataApiBuilderException.SubStatusCodes.OboAuthenticationFailure, exception.SubStatusCode, + $"Expected OboAuthenticationFailure sub-status code when missing {missingClaimDescription}"); + Assert.IsTrue(exception.Message.Contains("iss") && exception.Message.Contains("oid"), + $"Exception message should mention required claims. Actual: {exception.Message}"); + } + + /// + /// Creates an HttpContextAccessor mock with an authenticated user that has incomplete claims. + /// Used to test fail-safe behavior when OBO is enabled but required claims are missing. + /// + /// The issuer claim value, or null to omit. + /// The oid claim value, or null to omit. + /// A configured HttpContextAccessor mock with authenticated user. + private static Mock CreateHttpContextAccessorWithAuthenticatedUserMissingClaims( + string? issuer, + string? objectId) + { + Mock httpContextAccessor = new(); + DefaultHttpContext context = new(); + + // Create an authenticated identity (passing authenticationType makes IsAuthenticated = true) + System.Security.Claims.ClaimsIdentity identity = new("TestAuth"); + + // Only add claims if they are provided (non-null) + if (!string.IsNullOrEmpty(issuer)) + { + identity.AddClaim(new System.Security.Claims.Claim("iss", issuer)); + } + + if (!string.IsNullOrEmpty(objectId)) + { + identity.AddClaim(new System.Security.Claims.Claim("oid", objectId)); + } + + context.User = new System.Security.Claims.ClaimsPrincipal(identity); + httpContextAccessor.Setup(x => x.HttpContext).Returns(context); + + return httpContextAccessor; + } + #endregion [TestCleanup]