document in the FAQ how client and server might access authenticated identity of the remote

[jhump]

The Peer object only provides an address. For clients, it just echos back the host that was used in the HTTP request, without returning anything about the remote server's identity. It could at least return the resolved IP, to provide more information when multiple A records are available.

But it also provides no way to get the authenticated identity, when using TLS. This is easily available in the response object returned from the http.Client or the request object provided to the http.Handler.

The gRPC version of this type has a generic AuthInfo field with an interface type, and users can then try to type assert to a specific implementation. The idea is that different authn mechanisms might be used to authenticate parties (like JWTs or other custom auth cookies for client authn), so the representation needs to be flexible enough so an interceptor could provide the identity (instead of hard-coding the representation used in mutually-authenticated TLS). Speaking of which, there is not a way for an authenticating interceptor to override the peer since there is no exported setter (or constructor which allows setting it).

[jhump]

I take it back about an interceptor not being able to set the Peer. The interceptor uses interfaces to model requests, responses, and streams, so it could provide its own implementation that overrides the Peer() method.

[akshayjshah]

On the client side, users can already use net/http/httptrace. On the server side, users can employ one of the many authn/authz packages that wrap http.Handler - and if they like, they can attach the authenticated identity to the context. If we're going to add more API surface to Connect, we should provide something notably better than these two options. What could we do?

On the client side, we have access to a tls.ConnState on the HTTP response. We could also use httptrace to collect some info automatically. We could use one or both of those mechanisms to include some additional information in connect.AnyResponse. But since connect.AnyResponse is shared between the client and server, that makes the server more awkward - we'd end up with an API that just returns nil there. In general, I haven't seen much demand for this sort of thing on Slack or in other Github issues.

I do see more demand for server-side authentication. That said, I'm not sure what we get from a gRPC-style API. The gRPC authinfo.AuthInfo interface doesn't provide any useful capabilities, and the suggestion that implementations embed authinfo.CommonAuthInfo only provides the odd authinfo.Privacy enum. In short, this seems like a bunch of types that accomplish little beyond context.Value. Without a more substantive AuthInfo interface, I'm not sure how we can do any better.

I'm open to other ideas, but I'm not seeing a path forwards that's clearly better than the status quo. Unless we can come up with something compelling, I'd prefer to document the status quo and wait.

[jhump]

@akshayjshah, thanks for thorough reply! That all seems reasonable. I am perhaps too much in the mindset of how things are done in gRPC, forgetting that Connect just uses net/http under the hood.

Though I do wonder what the value of Peer is in its current form. It's really not what you want on the client side (just echoing back the domain used in the URL, instead of providing the actual remote server IP). I guess it's useful on the server for interceptors that want to do IP-based rate limiting? I assume this omission is due to a similar shortcoming in the net/http package: the http.Response type does not have a RemoteAddr field for communicating this value back to a client-side caller.

Since the use of TLS (and client-cert authn with TLS) is available without any other packages or custom middleware/interceptors, it seems awkward that there is no way to access TLS results without resorting to custom trace callbacks or custom middleware. But I concede that creating surface area to model custom authn protocols and results is fair to be out of bounds.

Unless we can come up with something compelling, I'd prefer to document the status quo and wait.

Speaking of documenting the status quo, I think this question (how to retrieve authenticated peer, from client or server) is likely a good candidate for the FAQ on connect.build.

[mattrobenolt]

Personally, I'm of the opinion all of these things can be done in a third party (or first party) module that is provided through interceptors. Similar to the popular grpc-middleware module.

I think pairing interceptors with context.Value's can implement all of this functionality without embedding it in core Connect imho. As a user of Connect, one of the appeals is how little surface area and how simple Connect is compare to gRPC. So I think trying to mirror all gRPC concepts brings us back to basically using gRPC.

[mattrobenolt]

Though I do wonder what the value of Peer is in its current form. It's really not what you want on the client side (just echoing back the domain used in the URL, instead of providing the actual remote server IP). I guess it's useful on the server for interceptors that want to do IP-based rate limiting? I assume this omission is due to a similar shortcoming in the net/http package: the http.Response type does not have a RemoteAddr field for communicating this value back to a client-side caller.

@jhump while you're right that net/http.Request doesn't have RemoteAddr, I've adopted a pattern of leveraging http.Server.ConnContext and binding it to the base context. This makes sense since a single connection might make multiple requests. So I've started binding my own ConnContext struct and a RequestContext struct in middleware. Then anything downstream has access to the raw net.Conn and the http.Request. This pattern opens the door pretty wide to every bit of information needed.

I'm happy to put together some code samples this weekend if you're curious.

[jhump]

@jhump while you're right that net/http.Request doesn't have RemoteAddr

Actually the http.Request struct does have a RemoteAddr field. So server-side handlers can make use of that to identify the ip:port of the client. I was pointing out that the other side, http.Response, has no such field. So it is not possible for the client to learn the actual IP of the server that handled the request. The best the client can do (other than just look at the domain it used for the request, as Connect currently does) is to examine the TLS results in the response's TLS field (which provides some identity in the certificates, assuming certificate verification was enabled, though not necessarily the server's IP address).

I take it back about an interceptor not being able to set the Peer. The interceptor uses interfaces to model requests, responses, and streams, so it could provide its own implementation that overrides the Peer() method.

I have to retract this statement. While the interceptor technically could provide its own implementation of AnyRequest to override fields on the underlying *Request that cannot be directly set, this will result in a runtime error because the actual handler logic expects to be able to type-assert the request to *Request. Maybe I can file a separate bug about that -- it should either be well documented that interceptors should not implement the AnyRequest interface or (better IMO) the handler should handle a failed type assertion by creating a *Request copy of the incoming value.

[saquibmian]

My 0.02c:

While we should do the actual authentication flow outside of Connect ideally, we need some way to surface the authenticated identity to Connect handlers. For example if I want to construct an authorizer for the user principal in order to authorize the RPC, or if I want to set a created_by value on some entity, I need the user's identity. Granted, this can be done via shoving the value into the Context, but this makes the code less explicit.

I also think that service identity and user identity are different forms of identity, and they look different based on the application's requirements, so I agree that Connect shouldn't necessarily try to model this.

Not wanting to get too much into implementation, but something open-ended like this (very rough sketch) comes to mind.

// AuthInfoProvider does not perform an authentication challenge, it assumes that a challenge 
// has already occurred higher level; it merely extracts the provided authentication information
// from the incoming request.
type AuthInfoProvider func(req http.Request, conn net.Conn, /* etc... */) (interface{}, error)
type Peer interface {
  AuthInfo() interface{}
}

With this, handlers can type guard req.Peer().AuthInfo().Details to pull out some domain-specific authentication state.
Developers would register these in the server via WithAuthInfoProvider(provider). Then in theory we can provide a couple of “standard” ones like mTLS or JWT that will get a good chunk people most of the way there. We probably want to handle multiples, though (e.g., mTLS and JWT).

[mattrobenolt]

@jhump you're right, I misspoke about http.Response, but my sentiment remains. Using ConnContext you can bind the actual net.Conn to context, allowing you to derive whatever you want from it, for example the tls.ConnectionState, etc.

I still think it'd be worthwhile to put together some community driver interceptors that provide these helpers. The peer info is a good case imo for an interceptor that's outside of the core here.

Granted, I have 0 skin in the game here other than a heavy user of connect-go, but I don't personally see why any of this is necessary in core. JWT, mTLS, all of this can be done through interceptors and Server.ConnContext and seems great for a separate repo providing this functionality.

[ImSingee]

I create a PR #422 for one simple way (just expose request so we can get request.TLS) to handle this scenario.

[akshayjshah]

Sorry for the confusion, @ImSingee - this issue hasn't reached enough consensus for PRs.

Though I do wonder what the value of Peer is in its current form. It's really not what you want on the client side (just echoing back the domain used in the URL, instead of providing the actual remote server IP).

I think you pretty much nailed it, @jhump - it makes the domain & port used in the URL accessible to client-side interceptors, which makes it feasible to include logs, traces, metrics, etc. I think this is fairly common, b/c the domain + port are useful while remaining relatively low-cardinality. The OpenTelemetry spec agrees, and encourages clients to prefer domain to IP address. This info is definitely less useful for mutual authentication, though.

I also think that service identity and user identity are different forms of identity, and they look different based on the application's requirements, so I agree that Connect shouldn't necessarily try to model this.

@saquibmian I'd really like to keep Connect as focused on RPC as possible, deferring generic HTTP concerns to the stdlib or other packages. A big portion of my dissatisfaction with grpc-go is that it's enormous. Beyond the simple fact that different applications will want to model authn/authz with different types, I can see a few problems with supporting this in connect-go itself:

• If we model auth information as a single any, authz/authn interceptors won't compose nicely - who sets the identity? We could go all the way to []any, but that's a pain for users.
• For mTLS in particular, we don't have a net.Conn when RPC clients make a request - that happens inside the HTTP client that the user supplies.

There are reasonable ways around this that don't require changes to this package. @mattrobenolt likes Server.ConnContext; I've played a bit with another approach; and I'm sure more will emerge with time. If we need a good solution for mTLS authentication in particular, let's put a day into seeing how good we can make a standalone package.

[akshayjshah]

As an update, I've plowed some more time into my standalone authentication package this weekend. It's now go.akshayshah.org/connectauth, and it looks a bit more reasonable to me.

In particular, the same AuthFunc can now be used to build either a Connect interceptor or HTTP middleware. Of course, the docs strongly recommend the latter - you don't want to decompress and unmarshal the request message, then realize that the caller isn't authenticated! It doesn't support mTLS authentication, but it does make any sort of header-based auth straightforward.

If this approach seems reasonable, we can move it into a proper bufbuild repo.