Skip to content

Commit bd5caba

Browse files
authored
feat(iam): throw ValidationErrors instead of untyped errors (#34579)
### Issue Relates to #32569 ### Reason for this change untyped Errors are not recommended ### Description of changes `ValidationError`s everywhere ### Describe any new or updated permissions being added None ### Description of how you validated changes Existing tests. Exemptions granted as this is a refactor of existing code. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 561cdc0 commit bd5caba

File tree

13 files changed

+58
-58
lines changed

13 files changed

+58
-58
lines changed

packages/aws-cdk-lib/.eslintrc.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ baseConfig.rules['import/no-extraneous-dependencies'] = [
1717
baseConfig.rules['@cdklabs/no-throw-default-error'] = ['error'];
1818
// not yet supported
1919
const noThrowDefaultErrorNotYetSupported = [
20-
'aws-iam',
2120
'aws-secretsmanager',
2221
'core',
2322
'custom-resources',

packages/aws-cdk-lib/aws-iam/lib/grant.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ export class Grant implements IDependable {
251251
}
252252

253253
if (!addedToPrincipal.policyDependable) {
254-
throw new Error('Contract violation: when Principal returns statementAdded=true, it should return a dependable');
254+
throw new cdk.UnscopedValidationError('Contract violation: when Principal returns statementAdded=true, it should return a dependable');
255255
}
256256

257257
return new Grant({ principalStatement: statement, options, policyDependable: addedToPrincipal.policyDependable });
@@ -372,8 +372,7 @@ export class Grant implements IDependable {
372372
*/
373373
public assertSuccess(): void {
374374
if (!this.success) {
375-
// eslint-disable-next-line max-len
376-
throw new Error(`${describeGrant(this.options)} could not be added on either identity or resource policy.`);
375+
throw new cdk.UnscopedValidationError(`${describeGrant(this.options)} could not be added on either identity or resource policy.`);
377376
}
378377
}
379378

packages/aws-cdk-lib/aws-iam/lib/managed-policy.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { AddToPrincipalPolicyResult, IGrantable, IPrincipal, PrincipalPolicyFrag
77
import { undefinedIfEmpty } from './private/util';
88
import { IRole } from './role';
99
import { IUser } from './user';
10-
import { ArnFormat, Resource, Stack, Arn, Aws } from '../../core';
10+
import { ArnFormat, Resource, Stack, Arn, Aws, UnscopedValidationError } from '../../core';
1111
import { getCustomizeRolesConfig, PolicySynthesizer } from '../../core/lib/helpers-internal';
1212
import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
1313
import { propertyInjectable } from '../../core/lib/prop-injectable';
@@ -348,7 +348,7 @@ class ManagedPolicyGrantPrincipal implements IPrincipal {
348348
// This property is referenced to add policy statements as a resource-based policy.
349349
// We should fail because a managed policy cannot be used as a principal of a policy document.
350350
// cf. https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_principal.html#Principal_specifying
351-
throw new Error(`Cannot use a ManagedPolicy '${this._managedPolicy.node.path}' as the 'Principal' or 'NotPrincipal' in an IAM Policy`);
351+
throw new UnscopedValidationError(`Cannot use a ManagedPolicy '${this._managedPolicy.node.path}' as the 'Principal' or 'NotPrincipal' in an IAM Policy`);
352352
}
353353

354354
public addToPolicy(statement: PolicyStatement): boolean {

packages/aws-cdk-lib/aws-iam/lib/policy-document.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export class PolicyDocument implements cdk.IResolvable {
5555
const newPolicyDocument = new PolicyDocument();
5656
const statement = obj.Statement ?? [];
5757
if (statement && !Array.isArray(statement)) {
58-
throw new Error('Statement must be an array');
58+
throw new cdk.UnscopedValidationError('Statement must be an array');
5959
}
6060
newPolicyDocument.addStatements(...obj.Statement.map((s: any) => PolicyStatement.fromJson(s)));
6161
return newPolicyDocument;

packages/aws-cdk-lib/aws-iam/lib/policy-statement.ts

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,17 @@ import {
77
import { normalizeStatement } from './private/postprocess-policy-document';
88
import { LITERAL_STRING_KEY, mergePrincipal, sum } from './private/util';
99
import * as cdk from '../../core';
10+
import { UnscopedValidationError } from '../../core';
1011

1112
const ensureArrayOrUndefined = (field: any) => {
1213
if (field === undefined) {
1314
return undefined;
1415
}
1516
if (typeof (field) !== 'string' && !Array.isArray(field)) {
16-
throw new Error('Fields must be either a string or an array of strings');
17+
throw new UnscopedValidationError('Fields must be either a string or an array of strings');
1718
}
1819
if (Array.isArray(field) && !!field.find((f: any) => typeof (f) !== 'string')) {
19-
throw new Error('Fields must be either a string or an array of strings');
20+
throw new UnscopedValidationError('Fields must be either a string or an array of strings');
2021
}
2122
return Array.isArray(field) ? field : [field];
2223
};
@@ -67,7 +68,7 @@ export class PolicyStatement {
6768
// validate that the PolicyStatement has the correct shape
6869
const errors = ret.validateForAnyPolicy();
6970
if (errors.length > 0) {
70-
throw new Error('Incorrect Policy Statement: ' + errors.join('\n'));
71+
throw new UnscopedValidationError('Incorrect Policy Statement: ' + errors.join('\n'));
7172
}
7273

7374
return ret;
@@ -148,7 +149,7 @@ export class PolicyStatement {
148149
public addActions(...actions: string[]) {
149150
this.assertNotFrozen('addActions');
150151
if (actions.length > 0 && this._notAction.length > 0) {
151-
throw new Error('Cannot add \'Actions\' to policy statement if \'NotActions\' have been added');
152+
throw new UnscopedValidationError('Cannot add \'Actions\' to policy statement if \'NotActions\' have been added');
152153
}
153154
this.validatePolicyActions(actions);
154155
this._action.push(...actions);
@@ -165,7 +166,7 @@ export class PolicyStatement {
165166
public addNotActions(...notActions: string[]) {
166167
this.assertNotFrozen('addNotActions');
167168
if (notActions.length > 0 && this._action.length > 0) {
168-
throw new Error('Cannot add \'NotActions\' to policy statement if \'Actions\' have been added');
169+
throw new UnscopedValidationError('Cannot add \'NotActions\' to policy statement if \'Actions\' have been added');
169170
}
170171
this.validatePolicyActions(notActions);
171172
this._notAction.push(...notActions);
@@ -192,7 +193,7 @@ export class PolicyStatement {
192193
public addPrincipals(...principals: IPrincipal[]) {
193194
this.assertNotFrozen('addPrincipals');
194195
if (principals.length > 0 && this._notPrincipals.length > 0) {
195-
throw new Error('Cannot add \'Principals\' to policy statement if \'NotPrincipals\' have been added');
196+
throw new UnscopedValidationError('Cannot add \'Principals\' to policy statement if \'NotPrincipals\' have been added');
196197
}
197198
for (const principal of principals) {
198199
this.validatePolicyPrincipal(principal);
@@ -217,7 +218,7 @@ export class PolicyStatement {
217218
public addNotPrincipals(...notPrincipals: IPrincipal[]) {
218219
this.assertNotFrozen('addNotPrincipals');
219220
if (notPrincipals.length > 0 && this._principals.length > 0) {
220-
throw new Error('Cannot add \'NotPrincipals\' to policy statement if \'Principals\' have been added');
221+
throw new UnscopedValidationError('Cannot add \'NotPrincipals\' to policy statement if \'Principals\' have been added');
221222
}
222223
for (const notPrincipal of notPrincipals) {
223224
this.validatePolicyPrincipal(notPrincipal);
@@ -236,14 +237,14 @@ export class PolicyStatement {
236237
if (cdk.Token.isUnresolved(actions)) return;
237238
for (const action of actions || []) {
238239
if (!cdk.Token.isUnresolved(action) && !/^(\*|[a-zA-Z0-9-]+:[a-zA-Z0-9*]+)$/.test(action)) {
239-
throw new Error(`Action '${action}' is invalid. An action string consists of a service namespace, a colon, and the name of an action. Action names can include wildcards.`);
240+
throw new UnscopedValidationError(`Action '${action}' is invalid. An action string consists of a service namespace, a colon, and the name of an action. Action names can include wildcards.`);
240241
}
241242
}
242243
}
243244

244245
private validatePolicyPrincipal(principal: IPrincipal) {
245246
if (principal instanceof Group) {
246-
throw new Error('Cannot use an IAM Group as the \'Principal\' or \'NotPrincipal\' in an IAM Policy');
247+
throw new UnscopedValidationError('Cannot use an IAM Group as the \'Principal\' or \'NotPrincipal\' in an IAM Policy');
247248
}
248249
}
249250

@@ -323,7 +324,7 @@ export class PolicyStatement {
323324
public addResources(...arns: string[]) {
324325
this.assertNotFrozen('addResources');
325326
if (arns.length > 0 && this._notResource.length > 0) {
326-
throw new Error('Cannot add \'Resources\' to policy statement if \'NotResources\' have been added');
327+
throw new UnscopedValidationError('Cannot add \'Resources\' to policy statement if \'NotResources\' have been added');
327328
}
328329
this._resource.push(...arns);
329330
}
@@ -339,7 +340,7 @@ export class PolicyStatement {
339340
public addNotResources(...arns: string[]) {
340341
this.assertNotFrozen('addNotResources');
341342
if (arns.length > 0 && this._resource.length > 0) {
342-
throw new Error('Cannot add \'NotResources\' to policy statement if \'Resources\' have been added');
343+
throw new UnscopedValidationError('Cannot add \'NotResources\' to policy statement if \'Resources\' have been added');
343344
}
344345
this._notResource.push(...arns);
345346
}
@@ -517,7 +518,7 @@ export class PolicyStatement {
517518
this.principalConditionsJson = theseConditions;
518519
} else {
519520
if (this.principalConditionsJson !== theseConditions) {
520-
throw new Error(`All principals in a PolicyStatement must have the same Conditions (got '${this.principalConditionsJson}' and '${theseConditions}'). Use multiple statements instead.`);
521+
throw new UnscopedValidationError(`All principals in a PolicyStatement must have the same Conditions (got '${this.principalConditionsJson}' and '${theseConditions}'). Use multiple statements instead.`);
521522
}
522523
}
523524
this.addConditions(conditions);
@@ -676,7 +677,7 @@ export class PolicyStatement {
676677
*/
677678
private assertNotFrozen(method: string) {
678679
if (this._frozen) {
679-
throw new Error(`${method}: freeze() has been called on this PolicyStatement previously, so it can no longer be modified`);
680+
throw new UnscopedValidationError(`${method}: freeze() has been called on this PolicyStatement previously, so it can no longer be modified`);
680681
}
681682
}
682683
}
@@ -810,7 +811,7 @@ class JsonPrincipal extends PrincipalBase {
810811
json = { [LITERAL_STRING_KEY]: [json] };
811812
}
812813
if (typeof(json) !== 'object') {
813-
throw new Error(`JSON IAM principal should be an object, got ${JSON.stringify(json)}`);
814+
throw new UnscopedValidationError(`JSON IAM principal should be an object, got ${JSON.stringify(json)}`);
814815
}
815816

816817
this.policyFragment = {
@@ -853,7 +854,7 @@ export function deriveEstimateSizeOptions(scope: IConstruct): EstimateSizeOption
853854
const actionEstimate = 20;
854855
const arnEstimate = scope.node.tryGetContext(ARN_SIZE_ESTIMATE_CONTEXT_KEY) ?? DEFAULT_ARN_SIZE_ESTIMATE;
855856
if (typeof arnEstimate !== 'number') {
856-
throw new Error(`Context value ${ARN_SIZE_ESTIMATE_CONTEXT_KEY} should be a number, got ${JSON.stringify(arnEstimate)}`);
857+
throw new UnscopedValidationError(`Context value ${ARN_SIZE_ESTIMATE_CONTEXT_KEY} should be a number, got ${JSON.stringify(arnEstimate)}`);
857858
}
858859

859860
return { actionEstimate, arnEstimate };

packages/aws-cdk-lib/aws-iam/lib/policy.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { AddToPrincipalPolicyResult, IGrantable, IPrincipal, PrincipalPolicyFrag
77
import { generatePolicyName, undefinedIfEmpty } from './private/util';
88
import { IRole } from './role';
99
import { IUser } from './user';
10-
import { IResource, Lazy, Resource } from '../../core';
10+
import { IResource, Lazy, Resource, UnscopedValidationError } from '../../core';
1111
import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
1212
import { propertyInjectable } from '../../core/lib/prop-injectable';
1313

@@ -294,7 +294,7 @@ class PolicyGrantPrincipal implements IPrincipal {
294294
// This property is referenced to add policy statements as a resource-based policy.
295295
// We should fail because a policy cannot be used as a principal of a policy document.
296296
// cf. https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_principal.html#Principal_specifying
297-
throw new Error(`Cannot use a Policy '${this._policy.node.path}' as the 'Principal' or 'NotPrincipal' in an IAM Policy`);
297+
throw new UnscopedValidationError(`Cannot use a Policy '${this._policy.node.path}' as the 'Principal' or 'NotPrincipal' in an IAM Policy`);
298298
}
299299

300300
public addToPolicy(statement: PolicyStatement): boolean {

packages/aws-cdk-lib/aws-iam/lib/principals.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { defaultAddPrincipalToAssumeRole } from './private/assume-role-policy';
66
import { LITERAL_STRING_KEY, mergePrincipal } from './private/util';
77
import { ISamlProvider } from './saml-provider';
88
import * as cdk from '../../core';
9+
import { UnscopedValidationError } from '../../core';
910
import { RegionInfo } from '../../region-info';
1011

1112
/**
@@ -354,7 +355,7 @@ export class PrincipalWithConditions extends PrincipalAdapter {
354355
// if either the existing condition or the new one contain unresolved
355356
// tokens, fail the merge. this is as far as we go at this point.
356357
if (cdk.Token.isUnresolved(condition) || cdk.Token.isUnresolved(existing)) {
357-
throw new Error(`multiple "${operator}" conditions cannot be merged if one of them contains an unresolved token`);
358+
throw new UnscopedValidationError(`multiple "${operator}" conditions cannot be merged if one of them contains an unresolved token`);
358359
}
359360

360361
validateConditionObject(existing);
@@ -479,7 +480,7 @@ export class AccountPrincipal extends ArnPrincipal {
479480
constructor(public readonly accountId: any) {
480481
super(new StackDependentToken(stack => `arn:${stack.partition}:iam::${accountId}:root`).toString());
481482
if (!cdk.Token.isUnresolved(accountId) && typeof accountId !== 'string') {
482-
throw new Error('accountId should be of type string');
483+
throw new UnscopedValidationError('accountId should be of type string');
483484
}
484485
this.principalAccount = accountId;
485486
}
@@ -617,7 +618,7 @@ export class OrganizationPrincipal extends PrincipalBase {
617618
// We can only validate if it's a literal string (not a token)
618619
if (!cdk.Token.isUnresolved(organizationId)) {
619620
if (!organizationId.match(/^o-[a-z0-9]{10,32}$/)) {
620-
throw new Error(`Expected Organization ID must match regex pattern ^o-[a-z0-9]{10,32}$, received ${organizationId}`);
621+
throw new UnscopedValidationError(`Expected Organization ID must match regex pattern ^o-[a-z0-9]{10,32}$, received ${organizationId}`);
621622
}
622623
}
623624
}
@@ -874,7 +875,7 @@ export class CompositePrincipal extends PrincipalBase {
874875
constructor(...principals: IPrincipal[]) {
875876
super();
876877
if (principals.length === 0) {
877-
throw new Error('CompositePrincipals must be constructed with at least 1 Principal but none were passed.');
878+
throw new UnscopedValidationError('CompositePrincipals must be constructed with at least 1 Principal but none were passed.');
878879
}
879880
this.assumeRoleAction = principals[0].assumeRoleAction;
880881
this.addPrincipals(...principals);
@@ -903,7 +904,7 @@ export class CompositePrincipal extends PrincipalBase {
903904
for (const p of this._principals) {
904905
const fragment = p.policyFragment;
905906
if (fragment.conditions && Object.keys(fragment.conditions).length > 0) {
906-
throw new Error(
907+
throw new UnscopedValidationError(
907908
'Components of a CompositePrincipal must not have conditions. ' +
908909
`Tried to add the following fragment: ${JSON.stringify(fragment)}`);
909910
}
@@ -1022,6 +1023,6 @@ class ServicePrincipalToken implements cdk.IResolvable {
10221023
*/
10231024
export function validateConditionObject(x: unknown): asserts x is Record<string, unknown> {
10241025
if (!x || typeof x !== 'object' || Array.isArray(x)) {
1025-
throw new Error('A Condition should be represented as a map of operator to value');
1026+
throw new UnscopedValidationError('A Condition should be represented as a map of operator to value');
10261027
}
10271028
}

packages/aws-cdk-lib/aws-iam/lib/private/util.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { IConstruct } from 'constructs';
2-
import { captureStackTrace, DefaultTokenResolver, IPostProcessor, IResolvable, IResolveContext, Lazy, StringConcat, Token, Tokenization } from '../../../core';
2+
import { captureStackTrace, DefaultTokenResolver, IPostProcessor, IResolvable, IResolveContext, Lazy, StringConcat, Token, Tokenization, UnscopedValidationError, ValidationError } from '../../../core';
33
import { IPolicy } from '../policy';
44

55
export const MAX_POLICY_NAME_LEN = 128;
@@ -60,7 +60,7 @@ export class AttachedPolicies {
6060
}
6161

6262
if (this.policies.find(p => p.policyName === policy.policyName)) {
63-
throw new Error(`A policy named "${policy.policyName}" is already attached`);
63+
throw new ValidationError(`A policy named "${policy.policyName}" is already attached`, policy);
6464
}
6565

6666
this.policies.push(policy);
@@ -79,7 +79,7 @@ export function mergePrincipal(target: { [key: string]: string[] }, source: { [k
7979

8080
if ((LITERAL_STRING_KEY in source && targetKeys.some(k => k !== LITERAL_STRING_KEY)) ||
8181
(LITERAL_STRING_KEY in target && sourceKeys.some(k => k !== LITERAL_STRING_KEY))) {
82-
throw new Error(`Cannot merge principals ${JSON.stringify(target)} and ${JSON.stringify(source)}; if one uses a literal principal string the other one must be empty`);
82+
throw new UnscopedValidationError(`Cannot merge principals ${JSON.stringify(target)} and ${JSON.stringify(source)}; if one uses a literal principal string the other one must be empty`);
8383
}
8484

8585
for (const key of sourceKeys) {

0 commit comments

Comments
 (0)