Missing proj ID on Get results in unauthorized

[blakestier]

From another ticket:

Fetching a rule with a missing rule ID gets a nice validation error

[42][default:/src:0]# curl -kH "api-token: $TOK" https://a2-dev.test/apis/iam/v2/projects/neato/rules/ | jq
{
  "error": "a rule id is required and must contain at least one non-whitespace character",
  "code": 3,
  "message": "a rule id is required and must contain at least one non-whitespace character",
  "details": []
}

But fetching a project with a missing ID gets this error that's a bit of a red herring:

[47][default:/src:0]# curl -kH "api-token: $TOK" https://a2-dev.test/apis/iam/v2/projects/ | jq
{
  "error": "error authorizing action \"iam:projects:get\" on resource \"iam:projects:\" filtered by projects [] for members [\"token:admin-token-1586543035\"]: rpc error: code = InvalidArgument desc = invalid ProjectsAuthorizedReq.Resource: value does not match regex pattern \"^[a-z][^:*]*(?::[^:*]+)*$\"",
  "code": 7,
  "message": "error authorizing action \"iam:projects:get\" on resource \"iam:projects:\" filtered by projects [] for members [\"token:admin-token-1586543035\"]: rpc error: code = InvalidArgument desc = invalid ProjectsAuthorizedReq.Resource: value does not match regex pattern \"^[a-z][^:*]*(?::[^:*]+)*$\"",
  "details": []
}

[bcmdarroch]

This is getting at a core design issue in the gateway middleware: any field that is needed to create the resource and action pair for the IsAuthorized call must be enforced as required at the proto validation level.

In the first case:

[42][default:/src:0]# curl -kH "api-token: $TOK" https://a2-dev.test/apis/iam/v2/projects/neato/rules/ | jq
{
  "error": "a rule id is required and must contain at least one non-whitespace character",
  "code": 3,
  "message": "a rule id is required and must contain at least one non-whitespace character",
  "details": []
}

1. The GetRule request comes in to the gateway
2. No validation on rule ID at the proto level
3. The authz middleware just needs the project ID to populate the resource, so IsAuthorized succeeds
4. Now that the request has gotten past proto validation, authentication, and authorization, the validation at the server level finally kicks in and throws a nice readable error

In the second case:

[47][default:/src:0]# curl -kH "api-token: $TOK" https://a2-dev.test/apis/iam/v2/projects/ | jq
{
  "error": "error authorizing action \"iam:projects:get\" on resource \"iam:projects:\" filtered by projects [] for members [\"token:admin-token-1586543035\"]: rpc error: code = InvalidArgument desc = invalid ProjectsAuthorizedReq.Resource: value does not match regex pattern \"^[a-z][^:*]*(?::[^:*]+)*$\"",
  "code": 7,
  "message": "error authorizing action \"iam:projects:get\" on resource \"iam:projects:\" filtered by projects [] for members [\"token:admin-token-1586543035\"]: rpc error: code = InvalidArgument desc = invalid ProjectsAuthorizedReq.Resource: value does not match regex pattern \"^[a-z][^:*]*(?::[^:*]+)*$\"",
  "details": []
}

1. The GetRule request comes in to the gateway
2. No validation on project ID at the proto level
3. The authz middleware needs the project ID to populate the resource. Since the ID is missing, the resource is malformed and IsAuthorized fails with a misleading PermissionDenied error
4. The request never makes it to the server, so we can never get that nice human readable validation error

Without any validation on a required field in the gateway proto, IsAuthorized does catch the error, but it provides a misleading message.

The problem is there is no easy required proto validator we can use. The closest approximate min_length = 1 results in a weird error message about "runes". We could use a regex validator but that seems excessive for something like a Get request.

One solution would be to use the min_length validator (or maybe some other combo of validators) and wrap the error in our own message before sending it back to the user.
Ideally this solution should be implemented across our APIs.

[srenatus]

Some alternatives to fix this, I think, are

1. Creating out own PGV-compatible message validation in the gateway. I.e. for CreatePolicyRequest, we'd define func (r *CreatePolicyRequest) Validate() error, and opporunistically call that from the gateway middleware.

   We'd control the error messages. The validation wouldn't be proto-driven.

2. Since only the fields required for the authorization check matter in the gateway (architectural concerns aside!), we could enhance the generated code that does the lookup of those fields. When it encounters "", it could return an error instead of constructing what ends up being an invalid resource identifier.

I think both of these are similarly low-effort.

[susanev]

this work has been deprioritized.