(dynamodb): grantReadData() should always grant permissions to secondary indexes

[ahammond]

Reproduction Steps

myTable.grantReadData(myLambda);

Then we query on a secondary index on myTable and get

AccessDeniedException

What did you expect to happen?

Expected the lambda to be able to query the table, including on secondary indices.

What actually happened?

AccessDeniedException

Environment

• CDK CLI Version : v1.89.0
• Framework Version: v1.89.0
• Node.js Version: v15.10.0
• OS : MacOS
• Language (Version): TypeScript (4.2.2)

Other

Resolved / workaround by adding

   myLambda.addToRolePolicy(
      new iam.PolicyStatement({
        actions: ['dynamodb:Query'],
        resources: [`${myTable.arn}/index/*`],
      }),
    );

The fix probably belongs somewhere near

aws-cdk/packages/@aws-cdk/aws-dynamodb/lib/table.ts

Line 906 in 0ea4b19

private combinedGrant(

---

This is 🐛 Bug Report

[skinny85]

Hey @ahammond ,

thanks for opening the issue. That's the behavior today. See this test:

aws-cdk/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts

Lines 2221 to 2277 in aa45ede

	test('creates the correct index grant if indexes have been provided when importing', () => {
	const stack = new Stack();
	const table = Table.fromTableAttributes(stack, 'ImportedTable', {
	tableName: 'MyTableName',
	globalIndexes: ['global'],
	localIndexes: ['local'],
	});
	const role = new iam.Role(stack, 'Role', {
	assumedBy: new iam.AnyPrincipal(),
	});
	table.grantReadData(role);
	expect(stack).toHaveResourceLike('AWS::IAM::Policy', {
	PolicyDocument: {
	Statement: [
	{
	Action: [
	'dynamodb:BatchGetItem',
	'dynamodb:GetRecords',
	'dynamodb:GetShardIterator',
	'dynamodb:Query',
	'dynamodb:GetItem',
	'dynamodb:Scan',
	'dynamodb:ConditionCheckItem',
	],
	Resource: [
	{
	'Fn::Join': ['', [
	'arn:',
	{ Ref: 'AWS::Partition' },
	':dynamodb:',
	{ Ref: 'AWS::Region' },
	':',
	{ Ref: 'AWS::AccountId' },
	':table/MyTableName',
	]],
	},
	{
	'Fn::Join': ['', [
	'arn:',
	{ Ref: 'AWS::Partition' },
	':dynamodb:',
	{ Ref: 'AWS::Region' },
	':',
	{ Ref: 'AWS::AccountId' },
	':table/MyTableName/index/*',
	]],
	},
	],
	},
	],
	},
	});
	});

.

Did you provide your indexes? How is myTable created?

Thanks,
Adam

[ahammond]

@skinny85 The table is created in the app's persistent stack:

    const emailAuthName = new Namer(['email', 'authorization']);
    const emailAuthorization = new Table(this, emailAuthName.pascal, {
      tableName: emailAuthName.pascal,
      partitionKey: { name: 'Token', type: AttributeType.STRING },
      encryption: TableEncryption.DEFAULT,
      audit: true,
    });
    emailAuthorization.addGlobalSecondaryIndex({
      indexName: 'EmailAndUniqueIdentifierIndex',
      partitionKey: { name: 'Email', type: AttributeType.STRING },
      sortKey: { name: 'UniqueIdentifier', type: AttributeType.STRING },
    });
    this.emailAuthorizationTable = Table.fromTableArn(
      this,
      emailAuthName.addSuffix(iface).pascal,
      emailAuthorization.tableArn,
    );

And referenced by the lambda in our API-GW stack:

    const resendROREmailName = new Namer(['resend', 'ror', 'email']);
    const resendROREmailResource = this.res.addResource(resendROREmailName.kebab, this.corsResourceOptions);
    const resendROREmailLambda = new hLambda.Function(this, resendROREmailName.pascal, {
      functionName: resendROREmailName.kebab,
      runtime: lambda.Runtime.GO_1_X,
      code: lambda.Code.fromAsset(`../artifacts/${resendROREmailName.snake}.zip`),
      handler: resendROREmailName.snake,
      environment: {
        EMAIL_AUTHORIZATION_NAME: props.emailAuthorizationTable.tableName,
        TEST_RESULT_NAME: props.testResultTable.tableName,
        ENVIRONMENT: props.environment,
        GO_ENV: props.namedEnv.name,
        PROJECT: props.serviceTag,
      },
      tracing: lambda.Tracing.ACTIVE,
      timeout: Duration.seconds(30),
      vpc: this.root.vpc,
      vpcSubnets: { subnetType: SubnetType.PRIVATE },
    });
    props.emailAuthorizationTable.grantReadWriteData(resendROREmailLambda);
    props.testResultTable.grantReadData(resendROREmailLambda);
    this.addMethodWithCors(resendROREmailName, 'POST', resendROREmailResource, resendROREmailLambda);
    this.watchResources(this.wf, resendROREmailLambda);

    resendROREmailLambda.addToRolePolicy(
      new iam.PolicyStatement({
        actions: ['dynamodb:Query'],
        resources: [`${props.emailAuthorizationTable.tableArn}/index/*`],
      }),
    );

I'll see if we can grant you read on the relevant repos.

[skinny85]

This is the problem:

    this.emailAuthorizationTable = Table.fromTableArn(
      this,
      emailAuthName.addSuffix(iface).pascal,
      emailAuthorization.tableArn,
    );

This "forgets" the indexes (you didn't specify the Table has any when importing it).

Why are you doing it that way? Why are you not just passing the emailAuthorization object directly to the other Stack?

[ahammond]

We had what looked like a circular dependency when just passing the object directly. This was a first and not terribly great iteration in what has become a pretty active desire to decouple our stacks. I'll take a look at adding the index on import.

[ahammond]

It seems to me that there's a reasonable argument that grantReadAccess should always grant the necessary permissions for indexes even if they don't exist? Is there any harm in doing that? And the benefit is that even if a table is imported via ARN it still correctly grants the expected permissions.

[skinny85]

I'm not completely opposed to this idea @ahammond . I'm turning this into a feature request.

PRs are welcome, as always 😉.

[skinny85]

Might require research confirming that granting those permissions on a Table without any indexes defined works without any problems.

[ahammond]

@skinny85 I did some ad-hoc testing and it doesn't seem to break anything by including the additional resources even when the indexes don't exist. I created #13740 but haven't got test passing yet.

[pszabop]

Note that if you import dynamodb using fromTableAttributes() the problem probably fixes itself both for lambda and appsync (appsync I just tested)

example:

    const recipeTable = dynamodb.Table.fromTableAttributes(this, 'importedRecipeTable', {
      tableArn: recipeTableArn.toString(),  // ARN recieved from another stack's output 
      localIndexes: [ 'byName' ],  // should be from another stack's output, hardcoded for quick verification
    }) as dynamodb.Table;

Note my reason for separating databases and APIs/non-storage deployments is lifetime: It's okay to blow destroyt stateless stuff like APIs in case of re-entrancy bugs in CDK or CloudFormation, but really bad to have to destroy databases and recreate them.

From what I can tell, sharing objects does not let you have independent lifetimes for databases and APIs, but export/import does.

[balan-jayavictor]

@pszabop thanks for sharing!
I was able to get CDK to grant permission to the index resource, by using dynamodb.Table.fromTableAttributes

[jagregory]

+1 for a way to grant access to indexes without knowing what they are up-front. I'd rather not have to keep track of all the index names across stacks, especially when using fromTableAttributes.

[bilalq]

Seems like the last PR got closed due to staleness. Other than commit message convention and failing test issues, did you all have any issues with the previous PR? I'd love to see this feature. The whole index permissioning issue has been the most frustrating part of migrating from SAM to CDK.

@ahammond Do you have capacity to continue on this, or are you cool with me picking it up?

[ahammond]

@bilalq I've been swamped for months. Please go ahead.

[tvb]

@bilalq Any luck you got time to commit a fix here soon?

[jbhurruth]

Is there still a fix en route for this?

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.