Merge pull request #122 from globus/release/0.15.0

Release 0.15.0

.github/workflows/ci.yaml

@@ -24,7 +24,7 @@ jobs:
 
       - name: "Setup Python"
         id: "setup-python"
-        uses: "actions/setup-python@65d7f2d534ac1bc67fcd62888c5f4f3d2cb2b236" # v4.7.1
+        uses: "actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c" # v5.0.0
         with:
           python-version: "3.12"
           cache: "pip"
@@ -73,7 +73,7 @@ jobs:
 
       - name: "Setup Python"
         id: "setup-python"
-        uses: "actions/setup-python@65d7f2d534ac1bc67fcd62888c5f4f3d2cb2b236" # v4.7.1
+        uses: "actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c" # v5.0.0
         with:
           python-version: "3.11"
           cache: "pip"
@@ -90,7 +90,7 @@ jobs:
           echo "wheel-filename=$(ls -1 globus_action_provider_tools-*.whl | head -n 1)" >> $GITHUB_OUTPUT
 
       - name: "Upload the artifact"
-        uses: "actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32" # v3.1.3
+        uses: "actions/upload-artifact@c7d193f32edcb7bfad88892161225aeda64e9392" # v4.0.0
         with:
           name: "globus_action_provider_tools-${{ github.sha }}.whl"
           path: "${{ steps.build-wheel.outputs.wheel-filename }}"
@@ -154,7 +154,7 @@ jobs:
 
       - name: "Setup Python"
         id: "setup-python"
-        uses: "actions/setup-python@65d7f2d534ac1bc67fcd62888c5f4f3d2cb2b236" # v4.7.1
+        uses: "actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c" # v5.0.0
         with:
           python-version: "${{ matrix.python-version }}"
           cache: "pip"
@@ -190,7 +190,7 @@ jobs:
           ${{ env.venv-path }}/pip install tox
 
       - name: "Download the artifact"
-        uses: "actions/download-artifact@9bc31d5ccc31df68ecc42ccf4149144866c47d8a" # v3.0.2
+        uses: "actions/download-artifact@f44cd7b40bfd40b6aa1cc1b9b5b7bf03d3c67110" # v4.1.0
         with:
           name: "globus_action_provider_tools-${{ github.sha }}.whl"
 

.github/workflows/release.yml

@@ -16,7 +16,7 @@ jobs:
           fetch-depth: 1
 
       - name: Set target python version
-        uses: actions/setup-python@65d7f2d534ac1bc67fcd62888c5f4f3d2cb2b236 # v4.7.1
+        uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c # v5.0.0
         with:
           python-version: "3.11"
 

.pre-commit-config.yaml

@@ -24,16 +24,16 @@ repos:
         args: [--py38-plus]
 
   - repo: https://github.com/psf/black-pre-commit-mirror
-    rev: 23.9.1
+    rev: 23.12.1
     hooks:
       - id: black
 
   - repo: https://github.com/PyCQA/isort
-    rev: 5.12.0
+    rev: 5.13.2
     hooks:
       - id: isort
 
   - repo: https://github.com/sirosen/slyp
-    rev: 0.1.1
+    rev: 0.1.2
     hooks:
       - id: slyp

CHANGELOG.rst

@@ -10,6 +10,24 @@ Unreleased changes are documented in files in the `changelog.d`_ directory.
 
 ..  scriv-insert-here
 
+.. _changelog-0.15.0:
+
+0.15.0 — 2024-01-26
+===================
+
+Bugfixes
+--------
+
+- Groups were not being properly considered in authorization checks.
+
+Changes
+-------
+
+- Error descriptions in responses are now always strings (previously they could also
+  be lists of strings or lists of dictionaries).
+- Input validation errors now use an HTTP response status code of 422.
+- Validation errors no longer return input data in their description.
+
 .. _changelog-0.14.1:
 
 0.14.1 — 2023-10-27

README.rst

@@ -96,27 +96,6 @@ are no breaking changes introduced. Both these steps must be run for us to
 accept incoming changes. Once you feel your work is ready to be submitted, feel
 free to create a PR.
 
-PyPi Releases
--------------
-
-Please follow the steps below when creating a new release of the toolkit:
-
-- Create a new release branch
-    - git checkout -b release/X.Y.Z
-- Update the project's dependencies
-    - poetry update
-- Update the project version (follow semantic versioning) in pyproject.toml
-    - poetry version patch|minor|major
-- Update the project version in `globus_action_provider_tools/__init__.py`
-- Create a pull request into the main branch, wait for CI tests to complete
-- Merge the passing pull request
-- Create and publish a git tag for the new release
-    - git tag v$(poetry version -s)
-    - git push --tags
-- Create a new GH release that references the recently created tag. Provide
-  release notes with information on the changeset. Once the release is created,
-  there's a GH workflow that will build the toolkit and publish it to pypi.
-
 Links
 -----
 | Full Documentation: https://action-provider-tools.readthedocs.io

globus_action_provider_tools/authentication.py

@@ -331,25 +331,23 @@ def check_authorization(
         allow_public: bool = False,
         allow_all_authenticated_users: bool = False,
     ) -> bool:
-        allowed_set = frozenset(allowed_principals)
+        allowed_set = set(allowed_principals)
         all_principals = self.identities
         # We only need to merge in the groups values to the principals list if there are
         # group principals in the list. Can save a round trip to the Groups service if
         # there's no need to check for group membership.
         if AuthState.group_in_principal_list(allowed_set):
-            allowed_principals = set(allowed_principals).union(self.groups)
-        if (
+            allowed_set = allowed_set.union(self.groups)
+
+        return (
             (allow_public and "public" in allowed_set)
-            or (allowed_set.intersection(all_principals))
+            or bool(allowed_set.intersection(all_principals))
             or (
                 allow_all_authenticated_users
                 and "all_authenticated_users" in allowed_set
                 and len(self.identities) > 0
             )
-        ):
-            return True
-        else:
-            return False
+        )
 
 
 class TokenChecker:

globus_action_provider_tools/flask/exceptions.py

@@ -20,19 +20,13 @@
     InternalServerError,
     NotFound,
     Unauthorized,
+    UnprocessableEntity,
 )
 
 JSONType = t.Union[str, int, float, bool, None, t.Dict[str, t.Any], t.List[t.Any]]
 
 
 class ActionProviderToolsException(HTTPException):
-    # This is only required to update allow mypy to recognize the description
-    # can be any JSON-able structure
-    def __init__(self, description: t.Optional[JSONType] = None, *args, **kwargs):
-        if description is not None:
-            description = json.dumps(description)
-        super().__init__(description, *args, **kwargs)
-
     @property
     def name(self):
         return type(self).__name__
@@ -41,19 +35,13 @@ def get_body(self, *args):
         return json.dumps(
             {
                 "code": self.name,
-                "description": self.get_description(),
+                "description": self.description,
             }
         )
 
     def get_headers(self, *args):
         return [("Content-Type", "application/json")]
 
-    def get_description(self, *args):
-        try:
-            return json.loads(self.description)
-        except json.decoder.JSONDecodeError:
-            return self.description
-
 
 class ActionNotFound(ActionProviderToolsException, NotFound):
     pass
@@ -63,6 +51,13 @@ class BadActionRequest(BadRequest, ActionProviderToolsException):
     pass
 
 
+class RequestValidationError(UnprocessableEntity, BadActionRequest):
+    # TODO: This inherits from BadActionRequest to avoid breaking
+    # downstream code that expects to catch BadActionRequest when an error occurs
+    # during validation. Remove this inheritance in a future release.
+    pass
+
+
 class ActionConflict(ActionProviderToolsException, Conflict):
     pass
 

globus_action_provider_tools/flask/helpers.py

@@ -24,7 +24,7 @@
 from globus_action_provider_tools.flask.exceptions import (
     ActionProviderError,
     ActionProviderToolsException,
-    BadActionRequest,
+    RequestValidationError,
     UnauthorizedRequest,
 )
 from globus_action_provider_tools.flask.types import ActionCallbackReturn, ViewReturn
@@ -144,7 +144,8 @@ def validate_input(
         request_json = RequestObject.parse_obj(request_json).__root__
         action_request = ActionRequest(**request_json)
     except ValidationError as ve:
-        raise BadActionRequest(ve.errors())
+        messages = [f"Field '{'.'.join(e['loc'])}': {e['msg']}" for e in ve.errors()]
+        raise RequestValidationError("; ".join(messages))
 
     input_body_validator(action_request.body)
 
@@ -197,7 +198,7 @@ def json_schema_input_validation(
     """
     result = validate_data(action_input, validator)
     if result.errors:
-        raise BadActionRequest(result.errors)
+        raise RequestValidationError(result.error_msg)
 
 
 def pydantic_input_validation(
@@ -210,7 +211,8 @@ def pydantic_input_validation(
     try:
         validator(**action_input)
     except ValidationError as ve:
-        raise BadActionRequest(ve.errors())
+        messages = [f"Field '{'.'.join(e['loc'])}': {e['msg']}" for e in ve.errors()]
+        raise RequestValidationError("; ".join(messages))
 
 
 try:

globus_action_provider_tools/validation.py

@@ -45,17 +45,12 @@ def request_validator(request: ValidationRequest) -> ValidationResult:
 def validate_data(
     data: Dict[str, Any], validator: jsonschema.protocols.Validator
 ) -> ValidationResult:
-    error_messages = []
-    for error in validator.iter_errors(data):
-        if error.path:
-            # Elements of the error path may be integers or other non-string types,
-            # but we need strings for use with join()
-            error_path_for_message = ".".join([str(x) for x in error.path])
-            error_message = f"'{error_path_for_message}' invalid due to {error.message}"
-        else:
-            error_message = error.message
-        error_messages.append(error_message)
-
-    error_msg = "; ".join(error_messages) if error_messages else None
-    result = ValidationResult(errors=error_messages, error_msg=error_msg)
+    # TODO: If python-jsonschema introduces a means of returning error messages that
+    # do not include input data, modify this to return more specific error information.
+    if not validator.is_valid(data):
+        message = "Input failed schema validation"
+        result = ValidationResult(errors=[message], error_msg=message)
+    else:
+        result = ValidationResult(errors=[], error_msg=None)
+
     return result

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "globus-action-provider-tools"
-version = "0.14.1"
+version = "0.15.0"
 description = "Tools to help developers build services that implement the Action Provider specification."
 authors = [
     "Globus Team <support@globus.org>",

tests/test_exceptions.py

@@ -7,6 +7,7 @@
     ActionNotFound,
     ActionProviderError,
     BadActionRequest,
+    RequestValidationError,
     UnauthorizedRequest,
 )
 
@@ -18,6 +19,7 @@
         ActionNotFound,
         ActionProviderError,
         BadActionRequest,
+        RequestValidationError,
         UnauthorizedRequest,
     ],
 )

tests/test_flask_helpers/test_validation_helpers.py

@@ -9,6 +9,7 @@
 from globus_action_provider_tools.flask.exceptions import (
     ActionProviderError,
     BadActionRequest,
+    RequestValidationError,
 )
 from globus_action_provider_tools.flask.helpers import (
     get_input_body_validator,
@@ -88,19 +89,18 @@ def test_validating_action_request():
 
 
 @pytest.mark.parametrize(
-    "document, type_, message",
+    "document, message",
     (
-        ("wrong object type", "type_error.dict", "value is not a valid dict"),
-        ({1: "wrong key type"}, "type_error.str", "str type expected"),
+        ("wrong object type", "Field '__root__': value is not a valid dict"),
+        ({1: "wrong key type"}, "Field '__root__.__key__': str type expected"),
     ),
 )
-def test_validate_input_typeerror(document, type_, message):
+def test_validate_input_typeerror(document, message):
     """Verify that the `request_json` argument types are validated."""
 
     ap_description.input_schema = json.dumps(action_provider_json_input_schema)
     validator = get_input_body_validator(ap_description)
     with pytest.raises(BadActionRequest) as catcher:
         validate_input(document, validator)
-    assert catcher.value.get_response().status_code == 400
-    assert catcher.value.get_description()[0]["msg"] == message
-    assert catcher.value.get_description()[0]["type"] == type_
+    assert catcher.value.get_response().status_code == 422
+    assert catcher.value.description == message