New serverless pattern APIGW-APIKey-tenantid-mapping#3124
Conversation
… to tenant" This reverts commit 004c876.
…Ikeyauthorization apigw tenantid pattern
| // API Gateway | ||
| const api = new apigateway.RestApi(this, "ApiGateway", { | ||
| restApiName: "DynamoDB API Key Protected Service", | ||
| description: "API protected with DynamoDB-based API key authorization", | ||
| }); | ||
|
|
||
| // Token authorizer using Authorization header (Cognito JWT) | ||
| const lambdaAuthorizer = new apigateway.TokenAuthorizer(this, "TokenAuthorizer", { | ||
| handler: authorizerFn, | ||
| identitySource: "method.request.header.Authorization", | ||
| }); | ||
|
|
||
| // Protected endpoint with mock integration | ||
| const protectedResource = api.root.addResource("protected"); | ||
|
|
||
| protectedResource.addMethod( | ||
| "GET", | ||
| new apigateway.MockIntegration({ | ||
| integrationResponses: [ | ||
| { | ||
| statusCode: "200", | ||
| responseTemplates: { | ||
| "application/json": '{ "message": "Access granted" }', | ||
| }, | ||
| }, | ||
| ], | ||
| passthroughBehavior: apigateway.PassthroughBehavior.NEVER, | ||
| requestTemplates: { | ||
| "application/json": '{ "statusCode": 200 }', | ||
| }, | ||
| }), | ||
| { | ||
| authorizer: lambdaAuthorizer, | ||
| methodResponses: [{ statusCode: "200" }], |
There was a problem hiding this comment.
The whole point of the pattern as per the README and title is mapping a tenant to an API Gateway usage plan via an API key so throttling is enforced. But the stack creates no apigateway.UsagePlan, no apigateway.ApiKey, no usage-plan to key association, and never sets the REST API's apiKeySource to AUTHORIZER. Returning usageIdentifierKey: apiKey from the authorizer has no effect unless
(a) apiKeySource = AUTHORIZER on the API, and
(b) the returned key value matches an API key that is attached to a usage plan applied to the stage/method.
Without those, the authorizer's API-key lookup is dead code and the pattern does not actually throttle anyone. The security/operational guarantee it advertises is absent.
There was a problem hiding this comment.
I have added steps to create and use usage plans while testing. The pattern is not a full solution and I have specified some pre requisites in readme
There was a problem hiding this comment.
Added apiKeySource = AUTHORIZER
There was a problem hiding this comment.
Thanks Lavanya!
apiKeySource = AUTHORIZER is the right call and documenting the usage plan + API key as a prerequisite is a reasonable approach for a starter pattern.
One thing to align on before merge: as deployed, cdk deploy alone doesn't produce a working throttled API. The user has to manually create the usage plan + API key and match the key value to what's in DynamoDB. Two options:
(Preferred, test before use) Provision it in the stack so the demo works end-to-end:
protectedResource.addMethod('GET', integration, {
authorizer: lambdaAuthorizer,
apiKeyRequired: true,
methodResponses: [{ statusCode: '200' }],
});
const plan = api.addUsagePlan('TenantUsagePlan', { throttle: { rateLimit: 10, burstLimit: 20 } });
plan.addApiStage({ stage: api.deploymentStage });
const key = api.addApiKey('SampleTenantKey', { value: 'my-api-key-123-456789012345' }); // >= 20 chars
plan.addApiKey(key);
(Acceptable) Keep it manual, but please make the README Prerequisites section explicit that the demo is non-functional until those steps are done, and note the API key value must be ≥ 20 characters and must match the DynamoDB-stored value.
Let me know which direction you'd prefer either is fine, I just want the deployed behavior and the docs to agree.
|
|
||
| if (!tenantId) { | ||
| throw new Error("Unauthorized: No tenant ID in claims"); | ||
| } |
There was a problem hiding this comment.
The authorizer extracts the tenantId by base64url-decoding the middle segment of the JWT and reading custom:tenantId it never validates the token's signature, issuer (iss), audience (aud/client_id), expiry (exp), or token_use. The inline comment ("Cognito token is already validated by API Gateway if needed") is incorrect: a Lambda TOKEN authorizer receives the raw header value and is itself responsible for authenticating the caller (per the Lambda authorizer workflow, step 3 which says "The Lambda function authenticates the caller"). API Gateway does not pre-validate a JWT for a custom authorizer. As written, an attacker can craft an unsigned JWT (header.{"custom:tenantId":"victim-tenant"}.) with any tenant ID, if that tenant exists in the table, the request is authorized and billed/throttled against the victim tenant. This is a full authorization bypass and undermines the pattern's stated security purpose.
Recommended fix: Verify the token against the Cognito User Pool JWKS before trusting any claim. Use aws-jwt-verify (the AWS-published library) and create the verifier outside the handler so the JWKS is cached across invocations.
This would require passing USER_POOL_ID and CLIENT_ID into the function's environment in the stack, and adding aws-jwt-verify as a bundled dependency (do not add it to externalModules)
| const userPoolClient = userPool.addClient("TenantUserPoolClient", { | ||
| authFlows: { userPassword: true }, | ||
| }); |
There was a problem hiding this comment.
authFlows: { userPassword: true } enables USER_PASSWORD_AUTH, which sends the raw password to Cognito. AWS recommends USER_SRP_AUTH (Secure Remote Password) so the password is never transmitted. For a demo USER_PASSWORD_AUTH keeps the get-token.js helper simple, but the README/pattern should at least call out that SRP is preferred for production, and ideally default to it, or you may want to use SRP if possible.
Recommended Fix: Prefer authFlows: { userSrp: true } and use an SRP-capable client in the helper, or document the trade-off explicitly in the README,
There was a problem hiding this comment.
added instructions in README
| const userPool = new cognito.UserPool(this, "TenantUserPool", { | ||
| selfSignUpEnabled: false, | ||
| signInAliases: { email: true }, | ||
| customAttributes: { | ||
| tenantId: new cognito.StringAttribute({ mutable: false }), | ||
| }, | ||
| removalPolicy: cdk.RemovalPolicy.DESTROY, | ||
| }); |
There was a problem hiding this comment.
The user pool sets no explicit passwordPolicy, no MFA, and no advanced security mode. Defaults are reasonable for a demo, but as this pattern is for "secure tenant-based", it's recommended to either set a password policy (preferred) / MFA or note these as production hardening steps in the README.
Recommended Fix: Add a passwordPolicy and consider mfa: cognito.Mfa.OPTIONAL
| @@ -0,0 +1,74 @@ | |||
| { | |||
There was a problem hiding this comment.
The repo's schema-validation workflow expects the metadata file to be named exactly example-pattern.json at the pattern root.
| { | ||
| "name": "Lavanya Tangutur", | ||
| "bio": "Lavanya Tangutur serves as a Senior Technical Account Manager at AWS ocused on helping customers build, deploy, and run secure, resilient, and cost-effective workloads on AWS.", | ||
| "linkedin": "www.linkedin.com/in/lavanyatangutur" |
There was a problem hiding this comment.
Only LinkedIn handle should be added
There was a problem hiding this comment.
Bio is good to have. I meant just linkedId handle is needed, not full URL
| "authors": [ | ||
| { | ||
| "name": "Lavanya Tangutur", | ||
| "bio": "Lavanya Tangutur serves as a Senior Technical Account Manager at AWS ocused on helping customers build, deploy, and run secure, resilient, and cost-effective workloads on AWS.", |
There was a problem hiding this comment.
Please add an image URL if you'd like that to appear with name & bio on serverlessland.com
| 1. Install dependencies: | ||
| ``` | ||
| npm install | ||
| ``` |
There was a problem hiding this comment.
Missing cdk bootstrap step, important for first time users
|
|
||
| Note the outputs from the CDK deployment process. The output will include the API Gateway URL, DynamoDB table name, Cognito User Pool ID, and User Pool Client ID. | ||
|
|
||
| ## How it works |
There was a problem hiding this comment.
The section says the API key is "returned in the authorization context via usageIdentifierKey," but the deployed stack never creates a usage plan or API key, and the test steps put a "apiKey": "my-api-key-123" item into DynamoDB that is shorter than the 20-character minimum for a real API key and is never associated with any plan. So even after following the README, throttling is not clearly demonstrated.
There was a problem hiding this comment.
I have added instructions for usage plan and APIkey in the testing section
There was a problem hiding this comment.
Minor naming items: the feature is "Lambda authorizer" (lowercase "authorizer") per AWS docs; first references should be "AWS Lambda", "Amazon API Gateway", "Amazon Cognito", "Amazon DynamoDB" (the README mostly does this well). The pattern folder is apigw-APIKey-tenantid-cdk with mixed-case APIKey repo convention is all-lowercase hyphenated slugs (e.g., apigw-apikey-tenantid-cdk).
Heads up that the example-pattern.json repoURL/projectFolder already use the mixed-case folder, so renaming the folder requires updating those too.
There was a problem hiding this comment.
The script name implies it deploys DynamoDB, but it actually runs cdk deploy for the whole stack with a hardcoded --app override duplicating cdk.json. The README never references it, so it's an undocumented second deploy path that can drift from cdk.json. It also runs npm install redundantly.
There was a problem hiding this comment.
This script is not required. so removed it
There was a problem hiding this comment.
package.json > bin points at bin/apigw-dynamodb-apikey-cdk.js (no such file, sources live under src/bin and src/lib). cdk.json app uses npx ts-node ... src/bin/apigw-dynamodb-apikey-cdk.ts (correct). The example-pattern.json templateFile says src/lib/apigw-dynamodb-apikey-stack.ts. The deploy_dynamodb.sh re-specifies the app on the CLI. This scattered/contradictory wiring is confusing and the package.json bin entry is simply wrong.
Recommended Fix: Standardize on the src/-based layout, fix the package.json bin path (or remove it), and ensure cdk.json is the single source of truth for the app command. Remove deploy_dynamodb.sh so there's one documented deploy path.
There was a problem hiding this comment.
Thanks for standardizing. cdk.json seems correct now.
One leftover: package.json "bin" points at apigw-apikey-tenantid-cdk.js, but the actual entry file is
apigw-dynamodb-apikey-cdk.ts (which is what cdk.json app uses). So this bin path references a file that doesn't exist. Please either fix the filename to match (apigw-dynamodb-apikey-cdk) or drop the bin entry entirely since cdk.json already defines the app.
| import { DynamoDBClient, GetItemCommand } from "@aws-sdk/client-dynamodb"; | ||
| const client = new DynamoDBClient(); | ||
|
|
||
| const TABLE_NAME = process.env.TABLE_NAME; | ||
|
|
||
| exports.handler = async (event) => { |
There was a problem hiding this comment.
The authorizer uses import { DynamoDBClient } ... ECMAScript Modules at the top but exports with exports.handler = ... (CommonJS). Mixing ESM import with CommonJS exports in the same .js file is invalid and may fail at runtime/bundling depending on how esbuild resolves it. NodejsFunction bundles with esbuild, but the handler contract must be consistent i.e. either full ESM (export const handler) or full CommonJS (const { DynamoDBClient } = require(...) + exports.handler).
Current Code/Configuration:
import { DynamoDBClient, GetItemCommand } from "@aws-sdk/client-dynamodb";
// ...
exports.handler = async (event) => { ... };
Recommended Fix: Pick one module system. For ESM: export const handler = async (event) => { ... }. For CJS: const { DynamoDBClient, GetItemCommand } = require("@aws-sdk/client-dynamodb");.
| 1. Client authenticates with Amazon Cognito and receives a JWT (ID token) containing the custom `tenantId` claim | ||
| 2. Client makes a request to the API with the JWT in the `Authorization` header | ||
| 3. API Gateway forwards the token to the Lambda Authorizer | ||
| 4. The Lambda Authorizer decodes the JWT, extracts the `custom:tenantId` claim, and looks up the tenant in the DynamoDB table |
There was a problem hiding this comment.
Step 4 says the Lambda authorizer "decodes the JWT" now that it uses aws-jwt-verify, it actually verifies (signature, exp, aud, iss, token_use). Suggest updating to "verifies the JWT" so the docs match the implementation. Small but worth it since the whole point of the earlier fix was that decoding ≠ verifying.
Issue #, if available:
Description of changes:
API Gateway usage plan tenant-id mapping
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.