Breaking change in TLS Handling in 7.3.3

[aquamatthias]

After upgrading python-arango from 7.3.1 to 7.3.3 we are no longer able to connect to arangodb servers with a TLS setup using a custom CA certificate.
The related change: #199 overrides the verify flag of the HTTP session with a boolean value and does not respect what has been defined before.

We followed the instructions on the docs page to set up our own HttpClient: https://docs.python-arango.com/en/main/http.html
and provide the CA bundle to the session via the verify property. See code ArangoDBClient

In order to fix the behavior I would suggest one of the following:

• remove the verify_certificate argument, since the same goal can be accomplished using the HttpClient
• make the verify_certificate argument optional and default to None

I am happy to provide a fix if you agree with one of the suggestions.

[cw00dw0rd]

Thanks for bringing this up and I would vote for the second option as adding this option has been frequently requested.
It would be great to add support for supplying a bundle or just supplying a boolean as you pointed out.

[lloesche]

requests.Session's verify is not a bool. It can be either that or a string containing path to a CA_BUNDLE file or directory with certificates of trusted CAs. Not super intuitive.

[cw00dw0rd]

I agree it isn't super intuitive from the link you provided and here it seems to be either a boolean or path to cert bundle.

I think defining it similar to @aquamatthias's supplied example verify: Union[str, bool, None] seems like a good approach. WDYT?

[cw00dw0rd]

Based on the above links Requests defaults verify to True and this seems like the more secure option as well. Should we instead allow for it to remain True but be optional? Open to it being None but that seems to go against the expectation of using the requests library, WDYT?

[aquamatthias]

The issue with the argument in that position: if defined it overrides everything that has been set at session level. That is why a default value other than None (no matter which one) is not ideal. So I would propose the following:

def __init__(...verify_override: Union[str, bool, None] = None):
   if verify_override is not None:
      for session in self._sessions:
         session.verify = verify_override

It still would allow for easy disabling of tls validation via verify_override=False or enable or custom bundle path,
but would not break existing sessions if the value is not provided.

wdyt?

[cw00dw0rd]

That is a good point, your suggested solution works for me. Thanks again for pointing it out!

[aquamatthias]

Great. I will provide a PR.