TypeError: The "config.authentication.options.credential" property must be an instance of the token credential class

[BrandonNoad]

trafficstars

I am trying to implement the Token Credential authentication added here: https://github.com/tediousjs/tedious/pull/1641

For some reason I am getting a TypeError: The "config.authentication.options.credential" property must be an instance of the token credential class error when trying to connect.

My authentication object in the config looks like

authentication: {
   type: 'token-credential',
   options: {
      credential,
   },
}

where credential is a new ClientSecretCredential from @azure/identity. If I call isTokenCredential(credential), it returns true. So I'm not sure why I am hitting this error.

Here is the line of code where the error occurs: https://github.com/tediousjs/tedious/blob/master/src/connection.ts#L1139

[BrandonNoad]

Also note that it works as expected if I pass

authentication: {
   type: 'azure-active-directory-service-principal-secret',
   options: {
      clientId: 'CLIENT_ID',
      clientSecret: 'CLIENT_SECRET',
      tenantId: 'TENANT_ID',
   },
}

But I would prefer to use a TokenCredential

[dhensby]

OK - interesting. Do you definitely have version >=18.3.0 of tedious installed?

I suspect the most likely thing is you have a different version of @azure/core-auth installed in tedious and your project. If you run npm ls @azure/core-auth what is the output?

For example it may look something like the below and you can see that I have v1.9.0 and v1.7.2 installed; if my application code is using version 1.7.2 and tedious is using 1.9.0 then that may cause the type guard that tedious is using to fail (especially if it's using instanceof checks).

./project
├─┬ [email protected]
│ └─┬ [email protected]
│   └─┬ [email protected]
│     └── @azure/[email protected] deduped
├─┬ [email protected]
│ ├─┬ @azure/[email protected]
│ │ ├── @azure/[email protected]
│ │ ├─┬ @azure/[email protected]
│ │ │ └── @azure/[email protected] deduped
│ │ └─┬ @azure/[email protected]
│ │   └── @azure/[email protected] deduped
│ ├─┬ @azure/[email protected]
│ │ ├── @azure/[email protected] deduped
│ │ └─┬ @azure/[email protected]
│ │   └── @azure/[email protected] deduped
│ ├─┬ @azure/[email protected]
│ │ └── @azure/[email protected] deduped
│ └─┬ @azure/[email protected]
│   └── @azure/[email protected] deduped
├─┬ [email protected]
│ └─┬ [email protected]
│   ├── @azure/[email protected]
│   └─┬ @azure/[email protected]
│     └── @azure/[email protected] deduped
└─┬ [email protected]
  ├─┬ @azure/[email protected]
  │ └── @azure/[email protected] deduped
  ├─┬ @azure/[email protected]
  │ └── @azure/[email protected] deduped
  ├─┬ @azure/[email protected]
  │ └── @azure/[email protected] deduped
  └─┬ [email protected]
    └── @azure/[email protected] deduped

[BrandonNoad]

Here is my output from the npm ls @azure/core-auth command

~/dev/my-project
├─┬ @azure-rest/[email protected] extraneous
│ └── @azure/[email protected] deduped
├── @azure/[email protected] extraneous
├─┬ @azure/[email protected] extraneous
│ └── @azure/[email protected] deduped
├─┬ @azure/[email protected] extraneous
│ └── @azure/[email protected] deduped
├─┬ @azure/[email protected] extraneous
│ └── @azure/[email protected] deduped
├─┬ @azure/[email protected] extraneous
│ └── @azure/[email protected] deduped
├─┬ @azure/[email protected] extraneous
│ └── @azure/[email protected] deduped
├─┬ @azure/[email protected] extraneous
│ └── @azure/[email protected] deduped
├─┬ @azure/[email protected] extraneous
│ └── @azure/[email protected] deduped
└─┬ [email protected] extraneous
  └── @azure/[email protected] deduped

Looks like all v1.9.0

[BrandonNoad]

Though I'm using a node-mssql fork (https://github.com/hightouchio/node-mssql) in order to get Fabric working, which uses a tedious fork (https://github.com/hightouchio/tedious), which looks like it may be locked to v1.7.2

https://github.com/hightouchio/tedious/blob/master/package.json#L45

[dhensby]

Ah, if you're using a fork, then best to raise your issue with the forked repo; I'm not sure any fix here would impact to the downstream fork.

But given the output of npm ls it doesn't look locked to 1.7.2 (and the ^ means any major version in the 1.x range greater or equal to 1.7.2)

[BrandonNoad]

I'll see if I can reproduce the issue without the fork. If not, I'll close this issue.

[BrandonNoad]

~~I can't reproduce when using the latest tediousjs/node-mssql~~

Edit - see comment below

[BrandonNoad]

Actually, I may have closed this issue prematurely. I am able to reproduce the issue using tediousjs/node-mssql.

I believe the issue stems from this line: https://github.com/tediousjs/node-mssql/blob/master/lib/base/connection-pool.js#L56

When the config is cloned, config.authentication.options.credential goes from a ClientSecretCredential to a plain Object, which causes the isTokenCredential check to fail (https://github.com/tediousjs/tedious/blob/master/src/connection.ts#L1139).

Before clone

authentication: {
    type: 'token-credential',
    options: { credential: [ClientSecretCredential] }
}

After clone

authentication: {
    type: 'token-credential',
    options: { credential: [Object] }
}

[dhensby]

Thanks for updating this and finding the problem. I don't think that the config option accepted complex types in this way when the cloning was first implemented, so that kind of makes sense.

We could just remove the cloning, it's not entirely necessary, or we could try to make use of constructorHandles in rfdc, but that feels like really tight coupling that is liable to be fragile.

[BrandonNoad]

I can create a PR to remove the cloning if that's the path you want to take. I don't have any context as to why the cloning was being done, so I wasn't sure if it'd be safe to remove it.

I did see the docs for the constructorHandlers, but agree it doesn't feel like the right approach.

[dhensby]

If you don't mind removing the use of clone (and the rfdc dependency) as a feat: commit, that would be fantastic and will solve this problem.

The only real reason for cloning is/was to stop misuse of changing the config by reference, but TBH, if people want to try to mess around with the config like that, they can suffer any consequences.

[github-actions[bot]]

:tada: This issue has been resolved in version 12.0.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: