Skip to content

PAPP-36780 cert based auth #58

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
NiemySpiewak-splunk wants to merge 4 commits into
base: msankowska/PSAAS-24763-porting_HTTP_to_SDK
Choose a base branch
from
Open

PAPP-36780 cert based auth #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
41f46f0
add input fields for public certificate and private key
NiemySpiewak-splunk Sep 23, 2025
258ac45
temporary file creator
NiemySpiewak-splunk Sep 23, 2025
66821ac
request_maker updated - enable certificate auth in requests
NiemySpiewak-splunk Sep 23, 2025
6c58252
refactoring
NiemySpiewak-splunk Sep 23, 2025
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
22 changes: 7 additions & 15 deletions package/src/asset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,12 @@ class Asset(BaseAsset):
description="Type of authentication token",
default="ph-auth-token",
)
auth_token: str = AssetField(
required=False, description="Value of authentication token"
)
username: str = AssetField(
required=False, description="Username (for HTTP basic auth)"
)
password: str = AssetField(
required=False, description="Password (for HTTP basic auth)"
)
oauth_token_url: str = AssetField(
required=False, description="URL to fetch oauth token from"
)
auth_token: str = AssetField(required=False, description="Value of authentication token")
username: str = AssetField(required=False, description="Username (for HTTP basic auth)")
password: str = AssetField(required=False, description="Password (for HTTP basic auth)")
oauth_token_url: str = AssetField(required=False, description="URL to fetch oauth token from")
client_id: str = AssetField(required=False, description="Client ID (for OAuth)")
client_secret: str = AssetField(
required=False, description="Client Secret (for OAuth)"
)
client_secret: str = AssetField(required=False, description="Client Secret (for OAuth)")
timeout: float = AssetField(required=False, description="Timeout for HTTP calls")
test_http_method: str = AssetField(
required=False,
Expand All @@ -60,3 +50,5 @@ class Asset(BaseAsset):
"PATCH",
],
)
public_cert: str = AssetField(required=False, sensitive=True, description="Public part of the client certificate")
private_key: str = AssetField(required=False, sensitive=True, description="Private key for the client certificate")
55 changes: 39 additions & 16 deletions package/src/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@
# limitations under the License.
import json
from typing import Optional

import tempfile
from contextlib import contextmanager, nullcontext
import xmltodict
from bs4 import BeautifulSoup
from pydantic import ValidationError
from soar_sdk.exceptions import ActionFailure

import base64
from .common import logger
from .schemas import ParsedResponseBody
from .asset import Asset
import os


def process_xml_response(response) -> dict:
Expand All @@ -36,9 +39,7 @@ def process_json_response(response) -> dict:
except json.JSONDecodeError as e:
raise ActionFailure(f"Server claimed JSON but failed to parse. Error: {e}")
except ValidationError as e:
raise ActionFailure(
f"Response JSON did not match expected structure. Details: {e}"
)
raise ActionFailure(f"Response JSON did not match expected structure. Details: {e}")


def process_html_response(response) -> ParsedResponseBody:
Expand All @@ -56,11 +57,7 @@ def process_html_response(response) -> ParsedResponseBody:


def process_empty_response(content_type) -> dict:
message = (
"Response includes a file"
if "octet-stream" in content_type
else "Empty response body"
)
message = "Response includes a file" if "octet-stream" in content_type else "Empty response body"
return {"message": message}


Expand Down Expand Up @@ -90,16 +87,12 @@ def parse_headers(headers_str: Optional[str]) -> dict:
parsed_headers = json.loads(headers_str)

except json.JSONDecodeError as e:
error_message = (
f"Failed to parse headers. Ensure it's a valid JSON object. Error: {e}"
)
error_message = f"Failed to parse headers. Ensure it's a valid JSON object. Error: {e}"
logger.error(error_message)
raise ActionFailure(error_message)

if not isinstance(parsed_headers, dict):
raise ActionFailure(
"Headers parameter must be a valid JSON object (dictionary)."
)
raise ActionFailure("Headers parameter must be a valid JSON object (dictionary).")

return parsed_headers

Expand All @@ -126,3 +119,33 @@ def handle_various_response(response):
else:
raw_body = response.text
return parsed_body, raw_body


@contextmanager
def temp_cert_files(cert_b64: str, key_b64: str):
"""
Safely creates temporary files for a client certificate and key from Base64 strings
and yields their paths. Cleans up the files automatically.
"""
if not (cert_b64 and key_b64):
yield (None, None)
return

cert_path, key_path = None, None
cert_bytes = base64.b64decode(asset.public_cert)
key_bytes = base64.b64decode(asset.private_key)
Comment on lines +135 to +136

@tapishj-splunk tapishj-splunk Sep 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't these use cert_b64 and key_b64 respectively


try:

with tempfile.NamedTemporaryFile(mode="wb", delete=False) as cert_f, tempfile.NamedTemporaryFile(mode="wb", delete=False) as key_f:
cert_f.write(cert_bytes)
key_f.write(key_bytes)
cert_path = cert_f.name
key_path = key_f.name

yield (cert_path, key_path)
finally:
if cert_path and os.path.exists(cert_path):
os.remove(cert_path)
if key_path and os.path.exists(key_path):
os.remove(key_path)
74 changes: 33 additions & 41 deletions package/src/request_maker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from .asset import Asset
from .auth import OAuth, get_auth_method
from .common import logger
from .helpers import temp_cert_files


def make_request(
Expand All @@ -49,48 +50,39 @@ def make_request(

logger.info(f"Making {method} request to: {full_url}")

body = (
UnicodeDammit(body).unicode_markup.encode("utf-8")
if isinstance(body, str)
else body
)

retries = 1
response = None

while retries >= 0:
auth_method = get_auth_method(asset, soar)
auth_object, final_headers = auth_method.create_auth(parsed_headers)

try:
response = requests.request(
method=method,
url=full_url,
auth=auth_object,
data=body,
verify=verify,
headers=final_headers,
timeout=asset.timeout,
)
response.raise_for_status()

break

except requests.exceptions.RequestException as e:
if (
isinstance(auth_method, OAuth)
and e.response
and e.response.status_code == 401
and retries > 0
):
logger.warning(
"Request failed with 401, token might be expired. Forcing a refresh."
body = UnicodeDammit(body).unicode_markup.encode("utf-8") if isinstance(body, str) else body

with temp_cert_files(asset.public_cert, asset.private_key) as cert_param:

@tapishj-splunk tapishj-splunk Sep 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

retries = 1
response = None

while retries >= 0:
auth_method = get_auth_method(asset, soar)
auth_object, final_headers = auth_method.create_auth(parsed_headers)

try:
response = requests.request(
method=method,
url=full_url,
auth=auth_object,
data=body,
verify=verify,
headers=final_headers,
cert=cert_param,
timeout=asset.timeout,
)
auth_method.get_token(force_new=True)
retries -= 1
continue
else:
raise ActionFailure(f"Request failed for {full_url}. Details: {e}")
response.raise_for_status()

break

except requests.exceptions.RequestException as e:
if isinstance(auth_method, OAuth) and e.response and e.response.status_code == 401 and retries > 0:

@tapishj-splunk tapishj-splunk Sep 23, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

logger.warning("Request failed with 401, token might be expired. Forcing a refresh.")
auth_method.get_token(force_new=True)
retries -= 1
continue
else:
raise ActionFailure(f"Request failed for {full_url}. Details: {e}")

parsed_body, raw_body = helpers.handle_various_response(response)
logger.info(f"Successfully processed data. Status: {response.status_code}")
Expand Down
Loading