Skip to content

Set SSL certificate verification to enabled by default with possible override from configuration #3

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.

Sign up for GitHub

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

Open
cmenguy wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Set SSL certificate verification to enabled by default with possible override from configuration #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion aepp/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"token": "",
"jwtTokenEndpoint": "",
"oauthTokenEndpoint": "",
"imsEndpoint": ""
"imsEndpoint": "",
"sslVerification": True
}

header = {"Accept": "application/json",
Expand Down
16 changes: 14 additions & 2 deletions aepp/configs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import json
import os
import sys
from pathlib import Path
from typing import Optional
import json
Expand Down Expand Up @@ -43,22 +44,26 @@ def createConfigFile(
environment: str = "prod",
verbose: object = False,
auth_type: str = "jwt",
ssl_verification: bool = True,
**kwargs,
) -> None:
"""
This function will create a 'config_admin.json' file where you can store your access data.
Arguments:
destination : OPTIONAL : if you wish to save the file at a specific location.
sandbox : OPTIONAL : You can directly set your sandbox name in this parameter.
environment : OPTIONAL : should be set to True except in internal Adobe use cases.
verbose : OPTIONAL : set to true, gives you a print stateent where is the location.
auth_type : OPTIONAL : type of authentication, either "jwt" or "oauth"
ssl_verification : OPTIONAL : whether to enable SSL verification. True is recommended.
"""
json_data: dict = {
"org_id": "<orgID>",
"client_id": "<client_id>",
"secret": "<YourSecret>",
"sandbox-name": sandbox,
"environment": environment
"environment": environment,
"ssl_verification": ssl_verification
}
if auth_type == "jwt":
json_data["tech_id"] = "<something>@techacct.adobe.com"
Expand Down Expand Up @@ -124,6 +129,7 @@ def importConfigFile(
"secret": provided_config["secret"],
"sandbox": provided_config.get("sandbox-name", "prod"),
"environment": provided_config.get("environment", "prod"),
"ssl_verification": provided_config["ssl_verification"],
"connectInstance": connectInstance
}
if sandbox is not None: ## overriding sandbox from parameter
Expand Down Expand Up @@ -153,7 +159,8 @@ def configure(
sandbox: str = "prod",
connectInstance: bool = False,
environment: str = "prod",
auth_code: str = None
auth_code: str = None,
ssl_verification: bool = True
):
"""Performs programmatic configuration of the API using provided values.
Arguments:
Expand All @@ -167,6 +174,7 @@ def configure(
connectInstance : OPTIONAL : If you want to return an instance of the ConnectObject class
environment : OPTIONAL : If not provided, default to prod
auth_code : OPTIONAL : If an authorization code is used directly instead of generating via JWT
ssl_verification : OPTIONAL : whether to enable SSL verification. True is recommended.
"""
if not org_id:
raise ValueError("`org_id` must be specified in the configuration.")
Expand All @@ -177,6 +185,9 @@ def configure(
if (auth_code is not None and (path_to_key is not None or private_key is not None)) \
or (auth_code is None and path_to_key is None and private_key is None):
raise ValueError("either `auth_code` needs to be specified or one of `private_key` or `path_to_key`")
if ssl_verification is False:
print("SSL verification disabled, this is not recommended in production workloads and "
"should only be used in development", file=sys.stderr)
config_object["org_id"] = org_id
header["x-gw-ims-org-id"] = org_id
config_object["client_id"] = client_id
Expand All @@ -187,6 +198,7 @@ def configure(
config_object["private_key"] = private_key
config_object["auth_code"] = auth_code
config_object["sandbox"] = sandbox
config_object["sslVerification"] = ssl_verification
Copy link
Contributor

@pitchmuc pitchmuc May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

header["x-sandbox-name"] = sandbox

# ensure we refer to the right environment endpoints
Expand Down
46 changes: 24 additions & 22 deletions aepp/connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ def __init__(
self.loggingEnabled = loggingEnabled
self.logger = logger
self.retry = retry
requests.packages.urllib3.disable_warnings()
if self.config["token"] == "" or time.time() > self.config["date_limit"]:
if self.config["private_key"] is not None or self.config["pathToKey"] is not None:
token_info = self.get_jwt_token_and_expiry_for_config(
Expand Down Expand Up @@ -139,7 +138,7 @@ def get_oauth_token_and_expiry_for_config(
"code": config["auth_code"]
}
response = requests.post(
config["oauthTokenEndpoint"], data=oauth_payload, verify=False
config["oauthTokenEndpoint"], data=oauth_payload, verify=self.config["sslVerification"]
)
return self._token_postprocess(response=response, verbose=verbose, save=save)

Expand Down Expand Up @@ -186,7 +185,10 @@ def get_jwt_token_and_expiry_for_config(
"jwt_token": encoded_jwt,
}
response = requests.post(
config["jwtTokenEndpoint"], headers=header_jwt, data=payload, verify=False
config["jwtTokenEndpoint"],
headers=header_jwt,
data=payload,
verify=self.config["sslVerification"]
)
return self._token_postprocess(response=response, verbose=verbose, save=save)

Expand Down Expand Up @@ -298,20 +300,20 @@ def getData(
f"Start GET request to {endpoint} with header: {json.dumps(headers)}"
)
Copy link
Contributor

@skanjila skanjila May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just set verify up top once and just use it in the requests.get workflow, feels cleaner

Copy link
Contributor Author

@cmenguy cmenguy May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@skanjila skanjila May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter

if params is None and data is None:
res = requests.get(endpoint, headers=headers, verify=False)
res = requests.get(endpoint, headers=headers, verify=self.config["sslVerification"])
elif params is not None and data is None:
if self.loggingEnabled:
self.logger.debug(f"params: {json.dumps(params)}")
res = requests.get(endpoint, headers=headers, params=params, verify=False)
res = requests.get(endpoint, headers=headers, params=params, verify=self.config["sslVerification"])
elif params is None and data is not None:
if self.loggingEnabled:
self.logger.debug(f"data: {json.dumps(data)}")
res = requests.get(endpoint, headers=headers, data=data, verify=False)
res = requests.get(endpoint, headers=headers, data=data, verify=self.config["sslVerification"])
elif params is not None and data is not None:
if self.loggingEnabled:
self.logger.debug(f"params: {json.dumps(params)}")
self.logger.debug(f"data: {json.dumps(data)}")
res = requests.get(endpoint, headers=headers, params=params, data=data, verify=False)
res = requests.get(endpoint, headers=headers, params=params, data=data, verify=self.config["sslVerification"])
if self.loggingEnabled:
self.logger.debug(f"endpoint used: {res.request.url}")
self.logger.debug(f"params used: {params}")
Expand Down Expand Up @@ -370,9 +372,9 @@ def headData(
f"Start GET request to {endpoint} with header: {json.dumps(headers)}"
)
if params is None:
res = requests.head(endpoint, headers=headers, verify=False)
res = requests.head(endpoint, headers=headers, verify=self.config["sslVerification"])
if params is not None:
res = requests.head(endpoint, headers=headers, params=params, verify=False)
res = requests.head(endpoint, headers=headers, params=params, verify=self.config["sslVerification"])
try:
res_header = res.headers()
except:
Expand Down Expand Up @@ -407,27 +409,27 @@ def postData(
f"Start POST request to {endpoint} with header: {json.dumps(headers)}"
)
if params is None and data is None:
res = requests.post(endpoint, headers=headers, verify=False)
res = requests.post(endpoint, headers=headers, verify=self.config["sslVerification"])
elif params is not None and data is None:
if self.loggingEnabled:
self.logger.debug(f"params: {json.dumps(params)}")
res = requests.post(endpoint, headers=headers, params=params, verify=False)
res = requests.post(endpoint, headers=headers, params=params, verify=self.config["sslVerification"])
elif params is None and data is not None:
if self.loggingEnabled:
self.logger.debug(f"data: {json.dumps(data)}")
res = requests.post(endpoint, headers=headers, data=json.dumps(data), verify=False)
res = requests.post(endpoint, headers=headers, data=json.dumps(data), verify=self.config["sslVerification"])
elif params is not None and data is not None:
if self.loggingEnabled:
self.logger.debug(f"params: {json.dumps(params)}")
self.logger.debug(f"data: {json.dumps(data)}")
res = requests.post(
endpoint, headers=headers, params=params, data=json.dumps(data), verify=False
endpoint, headers=headers, params=params, data=json.dumps(data), verify=self.config["sslVerification"]
)
elif bytesData is not None:
if self.loggingEnabled:
self.logger.debug(f"bytes data used")
res = requests.post(
endpoint, headers=headers, params=params, data=bytesData, verify=False
endpoint, headers=headers, params=params, data=bytesData, verify=self.config["sslVerification"]
)
try:
formatUse = kwargs.get("format", "json")
Expand Down Expand Up @@ -482,17 +484,17 @@ def patchData(
if params is not None and data is None:
if self.loggingEnabled:
self.logger.debug(f"params: {json.dumps(params)}")
res = requests.patch(endpoint, headers=headers, params=params, verify=False)
res = requests.patch(endpoint, headers=headers, params=params, verify=self.config["sslVerification"])
elif params is None and data is not None:
if self.loggingEnabled:
self.logger.debug(f"data: {json.dumps(data)}")
res = requests.patch(endpoint, headers=headers, data=json.dumps(data), verify=False)
res = requests.patch(endpoint, headers=headers, data=json.dumps(data), verify=self.config["sslVerification"])
elif params is not None and data is not None:
if self.loggingEnabled:
self.logger.debug(f"params: {json.dumps(params)}")
self.logger.debug(f"data: {json.dumps(data)}")
res = requests.patch(
endpoint, headers=headers, params=params, data=json.dumps(data), verify=False
endpoint, headers=headers, params=params, data=json.dumps(data), verify=self.config["sslVerification"]
)
try:
res_json = res.json()
Expand Down Expand Up @@ -536,17 +538,17 @@ def putData(
if params is not None and data is None:
if self.loggingEnabled:
self.logger.debug(f"params: {json.dumps(params)}")
res = requests.put(endpoint, headers=headers, params=params, verify=False)
res = requests.put(endpoint, headers=headers, params=params, verify=self.config["sslVerification"])
elif params is None and data is not None:
if self.loggingEnabled:
self.logger.debug(f"data: {json.dumps(data)}")
res = requests.put(endpoint, headers=headers, data=json.dumps(data), verify=False)
res = requests.put(endpoint, headers=headers, data=json.dumps(data), verify=self.config["sslVerification"])
elif params is not None and data is not None:
if self.loggingEnabled:
self.logger.debug(f"params: {json.dumps(params)}")
self.logger.debug(f"data: {json.dumps(data)}")
res = requests.put(
endpoint, headers=headers, params=params, data=json.dumps(data), verify=False
endpoint, headers=headers, params=params, data=json.dumps(data), verify=self.config["sslVerification"]
)
try:
res_json = res.json()
Expand Down Expand Up @@ -579,9 +581,9 @@ def deleteData(
f"Start PUT request to {endpoint} with header: {json.dumps(headers)}"
)
if params is None:
res = requests.delete(endpoint, headers=headers, verify=False)
res = requests.delete(endpoint, headers=headers, verify=self.config["sslVerification"])
elif params is not None:
res = requests.delete(endpoint, headers=headers, params=params, verify=False)
res = requests.delete(endpoint, headers=headers, params=params, verify=self.config["sslVerification"])
try:
status_code = res.status_code
if status_code >= 400:
Expand Down
4 changes: 2 additions & 2 deletions aepp/ingestion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ def __init__(
header : OPTIONAL : header object in the config module.
Additional kwargs will update the header.
"""
requests.packages.urllib3.disable_warnings()
if loggingObject is not None and sorted(
["level", "stream", "format", "filename", "file"]
) == sorted(list(loggingObject.keys())):
Expand Down Expand Up @@ -82,6 +81,7 @@ def __init__(
self.endpoint = (
aepp.config.endpoints["global"] + aepp.config.endpoints["ingestion"]
)
self.ssl_verification = config["sslVerification"]
self.endpoint_streaming = aepp.config.endpoints["streaming"]["collection"]
self.STREAMING_REFERENCE = {
"header": {
Expand Down Expand Up @@ -331,7 +331,7 @@ def uploadLargeFilePart(
if self.loggingEnabled:
self.logger.debug(f"Uploading large part for batch ID: ({batchId})")
path = f"/batches/{batchId}/datasets/{datasetId}/files/{filePath}"
res = requests.patch(self.endpoint + path, data=data, headers=privateHeader, verify=False)
res = requests.patch(self.endpoint + path, data=data, headers=privateHeader, verify=self.ssl_verification)
res_json = res.json()
return res_json

Expand Down
31 changes: 30 additions & 1 deletion docs/getting-started.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ Normally your config file will look like this:
"secret": "<YourSecret>",
"pathToKey": "<path/to/your/privatekey.key>",
"sandbox-name": "prod",
"environment": "prod"
"environment": "prod",
"ssl_verification": true
}
```

Expand Down Expand Up @@ -172,6 +173,34 @@ aepp.configure(

**NOTE** The `environment` parameter is optional and defaults to "prod".

### SSL certificate verification

By default `aepp` will enforce SSL certificate verification on any outgoing HTTP requests.

There can be cases where you might want to disable this verification. This includes, but not limited to:
- 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:
Copy link
Contributor

@skanjila skanjila May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: replace situation with situations, also one thought/question, how would the customer know if they have to do this or not?

Copy link
Contributor Author

@cmenguy cmenguy May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@pitchmuc pitchmuc May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...


```python
import aepp
aepp.configure(
org_id=my_org_id,
tech_id=my_tech_id,
secret=my_secret,
private_key=my_key_as_string,
client_id=my_client_id,
environment="prod",
sandbox=my_sandbox_id,
ssl_verification=False
)
```

Or simply update the JSON configuration generated earlier if you have one and set `ssl_verification` parameter to `false`.

Please note disabling SSL certificate verification is not recommended overall, but we provide this facility in `aepp` for the few situations that do warrant it if you know what you are doing.

### Tip for multi sandbox work

The `aepp` module contains a parameter named `connectInstance` for `importConfig` and `configure` methods that provide a way to store the configuration setup.\
Expand Down