mirror of
https://github.com/netbirdio/netbird.git
synced 2026-05-29 20:19:56 +00:00
fix(review): coderabbit follow-ups
- openapi.yml: declare default: false on ServiceTargetOptions.direct_upstream so generated clients/validators reflect the documented default. - proto/proxy_service.proto: ValidateTunnelPeer doc + denied_reason list said "distribution_groups" (bearer-auth field) but the actual gate is service.access_groups. Replaced both occurrences to match the code path in checkPeerGroupAccess. - peers/manager.go (GetPeerWithGroups) + users/manager.go (GetUserWithGroups): on store error after a successful first lookup, both now return (nil, nil, err) so callers can't get a valid entity alongside a non-nil error. Findings skipped with reasons: - embedded.go merged CLI/Dashboard redirect URIs: pre-existing on origin/main, not introduced by this PR. - account_mock.go MarkPeerDisconnected zero-time UnixNano: same — pre-existing. - openapi Service schema if/then conditionals: Go-side Validate() already enforces these invariants (Private + non-empty AccessGroups, mode=http, mutually-exclusive with bearer), and oapi-codegen on OpenAPI 3.1.x doesn't honour allOf/if/then anyway. - *.patch / *.diff / b-n-p.sh: untracked personal artifacts, not part of any commit.
This commit is contained in:
@@ -113,7 +113,9 @@ func (m *managerImpl) GetPeerByTunnelIP(ctx context.Context, accountID string, i
|
||||
return m.store.GetPeerByIP(ctx, store.LockingStrengthNone, accountID, ip)
|
||||
}
|
||||
|
||||
// GetPeerWithGroups returns the peer plus its group memberships.
|
||||
// GetPeerWithGroups returns the peer plus its group memberships. Any store
|
||||
// error returns (nil, nil, err) so callers never receive a valid peer
|
||||
// alongside a non-nil error.
|
||||
func (m *managerImpl) GetPeerWithGroups(ctx context.Context, accountID, peerID string) (*peer.Peer, []*types.Group, error) {
|
||||
p, err := m.store.GetPeerByID(ctx, store.LockingStrengthNone, accountID, peerID)
|
||||
if err != nil {
|
||||
@@ -121,7 +123,7 @@ func (m *managerImpl) GetPeerWithGroups(ctx context.Context, accountID, peerID s
|
||||
}
|
||||
groups, err := m.store.GetPeerGroups(ctx, store.LockingStrengthNone, accountID, peerID)
|
||||
if err != nil {
|
||||
return p, nil, err
|
||||
return nil, nil, err
|
||||
}
|
||||
return p, groups, nil
|
||||
}
|
||||
|
||||
@@ -33,6 +33,7 @@ func (m *managerImpl) GetUser(ctx context.Context, userID string) (*types.User,
|
||||
// GetUserWithGroups returns the user and the *types.Group records for the user's AutoGroups, in the same order as
|
||||
// AutoGroups. Group ids that don't resolve to a stored group are skipped from the returned slice (the parallel id list is
|
||||
// derivable from the returned User). Wraps two store calls today; can be optimised to a single JOIN later if needed.
|
||||
// Any store error returns (nil, nil, err) so callers never receive a valid user alongside a non-nil error.
|
||||
func (m *managerImpl) GetUserWithGroups(ctx context.Context, userID string) (*types.User, []*types.Group, error) {
|
||||
user, err := m.store.GetUserByUserID(ctx, store.LockingStrengthNone, userID)
|
||||
if err != nil {
|
||||
@@ -43,7 +44,7 @@ func (m *managerImpl) GetUserWithGroups(ctx context.Context, userID string) (*ty
|
||||
}
|
||||
groupsMap, err := m.store.GetGroupsByIDs(ctx, store.LockingStrengthNone, user.AccountID, user.AutoGroups)
|
||||
if err != nil {
|
||||
return user, nil, err
|
||||
return nil, nil, err
|
||||
}
|
||||
groups := make([]*types.Group, 0, len(user.AutoGroups))
|
||||
for _, id := range user.AutoGroups {
|
||||
|
||||
@@ -3213,7 +3213,8 @@ components:
|
||||
When true, the proxy dials this target via the host's network stack
|
||||
instead of through its embedded NetBird client. Use for upstreams
|
||||
reachable without WireGuard (public APIs, LAN services, localhost
|
||||
sidecars). Default false.
|
||||
sidecars).
|
||||
default: false
|
||||
example: false
|
||||
ServiceTarget:
|
||||
type: object
|
||||
|
||||
@@ -4248,7 +4248,7 @@ type ServiceTargetOptions struct {
|
||||
// DirectUpstream When true, the proxy dials this target via the host's network stack
|
||||
// instead of through its embedded NetBird client. Use for upstreams
|
||||
// reachable without WireGuard (public APIs, LAN services, localhost
|
||||
// sidecars). Default false.
|
||||
// sidecars).
|
||||
DirectUpstream *bool `json:"direct_upstream,omitempty"`
|
||||
|
||||
// PathRewrite Controls how the request path is rewritten before forwarding to the backend. Default strips the matched prefix. "preserve" keeps the full original request path.
|
||||
|
||||
@@ -2185,7 +2185,7 @@ func (x *ValidateTunnelPeerRequest) GetDomain() string {
|
||||
// "no_user" — peer exists but is not bound to a user
|
||||
// "service_not_found"
|
||||
// "account_mismatch"
|
||||
// "not_in_group" — user resolved but not in service.distribution_groups
|
||||
// "not_in_group" — peer resolved but not in service.access_groups
|
||||
type ValidateTunnelPeerResponse struct {
|
||||
state protoimpl.MessageState
|
||||
sizeCache protoimpl.SizeCache
|
||||
|
||||
@@ -36,12 +36,12 @@ service ProxyService {
|
||||
rpc ValidateSession(ValidateSessionRequest) returns (ValidateSessionResponse);
|
||||
|
||||
// ValidateTunnelPeer resolves an inbound peer by its WireGuard tunnel IP and
|
||||
// checks the resolved user's access against the service's required groups.
|
||||
// checks the resolved user's access against the service's access_groups.
|
||||
// Acts as a fast-path equivalent of OIDC for requests originating on the
|
||||
// netbird mesh: when the source IP maps to a known peer in the calling
|
||||
// account and that peer's user is in the service's distribution_groups,
|
||||
// the proxy can issue a session cookie without redirecting through the
|
||||
// OIDC flow. Mirrors ValidateSession's response shape.
|
||||
// account and that peer is in the service's access_groups, the proxy can
|
||||
// issue a session cookie without redirecting through the OIDC flow.
|
||||
// Mirrors ValidateSession's response shape.
|
||||
rpc ValidateTunnelPeer(ValidateTunnelPeerRequest) returns (ValidateTunnelPeerResponse);
|
||||
}
|
||||
|
||||
@@ -323,7 +323,7 @@ message ValidateTunnelPeerRequest {
|
||||
// "no_user" — peer exists but is not bound to a user
|
||||
// "service_not_found"
|
||||
// "account_mismatch"
|
||||
// "not_in_group" — user resolved but not in service.distribution_groups
|
||||
// "not_in_group" — peer resolved but not in service.access_groups
|
||||
message ValidateTunnelPeerResponse {
|
||||
bool valid = 1;
|
||||
string user_id = 2;
|
||||
|
||||
@@ -36,12 +36,12 @@ type ProxyServiceClient interface {
|
||||
// Called by the proxy after receiving a session token from OIDC callback.
|
||||
ValidateSession(ctx context.Context, in *ValidateSessionRequest, opts ...grpc.CallOption) (*ValidateSessionResponse, error)
|
||||
// ValidateTunnelPeer resolves an inbound peer by its WireGuard tunnel IP and
|
||||
// checks the resolved user's access against the service's required groups.
|
||||
// checks the resolved user's access against the service's access_groups.
|
||||
// Acts as a fast-path equivalent of OIDC for requests originating on the
|
||||
// netbird mesh: when the source IP maps to a known peer in the calling
|
||||
// account and that peer's user is in the service's distribution_groups,
|
||||
// the proxy can issue a session cookie without redirecting through the
|
||||
// OIDC flow. Mirrors ValidateSession's response shape.
|
||||
// account and that peer is in the service's access_groups, the proxy can
|
||||
// issue a session cookie without redirecting through the OIDC flow.
|
||||
// Mirrors ValidateSession's response shape.
|
||||
ValidateTunnelPeer(ctx context.Context, in *ValidateTunnelPeerRequest, opts ...grpc.CallOption) (*ValidateTunnelPeerResponse, error)
|
||||
}
|
||||
|
||||
@@ -201,12 +201,12 @@ type ProxyServiceServer interface {
|
||||
// Called by the proxy after receiving a session token from OIDC callback.
|
||||
ValidateSession(context.Context, *ValidateSessionRequest) (*ValidateSessionResponse, error)
|
||||
// ValidateTunnelPeer resolves an inbound peer by its WireGuard tunnel IP and
|
||||
// checks the resolved user's access against the service's required groups.
|
||||
// checks the resolved user's access against the service's access_groups.
|
||||
// Acts as a fast-path equivalent of OIDC for requests originating on the
|
||||
// netbird mesh: when the source IP maps to a known peer in the calling
|
||||
// account and that peer's user is in the service's distribution_groups,
|
||||
// the proxy can issue a session cookie without redirecting through the
|
||||
// OIDC flow. Mirrors ValidateSession's response shape.
|
||||
// account and that peer is in the service's access_groups, the proxy can
|
||||
// issue a session cookie without redirecting through the OIDC flow.
|
||||
// Mirrors ValidateSession's response shape.
|
||||
ValidateTunnelPeer(context.Context, *ValidateTunnelPeerRequest) (*ValidateTunnelPeerResponse, error)
|
||||
mustEmbedUnimplementedProxyServiceServer()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user