Skip to content

Commit 8255caf

Browse files
suryaiyer95claude
andcommitted
fix: delegate Azure AD credential creation to tedious and remove underscore column filter
- **Azure AD auth**: Pass `azure-active-directory-*` types directly to tedious instead of constructing `DefaultAzureCredential` ourselves. Tedious imports `@azure/identity` internally and creates credentials — avoids bun CJS/ESM `isTokenCredential` boundary issue that caused "not an instance of the token credential class" errors. - **Auth shorthands**: Map `CLI`, `default`, `password`, `service-principal`, `msi`, `managed-identity` to their full tedious type names. - **Column filter**: Remove `_.startsWith("_")` filter from `execute()` result columns — it stripped legitimate aliases like `_p` used by partition discovery, causing partitioned diffs to return empty results. - **Tests**: Remove `@azure/identity` mock (no longer imported by driver), update auth assertions, add shorthand mapping tests, fix column filter test. - **Verified**: All 97 driver tests pass. Full data-diff pipeline tested against real MSSQL server (profile, joindiff, auto, where_clause, partitioned). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5ca0cd7 commit 8255caf

File tree

2 files changed

+73
-44
lines changed

2 files changed

+73
-44
lines changed

packages/drivers/src/sqlserver.ts

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -37,29 +37,30 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
3737
},
3838
}
3939

40-
const authType = config.authentication as string | undefined
40+
// Normalize shorthand auth values to tedious-compatible types
41+
const AUTH_SHORTHANDS: Record<string, string> = {
42+
cli: "azure-active-directory-default",
43+
default: "azure-active-directory-default",
44+
password: "azure-active-directory-password",
45+
"service-principal": "azure-active-directory-service-principal-secret",
46+
serviceprincipal: "azure-active-directory-service-principal-secret",
47+
"managed-identity": "azure-active-directory-msi-vm",
48+
msi: "azure-active-directory-msi-vm",
49+
}
50+
const rawAuth = config.authentication as string | undefined
51+
const authType = rawAuth ? (AUTH_SHORTHANDS[rawAuth.toLowerCase()] ?? rawAuth) : undefined
4152

42-
if (authType?.startsWith("azure-active-directory") || authType === "token-credential") {
43-
// Azure AD / Entra ID — always encrypt
53+
if (authType?.startsWith("azure-active-directory")) {
54+
// Azure AD / Entra ID — tedious handles credential creation internally.
55+
// We pass the type + options; tedious imports @azure/identity itself.
4456
;(mssqlConfig.options as any).encrypt = true
4557

46-
if (authType === "token-credential" || authType === "azure-active-directory-default") {
47-
try {
48-
const { DefaultAzureCredential } = await import("@azure/identity")
49-
mssqlConfig.authentication = {
50-
type: "token-credential",
51-
options: {
52-
credential: new DefaultAzureCredential(
53-
config.azure_client_id
54-
? { managedIdentityClientId: config.azure_client_id as string }
55-
: undefined,
56-
),
57-
},
58-
}
59-
} catch {
60-
throw new Error(
61-
"Azure AD authentication requires @azure/identity. Run: npm install @azure/identity",
62-
)
58+
if (authType === "azure-active-directory-default") {
59+
mssqlConfig.authentication = {
60+
type: "azure-active-directory-default",
61+
options: {
62+
...(config.azure_client_id && { clientId: config.azure_client_id as string }),
63+
},
6364
}
6465
} else if (authType === "azure-active-directory-password") {
6566
mssqlConfig.authentication = {
@@ -128,7 +129,7 @@ export async function connect(config: ConnectionConfig): Promise<Connector> {
128129
const rows = result.recordset ?? []
129130
const columns =
130131
rows.length > 0
131-
? Object.keys(rows[0]).filter((k) => !k.startsWith("_"))
132+
? Object.keys(rows[0])
132133
: (result.recordset?.columns
133134
? Object.keys(result.recordset.columns)
134135
: [])

packages/drivers/test/sqlserver-unit.test.ts

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,6 @@ mock.module("mssql", () => ({
5353
},
5454
}))
5555

56-
// Mock @azure/identity for Azure AD tests
57-
class MockDefaultAzureCredential {
58-
opts: any
59-
constructor(opts?: any) {
60-
this.opts = opts
61-
}
62-
}
63-
64-
mock.module("@azure/identity", () => ({
65-
DefaultAzureCredential: MockDefaultAzureCredential,
66-
}))
67-
6856
// Import after mocking
6957
const { connect } = await import("../src/sqlserver")
7058

@@ -250,7 +238,7 @@ describe("SQL Server driver unit tests", () => {
250238
})
251239
})
252240

253-
test("azure-active-directory-default uses DefaultAzureCredential", async () => {
241+
test("azure-active-directory-default passes type to tedious (no credential object)", async () => {
254242
resetMocks()
255243
const c = await connect({
256244
host: "myserver.database.windows.net",
@@ -259,23 +247,22 @@ describe("SQL Server driver unit tests", () => {
259247
})
260248
await c.connect()
261249
const cfg = mockConnectCalls[0]
262-
expect(cfg.authentication.type).toBe("token-credential")
263-
expect(cfg.authentication.options.credential).toBeInstanceOf(MockDefaultAzureCredential)
250+
expect(cfg.authentication.type).toBe("azure-active-directory-default")
251+
expect(cfg.authentication.options.credential).toBeUndefined()
264252
})
265253

266-
test("token-credential uses DefaultAzureCredential with managed identity", async () => {
254+
test("azure-active-directory-default with client_id passes clientId option", async () => {
267255
resetMocks()
268256
const c = await connect({
269257
host: "myserver.database.windows.net",
270258
database: "db",
271-
authentication: "token-credential",
259+
authentication: "azure-active-directory-default",
272260
azure_client_id: "mi-client-id",
273261
})
274262
await c.connect()
275263
const cfg = mockConnectCalls[0]
276-
expect(cfg.authentication.type).toBe("token-credential")
277-
const cred = cfg.authentication.options.credential as MockDefaultAzureCredential
278-
expect(cred.opts).toEqual({ managedIdentityClientId: "mi-client-id" })
264+
expect(cfg.authentication.type).toBe("azure-active-directory-default")
265+
expect(cfg.authentication.options.clientId).toBe("mi-client-id")
279266
})
280267

281268
test("encryption forced for all Azure AD connections", async () => {
@@ -299,6 +286,47 @@ describe("SQL Server driver unit tests", () => {
299286
const cfg = mockConnectCalls[0]
300287
expect(cfg.options.encrypt).toBe(false)
301288
})
289+
290+
test("'CLI' shorthand maps to azure-active-directory-default", async () => {
291+
resetMocks()
292+
const c = await connect({
293+
host: "myserver.datawarehouse.fabric.microsoft.com",
294+
database: "migration",
295+
authentication: "CLI",
296+
})
297+
await c.connect()
298+
const cfg = mockConnectCalls[0]
299+
expect(cfg.authentication.type).toBe("azure-active-directory-default")
300+
expect(cfg.options.encrypt).toBe(true)
301+
})
302+
303+
test("'service-principal' shorthand maps correctly", async () => {
304+
resetMocks()
305+
const c = await connect({
306+
host: "myserver.database.windows.net",
307+
database: "db",
308+
authentication: "service-principal",
309+
azure_client_id: "cid",
310+
azure_client_secret: "csec",
311+
azure_tenant_id: "tid",
312+
})
313+
await c.connect()
314+
const cfg = mockConnectCalls[0]
315+
expect(cfg.authentication.type).toBe("azure-active-directory-service-principal-secret")
316+
expect(cfg.authentication.options.clientId).toBe("cid")
317+
})
318+
319+
test("'msi' shorthand maps to azure-active-directory-msi-vm", async () => {
320+
resetMocks()
321+
const c = await connect({
322+
host: "myserver.database.windows.net",
323+
database: "db",
324+
authentication: "msi",
325+
})
326+
await c.connect()
327+
const cfg = mockConnectCalls[0]
328+
expect(cfg.authentication.type).toBe("azure-active-directory-msi-vm")
329+
})
302330
})
303331

304332
// --- Schema introspection ---
@@ -372,12 +400,12 @@ describe("SQL Server driver unit tests", () => {
372400
])
373401
})
374402

375-
test("filters underscore-prefixed columns", async () => {
403+
test("preserves underscore-prefixed columns", async () => {
376404
mockQueryResult = {
377-
recordset: [{ id: 1, _bucket: 3, name: "x" }],
405+
recordset: [{ id: 1, _p: "Delivered", name: "x" }],
378406
}
379407
const result = await connector.execute("SELECT * FROM t")
380-
expect(result.columns).toEqual(["id", "name"])
408+
expect(result.columns).toEqual(["id", "_p", "name"])
381409
})
382410
})
383411
})

0 commit comments

Comments
 (0)