Properly validate Principal, Action, NotAction

unbounce · Mar 19, 2019 · 93be453 · 93be453
1 parent ad5a397
commit 93be453
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 23 deletions.
diff --git a/src/validators/IAMPolicyDocumentValidator.ts b/src/validators/IAMPolicyDocumentValidator.ts
@@ -113,13 +113,25 @@ const validateCondition = (path: Path, condition: any, errors: Error[]): boolean
   }
 }
 
+const validatePrincipal = (path: Path, value: any, errors: Error[]): boolean => {
+  // https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_principal.html
+  const spec = {
+    AWS: [validate.optional, validate.or(validate.string, listOf(validate.string))],
+    Service: [validate.optional, validate.or(validate.string, listOf(validate.string))],
+    Federated: [validate.optional, validate.or(validate.string, listOf(validate.string))],
+    CanonicalUser: [validate.optional, validate.or(validate.string, listOf(validate.string))],
+  }
+  return validate.or(validate.string, (p, v, e) => validate.object(p, v, e, spec))(path, value, errors);
+}
+
 const validateStatement = (path: Path, value: any, errors: Error[]): boolean => {
   const spec = {
     Sid: [validate.optional, validate.string],
     Effect: [validate.required, stringOf(['Allow', 'Deny'])],
-    // Principal can not be specified for inline policies
-    Principal: [validate.optional, validate.or(validate.string, listOf(validate.string))],
-    Action: [validate.required, validate.or(validate.string, listOf(validate.string))],
+    // TODO Principal can not be specified for inline policies
+    Principal: [validate.optional, validatePrincipal],
+    Action: [validate.optional, validate.or(validate.string, listOf(validate.string))],
+    NotAction: [validate.optional, validate.or(validate.string, listOf(validate.string))],
     Resource: [validate.optional, validate.or(validate.string, listOf(validate.string))],
     Condition: [validate.optional, validateCondition],
   };

diff --git a/src/validators/__tests__/IAMPolicyDocumentValidator.test.ts b/src/validators/__tests__/IAMPolicyDocumentValidator.test.ts
@@ -33,7 +33,7 @@ describe('IAMPolicyDocumentValidator', () => {
     });
     expect(lint(template)).toEqual([]);
   });
-  test('valid, with lists', () => {
+  test('valid, with resource list', () => {
     const template = JSON.stringify({
       Resources: {
         Role: {
@@ -48,7 +48,7 @@ describe('IAMPolicyDocumentValidator', () => {
                   {
                     Action: '',
                     Effect: 'Deny',
-                    Principal: [''],
+                    Principal: '',
                     Resource: [''],
                   }
                 ]
@@ -156,7 +156,7 @@ describe('IAMPolicyDocumentValidator', () => {
                     Sid: [],
                     Action: [],
                     Effect: [],
-                    Principal: {},
+                    Principal: [],
                     Resource: {},
                     Condition: '',
                   }

diff --git a/src/validators/__tests__/__snapshots__/IAMPolicyDocumentValidator.test.ts.snap b/src/validators/__tests__/__snapshots__/IAMPolicyDocumentValidator.test.ts.snap
@@ -3,7 +3,7 @@
 exports[`IAMPolicyDocumentValidator invalid condition key 1`] = `
 Array [
   Object {
-    "message": "key must be one of aws:CurrentTime, aws:EpochTime, aws:MultiFactorAuthAge, aws:MultiFactorAuthPresent, aws:SecureTransport, aws:UserAgent, aws:PrincipalOrgID, aws:PrincipalTag, aws:PrincipalType, aws:Referer, aws:RequestedRegion, aws:RequestTag, aws:SourceAccount, aws:SourceArn, aws:SourceIp, aws:SourceVpc, aws:SourceVpce, aws:TagKeys, aws:TokenIssueTime, aws:userid, aws:username, got cats",
+    "message": "key must be one of aws:CurrentTime, aws:EpochTime, aws:MultiFactorAuthAge, aws:MultiFactorAuthPresent, aws:SecureTransport, aws:UserAgent, aws:PrincipalOrgID, aws:PrincipalTag, aws:PrincipalType, aws:Referer, aws:RequestedRegion, aws:RequestTag, aws:SourceAccount, aws:SourceArn, aws:SourceIp, aws:SourceVpc, aws:SourceVpce, aws:TagKeys, aws:TokenIssueTime, aws:userid, aws:username, s:, got cats",
     "path": Array [
       "Root",
       "Resources",
@@ -96,7 +96,7 @@ Array [
     ],
   },
   Object {
-    "message": "must be a String, got {} or must be a List, got {}",
+    "message": "must be a String, got [] or must be an Object, got []",
     "path": Array [
       "Root",
       "Resources",
@@ -160,20 +160,5 @@ Array [
       "Effect",
     ],
   },
-  Object {
-    "message": "is required",
-    "path": Array [
-      "Root",
-      "Resources",
-      "Role",
-      "Properties",
-      "Policies",
-      "0",
-      "PolicyDocument",
-      "Statement",
-      "0",
-      "Action",
-    ],
-  },
 ]
 `;