Skip to content

Commit fd1fff9

Browse files
authored
feat(lambda): warn if you use function.grantInvoke while also using currentVersion (#19464)
‼️ Lambda is changing their authorization strategy, which means that some behavior that was previously valid now results in `access-denied` errors. Under the new behavior, customer lambda invocations will fail if the CDK generates a policy with an unqualified ARN as the resource, and the customer invokes lambda with the unqualified ARN and the `Qualifier` request parameter. Example of an affected setup: ``` Statement: { Effect: "Allow", Action: "lambda:InvokeFunction", Resource: "arn:aws:lambda:...:function:MyFunction", } API Call: lambda.Invoke({ FunctionName: "MyFunction", Qualifier: "1234", }) ``` This `Invoke` call *used* to succeed, but under the new authorization strategy it will fail. The required statement to make the call succeed would be (note the qualified ARN): ``` { Effect: "Allow", Action: "lambda:InvokeFunction", Resource: "arn:aws:lambda:...:function:MyFunction:1234", } ``` This PR aims to warn users who could be using an affected setup. Users will receive the a warning message under the following circumstances: - they grant `lambda:InvokeFunction` to an unqualified function arn - they call `lambda.currentVersion` somewhere in their code This is part of #19273. Related is #19318. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent ff85fd1 commit fd1fff9

File tree

3 files changed

+198
-2
lines changed

3 files changed

+198
-2
lines changed

packages/@aws-cdk/aws-lambda/lib/function-base.ts

+45-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
22
import * as ec2 from '@aws-cdk/aws-ec2';
33
import * as iam from '@aws-cdk/aws-iam';
4-
import { ArnFormat, ConstructNode, IResource, Resource, Token } from '@aws-cdk/core';
4+
import { Annotations, ArnFormat, ConstructNode, IResource, Resource, Token } from '@aws-cdk/core';
55
import { AliasOptions } from './alias';
66
import { Architecture } from './architecture';
77
import { EventInvokeConfig, EventInvokeConfigOptions } from './event-invoke-config';
@@ -12,6 +12,10 @@ import { CfnPermission } from './lambda.generated';
1212
import { Permission } from './permission';
1313
import { addAlias, flatMap } from './util';
1414

15+
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
16+
// eslint-disable-next-line no-duplicate-imports, import/order
17+
import { Construct } from '@aws-cdk/core';
18+
1519
export interface IFunction extends IResource, ec2.IConnectable, iam.IGrantable {
1620

1721
/**
@@ -274,12 +278,45 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC
274278

275279
private _latestVersion?: LatestVersion;
276280

281+
/**
282+
* Flag to delay adding a warning message until current version is invoked.
283+
* @internal
284+
*/
285+
protected _warnIfCurrentVersionCalled: boolean = false;
286+
277287
/**
278288
* Mapping of invocation principals to grants. Used to de-dupe `grantInvoke()` calls.
279289
* @internal
280290
*/
281291
protected _invocationGrants: Record<string, iam.Grant> = {};
282292

293+
/**
294+
* A warning will be added to functions under the following conditions:
295+
* - permissions that include `lambda:InvokeFunction` are added to the unqualified function.
296+
* - function.currentVersion is invoked before or after the permission is created.
297+
*
298+
* This applies only to permissions on Lambda functions, not versions or aliases.
299+
* This function is overridden as a noOp for QualifiedFunctionBase.
300+
*/
301+
public considerWarningOnInvokeFunctionPermissions(scope: Construct, action: string) {
302+
const affectedPermissions = ['lambda:InvokeFunction', 'lambda:*', 'lambda:Invoke*'];
303+
if (affectedPermissions.includes(action)) {
304+
if (scope.node.tryFindChild('CurrentVersion')) {
305+
this.warnInvokeFunctionPermissions(scope);
306+
} else {
307+
this._warnIfCurrentVersionCalled = true;
308+
}
309+
}
310+
}
311+
312+
protected warnInvokeFunctionPermissions(scope: Construct): void {
313+
Annotations.of(scope).addWarning([
314+
"AWS Lambda has changed their authorization strategy, which may cause client invocations using the 'Qualifier' parameter of the lambda function to fail with Access Denied errors.",
315+
"If you are using a lambda Version or Alias, make sure to call 'grantInvoke' or 'addPermission' on the Version or Alias, not the underlying Function",
316+
'See: https://github.com/aws/aws-cdk/issues/19273',
317+
].join('\n'));
318+
}
319+
283320
/**
284321
* Adds a permission to the Lambda resource policy.
285322
* @param id The id for the permission construct
@@ -296,6 +333,8 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC
296333
const action = permission.action ?? 'lambda:InvokeFunction';
297334
const scope = permission.scope ?? this;
298335

336+
this.considerWarningOnInvokeFunctionPermissions(scope, action);
337+
299338
new CfnPermission(scope, id, {
300339
action,
301340
principal,
@@ -554,6 +593,11 @@ export abstract class QualifiedFunctionBase extends FunctionBase {
554593
...options,
555594
});
556595
}
596+
597+
public considerWarningOnInvokeFunctionPermissions(_scope: Construct, _action: string): void {
598+
// noOp
599+
return;
600+
}
557601
}
558602

559603
/**

packages/@aws-cdk/aws-lambda/lib/function.ts

+4
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,10 @@ export class Function extends FunctionBase {
399399
return this._currentVersion;
400400
}
401401

402+
if (this._warnIfCurrentVersionCalled) {
403+
this.warnInvokeFunctionPermissions(this);
404+
};
405+
402406
this._currentVersion = new Version(this, 'CurrentVersion', {
403407
lambda: this,
404408
...this.currentVersionOptions,

packages/@aws-cdk/aws-lambda/test/function.test.ts

+149-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as path from 'path';
2-
import { Match, Template } from '@aws-cdk/assertions';
2+
import { Annotations, Match, Template } from '@aws-cdk/assertions';
33
import { ProfilingGroup } from '@aws-cdk/aws-codeguruprofiler';
44
import * as ec2 from '@aws-cdk/aws-ec2';
55
import * as efs from '@aws-cdk/aws-efs';
@@ -435,6 +435,154 @@ describe('function', () => {
435435
// THEN
436436
Template.fromStack(stack).resourceCountIs('AWS::Lambda::Permission', 0);
437437
});
438+
439+
describe('annotations on different IFunctions', () => {
440+
let stack: cdk.Stack;
441+
let fn: lambda.Function;
442+
let warningMessage: string;
443+
beforeEach(() => {
444+
warningMessage = 'AWS Lambda has changed their authorization strategy';
445+
stack = new cdk.Stack();
446+
fn = new lambda.Function(stack, 'MyLambda', {
447+
code: lambda.Code.fromAsset(path.join(__dirname, 'my-lambda-handler')),
448+
handler: 'index.handler',
449+
runtime: lambda.Runtime.PYTHON_3_6,
450+
});
451+
});
452+
453+
describe('permissions on functions', () => {
454+
test('without lambda:InvokeFunction', () => {
455+
// WHEN
456+
fn.addPermission('MyPermission', {
457+
action: 'lambda.GetFunction',
458+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
459+
});
460+
461+
// Simulate a workflow where a user has created a currentVersion with the intent to invoke it later.
462+
fn.currentVersion;
463+
464+
// THEN
465+
Annotations.fromStack(stack).hasNoWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
466+
});
467+
468+
describe('with lambda:InvokeFunction', () => {
469+
test('without invoking currentVersion', () => {
470+
// WHEN
471+
fn.addPermission('MyPermission', {
472+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
473+
});
474+
475+
// THEN
476+
Annotations.fromStack(stack).hasNoWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
477+
});
478+
479+
test('with currentVersion invoked first', () => {
480+
// GIVEN
481+
// Simulate a workflow where a user has created a currentVersion with the intent to invoke it later.
482+
fn.currentVersion;
483+
484+
// WHEN
485+
fn.addPermission('MyPermission', {
486+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
487+
});
488+
489+
// THEN
490+
Annotations.fromStack(stack).hasWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
491+
});
492+
493+
test('with currentVersion invoked after permissions created', () => {
494+
// WHEN
495+
fn.addPermission('MyPermission', {
496+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
497+
});
498+
499+
// Simulate a workflow where a user has created a currentVersion after adding permissions to the function.
500+
fn.currentVersion;
501+
502+
// THEN
503+
Annotations.fromStack(stack).hasWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
504+
});
505+
506+
test('multiple currentVersion calls does not result in multiple warnings', () => {
507+
// WHEN
508+
fn.currentVersion;
509+
510+
fn.addPermission('MyPermission', {
511+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
512+
});
513+
514+
fn.currentVersion;
515+
516+
// THEN
517+
const warns = Annotations.fromStack(stack).findWarning('/Default/MyLambda', Match.stringLikeRegexp(warningMessage));
518+
expect(warns).toHaveLength(1);
519+
});
520+
});
521+
});
522+
523+
test('permission on versions', () => {
524+
// GIVEN
525+
const version = new lambda.Version(stack, 'MyVersion', {
526+
lambda: fn.currentVersion,
527+
});
528+
529+
// WHEN
530+
version.addPermission('MyPermission', {
531+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
532+
});
533+
534+
// THEN
535+
Annotations.fromStack(stack).hasNoWarning('/Default/MyVersion', Match.stringLikeRegexp(warningMessage));
536+
});
537+
538+
test('permission on latest version', () => {
539+
// WHEN
540+
fn.latestVersion.addPermission('MyPermission', {
541+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
542+
});
543+
544+
// THEN
545+
// cannot add permissions on latest version, so no warning necessary
546+
Annotations.fromStack(stack).hasNoWarning('/Default/MyLambda/$LATEST', Match.stringLikeRegexp(warningMessage));
547+
});
548+
549+
describe('permission on alias', () => {
550+
test('of current version', () => {
551+
// GIVEN
552+
const version = new lambda.Version(stack, 'MyVersion', {
553+
lambda: fn.currentVersion,
554+
});
555+
const alias = new lambda.Alias(stack, 'MyAlias', {
556+
aliasName: 'alias',
557+
version,
558+
});
559+
560+
// WHEN
561+
alias.addPermission('MyPermission', {
562+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
563+
});
564+
565+
// THEN
566+
Annotations.fromStack(stack).hasNoWarning('/Default/MyAlias', Match.stringLikeRegexp(warningMessage));
567+
});
568+
569+
test('of latest version', () => {
570+
// GIVEN
571+
const alias = new lambda.Alias(stack, 'MyAlias', {
572+
aliasName: 'alias',
573+
version: fn.latestVersion,
574+
});
575+
576+
// WHEN
577+
alias.addPermission('MyPermission', {
578+
principal: new iam.ServicePrincipal('lambda.amazonaws.com'),
579+
});
580+
581+
// THEN
582+
Annotations.fromStack(stack).hasNoWarning('/Default/MyAlias', Match.stringLikeRegexp(warningMessage));
583+
});
584+
});
585+
});
438586
});
439587

440588
test('Lambda code can be read from a local directory via an asset', () => {

0 commit comments

Comments
 (0)