Skip to content

Commit

Permalink
Merge branch 'main' into bobertzh/docs
Browse files Browse the repository at this point in the history
  • Loading branch information
HBobertz authored Oct 6, 2023
2 parents 7af64c5 + 7e4fdc7 commit 3104733
Show file tree
Hide file tree
Showing 40 changed files with 726 additions and 539 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
certifi==2023.7.22
chardet==3.0.4
idna==2.10
urllib3==1.26.7
urllib3==1.26.17
# Requests used by this lambda
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable-next-line import/no-unresolved */
import * as AWSLambda from 'aws-lambda';
import { TablePrivilege, UserTablePrivilegesHandlerProps } from '../handler-props';
import { executeStatement } from './redshift-data';
import { ClusterProps } from './types';
import { makePhysicalId } from './util';
import { TablePrivilege, UserTablePrivilegesHandlerProps } from '../handler-props';

export async function handler(props: UserTablePrivilegesHandlerProps & ClusterProps, event: AWSLambda.CloudFormationCustomResourceEvent) {
const username = props.username;
Expand Down Expand Up @@ -62,10 +62,26 @@ async function updatePrivileges(
}

const oldTablePrivileges = oldResourceProperties.tablePrivileges;
if (oldTablePrivileges !== tablePrivileges) {
await revokePrivileges(username, oldTablePrivileges, clusterProps);
await grantPrivileges(username, tablePrivileges, clusterProps);
return { replace: false };
const tablesToRevoke = oldTablePrivileges.filter(({ tableId, actions }) => (
tablePrivileges.find(({ tableId: otherTableId, actions: otherActions }) => (
tableId === otherTableId && actions.some(action => !otherActions.includes(action))
))
));
if (tablesToRevoke.length > 0) {
await revokePrivileges(username, tablesToRevoke, clusterProps);
}

const tablesToGrant = tablePrivileges.filter(({ tableId, tableName, actions }) => {
const tableAdded = !oldTablePrivileges.find(({ tableId: otherTableId, tableName: otherTableName }) => (
tableId === otherTableId && tableName === otherTableName
));
const actionsAdded = oldTablePrivileges.find(({ tableId: otherTableId, actions: otherActions }) => (
tableId === otherTableId && otherActions.some(action => !actions.includes(action))
));
return tableAdded || actionsAdded;
});
if (tablesToGrant.length > 0) {
await grantPrivileges(username, tablesToGrant, clusterProps);
}

return { replace: false };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface TableHandlerProps {
}

export interface TablePrivilege {
readonly tableId: string;
readonly tableName: string;
readonly actions: string[];
}
Expand Down
31 changes: 19 additions & 12 deletions packages/@aws-cdk/aws-redshift-alpha/lib/private/privileges.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import * as cdk from 'aws-cdk-lib/core';
import { Construct } from 'constructs';
import { DatabaseQuery } from './database-query';
import { HandlerName } from './database-query-provider/handler-name';
import { TablePrivilege as SerializedTablePrivilege, UserTablePrivilegesHandlerProps } from './handler-props';
import { DatabaseOptions } from '../database-options';
import { ITable, TableAction } from '../table';
import { IUser } from '../user';
import { DatabaseQuery } from './database-query';
import { HandlerName } from './database-query-provider/handler-name';
import { TablePrivilege as SerializedTablePrivilege, UserTablePrivilegesHandlerProps } from './handler-props';

/**
* The Redshift table and action that make up a privilege that can be granted to a Redshift user.
Expand Down Expand Up @@ -63,23 +63,30 @@ export class UserTablePrivileges extends Construct {
tablePrivileges: cdk.Lazy.any({
produce: () => {
const reducedPrivileges = this.privileges.reduce((privileges, { table, actions }) => {
const tableName = table.tableName;
if (!(tableName in privileges)) {
privileges[tableName] = [];
const tableId = table.node.id;
if (!(tableId in privileges)) {
privileges[tableId] = {
tableName: table.tableName,
actions: [],
};
}
actions = actions.concat(privileges[tableName]);
actions = actions.concat(privileges[tableId].actions);
if (actions.includes(TableAction.ALL)) {
actions = [TableAction.ALL];
}
if (actions.includes(TableAction.UPDATE) || actions.includes(TableAction.DELETE)) {
actions.push(TableAction.SELECT);
}
privileges[tableName] = Array.from(new Set(actions));
privileges[tableId] = {
tableName: table.tableName,
actions: Array.from(new Set(actions)),
};
return privileges;
}, {} as { [key: string]: TableAction[] });
const serializedPrivileges: SerializedTablePrivilege[] = Object.entries(reducedPrivileges).map(([tableName, actions]) => ({
tableName: tableName,
actions: actions.map(action => TableAction[action]),
}, {} as { [key: string]: { tableName: string, actions: TableAction[] } });
const serializedPrivileges: SerializedTablePrivilege[] = Object.entries(reducedPrivileges).map(([tableId, config]) => ({
tableId,
tableName: config.tableName,
actions: config.actions.map(action => TableAction[action]),
}));
return serializedPrivileges;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import type * as AWSLambda from 'aws-lambda';

const username = 'username';
const tableName = 'tableName';
const tablePrivileges = [{ tableName, actions: ['INSERT', 'SELECT'] }];
const tableId = 'tableId';
const actions = ['INSERT', 'SELECT'];
const tablePrivileges = [{ tableId, tableName, actions }];
const clusterName = 'clusterName';
const adminUserArn = 'adminUserArn';
const databaseName = 'databaseName';
Expand Down Expand Up @@ -147,22 +149,99 @@ describe('update', () => {
}));
});

test('does not replace when privileges change', async () => {
test('does not replace when table name is changed', async () => {
const newTableName = 'newTableName';
const newTablePrivileges = [{ tableName: newTableName, actions: ['DROP'] }];
const newTablePrivileges = [{ tableId, tableName: newTableName, actions }];
const newResourceProperties = {
...resourceProperties,
tablePrivileges: newTablePrivileges,
};

// Checking if the table resource has not been recreated
await expect(managePrivileges(newResourceProperties, event)).resolves.toMatchObject({
PhysicalResourceId: physicalResourceId,
});
// Upon a table name change, Redshift maintains the same table priviliges as before.
// The name of the table has changed, a new table has not been created.
// Therefore 'REVOKE' statements should not be used.
expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
Sql: `REVOKE INSERT, SELECT ON ${newTableName} FROM ${username}`,
}));
// Likewise, here the table name has changed, so the current priviliges will still be intact.
expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
Sql: expect.stringMatching(new RegExp(`.+ ON ${tableName} TO ${username}`)),
}));
});

test('does not replace when table actions are changed', async () => {
const newTablePrivileges = [{ tableId, tableName, actions: ['DROP'] }];
const newResourceProperties = {
...resourceProperties,
tablePrivileges: newTablePrivileges,
};

// Checking if the table resource has not been recreated
await expect(managePrivileges(newResourceProperties, event)).resolves.toMatchObject({
PhysicalResourceId: physicalResourceId,
});
// Old actions are REVOKED, as they are not included in the list
expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
Sql: `REVOKE INSERT, SELECT ON ${tableName} FROM ${username}`,
}));
// New actions are GRANTED, as they are included in the list
expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
Sql: `GRANT DROP ON ${newTableName} TO ${username}`,
Sql: `GRANT DROP ON ${tableName} TO ${username}`,
}));
});
});

test('does not replace when table id is changed', async () => {
const newTableId = 'newTableId';
const newTablePrivileges = [{ tableId: newTableId, tableName, actions }];
const newResourceProperties = {
...resourceProperties,
tablePrivileges: newTablePrivileges,
};

// Checking if the table resource has not been recreated, we are not changing on table id either.
// Due to the construct only needing to be changed on a new user, not a new table
await expect(managePrivileges(newResourceProperties, event)).resolves.toMatchObject({
PhysicalResourceId: physicalResourceId,
});
// Upon removal of the old table, the priviliges will also be revoked automatically,
// as the table no longer exists.
// Calling REVOKE statments on a non-existing table will throw errors by Redshift
expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
Sql: expect.stringMatching(new RegExp(`REVOKE .+ ON ${tableName} FROM ${username}`)),
}));
// Adds the permissions onto the newly created table
expect(mockExecuteStatement).toHaveBeenCalledWith(expect.objectContaining({
Sql: expect.stringMatching(new RegExp(`GRANT .+ ON ${tableName} TO ${username}`)),
}));
});

test('does not replace when table id is appended', async () => {
const newTablePrivileges = [{ tableId: 'newTableId', tableName, actions }];
const newResourceProperties = {
...resourceProperties,
tablePrivileges: newTablePrivileges,
};

const newEvent = {
...event,
OldResourceProperties: {
...event.OldResourceProperties,
tablePrivileges: [{ tableName, actions }],
},
};

// Checking if the table resource has not been recreated
await expect(managePrivileges(newResourceProperties, newEvent)).resolves.toMatchObject({
PhysicalResourceId: physicalResourceId,
});
// Upon initial deployment from non table id usage to table id usage,
// permissions would not need to be granted/revoked, as the table should already exist
expect(mockExecuteStatement).not.toHaveBeenCalledWith(expect.objectContaining({
Sql: expect.stringMatching(new RegExp(`.+ ON ${tableName} FROM ${username}`)),
}));
});
});
Loading

0 comments on commit 3104733

Please sign in to comment.