From afa1899dc91b6b063bd22960c55990a3f92492dc Mon Sep 17 00:00:00 2001 From: ravi-signal <99042880+ravi-signal@users.noreply.github.com> Date: Thu, 30 May 2024 16:21:28 -0500 Subject: [PATCH] Add a require.proto presence annotation --- .../grpc/ValidatingInterceptor.java | 2 + .../grpc/validators/BaseFieldValidator.java | 12 +- .../validators/PresentFieldValidator.java | 35 +++ .../main/proto/org/signal/chat/require.proto | 223 +++++++++--------- .../grpc/ValidatingInterceptorTest.java | 23 +- service/src/test/proto/validation_test.proto | 4 + 6 files changed, 191 insertions(+), 108 deletions(-) create mode 100644 service/src/main/java/org/whispersystems/textsecuregcm/grpc/validators/PresentFieldValidator.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptor.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptor.java index 524711d53..97f5c0363 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptor.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptor.java @@ -21,6 +21,7 @@ import org.whispersystems.textsecuregcm.grpc.validators.EnumSpecifiedFieldValida import org.whispersystems.textsecuregcm.grpc.validators.ExactlySizeFieldValidator; import org.whispersystems.textsecuregcm.grpc.validators.FieldValidator; import org.whispersystems.textsecuregcm.grpc.validators.NonEmptyFieldValidator; +import org.whispersystems.textsecuregcm.grpc.validators.PresentFieldValidator; import org.whispersystems.textsecuregcm.grpc.validators.RangeFieldValidator; import org.whispersystems.textsecuregcm.grpc.validators.SizeFieldValidator; @@ -28,6 +29,7 @@ public class ValidatingInterceptor implements ServerInterceptor { private final Map fieldValidators = Map.of( "org.signal.chat.require.nonEmpty", new NonEmptyFieldValidator(), + "org.signal.chat.require.present", new PresentFieldValidator(), "org.signal.chat.require.specified", new EnumSpecifiedFieldValidator(), "org.signal.chat.require.e164", new E164FieldValidator(), "org.signal.chat.require.exactlySize", new ExactlySizeFieldValidator(), diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/validators/BaseFieldValidator.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/validators/BaseFieldValidator.java index a27f3af26..c9707fd86 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/validators/BaseFieldValidator.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/validators/BaseFieldValidator.java @@ -12,6 +12,7 @@ import static org.whispersystems.textsecuregcm.grpc.validators.ValidatorUtils.in import com.google.protobuf.ByteString; import com.google.protobuf.Descriptors; import com.google.protobuf.GeneratedMessageV3; +import com.google.protobuf.Message; import io.grpc.Status; import io.grpc.StatusException; import java.util.Set; @@ -91,7 +92,10 @@ public abstract class BaseFieldValidator implements FieldValidator { validateBytesValue(extensionValueTyped, (ByteString) msg.getField(fd)); case ENUM -> validateEnumValue(extensionValueTyped, (Descriptors.EnumValueDescriptor) msg.getField(fd)); - case FLOAT, DOUBLE, BOOL, MESSAGE, GROUP -> { + case MESSAGE -> { + validateMessageValue(extensionValueTyped, (Message) msg.getField(fd)); + } + case FLOAT, DOUBLE, BOOL, GROUP -> { // at this moment, there are no validations specific to these types of fields } } @@ -140,6 +144,12 @@ public abstract class BaseFieldValidator implements FieldValidator { throw internalError("`validateEnumValue` method needs to be implemented"); } + protected void validateMessageValue( + final T extensionValue, + final Message message) throws StatusException { + throw internalError("`validateMessageValue` method needs to be implemented"); + } + protected static boolean requireFlagExtension(final Object extensionValue) throws StatusException { if (extensionValue instanceof Boolean flagIsOn && flagIsOn) { return true; diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/grpc/validators/PresentFieldValidator.java b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/validators/PresentFieldValidator.java new file mode 100644 index 000000000..a516b476b --- /dev/null +++ b/service/src/main/java/org/whispersystems/textsecuregcm/grpc/validators/PresentFieldValidator.java @@ -0,0 +1,35 @@ +/* + * Copyright 2024 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.grpc.validators; + +import static org.whispersystems.textsecuregcm.grpc.validators.ValidatorUtils.invalidArgument; + +import com.google.protobuf.Descriptors; +import com.google.protobuf.Message; +import io.grpc.StatusException; +import java.util.Set; + +public class PresentFieldValidator extends BaseFieldValidator { + + public PresentFieldValidator() { + super("present", + Set.of(Descriptors.FieldDescriptor.Type.MESSAGE), + MissingOptionalAction.FAIL, + true); + } + + @Override + protected Boolean resolveExtensionValue(final Object extensionValue) throws StatusException { + return requireFlagExtension(extensionValue); + } + + @Override + protected void validateMessageValue(final Boolean extensionValue, final Message msg) throws StatusException { + if (msg == null) { + throw invalidArgument("message expected to be present"); + } + } +} diff --git a/service/src/main/proto/org/signal/chat/require.proto b/service/src/main/proto/org/signal/chat/require.proto index 3786f9656..65018faab 100644 --- a/service/src/main/proto/org/signal/chat/require.proto +++ b/service/src/main/proto/org/signal/chat/require.proto @@ -13,123 +13,134 @@ import "google/protobuf/descriptor.proto"; extend google.protobuf.FieldOptions { /* - Requires a field to have content of non-zero size/length. - Applies to both `optional` and regular fields, i.e. if the field is not set or has a default value, - it's considered to be empty. - - ``` - import "org/signal/chat/require.proto"; - - message Data { - - string nonEmptyString = 1 [(require.nonEmpty) = true]; - - bytes nonEmptyBytes = 2 [(require.nonEmpty) = true]; - - optional string nonEmptyStringOptional = 3 [(require.nonEmpty) = true]; - - optional bytes nonEmptyBytesOptional = 4 [(require.nonEmpty) = true]; - - repeated string nonEmptyList = 5 [(require.nonEmpty) = true]; - } - ``` - - Applicable to fields of type `string`, `byte`, and `repeated` fields. + * Requires a field to have content of non-zero size/length. + * Applies to both `optional` and regular fields, i.e. if the field is not set or has a default value, + * it's considered to be empty. + * + * ``` + * import "org/signal/chat/require.proto"; + * + * message Data { + * string nonEmptyString = 1 [(require.nonEmpty) = true]; + * bytes nonEmptyBytes = 2 [(require.nonEmpty) = true]; + * optional string nonEmptyStringOptional = 3 [(require.nonEmpty) = true]; + * optional bytes nonEmptyBytesOptional = 4 [(require.nonEmpty) = true]; + * repeated string nonEmptyList = 5 [(require.nonEmpty) = true]; + * } + * ``` + * + * Applicable to fields of type `string`, `byte`, and `repeated` fields. */ optional bool nonEmpty = 70001; /* - Requires a enum field to have value with an index greater than zero. - Applies to both `optional` and regular fields, i.e. if the field is not set or has a default value, - its index will be <= 0. - - ``` - import "org/signal/chat/require.proto"; - - message Data { - Color color = 1 [(require.specified) = true]; - } - - enum Color { - COLOR_UNSPECIFIED = 0; - COLOR_RED = 1; - COLOR_GREEN = 2; - COLOR_BLUE = 3; - } - ``` + * Requires a enum field to have value with an index greater than zero. + * Applies to both `optional` and regular fields, i.e. if the field is not set or has a default value, + * its index will be <= 0. + * + * ``` + * import "org/signal/chat/require.proto"; + * + * message Data { + * Color color = 1 [(require.specified) = true]; + * } + * + * enum Color { + * COLOR_UNSPECIFIED = 0; + * COLOR_RED = 1; + * COLOR_GREEN = 2; + * COLOR_BLUE = 3; + * } + * ``` */ optional bool specified = 70002; /* - Requires a size/length of a field to be within certain boundaries. - Applies to both `optional` and regular fields, i.e. if the field is not set or has a default value, - its size considered to be zero. - - ``` - import "org/signal/chat/require.proto"; - - message Data { - - string name = 1 [(require.size) = {min: 3, max: 8}]; - - optional string address = 2 [(require.size) = {min: 3, max: 8}]; - } - ``` - - Applicable to fields of type `string`, `byte`, and `repeated` fields. + * Requires a size/length of a field to be within certain boundaries. + * Applies to both `optional` and regular fields, i.e. if the field is not set or has a default value, + * its size considered to be zero. + * + * ``` + * import "org/signal/chat/require.proto"; + * + * message Data { + * + * string name = 1 [(require.size) = {min: 3, max: 8}]; + * + * optional string address = 2 [(require.size) = {min: 3, max: 8}]; + * } + * ``` + * + * Applicable to fields of type `string`, `byte`, and `repeated` fields. */ optional SizeConstraint size = 70003; /* - Requires a size/length of a field to be one of the specified values. - Applies to both `optional` and regular fields, i.e. if the field is not set or has a default value, - its size considered to be zero. - - ``` - import "org/signal/chat/require.proto"; - - message Data { - - string zip = 1 [(require.exactlySize) = 5]; - - optional string exactlySizeVariants = 2 [(require.exactlySize) = 2, (require.exactlySize) = 4]; - } - ``` - - Applicable to fields of type `string`, `byte`, and `repeated` fields. + * Requires a size/length of a field to be one of the specified values. + * Applies to both `optional` and regular fields, i.e. if the field is not set or has a default value, + * its size considered to be zero. + * + * ``` + * import "org/signal/chat/require.proto"; + * + * message Data { + * + * string zip = 1 [(require.exactlySize) = 5]; + * + * optional string exactlySizeVariants = 2 [(require.exactlySize) = 2, (require.exactlySize) = 4]; + * } + * ``` + * + * Applicable to fields of type `string`, `byte`, and `repeated` fields. */ repeated uint32 exactlySize = 70004; /* - Requires a value of a string field to be a valid E164-normalized phone number. - If the field is `optional`, this check allows a value to be not set. - - ``` - import "org/signal/chat/require.proto"; - - message Data { - string number = 1 [(require.e164)]; - } - ``` + * Requires a value of a string field to be a valid E164-normalized phone number. + * If the field is `optional`, this check allows a value to be not set. + * + * ``` + * import "org/signal/chat/require.proto"; + * + * message Data { + * string number = 1 [(require.e164)]; + * } + * ``` */ optional bool e164 = 70005; /* - Requires an integer value to be within a certain range. The range boundaries are specified - with the values of type `int32`, which should be enough for all practical purposes. - - If the field is `optional`, this check allows a value to be not set. - - ``` - import "org/signal/chat/require.proto"; - - message Data { - int32 byte = 1 [(require.range) = {min = -128, max = 127}]; - uint32 unsignedByte = 2 [(require.range).max = 255]; - } - ``` + * Requires an integer value to be within a certain range. The range boundaries are specified + * with the values of type `int32`, which should be enough for all practical purposes. + * + * If the field is `optional`, this check allows a value to be not set. + * + * ``` + * import "org/signal/chat/require.proto"; + * + * message Data { + * int32 byte = 1 [(require.range) = {min: -128, max: 127}]; + * uint32 unsignedByte = 2 [(require.range).max = 255]; + * } + * ``` */ optional ValueRangeConstraint range = 70006; + + /* + * Require a value of a message field to be present. + * + * Applies to both `optional` and regular fields (both of which have explicit + * presence for the message type anyways) + * + * ``` + * import "org/signal/chat/require.proto"; + * message Data { + * message MyMessage {} + * MyMessage myMessage = 1 [(require.present) = true]; + * } + *```` + */ + optional bool present = 70007; } message SizeConstraint { @@ -144,17 +155,17 @@ message ValueRangeConstraint { extend google.protobuf.ServiceOptions { /* - Indicates that all methods in a given service require a certain kind of authentication. - - ``` - import "org/signal/chat/require.proto"; - - service AuthService { - option (require.auth) = AUTH_ONLY_AUTHENTICATED; - - rpc AuthenticatedMethod (google.protobuf.Empty) returns (google.protobuf.Empty) {} - } - ``` + * Indicates that all methods in a given service require a certain kind of authentication. + * + * ``` + * import "org/signal/chat/require.proto"; + * + * service AuthService { + * option (require.auth) = AUTH_ONLY_AUTHENTICATED; + * + * rpc AuthenticatedMethod (google.protobuf.Empty) returns (google.protobuf.Empty) {} + * } + * ``` */ optional Auth auth = 71001; } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptorTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptorTest.java index 3ba6d8664..257b91ef3 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptorTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptorTest.java @@ -47,7 +47,9 @@ public class ValidatingInterceptorTest { @RegisterExtension static final GrpcServerExtension GRPC_SERVER_EXTENSION = new GrpcServerExtension(); - private static final class ValidationTestGrpcServiceImpl extends ReactorValidationTestServiceGrpc.ValidationTestServiceImplBase { + private static final class ValidationTestGrpcServiceImpl extends + ReactorValidationTestServiceGrpc.ValidationTestServiceImplBase { + @Override public Mono validationsEndpoint(final ValidationsRequest request) { return Mono.just(ValidationsResponse.newBuilder().build()); @@ -55,6 +57,7 @@ public class ValidatingInterceptorTest { } private static final class AuthGrpcServiceImpl extends ReactorAuthServiceGrpc.AuthServiceImplBase { + @Override public Mono authenticatedMethod(final Empty request) { return Mono.just(Empty.getDefaultInstance()); @@ -62,6 +65,7 @@ public class ValidatingInterceptorTest { } private static final class AnonymousGrpcServiceImpl extends ReactorAnonymousServiceGrpc.AnonymousServiceImplBase { + @Override public Mono anonymousMethod(final Empty request) { return Mono.just(Empty.getDefaultInstance()); @@ -350,6 +354,21 @@ public class ValidatingInterceptorTest { )); } + @Test + public void testPresent() throws Exception { + assertStatusException(Status.INVALID_ARGUMENT, () -> stub.validationsEndpoint( + builderWithValidDefaults() + .clearPresentMessage() + .build() + )); + + assertStatusException(Status.INVALID_ARGUMENT, () -> stub.validationsEndpoint( + builderWithValidDefaults() + .clearOptionalPresentMessage() + .build() + )); + } + @Test public void testAllFieldsValidationSuccess() throws Exception { stub.validationsEndpoint(builderWithValidDefaults().build()); @@ -369,6 +388,8 @@ public class ValidatingInterceptorTest { .setRangeSizeString("abc") .setNonEmptyString("abc") .setNonEmptyStringOptional("abc") + .setPresentMessage(ValidationsRequest.RequirePresentMessage.getDefaultInstance()) + .setOptionalPresentMessage(ValidationsRequest.RequirePresentMessage.getDefaultInstance()) .setColor(Color.COLOR_GREEN) .setColorOptional(Color.COLOR_GREEN) .setNonEmptyBytes(ByteString.copyFrom(new byte[5])) diff --git a/service/src/test/proto/validation_test.proto b/service/src/test/proto/validation_test.proto index b1fc1208f..6b35d0804 100644 --- a/service/src/test/proto/validation_test.proto +++ b/service/src/test/proto/validation_test.proto @@ -62,6 +62,10 @@ message ValidationsRequest { uint32 ui32 = 22 [(require.range).max = 100]; int32 i32range = 23 [(require.range) = {min: 10, max: 20}]; optional int32 i32OptRange = 24 [(require.range) = {min: 10, max: 20}]; + + message RequirePresentMessage {} + RequirePresentMessage presentMessage = 25 [(require.present) = true]; + optional RequirePresentMessage optionalPresentMessage = 26 [(require.present) = true]; } message MessageWithInvalidRangeConstraint {