PAPP-36780 cert based auth

[NiemySpiewak-splunk]

Features

List any features you're adding to this app (e.g. new actions, parameters, auth methods, etc.)

• Added support for client certificate authentication in the HTTP

Manual Documentation

Have you made any changes that should be documented in manual_readme_content.md? NO

The following changes require documentation in manual_readme_content.md:

• New, updated, or removed REST handlers
• New, updated, or removed authentication methods, especially complex methods like OAuth
• Compatibility considerations with respect to deployment types (e.g. actions that cannot be run on cloud or an automation broker)

• I have verified that manual documentation has been updated where appropriate

[mposluszny-splunk]

I'd move those 2 lines outside of try, at this point we don't have anything to delete

[mposluszny-splunk]

why are we checking for client_key and client_cert when temp_cert_files uses different params of an Asset?

Btw does the temp_cert_files need the whole asset? I think we might be good with just cert params ;)

also it would be cleaener to be able to use it just like with temp_cert_files(cert ,key) as cert_param:
...

[mposluszny-splunk]

I think we have kind of scope creep here, block inside a block inside another block.
Any ideas how we can get around that?

[tapishj-splunk]

This looks like an old version of the base branch. This should be

if isinstance(auth_method, OAuth) and retries > 0: see https://github.com/splunk-soar-connectors/http_app/blob/msankowska/PSAAS-24763-porting_HTTP_to_SDK/src/request_maker.py#L82

you probably need to merge the base branch with this branch and fix any merge conflicts

[tapishj-splunk]

This seems like somewhat of an anti-pattern. It implies that we can have token based auth and then get_auth_method can still return a different type of auth. We're implementing standalone cert based auth. Let's change this logic by creating a CertBasedAuth class that inherits Authorization in auth.py, then have get_auth_method return CertBasedAuth is asset.public_cert and asset.private_key are present.

I also think we shouldn't surround the entire function with a context manager when it's not going to be used most of the time. Lets do something like this

Suggested change

	with temp_cert_files(asset.public_cert, asset.private_key) as cert_param:
	auth_method = get_auth_method(asset, soar)
	if isinstance(auth_method, CertificateAuth):
	return _execute_certificate_request(auth_method, full_url, method, body, verify, parsed_headers, output, asset, soar)
	else:
	return _execute_standard_request(auth_method, full_url, method, body, verify, parsed_headers, output, asset, soar)

_execute_certificate_request would then make certificate based requests and _execute_standard_request would be responsible for everything else

[tapishj-splunk]

shouldn't these use cert_b64 and key_b64 respectively