Conversation
* 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 ""
williamong-tel
requested review from
eriktate,
fspmarshall,
nklaassen,
rosstimothy and
strideynet
and removed request for
rosstimothy
github-actions
bot
added
size/md
tsh
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adea2a772b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lib/client/client_store_test.go
Outdated
| // No key ring saved — ReadProfileStatus should return partial status. | ||
| status, err := clientStore.ReadProfileStatus(p.Name()) | ||
| require.NoError(t, err) | ||
| require.Nil(t, status.Scope) |
There was a problem hiding this comment.
ProfileStatus.Scope is a string in this branch, so require.Nil(t, status.Scope) can never be true and this new subtest will fail even when ReadProfileStatus behaves correctly for an unscoped profile. This makes TestPartialProfileStatusScope a permanent false failure in CI; the check should compare against "" (and avoid nil/not-nil assertions for this field) to validate the intended behavior.
Useful? React with 👍 / 👎.
Update client_store_test.go Update client_store_test.go
williamong-tel
force-pushed
the
williamo/v18-scope-aware-tsh-profiles
branch
from
adea2a7 to
90709c3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90709c3c44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 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 { |
There was a problem hiding this comment.
This new guard makes scope-changing logins skip the branch that contains the fallback autoupdatetools.CheckAndUpdateRemote call, while the earlier profile == nil update path is also skipped because an unexpired profile exists. In practice, tsh login --scope=/new (or --scope="") from an existing valid session now reauthenticates without any managed update check, which regresses the previous behavior where scoped logins forced profile=nil and always performed the update check first.
Useful? React with 👍 / 👎.
1 participant
Backport of #64233
Actual Changes:
Changed references to
ScopePinto justScopein several places. Latest commit contains what needed to change. See adea2a7Changelog: add scope aware profiles after tsh login
Manual Test Plan
Test Environment
Exercised with multiple teleport clusters locally
Test Cases