-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature disable tls certs #1511
Conversation
Could you add documentation to this in the README/API? |
Do not reject expired or invalid TLS certs. Sets `rejectUnauthorized=true`. Be warned that this allows MITM attacks.
Need upgrade to pass linter errors: The 'Object.getOwnPropertyDescriptors' is not supported until Node.js 7.0.0. The configured version range is '>= 6.4.0' node/no-unsupported-features/es-builtins
b107e33
to
ba08767
Compare
@niftylettuce new method is documented in docs/index.md. I hope that this was meant with README/API. If there are other places which need documentation update, please let me know. |
@niftylettuce Caould you please merge it if all ok? |
PLEASE MERGE THIS FEATURE |
If @niftylettuce is responsible or anyone else who can merge at visionmedia. Please merge! I've just been reading about 4 issues regarding this issue and saw there are a ton more about this issue. It is unbelievable this is not currently available. I totally understand the principle standpoint of wanting HTTP-only or HTTPS correct-only (referring to @kornelski) but that is just not the real world! Therefore you completely make If developers decide to misuse that, it is their problem. Document it well and/or warn the shit out of developers in log files and in documentation. It is their responsibility and you're ignorantly taking the freedom of developers away, as a matter of fact it is so common to have this basic option that with "product/library selection" it is not even a requirement exactly because it is such a basic functionality. We've chosen to use superagent because of other cool easy-to-use features such as throtteling plugin and I was quite happy about it, but this is just unbelievable. Not having this options makes the library completely useless for our and many other use cases. In our case, setting back development for at least 3 months if this PR is not merged. Again, please merge to the new version as soon as possible! And to @commenthol finally someone who took action. Thanks for developing and documenting this PR. |
this feature has been released to npm as of
thanks for your patience everyone and to @commenthol for the hard work 🙏 see documentation for this feature at https://github.com/visionmedia/superagent/blob/master/docs/index.md#tls-options or simply append if you guys need anything else ping me or email niftylettuce@gmail.com if I don't catch something the first time around |
Thank you |
Thanks a lot @niftylettuce and @commenthol. You made my day! |
@niftylettuce Thanks for the merge and for maintaining this great project. 👏 |
Unfortunately it seems like this has broken the use of Also I think this probably should have been a minor version bump? I think it may have been overlooked as there isn't any info for this feature (or any changes) in the 5.1.1 release. |
I can confirm @josh18 message after struggling the whole afternoon with the same exact issue. |
Will fix. One sec. |
Released in v5.1.3. npm install superagent@latest or with yarn: yarn add superagent@latest Please consider sponsoring my work at https://github.com/sponsors/niftylettuce. According to Node docs, the use of that environment variable is "strongly discouraged" just so you all know. Ref: https://nodejs.org/api/cli.html#cli_node_tls_reject_unauthorized_value |
Thanks! Yeah currently we use |
Tackles Issue #926 by adding a new method
disableTLSCerts
to node client to allow bypassing TLS certificate checks.