mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-24 16:41:30 +02:00
[management] Add a proxy-connect authorizer seam (#7136)
At proxy connect time, the declared cluster address is validated for shape and checked for availability (`IsClusterAddressAvailable`), and from then on the declaration is what routes the cluster's mappings to the connection. Deployments that embed management through the integrations seam may need a policy on that claim — deciding which credential is allowed to declare which address. This adds an optional `ProxyConnectAuthorizer` hook on `ProxyServiceServer`, following the pattern of the existing `Set*` seams (`SetServiceManager`, `SetAgentNetworkSynthesizer`, `SetAgentNetworkLimitsService`, `SetProxyController`): - A nil-able interface field plus `SetProxyConnectAuthorizer`, guarded by the existing mutex. - One call at the end of `validateProxyConnect`, so both `GetMappingUpdate` and `SyncMappings` are covered by a single site. - **Nothing installs it by default** — with the hook unset (always, in this repo), behavior is byte-for-byte unchanged, which the tests pin. Design details: - The authorizer runs **last** — after input validation and the availability check — and **outside** the account-scoped branch, so management-wide tokens and token-less connects are also presented to it rather than bypassing policy. - The authorizer receives the presented `*types.ProxyAccessToken` (nil when none), the proxy ID, and the declared address. Everything it needs is already in the request/context; no proto or schema change. - A plain error from the authorizer surfaces as `PermissionDenied`, keeping an authorization rejection distinguishable from the `AlreadyExists` used for address conflicts in proxy logs. A status error passes through unchanged so implementations can pick their own code.
This commit is contained in:
@@ -61,6 +61,17 @@ type ProxyTokenChecker interface {
|
||||
IsProxyAccessTokenValid(ctx context.Context, tokenID string) (bool, error)
|
||||
}
|
||||
|
||||
// ProxyConnectAuthorizer authorizes a proxy's claim to the cluster address it
|
||||
// declares at connect time. Implementations are supplied by integrations; none
|
||||
// is installed by default, so every well-formed claim is authorized — the
|
||||
// declared address is otherwise only checked for availability. token is nil
|
||||
// when the connection carries no proxy access token. A returned status error
|
||||
// is sent to the proxy unchanged; any other error is wrapped as
|
||||
// PermissionDenied.
|
||||
type ProxyConnectAuthorizer interface {
|
||||
AuthorizeProxyConnect(ctx context.Context, token *types.ProxyAccessToken, proxyID, address string) error
|
||||
}
|
||||
|
||||
// ProxyServiceServer implements the ProxyService gRPC server
|
||||
// AgentNetworkSynthesizer produces in-memory reverse-proxy services from
|
||||
// Agent Network provider/policy state for the proxy snapshot path; synthesised
|
||||
@@ -99,6 +110,9 @@ type ProxyServiceServer struct {
|
||||
// and the post-flight consumption write (RecordLLMUsage). Optional — when
|
||||
// nil both RPCs return Unimplemented.
|
||||
agentNetworkLimits AgentNetworkLimitsService
|
||||
// connectAuthorizer authorizes address claims at proxy connect time.
|
||||
// Optional — when nil every well-formed claim is authorized.
|
||||
connectAuthorizer ProxyConnectAuthorizer
|
||||
// ProxyController for service updates and cluster management
|
||||
proxyController proxy.Controller
|
||||
|
||||
@@ -262,6 +276,23 @@ func (s *ProxyServiceServer) agentNetworkSynthesizer() AgentNetworkSynthesizer {
|
||||
return s.agentNetworkSynth
|
||||
}
|
||||
|
||||
// SetProxyConnectAuthorizer wires the connect-time address-claim authorizer.
|
||||
// Optional — when nil (the default) every well-formed claim is authorized,
|
||||
// which is the behavior without the hook. The modules layer injects this
|
||||
// after the proxy server is constructed, like the other setters.
|
||||
func (s *ProxyServiceServer) SetProxyConnectAuthorizer(authorizer ProxyConnectAuthorizer) {
|
||||
s.mu.Lock()
|
||||
s.connectAuthorizer = authorizer
|
||||
s.mu.Unlock()
|
||||
}
|
||||
|
||||
// proxyConnectAuthorizer returns the connect authorizer under read lock.
|
||||
func (s *ProxyServiceServer) proxyConnectAuthorizer() ProxyConnectAuthorizer {
|
||||
s.mu.RLock()
|
||||
defer s.mu.RUnlock()
|
||||
return s.connectAuthorizer
|
||||
}
|
||||
|
||||
// CheckLLMPolicyLimits is the pre-flight policy gate the proxy calls before
|
||||
// forwarding an LLM request upstream. Delegates to the agent-network selector,
|
||||
// which scores applicable policies by remaining headroom and returns the
|
||||
@@ -446,8 +477,9 @@ func recvSyncInit(stream proto.ProxyService_SyncMappingsServer) (*proto.SyncMapp
|
||||
return init, nil
|
||||
}
|
||||
|
||||
// validateProxyConnect validates the proxy ID and address, and checks cluster
|
||||
// address availability for account-scoped tokens.
|
||||
// validateProxyConnect validates the proxy ID and address, checks cluster
|
||||
// address availability for account-scoped tokens, and finally consults the
|
||||
// connect authorizer (when installed) on the address claim.
|
||||
func (s *ProxyServiceServer) validateProxyConnect(proxyID, address string, ctx context.Context) (proxyConnectParams, error) {
|
||||
if proxyID == "" {
|
||||
return proxyConnectParams{}, status.Errorf(codes.InvalidArgument, "proxy_id is required")
|
||||
@@ -467,6 +499,19 @@ func (s *ProxyServiceServer) validateProxyConnect(proxyID, address string, ctx c
|
||||
}
|
||||
}
|
||||
|
||||
// The authorizer runs last, outside the account-scoped branch, so it also
|
||||
// sees management-wide and token-less connects. PermissionDenied keeps an
|
||||
// authorization rejection distinguishable from the AlreadyExists address
|
||||
// conflict above in proxy logs.
|
||||
if authorizer := s.proxyConnectAuthorizer(); authorizer != nil {
|
||||
if err := authorizer.AuthorizeProxyConnect(ctx, token, proxyID, address); err != nil {
|
||||
if _, ok := status.FromError(err); ok {
|
||||
return proxyConnectParams{}, err
|
||||
}
|
||||
return proxyConnectParams{}, status.Errorf(codes.PermissionDenied, "proxy connect not authorized: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
return proxyConnectParams{proxyID: proxyID, address: address}, nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,168 @@
|
||||
package grpc
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/golang/mock/gomock"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/grpc/codes"
|
||||
grpcstatus "google.golang.org/grpc/status"
|
||||
|
||||
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
|
||||
"github.com/netbirdio/netbird/management/server/types"
|
||||
)
|
||||
|
||||
// capturingAuthorizer records the arguments of the last AuthorizeProxyConnect
|
||||
// call and returns a fixed error.
|
||||
type capturingAuthorizer struct {
|
||||
called int
|
||||
token *types.ProxyAccessToken
|
||||
proxyID string
|
||||
address string
|
||||
err error
|
||||
}
|
||||
|
||||
func (a *capturingAuthorizer) AuthorizeProxyConnect(_ context.Context, token *types.ProxyAccessToken, proxyID, address string) error {
|
||||
a.called++
|
||||
a.token = token
|
||||
a.proxyID = proxyID
|
||||
a.address = address
|
||||
return a.err
|
||||
}
|
||||
|
||||
// authorizerServer builds a ProxyServiceServer whose proxy manager reports
|
||||
// every cluster address as available, so the authorizer is the only thing
|
||||
// standing between a claim and success.
|
||||
func authorizerServer(t *testing.T) *ProxyServiceServer {
|
||||
t.Helper()
|
||||
ctrl := gomock.NewController(t)
|
||||
mgr := proxy.NewMockManager(ctrl)
|
||||
mgr.EXPECT().IsClusterAddressAvailable(gomock.Any(), gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes()
|
||||
return &ProxyServiceServer{proxyManager: mgr}
|
||||
}
|
||||
|
||||
// TestValidateProxyConnect_NilAuthorizerUnchanged guards the no-behavior-change
|
||||
// claim: with no authorizer installed — the OSS default — a well-formed claim
|
||||
// succeeds and malformed input is rejected exactly as before the hook existed.
|
||||
func TestValidateProxyConnect_NilAuthorizerUnchanged(t *testing.T) {
|
||||
s := authorizerServer(t)
|
||||
|
||||
params, err := s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "proxy-1", params.proxyID)
|
||||
assert.Equal(t, "cluster.example.com", params.address)
|
||||
|
||||
_, err = s.validateProxyConnect("", "cluster.example.com", scopedCtx("acc-1"))
|
||||
require.Error(t, err)
|
||||
st, ok := grpcstatus.FromError(err)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, codes.InvalidArgument, st.Code(), "missing proxy_id must stay InvalidArgument")
|
||||
|
||||
_, err = s.validateProxyConnect("proxy-1", "not a hostname", scopedCtx("acc-1"))
|
||||
require.Error(t, err)
|
||||
st, ok = grpcstatus.FromError(err)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, codes.InvalidArgument, st.Code(), "invalid address must stay InvalidArgument")
|
||||
}
|
||||
|
||||
// TestValidateProxyConnect_AuthorizerReceivesClaim pins the hook contract: the
|
||||
// authorizer sees the presented token and the claimed proxy ID and address,
|
||||
// and an authorized claim proceeds.
|
||||
func TestValidateProxyConnect_AuthorizerReceivesClaim(t *testing.T) {
|
||||
s := authorizerServer(t)
|
||||
auth := &capturingAuthorizer{}
|
||||
s.SetProxyConnectAuthorizer(auth)
|
||||
|
||||
params, err := s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "cluster.example.com", params.address)
|
||||
|
||||
require.Equal(t, 1, auth.called, "authorizer must be consulted exactly once per connect")
|
||||
assert.Equal(t, "proxy-1", auth.proxyID)
|
||||
assert.Equal(t, "cluster.example.com", auth.address)
|
||||
require.NotNil(t, auth.token, "the presented token must be handed to the authorizer")
|
||||
require.NotNil(t, auth.token.AccountID)
|
||||
assert.Equal(t, "acc-1", *auth.token.AccountID)
|
||||
}
|
||||
|
||||
// TestValidateProxyConnect_PlainErrorBecomesPermissionDenied pins the error
|
||||
// mapping: a non-status error from the authorizer surfaces as
|
||||
// PermissionDenied — distinguishable from the AlreadyExists used for address
|
||||
// conflicts — and the claim does not proceed even though the address itself
|
||||
// was available.
|
||||
func TestValidateProxyConnect_PlainErrorBecomesPermissionDenied(t *testing.T) {
|
||||
s := authorizerServer(t)
|
||||
s.SetProxyConnectAuthorizer(&capturingAuthorizer{err: errors.New("not the assigned credential")})
|
||||
|
||||
_, err := s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1"))
|
||||
require.Error(t, err)
|
||||
st, ok := grpcstatus.FromError(err)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, codes.PermissionDenied, st.Code())
|
||||
assert.Contains(t, st.Message(), "not the assigned credential", "the authorizer's reason must survive into the status message")
|
||||
}
|
||||
|
||||
// TestValidateProxyConnect_StatusErrorPassesThrough pins that an authorizer
|
||||
// which chooses its own status code is not second-guessed.
|
||||
func TestValidateProxyConnect_StatusErrorPassesThrough(t *testing.T) {
|
||||
s := authorizerServer(t)
|
||||
s.SetProxyConnectAuthorizer(&capturingAuthorizer{
|
||||
err: grpcstatus.Errorf(codes.ResourceExhausted, "try later"),
|
||||
})
|
||||
|
||||
_, err := s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1"))
|
||||
require.Error(t, err)
|
||||
st, ok := grpcstatus.FromError(err)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, codes.ResourceExhausted, st.Code(), "a status error must pass through unchanged")
|
||||
assert.Equal(t, "try later", st.Message())
|
||||
}
|
||||
|
||||
// TestValidateProxyConnect_AuthorizerSeesGlobalAndTokenlessConnects pins the
|
||||
// call-site placement: the authorizer sits outside the account-scoped branch,
|
||||
// so management-wide tokens (AccountID == nil) and connections without any
|
||||
// token are also presented to it rather than bypassing policy.
|
||||
func TestValidateProxyConnect_AuthorizerSeesGlobalAndTokenlessConnects(t *testing.T) {
|
||||
s := &ProxyServiceServer{} // no proxy manager: neither path may reach the availability check
|
||||
|
||||
auth := &capturingAuthorizer{}
|
||||
s.SetProxyConnectAuthorizer(auth)
|
||||
|
||||
_, err := s.validateProxyConnect("proxy-1", "cluster.example.com", globalCtx())
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 1, auth.called, "a management-wide token must still be presented to the authorizer")
|
||||
require.NotNil(t, auth.token)
|
||||
assert.Nil(t, auth.token.AccountID)
|
||||
|
||||
_, err = s.validateProxyConnect("proxy-1", "cluster.example.com", context.Background())
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 2, auth.called, "a token-less connect must still be presented to the authorizer")
|
||||
assert.Nil(t, auth.token, "no token in context must surface as a nil token, not a zero value")
|
||||
}
|
||||
|
||||
// TestValidateProxyConnect_AuthorizerRunsLast pins the ordering: input
|
||||
// validation and the availability check precede policy, so the authorizer is
|
||||
// never consulted about a claim that is malformed or already rejected.
|
||||
func TestValidateProxyConnect_AuthorizerRunsLast(t *testing.T) {
|
||||
ctrl := gomock.NewController(t)
|
||||
mgr := proxy.NewMockManager(ctrl)
|
||||
mgr.EXPECT().IsClusterAddressAvailable(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil)
|
||||
s := &ProxyServiceServer{proxyManager: mgr}
|
||||
|
||||
auth := &capturingAuthorizer{}
|
||||
s.SetProxyConnectAuthorizer(auth)
|
||||
|
||||
_, err := s.validateProxyConnect("proxy-1", "not a hostname", scopedCtx("acc-1"))
|
||||
require.Error(t, err)
|
||||
assert.Zero(t, auth.called, "a malformed address must be rejected before policy runs")
|
||||
|
||||
_, err = s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1"))
|
||||
require.Error(t, err)
|
||||
st, ok := grpcstatus.FromError(err)
|
||||
require.True(t, ok)
|
||||
assert.Equal(t, codes.AlreadyExists, st.Code(), "an address conflict must keep its own status")
|
||||
assert.Zero(t, auth.called, "a conflicting address must be rejected before policy runs")
|
||||
}
|
||||
Reference in New Issue
Block a user