diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts index a3b42438f945c..42e415158ec6d 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -3,7 +3,7 @@ import { IConstruct } from 'constructs'; import { Group } from './group'; import { AccountPrincipal, AccountRootPrincipal, AnyPrincipal, ArnPrincipal, CanonicalUserPrincipal, - FederatedPrincipal, IPrincipal, PrincipalBase, PrincipalPolicyFragment, ServicePrincipal, ServicePrincipalOpts, + FederatedPrincipal, IPrincipal, PrincipalBase, PrincipalPolicyFragment, ServicePrincipal, ServicePrincipalOpts, validateConditionObject, } from './principals'; import { normalizeStatement } from './private/postprocess-policy-document'; import { LITERAL_STRING_KEY, mergePrincipal, sum } from './util'; @@ -380,6 +380,8 @@ export class PolicyStatement { */ public addCondition(key: string, value: Condition) { this.assertNotFrozen('addCondition'); + validateConditionObject(value); + const existingValue = this._condition[key]; this._condition[key] = existingValue ? { ...existingValue, ...value } : value; } @@ -670,19 +672,15 @@ export enum Effect { * Condition for when an IAM policy is in effect. Maps from the keys in a request's context to * a string value or array of string values. See the Conditions interface for more details. */ -export type Condition = any; +export type Condition = unknown; -// NOTE! We'd ideally like to type this as `Record`, because the -// API expects a map which can take either strings or lists of strings. -// -// However, if we were to change this right now, the Java bindings for CDK would -// emit a type of `Map`, but the most common types people would -// instantiate would be an `ImmutableMap` which would not be -// assignable to `Map`. The types don't have a built-in notion -// of co-contravariance, you have to indicate that on the type. So jsii would first -// need to emit the type as `Map`. +// NOTE! We would have liked to have typed this as `Record`, but in some places +// of the code we are assuming we can pass a `CfnJson` object into where a `Condition` is expected, +// and that wouldn't typecheck anymore. // -// Feature request in https://github.com/aws/jsii/issues/1517 +// Needs to be `unknown` instead of `any` so that the type of `Conditions` is +// `Record`; if it had been `Record`, TypeScript would have allowed +// passing an array into `conditions` arguments (where it needs to be a map). /** * Conditions for when an IAM Policy is in effect, specified in the following structure: @@ -877,4 +875,4 @@ class OrderedSet { public direct(): readonly A[] { return this.array; } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-iam/lib/principals.ts b/packages/@aws-cdk/aws-iam/lib/principals.ts index 27ece9e4d59f5..3a411287d6114 100644 --- a/packages/@aws-cdk/aws-iam/lib/principals.ts +++ b/packages/@aws-cdk/aws-iam/lib/principals.ts @@ -267,8 +267,15 @@ export class PrincipalWithConditions extends PrincipalAdapter { * Add a condition to the principal */ public addCondition(key: string, value: Condition) { + validateConditionObject(value); + const existingValue = this.additionalConditions[key]; - this.additionalConditions[key] = existingValue ? { ...existingValue, ...value } : value; + if (!existingValue) { + this.additionalConditions[key] = value; + } + validateConditionObject(existingValue); + + this.additionalConditions[key] = { ...existingValue, ...value }; } /** @@ -335,6 +342,9 @@ export class PrincipalWithConditions extends PrincipalAdapter { throw new Error(`multiple "${operator}" conditions cannot be merged if one of them contains an unresolved token`); } + validateConditionObject(existing); + validateConditionObject(condition); + mergedConditions[operator] = { ...existing, ...condition }; }); return mergedConditions; @@ -913,3 +923,17 @@ class ServicePrincipalToken implements cdk.IResolvable { return `<${this.service}>`; } } + +/** + * Validate that the given value is a valid Condition object + * + * The type of `Condition` should have been different, but it's too late for that. + * + * Also, the IAM library relies on being able to pass in a `CfnJson` instance for + * a `Condition`. + */ +export function validateConditionObject(x: unknown): asserts x is Record { + if (!x || typeof x !== 'object' || Array.isArray(x)) { + throw new Error('A Condition should be represented as a map of operator to value'); + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index 14376c3b32909..c26b6b6df38da 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -618,9 +618,9 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC if (!conditions) { return undefined; } - const sourceArn = conditions.ArnLike ? conditions.ArnLike['aws:SourceArn'] : undefined; - const sourceAccount = conditions.StringEquals ? conditions.StringEquals['aws:SourceAccount'] : undefined; - const principalOrgID = conditions.StringEquals ? conditions.StringEquals['aws:PrincipalOrgID'] : undefined; + const sourceArn = requireString(requireObject(conditions.ArnLike)?.['aws:SourceArn']); + const sourceAccount = requireString(requireObject(conditions.StringEquals)?.['aws:SourceAccount']); + const principalOrgID = requireString(requireObject(conditions.StringEquals)?.['aws:PrincipalOrgID']); // PrincipalOrgID cannot be combined with any other conditions if (principalOrgID && (sourceArn || sourceAccount)) { @@ -768,3 +768,11 @@ class LatestVersion extends FunctionBase implements IVersion { return addAlias(this, this, aliasName, options); } } + +function requireObject(x: unknown): Record | undefined { + return x && typeof x === 'object' && !Array.isArray(x) ? x as any : undefined; +} + +function requireString(x: unknown): string | undefined { + return x && typeof x === 'string' ? x : undefined; +} \ No newline at end of file