401 Unauthorized response doesn't contain WWW-Authenticate header

[philippgille]

I asked in the official gRPC Gitter channel, but didn't get a response so far: https://gitter.im/grpc/grpc?at=5ca0e2c1016a930a453cdc6d

Working on lightningnetwork/lnd#2332, where this question came up.

Maybe related to grpc-gateway issues #4, #221 and #309.

Steps you follow to reproduce the error:

When returning a code.Unauthenticated gRPC error, grpc-gateway maps this to a response with a 401 Unauthorized HTTP status code.

The response doesn't contain a WWW-Authenticate error though.

What did you expect to happen instead:

According to the HTTP specification https://tools.ietf.org/html/rfc7235#section-3.1, a 401 Unauthorized response "MUST" be accompanied by a WWW-Authenticate header.

I understand that no static value like WWW-Authenticate: Basic realm="simple" can be used, as it completely depends on the used authentication scheme, but maybe there can be a way to easily configure this when setting up the gateway?

I know that error responses can be customized (apparently in two different ways: via response forwarder and via custom error handler), but shouldn't there be a way for grpc-gateway to adhere to the HTTP specification by default?

Also, from my understanding you can't just add a header in those response customizations, but instead you have to create a complete response, which could lead to inconsistent responses or breaking changes, but I'm not sure about this.

What's your theory on why it isn't working:

I think setting the WWW-Authenticate header is just not implemented and I didn't find an easy way to set one (other than using one of the above mentioned response customizations).

I think the implementation is this one:

grpc-gateway/runtime/errors.go

Line 88 in 7aca14d

func DefaultHTTPError(ctx context.Context, mux *ServeMux, marshaler Marshaler, w http.ResponseWriter, _ *http.Request, err error) {

[johanbrandhorst]

Thanks for this bug report! This sounds like something we could enable by default as you say. What would be a reasonable value to put in the authenticate header if one isn't set?

Would you be interested in contributing this? I think it's be as simple as changing the behaviour of the DefaultHTTPError method as you say. We could detect this error code and set the header to a default value and also allow the user the configure the string value returned. What can I do to help you make this a reality?

[philippgille]

Thanks for the quick reply! Yes I'm interested in contributing this.

I'm not sure if we can set a reasonable default value though, I think we might need to rely on a user configured value.

I didn't work with grpc-gateway a lot yet, so I don't know where the user typically passes configuration in a programmatical way. Maybe as a runtime.ServeMuxOption when calling runtime.NewServeMux()?

[johanbrandhorst]

That would be the usual way, yes. Obviously, those are all optional, so the only other option is to only return this header when the user has supplied an option, which is still in violation of the HTTP spec 😢.

Another alternative altogether is to introduce an automatic translation of some magic gRPC metadata value to the WWW-Authenticate header and rely on users setting this magic metadata value (we could export a named constant that users could set whenever they return codes.Unauthenticated. I guess that still doesn't get us quite into compliance territory since we would depend on users to set that magic value).

What do you think?

[philippgille]

I didn't work with gRPC metadata yet. It sounds to be more flexible when you can set it in a different way for each returned error instead of having to configure it up front when creating the SeverMux. On the other hand this might make it more verbose as well, having to set that with each Unauthenticated error (or lead to one more level of indirection when having to call some factored out function that sets the metadata and returns the error).

One requirement to keep in mind is that within a single app multiple authentication schemes could be supported. For example it's probably common that a web service accepts Basic credentials, as well as a Bearer token for OAuth. Depending on some internal logic the web service could determine which of the schemes is required for the specific request and set the WWW-Authenticate header accordingly, on a per-request basis.

[johanbrandhorst]

I think setting the header automatically for when we return 401, but allowing the user to override the content via a magic gRPC metadata value seems best. That would make us compliant with the spec, which I think is a reasonable excuse for breaking backwards compatibility in this case.

Would you be interested in contributing this? I could help you get started.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jefferai]

So this got marked as stale, but 401 responses still violate the RFC by not returning this header, which makes this rather timeless until it's fixed. Can it be reopened? :-)

[momom-i]

Hello, I'm here from good first issue tag and I'd like to work on this if anyone haven't done yet.
@johanbrandhorst Would you tell me more about what you said before?

Another alternative altogether is to introduce an automatic translation of some magic gRPC metadata value to the WWW-Authenticate header and rely on users setting this magic metadata value (we could export a named constant that users could set whenever they return codes.Unauthenticated.

[johanbrandhorst]

Hi! I'm really pleased that you want to contribute to the project. I think the first step to making a contribution is to create a failing test, so that we can assert the behavior we expect. It should be pretty straightforward to add in these places:

1. Add a new rpc to the EchoService:

   grpc-gateway/examples/internal/proto/examplepb/echo_service.proto

   Line 51 in 1036e87

   service EchoService {

   . This could be something like EchoUnauthorized.
2. Regenerate the protobuf files with make generate. You may need to run make install first.
3. Implement the new EchoUnauthorized function on the test echo server: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/server/echo.go.
4. Add a new TestEchoUnauthorized to the integration tests. You can copy the existing FieldMask tests for inspiration: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/internal/integration/integration_test.go#L59. The new test should check that the WWW-Authenticate header is returned.

Once we have a failing test, we can start thinking about how to implement a fix. It will probably be somewhere in https://github.com/grpc-ecosystem/grpc-gateway/blob/master/runtime/handler.go#L134 and https://github.com/grpc-ecosystem/grpc-gateway/blob/master/runtime/handler.go#L19.

I hope that helps. Let me know if you have any more questions. You could also reach out in the #grpc-gateway channel on Gophers slack.

[momom-i]

Thank you very much for your kind instruction. It helped me a lot.
I'm not too confident but I intend to complete test so would you check that before moving on?