[Client] Surface readable Authz errors and add profile claim command (#7540)

* [client] Surface error messages for IPC authz in UI (#7553)

client/server/claim_profile_test.go

@@ -0,0 +1,185 @@
+package server
+
+import (
+	"context"
+	"fmt"
+	"os/user"
+	"path/filepath"
+	"runtime"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	gstatus "google.golang.org/grpc/status"
+
+	"github.com/netbirdio/netbird/client/internal/ipcauth"
+	"github.com/netbirdio/netbird/client/internal/profilemanager"
+	"github.com/netbirdio/netbird/client/proto"
+)
+
+// claimOwner names an owner the platform could actually hold, the way
+// privilegedIdentity does for callers: a uid names nobody on Windows, where an
+// owner is a SID, so a hardcoded one is refused before a test reaches what it
+// is checking. The account itself need not exist, since a claim never looks one
+// up.
+func claimOwner(n uint32) string {
+	if runtime.GOOS == "windows" {
+		return ipcauth.OwnerPrincipalForIdentity(ipcauth.Identity{SID: fmt.Sprintf("S-1-5-21-1-2-3-%d", n)})
+	}
+	return ipcauth.OwnerPrincipalForIdentity(ipcauth.Identity{UID: n})
+}
+
+// claimTestServer points the profile manager at a temp dir holding one default
+// profile, which is the profile a claim exists to settle.
+func claimTestServer(t *testing.T) *Server {
+	t.Helper()
+
+	origDir, origPath, origActive := profilemanager.DefaultConfigPathDir, profilemanager.DefaultConfigPath, profilemanager.ActiveProfileStatePath
+	t.Cleanup(func() {
+		profilemanager.DefaultConfigPathDir = origDir
+		profilemanager.DefaultConfigPath = origPath
+		profilemanager.ActiveProfileStatePath = origActive
+	})
+
+	dir := t.TempDir()
+	profilemanager.DefaultConfigPathDir = dir
+	profilemanager.DefaultConfigPath = filepath.Join(dir, "default.json")
+	profilemanager.ActiveProfileStatePath = filepath.Join(dir, "active_profile.json")
+
+	sm := profilemanager.NewServiceManager("")
+	require.NoError(t, sm.CreateDefaultProfile())
+
+	srv := newTestServer()
+	srv.profileManager = sm
+	return srv
+}
+
+func TestClaimProfile_RecordsTheOwner(t *testing.T) {
+	srv := claimTestServer(t)
+
+	owner := claimOwner(4242)
+	resp, err := srv.ClaimProfile(rootCtx(), &proto.ClaimProfileRequest{
+		Handle: "default",
+		Owner:  owner,
+	})
+	require.NoError(t, err)
+	assert.Equal(t, "default", resp.GetId())
+	assert.Equal(t, owner, resp.GetOwner())
+
+	list, err := srv.ListProfiles(rootCtx(), &proto.ListProfilesRequest{})
+	require.NoError(t, err)
+	require.Len(t, list.GetProfiles(), 1)
+	assert.Equal(t, []string{owner}, list.GetProfiles()[0].GetOwners(),
+		"the listing has to show the owner, it is the only way to confirm a claim")
+}
+
+// The owner is not looked up, so a claim lands on a machine whose accounts do
+// not exist yet. Only its shape is checked.
+func TestClaimProfile_RejectsWhatWouldMatchNobody(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		owner string
+	}{
+		{"unparseable uid", "uid:abc"},
+		{"unknown kind", "bogus:1000"},
+		{"malformed sid", "sid:hello"},
+		{"bare number", "1000"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			srv := claimTestServer(t)
+
+			_, err := srv.ClaimProfile(rootCtx(), &proto.ClaimProfileRequest{
+				Handle: "default",
+				Owner:  tc.owner,
+			})
+			require.Error(t, err)
+			assert.Equal(t, codes.InvalidArgument, gstatus.Convert(err).Code())
+
+			list, err := srv.ListProfiles(rootCtx(), &proto.ListProfilesRequest{})
+			require.NoError(t, err)
+			assert.Empty(t, list.GetProfiles()[0].GetOwners(),
+				"a refused claim must leave the profile as it was")
+		})
+	}
+}
+
+func TestClaimProfile_RequiresBothArguments(t *testing.T) {
+	srv := claimTestServer(t)
+
+	_, err := srv.ClaimProfile(rootCtx(), &proto.ClaimProfileRequest{Owner: "uid:4242"})
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, gstatus.Convert(err).Code())
+
+	_, err = srv.ClaimProfile(rootCtx(), &proto.ClaimProfileRequest{Handle: "default"})
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, gstatus.Convert(err).Code())
+}
+
+func TestClaimProfile_RefusesAnUnknownProfile(t *testing.T) {
+	srv := claimTestServer(t)
+
+	_, err := srv.ClaimProfile(rootCtx(), &proto.ClaimProfileRequest{
+		Handle: "no-such-profile",
+		Owner:  claimOwner(4242),
+	})
+	require.Error(t, err)
+	assert.Equal(t, codes.NotFound, gstatus.Convert(err).Code())
+}
+
+func TestClaimProfile_NeedsAnIdentifiedCaller(t *testing.T) {
+	srv := claimTestServer(t)
+
+	_, err := srv.ClaimProfile(context.Background(), &proto.ClaimProfileRequest{
+		Handle: "default",
+		Owner:  claimOwner(4242),
+	})
+	require.Error(t, err)
+	assert.Equal(t, codes.Unauthenticated, gstatus.Convert(err).Code())
+}
+
+// A principal is taken as given. The account deliberately does not exist, which
+// is the provisioning case: a machine-wide profile is configured before the
+// account that will own it.
+func TestClaimProfile_TakesAPrincipalWithoutResolvingIt(t *testing.T) {
+	for _, owner := range []string{claimOwner(4242), claimOwner(999999)} {
+		t.Run(owner, func(t *testing.T) {
+			srv := claimTestServer(t)
+
+			resp, err := srv.ClaimProfile(rootCtx(), &proto.ClaimProfileRequest{
+				Handle: "default",
+				Owner:  owner,
+			})
+			require.NoError(t, err, "a principal must never need an account lookup")
+			assert.Equal(t, owner, resp.GetOwner())
+		})
+	}
+}
+
+func TestClaimProfile_ResolvesAnAccountName(t *testing.T) {
+	srv := claimTestServer(t)
+
+	u, err := user.Current()
+	require.NoError(t, err)
+	want, ok := profilemanager.PrincipalForUser(u)
+	require.True(t, ok)
+
+	resp, err := srv.ClaimProfile(rootCtx(), &proto.ClaimProfileRequest{
+		Handle: "default",
+		Owner:  u.Username,
+	})
+	require.NoError(t, err)
+	assert.Equal(t, want, resp.GetOwner(),
+		"a name has no shortcut, only a lookup turns it into a principal")
+}
+
+func TestClaimProfile_RefusesAnUnknownAccountName(t *testing.T) {
+	srv := claimTestServer(t)
+
+	_, err := srv.ClaimProfile(rootCtx(), &proto.ClaimProfileRequest{
+		Handle: "default",
+		Owner:  "no-such-account-here",
+	})
+	require.Error(t, err)
+	assert.Equal(t, codes.InvalidArgument, gstatus.Convert(err).Code())
+}

client/server/server.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/netbirdio/netbird/client/internal/auth"
 	"github.com/netbirdio/netbird/client/internal/expose"
+	"github.com/netbirdio/netbird/client/internal/getent"
 	"github.com/netbirdio/netbird/client/internal/ipcauth"
 	"github.com/prometheus/client_golang/prometheus"
 
@@ -2480,6 +2481,80 @@ func (s *Server) RemoveProfile(ctx context.Context, msg *proto.RemoveProfileRequ
 	return &proto.RemoveProfileResponse{Id: resolved.ID.String()}, nil
 }
 
+// ClaimProfile records an owner on a profile.
+//
+// Root or administrator only, enforced by the gate. The owner is whoever the
+// caller names rather than the caller's own identity, so it is turned into a
+// principal by ownerPrincipal and validated before anything is written.
+func (s *Server) ClaimProfile(ctx context.Context, msg *proto.ClaimProfileRequest) (*proto.ClaimProfileResponse, error) {
+	s.mutex.Lock()
+	defer s.mutex.Unlock()
+
+	if s.checkProfilesDisabled() {
+		return nil, gstatus.Errorf(codes.Unavailable, errProfilesDisabled)
+	}
+
+	if msg.Handle == "" {
+		return nil, gstatus.Errorf(codes.InvalidArgument, "profile must be provided")
+	}
+	if msg.Owner == "" {
+		return nil, gstatus.Errorf(codes.InvalidArgument, "owner must be provided")
+	}
+
+	principal, err := ownerPrincipal(msg.Owner)
+	if err != nil {
+		return nil, gstatus.Errorf(codes.InvalidArgument, "%v", err)
+	}
+
+	callerID, err := callerIdentity(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	resolved, err := s.resolveProfileHandle(msg.Handle, callerID)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := s.profileManager.ClaimProfile(resolved, principal); err != nil {
+		return nil, fmt.Errorf("failed to claim profile: %w", err)
+	}
+
+	s.publishProfileListChanged(resolved.Name)
+
+	return &proto.ClaimProfileResponse{
+		Id:    resolved.ID.String(),
+		Owner: principal.String(),
+	}, nil
+}
+
+// ownerPrincipal turns what the caller supplied into an owner principal.
+//
+// A principal is taken as given and never looked up. A machine-wide profile is
+// routinely configured before the account that will own it exists, and a
+// directory service that is briefly unreachable cannot be told apart from an
+// account that is not there, so requiring a lookup would refuse both. Only its
+// shape is checked. Anything else is an account name, which nothing but a lookup
+// turns into a principal.
+//
+// Names resolve here rather than on the client so the daemon's own account
+// database is the one consulted.
+func ownerPrincipal(owner string) (ipcauth.Principal, error) {
+	candidate := owner
+	if _, ok := ipcauth.ParsePrincipal(owner); !ok {
+		u, err := getent.LookupUser(owner)
+		if err != nil {
+			return ipcauth.Principal{}, fmt.Errorf("resolve account %q: %w", owner, err)
+		}
+		resolved, ok := profilemanager.PrincipalForUser(u)
+		if !ok {
+			return ipcauth.Principal{}, fmt.Errorf("account %q has no usable id %q", owner, u.Uid)
+		}
+		candidate = resolved
+	}
+	return ipcauth.ValidatePrincipal(candidate)
+}
+
 // publishProfileListChanged nudges the desktop UI to refresh its profile list
 // after a CLI-driven add/remove. The daemon exposes no dedicated
 // profile-changed RPC event, and a profile add/remove doesn't move the
@@ -2540,10 +2615,15 @@ func (s *Server) ListProfiles(ctx context.Context, msg *proto.ListProfilesReques
 		Profiles: make([]*proto.Profile, len(profiles)),
 	}
 	for i, profile := range profiles {
+		owners := make([]string, 0, len(profile.Owners))
+		for _, owner := range profile.Owners {
+			owners = append(owners, owner.String())
+		}
 		response.Profiles[i] = &proto.Profile{
 			Id:       profile.ID.String(),
 			Name:     profile.Name,
 			IsActive: profile.IsActive,
+			Owners:   owners,
 		}
 	}
 
@@ -2568,15 +2648,21 @@ func (s *Server) GetActiveProfile(ctx context.Context, msg *proto.GetActiveProfi
 		return nil, gstatus.Error(codes.Unauthenticated, "caller identity could not be resolved")
 	}
 
-	// Fallback to legacy name == ID
-	displayName := activeProfile.ID.String()
-	if activeProfile.ID != profilemanager.DefaultProfileName {
-		if profiles, lerr := s.profileManager.ListProfiles(userID); lerr == nil {
-			for _, p := range profiles {
-				if p.ID == activeProfile.ID {
-					displayName = p.Name
-					break
-				}
+	// The name is resolved through the caller's own listing, so a profile
+	// belonging to somebody else is not in it. Leave the name empty rather than
+	// falling back to the ID: a 32 character hex string tells the user nothing,
+	// and the owner's chosen name is not the caller's to read. Clients render
+	// their own wording for an active profile that is not theirs.
+	//
+	// A legacy profile is its own name, so the ID stands in for it.
+	displayName := ""
+	if activeProfile.ID == profilemanager.DefaultProfileName {
+		displayName = activeProfile.ID.String()
+	} else if profiles, lerr := s.profileManager.ListProfiles(userID); lerr == nil {
+		for _, p := range profiles {
+			if p.ID == activeProfile.ID {
+				displayName = p.Name
+				break
 			}
 		}
 	}
@@ -2863,21 +2949,22 @@ func (s *Server) SessionHolder() (ipcauth.Principal, bool) {
 }
 
 // OwnsProfile reports whether the profile the handle resolves to answers to
-// this identity.
+// this identity, and what was wrong with the handle when resolution failed.
 //
 // This triggers stamping of legacy profiles, and reloads the active profile's
 // config so the stamp is visible to SessionHolder.
-func (s *Server) OwnsProfile(id ipcauth.Identity, handle string) bool {
+func (s *Server) OwnsProfile(id ipcauth.Identity, handle string) (bool, error) {
 	// Without the active profile there is nothing to fall back to and nothing
-	// to refresh, so the gate gets a no rather than a guess.
+	// to refresh, so the gate gets a no rather than a guess. The handle is not
+	// what went wrong here, so the gate is left to refuse in its own words.
 	activeProfile, err := s.profileManager.GetActiveProfileState()
 	if err != nil {
 		log.Warnf("failed to get active profile: %v", err)
-		return false
+		return false, nil
 	}
 	if activeProfile == nil {
 		log.Warn("no active profile to authorize against")
-		return false
+		return false, nil
 	}
 	if handle == "" {
 		handle = activeProfile.ID.String()
@@ -2897,10 +2984,10 @@ func (s *Server) OwnsProfile(id ipcauth.Identity, handle string) bool {
 	s.reloadActiveConfig()
 
 	if resolveErr != nil {
-		log.Errorf("failed to resolve profile %q: %v", handle, resolveErr)
-		return false
+		log.Debugf("failed to resolve profile %q: %v", handle, resolveErr)
+		return false, resolveErr
 	}
-	return resolved.AccessibleBy(id)
+	return resolved.AccessibleBy(id), nil
 }
 
 // afterProfileResolve is a seam for tests to run a concurrent profile switch

client/server/server_ownsprofile_test.go

@@ -6,6 +6,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	gstatus "google.golang.org/grpc/status"
 
 	"github.com/netbirdio/netbird/client/internal/ipcauth"
 	"github.com/netbirdio/netbird/client/internal/profilemanager"
@@ -45,7 +47,9 @@ func TestOwnsProfile_RefreshesActiveConfigForAnyHandle(t *testing.T) {
 			_, running := s.SessionHolder()
 			require.False(t, running, "fixture is wrong: the stale copy already names an owner")
 
-			require.True(t, s.OwnsProfile(owner, tc.handle), "the caller owns every profile in this fixture")
+			owns, err := s.OwnsProfile(owner, tc.handle)
+			require.NoError(t, err)
+			require.True(t, owns, "the caller owns every profile in this fixture")
 
 			holder, running := s.SessionHolder()
 			require.True(t, running, "the claimed owner never reached the daemon's config, so the live session is unowned")
@@ -61,7 +65,9 @@ func TestOwnsProfile_UnreadableActiveProfileStateDenies(t *testing.T) {
 
 	require.NoError(t, os.WriteFile(profilemanager.ActiveProfileStatePath, []byte("{"), 0600))
 
-	require.False(t, s.OwnsProfile(unprivilegedIdentity(), ""))
+	owns, err := s.OwnsProfile(unprivilegedIdentity(), "")
+	require.NoError(t, err, "an unreadable active profile is not the caller's handle to fix")
+	require.False(t, owns)
 }
 
 // A config the daemon cannot re-read leaves the one it already has in place.
@@ -78,7 +84,9 @@ func TestOwnsProfile_UnreadableConfigKeepsTheOneInPlace(t *testing.T) {
 	s.config = kept
 	s.clientRunning = true
 
-	require.False(t, s.OwnsProfile(unprivilegedIdentity(), ""))
+	owns, err := s.OwnsProfile(unprivilegedIdentity(), "")
+	require.False(t, owns, "a profile that did not resolve is nobody's")
+	require.Equal(t, codes.NotFound, gstatus.Code(err), "resolution failed")
 	require.Same(t, kept, s.config, "a failed reload replaced the daemon's config")
 
 	holder, running := s.SessionHolder()
@@ -115,7 +123,9 @@ func TestOwnsProfile_ReloadFollowsASwitchThatLandsMidCheck(t *testing.T) {
 	}
 	t.Cleanup(func() { afterProfileResolve = nil })
 
-	require.True(t, s.OwnsProfile(owner, activeProfile))
+	owns, err := s.OwnsProfile(owner, activeProfile)
+	require.NoError(t, err)
+	require.True(t, owns)
 
 	require.NotNil(t, s.config.ManagementURL)
 	require.Equal(t, switchedToURL, s.config.ManagementURL.String(),
@@ -130,6 +140,8 @@ func TestOwnsProfile_IdleDaemonKeepsItsConfig(t *testing.T) {
 	s.config = untouched
 	s.clientRunning = false
 
-	require.True(t, s.OwnsProfile(unprivilegedIdentity(), activeProfile))
+	owns, err := s.OwnsProfile(unprivilegedIdentity(), activeProfile)
+	require.NoError(t, err)
+	require.True(t, owns)
 	require.Same(t, untouched, s.config)
 }

client/server/setconfig_mdm_test.go

@@ -103,8 +103,10 @@ func setupServerWithProfile(t *testing.T) (s *Server, ctx context.Context, profN
 	return s, ctx, profName, currUser.Username, cfgPath
 }
 
-// testProfileOwner is the identity userCtx carries, which is who a fixture
-// profile belongs to.
+// testProfileOwner is the identity the unprivileged test contexts carry, so a
+// fixture profile can be owned by the very caller that drives the handler.
+// Without an owner the profile is unowned, which the loader hides from every
+// unprivileged caller.
 func testProfileOwner() *ipcauth.Identity {
 	id := unprivilegedIdentity()
 	return &id

client/server/ssh_gate.go

@@ -8,9 +8,6 @@ import (
 	"strings"
 
 	log "github.com/sirupsen/logrus"
-	"google.golang.org/genproto/googleapis/rpc/errdetails"
-	"google.golang.org/grpc/codes"
-	gstatus "google.golang.org/grpc/status"
 
 	"github.com/netbirdio/netbird/client/internal/daemonaddr"
 	"github.com/netbirdio/netbird/client/internal/ipcauth"
@@ -154,7 +151,7 @@ func denyPrivileged(ctx context.Context, action, command string) error {
 	id, ok := ipcauth.CallerIdentity(ctx)
 	if !ok {
 		log.Warnf("denying %s: the caller's identity cannot be verified on this control channel", action)
-		return privilegeError(unidentifiedSummary(action), reinstallCommand())
+		return ipcauth.PrivilegeError(unidentifiedSummary(action), reinstallCommand())
 	}
 
 	if ipcauth.IsPrivilegedCaller(id) {
@@ -163,45 +160,8 @@ func denyPrivileged(ctx context.Context, action, command string) error {
 	}
 
 	log.Warnf("denying %s for unprivileged caller %s", action, id)
-	actor, command := requiredActor(command)
-	return privilegeError(privilegeSummary(action, actor), command)
-}
-
-// requiredActor names who may perform the operation and adjusts the command to
-// match. A daemon that is not itself privileged delegates to its own identity, so
-// telling that host's user to become root is wrong twice over: root is not what the
-// daemon checks for, and a rootless container has neither root nor sudo.
-func requiredActor(command string) (string, string) {
-	self, delegates := ipcauth.SelfDelegatesTo()
-	if !delegates {
-		return ipcauth.PrivilegedActor(), command
-	}
-	return fmt.Sprintf("the user the daemon runs as (%s)", self), strings.ReplaceAll(command, "sudo ", "")
-}
-
-// privilegeError builds the PermissionDenied carrying summary and command.
-func privilegeError(summary, command string) error {
-	st := gstatus.New(codes.PermissionDenied, fmt.Sprintf("%s\n\n%s", summary, command))
-
-	detailed, err := st.WithDetails(&errdetails.ErrorInfo{
-		Reason: ipcauth.ErrorReasonPrivilegeRequired,
-		Domain: ipcauth.ErrorDomain,
-		Metadata: map[string]string{
-			ipcauth.ErrorMetaSummary: summary,
-			ipcauth.ErrorMetaCommand: command,
-		},
-	})
-	if err != nil {
-		log.Debugf("attach privilege error detail: %v", err)
-		return st.Err()
-	}
-	return detailed.Err()
-}
-
-// privilegeSummary states what is refused and what it needs, in one sentence
-// that reads the same in a dialog and in a terminal.
-func privilegeSummary(action, actor string) string {
-	return fmt.Sprintf("%s requires %s.", capitalize(action), actor)
+	actor, command := ipcauth.RequiredActor(command)
+	return ipcauth.PrivilegeError(ipcauth.PrivilegeSummary(action, actor), command)
 }
 
 // unidentifiedSummary covers a control channel that carries no caller identity.