From 29a3526249758619667279346aee6f947eb98a2e Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Thu, 28 May 2020 02:29:00 +0000 Subject: [PATCH 1/6] Check for sensitive trait in Shape members --- .../codegen/StructuredMemberWriter.java | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java index 7d81260cf8a..401cf7807c8 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java @@ -139,7 +139,7 @@ private void writeCollectionFilterSensitiveLog( MemberShape mapMember = ((MapShape) collectionMemberTarget).getValue(); writeMapFilterSensitiveLog(writer, mapMember, itemParam); } else { - // This path should not reach because of recursive isIterationRequired. + // This path should not reach because of recursive isMemberOverwriteRequired. throw new CodegenException(String.format( "CollectionFilterSensitiveLog attempted for %s while it was not required", collectionMemberTarget.getType() @@ -180,7 +180,7 @@ private void writeMapFilterSensitiveLog(TypeScriptWriter writer, MemberShape map MemberShape nestedMapMember = ((MapShape) mapMemberTarget).getValue(); writeMapFilterSensitiveLog(writer, nestedMapMember, valueParam); } else { - // This path should not reach because of recursive isIterationRequired. + // This path should not reach because of recursive isMemberOverwriteRequired. throw new CodegenException(String.format( "MapFilterSensitiveLog attempted for %s while it was not required", mapMemberTarget.getType() @@ -196,37 +196,29 @@ private void writeMapFilterSensitiveLog(TypeScriptWriter writer, MemberShape map } /** - * Identifies if iteration is required on member. + * Identifies if member needs to be overwritten in filterSensitiveLog. * - * @param member a {@link MemberShape} to check for iteration required. - * @return Returns true if the iteration is required on member. + * @param member a {@link MemberShape} to check if overwrite is required. + * @return Returns true if the overwrite is required on member. */ - private boolean isIterationRequired(MemberShape member) { + private boolean isMemberOverwriteRequired(MemberShape member) { + if (member.getMemberTrait(model, SensitiveTrait.class).isPresent()) { + return true; + } + Shape memberTarget = model.expectShape(member.getTarget()); if (memberTarget instanceof StructureShape) { return true; } else if (memberTarget instanceof CollectionShape) { MemberShape collectionMember = ((CollectionShape) memberTarget).getMember(); - return isIterationRequired(collectionMember); + return isMemberOverwriteRequired(collectionMember); } else if (memberTarget instanceof MapShape) { MemberShape mapMember = ((MapShape) memberTarget).getValue(); - return isIterationRequired(mapMember); + return isMemberOverwriteRequired(mapMember); } return false; } - /** - * Identifies if member needs to be overwritten in filterSensitiveLog. - * - * @param member a {@link MemberShape} to check if overwrite is required. - * @return Returns true if the overwrite is required on member. - */ - private boolean isMemberOverwriteRequired(MemberShape member) { - return ( - member.getMemberTrait(model, SensitiveTrait.class).isPresent() || isIterationRequired(member) - ); - } - /** * Returns the member name to be used in generation. * From ee025ce32eb8496eb0d86e3dfbc61ab4acb9b115 Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Thu, 28 May 2020 02:46:20 +0000 Subject: [PATCH 2/6] Tests for checking sensitive trait in Shape members --- .../codegen/StructureGeneratorTest.java | 22 +++++++++++++++++++ .../test-list-with-sensitive-member.smithy | 18 +++++++++++++++ .../test-map-with-sensitive-member.smithy | 20 +++++++++++++++++ 3 files changed, 60 insertions(+) create mode 100644 smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-list-with-sensitive-member.smithy create mode 100644 smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-map-with-sensitive-member.smithy diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java index d15c1179d0d..57d4de70602 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java @@ -123,6 +123,17 @@ public void callsFilterInListWithSensitiveData() { + " })\n"); } + @Test + public void callsFilterForListWithSensitiveMember() { + testStructureCodegen("test-list-with-sensitive-member.smithy", + " export const filterSensitiveLog = (obj: GetFooInput): any => ({\n" + + " ...obj,\n" + + " ...(obj.foo && { foo:\n" + + " SENSITIVE_STRING\n" + + " }),\n" + + " })\n"); + } + @Test public void filtersSensitiveList() { testStructureCodegen("test-sensitive-list.smithy", @@ -172,6 +183,17 @@ public void callsFilterInMapWithSensitiveData() { + " })\n"); } + @Test + public void callsFilterForMapWithSensitiveMember() { + testStructureCodegen("test-map-with-sensitive-member.smithy", + " export const filterSensitiveLog = (obj: GetFooInput): any => ({\n" + + " ...obj,\n" + + " ...(obj.foo && { foo:\n" + + " SENSITIVE_STRING\n" + + " }),\n" + + " })\n"); + } + @Test public void filtersSensitiveMap() { testStructureCodegen("test-sensitive-map.smithy", diff --git a/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-list-with-sensitive-member.smithy b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-list-with-sensitive-member.smithy new file mode 100644 index 00000000000..e3b513d29db --- /dev/null +++ b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-list-with-sensitive-member.smithy @@ -0,0 +1,18 @@ +namespace smithy.example + +@protocols([{name: "aws.rest-json-1.1"}]) +service Example { + version: "1.0.0", + operations: [GetFoo] +} + +operation GetFoo(GetFooInput) + +structure GetFooInput { + foo: PhoneNumbersList +} + +list PhoneNumbersList { + @sensitive + member: String +} \ No newline at end of file diff --git a/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-map-with-sensitive-member.smithy b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-map-with-sensitive-member.smithy new file mode 100644 index 00000000000..3b63f7be1d6 --- /dev/null +++ b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-map-with-sensitive-member.smithy @@ -0,0 +1,20 @@ +namespace smithy.example + +@protocols([{name: "aws.rest-json-1.1"}]) +service Example { + version: "1.0.0", + operations: [GetFoo] +} + +operation GetFoo(GetFooInput) + +structure GetFooInput { + foo: PhoneNumbersMap +} + +map PhoneNumbersMap { + key: String, + + @sensitive + value: String +} \ No newline at end of file From db849df77918e4b4d4597585a3ce38ff629ac26a Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Thu, 28 May 2020 03:44:56 +0000 Subject: [PATCH 3/6] Check for sensitive trait in Structure members --- .../smithy/typescript/codegen/StructuredMemberWriter.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java index 401cf7807c8..c1e10e522b8 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java @@ -208,7 +208,13 @@ private boolean isMemberOverwriteRequired(MemberShape member) { Shape memberTarget = model.expectShape(member.getTarget()); if (memberTarget instanceof StructureShape) { - return true; + Collection structureMemberList = ((StructureShape) memberTarget).getAllMembers().values(); + for (MemberShape structureMember: structureMemberList) { + if (isMemberOverwriteRequired(structureMember)) { + return true; + } + } + return false; } else if (memberTarget instanceof CollectionShape) { MemberShape collectionMember = ((CollectionShape) memberTarget).getMember(); return isMemberOverwriteRequired(collectionMember); From 4d3d833c24d1ba98045812a04410497f938ac43f Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Thu, 28 May 2020 04:00:37 +0000 Subject: [PATCH 4/6] Add Set of parents in isMemberOverwriteRequired to avoid unending recursion --- .../codegen/StructuredMemberWriter.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java index c1e10e522b8..3fe964825d5 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java @@ -73,7 +73,7 @@ void writeMembers(TypeScriptWriter writer, Shape shape) { void writeFilterSensitiveLog(TypeScriptWriter writer, String objectParam) { writer.write("...$L,", objectParam); for (MemberShape member : members) { - if (isMemberOverwriteRequired(member)) { + if (isMemberOverwriteRequired(member, new HashSet())) { Shape memberTarget = model.expectShape(member.getTarget()); String memberName = getSanitizedMemberName(member); String memberParam = String.format("%s.%s", objectParam, memberName); @@ -199,28 +199,35 @@ private void writeMapFilterSensitiveLog(TypeScriptWriter writer, MemberShape map * Identifies if member needs to be overwritten in filterSensitiveLog. * * @param member a {@link MemberShape} to check if overwrite is required. + * @param parents a set of membernames which are parents of existing member to avoid unending recursion. * @return Returns true if the overwrite is required on member. */ - private boolean isMemberOverwriteRequired(MemberShape member) { + private boolean isMemberOverwriteRequired(MemberShape member, Set parents) { if (member.getMemberTrait(model, SensitiveTrait.class).isPresent()) { return true; } Shape memberTarget = model.expectShape(member.getTarget()); + parents.add(symbolProvider.toMemberName(member)); if (memberTarget instanceof StructureShape) { Collection structureMemberList = ((StructureShape) memberTarget).getAllMembers().values(); for (MemberShape structureMember: structureMemberList) { - if (isMemberOverwriteRequired(structureMember)) { + if (!parents.contains(symbolProvider.toMemberName(structureMember)) + && isMemberOverwriteRequired(structureMember, parents)) { return true; } } return false; } else if (memberTarget instanceof CollectionShape) { MemberShape collectionMember = ((CollectionShape) memberTarget).getMember(); - return isMemberOverwriteRequired(collectionMember); + if (!parents.contains(symbolProvider.toMemberName(collectionMember))) { + return isMemberOverwriteRequired(collectionMember, parents); + } } else if (memberTarget instanceof MapShape) { MemberShape mapMember = ((MapShape) memberTarget).getValue(); - return isMemberOverwriteRequired(mapMember); + if (!parents.contains(symbolProvider.toMemberName(mapMember))) { + return isMemberOverwriteRequired(mapMember, parents); + } } return false; } From 82f96696b72dd18dae14b014aa23c317fde9c44f Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Wed, 27 May 2020 18:32:56 +0000 Subject: [PATCH 5/6] Add tests for members without sensitive trait --- .../codegen/StructureGeneratorTest.java | 40 +++++++++++++++++++ .../codegen/test-insensitive-list.smithy | 17 ++++++++ .../codegen/test-insensitive-map.smithy | 18 +++++++++ .../test-insensitive-simple-shape.smithy | 14 +++++++ .../codegen/test-insensitive-structure.smithy | 18 +++++++++ .../codegen/test-recursive-shapes.smithy | 28 +++++++++++++ 6 files changed, 135 insertions(+) create mode 100644 smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-list.smithy create mode 100644 smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-map.smithy create mode 100644 smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-simple-shape.smithy create mode 100644 smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-structure.smithy create mode 100644 smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-recursive-shapes.smithy diff --git a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java index 57d4de70602..f34d15b3c91 100644 --- a/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java +++ b/smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/StructureGeneratorTest.java @@ -54,6 +54,14 @@ public void filtersSensitiveSimpleShape() { + " })\n"); } + @Test + public void skipsFilterForInsensitiveSimpleShape() { + testStructureCodegen("test-insensitive-simple-shape.smithy", + " export const filterSensitiveLog = (obj: GetFooInput): any => ({\n" + + " ...obj,\n" + + " })\n"); + } + @Test public void callsFilterForStructureWithSensitiveData() { testStructureCodegen("test-structure-with-sensitive-data.smithy", @@ -87,6 +95,14 @@ public void filtersSensitiveStructure() { + " })\n"); } + @Test + public void skipsFilterForInsensitiveStructure() { + testStructureCodegen("test-insensitive-structure.smithy", + " export const filterSensitiveLog = (obj: GetFooInput): any => ({\n" + + " ...obj,\n" + + " })\n"); + } + @Test public void filtersSensitiveMemberPointingToStructure() { testStructureCodegen("test-sensitive-member-pointing-to-structure.smithy", @@ -145,6 +161,14 @@ public void filtersSensitiveList() { + " })\n"); } + @Test + public void skipsFilterForInsensitiveList() { + testStructureCodegen("test-insensitive-list.smithy", + " export const filterSensitiveLog = (obj: GetFooInput): any => ({\n" + + " ...obj,\n" + + " })\n"); + } + @Test public void filtersSensitiveMemberPointingToList() { testStructureCodegen("test-sensitive-member-pointing-to-list.smithy", @@ -216,6 +240,22 @@ public void filtersSensitiveMemberPointingToMap() { + " })\n"); } + @Test + public void skipsFilterForInsensitiveMap() { + testStructureCodegen("test-insensitive-map.smithy", + " export const filterSensitiveLog = (obj: GetFooInput): any => ({\n" + + " ...obj,\n" + + " })\n"); + } + + @Test + public void skipsFilterOnEncounteringRecursiveShapes() { + testStructureCodegen("test-recursive-shapes.smithy", + " export const filterSensitiveLog = (obj: GetFooInput): any => ({\n" + + " ...obj,\n" + + " })\n"); + } + private String testStructureCodegen(String file, String expectedType) { Model model = Model.assembler() .addImport(getClass().getResource(file)) diff --git a/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-list.smithy b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-list.smithy new file mode 100644 index 00000000000..030f0f26c3d --- /dev/null +++ b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-list.smithy @@ -0,0 +1,17 @@ +namespace smithy.example + +@protocols([{name: "aws.rest-json-1.1"}]) +service Example { + version: "1.0.0", + operations: [GetFoo] +} + +operation GetFoo(GetFooInput) + +structure GetFooInput { + foo: NamesList +} + +list NamesList { + member: String +} \ No newline at end of file diff --git a/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-map.smithy b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-map.smithy new file mode 100644 index 00000000000..101c9beec8e --- /dev/null +++ b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-map.smithy @@ -0,0 +1,18 @@ +namespace smithy.example + +@protocols([{name: "aws.rest-json-1.1"}]) +service Example { + version: "1.0.0", + operations: [GetFoo] +} + +operation GetFoo(GetFooInput) + +structure GetFooInput { + foo: NamesMap +} + +map NamesMap { + key: String, + value: String +} \ No newline at end of file diff --git a/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-simple-shape.smithy b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-simple-shape.smithy new file mode 100644 index 00000000000..3517a6bea52 --- /dev/null +++ b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-simple-shape.smithy @@ -0,0 +1,14 @@ +namespace smithy.example + +@protocols([{name: "aws.rest-json-1.1"}]) +service Example { + version: "1.0.0", + operations: [GetFoo] +} + +operation GetFoo(GetFooInput) + +structure GetFooInput { + firstname: String, + lastname: String +} \ No newline at end of file diff --git a/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-structure.smithy b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-structure.smithy new file mode 100644 index 00000000000..9d0743ff32a --- /dev/null +++ b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-insensitive-structure.smithy @@ -0,0 +1,18 @@ +namespace smithy.example + +@protocols([{name: "aws.rest-json-1.1"}]) +service Example { + version: "1.0.0", + operations: [GetFoo] +} + +operation GetFoo(GetFooInput) + +structure GetFooInput { + foo: User +} + +structure User { + firstname: String, + lastname: String +} \ No newline at end of file diff --git a/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-recursive-shapes.smithy b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-recursive-shapes.smithy new file mode 100644 index 00000000000..874d8a86829 --- /dev/null +++ b/smithy-typescript-codegen/src/test/resources/software/amazon/smithy/typescript/codegen/test-recursive-shapes.smithy @@ -0,0 +1,28 @@ +namespace smithy.example + +@protocols([{name: "aws.rest-json-1.1"}]) +service Example { + version: "1.0.0", + operations: [GetFoo] +} + +operation GetFoo(GetFooInput) + +structure GetFooInput { + foo: User +} + +structure User { + recursiveUser: User, + recursiveList: UsersList, + recursiveMap: UsersMap +} + +list UsersList { + member: User +} + +map UsersMap { + key: String, + value: User +} \ No newline at end of file From 5fc0856351371ec82a4a4c1e289168e8521a627d Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Thu, 28 May 2020 05:12:57 +0000 Subject: [PATCH 6/6] Remove redundant return false --- .../amazon/smithy/typescript/codegen/StructuredMemberWriter.java | 1 - 1 file changed, 1 deletion(-) diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java index 3fe964825d5..01148f750fc 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java @@ -217,7 +217,6 @@ && isMemberOverwriteRequired(structureMember, parents)) { return true; } } - return false; } else if (memberTarget instanceof CollectionShape) { MemberShape collectionMember = ((CollectionShape) memberTarget).getMember(); if (!parents.contains(symbolProvider.toMemberName(collectionMember))) {