fix(event_handler): security scheme unhashable list when working with router

[leandrodamascena]

Issue number: #4375

Summary

Changes

This pull request ensures that the get_openapi_json_schema method can correctly generate the OpenAPI JSON schema for routes that include security parameters when using the APIGatewayHttpRouter.

Previously, when the security parameter was included in a router, any endpoint would return an "Internal Server Error".

[ERROR] TypeError: unhashable type: 'list' Traceback (most recent call last): File "/var/lang/lib/python3.12/importlib/__init__.py", line 90, in import_module return _bootstrap._gcd_import(name[level:], package, level) File "<frozen importlib._bootstrap>", line 1387, in _gcd_import File "<frozen importlib._bootstrap>", line 1360, in _find_and_load File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 935, in _load_unlocked File "<frozen importlib._bootstrap_external>", line 995, in exec_module File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed File "/var/task/app.py", line 14, in <module> from project_member_v1 import router as project_member_router File "/var/task/project_member_v1/router.py", line 27, in <module> @router.get( File "/opt/python/aws_lambda_powertools/event_handler/api_gateway.py", line 2412, in register_route self._routes_with_middleware[route_key] = []

Example

app.py

from aws_lambda_powertools.event_handler import APIGatewayHttpResolver
from router import OpenApiConfiguration, router


app = APIGatewayHttpResolver(enable_validation=True)
OpenApiConfiguration.configure_swagger(app)


app.include_router(router, prefix="/v1/test")

@app.get("/openapispec", tags=["Docs"], summary="Get the OpenAPI specification", security=[{"TestAuth": []}])
def get_openapi():
    return app.get_openapi_json_schema(**OpenApiConfiguration.get_meta_data())

def lambda_handler(event, context):
    return app.resolve(event, context)

router.py

from aws_lambda_powertools.event_handler import APIGatewayHttpResolver
from aws_lambda_powertools.event_handler.api_gateway import Router
from aws_lambda_powertools.event_handler.openapi.models import HTTPBearer

router = Router()

@router.get(
    "/<test_id>",
    tags=["Test"],
    summary="Test endpoint",
    security=[{"TestAuth": ["a"]}],
)
def get_test_item(test_id: str) :
    return "OK"

class OpenApiConfiguration:
    @staticmethod
    def get_meta_data():
        return {
            "title": "TestAPI",
            "version": "0.0.1",
            "description": "TestAPI",
            "terms_of_service": None,
            "license_info": None,
            "openapi_version": "3.0.1",
            "security_schemes": {
                "TestAuth": HTTPBearer(scheme="bearer", bearerFormat="JWT", description="JWT token authentication"),
            },
        }

    @staticmethod
    def configure_swagger(resolver: APIGatewayHttpResolver) -> None:
        resolver.enable_swagger(
            **OpenApiConfiguration.get_meta_data(),
            path="/swagger",
            swagger_base_url="https://cdnjs.cloudflare.com/ajax/libs/swagger-ui/5.11.0"
        )

User experience

Please share what the user experience looks like before and after this change

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• Meet tenets criteria
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented
• PR title follows conventional commit semantics

Is this a breaking change?

RFC issue number:

Checklist:

• Migration process documented
• Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[heitorlessa]

one tiny change, a few suggestions to improve tests, and one major question to prevent another bug

[heitorlessa]

Adding a summary shortly after our call that led to important discoveries

[heitorlessa]

last changes before merging the PR.

• Suggested a multi-expression and slightly better perf for hashing (along w/ explanation)
• Suggested a change in tests to make it more explicit that there is a setup for OpenAPI until we have a new API to solve the ambiguity
• We're missing a test for Security Scheme mismatch. Top-level says ApiKey but path level uses a JWT.

If you've got time, it'd be great to already address other pieces we missed prior to this PR

• An OpenAPI Schema specific exception, not ValueError. The exceptions we're returning can be more actionable too (we can start with the security scheme)

Thank you so much for the hard work on this <3

[heitorlessa]

For anyone reading this later...

As part of the security scheme bug fix review (#4421), we found that the responsibilities between OpenAPI and SwaggerUI are ambiguous, leading to confusion among customers and maintainers at times.

As of now, these are common use cases:

1. Setting up OpenAPI. Customers use a combination of implicit type annotations, OpenAPI params at the route level, and extra params via enable_swagger.
2. Setting up SwaggerUI. Customers use enable_swagger to render the implicit OpenAPI Schema in a web app (SwaggerUI), customize assets, etc.
3. Serializing OpenAPI JSON Schema. Customers create an explicit route e.g., /schema, and use app.get_openapi_json_schema to serialize it. Those using SwaggerUI can download it at the homepage (https://endpoint.url/swagger?format=json).

We're confusing customers on 1 and 2, because enable_swagger or get_openapi_*schema is the only way to further extend / override OpenAPI. Customers should not need to use enable_swagger or get_openapi_*schema to further customize OpenAPI.

How do we fix it in a backwards compatible way?

need two new issues

• Introduce a new API to configure / extend OpenAPI in the resolver instance (app)
• Improve docs by surfacing that API instead of get_openapi_*schema and enable_swagger - example
• Rename and revamp OpenAPI docs sections to match the use-case driven docs we have. For now, it should be easier to find how to
  • extend OpenAPI spec
  • export OpenAPI schema programmatically
  • override OpenAPI schema when exporting
  • when to override OpenAPI metadata in SwaggerUI

[leandrodamascena]

Thanks for the review and suggestions @heitorlessa!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.36%. Comparing base (e14e768) to head (e27bebf).
Report is 539 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4421      +/-   ##
===========================================
- Coverage    96.38%   96.36%   -0.02%     
===========================================
  Files          214      219       +5     
  Lines        10030    10502     +472     
  Branches      1846     1944      +98     
===========================================
+ Hits          9667    10120     +453     
- Misses         259      270      +11     
- Partials       104      112       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[heitorlessa]

checking in 👀 ;)

[heitorlessa]

THANK YOU for working out the missing tests <3

Made last round of suggestions and another accidental bug in the validator. We can merge afterwards

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[heitorlessa]

suuuuperb working in going through bugs, finding new ones, and improving that section of the docs. It's also great that we identified the whole OpenAPI docs need a big revamp, and that we need a new API to make this easier and less confusing <3