APIGW NG implement api key validation handler #11114

cloutierMat · 2024-06-28T21:23:02Z

Motivation

Implementation of a new api key validation handler. There is a mix of moto helpers in the implementation as not all relevant information live in the deployment.

Important configuration in the frozen deployment:

The resource method describes apiKeyRequired, which determines if validation is required or skipped.
The rest api describes apiKeySource, which determines if the key is to be retrieved from the headers or from an identity context populated by an Lambda Authorizer

Important configuration in apigateway store.

Those items, when created, modified or deleted, will affect a deployment without the need to redeploy. see https://docs.aws.amazon.com/apigateway/latest/developerguide/updating-api.html

Api keys and their properties are stored in moto (contradicts the documentation, but manually tested that updating enabled from the key will affect without redeploying)
Usage plans are stored in moto
Usage plan key (association of a key to a usage plan) are also stored in moto

Changes

Finds and validates api key based on deployment configuration and keys/usage plans stored on the backend
Updates the $context variables with relevant data.
Added moto_helpers to keep moto backend isolated and ease technical debt.
Added tests to validate functionality

TODO

What's left to do:

This PR currently targets add context variables and stage variables to API NG context #11111, as it requires changes to the imports. When merged, we should rebase and target master

bentsku

LGTM! 🚀 extremely well covered PR, the tests are really great, love the abstractions and usage of fixtures.

The handler is looking extremely good, well documented, nicely encapsulated functions and helpers, this is awesome 💯

edit: I've merged #11111 so I've took the liberty to rebase this PR on master and push it so I can create branches based on top of #11114 😄

bentsku · 2024-07-01T12:12:02Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/moto_helpers.py

+    Will return an api key from the moto store.
+    """
+    apigateway_backend: APIGatewayBackend = apigateway_backends[account_id][region_name]
+    return ApiKey(**apigateway_backend.keys[api_key_id].to_json())


I know it's not supposed to happen, but should we protect against KeyError here?
Also, I believe we could avoid the ** destructing into typed dict by doing the following:

Suggested change

return ApiKey(**apigateway_backend.keys[api_key_id].to_json())

api_key: ApiKey = apigateway_backend.keys[api_key_id].to_json()

return api_key

Not sure how to do it with the list comprehension, but the creation of a typed dict by de-structuring a dict is not really required, as we're just passing dict around.

Actually realised I don't even need to go that far and and with simply return apigateway_backend.keys[api_key_id].to_json() all the typing works fine. Which I can also use to simplify the code of get_usage_plans and get_usage_plan_keys and remove the destructing while keeping typing!
👍

bentsku · 2024-07-01T14:10:15Z

...tack-core/localstack/services/apigateway/next_gen/execute_api/handlers/api_key_validation.py

+        if not (identity := context.context_variables.get("identity", ContextVarsIdentity())):
+            context.context_variables["identity"] = identity


You can simplify with setdefault(key, default_value) which does the same thing under the hood:

Suggested change

if not (identity := context.context_variables.get("identity", ContextVarsIdentity())):

context.context_variables["identity"] = identity

identity = context.context_variables.setdefault("identity", ContextVarsIdentity())

bentsku · 2024-07-01T14:14:04Z

...tack-core/localstack/services/apigateway/next_gen/execute_api/handlers/api_key_validation.py

+        LOG.debug("Updating $context.identity.apiKeyId='%s'", validated_key["id"])
+        identity["apiKeyId"] = validated_key["id"]
+
+    def validate_api_key(


I believe once we migrate UsagePlan and ApiKey to our store, seeing this usage, we should keep a map of API Key values (+ maybe stage?) directly in order to quickly validate them? We could maybe then simplify this to not have to iterate over all usage plans and usage plan keys.

Yes, I believe we can keep the item in the store in a way that will be more favourable to this specific access pattern. I don't think we will be able to avoid all looping, without creating some data duplication but we should be able to reduce the amount of work being done here.

github-actions · 2024-07-01T16:11:39Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 23m 29s ⏱️ - 1h 11m 46s
643 tests - 2 518 598 ✅ - 2 167 45 💤 - 351 0 ❌ ±0
645 runs - 2 518 598 ✅ - 2 167 47 💤 - 351 0 ❌ ±0

Results for commit 0149699. ± Comparison against base commit be9149c.

This pull request removes 2518 tests.

tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

cloutierMat added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Jun 28, 2024

cloutierMat added this to the 3.6 milestone Jun 28, 2024

cloutierMat self-assigned this Jun 28, 2024

cloutierMat requested a review from bentsku as a code owner June 28, 2024 21:23

cloutierMat mentioned this pull request Jun 28, 2024

make next-gen APIGW provider default #11035

Merged

2 tasks

cloutierMat changed the title ~~Apigw ng implement api key validation~~ APIGW NG implement api key validation Jun 28, 2024

cloutierMat changed the title ~~APIGW NG implement api key validation~~ APIGW NG implement api key validation handler Jun 28, 2024

bentsku mentioned this pull request Jul 1, 2024

add context variables and stage variables to API NG context #11111

Merged

1 task

bentsku approved these changes Jul 1, 2024

View reviewed changes

Base automatically changed from apigw-add-contexts to master July 1, 2024 14:47

cloutierMat added 4 commits July 1, 2024 17:53

Implement api key validation

7953964

Added robustness when the context is missing optional values

e6e0eec

added unit tests

ceef8f6

Updated imports

a324a08

bentsku force-pushed the apigw-ng-implement-api-key-validation branch from 8288e20 to a324a08 Compare July 1, 2024 15:54

pr comments

0149699

cloutierMat merged commit faf13f7 into master Jul 2, 2024
32 checks passed

cloutierMat deleted the apigw-ng-implement-api-key-validation branch July 2, 2024 19:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

APIGW NG implement api key validation handler #11114

APIGW NG implement api key validation handler #11114

Uh oh!

cloutierMat commented Jun 28, 2024 •

edited

Loading

Uh oh!

bentsku left a comment •

edited

Loading

Uh oh!

bentsku Jul 1, 2024

Uh oh!

cloutierMat Jul 2, 2024

Uh oh!

bentsku Jul 1, 2024

Uh oh!

bentsku Jul 1, 2024

Uh oh!

cloutierMat Jul 2, 2024

Uh oh!

github-actions bot commented Jul 1, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

	return ApiKey(**apigateway_backend.keys[api_key_id].to_json())
	api_key: ApiKey = apigateway_backend.keys[api_key_id].to_json()
	return api_key

		if not (identity := context.context_variables.get("identity", ContextVarsIdentity())):
		context.context_variables["identity"] = identity

	if not (identity := context.context_variables.get("identity", ContextVarsIdentity())):
	context.context_variables["identity"] = identity
	identity = context.context_variables.setdefault("identity", ContextVarsIdentity())

Uh oh!

APIGW NG implement api key validation handler #11114

APIGW NG implement api key validation handler #11114

Uh oh!

Conversation

cloutierMat commented Jun 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Important configuration in the frozen deployment:

Important configuration in apigateway store.

Changes

TODO

Uh oh!

bentsku left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bentsku Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

cloutierMat Jul 2, 2024

Choose a reason for hiding this comment

Uh oh!

bentsku Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

bentsku Jul 1, 2024

Choose a reason for hiding this comment

Uh oh!

cloutierMat Jul 2, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LocalStack Community integration with Pro

Uh oh!

Uh oh!

Uh oh!

cloutierMat commented Jun 28, 2024 •

edited

Loading

bentsku left a comment •

edited

Loading

github-actions bot commented Jul 1, 2024 •

edited

Loading