Description
Hey @tvrprasad!
I took a short, not very in depth, look over the code here. I'm not an expert on writing native Node.JS modules, so take everything I suggest here with a grain of salt. 😄
- Why is
SSPIinitialization performed asynchronously? Wouldn't it be simpler to make it a synchronous operation (both from implementation as well as usage perspective)?
const SSPIClient = require('sspi-client');
// SSPIClient([securityPackage])
// This allows a user to either specify one or multiple SecurityPackage to use (like 'kerberos', 'ntlm', etc).
// If at least one of the given packages is available, this returns a new client, otherwise it
// throws some sort of exception.
const client = new SSPIClient('kerbereos');or you could even go as far as making the securityPackage option mandatory and expose the list of available securityPackages via SSPIClient.availableSecurityPackages. This would shift the handling of which security package to use onto the client code.
Also, it looks like currently it's not possible to have multiple SSPIClient instances that use different security packages? SSPIClient.getSspiPackageName seems to be shared across all instances?
-
SspiImpl::GetNextBlobtakes aninBlobargument that is not used. Is that still missing? -
Is there some specific reason why all these methods are async? If the calls to the
secur32API are reasonable fast, going through the thread pool might actually cause worse performance than just calling these functions in a blocking style. That's also the reason why the Node.js crypto APIs are all implemented in a blocking fashion - making them non-blocking has awful performance. -
Overall, the SSPI API feels very much like some sort of
DuplexStream. I don't know whether this actually fits or not. A potential API for this would be:
const client = new SSPIClient('kerbereos');
client.on('data', (chunk) => {
// Do something with the data in `chunk`
});
client.on('end', () => {
// Authentication completed?
});
client.on('error', () => {
// Fired if authentication failed?
});
// If there there is multiple roundtrips for authentication, we could use write to
chunk.write(input);The nice thing about a DuplexStream would be that we could simply use .pipe when performing the authentication:
const client = new SSPIClient('kerbereos');
client.on('end', () => {
// Authentication was successful, we can stop the pipes
realSocket.unpipe(client);
client.unpipe(realSocket);
});
client.on('error', (error) => {
// Close `realSocket`;
realSocket.destroy();
// Handle `error`
...
});
realSocket.pipe(client).pipe(realSocket);Overall great work so far! ❤️ I bet many of the current tedious users will be looking forward to this getting integrated!