search

s-KaiNet / sp-request

Simplified SharePoint HTTP client
52 stars 9 forks source link

Use of rejectUnauthorized: false as default #17

Open Frank683 opened 4 years ago

Frank683 commented 4 years ago

Hi,

It is a bad idea to use a default setting of rejectUnauthorized: false If anyone uses a local, non public CA for their certificates they need to provide it via https.certificateAuthority to make the request work. Disabling TLS certificate checks in general - without even mentioning it in the documentation - is a really bad idea.

Appreciate your comments and hopefully a change of this setting with the next release.

s-KaiNet commented 4 years ago

Hi, thank you for your valuable feedback!

I know, that in nodejs programming, rejectUnauthorized: false might be considered as harmful and bad practice. However, the target platform for this package is SharePoint. It's used by many other related projects as well. A lot of SharePoint installations used to be on-premisses. I saw a lot of issues when users complain that everything stopped working because something wrong was with their on-premises certificates. That is why from the beginning of the project that was the default value and I haven't heard any complaints about it. When you work with on-premises installations, you have a clear understanding, what is your target platform, what is the security configuration. Thus I don't see any big issues having this default. Now you see why I can't change it - it will immediately break a lot of projects, and what is more important, I don't see any good reason to do it.

I updated documentation to indicate the default parameters.

Frank683 commented 4 years ago

I see why you can't change it right now, that's clear so far. A major version bump (like 2.x to 3.x) would have been a chance to do it since it introduced breaking changes (got as replacement of request-promise) anyway.

For sure lots of SP installations were and are on-premise but I think more and more will also be on SP online what makes the topic quite relevant.

If you google for the usual SSL certificate error messages from the node SSL module you'll find like 90% of answers suggesting to just set rejectUnauthorized: false. This will surely get rid of the error messages but at the cost of security, so why use SSL anyway then?

To conclude, I understand that it can't be changed right now but I would appreciate if it could be done somehow in future versions. Thank you for adapting the documentation to point out the default behavior or the module.

s-KaiNet commented 4 years ago

The problem is not only about version change, but code change as well. I will have to change sp-request, node-sp-auth, and some other related modules. All modules, which depend on node-sp-auth and sp-request should also be adjusted accordingly. Also, I don't think that SharePoint Online certificates will be ever compromised.

Taking all this into account, at least for now, I don't want to make these changes. I'm really sorry if I made you feel frustrated, but sometimes we all have to make complicated decisions.

patlachance commented 2 years ago

Also, I don't think that SharePoint Online certificates will be ever compromised.

Data Leakage Prevention solutions used within major companies intercept TLS traffic and usually use internal root certificate authorities to dynamically generate custom TLS certificates presented to HTTP clients for them to establish a secured connection with the browsing appliance.

The problem is not only about version change, but code change as well. I will have to change sp-request, node-sp-auth, and some other related modules. All modules, which depend on node-sp-auth and sp-request should also be adjusted accordingly.

I know this is lot of work but would you mind: 1- reconsidering 2- explain what's the best way to change the various code so that others could propose pull requests?

Thanks for your help