Set SSL certificate verification to enabled by default with possible override from configuration#3
Set SSL certificate verification to enabled by default with possible override from configuration#3
Conversation
…override from configuration.
| @@ -298,20 +300,20 @@ def getData( | |||
| f"Start GET request to {endpoint} with header: {json.dumps(headers)}" | |||
| ) | |||
There was a problem hiding this comment.
can we just set verify up top once and just use it in the requests.get workflow, feels cleaner
There was a problem hiding this comment.
I wasn't clear what you meant - the verify=... parameter has to be passed in every type of HTTP call. Or are you saying just for getting the value in the config object once?
| - Development workloads might not need the same strong guarantees you would want in production workloads. | ||
| - If you are behind a proxy / VPN you might under certain scenarios get a `SSLCertVerificationError` when trying to connect to AEP APIs. | ||
|
|
||
| If you are in one of these situation where disabling SSL certificate verification is appropriate, you can tell `aepp` to do that by using: |
There was a problem hiding this comment.
nit: replace situation with situations, also one thought/question, how would the customer know if they have to do this or not?
There was a problem hiding this comment.
It's a good question, but I would expect in most cases if someone is behind a proxy or on a VPN with special rules they will be aware of it. From what I could see there's only very few situations where SSL certificate verification can fail. If you know other cases let me know I can add more, otherwise we can stick to that for now.
There was a problem hiding this comment.
I heard for other packages that I developed that it prevented network operation (so the package cannot be used). That would be one possible scenario to test if that is the case.
Maybe we can ask the customer to provide a screenshot of the error and place it in a FAQ page.
I am thinking that we may want to document the frequent question that people ask about this package there.
Example of questions I receive:
- Does there is an SLA?
- Do I need to pay to use it ?
- Is it billed against AEP consumption ?
- How can I request assistance in case of issue ?
etc...
| config_object["private_key"] = private_key | ||
| config_object["auth_code"] = auth_code | ||
| config_object["sandbox"] = sandbox | ||
| config_object["sslVerification"] = ssl_verification |
There was a problem hiding this comment.
Can you also add that in the connectInstance class.
It is used to save the configuration loaded into an instance in order to re-use it later on with all the sub module.
I would imagine we need to add it in 3 places:
- as an attrbute (line 319)
- in the dictionary definition saved internally
self.__configObject__(starting line 320) - you may want to create a setter function to change this value, the way I did for sandbox.(up to you)
3 participants
This PR sets the SSL certificate verification to true by default, with the possibility to override it in the
configuremethod or in the JSON configuration file directly. Also includes some details in the README to explain when it is appropriate to use that and how to use it.Description
aepp.configobject a new field calledsslVerificationconfiguremethod calledssl_verificationto let the user control that.verifyoption with value coming from the configured value.ingestionmodule to have the verification come from configRelated Issue
#2
Motivation and Context
Makes the library secure by default and gives user control over SSL configuration
How Has This Been Tested?
Tested on several orgs
Screenshots (if appropriate):
Types of changes
Checklist: