-
Notifications
You must be signed in to change notification settings - Fork 4.3k
fix(cloudfront): add require Function URL permissions for OAC #35919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(cloudfront): add require Function URL permissions for OAC #35919
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request linter fails with the following errors:
❌ Fixes must contain a change to an integration test file and the resulting snapshot.
If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.
✅ A exemption request has been requested. Please wait for a maintainer's review.
|
Exemption Request : This is a bug fix for a missing required permission, and updating the existing integration test snapshots is sufficient without adding a new test file. |
badmintoncryer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've added some comments.
If you have any questions, please don't hesitate to reach out to me on GitHub or X (feel free to write in Japanese!).
|
|
||
| new lambda.CfnPermission(scope, `InvokeFromApiFor${options.originId}`, { | ||
| principal: 'cloudfront.amazonaws.com', | ||
| new lambda.CfnPermission(scope, `InvokeFunctionUrlFromCloudFrontFor${options.originId}`, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, Avoided updating the construct IDs: f1cc348.
|
|
||
| private addInvokePermission(scope: Construct, options: cloudfront.OriginBindOptions) { | ||
| const distributionId = options.distributionId; | ||
| const sourceArn = `arn:${cdk.Aws.PARTITION}:cloudfront::${cdk.Aws.ACCOUNT_ID}:distribution/${distributionId}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to use Stack.of(scope).formatArn() function to create an ARN string.
Stack.of(scope).formatArn({
service: 'cloudfront',
resource: 'distribution',
resourceName: distributionId,
arnFormat: ArnFormat.SLASH_RESOURCE_NAME,
}),There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the suggestion! Updated to use Stack.of(scope).formatArn(), including region: '' for correct global CloudFront ARN formatting.
Fixed: 9400a40.
| new lambda.CfnPermission(scope, `InvokeFunctionFromCloudFrontFor${options.originId}`, { | ||
| principal: principal, | ||
| action: 'lambda:InvokeFunction', | ||
| functionName: this.functionUrl.functionArn, | ||
| sourceArn: sourceArn, | ||
| invokedViaFunctionUrl: true, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This modification is also a breaking change, so introducing a feature flag is required.
Unlike the construct ID update, this one is a mandatory change, so it seems you’ll have to introduce a feature flag after all...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your valuable feedback.
With that in mind, I believe we should align with the core aws-lambda package and implement this permission addition as the new default behavior immediately, without a flag. The core package treated this same Dual Auth fix as a mandatory fix to prevent service disruption after the November 1, 2026, deadline (PR #35725), implementing it without requiring opt-in.
Could you please share your thoughts on this approach?
(As this is my first PR to this repository, please forgive me if I've made any mistakes or if my understanding is flawed.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@c19yamamoto Thank you for the detailed explanation! I understand that breaking changes are already being accepted on the Lambda side.
The PR you sent was issued by a maintainer, so there must have been internal agreement to allow breaking changes.
I'm not sure how this particular case will be handled, but since there's precedent, I'll approve it on my end!
Please continue working with the maintainers on this.
|
@c19yamamoto Cloud you please update your PR title?
|
|
@c19yamamoto Could you please merge the latest main branch? |
Issue # (if applicable)
Closes #35872.
Reason for this change
AWS Lambda's Dual Auth requirement (introduced in PR #35725) now requires both
lambda:InvokeFunctionUrlANDlambda:InvokeFunction(withinvokedViaFunctionUrl: truecondition) for Function URL invocations.However, the
FunctionUrlOriginWithOAC.addInvokePermission()method currently grants only thelambda:InvokeFunctionUrlpermission:aws-cdk/packages/aws-cdk-lib/aws-cloudfront-origins/lib/function-url-origin.ts
Lines 144 to 153 in 75139b2
Consequently, when integrating a Lambda Function URLs with CloudFront's OAC using the
aws_cloudfront_origins.FunctionUrlOrigin.withOriginAccessControlconstruct, the resulting resource-based policy grants the CloudFront Service Principal onlylambda:InvokeFunctionUrl. It is currently missing the requiredlambda:InvokeFunctionpermission.Description of changes
Added a second permission grant for
lambda:InvokeFunctionwithinFunctionUrlOriginWithOAC.addInvokePermission. This grant includes theinvokedViaFunctionUrl: truecondition, following the dual-permission pattern implemented ingrantInvokeUrl()in the Lambda module.This is implemented as the new default behavior immediately, without an feature flag. This aligns with the core aws-lambda package's precedent for implementing mandatory service fixes as non-optional defaults (PR #35725).
Describe any new or updated permissions being added
The change updates the resource-based policy for the Lambda Function URL to include an additional
lambda:InvokeFunctionpermission grant, which is now required by AWS Lambda's dual-authentication mechanism for Function URL invocations, especially when using CloudFront Origin Access Control (OAC).Specifically, the following permission is added to the Lambda's resource-based policy:
lambda:InvokeFunctioncloudfront.amazonaws.com(CloudFront Service Principal)invokedViaFunctionUrl: trueDescription of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license