grpc-ecosystem / grpc-gateway

gRPC to JSON proxy generator following the gRPC HTTP spec
https://grpc-ecosystem.github.io/grpc-gateway/
BSD 3-Clause "New" or "Revised" License
17.99k stars 2.22k forks source link

Unable to mark the body parameter as REQUIRED when part of the proto message is path param #4666

Open HoaiDang-work opened 2 weeks ago

HoaiDang-work commented 2 weeks ago

🐛 Bug Report

We have the below proto description with openAPI annotations. FederatedIdentity identity = 4 [(google.api.field_behavior) = REQUIRED]; is request body param which is required. However, part of the identity, the type field is also path param /v1/identity/{identity.type}/register-by-link/cancel

rpc cancelRegisterIdentityByInstructionLink(MessagingIdentityRequest) returns (CancelRegistrationByLinkResponse) {
    option (google.api.method_visibility).restriction = "EXTERNAL";
    option (google.api.http) = {
      post: "/v1/identity/{identity.type}/register-by-link/cancel"
      body: "*"
    };
}

message MessagingIdentityRequest {
  common.ServiceRequestIdentity requestIdentity = 1 [(google.api.field_visibility).restriction = "HIDDEN"];
  string userId = 2 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "The provisioned User ID."}, (google.api.field_behavior) = REQUIRED];
  string companyId = 3 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "The company ID where the corporate phone number belongs."}, (google.api.field_behavior) = REQUIRED];
  //Contains 2 properties: `id` (string, required) and `attributes` (array of objects). Property `id` is the corporate phone number. Property `attributes` contains `com.leapxpert.official.account.phone.number` for key `name`, `STRING` for key `type` and corporate phone number for key `values`.
  FederatedIdentity identity = 4 [(google.api.field_behavior) = REQUIRED];
  string corporateAccountId = 5 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "The corporate phone number ID."}];
}

message FederatedIdentity {
  IdentityProviderType type = 1;
  string id = 2 [(common.sensitive) = true, (common.secureLogFormatter) = PHONE_OR_EMAIL];
  repeated MultiValueAttribute attributes = 3;
  IdentityScope scope = 4;
  // database entity id
  string objectId = 5;
  string companyId = 6;
  FederatedIdentityVerificationState verificationState = 7;
}

In the generated openapi spec, the identity is not marked as required.

image

Expected behavior

The remaining parts of identity should be marked as required.

Actual Behavior

Only the part including in the path param is marked as required, the remaining parts as request body params are not marked as required.

Your Environment

MacOS M1

johanbrandhorst commented 2 weeks ago

Thanks for your detailed bug report. This sounds like a reasonable bug, I expect the logic for parsing the path parameters and marking them required doesn't take this annotation into account. Would you be willing to help contribute a fix? The logic for this is in here: https://github.com/grpc-ecosystem/grpc-gateway/blob/main/protoc-gen-openapiv2/internal/genopenapi/template.go.

HoaiDang-work commented 2 weeks ago

I'm not familiar with Golang to contribute quality code to fix the problem 😓