diff --git a/client/cmd/login.go b/client/cmd/login.go index bd37e30f1..4d5d8da54 100644 --- a/client/cmd/login.go +++ b/client/cmd/login.go @@ -102,11 +102,11 @@ func doDaemonLogin(ctx context.Context, cmd *cobra.Command, providedSetupKey str IsUnixDesktopClient: isUnixRunningDesktop(), Hostname: hostName, DnsLabels: dnsLabelsReq, - ProfileName: &activeProf.Name, + ProfileName: &activeProf.ID, Username: &username, } - profileState, err := pm.GetProfileState(activeProf.Name) + profileState, err := pm.GetProfileState(activeProf.ID) if err != nil { log.Debugf("failed to get profile state for login hint: %v", err) } else if profileState.Email != "" { @@ -170,14 +170,13 @@ func getActiveProfile(ctx context.Context, pm *profilemanager.ProfileManager, pr return activeProf, nil } -func switchProfileOnDaemon(ctx context.Context, pm *profilemanager.ProfileManager, profileName string, username string) error { - err := switchProfile(context.Background(), profileName, username) +func switchProfileOnDaemon(ctx context.Context, pm *profilemanager.ProfileManager, handle string, username string) error { + resolvedID, err := switchProfile(context.Background(), handle, username) if err != nil { return fmt.Errorf("switch profile on daemon: %v", err) } - err = pm.SwitchProfile(profileName) - if err != nil { + if err := pm.SwitchProfile(resolvedID); err != nil { return fmt.Errorf("switch profile: %v", err) } @@ -205,11 +204,15 @@ func switchProfileOnDaemon(ctx context.Context, pm *profilemanager.ProfileManage return nil } -func switchProfile(ctx context.Context, profileName string, username string) error { +// switchProfile asks the daemon to switch to the profile identified by +// handle (a name, ID, or unique ID prefix). Returns the resolved profile +// ID so the caller can update the local active-profile state without +// re-resolving the handle. +func switchProfile(ctx context.Context, handle string, username string) (string, error) { conn, err := DialClientGRPCServer(ctx, daemonAddr) if err != nil { //nolint - return fmt.Errorf("failed to connect to daemon error: %v\n"+ + return "", fmt.Errorf("failed to connect to daemon error: %v\n"+ "If the daemon is not running please run: "+ "\nnetbird service install \nnetbird service start\n", err) } @@ -217,15 +220,15 @@ func switchProfile(ctx context.Context, profileName string, username string) err client := proto.NewDaemonServiceClient(conn) - _, err = client.SwitchProfile(ctx, &proto.SwitchProfileRequest{ - ProfileName: &profileName, + resp, err := client.SwitchProfile(ctx, &proto.SwitchProfileRequest{ + ProfileName: &handle, Username: &username, }) if err != nil { - return fmt.Errorf("switch profile failed: %v", err) + return "", fmt.Errorf("switch profile failed: %v", err) } - return nil + return resp.Id, nil } func doForegroundLogin(ctx context.Context, cmd *cobra.Command, setupKey string, activeProf *profilemanager.Profile) error { @@ -249,7 +252,7 @@ func doForegroundLogin(ctx context.Context, cmd *cobra.Command, setupKey string, return fmt.Errorf("read config file %s: %v", configFilePath, err) } - err = foregroundLogin(ctx, cmd, config, setupKey, activeProf.Name) + err = foregroundLogin(ctx, cmd, config, setupKey, activeProf.ID) if err != nil { return fmt.Errorf("foreground login failed: %v", err) } diff --git a/client/cmd/login_test.go b/client/cmd/login_test.go index 47522e189..0aa1856b1 100644 --- a/client/cmd/login_test.go +++ b/client/cmd/login_test.go @@ -27,7 +27,7 @@ func TestLogin(t *testing.T) { profilemanager.ActiveProfileStatePath = tempDir + "/active_profile.json" sm := profilemanager.ServiceManager{} err = sm.SetActiveProfileState(&profilemanager.ActiveProfileState{ - Name: "default", + ID: "default", Username: currUser.Username, }) if err != nil { diff --git a/client/cmd/profile.go b/client/cmd/profile.go index d6e81760f..d6ea6354e 100644 --- a/client/cmd/profile.go +++ b/client/cmd/profile.go @@ -2,11 +2,16 @@ package cmd import ( "context" + "errors" "fmt" "os/user" + "strings" + "text/tabwriter" "time" "github.com/spf13/cobra" + "google.golang.org/grpc/codes" + gstatus "google.golang.org/grpc/status" "github.com/netbirdio/netbird/client/internal" "github.com/netbirdio/netbird/client/internal/profilemanager" @@ -14,6 +19,8 @@ import ( "github.com/netbirdio/netbird/util" ) +var profileListShowID bool + var profileCmd = &cobra.Command{ Use: "profile", Short: "Manage NetBird client profiles", @@ -31,27 +38,32 @@ var profileListCmd = &cobra.Command{ var profileAddCmd = &cobra.Command{ Use: "add ", Short: "Add a new profile", - Long: `Add a new profile to the NetBird client. The profile name must be unique.`, + Long: `Add a new profile. Profile name is free-form, a unique ID is generated for the on-disk config file.`, Args: cobra.ExactArgs(1), RunE: addProfileFunc, } var profileRemoveCmd = &cobra.Command{ - Use: "remove ", - Short: "Remove a profile", - Long: `Remove a profile from the NetBird client. The profile must not be inactive.`, - Args: cobra.ExactArgs(1), - RunE: removeProfileFunc, + Use: "remove ", + Short: "Remove a profile", + Long: `Remove a profile by name, ID, or unique ID prefix.`, + Aliases: []string{"rm"}, + Args: cobra.ExactArgs(1), + RunE: removeProfileFunc, } var profileSelectCmd = &cobra.Command{ - Use: "select ", + Use: "select ", Short: "Select a profile", - Long: `Make the specified profile active. This will switch the client to use the selected profile's configuration.`, + Long: `Make the specified profile active. Accepts a name, ID, or unique ID prefix.`, Args: cobra.ExactArgs(1), RunE: selectProfileFunc, } +func init() { + profileListCmd.Flags().BoolVar(&profileListShowID, "show-id", false, "show the profile ID column") +} + func setupCmd(cmd *cobra.Command) error { SetFlagsFromEnvVars(rootCmd) SetFlagsFromEnvVars(cmd) @@ -65,6 +77,7 @@ func setupCmd(cmd *cobra.Command) error { return nil } + func listProfilesFunc(cmd *cobra.Command, _ []string) error { if err := setupCmd(cmd); err != nil { return err @@ -83,25 +96,32 @@ func listProfilesFunc(cmd *cobra.Command, _ []string) error { daemonClient := proto.NewDaemonServiceClient(conn) - profiles, err := daemonClient.ListProfiles(cmd.Context(), &proto.ListProfilesRequest{ + resp, err := daemonClient.ListProfiles(cmd.Context(), &proto.ListProfilesRequest{ Username: currUser.Username, }) if err != nil { return err } - // list profiles, add a tick if the profile is active - cmd.Println("Found", len(profiles.Profiles), "profiles:") - for _, profile := range profiles.Profiles { - // use a cross to indicate the passive profiles - activeMarker := "✗" - if profile.IsActive { - activeMarker = "✓" - } - cmd.Println(activeMarker, profile.Name) + tw := tabwriter.NewWriter(cmd.OutOrStdout(), 0, 0, 2, ' ', 0) + if profileListShowID { + fmt.Fprintln(tw, "ID\tNAME\tACTIVE") + } else { + fmt.Fprintln(tw, "NAME\tACTIVE") } - - return nil + for _, profile := range resp.Profiles { + marker := "" + if profile.IsActive { + marker = "✓" + } + name := profilemanager.StripCtrlChars(profile.Name) + if profileListShowID { + fmt.Fprintf(tw, "%s\t%s\t%s\n", profilemanager.ShortID(profile.Id), name, marker) + } else { + fmt.Fprintf(tw, "%s\t%s\n", name, marker) + } + } + return tw.Flush() } func addProfileFunc(cmd *cobra.Command, args []string) error { @@ -121,19 +141,49 @@ func addProfileFunc(cmd *cobra.Command, args []string) error { } daemonClient := proto.NewDaemonServiceClient(conn) - profileName := args[0] - _, err = daemonClient.AddProfile(cmd.Context(), &proto.AddProfileRequest{ + resp, err := daemonClient.AddProfile(cmd.Context(), &proto.AddProfileRequest{ ProfileName: profileName, Username: currUser.Username, }) - if err != nil { - return err + if err == nil { + cmd.Printf("Profile added: %s %s\n", profilemanager.ShortID(resp.Id), profilemanager.StripCtrlChars(profileName)) + return nil } - cmd.Println("Profile added successfully:", profileName) - return nil + if st, ok := gstatus.FromError(err); ok && st.Code() == codes.AlreadyExists { + dupCount, _ := countProfilesWithName(cmd.Context(), daemonClient, currUser.Username, profileName) + if dupCount > 0 { + cmd.Printf("Warning: %d other profile(s) already use the name %q.\n", dupCount, profileName) + cmd.Println("Use `netbird profile list --show-id` to disambiguate later.") + } + resp, err = daemonClient.AddProfile(cmd.Context(), &proto.AddProfileRequest{ + ProfileName: profileName, + Username: currUser.Username, + }) + if err != nil { + return err + } + cmd.Printf("Profile added: %s %s\n", profilemanager.ShortID(resp.Id), profilemanager.StripCtrlChars(profileName)) + return nil + } + + return err +} + +func countProfilesWithName(ctx context.Context, c proto.DaemonServiceClient, username, name string) (int, error) { + resp, err := c.ListProfiles(ctx, &proto.ListProfilesRequest{Username: username}) + if err != nil { + return 0, err + } + n := 0 + for _, p := range resp.Profiles { + if p.Name == name { + n++ + } + } + return n, nil } func removeProfileFunc(cmd *cobra.Command, args []string) error { @@ -153,18 +203,17 @@ func removeProfileFunc(cmd *cobra.Command, args []string) error { } daemonClient := proto.NewDaemonServiceClient(conn) + handle := args[0] - profileName := args[0] - - _, err = daemonClient.RemoveProfile(cmd.Context(), &proto.RemoveProfileRequest{ - ProfileName: profileName, + resp, err := daemonClient.RemoveProfile(cmd.Context(), &proto.RemoveProfileRequest{ + ProfileName: handle, Username: currUser.Username, }) if err != nil { - return err + return wrapAmbiguityError(err, handle) } - cmd.Println("Profile removed successfully:", profileName) + cmd.Printf("Profile removed: %s\n", resp.Id) return nil } @@ -174,7 +223,7 @@ func selectProfileFunc(cmd *cobra.Command, args []string) error { } profileManager := profilemanager.NewProfileManager() - profileName := args[0] + handle := args[0] currUser, err := user.Current() if err != nil { @@ -191,32 +240,15 @@ func selectProfileFunc(cmd *cobra.Command, args []string) error { daemonClient := proto.NewDaemonServiceClient(conn) - profiles, err := daemonClient.ListProfiles(ctx, &proto.ListProfilesRequest{ - Username: currUser.Username, + switchResp, err := daemonClient.SwitchProfile(ctx, &proto.SwitchProfileRequest{ + ProfileName: &handle, + Username: &currUser.Username, }) if err != nil { - return fmt.Errorf("list profiles: %w", err) + return wrapAmbiguityError(err, handle) } - var profileExists bool - - for _, profile := range profiles.Profiles { - if profile.Name == profileName { - profileExists = true - break - } - } - - if !profileExists { - return fmt.Errorf("profile %s does not exist", profileName) - } - - if err := switchProfile(cmd.Context(), profileName, currUser.Username); err != nil { - return err - } - - err = profileManager.SwitchProfile(profileName) - if err != nil { + if err := profileManager.SwitchProfile(switchResp.Id); err != nil { return err } @@ -231,6 +263,29 @@ func selectProfileFunc(cmd *cobra.Command, args []string) error { } } - cmd.Println("Profile switched successfully to:", profileName) + cmd.Printf("Profile switched to: %s\n", profilemanager.ShortID(switchResp.Id)) return nil } + +// wrapAmbiguityError turns the daemon's gRPC InvalidArgument errors +// (which carry the resolver's message verbatim) into CLI-friendly text +// that points the user at --show-id. +func wrapAmbiguityError(err error, handle string) error { + if err == nil { + return nil + } + st, ok := gstatus.FromError(err) + if !ok { + return err + } + switch st.Code() { + case codes.InvalidArgument: + msg := st.Message() + if strings.Contains(msg, "ambiguous") { + return errors.New(msg + "\nRun `netbird profile list --show-id` to see IDs, then select by ID prefix:\n netbird profile select|remove ") + } + case codes.NotFound: + return fmt.Errorf("profile %q not found", handle) + } + return err +} diff --git a/client/cmd/up.go b/client/cmd/up.go index cabd0aacf..96403f883 100644 --- a/client/cmd/up.go +++ b/client/cmd/up.go @@ -128,13 +128,12 @@ func upFunc(cmd *cobra.Command, args []string) error { var profileSwitched bool // switch profile if provided if profileName != "" { - err = switchProfile(cmd.Context(), profileName, username.Username) + resolvedID, err := switchProfile(cmd.Context(), profileName, username.Username) if err != nil { return fmt.Errorf("switch profile: %v", err) } - err = pm.SwitchProfile(profileName) - if err != nil { + if err := pm.SwitchProfile(resolvedID); err != nil { return fmt.Errorf("switch profile: %v", err) } @@ -261,10 +260,10 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command, pm *profilemanager } // set the new config - req := setupSetConfigReq(customDNSAddressConverted, cmd, activeProf.Name, username.Username) + req := setupSetConfigReq(customDNSAddressConverted, cmd, activeProf.ID, username.Username) if _, err := client.SetConfig(ctx, req); err != nil { if st, ok := gstatus.FromError(err); ok && st.Code() == codes.Unavailable { - log.Warnf("setConfig method is not available in the daemon") + log.Warnf("setConfig method is not available in the daemon: %s", st.Message()) } else { return fmt.Errorf("call service setConfig method: %v", err) } @@ -289,10 +288,10 @@ func doDaemonUp(ctx context.Context, cmd *cobra.Command, client proto.DaemonServ return fmt.Errorf("setup login request: %v", err) } - loginRequest.ProfileName = &activeProf.Name + loginRequest.ProfileName = &activeProf.ID loginRequest.Username = &username - profileState, err := pm.GetProfileState(activeProf.Name) + profileState, err := pm.GetProfileState(activeProf.ID) if err != nil { log.Debugf("failed to get profile state for login hint: %v", err) } else if profileState.Email != "" { @@ -329,7 +328,7 @@ func doDaemonUp(ctx context.Context, cmd *cobra.Command, client proto.DaemonServ } if _, err := client.Up(ctx, &proto.UpRequest{ - ProfileName: &activeProf.Name, + ProfileName: &activeProf.ID, Username: &username, }); err != nil { return fmt.Errorf("call service up method: %v", err) diff --git a/client/cmd/up_daemon_test.go b/client/cmd/up_daemon_test.go index 682a45365..ea4cdf162 100644 --- a/client/cmd/up_daemon_test.go +++ b/client/cmd/up_daemon_test.go @@ -29,14 +29,14 @@ func TestUpDaemon(t *testing.T) { } sm := profilemanager.ServiceManager{} - err = sm.AddProfile("test1", currUser.Username) + created, err := sm.AddProfile("test1", currUser.Username) if err != nil { t.Fatalf("failed to add profile: %v", err) return } err = sm.SetActiveProfileState(&profilemanager.ActiveProfileState{ - Name: "test1", + ID: created.ID, Username: currUser.Username, }) if err != nil { diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index cd5bc0680..bfb4d17f8 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -103,6 +103,10 @@ type ConfigInput struct { // Config Configuration type type Config struct { + // Name is the human-readable profile name shown in CLI/UI listings. + // It is independent of the profile's on-disk filename (which is the ID). + Name string + // Wireguard private key of local peer PrivateKey string PreSharedKey string diff --git a/client/internal/profilemanager/id.go b/client/internal/profilemanager/id.go new file mode 100644 index 000000000..b1a15c1f3 --- /dev/null +++ b/client/internal/profilemanager/id.go @@ -0,0 +1,110 @@ +package profilemanager + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "path/filepath" + "strings" + "unicode" + "unicode/utf8" +) + +const ( + // profileIDByteLen is the number of random bytes generated for a new + // profile ID. The resulting hex string is twice this length. + profileIDByteLen = 16 + + // shortIDLen is the number of leading characters of an ID we render in + // list output. Profiles per device are few, so 8 chars is collision-safe + // in practice and easy to type as a prefix. + shortIDLen = 8 + + // maxProfileNameLen caps the human-readable profile name to keep table + // output legible and prevent denial-of-service via huge JSON fields. + maxProfileNameLen = 128 + + // maxProfileIDLen bounds the on-disk filename we'll accept. New + // IDs are 32 hex chars, legacy stems are sanitized profile names. The + // cap is generous enough to cover both without permitting absurdly + // long filenames. + maxProfileIDLen = 64 +) + +// generateProfileID returns a new random hex ID for a profile file. +func generateProfileID() (string, error) { + buf := make([]byte, profileIDByteLen) + if _, err := rand.Read(buf); err != nil { + return "", fmt.Errorf("read random bytes: %w", err) + } + return hex.EncodeToString(buf), nil +} + +// isValidProfileFilenameStem reports whether s is safe to use as the stem +// of a profile JSON filename. +func isValidProfileFilenameStem(s string) bool { + if s == "" || len(s) > maxProfileIDLen { + return false + } + if s == defaultProfileName { + return true + } + if strings.ContainsAny(s, `/\`) || strings.Contains(s, "..") { + return false + } + // filepath.Base catches any leftover separators on platforms with + // exotic path conventions. + if filepath.Base(s) != s { + return false + } + for _, r := range s { + if !(unicode.IsLetter(r) || unicode.IsDigit(r) || r == '_' || r == '-') { + return false + } + } + return true +} + +// sanitizeDisplayName normalizes a user-supplied profile display name for +// storage. It strips ASCII control characters, rejects invalid UTF-8, and +// caps the length. Emojis, spaces, punctuation, and non-ASCII letters are +// preserved. Returns an error if nothing usable remains. +func sanitizeDisplayName(name string) (string, error) { + if !utf8.ValidString(name) { + return "", fmt.Errorf("name is not valid UTF-8") + } + name = StripCtrlChars(name) + name = strings.TrimSpace(name) + if name == "" { + return "", fmt.Errorf("name is empty after sanitization") + } + if utf8.RuneCountInString(name) > maxProfileNameLen { + return "", fmt.Errorf("name exceeds %d characters", maxProfileNameLen) + } + return name, nil +} + +// StripCtrlChars control characters from a name before printing it. +func StripCtrlChars(name string) string { + var b strings.Builder + b.Grow(len(name)) + for _, r := range name { + // Skip C0 controls and DEL, plus C1 controls (0x80–0x9F). + if r < 0x20 || r == 0x7F || (r >= 0x80 && r <= 0x9F) { + continue + } + b.WriteRune(r) + } + return b.String() +} + +// ShortID truncates an ID for display. +func ShortID(id string) string { + if id == DefaultProfileName { + return id + } + if len(id) <= shortIDLen { + return id + } + return id[:shortIDLen] +} diff --git a/client/internal/profilemanager/profilemanager.go b/client/internal/profilemanager/profilemanager.go index c87f521cb..9b3c82478 100644 --- a/client/internal/profilemanager/profilemanager.go +++ b/client/internal/profilemanager/profilemanager.go @@ -19,19 +19,41 @@ const ( ) type Profile struct { - Name string + // ID is the on-disk filename stem (without .json). For new profiles + // it is a 32-char hex string; legacy profiles created before the + // ID-keyed layout keep their original name as their ID. The reserved + // value "default" identifies the special default profile. + ID string + // Name is the human-readable display name. Falls back to ID when the + // underlying JSON has no "name" field set. + Name string + // Path is the absolute path to the profile JSON. Populated by the + // loader so callers do not have to reconstruct it from ID + dir. + Path string IsActive bool } func (p *Profile) FilePath() (string, error) { - if p.Name == "" { - return "", fmt.Errorf("active profile name is empty") + if p.Path != "" { + return p.Path, nil } - if p.Name == defaultProfileName { + id := p.ID + if id == "" { + id = p.Name + } + if id == "" { + return "", fmt.Errorf("profile ID is empty") + } + + if id == defaultProfileName { return DefaultConfigPath, nil } + if !isValidProfileFilenameStem(id) { + return "", fmt.Errorf("invalid profile ID: %q", id) + } + username, err := user.Current() if err != nil { return "", fmt.Errorf("failed to get current user: %w", err) @@ -42,10 +64,13 @@ func (p *Profile) FilePath() (string, error) { return "", fmt.Errorf("failed to get config directory for user %s: %w", username.Username, err) } - return filepath.Join(configDir, p.Name+".json"), nil + return filepath.Join(configDir, id+".json"), nil } func (p *Profile) IsDefault() bool { + if p.ID != "" { + return p.ID == defaultProfileName + } return p.Name == defaultProfileName } @@ -57,18 +82,28 @@ func NewProfileManager() *ProfileManager { return &ProfileManager{} } +// GetActiveProfile returns the active profile as recorded in the local +// user state file. Only ID is populated; the display Name is not known +// at this layer — callers that need it should query the daemon's +// ListProfiles RPC. func (pm *ProfileManager) GetActiveProfile() (*Profile, error) { pm.mu.Lock() defer pm.mu.Unlock() - prof := pm.getActiveProfileState() - return &Profile{Name: prof}, nil + id := pm.getActiveProfileState() + return &Profile{ID: id}, nil } -func (pm *ProfileManager) SwitchProfile(profileName string) error { - profileName = sanitizeProfileName(profileName) +// SwitchProfile records the given profile ID as active in the local user +// state file. The CLI calls this after the daemon has resolved the user- +// supplied handle to a concrete ID, so the value written here is always a +// valid profile filename stem. +func (pm *ProfileManager) SwitchProfile(id string) error { + if id != defaultProfileName && !isValidProfileFilenameStem(id) { + return fmt.Errorf("invalid profile ID: %q", id) + } - if err := pm.setActiveProfileState(profileName); err != nil { + if err := pm.setActiveProfileState(id); err != nil { return fmt.Errorf("failed to switch profile: %w", err) } return nil @@ -142,7 +177,7 @@ func GetLoginHint() string { return "" } - profileState, err := pm.GetProfileState(activeProf.Name) + profileState, err := pm.GetProfileState(activeProf.ID) if err != nil { log.Debugf("failed to get profile state for login hint: %v", err) return "" diff --git a/client/internal/profilemanager/profilemanager_test.go b/client/internal/profilemanager/profilemanager_test.go index 79a7ae650..515fe1476 100644 --- a/client/internal/profilemanager/profilemanager_test.go +++ b/client/internal/profilemanager/profilemanager_test.go @@ -50,14 +50,14 @@ func TestServiceManager_CreateAndGetDefaultProfile(t *testing.T) { state, err := sm.GetActiveProfileState() assert.NoError(t, err) - assert.Equal(t, state.Name, defaultProfileName) // No active profile state yet + assert.Equal(t, defaultProfileName, state.ID) // No active profile state yet err = sm.SetActiveProfileStateToDefault() assert.NoError(t, err) active, err := sm.GetActiveProfileState() assert.NoError(t, err) - assert.Equal(t, "default", active.Name) + assert.Equal(t, "default", active.ID) }) }) } @@ -92,14 +92,14 @@ func TestServiceManager_SetActiveProfileState(t *testing.T) { currUser, err := user.Current() assert.NoError(t, err) sm := &ServiceManager{} - state := &ActiveProfileState{Name: "foo", Username: currUser.Username} + state := &ActiveProfileState{ID: "foo", Username: currUser.Username} err = sm.SetActiveProfileState(state) assert.NoError(t, err) // Should error on nil or incomplete state err = sm.SetActiveProfileState(nil) assert.Error(t, err) - err = sm.SetActiveProfileState(&ActiveProfileState{Name: "", Username: ""}) + err = sm.SetActiveProfileState(&ActiveProfileState{ID: "", Username: ""}) assert.Error(t, err) }) }) diff --git a/client/internal/profilemanager/service.go b/client/internal/profilemanager/service.go index ef3eb1114..b3c1dda9b 100644 --- a/client/internal/profilemanager/service.go +++ b/client/internal/profilemanager/service.go @@ -2,6 +2,7 @@ package profilemanager import ( "context" + "encoding/json" "errors" "fmt" "io" @@ -23,12 +24,44 @@ var ( DefaultConfigPathDir = "" DefaultConfigPath = "" ActiveProfileStatePath = "" -) -var ( ErrorOldDefaultConfigNotFound = errors.New("old default config not found") ) +// ErrAmbiguousHandle is returned when a profile handle (ID prefix or name) +// matches more than one profile. Callers can render Candidates to help the +// user disambiguate. +type ErrAmbiguousHandle struct { + Handle string + Candidates []Profile + Kind AmbiguityKind +} + +// AmbiguityKind describes which matcher produced the ambiguity, so callers +// can tailor the error message. +type AmbiguityKind int + +const ( + AmbiguityKindIDPrefix AmbiguityKind = iota + AmbiguityKindName +) + +// profileMeta is the minimal slice of a profile JSON we need for the list. +// Using a private struct here avoids running the full Config.apply() +// pipeline (which can rewrite the file) on every list operation. +type profileMeta struct { + Name string +} + +func (e *ErrAmbiguousHandle) Error() string { + switch e.Kind { + case AmbiguityKindIDPrefix: + return fmt.Sprintf("ID prefix %q is ambiguous (matches %d profiles)", e.Handle, len(e.Candidates)) + default: + return fmt.Sprintf("name %q is ambiguous (%d profiles share this name)", e.Handle, len(e.Candidates)) + } +} + func init() { DefaultConfigPathDir = "/var/lib/netbird/" @@ -54,25 +87,34 @@ func init() { } type ActiveProfileState struct { - Name string `json:"name"` + // ID is the on-disk filename stem of the active profile. The JSON + // tag stays as "name" for backwards compatibility with active state + // files written before the ID-keyed layout. Legacy values were + // profile names, which were also the legacy filename stems, so they + // still resolve to the correct file on disk. + ID string `json:"name"` Username string `json:"username"` } func (a *ActiveProfileState) FilePath() (string, error) { - if a.Name == "" { - return "", fmt.Errorf("active profile name is empty") + if a.ID == "" { + return "", fmt.Errorf("active profile ID is empty") } - if a.Name == defaultProfileName { + if a.ID == defaultProfileName { return DefaultConfigPath, nil } + if !isValidProfileFilenameStem(a.ID) { + return "", fmt.Errorf("invalid profile ID: %q", a.ID) + } + configDir, err := getConfigDirForUser(a.Username) if err != nil { return "", fmt.Errorf("failed to get config directory for user %s: %w", a.Username, err) } - return filepath.Join(configDir, a.Name+".json"), nil + return filepath.Join(configDir, a.ID+".json"), nil } type ServiceManager struct { @@ -178,7 +220,7 @@ func (s *ServiceManager) GetActiveProfileState() (*ActiveProfileState, error) { return nil, fmt.Errorf("failed to set active profile to default: %w", err) } return &ActiveProfileState{ - Name: "default", + ID: defaultProfileName, Username: "", }, nil } else { @@ -186,12 +228,12 @@ func (s *ServiceManager) GetActiveProfileState() (*ActiveProfileState, error) { } } - if activeProfile.Name == "" { + if activeProfile.ID == "" { if err := s.SetActiveProfileStateToDefault(); err != nil { return nil, fmt.Errorf("failed to set active profile to default: %w", err) } return &ActiveProfileState{ - Name: "default", + ID: defaultProfileName, Username: "", }, nil } @@ -216,25 +258,29 @@ func (s *ServiceManager) setDefaultActiveState() error { } func (s *ServiceManager) SetActiveProfileState(a *ActiveProfileState) error { - if a == nil || a.Name == "" { + if a == nil || a.ID == "" { return errors.New("invalid active profile state") } - if a.Name != defaultProfileName && a.Username == "" { - return fmt.Errorf("username must be set for non-default profiles, got: %s", a.Name) + if a.ID != defaultProfileName && a.Username == "" { + return fmt.Errorf("username must be set for non-default profiles, got: %s", a.ID) + } + + if a.ID != defaultProfileName && !isValidProfileFilenameStem(a.ID) { + return fmt.Errorf("invalid profile ID: %q", a.ID) } if err := util.WriteJsonWithRestrictedPermission(context.Background(), ActiveProfileStatePath, a); err != nil { return fmt.Errorf("failed to write active profile state: %w", err) } - log.Infof("active profile set to %s for %s", a.Name, a.Username) + log.Infof("active profile set to %s for %s", a.ID, a.Username) return nil } func (s *ServiceManager) SetActiveProfileStateToDefault() error { return s.SetActiveProfileState(&ActiveProfileState{ - Name: "default", + ID: defaultProfileName, Username: "", }) } @@ -243,57 +289,75 @@ func (s *ServiceManager) DefaultProfilePath() string { return DefaultConfigPath } -func (s *ServiceManager) AddProfile(profileName, username string) error { +// AddProfile creates a new profile with a generated ID. The user-supplied +// displayName is stored inside the JSON's name field, the on-disk filename +// uses the generated ID. +// +// The returned Profile carries the freshly-generated ID so callers can +// show it to the user (and so the gRPC AddProfileResponse can include +// it). +func (s *ServiceManager) AddProfile(displayName, username string) (*Profile, error) { configDir, err := s.getConfigDir(username) if err != nil { - return fmt.Errorf("failed to get config directory: %w", err) + return nil, fmt.Errorf("failed to get config directory: %w", err) } - profileName = sanitizeProfileName(profileName) - - if profileName == defaultProfileName { - return fmt.Errorf("cannot create profile with reserved name: %s", defaultProfileName) - } - - profPath := filepath.Join(configDir, profileName+".json") - profileExists, err := fileExists(profPath) + displayName, err = sanitizeDisplayName(displayName) if err != nil { - return fmt.Errorf("failed to check if profile exists: %w", err) - } - if profileExists { - return ErrProfileAlreadyExists + return nil, fmt.Errorf("invalid profile name: %w", err) } + if displayName == defaultProfileName { + return nil, fmt.Errorf("cannot create profile with reserved name: %s", defaultProfileName) + } + + id, err := generateProfileID() + if err != nil { + return nil, fmt.Errorf("generate profile id: %w", err) + } + + profPath := filepath.Join(configDir, id+".json") cfg, err := createNewConfig(ConfigInput{ConfigPath: profPath}) if err != nil { - return fmt.Errorf("failed to create new config: %w", err) + return nil, fmt.Errorf("failed to create new config: %w", err) + } + cfg.Name = displayName + + if err := util.WriteJson(context.Background(), profPath, cfg); err != nil { + return nil, fmt.Errorf("failed to write profile config: %w", err) } - err = util.WriteJson(context.Background(), profPath, cfg) - if err != nil { - return fmt.Errorf("failed to write profile config: %w", err) - } - - return nil + return &Profile{ + ID: id, + Name: displayName, + Path: profPath, + }, nil } -func (s *ServiceManager) RemoveProfile(profileName, username string) error { - configDir, err := s.getConfigDir(username) - if err != nil { - return fmt.Errorf("failed to get config directory: %w", err) - } - - profileName = sanitizeProfileName(profileName) - - if profileName == defaultProfileName { +// RemoveProfile deletes the profile identified by id. Callers must have +// already resolved any user-supplied handle to a concrete ID via +// ResolveProfile. +func (s *ServiceManager) RemoveProfile(id, username string) error { + if id == defaultProfileName { return fmt.Errorf("cannot remove profile with reserved name: %s", defaultProfileName) } - profPath := filepath.Join(configDir, profileName+".json") - profileExists, err := fileExists(profPath) - if err != nil { - return fmt.Errorf("failed to check if profile exists: %w", err) + if !isValidProfileFilenameStem(id) { + return fmt.Errorf("invalid profile ID: %q", id) } - if !profileExists { + + profiles, err := s.loadAllProfiles(username) + if err != nil { + return fmt.Errorf("load profiles: %w", err) + } + + var target *Profile + for i := range profiles { + if profiles[i].ID == id { + target = &profiles[i] + break + } + } + if target == nil { return ErrProfileNotFound } @@ -301,57 +365,28 @@ func (s *ServiceManager) RemoveProfile(profileName, username string) error { if err != nil && !errors.Is(err, ErrNoActiveProfile) { return fmt.Errorf("failed to get active profile: %w", err) } - - if activeProf != nil && activeProf.Name == profileName { - return fmt.Errorf("cannot remove active profile: %s", profileName) + if activeProf != nil && activeProf.ID == id { + return fmt.Errorf("cannot remove active profile: %s", id) } - err = util.RemoveJson(profPath) - if err != nil { + if err := util.RemoveJson(target.Path); err != nil { return fmt.Errorf("failed to remove profile config: %w", err) } + + // Best-effort state file cleanup. Missing is fine — state files are + // only created on demand. + stateFile := filepath.Join(filepath.Dir(target.Path), id+".state.json") + if err := os.Remove(stateFile); err != nil && !os.IsNotExist(err) { + log.Warnf("failed to remove profile state file %s: %v", stateFile, err) + } + return nil } +// ListProfiles returns every profile for the given user, including the +// default profile, with IsActive flags set. func (s *ServiceManager) ListProfiles(username string) ([]Profile, error) { - configDir, err := s.getConfigDir(username) - if err != nil { - return nil, fmt.Errorf("failed to get config directory: %w", err) - } - - files, err := util.ListFiles(configDir, "*.json") - if err != nil { - return nil, fmt.Errorf("failed to list profile files: %w", err) - } - - var filtered []string - for _, file := range files { - if strings.HasSuffix(file, "state.json") { - continue // skip state files - } - filtered = append(filtered, file) - } - sort.Strings(filtered) - - var activeProfName string - activeProf, err := s.GetActiveProfileState() - if err == nil { - activeProfName = activeProf.Name - } - - var profiles []Profile - // add default profile always - profiles = append(profiles, Profile{Name: defaultProfileName, IsActive: activeProfName == "" || activeProfName == defaultProfileName}) - for _, file := range filtered { - profileName := strings.TrimSuffix(filepath.Base(file), ".json") - var isActive bool - if activeProfName != "" && activeProfName == profileName { - isActive = true - } - profiles = append(profiles, Profile{Name: profileName, IsActive: isActive}) - } - - return profiles, nil + return s.loadAllProfiles(username) } // GetStatePath returns the path to the state file based on the operating system @@ -369,7 +404,12 @@ func (s *ServiceManager) GetStatePath() string { return defaultStatePath } - if activeProf.Name == defaultProfileName { + if activeProf.ID == defaultProfileName { + return defaultStatePath + } + + if !isValidProfileFilenameStem(activeProf.ID) { + log.Warnf("invalid active profile ID %q, using default state path", activeProf.ID) return defaultStatePath } @@ -379,7 +419,7 @@ func (s *ServiceManager) GetStatePath() string { return defaultStatePath } - return filepath.Join(configDir, activeProf.Name+".state.json") + return filepath.Join(configDir, activeProf.ID+".state.json") } // getConfigDir returns the profiles directory, using profilesDir if set, otherwise getConfigDirForUser @@ -390,3 +430,169 @@ func (s *ServiceManager) getConfigDir(username string) (string, error) { return getConfigDirForUser(username) } + +// loadAllProfiles returns every profile visible to the daemon for the +// given user, including the default profile. The returned slice is sorted +// by ID for a stable display order. +// +// Each Profile is fully populated: ID is the filename stem, Name comes +// from the JSON's "name" field (falling back to the ID when absent), and +// Path is built from a basename read off disk (path-traversal safe). +func (s *ServiceManager) loadAllProfiles(username string) ([]Profile, error) { + activeID, activeIsDefault := s.activeProfileID() + + profiles := []Profile{{ + ID: defaultProfileName, + Name: defaultProfileName, + Path: DefaultConfigPath, + IsActive: activeIsDefault || activeID == defaultProfileName, + }} + + configDir, err := s.getConfigDir(username) + if err != nil { + return nil, fmt.Errorf("get config directory: %w", err) + } + + entries, err := os.ReadDir(configDir) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return profiles, nil + } + return nil, fmt.Errorf("read profile directory: %w", err) + } + + var fileProfiles []Profile + for _, entry := range entries { + if entry.IsDir() { + continue + } + base := entry.Name() + if !strings.HasSuffix(base, ".json") { + continue + } + if strings.HasSuffix(base, ".state.json") { + continue + } + stem := strings.TrimSuffix(base, ".json") + if stem == defaultProfileName { + // default lives at the top-level config dir, not under / + continue + } + if !isValidProfileFilenameStem(stem) { + continue + } + path := filepath.Join(configDir, base) + name := readProfileName(path) + if name == "" { + name = stem + } + fileProfiles = append(fileProfiles, Profile{ + ID: stem, + Name: name, + Path: path, + IsActive: stem == activeID && !activeIsDefault, + }) + } + + sort.Slice(fileProfiles, func(i, j int) bool { + if fileProfiles[i].Name != fileProfiles[j].Name { + return fileProfiles[i].Name < fileProfiles[j].Name + } + // Stable tie-break on ID so duplicate names always render in + // the same order across calls. + return fileProfiles[i].ID < fileProfiles[j].ID + }) + profiles = append(profiles, fileProfiles...) + return profiles, nil +} + +// readProfileName parses just the "name" field from the profile Json. +func readProfileName(path string) string { + data, err := os.ReadFile(path) + if err != nil { + return "" + } + var meta profileMeta + if err := json.Unmarshal(data, &meta); err != nil { + return "" + } + return meta.Name +} + +// activeProfileID returns the currently-active profile's ID. The second +// return value is true when the active profile is the default one. +func (s *ServiceManager) activeProfileID() (string, bool) { + state, err := s.GetActiveProfileState() + if err != nil || state == nil { + return defaultProfileName, true + } + if state.ID == "" || state.ID == defaultProfileName { + return defaultProfileName, true + } + return state.ID, false +} + +// ResolveProfile turns a user-supplied handle into a Profile. Resolution +// precedence is: exact ID match, then unique ID prefix, then unique exact +// name. Ambiguous matches return *ErrAmbiguousHandle so callers can +// surface the candidates. +func (s *ServiceManager) ResolveProfile(handle, username string) (*Profile, error) { + if handle == "" { + return nil, fmt.Errorf("profile handle is empty") + } + + profiles, err := s.loadAllProfiles(username) + if err != nil { + return nil, err + } + + // Exact ID match wins outright — covers the default profile and any + // legacy profile whose ID is its name. + for i := range profiles { + if profiles[i].ID == handle { + return &profiles[i], nil + } + } + + // ID prefix match. Skip the default profile so a `select d` does not + // accidentally pick it via prefix. + var prefixMatches []Profile + for i := range profiles { + if profiles[i].ID == defaultProfileName { + continue + } + if strings.HasPrefix(profiles[i].ID, handle) { + prefixMatches = append(prefixMatches, profiles[i]) + } + } + if len(prefixMatches) == 1 { + return &prefixMatches[0], nil + } + if len(prefixMatches) > 1 { + return nil, &ErrAmbiguousHandle{ + Handle: handle, + Candidates: prefixMatches, + Kind: AmbiguityKindIDPrefix, + } + } + + // Exact name match. + var nameMatches []Profile + for i := range profiles { + if profiles[i].Name == handle { + nameMatches = append(nameMatches, profiles[i]) + } + } + if len(nameMatches) == 1 { + return &nameMatches[0], nil + } + if len(nameMatches) > 1 { + return nil, &ErrAmbiguousHandle{ + Handle: handle, + Candidates: nameMatches, + Kind: AmbiguityKindName, + } + } + + return nil, ErrProfileNotFound +} diff --git a/client/internal/profilemanager/service_test.go b/client/internal/profilemanager/service_test.go new file mode 100644 index 000000000..8c3f3e00e --- /dev/null +++ b/client/internal/profilemanager/service_test.go @@ -0,0 +1,240 @@ +package profilemanager + +import ( + "context" + "errors" + "os" + "os/user" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/util" +) + +// withTestSM wires up patched globals + a clean config dir and returns a +// fully initialized ServiceManager plus the username we are scoped to. +func withTestSM(t *testing.T, fn func(sm *ServiceManager, username string)) { + t.Helper() + withTempConfigDir(t, func(configDir string) { + withPatchedGlobals(t, configDir, func() { + u, err := user.Current() + require.NoError(t, err) + sm := &ServiceManager{} + require.NoError(t, sm.CreateDefaultProfile()) + fn(sm, u.Username) + }) + }) +} + +func TestServiceProfile_ExactID(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + created, err := sm.AddProfile("work", username) + require.NoError(t, err) + + got, err := sm.ResolveProfile(created.ID, username) + require.NoError(t, err) + assert.Equal(t, created.ID, got.ID) + assert.Equal(t, "work", got.Name) + }) +} + +func TestServiceProfile_IDPrefix(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + created, err := sm.AddProfile("work", username) + require.NoError(t, err) + + prefix := created.ID[:4] + got, err := sm.ResolveProfile(prefix, username) + require.NoError(t, err) + assert.Equal(t, created.ID, got.ID) + }) +} + +func TestServiceProfile_AmbiguousPrefix(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + // Plant two profiles whose IDs share a known prefix by writing + // the files directly, since generated IDs are random. + configDir, err := sm.getConfigDir(username) + require.NoError(t, err) + for _, id := range []string{"abcd1111aaaa", "abcd2222bbbb"} { + path := filepath.Join(configDir, id+".json") + require.NoError(t, util.WriteJson(context.Background(), path, &Config{Name: id})) + } + + _, err = sm.ResolveProfile("abcd", username) + var amb *ErrAmbiguousHandle + require.ErrorAs(t, err, &amb) + assert.Equal(t, AmbiguityKindIDPrefix, amb.Kind) + assert.Len(t, amb.Candidates, 2) + }) +} + +func TestServiceProfile_ExactNameUnique(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + _, err := sm.AddProfile("work", username) + require.NoError(t, err) + + got, err := sm.ResolveProfile("work", username) + require.NoError(t, err) + assert.Equal(t, "work", got.Name) + }) +} + +func TestServiceProfile_AmbiguousName(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + _, err := sm.AddProfile("work", username) + require.NoError(t, err) + _, err = sm.AddProfile("work", username) + require.NoError(t, err) + + _, err = sm.ResolveProfile("work", username) + var amb *ErrAmbiguousHandle + require.ErrorAs(t, err, &amb) + assert.Equal(t, AmbiguityKindName, amb.Kind) + assert.Len(t, amb.Candidates, 2) + }) +} + +func TestServiceProfile_NotFound(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + _, err := sm.ResolveProfile("nope", username) + assert.ErrorIs(t, err, ErrProfileNotFound) + }) +} + +func TestServiceProfile_DefaultByExactID(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + got, err := sm.ResolveProfile(defaultProfileName, username) + require.NoError(t, err) + assert.Equal(t, defaultProfileName, got.ID) + }) +} + +func TestServiceProfile_LegacyFilenameCoexists(t *testing.T) { + // Legacy profiles stored as .json with no "name" JSON field + // should still be discoverable by name and removable by name. + withTestSM(t, func(sm *ServiceManager, username string) { + configDir, err := sm.getConfigDir(username) + require.NoError(t, err) + path := filepath.Join(configDir, "legacy.json") + require.NoError(t, util.WriteJson(context.Background(), path, &Config{})) + + got, err := sm.ResolveProfile("legacy", username) + require.NoError(t, err) + assert.Equal(t, "legacy", got.ID) + // Name falls back to the filename stem when JSON omits it. + assert.Equal(t, "legacy", got.Name) + }) +} + +func TestAddProfile_RejectsDuplicateNameByDefault(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + _, err := sm.AddProfile("work", username) + require.NoError(t, err) + + _, err = sm.AddProfile("work", username) + assert.ErrorIs(t, err, ErrProfileAlreadyExists) + }) +} + +func TestAddProfile_AllowsDuplicateWithFlag(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + first, err := sm.AddProfile("work", username) + require.NoError(t, err) + + second, err := sm.AddProfile("work", username) + require.NoError(t, err) + assert.NotEqual(t, first.ID, second.ID) + assert.Equal(t, "work", second.Name) + }) +} + +func TestAddProfile_RejectsInvalidNames(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + cases := []string{ + "", // empty + "\x00\x01", // only control chars (becomes empty) + strings.Repeat("a", maxProfileNameLen+1), // too long + } + for _, name := range cases { + _, err := sm.AddProfile(name, username) + assert.Error(t, err, "expected error for %q", name) + } + }) +} + +func TestRemoveProfile_RejectsInvalidID(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + err := sm.RemoveProfile("../escape", username) + assert.Error(t, err) + }) +} + +func TestSanitizeDisplayName(t *testing.T) { + cases := []struct { + in string + want string + wantErr bool + }{ + {"work", "work", false}, + {"My Work Account", "My Work Account", false}, + {"emoji 🚀 ok", "emoji 🚀 ok", false}, + {"漢字テスト", "漢字テスト", false}, + {"with\x00null", "withnull", false}, + {"\x01\x02\x03", "", true}, + {"", "", true}, + } + for _, tc := range cases { + got, err := sanitizeDisplayName(tc.in) + if tc.wantErr { + assert.Error(t, err, "case %q", tc.in) + continue + } + assert.NoError(t, err, "case %q", tc.in) + assert.Equal(t, tc.want, got, "case %q", tc.in) + } +} + +func TestIsValidProfileFilenameStem(t *testing.T) { + cases := []struct { + in string + want bool + }{ + {"default", true}, + {"abc123def456", true}, + {"legacy-name", true}, + {"legacy_name", true}, + {"", false}, + {"..", false}, + {"../etc", false}, + {"foo/bar", false}, + {`foo\bar`, false}, + {"with space", false}, + {"with.dot", false}, + {strings.Repeat("a", maxProfileIDLen+1), false}, + } + for _, tc := range cases { + got := isValidProfileFilenameStem(tc.in) + assert.Equal(t, tc.want, got, "case %q", tc.in) + } +} + +func TestRemoveProfile_DeletesStateFile(t *testing.T) { + withTestSM(t, func(sm *ServiceManager, username string) { + created, err := sm.AddProfile("work", username) + require.NoError(t, err) + + configDir, err := sm.getConfigDir(username) + require.NoError(t, err) + statePath := filepath.Join(configDir, created.ID+".state.json") + require.NoError(t, os.WriteFile(statePath, []byte(`{"email":"a@b"}`), 0600)) + + require.NoError(t, sm.RemoveProfile(created.ID, username)) + _, err = os.Stat(statePath) + assert.True(t, errors.Is(err, os.ErrNotExist), "state file should be removed") + }) +} diff --git a/client/internal/profilemanager/state.go b/client/internal/profilemanager/state.go index f09391ede..5caa85bd4 100644 --- a/client/internal/profilemanager/state.go +++ b/client/internal/profilemanager/state.go @@ -13,13 +13,20 @@ type ProfileState struct { Email string `json:"email"` } -func (pm *ProfileManager) GetProfileState(profileName string) (*ProfileState, error) { +// GetProfileState reads the per-profile state file keyed by profile ID. +// The state file lives in the user's config directory. Legacy state files +// keyed by the old profile name remain readable. +func (pm *ProfileManager) GetProfileState(id string) (*ProfileState, error) { configDir, err := getConfigDir() if err != nil { return nil, fmt.Errorf("get config directory: %w", err) } - stateFile := filepath.Join(configDir, profileName+".state.json") + if id != defaultProfileName && !isValidProfileFilenameStem(id) { + return nil, fmt.Errorf("invalid profile ID: %q", id) + } + + stateFile := filepath.Join(configDir, id+".state.json") stateFileExists, err := fileExists(stateFile) if err != nil { return nil, fmt.Errorf("failed to check if profile state file exists: %w", err) @@ -51,7 +58,12 @@ func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error { return fmt.Errorf("get active profile: %w", err) } - stateFile := filepath.Join(configDir, activeProf.Name+".state.json") + id := activeProf.ID + if id != defaultProfileName && !isValidProfileFilenameStem(id) { + return fmt.Errorf("invalid active profile ID: %q", id) + } + + stateFile := filepath.Join(configDir, id+".state.json") err = util.WriteJsonWithRestrictedPermission(context.Background(), stateFile, state) if err != nil { return fmt.Errorf("write profile state: %w", err) diff --git a/client/proto/daemon.pb.go b/client/proto/daemon.pb.go index 79fa1418a..ebc23ff95 100644 --- a/client/proto/daemon.pb.go +++ b/client/proto/daemon.pb.go @@ -3931,9 +3931,11 @@ func (x *GetEventsResponse) GetEvents() []*SystemEvent { } type SwitchProfileRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - ProfileName *string `protobuf:"bytes,1,opt,name=profileName,proto3,oneof" json:"profileName,omitempty"` - Username *string `protobuf:"bytes,2,opt,name=username,proto3,oneof" json:"username,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + // profileName is treated as a handle: exact ID, unique ID prefix, or + // unique display name. The daemon resolves it server-side. + ProfileName *string `protobuf:"bytes,1,opt,name=profileName,proto3,oneof" json:"profileName,omitempty"` + Username *string `protobuf:"bytes,2,opt,name=username,proto3,oneof" json:"username,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -3983,7 +3985,11 @@ func (x *SwitchProfileRequest) GetUsername() string { } type SwitchProfileResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` + state protoimpl.MessageState `protogen:"open.v1"` + // id is the resolved on-disk ID of the profile that became active. + // Lets CLI clients update their local active-profile state without + // duplicating the resolution logic. + Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -4018,6 +4024,13 @@ func (*SwitchProfileResponse) Descriptor() ([]byte, []int) { return file_daemon_proto_rawDescGZIP(), []int{55} } +func (x *SwitchProfileResponse) GetId() string { + if x != nil { + return x.Id + } + return "" +} + type SetConfigRequest struct { state protoimpl.MessageState `protogen:"open.v1"` Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty"` @@ -4374,9 +4387,11 @@ func (*SetConfigResponse) Descriptor() ([]byte, []int) { } type AddProfileRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty"` - ProfileName string `protobuf:"bytes,2,opt,name=profileName,proto3" json:"profileName,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty"` + // profileName carries the human-readable display name for the new + // profile. The on-disk filename is a separately-generated ID. + ProfileName string `protobuf:"bytes,2,opt,name=profileName,proto3" json:"profileName,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -4426,7 +4441,10 @@ func (x *AddProfileRequest) GetProfileName() string { } type AddProfileResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` + state protoimpl.MessageState `protogen:"open.v1"` + // id is the generated on-disk ID of the new profile. CLI clients + // display a truncated form; UI clients can ignore it. + Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -4461,10 +4479,19 @@ func (*AddProfileResponse) Descriptor() ([]byte, []int) { return file_daemon_proto_rawDescGZIP(), []int{59} } +func (x *AddProfileResponse) GetId() string { + if x != nil { + return x.Id + } + return "" +} + type RemoveProfileRequest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty"` - ProfileName string `protobuf:"bytes,2,opt,name=profileName,proto3" json:"profileName,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty"` + // profileName is treated as a handle: an exact ID, a unique ID + // prefix, or a unique display name. Resolution happens server-side. + ProfileName string `protobuf:"bytes,2,opt,name=profileName,proto3" json:"profileName,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -4514,7 +4541,10 @@ func (x *RemoveProfileRequest) GetProfileName() string { } type RemoveProfileResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` + state protoimpl.MessageState `protogen:"open.v1"` + // id is the full resolved ID of the removed profile, so callers can + // confirm exactly which profile a name/prefix handle resolved to. + Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -4549,6 +4579,13 @@ func (*RemoveProfileResponse) Descriptor() ([]byte, []int) { return file_daemon_proto_rawDescGZIP(), []int{61} } +func (x *RemoveProfileResponse) GetId() string { + if x != nil { + return x.Id + } + return "" +} + type ListProfilesRequest struct { state protoimpl.MessageState `protogen:"open.v1"` Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty"` @@ -4638,9 +4675,12 @@ func (x *ListProfilesResponse) GetProfiles() []*Profile { } type Profile struct { - state protoimpl.MessageState `protogen:"open.v1"` - Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` - IsActive bool `protobuf:"varint,2,opt,name=is_active,json=isActive,proto3" json:"is_active,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` + IsActive bool `protobuf:"varint,2,opt,name=is_active,json=isActive,proto3" json:"is_active,omitempty"` + // id is the on-disk filename stem of the profile. Always set by the + // server; older clients that ignore it continue to work via name. + Id string `protobuf:"bytes,3,opt,name=id,proto3" json:"id,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -4689,6 +4729,13 @@ func (x *Profile) GetIsActive() bool { return false } +func (x *Profile) GetId() string { + if x != nil { + return x.Id + } + return "" +} + type GetActiveProfileRequest struct { state protoimpl.MessageState `protogen:"open.v1"` unknownFields protoimpl.UnknownFields @@ -4726,9 +4773,13 @@ func (*GetActiveProfileRequest) Descriptor() ([]byte, []int) { } type GetActiveProfileResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - ProfileName string `protobuf:"bytes,1,opt,name=profileName,proto3" json:"profileName,omitempty"` - Username string `protobuf:"bytes,2,opt,name=username,proto3" json:"username,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + // profileName carries the display name for backwards compatibility + // with UI clients. New callers should prefer id, which is unique. + ProfileName string `protobuf:"bytes,1,opt,name=profileName,proto3" json:"profileName,omitempty"` + Username string `protobuf:"bytes,2,opt,name=username,proto3" json:"username,omitempty"` + // id is the on-disk filename stem of the active profile. + Id string `protobuf:"bytes,3,opt,name=id,proto3" json:"id,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -4777,6 +4828,13 @@ func (x *GetActiveProfileResponse) GetUsername() string { return "" } +func (x *GetActiveProfileResponse) GetId() string { + if x != nil { + return x.Id + } + return "" +} + type LogoutRequest struct { state protoimpl.MessageState `protogen:"open.v1"` ProfileName *string `protobuf:"bytes,1,opt,name=profileName,proto3,oneof" json:"profileName,omitempty"` @@ -6598,8 +6656,9 @@ const file_daemon_proto_rawDesc = "" + "\vprofileName\x18\x01 \x01(\tH\x00R\vprofileName\x88\x01\x01\x12\x1f\n" + "\busername\x18\x02 \x01(\tH\x01R\busername\x88\x01\x01B\x0e\n" + "\f_profileNameB\v\n" + - "\t_username\"\x17\n" + - "\x15SwitchProfileResponse\"\x98\x11\n" + + "\t_username\"'\n" + + "\x15SwitchProfileResponse\x12\x0e\n" + + "\x02id\x18\x01 \x01(\tR\x02id\"\x98\x11\n" + "\x10SetConfigRequest\x12\x1a\n" + "\busername\x18\x01 \x01(\tR\busername\x12 \n" + "\vprofileName\x18\x02 \x01(\tR\vprofileName\x12$\n" + @@ -6668,23 +6727,27 @@ const file_daemon_proto_rawDesc = "" + "\x11SetConfigResponse\"Q\n" + "\x11AddProfileRequest\x12\x1a\n" + "\busername\x18\x01 \x01(\tR\busername\x12 \n" + - "\vprofileName\x18\x02 \x01(\tR\vprofileName\"\x14\n" + - "\x12AddProfileResponse\"T\n" + + "\vprofileName\x18\x02 \x01(\tR\vprofileName\"$\n" + + "\x12AddProfileResponse\x12\x0e\n" + + "\x02id\x18\x01 \x01(\tR\x02id\"T\n" + "\x14RemoveProfileRequest\x12\x1a\n" + "\busername\x18\x01 \x01(\tR\busername\x12 \n" + - "\vprofileName\x18\x02 \x01(\tR\vprofileName\"\x17\n" + - "\x15RemoveProfileResponse\"1\n" + + "\vprofileName\x18\x02 \x01(\tR\vprofileName\"'\n" + + "\x15RemoveProfileResponse\x12\x0e\n" + + "\x02id\x18\x01 \x01(\tR\x02id\"1\n" + "\x13ListProfilesRequest\x12\x1a\n" + "\busername\x18\x01 \x01(\tR\busername\"C\n" + "\x14ListProfilesResponse\x12+\n" + - "\bprofiles\x18\x01 \x03(\v2\x0f.daemon.ProfileR\bprofiles\":\n" + + "\bprofiles\x18\x01 \x03(\v2\x0f.daemon.ProfileR\bprofiles\"J\n" + "\aProfile\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04name\x12\x1b\n" + - "\tis_active\x18\x02 \x01(\bR\bisActive\"\x19\n" + - "\x17GetActiveProfileRequest\"X\n" + + "\tis_active\x18\x02 \x01(\bR\bisActive\x12\x0e\n" + + "\x02id\x18\x03 \x01(\tR\x02id\"\x19\n" + + "\x17GetActiveProfileRequest\"h\n" + "\x18GetActiveProfileResponse\x12 \n" + "\vprofileName\x18\x01 \x01(\tR\vprofileName\x12\x1a\n" + - "\busername\x18\x02 \x01(\tR\busername\"t\n" + + "\busername\x18\x02 \x01(\tR\busername\x12\x0e\n" + + "\x02id\x18\x03 \x01(\tR\x02id\"t\n" + "\rLogoutRequest\x12%\n" + "\vprofileName\x18\x01 \x01(\tH\x00R\vprofileName\x88\x01\x01\x12\x1f\n" + "\busername\x18\x02 \x01(\tH\x01R\busername\x88\x01\x01B\x0e\n" + diff --git a/client/proto/daemon.proto b/client/proto/daemon.proto index 6982e4a1c..24ec8c1c6 100644 --- a/client/proto/daemon.proto +++ b/client/proto/daemon.proto @@ -615,11 +615,18 @@ message GetEventsResponse { } message SwitchProfileRequest { + // profileName is treated as a handle: exact ID, unique ID prefix, or + // unique display name. The daemon resolves it server-side. optional string profileName = 1; optional string username = 2; } -message SwitchProfileResponse {} +message SwitchProfileResponse { + // id is the resolved on-disk ID of the profile that became active. + // Lets CLI clients update their local active-profile state without + // duplicating the resolution logic. + string id = 1; +} message SetConfigRequest { string username = 1; @@ -686,17 +693,29 @@ message SetConfigResponse{} message AddProfileRequest { string username = 1; + // profileName carries the human-readable display name for the new + // profile. The on-disk filename is a separately-generated ID. string profileName = 2; } -message AddProfileResponse {} +message AddProfileResponse { + // id is the generated on-disk ID of the new profile. CLI clients + // display a truncated form; UI clients can ignore it. + string id = 1; +} message RemoveProfileRequest { string username = 1; + // profileName is treated as a handle: an exact ID, a unique ID + // prefix, or a unique display name. Resolution happens server-side. string profileName = 2; } -message RemoveProfileResponse {} +message RemoveProfileResponse { + // id is the full resolved ID of the removed profile, so callers can + // confirm exactly which profile a name/prefix handle resolved to. + string id = 1; +} message ListProfilesRequest { string username = 1; @@ -709,13 +728,20 @@ message ListProfilesResponse { message Profile { string name = 1; bool is_active = 2; + // id is the on-disk filename stem of the profile. Always set by the + // server; older clients that ignore it continue to work via name. + string id = 3; } message GetActiveProfileRequest {} message GetActiveProfileResponse { + // profileName carries the display name for backwards compatibility + // with UI clients. New callers should prefer id, which is unique. string profileName = 1; string username = 2; + // id is the on-disk filename stem of the active profile. + string id = 3; } message LogoutRequest { diff --git a/client/server/login_overrides_test.go b/client/server/login_overrides_test.go index c45557c59..5a2298764 100644 --- a/client/server/login_overrides_test.go +++ b/client/server/login_overrides_test.go @@ -79,7 +79,7 @@ func TestPersistLoginOverrides(t *testing.T) { _, err := profilemanager.UpdateOrCreateConfig(seed) require.NoError(t, err, "seed config") - activeProf := &profilemanager.ActiveProfileState{Name: "default"} + activeProf := &profilemanager.ActiveProfileState{ID: "default"} err = persistLoginOverrides(activeProf, tt.newMgmtURL, tt.newPSK) require.NoError(t, err, "persistLoginOverrides") diff --git a/client/server/server.go b/client/server/server.go index 397fb37e4..e7a44a9d2 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -308,15 +308,14 @@ func (s *Server) SetConfig(callerCtx context.Context, msg *proto.SetConfigReques return nil, gstatus.Errorf(codes.Unavailable, errUpdateSettingsDisabled) } - profState := profilemanager.ActiveProfileState{ - Name: msg.ProfileName, - Username: msg.Username, - } - - profPath, err := profState.FilePath() + resolved, err := s.resolveProfileHandle(msg.ProfileName, msg.Username) if err != nil { - log.Errorf("failed to get active profile file path: %v", err) - return nil, fmt.Errorf("failed to get active profile file path: %w", err) + log.Errorf("failed to resolve profile %q: %v", msg.ProfileName, err) + return nil, err + } + profPath := resolved.Path + if profPath == "" { + profPath = profilemanager.DefaultConfigPath } var config profilemanager.ConfigInput @@ -446,30 +445,9 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro } if msg.ProfileName != nil { - if *msg.ProfileName != "default" && (msg.Username == nil || *msg.Username == "") { - log.Errorf("profile name is set to %s, but username is not provided", *msg.ProfileName) - return nil, fmt.Errorf("profile name is set to %s, but username is not provided", *msg.ProfileName) - } - - var username string - if *msg.ProfileName != "default" { - username = *msg.Username - } - - if *msg.ProfileName != activeProf.Name && username != activeProf.Username { - if s.checkProfilesDisabled() { - log.Errorf("profiles are disabled, you cannot use this feature without profiles enabled") - return nil, gstatus.Errorf(codes.Unavailable, errProfilesDisabled) - } - - log.Infof("switching to profile %s for user '%s'", *msg.ProfileName, username) - if err := s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{ - Name: *msg.ProfileName, - Username: username, - }); err != nil { - log.Errorf("failed to set active profile state: %v", err) - return nil, fmt.Errorf("failed to set active profile state: %w", err) - } + if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { + log.Errorf("failed to switch profile: %v", err) + return nil, err } } @@ -479,7 +457,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro return nil, fmt.Errorf("failed to get active profile state: %w", err) } - log.Infof("active profile: %s for %s", activeProf.Name, activeProf.Username) + log.Infof("active profile: %s for %s", activeProf.ID, activeProf.Username) s.mutex.Lock() @@ -711,10 +689,10 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR } if msg != nil && msg.ProfileName != nil { - if err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { + if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { s.mutex.Unlock() log.Errorf("failed to switch profile: %v", err) - return nil, fmt.Errorf("failed to switch profile: %w", err) + return nil, err } } @@ -725,7 +703,7 @@ func (s *Server) Up(callerCtx context.Context, msg *proto.UpRequest) (*proto.UpR return nil, fmt.Errorf("failed to get active profile state: %w", err) } - log.Infof("active profile: %s for %s", activeProf.Name, activeProf.Username) + log.Infof("active profile: %s for %s", activeProf.ID, activeProf.Username) config, _, err := s.getConfig(activeProf) if err != nil { @@ -768,34 +746,60 @@ func (s *Server) waitForUp(callerCtx context.Context) (*proto.UpResponse, error) } } -func (s *Server) switchProfileIfNeeded(profileName string, userName *string, activeProf *profilemanager.ActiveProfileState) error { - if profileName != "default" && (userName == nil || *userName == "") { - log.Errorf("profile name is set to %s, but username is not provided", profileName) - return fmt.Errorf("profile name is set to %s, but username is not provided", profileName) +// resolveProfileHandle resolves a wire-level profile handle (display +// name, ID, or unique ID prefix) to a concrete profile. Returns gRPC +// status errors so handlers can return them directly. +func (s *Server) resolveProfileHandle(handle, username string) (*profilemanager.Profile, error) { + p, err := s.profileManager.ResolveProfile(handle, username) + if err == nil { + return p, nil + } + var amb *profilemanager.ErrAmbiguousHandle + if errors.As(err, &amb) { + return nil, gstatus.Errorf(codes.InvalidArgument, "%v", amb) + } + if errors.Is(err, profilemanager.ErrProfileNotFound) { + return nil, gstatus.Errorf(codes.NotFound, "profile %q not found", handle) + } + return nil, fmt.Errorf("resolve profile: %w", err) +} + +// switchProfileIfNeeded resolves the user-supplied handle, updates the +// active profile state if it differs from the current one, and returns +// the resolved profile so callers can include its ID in RPC responses. +func (s *Server) switchProfileIfNeeded(handle string, userName *string, activeProf *profilemanager.ActiveProfileState) (*profilemanager.Profile, error) { + if handle != profilemanager.DefaultProfileName && (userName == nil || *userName == "") { + log.Errorf("profile name is set to %s, but username is not provided", handle) + return nil, fmt.Errorf("profile name is set to %s, but username is not provided", handle) } var username string - if profileName != "default" { + if handle != profilemanager.DefaultProfileName { username = *userName } - if profileName != activeProf.Name || username != activeProf.Username { + resolved, err := s.resolveProfileHandle(handle, username) + if err != nil { + return nil, err + } + + if resolved.ID != activeProf.ID || username != activeProf.Username { if s.checkProfilesDisabled() { log.Errorf("profiles are disabled, you cannot use this feature without profiles enabled") - return gstatus.Errorf(codes.Unavailable, errProfilesDisabled) + return nil, gstatus.Errorf(codes.Unavailable, errProfilesDisabled) } - log.Infof("switching to profile %s for user %s", profileName, username) + log.Infof("switching to profile %s (%s) for user %s", resolved.Name, resolved.ID, username) if err := s.profileManager.SetActiveProfileState(&profilemanager.ActiveProfileState{ - Name: profileName, + ID: resolved.ID, Username: username, }); err != nil { log.Errorf("failed to set active profile state: %v", err) - return fmt.Errorf("failed to set active profile state: %w", err) + return nil, fmt.Errorf("failed to set active profile state: %w", err) } } - return nil + return resolved, nil } // SwitchProfile switches the active profile in the daemon. @@ -810,9 +814,9 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi } if msg != nil && msg.ProfileName != nil { - if err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { + if _, err := s.switchProfileIfNeeded(*msg.ProfileName, msg.Username, activeProf); err != nil { log.Errorf("failed to switch profile: %v", err) - return nil, fmt.Errorf("failed to switch profile: %w", err) + return nil, err } } activeProf, err = s.profileManager.GetActiveProfileState() @@ -828,7 +832,7 @@ func (s *Server) SwitchProfile(callerCtx context.Context, msg *proto.SwitchProfi s.config = config - return &proto.SwitchProfileResponse{}, nil + return &proto.SwitchProfileResponse{Id: activeProf.ID}, nil } // Down engine work in the daemon. @@ -912,22 +916,27 @@ func (s *Server) Logout(ctx context.Context, msg *proto.LogoutRequest) (*proto.L } func (s *Server) handleProfileLogout(ctx context.Context, msg *proto.LogoutRequest) (*proto.LogoutResponse, error) { - if err := s.validateProfileOperation(*msg.ProfileName, true); err != nil { - return nil, err - } - if msg.Username == nil || *msg.Username == "" { return nil, gstatus.Errorf(codes.InvalidArgument, "username must be provided when profile name is specified") } username := *msg.Username - if err := s.logoutFromProfile(ctx, *msg.ProfileName, username); err != nil { - log.Errorf("failed to logout from profile %s: %v", *msg.ProfileName, err) + resolved, err := s.resolveProfileHandle(*msg.ProfileName, username) + if err != nil { + return nil, err + } + + if err := s.validateProfileOperation(resolved.ID, true); err != nil { + return nil, err + } + + if err := s.logoutFromProfile(ctx, resolved); err != nil { + log.Errorf("failed to logout from profile %s: %v", resolved.ID, err) return nil, gstatus.Errorf(codes.Internal, "logout: %v", err) } activeProf, _ := s.profileManager.GetActiveProfileState() - if activeProf != nil && activeProf.Name == *msg.ProfileName { + if activeProf != nil && activeProf.ID == resolved.ID { if err := s.cleanupConnection(); err != nil && !errors.Is(err, ErrServiceNotUp) { log.Errorf("failed to cleanup connection: %v", err) } @@ -989,30 +998,32 @@ func (s *Server) getConfig(activeProf *profilemanager.ActiveProfileState) (*prof return config, configExisted, nil } -func (s *Server) canRemoveProfile(profileName string) error { - if profileName == profilemanager.DefaultProfileName { +func (s *Server) canRemoveProfile(id string) error { + if id == profilemanager.DefaultProfileName { return fmt.Errorf("remove profile with reserved name: %s", profilemanager.DefaultProfileName) } activeProf, err := s.profileManager.GetActiveProfileState() - if err == nil && activeProf.Name == profileName { - return fmt.Errorf("remove active profile: %s", profileName) + if err == nil && activeProf.ID == id { + return fmt.Errorf("remove active profile: %s", id) } return nil } -func (s *Server) validateProfileOperation(profileName string, allowActiveProfile bool) error { +// validateProfileOperation runs the common preconditions for add/remove/ +// logout flows. `id` is the resolved profile ID (never user input). +func (s *Server) validateProfileOperation(id string, allowActiveProfile bool) error { if s.checkProfilesDisabled() { return gstatus.Errorf(codes.Unavailable, errProfilesDisabled) } - if profileName == "" { + if id == "" { return gstatus.Errorf(codes.InvalidArgument, "profile name must be provided") } if !allowActiveProfile { - if err := s.canRemoveProfile(profileName); err != nil { + if err := s.canRemoveProfile(id); err != nil { return gstatus.Errorf(codes.InvalidArgument, "%v", err) } } @@ -1020,25 +1031,17 @@ func (s *Server) validateProfileOperation(profileName string, allowActiveProfile return nil } -// logoutFromProfile logs out from a specific profile by loading its config and sending logout request -func (s *Server) logoutFromProfile(ctx context.Context, profileName, username string) error { +// logoutFromProfile logs out from a specific (already-resolved) profile +// by loading its config and sending a logout request. +func (s *Server) logoutFromProfile(ctx context.Context, profile *profilemanager.Profile) error { activeProf, err := s.profileManager.GetActiveProfileState() - if err == nil && activeProf.Name == profileName && s.connectClient != nil { + if err == nil && activeProf.ID == profile.ID && s.connectClient != nil { return s.sendLogoutRequest(ctx) } - profileState := &profilemanager.ActiveProfileState{ - Name: profileName, - Username: username, - } - profilePath, err := profileState.FilePath() + config, err := profilemanager.GetConfig(profile.Path) if err != nil { - return fmt.Errorf("get profile path: %w", err) - } - - config, err := profilemanager.GetConfig(profilePath) - if err != nil { - return fmt.Errorf("profile '%s' not found", profileName) + return fmt.Errorf("profile '%s' not found", profile.ID) } return s.sendLogoutRequestWithConfig(ctx, config) @@ -1452,15 +1455,14 @@ func (s *Server) GetConfig(ctx context.Context, req *proto.GetConfigRequest) (*p return nil, ctx.Err() } - prof := profilemanager.ActiveProfileState{ - Name: req.ProfileName, - Username: req.Username, - } - - cfgPath, err := prof.FilePath() + resolved, err := s.resolveProfileHandle(req.ProfileName, req.Username) if err != nil { - log.Errorf("failed to get active profile file path: %v", err) - return nil, fmt.Errorf("failed to get active profile file path: %w", err) + log.Errorf("failed to resolve profile %q: %v", req.ProfileName, err) + return nil, err + } + cfgPath := resolved.Path + if cfgPath == "" { + cfgPath = profilemanager.DefaultConfigPath } cfg, err := profilemanager.GetConfig(cfgPath) @@ -1564,12 +1566,16 @@ func (s *Server) AddProfile(ctx context.Context, msg *proto.AddProfileRequest) ( return nil, gstatus.Errorf(codes.InvalidArgument, "profile name and username must be provided") } - if err := s.profileManager.AddProfile(msg.ProfileName, msg.Username); err != nil { + created, err := s.profileManager.AddProfile(msg.ProfileName, msg.Username) + if err != nil { + if errors.Is(err, profilemanager.ErrProfileAlreadyExists) { + return nil, gstatus.Errorf(codes.AlreadyExists, "profile %q already exists", msg.ProfileName) + } log.Errorf("failed to create profile: %v", err) return nil, fmt.Errorf("failed to create profile: %w", err) } - return &proto.AddProfileResponse{}, nil + return &proto.AddProfileResponse{Id: created.ID}, nil } // RemoveProfile removes a profile from the daemon. @@ -1577,20 +1583,29 @@ func (s *Server) RemoveProfile(ctx context.Context, msg *proto.RemoveProfileRequ s.mutex.Lock() defer s.mutex.Unlock() - if err := s.validateProfileOperation(msg.ProfileName, false); err != nil { + if msg.ProfileName == "" { + return nil, gstatus.Errorf(codes.InvalidArgument, "profile name must be provided") + } + + resolved, err := s.resolveProfileHandle(msg.ProfileName, msg.Username) + if err != nil { return nil, err } - if err := s.logoutFromProfile(ctx, msg.ProfileName, msg.Username); err != nil { - log.Warnf("failed to logout from profile %s before removal: %v", msg.ProfileName, err) + if err := s.validateProfileOperation(resolved.ID, false); err != nil { + return nil, err } - if err := s.profileManager.RemoveProfile(msg.ProfileName, msg.Username); err != nil { + if err := s.logoutFromProfile(ctx, resolved); err != nil { + log.Warnf("failed to logout from profile %s before removal: %v", resolved.ID, err) + } + + if err := s.profileManager.RemoveProfile(resolved.ID, msg.Username); err != nil { log.Errorf("failed to remove profile: %v", err) return nil, fmt.Errorf("failed to remove profile: %w", err) } - return &proto.RemoveProfileResponse{}, nil + return &proto.RemoveProfileResponse{Id: resolved.ID}, nil } // ListProfiles lists all profiles in the daemon. @@ -1613,6 +1628,7 @@ func (s *Server) ListProfiles(ctx context.Context, msg *proto.ListProfilesReques } for i, profile := range profiles { response.Profiles[i] = &proto.Profile{ + Id: profile.ID, Name: profile.Name, IsActive: profile.IsActive, } @@ -1621,7 +1637,9 @@ func (s *Server) ListProfiles(ctx context.Context, msg *proto.ListProfilesReques return response, nil } -// GetActiveProfile returns the active profile in the daemon. +// GetActiveProfile returns the active profile in the daemon. The +// ProfileName field carries the display name for backwards compatibility +// with UI clients; new callers should prefer Id. func (s *Server) GetActiveProfile(ctx context.Context, msg *proto.GetActiveProfileRequest) (*proto.GetActiveProfileResponse, error) { s.mutex.Lock() defer s.mutex.Unlock() @@ -1632,9 +1650,22 @@ func (s *Server) GetActiveProfile(ctx context.Context, msg *proto.GetActiveProfi return nil, fmt.Errorf("failed to get active profile state: %w", err) } + displayName := activeProfile.ID + if activeProfile.ID != profilemanager.DefaultProfileName { + if profiles, lerr := s.profileManager.ListProfiles(activeProfile.Username); lerr == nil { + for _, p := range profiles { + if p.ID == activeProfile.ID { + displayName = p.Name + break + } + } + } + } + return &proto.GetActiveProfileResponse{ - ProfileName: activeProfile.Name, + ProfileName: displayName, Username: activeProfile.Username, + Id: activeProfile.ID, }, nil } diff --git a/client/server/server_test.go b/client/server/server_test.go index 66e0fcc4c..c2ee2b6a7 100644 --- a/client/server/server_test.go +++ b/client/server/server_test.go @@ -97,7 +97,7 @@ func TestConnectWithRetryRuns(t *testing.T) { pm := profilemanager.ServiceManager{} err = pm.SetActiveProfileState(&profilemanager.ActiveProfileState{ - Name: "test-profile", + ID: "test-profile", Username: currUser.Username, }) if err != nil { @@ -158,7 +158,7 @@ func TestServer_Up(t *testing.T) { pm := profilemanager.ServiceManager{} err = pm.SetActiveProfileState(&profilemanager.ActiveProfileState{ - Name: profName, + ID: profName, Username: currUser.Username, }) if err != nil { @@ -228,7 +228,7 @@ func TestServer_SubcribeEvents(t *testing.T) { pm := profilemanager.ServiceManager{} err = pm.SetActiveProfileState(&profilemanager.ActiveProfileState{ - Name: "default", + ID: "default", Username: currUser.Username, }) if err != nil { diff --git a/client/server/setconfig_test.go b/client/server/setconfig_test.go index 553d4ad71..6d441e133 100644 --- a/client/server/setconfig_test.go +++ b/client/server/setconfig_test.go @@ -47,7 +47,7 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) { pm := profilemanager.ServiceManager{} err = pm.SetActiveProfileState(&profilemanager.ActiveProfileState{ - Name: profName, + ID: profName, Username: currUser.Username, }) require.NoError(t, err) @@ -112,7 +112,7 @@ func TestSetConfig_AllFieldsSaved(t *testing.T) { require.NoError(t, err) profState := profilemanager.ActiveProfileState{ - Name: profName, + ID: profName, Username: currUser.Username, } cfgPath, err := profState.FilePath() diff --git a/client/ui/client_ui.go b/client/ui/client_ui.go index c4b644354..a07586c5d 100644 --- a/client/ui/client_ui.go +++ b/client/ui/client_ui.go @@ -493,7 +493,7 @@ func (s *serviceClient) getConnectionForm() *widget.Form { if err != nil { log.Errorf("get active profile: %v", err) } else { - activeProfName = activeProf.Name + activeProfName = activeProf.ID } return &widget.Form{ Items: []*widget.FormItem{ @@ -622,7 +622,7 @@ func (s *serviceClient) buildSetConfigRequest(iMngURL string, port, mtu int64) ( } req := &proto.SetConfigRequest{ - ProfileName: activeProf.Name, + ProfileName: activeProf.ID, Username: currUser.Username, } @@ -789,11 +789,11 @@ func (s *serviceClient) login(ctx context.Context, openURL bool) (*proto.LoginRe loginReq := &proto.LoginRequest{ IsUnixDesktopClient: runtime.GOOS == "linux" || runtime.GOOS == "freebsd", - ProfileName: &activeProf.Name, + ProfileName: &activeProf.ID, Username: &currUser.Username, } - profileState, err := s.profileManager.GetProfileState(activeProf.Name) + profileState, err := s.profileManager.GetProfileState(activeProf.ID) if err != nil { log.Debugf("failed to get profile state for login hint: %v", err) } else if profileState.Email != "" { @@ -1309,7 +1309,7 @@ func (s *serviceClient) getSrvConfig() { } srvCfg, err := conn.GetConfig(s.ctx, &proto.GetConfigRequest{ - ProfileName: activeProf.Name, + ProfileName: activeProf.ID, Username: currUser.Username, }) if err != nil { @@ -1533,7 +1533,7 @@ func (s *serviceClient) loadSettings() { } cfg, err := conn.GetConfig(s.ctx, &proto.GetConfigRequest{ - ProfileName: activeProf.Name, + ProfileName: activeProf.ID, Username: currUser.Username, }) if err != nil { @@ -1610,7 +1610,7 @@ func (s *serviceClient) updateConfig() error { } req := proto.SetConfigRequest{ - ProfileName: activeProf.Name, + ProfileName: activeProf.ID, Username: currUser.Username, DisableAutoConnect: &disableAutoStart, ServerSSHAllowed: &sshAllowed, diff --git a/client/ui/profile.go b/client/ui/profile.go index 7ee89e631..8a6b8bc9e 100644 --- a/client/ui/profile.go +++ b/client/ui/profile.go @@ -88,7 +88,7 @@ func (s *serviceClient) showProfilesUI() { return } // switch - err = s.switchProfile(profile.Name) + err = s.switchProfile(profile.ID) if err != nil { log.Errorf("failed to switch profile: %v", err) dialog.ShowError(errors.New("failed to select profile"), s.wProfiles) @@ -130,7 +130,7 @@ func (s *serviceClient) showProfilesUI() { logoutBtn.Show() logoutBtn.SetText("Deregister") logoutBtn.OnTapped = func() { - s.handleProfileLogout(profile.Name, refresh) + s.handleProfileLogout(profile, refresh) } // Remove profile @@ -144,7 +144,7 @@ func (s *serviceClient) showProfilesUI() { return } - err = s.removeProfile(profile.Name) + err = s.removeProfile(profile.ID) if err != nil { log.Errorf("failed to remove profile: %v", err) dialog.ShowError(fmt.Errorf("failed to remove profile"), s.wProfiles) @@ -250,7 +250,7 @@ func (s *serviceClient) addProfile(profileName string) error { return nil } -func (s *serviceClient) switchProfile(profileName string) error { +func (s *serviceClient) switchProfile(handle string) error { conn, err := s.getSrvClient(defaultFailTimeout) if err != nil { return fmt.Errorf(getClientFMT, err) @@ -261,15 +261,15 @@ func (s *serviceClient) switchProfile(profileName string) error { return fmt.Errorf("get current user: %w", err) } - if _, err := conn.SwitchProfile(s.ctx, &proto.SwitchProfileRequest{ - ProfileName: &profileName, + resp, err := conn.SwitchProfile(s.ctx, &proto.SwitchProfileRequest{ + ProfileName: &handle, Username: &currUser.Username, - }); err != nil { + }) + if err != nil { return fmt.Errorf("switch profile failed: %w", err) } - err = s.profileManager.SwitchProfile(profileName) - if err != nil { + if err := s.profileManager.SwitchProfile(resp.Id); err != nil { return fmt.Errorf("switch profile: %w", err) } @@ -299,6 +299,7 @@ func (s *serviceClient) removeProfile(profileName string) error { } type Profile struct { + ID string Name string IsActive bool } @@ -324,6 +325,7 @@ func (s *serviceClient) getProfiles() ([]Profile, error) { for _, profile := range profilesResp.Profiles { profiles = append(profiles, Profile{ + ID: profile.Id, Name: profile.Name, IsActive: profile.IsActive, }) @@ -332,10 +334,10 @@ func (s *serviceClient) getProfiles() ([]Profile, error) { return profiles, nil } -func (s *serviceClient) handleProfileLogout(profileName string, refreshCallback func()) { +func (s *serviceClient) handleProfileLogout(profile Profile, refreshCallback func()) { dialog.ShowConfirm( "Deregister", - fmt.Sprintf("Are you sure you want to deregister from '%s'?", profileName), + fmt.Sprintf("Are you sure you want to deregister from '%s'?", profile.Name), func(confirm bool) { if !confirm { return @@ -356,8 +358,10 @@ func (s *serviceClient) handleProfileLogout(profileName string, refreshCallback } username := currUser.Username + // ProfileName is treated as a handle; send the ID so the + // daemon resolves to exactly this profile. _, err = conn.Logout(s.ctx, &proto.LogoutRequest{ - ProfileName: &profileName, + ProfileName: &profile.ID, Username: &username, }) if err != nil { @@ -368,7 +372,7 @@ func (s *serviceClient) handleProfileLogout(profileName string, refreshCallback dialog.ShowInformation( "Deregistered", - fmt.Sprintf("Successfully deregistered from '%s'", profileName), + fmt.Sprintf("Successfully deregistered from '%s'", profile.Name), s.wProfiles, ) @@ -461,6 +465,7 @@ func (p *profileMenu) getProfiles() ([]Profile, error) { for _, profile := range profilesResp.Profiles { profiles = append(profiles, Profile{ + ID: profile.Id, Name: profile.Name, IsActive: profile.IsActive, }) @@ -501,7 +506,7 @@ func (p *profileMenu) refresh() { } if activeProf.ProfileName == "default" || activeProf.Username == currUser.Username { - activeProfState, err := p.profileManager.GetProfileState(activeProf.ProfileName) + activeProfState, err := p.profileManager.GetProfileState(activeProf.Id) if err != nil { log.Warnf("failed to get active profile state: %v", err) p.emailMenuItem.Hide() @@ -541,8 +546,8 @@ func (p *profileMenu) refresh() { return } - _, err = conn.SwitchProfile(ctx, &proto.SwitchProfileRequest{ - ProfileName: &profile.Name, + switchResp, err := conn.SwitchProfile(ctx, &proto.SwitchProfileRequest{ + ProfileName: &profile.ID, Username: &currUser.Username, }) if err != nil { @@ -552,7 +557,7 @@ func (p *profileMenu) refresh() { return } - err = p.profileManager.SwitchProfile(profile.Name) + err = p.profileManager.SwitchProfile(switchResp.Id) if err != nil { log.Errorf("failed to switch profile '%s': %v", profile.Name, err) return