Skip to content

Commit

Permalink
fix handling of omitted token/url combinations in RequestAuthHandler …
Browse files Browse the repository at this point in the history
…- undetected by CI due to get_request_options invocation with empty settings followed by requests mock never leading to actual requests call
  • Loading branch information
fmigneault committed Jan 29, 2025
1 parent 0ef74f3 commit 9401a5b
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 10 deletions.
1 change: 0 additions & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ jobs:
env | sort
- name: Run Tests
run: make stop ${{ matrix.test-case }}
continue-on-error: ${{ matrix.test-case == 'test-coverage-only' || matrix.allow-failure }}

# manually invoke reporting in case of test failure to still generate them
# otherwise, they would have been generated automatically following the successful coverage run
Expand Down
3 changes: 2 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ Changes:

Fixes:
------
- No change.
- Fix ``weaver.cli.RequestAuthHandler`` and its derived classes erroneously invoking ``request_auth`` method when
both the ``url`` and ``token`` are omitted, leading to invalid ``requests`` call under ``weaver.utils.request_extra``.

.. _changes_6.1.1:

Expand Down
33 changes: 29 additions & 4 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
WeaverClient,
main as weaver_cli
)
from weaver.exceptions import AuthenticationError
from weaver.formats import ContentEncoding, ContentType


Expand Down Expand Up @@ -408,7 +409,7 @@ def test_auth_handler_basic():

def test_auth_handler_bearer():
req = WebTestRequest({})
auth = BearerAuthHandler(identity=str(uuid.uuid4()))
auth = BearerAuthHandler(identity=str(uuid.uuid4()), url="https://example.com")
token = str(uuid.uuid4())
with mock.patch(
"requests.Session.request",
Expand All @@ -435,7 +436,7 @@ def test_auth_handler_bearer_explicit_token_matches_request_token():
req_request_token = WebTestRequest({})
token = str(uuid.uuid4())
auth_explicit_token = BearerAuthHandler(token=token)
auth_request_token = BearerAuthHandler(identity=str(uuid.uuid4()))
auth_request_token = BearerAuthHandler(identity=str(uuid.uuid4()), url="https://example.com")
with mock.patch(
"requests.Session.request",
side_effect=lambda *_, **__: mocked_auth_response("access_token", token)
Expand All @@ -450,7 +451,7 @@ def test_auth_handler_bearer_explicit_token_matches_request_token():

def test_auth_handler_cookie():
req = WebTestRequest({})
auth = CookieAuthHandler(identity=str(uuid.uuid4()))
auth = CookieAuthHandler(identity=str(uuid.uuid4()), url="https://example.com")
token = str(uuid.uuid4())
with mock.patch(
"requests.Session.request",
Expand Down Expand Up @@ -506,7 +507,7 @@ def test_auth_handler_cookie_explicit_token_matches_request_token():
req_request_token = WebTestRequest({})
token = str(uuid.uuid4())
auth_explicit_token = CookieAuthHandler(token=token)
auth_request_token = CookieAuthHandler(identity=str(uuid.uuid4()))
auth_request_token = CookieAuthHandler(identity=str(uuid.uuid4()), url="https://example.com")
with mock.patch(
"requests.Session.request",
side_effect=lambda *_, **__: mocked_auth_response("access_token", token)
Expand All @@ -519,6 +520,30 @@ def test_auth_handler_cookie_explicit_token_matches_request_token():
assert resp_explicit_token.headers["Cookie"] == resp_request_token.headers["Cookie"]


def test_auth_request_handler_no_url_or_token_init():
with pytest.raises(AuthenticationError):
BearerAuthHandler(identity=str(uuid.uuid4()))

try:
BearerAuthHandler(token=str(uuid.uuid4())) # OK
BearerAuthHandler(url="https://example.com") # OK
except Exception as exc:
pytest.fail(msg=f"Expected no init error from valid combinations. Got [{exc}]")


def test_auth_request_handler_no_url_ignored_request():
req = WebTestRequest({})
auth = BearerAuthHandler(
identity=str(uuid.uuid4()),
url="https://example.com", # URL must be passed to avoid error
)
auth.url = None # reset after init check
with mock.patch("requests.Session.request") as mock_request:
resp = auth(req) # type: ignore
mock_request.assert_not_called()
assert not resp.headers, "No headers should have been added since URL could not be resolved."


def test_upload_file_not_found():
with tempfile.NamedTemporaryFile() as tmp_file_deleted:
pass # delete on close
Expand Down
9 changes: 6 additions & 3 deletions weaver/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

from weaver import __meta__
from weaver.datatype import AutoBase
from weaver.exceptions import PackageRegistrationError
from weaver.exceptions import AuthenticationError, PackageRegistrationError
from weaver.execute import ExecuteMode, ExecuteResponse, ExecuteReturnPreference, ExecuteTransmissionMode
from weaver.formats import ContentEncoding, ContentType, OutputFormat, get_content_type, get_format, repr_json
from weaver.processes.constants import ProcessSchema
Expand Down Expand Up @@ -278,6 +278,9 @@ def __init__(
HTTPBasicAuth.__init__(self, username=identity, password=password)
self.token = token

if not self.token and not self.url:
raise AuthenticationError("Either the token or the URL to retrieve it must be provided to the handler.")

@property
def auth_token_name(self):
# type: () -> str
Expand Down Expand Up @@ -334,12 +337,12 @@ def response_parser(self, response):

def __call__(self, request):
# type: (AnyRequestType) -> AnyRequestType
if self.token is None:
if self.token is None and self.url:
auth_token = self.request_auth()
else:
auth_token = self.token
if not auth_token:
LOGGER.warning("Expected authorization token could not be retrieved from: [%s] in [%s]",
LOGGER.warning("Expected authorization token could not be retrieved from URL: [%s] in [%s]",
self.url, fully_qualified_name(self))
else:
auth_token = self.parse_token(auth_token)
Expand Down
8 changes: 7 additions & 1 deletion weaver/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ class WeaverExecutionError(WeaverException):
"""


class AuthenticationError(WeaverException):
"""
Generic exception related to an issue about authentication configuration or resolution.
"""


class ListingInvalidParameter(WeaverException, OWSInvalidParameterValue, ValueError):
"""
Error related to an invalid parameter for listing queries.
Expand Down Expand Up @@ -265,7 +271,7 @@ class PackageRegistrationError(HTTPInternalServerError, OWSNoApplicableCode, Pac
"""


class PackageAuthenticationError(HTTPForbidden, OWSAccessForbidden, PackageException):
class PackageAuthenticationError(HTTPForbidden, OWSAccessForbidden, PackageException, AuthenticationError):
"""
Error related to a runtime failure caused by failing authentication prerequisite.
Expand Down

0 comments on commit 9401a5b

Please sign in to comment.