define http-request-timeouts (+ refactor)

As suggested by `requests`'s documentation [0], define time-outs for
http-requests. This change was inspired/suggested by bandit [1][2]. This
change addresses all findings of `medium` severity. In addition, switch
to using `requests.sessions.Session` where reasonable (this is the case
if there is a reasonable likelihood for a session to actually be used
more than once). Also, refactor some quirky code.

Considerations w.r.t. redundantly defining timeout parameter
------------------------------------------------------------

As discussed on `requests`'s GitHub-Repository (e.g. at [3]), there is
no means (except, of course, through "monkey-patching") to define a
default timeout, neither globally, nor e.g. for a session. While it is
an option to implement a custom transport-adaptor (and use it to inject
timeout parameter into issued requests), this approach seems rather
heavyweight, and will add a lot of boilerplate code.

Therefore, it seems like the best available option to pass `timeout`
parameter to each individual invocation of `requests`'s
"request-issueing-functions" (request, get, put, ...).

One might feel the urge to deduplicate the specified timeout values.
However, there are reasons against this:
- will also add boilerplate (one extra import stmt)
- it is unlikely that timeouts will ever (have to) be changed (more
  likely: specific individual timeouts might require "tuning" - which is
  very easy with the chosen approach)

Therefore - assuming the chosen de-facto default timeout value proves to
be "good enough" - it seems like an adequate choice to redundantly
define timeout values.

[0]
https://requests.readthedocs.io/en/latest/user/advanced/#timeouts
[1]
https://bandit.readthedocs.io/en/1.7.6/plugins/b113_request_without_timeout.html
[2]
https://bandit.readthedocs.io/
[3]
psf/requests#3070

ccc/secrets_server.py

@@ -131,7 +131,7 @@ def retrieve_secrets(self):
                     return json.load(f)
 
         request_url = urljoin(self.url, self.concourse_secret_name)
-        response = requests.get(request_url)
+        response = requests.get(request_url, timeout=(3, 30))
 
         if not response.ok:
         # pylint: enable=no-member

cfg_mgmt/azure.py

@@ -69,6 +69,7 @@ def _retrieve_password_credentials(
     response = requests.get(
         url=f'{azure_app_root_url}/{service_principal.object_id()}',
         headers={'Authorization': f'Bearer {access_token}'},
+        timeout=(3, 30),
     )
     response_dict = response.json()
     if not (password_credentials := response_dict.get('passwordCredentials')):
@@ -99,6 +100,7 @@ def _add_password_credential(
         f'{azure_app_root_url}/{service_principal.object_id()}/addPassword',
         json=body,
         headers={'Authorization': f'Bearer {access_token}'},
+        timeout=(3, 30),
     )
     response.raise_for_status()
     response_dict = response.json()
@@ -119,6 +121,7 @@ def _remove_password_credential(
         f'{azure_app_root_url}/{service_principal.object_id()}/removePassword',
         json={"keyId": key_id},
         headers={'Authorization': f'Bearer {access_token}'},
+        timeout=(3, 30),
     )
     # successful request returns "204 No content"
     response.raise_for_status()

cfg_mgmt/btp_application_certificate.py

@@ -45,7 +45,12 @@ def _setup_oauth_token(self):
         headers = {
             'Accept': 'application/json',
         }
-        resp = requests.post(f'{uaa["url"]}/oauth/token', data=data, headers=headers)
+        resp = requests.post(
+            f'{uaa["url"]}/oauth/token',
+            data=data,
+            headers=headers,
+            timeout=(3, 30),
+        )
         resp.raise_for_status()
         self.access_token = resp.json()['access_token']
 
@@ -60,7 +65,12 @@ def create_client_certificate_chain(self, csr_pem: str, validity_in_days: int) -
             'validity': {'type': 'DAYS', 'value': validity_in_days},
         }}
         url = self.credentials['certificateservice']['profileurl']
-        resp = requests.post(url, json=data, headers=headers)
+        resp = requests.post(
+            url,
+            json=data,
+            headers=headers,
+            timeout=(3, 30),
+        )
         resp.raise_for_status()
         logger.info('Created certificate')
         return resp.json()['certificate-response']
@@ -108,7 +118,11 @@ def put_certificate(self, cert_pem: str, desc: str, scopes: list[str]) -> str:
             'base64': cert_pem,
         }
         with self._session_with_cert() as session:
-            resp = session.put(self.url, json=data)
+            resp = session.put(
+                self.url,
+                json=data,
+                timeout=(3, 30),
+            )
         resp.raise_for_status()
         id = resp.json()['certificateId']
         logger.info(f'Added certificate {id}')
@@ -119,7 +133,11 @@ def delete_certificate(self, common_name: str, cert_id: str):
             'certificateId': cert_id,
         }
         with self._session_with_cert() as session:
-            resp = session.delete(self.url, json=data)
+            resp = session.delete(
+                self.url,
+                json=data,
+                timeout=(3, 30),
+            )
         resp.raise_for_status()
         logger.info(f'Deleted certificate {common_name} ({cert_id})')
 
@@ -136,7 +154,10 @@ def list_certificates_by_base(
         common_name_base: str
     ) -> typing.Generator[CertInfo, None, None]:
         with self._session_with_cert() as session:
-            resp = session.get(self.clienturl)
+            resp = session.get(
+                self.clienturl,
+                timeout=(3, 30),
+            )
         resp.raise_for_status()
         for item in resp.json():
             id = item['dnId']

cfg_mgmt/btp_service_binding.py

@@ -65,7 +65,11 @@ def delete_service_binding(self, name: str, id: str):
             'Authorization': f'Bearer {self.access_token}',
         }
         url = f'{self.sm_url}/v1/service_bindings/{id}'
-        resp = requests.delete(url, headers=headers)
+        resp = requests.delete(
+            url,
+            headers=headers,
+            timeout=(3, 30),
+        )
         if not resp.ok:
             msg = f'delete_service_binding failed: {resp.status_code} {resp.text}'
             logger.error(msg)
@@ -83,7 +87,12 @@ def create_service_binding(self, instance_id: str, binding_name: str) \
             'Accept': 'application/json',
             'Authorization': f'Bearer {self.access_token}',
         }
-        resp = requests.post(url, json=data, headers=headers)
+        resp = requests.post(
+            url,
+            json=data,
+            headers=headers,
+            timeout=(3, 30),
+        )
         if not resp.ok:
             msg = f'create_service_binding failed: {resp.status_code} {resp.text}'
             logger.error(msg)
@@ -99,7 +108,11 @@ def get_service_binding(self, id: str) -> ServiceBindingDetailedInfo:
             'Accept': 'application/json',
             'Authorization': f'Bearer {self.access_token}',
         }
-        resp = requests.get(url, headers=headers)
+        resp = requests.get(
+            url,
+            headers=headers,
+            timeout=(3, 30),
+        )
         if not resp.ok:
             msg = f'get_service_binding failed: {resp.status_code} {resp.text}'
             logger.error(msg)
@@ -113,7 +126,11 @@ def get_service_bindings(self) -> list[ServiceBindingInfo]:
             'Accept': 'application/json',
             'Authorization': f'Bearer {self.access_token}',
         }
-        resp = requests.get(url, headers=headers)
+        resp = requests.get(
+            url,
+            headers=headers,
+            timeout=(3, 30),
+        )
         if not resp.ok:
             msg = f'get_service_bindings failed: {resp.status_code} {resp.text}'
             logger.error(msg)
@@ -156,7 +173,12 @@ def _get_oauth_token(credentials: dict) -> str:
     headers = {
         'Accept': 'application/json',
     }
-    resp = requests.post(url, data=data, headers=headers)
+    resp = requests.post(
+        url,
+        data=data,
+        headers=headers,
+        timeout=(3, 30),
+    )
     if not resp.ok:
         msg = f'_get_oauth_token failed: {resp.status_code} {resp.reason}'
         logger.error(msg)

cfg_mgmt/github.py

@@ -3,7 +3,6 @@
 import enum
 import hashlib
 import logging
-import sys
 import typing
 
 import requests
@@ -102,14 +101,16 @@ def _rotate_oauth_token(
     client_secret = '18867509d956965542b521a529a79bb883344c90'
 
     request_url = f'{github_api_url}/applications/{client_id}/token'
-    request_kwargs = {
-        'url': request_url,
-        'auth': (client_id, client_secret), # basic auth
-        'json': {'access_token': token_to_rotate},
-    }
+    session = requests.sessions.Session()
+    session.auth = (client_id, client_secret)
+
     # check token (to catch the case if the token not being associated with the git-credentials-
     # manager)
-    resp = requests.post(**request_kwargs)
+    resp = session.post(
+        request_url,
+        json={'access_token': token_to_rotate},
+        timeout=(3, 30),
+    )
     if not resp.ok:
         # No way for the user to see the oAuth token that belongs to the app, but at least checking
         # whether the given token is currently associated with the app can be done.
@@ -118,11 +119,15 @@ def _rotate_oauth_token(
             'oAuth token belong to the git-credentials-manager application and is valid?'
         )
         return None
+
     # fire request that causes the actual refresh (i.e. "rotation") to happen.
     # Note: If successful, old token is immediately invalidated.
-    resp = requests.patch(**request_kwargs)
-    if not resp.ok:
-        resp.raise_for_status()
+    resp = session.patch(
+        request_url,
+        json={'access_token': token_to_rotate},
+        timeout=(3, 30),
+    )
+    resp.raise_for_status()
 
     response_content = resp.json()
 
@@ -184,15 +189,12 @@ def rotate_cfg_element(
                     f"'{credential.username}' of github-config '{cfg_to_rotate.name()}' "
                     '- will not attempt rotation.'
                 )
-        except:
-            exception = sys.exception()
-            if isinstance(exception, requests.exceptions.HTTPError):
-                logger.error(
-                    f'Error when trying to refresh oAuth token: {exception}. Response from server: '
-                    f'{exception.response}'
-                )
-            else:
-                logger.error(f'Error when trying to refresh oAuth token: {exception}')
+        except requests.exceptions.HTTPError as http_error:
+            logger.error(f'Error while trying to refresh oAuth token: {http_error=}')
+            if i == 0:
+                raise
+        except Exception as e:
+            logger.error(f'Error while trying to refresh oAuth token: {e}')
 
             # we only abort here if the first rotation failed (assuming that the token was not
             # refreshed). Otherwise we keep on rotating keys/tokens to prevent the previously

checkmarx/client.py

@@ -83,13 +83,14 @@ def __init__(self, checkmarx_cfg: model.checkmarx.CheckmarxConfig):
         self.routes = CheckmarxRoutes(base_url=checkmarx_cfg.base_url())
         self.config = checkmarx_cfg
         self.auth = None
+        self.session = requests.sessions.Session()
 
     def _auth(self):
         if self.auth and self.auth.is_valid():
             return self.auth
 
         creds = self.config.credentials()
-        res = requests.post(
+        res = self.session.post(
             self.routes.auth(),
             data={
                 'username': creds.qualified_username(),
@@ -99,7 +100,10 @@ def _auth(self):
                 'scope': creds.scope(),
                 'grant_type': 'password',
             },
+            timeout=(3, 30),
         )
+        res.raise_for_status()
+
         res = cxmodel.AuthResponse(**res.json())
         res.expires_at = datetime.datetime.fromtimestamp(
             datetime.datetime.now().timestamp() + res.expires_in - 10
@@ -121,7 +125,18 @@ def request(
         if 'Accept' not in headers:
             headers['Accept'] = f'application/json;v={api_version}'
 
-        res = requests.request(method=method, headers=headers, *args, **kwargs)
+        try:
+            timeout = kwargs.pop('timeout')
+        except KeyError:
+            timeout = (3, 30)
+
+        res = self.session.request(
+            method=method,
+            headers=headers,
+            timeout=timeout,
+            *args,
+            **kwargs,
+        )
 
         if not res.ok:
             msg = f'{method} request to {res.url=} failed with {res.status_code=} {res.reason=}'

concourse/client/api.py

@@ -105,12 +105,8 @@ class ConcourseApiBase:
     def __init__(
         self,
         routes: ConcourseApiRoutesBase,
-        request_builder: AuthenticatedRequestBuilder=None,
         verify_ssl=False,
     ):
-        if request_builder:
-            logger.warning('passing-in requests-builder is deprecated')
-
         self.routes = routes
         self.request_builder = None
         self.verify_ssl = verify_ssl
@@ -156,7 +152,8 @@ def login(self, username: str, passwd: str):
             url=login_url,
             data=form_data,
             auth=(AUTH_TOKEN_REQUEST_USER, AUTH_TOKEN_REQUEST_PWD),
-            headers={'content-type': 'application/x-www-form-urlencoded'}
+            headers={'content-type': 'application/x-www-form-urlencoded'},
+            timeout=(3, 30),
         )
         response.raise_for_status()
         auth_token = response.json()[self.AUTH_TOKEN_ATTRIBUTE_NAME]

ctt/util.py

@@ -89,6 +89,7 @@ def sign_with_signing_server(
         headers=headers,
         data=content,
         verify=root_ca_cert_path,
+        timeout=(3, 30),
     )
     response.raise_for_status()