From fee042656807d74c610db7c1ccc538feba2e0517 Mon Sep 17 00:00:00 2001 From: williamong-tel Date: Thu, 12 Mar 2026 12:20:53 -0700 Subject: [PATCH] Add scope aware profiles after tsh login (#64233) * Add scope aware profiles after tsh login Update tsh.go fix some minor issues Update client_store_test.go fix rebase Update client_store.go Add scope changing feature Update tsh.go Update tsh.go * Remove "none" descoping Update tsh.go clean up * nil the scope pin when "" is in the profile instead of scope pin "" --- api/profile/profile.go | 3 + api/profile/profile_test.go | 1 + lib/client/api.go | 2 + lib/client/client_store.go | 7 ++ lib/client/client_store_test.go | 44 +++++++++++ tool/tsh/common/scope_test.go | 128 ++++++++++++++++++++++++++++++++ tool/tsh/common/tsh.go | 54 +++++++++++--- 7 files changed, 229 insertions(+), 10 deletions(-) create mode 100644 tool/tsh/common/scope_test.go diff --git a/api/profile/profile.go b/api/profile/profile.go index c01ccad841f..43edeb69b0b 100644 --- a/api/profile/profile.go +++ b/api/profile/profile.go @@ -136,6 +136,9 @@ type Profile struct { // with WebProxyAddr, to determine if a webpage is safe to open. Currently used by Teleport // Connect in the proxy host allow list. SSOHost string `yaml:"sso_host,omitempty"` + + // Scope is the target scope that credentials are pinned to, if any. + Scope string `yaml:"scope,omitempty"` } // Copy returns a shallow copy of p, or nil if p is nil. diff --git a/api/profile/profile_test.go b/api/profile/profile_test.go index 7bab6b82213..f17cabc6a70 100644 --- a/api/profile/profile_test.go +++ b/api/profile/profile_test.go @@ -46,6 +46,7 @@ func TestProfileBasics(t *testing.T) { SiteName: "example.com", AuthConnector: "passwordless", MFAMode: "auto", + Scope: "/team-a", } // verify that profile name is proxy host component diff --git a/lib/client/api.go b/lib/client/api.go index 074d638bc1a..2aaa01be507 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -942,6 +942,7 @@ func (c *Config) LoadProfile(proxyAddr string) error { c.SAMLSingleLogoutEnabled = profile.SAMLSingleLogoutEnabled c.SSHDialTimeout = profile.SSHDialTimeout c.SSOHost = profile.SSOHost + c.Scope = profile.Scope c.AuthenticatorAttachment, err = parseMFAMode(profile.MFAMode) if err != nil { @@ -1010,6 +1011,7 @@ func (c *Config) Profile() *profile.Profile { SAMLSingleLogoutEnabled: c.SAMLSingleLogoutEnabled, SSHDialTimeout: c.SSHDialTimeout, SSOHost: c.SSOHost, + Scope: c.Scope, } } diff --git a/lib/client/client_store.go b/lib/client/client_store.go index 00fb18c45f8..c4e1f3d7788 100644 --- a/lib/client/client_store.go +++ b/lib/client/client_store.go @@ -30,6 +30,7 @@ import ( "golang.org/x/crypto/ssh" "github.com/gravitational/teleport" + scopesv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/scopes/v1" "github.com/gravitational/teleport/api/profile" "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" @@ -267,6 +268,11 @@ func (s *Store) ReadProfileStatus(proxyAddressOrProfile string) (*ProfileStatus, Username: profile.Username, } + var scopePin *scopesv1.Pin + if profile.Scope != "" { + scopePin = &scopesv1.Pin{Scope: profile.Scope} + } + // If we can't find a keyRing to match the profile, connect to the keyRing (hardware key), // or read the full profile status, return a partial status. // This is used for some superficial functions `tsh logout` and `tsh status`. @@ -284,6 +290,7 @@ func (s *Store) ReadProfileStatus(proxyAddressOrProfile string) (*ProfileStatus, ValidUntil: time.Now(), SAMLSingleLogoutEnabled: profile.SAMLSingleLogoutEnabled, SSOHost: profile.SSOHost, + ScopePin: scopePin, } keyRing, err := s.GetKeyRing(idx, WithAllCerts...) diff --git a/lib/client/client_store_test.go b/lib/client/client_store_test.go index 60799f3346f..dc48a4f5754 100644 --- a/lib/client/client_store_test.go +++ b/lib/client/client_store_test.go @@ -262,6 +262,7 @@ func TestClientStore(t *testing.T) { WebProxyAddr: net.JoinHostPort(idx.ProxyHost, "3080"), SiteName: idx.ClusterName, Username: idx.Username, + Scope: "/production", } err = clientStore.SaveProfile(profile, true) require.NoError(t, err) @@ -292,6 +293,7 @@ func TestClientStore(t *testing.T) { otherProfile := profile.Copy() otherProfile.WebProxyAddr = "other.example.com:3080" + otherProfile.Scope = "/staging" err = clientStore.SaveProfile(otherProfile, false) require.NoError(t, err) @@ -319,11 +321,53 @@ func TestClientStore(t *testing.T) { require.Equal(t, expectOtherStatus, currentStatus) require.Len(t, otherStatuses, 1) require.Equal(t, expectStatus, otherStatuses[0]) + require.Equal(t, currentStatus.ScopePin, expectOtherStatus.ScopePin) }) }) } } +func TestPartialProfileStatusScope(t *testing.T) { + t.Parallel() + + t.Run("nil ScopePin when profile has no scope", func(t *testing.T) { + t.Parallel() + testEachClientStore(t, func(t *testing.T, clientStore *Store) { + p := &profile.Profile{ + WebProxyAddr: "noscope.example.com:3080", + SiteName: "root", + Username: "alice", + } + err := clientStore.SaveProfile(p, true) + require.NoError(t, err) + + // No key ring saved — ReadProfileStatus should return partial status. + status, err := clientStore.ReadProfileStatus(p.Name()) + require.NoError(t, err) + require.Nil(t, status.ScopePin) + }) + }) + + t.Run("ScopePin set when profile has scope", func(t *testing.T) { + t.Parallel() + testEachClientStore(t, func(t *testing.T, clientStore *Store) { + p := &profile.Profile{ + WebProxyAddr: "scoped.example.com:3080", + SiteName: "root", + Username: "alice", + Scope: "/production", + } + err := clientStore.SaveProfile(p, true) + require.NoError(t, err) + + status, err := clientStore.ReadProfileStatus(p.Name()) + require.NoError(t, err) + require.NotNil(t, status.ScopePin) + require.Equal(t, "/production", status.ScopePin.Scope) + }) + }) +} + // TestProxySSHConfig tests proxy client SSH config function // that generates SSH client configuration for proxy tunnel connections func TestProxySSHConfig(t *testing.T) { diff --git a/tool/tsh/common/scope_test.go b/tool/tsh/common/scope_test.go new file mode 100644 index 00000000000..31c757398a8 --- /dev/null +++ b/tool/tsh/common/scope_test.go @@ -0,0 +1,128 @@ +/* + * Teleport + * Copyright (C) 2026 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package common + +import ( + "testing" + + "github.com/stretchr/testify/require" + + scopesv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/scopes/v1" + "github.com/gravitational/teleport/lib/client" +) + +func TestResolveDesiredScope(t *testing.T) { + tests := []struct { + name string + cf *CLIConf + profile *client.ProfileStatus + wantScope string + wantScopeChange bool + }{ + { + name: "no flag, no profile -> empty scope, no change", + cf: &CLIConf{}, + profile: nil, + wantScope: "", + wantScopeChange: false, + }, + { + name: "no flag, unscoped profile -> inherit empty, no change", + cf: &CLIConf{}, + profile: &client.ProfileStatus{}, + wantScope: "", + wantScopeChange: false, + }, + { + name: "no flag, scoped profile -> inherit scope, no change", + cf: &CLIConf{}, + profile: &client.ProfileStatus{ScopePin: &scopesv1.Pin{Scope: "/staging/west"}}, + wantScope: "/staging/west", + wantScopeChange: false, + }, + { + name: "explicit scope, no profile -> set scope, changed", + cf: &CLIConf{ + Scope: "/staging/east", + ScopeSetByUser: true, + }, + profile: nil, + wantScope: "/staging/east", + wantScopeChange: true, + }, + { + name: "explicit scope, unscoped profile -> set scope, changed", + cf: &CLIConf{ + Scope: "/staging/east", + ScopeSetByUser: true, + }, + profile: &client.ProfileStatus{}, + wantScope: "/staging/east", + wantScopeChange: true, + }, + { + name: "explicit scope, same scoped profile -> same scope, no change", + cf: &CLIConf{ + Scope: "/staging/east", + ScopeSetByUser: true, + }, + profile: &client.ProfileStatus{ScopePin: &scopesv1.Pin{Scope: "/staging/east"}}, + wantScope: "/staging/east", + wantScopeChange: false, + }, + { + name: "explicit scope, different scoped profile -> new scope, changed", + cf: &CLIConf{ + Scope: "/staging/east", + ScopeSetByUser: true, + }, + profile: &client.ProfileStatus{ScopePin: &scopesv1.Pin{Scope: "/staging/west"}}, + wantScope: "/staging/east", + wantScopeChange: true, + }, + { + name: "descope with empty string, scoped profile -> empty, changed", + cf: &CLIConf{ + Scope: "", + ScopeSetByUser: true, + }, + profile: &client.ProfileStatus{ScopePin: &scopesv1.Pin{Scope: "/staging/west"}}, + wantScope: "", + wantScopeChange: true, + }, + { + name: "descope with empty string, unscoped profile -> empty, no change", + cf: &CLIConf{ + Scope: "", + ScopeSetByUser: true, + }, + profile: &client.ProfileStatus{}, + wantScope: "", + wantScopeChange: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotScope, gotChanged := resolveScope(tt.cf, tt.profile) + require.Equal(t, tt.wantScope, gotScope) + require.Equal(t, tt.wantScopeChange, gotChanged) + }) + } +} diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index 93ea9fea3aa..448c24847d6 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -156,8 +156,10 @@ type ClientInitFunc func(cf *CLIConf) (*client.TeleportClient, error) // CLIConf stores command line arguments and flags: type CLIConf struct { - // Scope constrains the current operation to a specific target scope + // Scope constrains the current operation to a specific target scope. A scope of "" descopes the user. Scope string + // ScopeSetByUser specifies whether the flag was set by the user. + ScopeSetByUser bool // UserHost contains "[login]@hostname" argument to SSH command UserHost string // Commands to execute on a remote host @@ -1273,7 +1275,9 @@ func Run(ctx context.Context, args []string, opts ...CliOption) error { login.Flag("request-nowait", "Finish without waiting for request resolution.").BoolVar(&cf.NoWait) login.Flag("request-id", "Login with the roles requested in the given request.").StringVar(&cf.RequestID) login.Arg("cluster", clusterHelp).StringVar(&cf.SiteName) - login.Flag("scope", "Scope pins credentials to a given scope.").StringVar(&cf.Scope) + login.Flag("scope", `Scope pins credentials to a given scope. Use "" to explicitly remove scoping.`). + IsSetByUser(&cf.ScopeSetByUser). + StringVar(&cf.Scope) login.Flag("browser", browserHelp).StringVar(&cf.Browser) login.Flag("kube-cluster", "Name of the Kubernetes cluster to login to.").StringVar(&cf.KubernetesCluster) login.Flag("verbose", "Show extra status information.").Short('v').BoolVar(&cf.Verbose) @@ -2238,6 +2242,35 @@ func serializeVersion(format string, proxyVersion string, proxyPublicAddress str return string(out), trace.Wrap(err) } +// resolveScope determines the target scope based on the CLI flag state and the current +// profile. It returns the desired scope string and whether the scope differs from the profile's +// current scope. The 3 cases are: +// 1. --scope not provided at all -> inherit profile.ScopePin.Scope, scopeChanged = false +// 2. --scope="" -> explicitly descope, scopeChanged = (profile.ScopePin.Scope != "") +// 3. --scope=/foo -> switch to /foo, scopeChanged = (profile.ScopePin.Scope != "/foo") +func resolveScope(cf *CLIConf, profile *client.ProfileStatus) (string, bool) { + // --scope was explicitly set by the user + if cf.ScopeSetByUser { + // passing in "" descopes + targetScope := cf.Scope + + currentScope := "" + if profile != nil && profile.ScopePin != nil { + currentScope = profile.ScopePin.Scope + } + + return targetScope, targetScope != currentScope + } + + // --scope not provided, inherit from profile. + if profile != nil && profile.ScopePin != nil { + return profile.ScopePin.Scope, false + } + + return "", false + +} + // onLogin logs in with remote proxy and gets signed certificates func onLogin(cf *CLIConf, reExecArgs ...string) (err error) { showAlerts := true @@ -2275,6 +2308,10 @@ func onLogin(cf *CLIConf, reExecArgs ...string) (err error) { } } + // Resolve the desired scope based on CLI flags and current profile state. + targetScope, scopeChanged := resolveScope(cf, profile) + cf.Scope = targetScope + if cf.Scope != "" { // auto-request behavior is incompatible with scopes autoRequest = false @@ -2287,10 +2324,6 @@ func onLogin(cf *CLIConf, reExecArgs ...string) (err error) { if err := scopes.StrongValidate(cf.Scope); err != nil { return trace.Wrap(err) } - - // TODO(fspmarshall/scopes): this is a clunky way to handle the forced reauth on scope change, - // look into doing something smarter. - profile, profiles = nil, nil } // make the teleport client and retrieve the certificate from the proxy: @@ -2328,8 +2361,8 @@ func onLogin(cf *CLIConf, reExecArgs ...string) (err error) { } } - // client is already logged in and profile is not expired - if profile != nil && !profile.IsExpired(time.Now()) { + // client is already logged in and profile is not expired and scope hasn't changed + if profile != nil && !profile.IsExpired(time.Now()) && !scopeChanged { switch { // in case if nothing is specified, re-fetch kube clusters and print // current status @@ -4992,8 +5025,9 @@ func loadClientConfigFromCLIConf(cf *CLIConf, proxy string) (*client.Config, err c.RemoteForwardPorts = rPorts } - // TODO(fspmarshall/scopes): decide if we want some kind of persistence for the CLI arg. - c.Scope = cf.Scope + if cf.ScopeSetByUser || cf.Scope != "" { + c.Scope = cf.Scope + } if cf.SiteName != "" { c.SiteName = cf.SiteName