feat: fail ntlm auth gracefully when md4 hashing is not available #1400
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1400 +/- ##
==========================================
- Coverage 80.61% 80.49% -0.13%
==========================================
Files 88 88
Lines 4710 4716 +6
Branches 870 871 +1
==========================================
- Hits 3797 3796 -1
- Misses 636 643 +7
Partials 277 277
Continue to review full report at Codecov.
|
src/connection.ts
Outdated
| this.ntlmpacket = undefined; | ||
|
|
||
| this.messageIo.readMessage().then((message) => { | ||
| this.dispatchEvent('message', message); |
There was a problem hiding this comment.
Not sure if you need to try to catch the whole block. I have not tried it myself, but you could try to check if the income message or the err object has a code match ERR_OSSL_EVP_UNSUPPORTED, and emit the error.
mShan0
force-pushed
the
mark-node17-support-with-ntlm
branch
from
ab7b7cd
to
20f72d7
Compare
src/connection.ts
Outdated
| this.emit('connect', new ConnectionError(`Node 17 now uses OpenSSL 3, which considers md4 encryption a legacy type. | ||
| In order to use NTLM with Node 17, enable the '--openssl-legacy-provider' command line flag. | ||
| Check the Tedious FAQ for more information.`, 'ELOGIN')); |
There was a problem hiding this comment.
Would it make sense to turn this into an AggregateError (that would require merging #1409 first to pull in the AggregateError class)?
Also, it might be good to change how the error message is formatted to remove the leading whitespace on lines 2 and 3 of the error message.
src/connection.ts
Outdated
| In order to use NTLM with Node 17, enable the '--openssl-legacy-provider' command line flag. | ||
| Check the Tedious FAQ for more information.`, 'ELOGIN')); | ||
| } else { | ||
| this.emit('connect', new ConnectionError('Login failed.' + error.message, 'ELOGIN')); |
There was a problem hiding this comment.
I think it would be best to just re-throw any "unknown" errors we encounter here. Otherwise other errors like syntax errors or trying to call methods on an undefined object and so on will get turned into ConnectionErrors here.
What do you think?
mShan0
force-pushed
the
mark-node17-support-with-ntlm
branch
from
20f72d7
to
4af64f6
Compare
mShan0
force-pushed
the
mark-node17-support-with-ntlm
branch
from
1f0cbd8
to
e6c6496
Compare
mShan0
force-pushed
the
mark-node17-support-with-ntlm
branch
from
0f6838f
to
dbfc679
Compare
…rk-node17-support-with-ntlm
arthurschreiber
changed the title
|
🎉 This PR is included in version 14.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Emits an error explaining that the user needs to enable
--openssl-legacy-provider. Also adds tests to test the flag's functionality.Document changes: #1399 (already merged)
CI Config testing node 17: #1396