Skip to content

Commit

Permalink
fix(dynamodb): grant also gives access to indexes (#1564)
Browse files Browse the repository at this point in the history 
If a table has an index, the `grant()` methods will give permissions to
the index as well.

Updates the security diff engine to support `AWS::NoValue`.

Fixes #1540.
  • Loading branch information
@rix0rrr
rix0rrr authored Jan 17, 2019
1 parent 6d0cdc7 commit 33c2a6d
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 29 deletions.
9 changes: 8 additions & 1 deletion packages/@aws-cdk/aws-dynamodb/lib/table.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ export class Table extends Construct {
return;
}
principal.addToPolicy(new iam.PolicyStatement()
.addResource(this.tableArn)
.addResources(this.tableArn, new cdk.Token(() => this.hasIndex ? `${this.tableArn}/index/*` : new cdk.Aws().noValue).toString())
.addActions(...actions));
}

Expand Down Expand Up @@ -622,6 +622,13 @@ export class Table extends Construct {
})
});
}

/**
* Whether this table has indexes
*/
private get hasIndex(): boolean {
return this.globalSecondaryIndexes.length + this.localSecondaryIndexes.length > 0;
}
}

export enum AttributeType {
Expand Down
110 changes: 92 additions & 18 deletions packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@
"KeyType": "HASH"
}
],
"ProvisionedThroughput": {
"ReadCapacityUnits": 5,
"WriteCapacityUnits": 5
},
"AttributeDefinitions": [
{
"AttributeName": "hashKey",
"AttributeType": "S"
}
]
],
"ProvisionedThroughput": {
"ReadCapacityUnits": 5,
"WriteCapacityUnits": 5
}
}
},
"TableWithGlobalAndLocalSecondaryIndexBC540710": {
Expand All @@ -34,10 +34,6 @@
"KeyType": "RANGE"
}
],
"ProvisionedThroughput": {
"ReadCapacityUnits": 5,
"WriteCapacityUnits": 5
},
"AttributeDefinitions": [
{
"AttributeName": "hashKey",
Expand Down Expand Up @@ -251,6 +247,10 @@
"PointInTimeRecoverySpecification": {
"PointInTimeRecoveryEnabled": true
},
"ProvisionedThroughput": {
"ReadCapacityUnits": 5,
"WriteCapacityUnits": 5
},
"SSESpecification": {
"SSEEnabled": true
},
Expand Down Expand Up @@ -278,10 +278,6 @@
"KeyType": "HASH"
}
],
"ProvisionedThroughput": {
"ReadCapacityUnits": 5,
"WriteCapacityUnits": 5
},
"AttributeDefinitions": [
{
"AttributeName": "hashKey",
Expand Down Expand Up @@ -309,7 +305,11 @@
"WriteCapacityUnits": 5
}
}
]
],
"ProvisionedThroughput": {
"ReadCapacityUnits": 5,
"WriteCapacityUnits": 5
}
}
},
"TableWithLocalSecondaryIndex4DA3D08F": {
Expand All @@ -325,10 +325,6 @@
"KeyType": "RANGE"
}
],
"ProvisionedThroughput": {
"ReadCapacityUnits": 5,
"WriteCapacityUnits": 5
},
"AttributeDefinitions": [
{
"AttributeName": "hashKey",
Expand Down Expand Up @@ -360,6 +356,84 @@
"ProjectionType": "ALL"
}
}
],
"ProvisionedThroughput": {
"ReadCapacityUnits": 5,
"WriteCapacityUnits": 5
}
}
},
"User00B015A1": {
"Type": "AWS::IAM::User"
},
"UserDefaultPolicy1F97781E": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": [
"dynamodb:BatchGetItem",
"dynamodb:GetRecords",
"dynamodb:GetShardIterator",
"dynamodb:Query",
"dynamodb:GetItem",
"dynamodb:Scan"
],
"Effect": "Allow",
"Resource": [
{
"Fn::GetAtt": [
"TableCD117FA1",
"Arn"
]
},
{
"Ref": "AWS::NoValue"
}
]
},
{
"Action": [
"dynamodb:BatchGetItem",
"dynamodb:GetRecords",
"dynamodb:GetShardIterator",
"dynamodb:Query",
"dynamodb:GetItem",
"dynamodb:Scan"
],
"Effect": "Allow",
"Resource": [
{
"Fn::GetAtt": [
"TableWithGlobalAndLocalSecondaryIndexBC540710",
"Arn"
]
},
{
"Fn::Join": [
"",
[
{
"Fn::GetAtt": [
"TableWithGlobalAndLocalSecondaryIndexBC540710",
"Arn"
]
},
"/index/*"
]
]
}
]
}
],
"Version": "2012-10-17"
},
"PolicyName": "UserDefaultPolicy1F97781E",
"Users": [
{
"Ref": "User00B015A1"
}
]
}
}
Expand Down
5 changes: 5 additions & 0 deletions packages/@aws-cdk/aws-dynamodb/test/integ.dynamodb.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import iam = require('@aws-cdk/aws-iam');
import { App, Stack } from '@aws-cdk/cdk';
import { Attribute, AttributeType, ProjectionType, StreamViewType, Table } from '../lib';

Expand Down Expand Up @@ -118,4 +119,8 @@ tableWithLocalSecondaryIndex.addLocalSecondaryIndex({
sortKey: LSI_SORT_KEY
});

const user = new iam.User(stack, 'User');
table.grantReadData(user);
tableWithGlobalAndLocalSecondaryIndex.grantReadData(user);

app.run();
51 changes: 44 additions & 7 deletions packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1353,7 +1353,46 @@ export = {

'"grantFullAccess" allows the principal to perform any action on the table ("*")'(test: Test) {
testGrant(test, [ '*' ], (p, t) => t.grantFullAccess(p));
}
},

'if table has an index grant gives access to the index'(test: Test) {
// GIVEN
const stack = new Stack();

const table = new Table(stack, 'my-table');
table.addPartitionKey({ name: 'ID', type: AttributeType.String });
table.addGlobalSecondaryIndex({ indexName: 'MyIndex', partitionKey: { name: 'Age', type: AttributeType.Number }});
const user = new iam.User(stack, 'user');

// WHEN
table.grantReadData(user);

// THEN
expect(stack).to(haveResource('AWS::IAM::Policy', {
"PolicyDocument": {
"Statement": [
{
"Action": [
'dynamodb:BatchGetItem',
'dynamodb:GetRecords',
'dynamodb:GetShardIterator',
'dynamodb:Query',
'dynamodb:GetItem',
'dynamodb:Scan'
],
"Effect": "Allow",
"Resource": [
{ "Fn::GetAtt": ["mytable0324D45C", "Arn"] },
{ "Fn::Join": [ "", [ { "Fn::GetAtt": [ "mytable0324D45C", "Arn" ] }, "/index/*" ] ] }
]
}
],
"Version": "2012-10-17"
},
}));
test.done();
},

},
};

Expand Down Expand Up @@ -1387,12 +1426,10 @@ function testGrant(test: Test, expectedActions: string[], invocation: (user: iam
{
"Action": action,
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"mytable0324D45C",
"Arn"
]
}
"Resource": [
{ "Fn::GetAtt": [ "mytable0324D45C", "Arn" ] },
{ "Ref" : "AWS::NoValue" }
]
}
],
"Version": "2012-10-17"
Expand Down
17 changes: 14 additions & 3 deletions packages/@aws-cdk/cloudformation-diff/lib/render-intrinsics.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,18 @@
*
* Evaluates Fn::Join directly if the second argument is a literal list of strings.
*
* Removes list and object values evaluating to { Ref: 'AWS::NoValue' }.
*
* For other intrinsics we choose a string representation that CloudFormation
* cannot actually parse, but is comprehensible to humans.
*/
export function renderIntrinsics(x: any): any {
if (Array.isArray(x)) {
return x.map(renderIntrinsics);
return x.filter(el => !isNoValue(el)).map(renderIntrinsics);
}

if (isNoValue(x)) { return undefined; }

const intrinsic = getIntrinsic(x);
if (intrinsic) {
if (intrinsic.fn === 'Ref') { return '${' + intrinsic.args + '}'; }
Expand All @@ -32,7 +36,9 @@ export function renderIntrinsics(x: any): any {
if (typeof x === 'object' && x !== null) {
const ret: any = {};
for (const [key, value] of Object.entries(x)) {
ret[key] = renderIntrinsics(value);
if (!isNoValue(value)) {
ret[key] = renderIntrinsics(value);
}
}
return ret;
}
Expand All @@ -41,7 +47,7 @@ export function renderIntrinsics(x: any): any {

function unCloudFormationFnJoin(separator: string, args: any) {
if (Array.isArray(args)) {
return args.map(renderIntrinsics).join(separator);
return args.filter(el => !isNoValue(el)).map(renderIntrinsics).join(separator);
}
return stringifyIntrinsic('Fn::Join', [separator, args]);
}
Expand All @@ -57,6 +63,11 @@ function getIntrinsic(x: any): Intrinsic | undefined {
return keys.length === 1 && (keys[0] === 'Ref' || keys[0].startsWith('Fn::')) ? { fn: keys[0], args: x[keys[0]] } : undefined;
}

function isNoValue(x: any) {
const int = getIntrinsic(x);
return int && int.fn === 'Ref' && int.args === 'AWS::NoValue';
}

interface Intrinsic {
fn: string;
args: any;
Expand Down
35 changes: 35 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/test/test.render-intrinsics.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ export = {
test.done();
},

'removes AWS::NoValue from Fn::Join'(test: Test) {
test.equals(
renderIntrinsics({ 'Fn::Join': ['/', ['a', { Ref: 'AWS::NoValue' }, 'b', 'c']] }),
'a/b/c'
);

test.done();
},

'does not resolve Fn::Join if the second argument is not a list literal'(test: Test) {
test.equals(
renderIntrinsics({ 'Fn::Join': ['/', { Ref: 'ListParameter' }] }),
Expand Down Expand Up @@ -63,4 +72,30 @@ export = {
);
test.done();
},

'removes NoValue from object'(test: Test) {
test.deepEqual(
renderIntrinsics({
Deeper1: { Ref: 'SomeLogicalId' },
Deeper2: { Ref: 'AWS::NoValue' }
}),
{
Deeper1: '${SomeLogicalId}',
}
);
test.done();
},

'removes NoValue from array'(test: Test) {
test.deepEqual(
renderIntrinsics([
{ Ref: 'SomeLogicalId' },
{ Ref: 'AWS::NoValue' },
]),
[
'${SomeLogicalId}',
]
);
test.done();
},
};

0 comments on commit 33c2a6d

Please sign in to comment.