External Auth feedback

[howardjohn]

	// It is expected that implementations will buffer the request body up to
	// `forwardBody.maxSize` bytes. Bodies over that size must be rejected with a
	// 4xx series error (413 or 403 are common examples), and fail processing
	// of the filter.

	// MaxSize specifies how large in bytes the largest body that will be buffered
	// and sent to the authorization server. If the body size is larger than
	// `maxSize`, then the body sent to the authorization server must be
	// truncated to `maxSize` bytes.

These two are contradictory. We should clarify which is the expected behavior.

While we are at it: the gRPC protobuf has a body and raw_body. We don't specify which to set. Do we want to intentionally leave this unspecified?

[howardjohn]

cc @youngnick

[youngnick]

The former is a leftover, it should have been updated, we agreed on the latter (I seem to recall).

[youngnick]

I think we can leave the body vs raw_body thing unspecified for now. Interested in your feedback if you implement it though.

[howardjohn]

My problem with body vs raw_body is I was looking to implement and don't really have a clue what to set. Other APIs I have seen have a bool to set it (which isn't great). body is kind of the legacy field but I am wondering if many servers read it... its kind of a big issue if they see "There is no body here, no problem!" because they don't look at raw_body.

If all the servers read raw_body there is ~no reason to not use it AFAIK

[youngnick]

Seems like we do a "SHOULD use raw_body" or something, and see if that works for everyone?

[howardjohn]

Adding some more general feedback on implementing it

• Path should be required to start with / (and NOT end with /, maybe) I think

"AllowedRequestHeaders must send..."

• 3 of these fields are not headers (though Envoy treats them as such) so it seems a bit confusing?
• "Path" in AllowedRequestHeaders and the Path field are contradictory/duplicative?
• The wording is slightly vague in that it says "must be sent", but it doesn't say what header must be sent. I assume its from the client's request. But for example, Host could be set to the hostname of the ext authz server
• allowedHeaders json tag vs AllowedRequestHeaders go struct. These don't need to align, but they probably should. And, IMO, the allowedRequestHeaders should be the one since its more clear