From f4978b4b9b77df0febef1b77596e68b33bcdb077 Mon Sep 17 00:00:00 2001 From: Rollie Ma Date: Wed, 22 Jul 2026 20:36:05 -0700 Subject: [PATCH] feature,client: add serviceclientprefs for desktop client service launch (#20501) Add serviceclientprefs, an optional feature that stores and loads the desktop clients' saved service launch preferences, one file per login profile. - Add GET|POST /localapi/v0/prefs/service-clients to load and save the current profile's service client prefs. - Add local client GetServiceClientPrefs and SetServiceClientPref that call the new local api endpoint. - Store the prefs with the ipn/store FileStore at TailscaleVarRoot()/profile-data//service-client-prefs/, so DeleteProfile cleans them up for free. Fall back to an in-memory store when there's no var root. - Register the feature and its local api route from build tagged files so the whole thing drops out under ts_omit_serviceclientprefs. - Add the serviceclient package holding Pref and Prefs (saved client, username, database name, and last used time), so the local api client and desktop apps can import the types without the feature machinery. Change-Id: I340a99c1b332d181fb1556fbf3e8003bb3b95a08 Updates: https://github.com/tailscale/tailscale/issues/20429 Signed-off-by: Rollie Ma --- client/local/serviceclientprefs.go | 37 ++++++ client/tailscale/apitype/apitype.go | 17 +++ cmd/derper/depaware.txt | 1 + cmd/k8s-operator/depaware.txt | 1 + cmd/tailscale/depaware.txt | 1 + cmd/tailscaled/depaware.txt | 2 + cmd/tailscaled/deps_test.go | 12 ++ cmd/tsidp/depaware.txt | 1 + .../feature_serviceclientprefs_disabled.go | 13 ++ .../feature_serviceclientprefs_enabled.go | 13 ++ .../condregister/maybe_serviceclientprefs.go | 8 ++ feature/featuretags/featuretags.go | 5 +- feature/serviceclientprefs/local_api.go | 68 ++++++++++ feature/serviceclientprefs/service_client.go | 98 +++++++++++++++ .../serviceclientprefs/service_client_test.go | 116 ++++++++++++++++++ .../serviceclient/serviceclient.go | 35 ++++++ .../serviceclientprefs/serviceclientprefs.go | 102 +++++++++++++++ .../serviceclientprefs_test.go | 64 ++++++++++ ipn/ipnlocal/local.go | 5 +- tsnet/depaware.txt | 1 + 20 files changed, 597 insertions(+), 3 deletions(-) create mode 100644 client/local/serviceclientprefs.go create mode 100644 feature/buildfeatures/feature_serviceclientprefs_disabled.go create mode 100644 feature/buildfeatures/feature_serviceclientprefs_enabled.go create mode 100644 feature/condregister/maybe_serviceclientprefs.go create mode 100644 feature/serviceclientprefs/local_api.go create mode 100644 feature/serviceclientprefs/service_client.go create mode 100644 feature/serviceclientprefs/service_client_test.go create mode 100644 feature/serviceclientprefs/serviceclient/serviceclient.go create mode 100644 feature/serviceclientprefs/serviceclientprefs.go create mode 100644 feature/serviceclientprefs/serviceclientprefs_test.go diff --git a/client/local/serviceclientprefs.go b/client/local/serviceclientprefs.go new file mode 100644 index 000000000..44dc40243 --- /dev/null +++ b/client/local/serviceclientprefs.go @@ -0,0 +1,37 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !ts_omit_serviceclientprefs + +package local + +import ( + "context" + "net/http" + + "tailscale.com/client/tailscale/apitype" + "tailscale.com/feature/serviceclientprefs/serviceclient" +) + +// GetServiceClientPrefs returns all of the current profile's [serviceclient.Prefs]. +// +// API maturity: this method is not considered a stable API and is subject to change between releases. +func (lc *Client) GetServiceClientPrefs(ctx context.Context) (serviceclient.Prefs, error) { + body, err := lc.get200(ctx, "/localapi/v0/prefs/service-clients") + if err != nil { + return nil, err + } + return decodeJSON[serviceclient.Prefs](body) +} + +// SetServiceClientPref merges the non-empty fields from an [apitype.ServiceClientPrefRequest] into the +// saved service client prefs for the current profile and returns the full updated set. +// +// API maturity: this method is not considered a stable API and is subject to change between releases. +func (lc *Client) SetServiceClientPref(ctx context.Context, req apitype.ServiceClientPrefRequest) (serviceclient.Prefs, error) { + body, err := lc.send(ctx, "POST", "/localapi/v0/prefs/service-clients", http.StatusOK, jsonBody(req)) + if err != nil { + return nil, err + } + return decodeJSON[serviceclient.Prefs](body) +} diff --git a/client/tailscale/apitype/apitype.go b/client/tailscale/apitype/apitype.go index eb4d6c13e..12293762b 100644 --- a/client/tailscale/apitype/apitype.go +++ b/client/tailscale/apitype/apitype.go @@ -104,3 +104,20 @@ type OptionalFeatures struct { // are not guaranteed to be present.) Features map[string]bool } + +// ServiceClientPrefRequest is the body POSTed to the LocalAPI endpoint /localapi/v0/prefs/service-clients. +// Empty values for Client, Username, and DatabaseName mean "don't change this value". +type ServiceClientPrefRequest struct { + // Key is the identifier for the service client pref. Required. Format is ":" + // where serviceName is a [tailcfg.ServiceName], e.g. "svc:my-db:5432". + Key string + + // Client is the name of the client that the user picked in the service launch. Optional. + Client string `json:",omitzero"` + + // Username is the username that the user entered in the service launch. Optional. + Username string `json:",omitzero"` + + // DatabaseName is the database name that the user entered in the service launch. Optional. + DatabaseName string `json:",omitzero"` +} diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index 7e85af037..865dbab42 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -91,6 +91,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa tailscale.com/envknob from tailscale.com/client/local+ tailscale.com/feature from tailscale.com/tsweb+ tailscale.com/feature/buildfeatures from tailscale.com/feature+ + tailscale.com/feature/serviceclientprefs/serviceclient from tailscale.com/client/local tailscale.com/health from tailscale.com/net/tlsdial+ tailscale.com/hostinfo from tailscale.com/net/netmon+ tailscale.com/ipn from tailscale.com/client/local diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index 652cce2f0..51222b6b0 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -741,6 +741,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper + tailscale.com/feature/serviceclientprefs/serviceclient from tailscale.com/client/local tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy tailscale.com/health from tailscale.com/control/controlclient+ tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 8abb5e082..f3273d63e 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -211,6 +211,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/feature/identityfederation from tailscale.com/feature/condregister/identityfederation tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper + tailscale.com/feature/serviceclientprefs/serviceclient from tailscale.com/client/local tailscale.com/feature/syspolicy from tailscale.com/cmd/tailscale/cli tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy tailscale.com/gokrazy/mkfs from tailscale.com/cmd/tailscale/cli diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 0f8ec3ada..072d1bd5f 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -322,6 +322,8 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/routecheck from tailscale.com/feature/condregister tailscale.com/feature/runtimemetrics from tailscale.com/feature/condregister L tailscale.com/feature/sdnotify from tailscale.com/feature/condregister + tailscale.com/feature/serviceclientprefs from tailscale.com/feature/condregister + tailscale.com/feature/serviceclientprefs/serviceclient from tailscale.com/client/local+ LD tailscale.com/feature/ssh from tailscale.com/cmd/tailscaled L tailscale.com/feature/syslog from tailscale.com/feature/condregister tailscale.com/feature/syspolicy from tailscale.com/feature/condregister diff --git a/cmd/tailscaled/deps_test.go b/cmd/tailscaled/deps_test.go index f18facd7b..a611644d1 100644 --- a/cmd/tailscaled/deps_test.go +++ b/cmd/tailscaled/deps_test.go @@ -13,6 +13,18 @@ "tailscale.com/tstest/deptest" ) +func TestOmitServiceClientPrefs(t *testing.T) { + const msg = "unexpected with ts_omit_serviceclientprefs" + deptest.DepChecker{ + GOOS: "linux", + GOARCH: "amd64", + Tags: "ts_omit_serviceclientprefs,ts_include_cli", + BadDeps: map[string]string{ + "tailscale.com/feature/serviceclientprefs": msg, + }, + }.Check(t) +} + func TestOmitSSH(t *testing.T) { const msg = "unexpected with ts_omit_ssh" deptest.DepChecker{ diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index c8c6683b6..0e107b4f0 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -158,6 +158,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper + tailscale.com/feature/serviceclientprefs/serviceclient from tailscale.com/client/local tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy tailscale.com/health from tailscale.com/control/controlclient+ tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal diff --git a/feature/buildfeatures/feature_serviceclientprefs_disabled.go b/feature/buildfeatures/feature_serviceclientprefs_disabled.go new file mode 100644 index 000000000..480b5726c --- /dev/null +++ b/feature/buildfeatures/feature_serviceclientprefs_disabled.go @@ -0,0 +1,13 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Code generated by gen.go; DO NOT EDIT. + +//go:build ts_omit_serviceclientprefs + +package buildfeatures + +// HasServiceClientPrefs is whether the binary was built with support for modular feature "Desktop client service launch preferences". +// Specifically, it's whether the binary was NOT built with the "ts_omit_serviceclientprefs" build tag. +// It's a const so it can be used for dead code elimination. +const HasServiceClientPrefs = false diff --git a/feature/buildfeatures/feature_serviceclientprefs_enabled.go b/feature/buildfeatures/feature_serviceclientprefs_enabled.go new file mode 100644 index 000000000..eeda83c81 --- /dev/null +++ b/feature/buildfeatures/feature_serviceclientprefs_enabled.go @@ -0,0 +1,13 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Code generated by gen.go; DO NOT EDIT. + +//go:build !ts_omit_serviceclientprefs + +package buildfeatures + +// HasServiceClientPrefs is whether the binary was built with support for modular feature "Desktop client service launch preferences". +// Specifically, it's whether the binary was NOT built with the "ts_omit_serviceclientprefs" build tag. +// It's a const so it can be used for dead code elimination. +const HasServiceClientPrefs = true diff --git a/feature/condregister/maybe_serviceclientprefs.go b/feature/condregister/maybe_serviceclientprefs.go new file mode 100644 index 000000000..461276bf1 --- /dev/null +++ b/feature/condregister/maybe_serviceclientprefs.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !ts_omit_serviceclientprefs + +package condregister + +import _ "tailscale.com/feature/serviceclientprefs" diff --git a/feature/featuretags/featuretags.go b/feature/featuretags/featuretags.go index d567baeb9..1348cc15d 100644 --- a/feature/featuretags/featuretags.go +++ b/feature/featuretags/featuretags.go @@ -229,8 +229,9 @@ type FeatureMeta struct { Desc: "Linux NetworkManager integration", Deps: []FeatureTag{"dbus"}, }, - "qrcodes": {Sym: "QRCodes", Desc: "QR codes in tailscale CLI"}, - "relayserver": {Sym: "RelayServer", Desc: "Relay server"}, + "serviceclientprefs": {Sym: "ServiceClientPrefs", Desc: "Desktop client service launch preferences"}, + "qrcodes": {Sym: "QRCodes", Desc: "QR codes in tailscale CLI"}, + "relayserver": {Sym: "RelayServer", Desc: "Relay server"}, "remoteconfig": { Sym: "RemoteConfig", Desc: "Full remote configuration of this node by the tailnet admin, opting out of Tailscale's per-feature double opt-in in favor of a single client-side trust decision", diff --git a/feature/serviceclientprefs/local_api.go b/feature/serviceclientprefs/local_api.go new file mode 100644 index 000000000..b99de295e --- /dev/null +++ b/feature/serviceclientprefs/local_api.go @@ -0,0 +1,68 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package serviceclientprefs + +import ( + "encoding/json" + "errors" + "net/http" + + "tailscale.com/client/tailscale/apitype" + "tailscale.com/feature/serviceclientprefs/serviceclient" + "tailscale.com/ipn/ipnlocal" + "tailscale.com/ipn/localapi" + "tailscale.com/util/httpm" +) + +func init() { + localapi.Register("prefs/service-clients", serveServiceClientPrefs) +} + +// serveServiceClientPrefs handles GET and POST /localapi/v0/prefs/service-clients. GET returns all +// of the current profile's service client prefs. POST merges one [apitype.ServiceClientPrefRequest] +// into the saved service client prefs and returns the full updated set. +func serveServiceClientPrefs(h *localapi.Handler, w http.ResponseWriter, r *http.Request) { + if !h.PermitRead { + http.Error(w, "service-clients access denied", http.StatusForbidden) + return + } + ext, ok := ipnlocal.GetExt[*extension](h.LocalBackend()) + if !ok { + http.Error(w, "misconfigured serviceclientprefs extension", http.StatusInternalServerError) + return + } + var out serviceclient.Prefs + switch r.Method { + case httpm.GET: + var err error + if out, err = ext.serviceClientPrefs(); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + case httpm.POST: + if !h.PermitWrite { + http.Error(w, "service-clients write access denied", http.StatusForbidden) + return + } + var req apitype.ServiceClientPrefRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + var err error + if out, err = ext.setServiceClientPref(req); err != nil { + status := http.StatusInternalServerError + if errors.Is(err, errInvalidServiceClientPref) { + status = http.StatusBadRequest + } + http.Error(w, err.Error(), status) + return + } + default: + http.Error(w, "use GET or POST", http.StatusMethodNotAllowed) + return + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(out) +} diff --git a/feature/serviceclientprefs/service_client.go b/feature/serviceclientprefs/service_client.go new file mode 100644 index 000000000..6d1b09fdb --- /dev/null +++ b/feature/serviceclientprefs/service_client.go @@ -0,0 +1,98 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package serviceclientprefs + +import ( + "encoding/json" + "errors" + + "tailscale.com/client/tailscale/apitype" + "tailscale.com/feature/serviceclientprefs/serviceclient" + "tailscale.com/ipn" +) + +// errInvalidServiceClientPref is returned by [extension.setServiceClientPref] for a request that +// fails validation, such as a missing key. The handler maps it to a 400, unlike backend failures. +var errInvalidServiceClientPref = errors.New("service client pref key is required") + +// serviceClientPrefs returns the saved service client prefs for the current profile. It returns an +// empty, non-nil map when nothing has been saved yet, or when there's no current profile (a logged +// out node simply has no service client prefs). +func (e *extension) serviceClientPrefs() (serviceclient.Prefs, error) { + e.mu.Lock() + defer e.mu.Unlock() + return e.loadLocked() +} + +// setServiceClientPref merges the non-empty fields from [apitype.ServiceClientPrefRequest] into the +// saved pref for its key, stamps [serviceclient.Pref.LastUsed] with the current time, and returns +// the full updated service client prefs for the current profile. +func (e *extension) setServiceClientPref(req apitype.ServiceClientPrefRequest) (serviceclient.Prefs, error) { + if req.Key == "" { + return nil, errInvalidServiceClientPref + } + + // Hold mu across the whole read, change, and write so two writers can't clobber each other, + // since each writes back the full prefs map. + e.mu.Lock() + defer e.mu.Unlock() + + prefs, err := e.loadLocked() + if err != nil { + return nil, err + } + pref := prefs[req.Key] + if req.Client != "" { + pref.Client = req.Client + } + if req.Username != "" { + pref.Username = req.Username + } + if req.DatabaseName != "" { + pref.DatabaseName = req.DatabaseName + } + pref.LastUsed = e.sb.Clock().Now() + prefs[req.Key] = pref + + if err := e.saveLocked(prefs); err != nil { + return nil, err + } + return prefs, nil +} + +// loadLocked reads the current profile's prefs, returning an empty map when there's no store (no +// current profile) or nothing saved yet. The caller must hold e.mu. +func (e *extension) loadLocked() (serviceclient.Prefs, error) { + if e.store == nil { + return serviceclient.Prefs{}, nil + } + data, err := e.store.ReadState(storeKey) + if errors.Is(err, ipn.ErrStateNotExist) { + return serviceclient.Prefs{}, nil + } + if err != nil { + return nil, err + } + var prefs serviceclient.Prefs + if err := json.Unmarshal(data, &prefs); err != nil { + return nil, err + } + if prefs == nil { + prefs = serviceclient.Prefs{} + } + return prefs, nil +} + +// saveLocked writes prefs to the current profile's store. It returns an error when there's no +// store to persist to (no current profile, or the store failed to build). The caller must hold e.mu. +func (e *extension) saveLocked(prefs serviceclient.Prefs) error { + if e.store == nil { + return errors.New("no store available to persist service client prefs") + } + data, err := json.Marshal(prefs) + if err != nil { + return err + } + return e.store.WriteState(storeKey, data) +} diff --git a/feature/serviceclientprefs/service_client_test.go b/feature/serviceclientprefs/service_client_test.go new file mode 100644 index 000000000..8fdcd82f7 --- /dev/null +++ b/feature/serviceclientprefs/service_client_test.go @@ -0,0 +1,116 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package serviceclientprefs + +import ( + "errors" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "tailscale.com/client/tailscale/apitype" + "tailscale.com/feature/serviceclientprefs/serviceclient" + "tailscale.com/ipn" +) + +func TestServiceClientPrefsSetAndGet(t *testing.T) { + tests := []struct { + name string + requests []apitype.ServiceClientPrefRequest + want serviceclient.Prefs + }{ + { + name: "records_a_single_launch", + requests: []apitype.ServiceClientPrefRequest{{Key: "ssh:22", Client: "terminal", Username: "rollie"}}, + want: serviceclient.Prefs{"ssh:22": {Client: "terminal", Username: "rollie"}}, + }, + { + name: "partial_update_preserves_existing_fields", + requests: []apitype.ServiceClientPrefRequest{ + {Key: "ssh:22", Client: "terminal", Username: "rollie"}, + {Key: "ssh:22", Client: "iterm2"}, + }, + want: serviceclient.Prefs{"ssh:22": {Client: "iterm2", Username: "rollie"}}, + }, + { + name: "independent_services_kept_separate", + requests: []apitype.ServiceClientPrefRequest{ + {Key: "ssh:22", Client: "terminal"}, + {Key: "db:5432", Client: "psql", DatabaseName: "prod"}, + }, + want: serviceclient.Prefs{ + "ssh:22": {Client: "terminal"}, + "db:5432": {Client: "psql", DatabaseName: "prod"}, + }, + }, + } + + ignoreLastUsed := cmpopts.IgnoreFields(serviceclient.Pref{}, "LastUsed") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := newTestExtension(t, "pid") + for _, req := range tt.requests { + if _, err := e.setServiceClientPref(req); err != nil { + t.Fatal(err) + } + } + got, err := e.serviceClientPrefs() + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(tt.want, got, ignoreLastUsed, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + } + for key := range tt.want { + if got[key].LastUsed.IsZero() { + t.Errorf("%s: LastUsed was not stamped", key) + } + } + }) + } +} + +func TestServiceClientPrefsEmptyKeyRejected(t *testing.T) { + e := newTestExtension(t, "pid") + _, err := e.setServiceClientPref(apitype.ServiceClientPrefRequest{Client: "terminal"}) + if !errors.Is(err, errInvalidServiceClientPref) { + t.Errorf("want errInvalidServiceClientPref for empty key, got %v", err) + } +} + +func TestServiceClientPrefsNoCurrentProfileReturnsEmpty(t *testing.T) { + e := newTestExtension(t, "") // no current profile + got, err := e.serviceClientPrefs() + if err != nil { + t.Fatalf("want nil error for no current profile, got %v", err) + } + if got == nil || len(got) != 0 { + t.Errorf("want empty non-nil map, got %#v", got) + } +} + +func TestServiceClientPrefsRepopulatedOnProfileSwitch(t *testing.T) { + e := newTestExtension(t, "pid0") + + // Save a pref for pid0. + if _, err := e.setServiceClientPref(apitype.ServiceClientPrefRequest{Key: "ssh:22", Client: "terminal"}); err != nil { + t.Fatal(err) + } + + // Switch to a different profile; it must not see pid0's data. + e.onChangeProfile((&ipn.LoginProfile{ID: "pid1"}).View(), ipn.PrefsView{}, false) + if got, _ := e.serviceClientPrefs(); len(got) != 0 { + t.Fatalf("pid1 unexpectedly saw pid0's prefs: %v", got) + } + + // Switch back to pid0; its saved pref should be read back from disk into the cache. + e.onChangeProfile((&ipn.LoginProfile{ID: "pid0"}).View(), ipn.PrefsView{}, false) + got, err := e.serviceClientPrefs() + if err != nil { + t.Fatal(err) + } + if got["ssh:22"].Client != "terminal" { + t.Errorf("after switching back, got %v, want Client=terminal", got["ssh:22"]) + } +} diff --git a/feature/serviceclientprefs/serviceclient/serviceclient.go b/feature/serviceclientprefs/serviceclient/serviceclient.go new file mode 100644 index 000000000..ed9a90413 --- /dev/null +++ b/feature/serviceclientprefs/serviceclient/serviceclient.go @@ -0,0 +1,35 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Package serviceclient holds the types for the desktop clients' saved service launch preferences. +// They're shared by the serviceclientprefs feature and the LocalAPI client. +package serviceclient + +import ( + "time" +) + +// Prefs maps a service key to the user's saved preferences for that service. +// Keys are ":" where serviceName is a [tailcfg.ServiceName] +// (e.g. key "svc:my-db:5432" for service "svc:my-db" on port 5432). +type Prefs map[string]Pref + +// Pref captures the saved preferences for one service. +type Pref struct { + // Client is the saved client identifier the user picked in the last service launch, + // for example, terminal/putty/iterm2/... for SSH, and dbeaver/psql/mycli/... for + // database. Empty means no client has been saved for this service. + Client string `json:",omitzero"` + + // Username is the saved login name for SSH and future services that require username. + // Empty means none saved. + Username string `json:",omitzero"` + + // DatabaseName is the saved DB name (database service types). Empty means none saved. + DatabaseName string `json:",omitzero"` + + // LastUsed is when this service was last launched. Zero means never. + // When the macOS and Windows apps generate a "recently used" list, they sort by + // LastUsed descending. + LastUsed time.Time `json:",omitzero"` +} diff --git a/feature/serviceclientprefs/serviceclientprefs.go b/feature/serviceclientprefs/serviceclientprefs.go new file mode 100644 index 000000000..88a2d2cfe --- /dev/null +++ b/feature/serviceclientprefs/serviceclientprefs.go @@ -0,0 +1,102 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Package serviceclientprefs stores the desktop clients' saved service launch preferences, in a +// file per login profile. +package serviceclientprefs + +import ( + "encoding/hex" + "path/filepath" + "sync" + + "tailscale.com/feature" + "tailscale.com/ipn" + "tailscale.com/ipn/ipnext" + "tailscale.com/ipn/ipnlocal" + "tailscale.com/ipn/store" + "tailscale.com/ipn/store/mem" + "tailscale.com/types/logger" +) + +const featureName = "serviceclientprefs" + +// storeKey is the key under which a profile's service client prefs are stored. +const storeKey = "service-client-prefs" + +func init() { + feature.Register(featureName) + ipnext.RegisterExtension(featureName, newExtension) +} + +// newExtension is the [ipnext.NewExtensionFn] for this feature. +func newExtension(logf logger.Logf, sb ipnext.SafeBackend) (ipnext.Extension, error) { + return &extension{ + logf: logger.WithPrefix(logf, featureName+": "), + sb: sb, + }, nil +} + +// extension stores the service client prefs for the current login profile. +type extension struct { + logf logger.Logf + sb ipnext.SafeBackend + host ipnext.Host // from Init + + // mu guards store and curPID, and the read/change/write in setServiceClientPref. + mu sync.Mutex + store ipn.StateStore // for the current profile + curPID ipn.ProfileID // the profile store is built for +} + +// Name implements [ipnext.Extension]. +func (e *extension) Name() string { return featureName } + +// Init implements [ipnext.Extension]. It subscribes to profile changes and sets up storage for the +// current profile. +func (e *extension) Init(h ipnext.Host) error { + e.host = h + h.Hooks().ProfileStateChange.Add(e.onChangeProfile) + profile, prefs := h.Profiles().CurrentProfileState() + e.onChangeProfile(profile, prefs, false) + return nil +} + +// Shutdown implements [ipnext.Extension]. +func (e *extension) Shutdown() error { return nil } + +// onChangeProfile builds the store for the profile we switched to. The prefs live under +// profile-data//, which [ipnlocal.LocalBackend] removes when the profile is deleted. +func (e *extension) onChangeProfile(profile ipn.LoginProfileView, _ ipn.PrefsView, sameNode bool) { + e.mu.Lock() + defer e.mu.Unlock() + + pid := profile.ID() + if sameNode && e.curPID == pid && e.store != nil { + // Same profile, just a prefs or metadata update; keep the store we already built. + return + } + e.curPID = pid + e.store = nil + if pid == "" { + return + } + varRoot := e.sb.TailscaleVarRoot() + if varRoot == "" { + // No writable storage (ephemeral node, or a non-file [ipn.StateStore] like Kubernetes). + // Service client prefs are only used by the desktop clients, so an in-memory store that + // doesn't survive restarts is fine here. + e.store = new(mem.Store) + return + } + // Store the file under a dir plus a hex-encoded filename with no extension, matching + // netmapcache.Store's layout so a shared store could read these files if we consolidate later. + hexStoreKey := hex.EncodeToString([]byte(storeKey)) + path := filepath.Join(varRoot, ipnlocal.ProfileDataDir, string(pid), storeKey, hexStoreKey) + st, err := store.NewFileStore(e.logf, path) + if err != nil { + e.logf("building store for profile %q: %v", pid, err) + return + } + e.store = st +} diff --git a/feature/serviceclientprefs/serviceclientprefs_test.go b/feature/serviceclientprefs/serviceclientprefs_test.go new file mode 100644 index 000000000..ab3fea715 --- /dev/null +++ b/feature/serviceclientprefs/serviceclientprefs_test.go @@ -0,0 +1,64 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package serviceclientprefs + +import ( + "testing" + "time" + + "tailscale.com/client/tailscale/apitype" + "tailscale.com/ipn" + "tailscale.com/tsd" + "tailscale.com/tstest" + "tailscale.com/tstime" + "tailscale.com/types/logger" +) + +// fakeBackend is a minimal [ipnext.SafeBackend] for tests. Only Clock and TailscaleVarRoot are +// used by the extension; Sys is never called. +type fakeBackend struct { + clock tstime.Clock + varRoot string +} + +func (f *fakeBackend) Sys() *tsd.System { return nil } +func (f *fakeBackend) Clock() tstime.Clock { return f.clock } +func (f *fakeBackend) TailscaleVarRoot() string { return f.varRoot } + +// newTestExtension returns an extension backed by a temp var root and a fixed test clock, with its +// current profile set to pid. +func newTestExtension(t *testing.T, pid ipn.ProfileID) *extension { + t.Helper() + e := &extension{ + logf: logger.Discard, + sb: &fakeBackend{ + clock: tstest.NewClock(tstest.ClockOpts{Start: time.Now()}), + varRoot: t.TempDir(), + }, + } + e.onChangeProfile((&ipn.LoginProfile{ID: pid}).View(), ipn.PrefsView{}, false) + return e +} + +func TestNoVarRootUsesMemory(t *testing.T) { + e := &extension{ + logf: logger.Discard, + sb: &fakeBackend{ + clock: tstest.NewClock(tstest.ClockOpts{Start: time.Now()}), + varRoot: "", // force the in-memory fallback + }, + } + e.onChangeProfile((&ipn.LoginProfile{ID: "pid"}).View(), ipn.PrefsView{}, false) + + if _, err := e.setServiceClientPref(apitype.ServiceClientPrefRequest{Key: "ssh:22", Client: "terminal"}); err != nil { + t.Fatal(err) + } + got, err := e.serviceClientPrefs() + if err != nil { + t.Fatal(err) + } + if got["ssh:22"].Client != "terminal" { + t.Errorf("in-memory store: got %v, want Client=terminal", got["ssh:22"]) + } +} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 75e622343..5170b9ee7 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -6296,6 +6296,9 @@ func (b *LocalBackend) ProfileMkdirAll(id ipn.ProfileID, subs ...string) (string return b.profileMkdirAllLocked(id, subs...) } +// ProfileDataDir is the directory name, under the Tailscale var root, that holds per-profile data. +const ProfileDataDir = "profile-data" + // profileDataPathLocked returns a path of a profile-specific (sub)directory // inside the writable storage area for the given profile ID. It does not // create or verify the existence of the path in the filesystem. @@ -6310,7 +6313,7 @@ func (b *LocalBackend) profileDataPathLocked(id ipn.ProfileID, subs ...string) s if vr == "" { return "" } - return filepath.Join(append([]string{vr, "profile-data", string(id)}, subs...)...) + return filepath.Join(append([]string{vr, ProfileDataDir, string(id)}, subs...)...) } // profileMkdirAllLocked implements ProfileMkdirAll. diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index 0390cb577..4f80f04d0 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -154,6 +154,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper + tailscale.com/feature/serviceclientprefs/serviceclient from tailscale.com/client/local tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy tailscale.com/health from tailscale.com/control/controlclient+ tailscale.com/health/healthmsg from tailscale.com/ipn/ipnlocal