Add named login profiles and directory-scoped tenant selection - #372
Add named login profiles and directory-scoped tenant selection#372scott-ray-wilson wants to merge 2 commits into
Conversation
|
💬 Discussion in Slack: #pr-review-cli-372-add-named-login-profiles-and-directory-scoped-tenant-selection Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
|
| Filename | Overview |
|---|---|
| packages/util/profile.go | Implements profile migration, selection, directory binding, organization resolution, and persistence; the previously reported concurrent whole-config update remains outstanding. |
| packages/util/credentials.go | Loads credentials by profile name, prevents explicit cross-domain token use, applies organization overrides, and intentionally leaves session renewal disabled. |
| packages/cmd/user.go | Reworks domain updates to target one named profile and aborts before repointing when credential cleanup fails. |
| packages/cmd/profile.go | Adds the profile command surface for creation, selection, terminal pinning, directory binding, organization defaults, and deletion. |
| packages/util/logout.go | Adds profile-aware local cleanup and session revocation while retaining server sessions shared by other profiles. |
| packages/cmd/root.go | Adds global profile and organization overrides and integrates them into command-wide resolution. |
| packages/api/api.go | Adds the authenticated API operation used to revoke a server-side user session. |
Reviews (5): Last reviewed commit: "feat(cli): named login profiles for mult..." | Re-trigger Greptile
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 6 · PR risk: 0/10 |
5c2f55a to
cff3364
Compare
|
@greptile review |
cff3364 to
84e2fcb
Compare
|
Verification script referenced in the description. Runs in a throwaway INFISICAL_BIN=./infisical \
VERIFY_DOMAIN=https://app.infisical.com \
VERIFY_EMAIL=you@example.com VERIFY_PASSWORD='...' \
VERIFY_ORG_A=<org-id> VERIFY_ORG_B=<org-id> \
./verify.shLast run against a local instance on the current head: 21 passed, 0 failed.
|
84e2fcb to
0cf3c39
Compare
|
@veria-ai review |
|
@greptile review |
0cf3c39 to
d6eb7bc
Compare
|
@greptile review |
Switching organizations meant logging out and back in. The CLI stored one
session per account, keyed by email in the keyring, and a session token is
scoped to a single organization, so a second login destroyed the first.
Working across tenants in parallel meant exporting tokens into env vars or
.env files.
A profile is now one login: an account on one instance, plus the organization
it uses by default. Each profile has its own keyring entry, so sessions
coexist, and selecting a profile selects the account, instance and
organization together.
The organization is a field of the profile rather than part of its identity.
--org and INFISICAL_ORG retarget a single command by name, slug or id, and the
organization-scoped token is cached per organization in the keyring, so the
switch costs one exchange and nothing thereafter. Changing the profile's
default is `profile set-org` (also reachable as `org switch`).
Which profile a command uses is decided by --profile, then INFISICAL_PROFILE,
then a bound directory, then the machine default. An explicit override wins over
a bound directory, and says so, so that a binding which did not apply is
explained rather than silently ignored. Those last three each get
their own verb, so all of them are discoverable from `profile --help`:
profile use <name> the default for this machine
profile pin <name> this terminal only, via eval
profile bind [name] [path] a directory and everything under it
Sub-organizations are handled throughout: they appear nested in `org list`,
`--org` resolves them by name, slug or id, and a profile scoped to one reports
it as "Acme / Research" rather than as the root organization it would otherwise
be indistinguishable from. Organizations that require MFA prompt during
`profile new` and `profile set-org`, which perform their own exchange; `--org`
on an ordinary command cannot prompt, so it fails with a message pointing at
the command that can.
Commands added:
profile list | current | new | use | pin | unpin | bind | unbind
| set-org | delete
org list | switch
logout
Session handling. Sessions continue to expire at JWT_AUTH_LIFETIME, with expiry
sending the user back through login, unchanged from today. Renewal via the
stored refresh token stays unimplemented on purpose: the server rotates the
refresh token on every refresh and treats a stale one as theft by revoking the
session, which several CLI processes sharing one vault entry cannot coordinate
safely. The token is also no longer written to the vault, since nothing read it
and storing it only widens what a stolen vault yields. `logout` revokes
server-side, and so do `profile delete` and `reset`. Because the server keys
sessions by user, IP and user agent, several profiles for one account on one
machine share a session, so a session another profile still uses is left intact
and only local credentials are removed.
Integration with existing commands: `init` uses the profile's organization
instead of asking again and offers to bind the directory; `user switch`
operates on profiles; `vault set` clears them. An explicit --domain now beats a
profile's saved domain instead of being silently overridden, `user update
domain` only repoints profiles that were on the instance being changed rather
than every profile sharing an email, and `reset` removes every stored session
instead of orphaning all but the active one.
Hardening from review: profile names are shell-quoted where pin prints an
export, since a derived name comes from a server-supplied email and would
otherwise run as a command under eval; organization selectors match by id, then
slug, then name, with ambiguity rejected, so an organization named after
another's id cannot be selected in its place; logout authenticates revocation
with any live token rather than only the profile's own, which previously let a
cached organization token survive locally deleted credentials; a profile's
session is refused rather than sent when an explicit --domain names a different
instance; `user update domain` selects a profile rather than an account, so profiles
sharing an email and instance for different organizations are not moved
together, and the moved profile's session is cleared, before the new instance is
recorded, since a session that outlived the change would be sent there; server-supplied names are stripped
of control characters before reaching a terminal; and the legacy login pointer
is published only for email-named profiles, so an older binary cannot load one
profile's token while aimed at another's instance.
Migration is lazy and requires no re-login. Legacy loggedInUserEmail and
loggedInUsers entries become profiles named after the account email, which is
also the legacy keyring key, so existing sessions keep working untouched, and
those fields stay in sync with the active profile for older binaries and
scripts that read them. Single-profile users see no change in behavior.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d6eb7bc to
77d6522
Compare
|
@greptile review |
There was a problem hiding this comment.
Unrelated to the file, but some considerations I had while testing this:
- Why the organization name and not the organization slug in the generated profile name?
- Why not use the org in the first generated profile name? After you have logged in to a few orgs it can get hard to remember the org without listing
- With these auto-generated names, a rename command could be useful here, instead of deleting and setting up a new profile.
- Should login status https://infisical.com/docs/cli/commands/login#infisical-login-status:check-the-active-user-session show profile information?
There was a problem hiding this comment.
All four are in now.
Slug, not name. Profiles are named <email>--<org-slug>, e.g. scott@example.com--acme-x4k2. The slug is stable and already URL-safe. If an instance reports no slug we fall back to the slugified name, then to a short piece of the id.
Org in the first name too. Every login gets the suffix now, including the first one, so you can tell the tenant from the name without listing.
Rename. Added infisical profile rename <old> <new>. It moves the stored session and the cached org sessions, and updates the default profile and any directory bindings. A pinned terminal keeps pointing at the old name, so it tells you to pin again.
login status. It now shows the profile and how it was selected, the org name next to its id, and Organization via when --org is in play. Same fields in --json (profile, profileSource, organizationName).
One trade-off worth flagging: a profile that is not named after its email does not publish the legacy loggedInUserEmail field. An older CLI binary reading the same config will ask for a fresh login instead of loading the wrong profile's token. Scott and I decided that is the right call, but it does mean a downgrade after a fresh login is no longer seamless.
| } | ||
| util.PrintWarning(fmt.Sprintf("Profile '%s' defaults to organization %s, so later commands here will not find this project unless you pass --org again. Run [infisical profile set-org %s] to make it the default.", userCreds.ProfileName, profileOrg, orgDisplay)) | ||
| } |
There was a problem hiding this comment.
I'm a bit concerned that this warning is not enough here.
They can be silenced by --silent, and the errors from the command won't hint that the issue is this mismatch between the profile org and the project from another org.
I think we should raise an error in this case.
There was a problem hiding this comment.
Agreed. It is an error now, not a warning.
init exits 1 without writing anything, and names the two durable fixes: make it the profile's default with profile set-org, or keep both orgs by creating a second profile and profile binding it to the directory.
| return "", requestError | ||
| } else if mfaErrorResponse != nil { | ||
| if mfaErrorResponse.Context.Code == "mfa_invalid" { | ||
| msg := fmt.Sprintf("Incorrect, verification code. You have %v attempts left", 5-i) |
There was a problem hiding this comment.
We use this same "5" in the login.go, we should put it in a shared const to keep them synced.
There was a problem hiding this comment.
Done, both loops use a shared mfaMaxAttempts.
Unrelated thing I hit while testing this with MFA turned on: a wrong code never prints "You have N attempts left". The CLI matches on context.code == "mfa_invalid", but the backend now returns a plain 500 with no code, so the CLI just re-prompts with no explanation. That is pre-existing on main, not from this PR. Happy to file it separately.
| // shellOutputIsCaptured reports whether stdout is being read by something | ||
| // rather than shown on screen. Commands that work by printing shell statements | ||
| // only take effect when the caller captures them, as in eval "$(...)"; a | ||
| // terminal on stdout means the statement was displayed and nothing changed. | ||
| func shellOutputIsCaptured() bool { | ||
| return !isatty.IsTerminal(os.Stdout.Fd()) | ||
| } | ||
|
|
||
| // requireShellCapture stops a shell-mutating command that was run bare, and | ||
| // shows the form that actually works, rather than reporting a success that did | ||
| // not happen. | ||
| func requireShellCapture(invocation string) { | ||
| if shellOutputIsCaptured() { | ||
| return | ||
| } | ||
| util.PrintlnStderr(fmt.Sprintf("This command works by printing a shell statement, so it only takes effect when the shell reads it:\n\n eval \"$(%s)\"\n\nNothing has been changed. Tip: add a shell alias if you use this often.", invocation)) | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
This may not work depending on the OS/shell/SSH setup. For example, PowerShell wouldn’t support the eval "$(…)" flow, and non-TTY SSH/scripts can look successful without actually pinning.
We should update the error message (and profile pin help) to address these scenarios too, as the pin command would be unusable for them.
We should also mention the PowerShell/manual env-var alternative.
There was a problem hiding this comment.
Done. The error and profile pin --help now spell out the alternatives:
$env:INFISICAL_PROFILE = 'globex' # PowerShell
set INFISICAL_PROFILE=globex # cmd.exe
export INFISICAL_PROFILE=globex # scripts and CI, or just use --profile
I also fixed the false-success problem you implied. Capturing stdout does not prove a shell evaluated it, it could be a pipe or a file, so the message now says the profile is pinned once the shell evaluates the printed export rather than claiming it already happened.
|
|
||
| if !found { | ||
| util.PrintlnStdout("Status: profile does not exist. Run [infisical login --profile " + resolved.Name + "] to create it.") | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm getting a bit confused by profile login and profile new. I understand the difference and usage of each, but when it comes to what feels more natural, I would always try to create a new profile using the new command.
login for me -> logging into a profile, not creating one
new for me -> actually creating a new profile from the ground up
Maybe some renaming here; what about "configure" instead of login? or "setup"?
And the new feels more like "copy" or "duplicate" to me. "fork"/"clone" maybe?
There was a problem hiding this comment.
You are right that login --profile x reads like "log into profile x", not "create it".
I kept the command names and split the two meanings onto separate flags:
login --save-as <name>creates the profile, or replaces its session. This is the "set it up" verb.login --profile <name>only signs back in to a profile that already exists. It keeps the account, instance and org, so you do not pass--organization-idagain and you get no hosting prompt. If the profile does not exist it fails and points you at--save-as.profile newis unchanged: reuse the login you already have for another org.
So --profile now means the same thing on login as everywhere else, which is "use this profile", and creation has its own flag.
I did not rename the commands to configure/setup or clone/fork. The confusing part was the flag rather than the verb, and renaming would break people already scripting against them. Say the word if you would still rather have the rename.
| if OrgMatchTier(selector, profile.OrganizationID, "", "") == orgMatchID { | ||
| details.OrganizationSource = selectorSource | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Same fix. The --org comparison uses the scoped id, so --org <root-id> on a sub-org profile now does the exchange instead of matching and silently doing nothing.
| } | ||
| httpClient.SetAuthToken(details.UserCredentials.JTWToken) | ||
|
|
||
| currentOrgID := details.OrganizationID |
There was a problem hiding this comment.
Same fix. org list reads subOrganizationId from the token first, so the * lands on the sub-org row rather than on its parent.
There was a problem hiding this comment.
getLoggedInUsers and LoggedInUsersPrompt are now dead code, right?
There was a problem hiding this comment.
Yes. Both removed.
| bestCached := models.CachedOrgSession{} | ||
| bestTier := 0 | ||
| for _, cached := range details.UserCredentials.OrgTokens { | ||
| tier := OrgMatchTier(selector, cached.OrgID, cached.OrgSlug, cached.OrgName) | ||
| if tier > bestTier && !IsJWTExpired(cached.Token) { | ||
| bestCached, bestTier = cached, tier | ||
| } | ||
| } |
There was a problem hiding this comment.
We resolve by bare name (Research) but store OrgName as "Acme / Research". Repeat --org Research never hits orgTokens, so every command re-lists orgs and re-exchanges.
There was a problem hiding this comment.
Fixed. A selector now matches a sub-org's own name as well as the full Acme / Research label, so --org Research hits the cache instead of re-listing every time.
Verified by counting requests: the first --org Research does one lookup and one exchange, and every run after that does neither.
There was a problem hiding this comment.
I log in once (default org Acme / Production). Over time, I run:
infisical --org staging secrets ...
infisical --org qa secrets ...
infisical --org client-a secrets ...
infisical --org client-b secrets ...
# ... or INFISICAL_ORG=client-c in CI
Each new non-default org adds a full session JWT to orgTokens in the same keyring blob. After ~5 such orgs (on macOS, with our token sizes), the keyring write can fail; commands still work for that run, but the cache never sticks and every --org re-lists/re-exchanges.
There was a problem hiding this comment.
Fixed by not storing the tokens there at all.
Each cached org session is now its own keyring entry, org-session:<profile>:<orgId>. The profile keeps only the id, name and slug, which is all a selector needs to match on. So the profile's own entry does not grow with the number of orgs you use.
Checked with three cached orgs: three separate keyring entries, and the profile's own entry stayed around 1 KB, well under the macOS limit. logout, profile delete, profile rename, user update domain and reset all clean up the extra entries.
Sub-organizations were the main correctness problem. A session scoped to one carries the root organization in its token and acts in the sub-organization, so filtering projects by the token's id found nothing and init aborted even when projects existed. Profiles now record both ids and expose the scoped one, which init, org list and --org all use. --org is resolved against what the profile already knows before any API call. Profiles store the organization slug, and organizations used before are kept in a small index, so a repeated selector costs nothing. Their tokens moved out of the profile's keyring entry into one entry each, keeping that entry well under the platform size limit no matter how many organizations are used. Generated profile names now carry the organization slug from the first login, so they say which tenant they belong to, and profile rename can change them. A profile not named after its email no longer publishes the legacy loggedInUserEmail pointer, so an older binary asks for a fresh login rather than loading another profile's session. login gains --save-as to store a login under a chosen name, creating that profile or replacing its session. --profile now only signs back in to an existing profile, keeping its account, instance and organization, so --organization-id is not needed again. Also: init refuses instead of warning when --org would link a project later commands could not find; login status reports the profile, its selection source and the organization name; the "Using profile" notice moved into the session loader so commands that never authenticate stay quiet; pin explains the PowerShell and CI alternatives and no longer claims a pin took effect before a shell evaluates it; typed profile names are length checked; the MFA attempt limit is shared between the login and organization flows; and the dead legacy user helpers are removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Problem
Switching organizations means logging out and back in. The CLI keeps one session per account, keyed by email in the keyring, and a session token is scoped to a single organization, so a second login destroys the first. Working across tenants in parallel means exporting tokens into env vars or
.envfiles. Community ask: Infisical/infisical#2191.Model
A profile is one login: an account on one instance, plus the organization it uses by default. Each profile gets its own keyring entry, so sessions coexist.
The organization is a field of the profile, not part of its identity.
--org/INFISICAL_ORGretarget a single command by name, slug or id, backed by a per-organization token cache, so the switch costs one exchange and nothing after.profile set-org(akaorg switch) changes the profile's default.Selection per invocation:
--profile>INFISICAL_PROFILE> bound directory > machine default. Those last three each get a verb, so all are discoverable fromprofile --help:profile use <name>profile pin <name>(viaeval)profile unpinprofile bind [name] [path]profile unbindAn explicit override beats a bound directory and says so, so a binding that did not apply is explained rather than silently ignored.
Profiles are named
<email>--<org-slug>(scott@example.com--acme-x4k2), so a name says which tenant it is without listing anything.profile renamechanges it.Commands
login --save-as <name>stores the login under a name you choose, creating that profile or replacing its session.login --profile <name>signs back in to a profile that already exists, keeping its account, instance and organization, so--organization-idis not needed again; naming a profile that does not exist fails and points at--save-as.Rationale for the less obvious calls
org switchand profiles felt like unrelated features. They were: org lived inside the profile's identity, so changing it meant mutating or forking a profile. Making it a field is the kubectl context/namespace split.login --save-as,login --profile,profile new,org switchfrom a pinned shell all leave it alone. The default is what every unpinned terminal resolves to, so moving it reaches across terminals.--useopts in explicitly.--orgresolves locally first. Profiles record the organization slug, and organizations reached before are indexed on the profile, each matched by id, slug or name. A repeated--orgtherefore makes no API calls at all; only the first use of a new organization lists and exchanges. The trade-off is that a name matching your own organization short-circuits without asking the server, so a same-named organization you have never used is no longer detected. Ids and slugs are unique, so they are unaffected.org-session:<profile>:<orgId>; the profile stores only id, name and slug, which is what a selector matches on.organizationIdand the sub-organization insubOrganizationId, and acts in the latter. Profiles record both and expose the scoped one, which is whatinitfilters projects by, whatorg listmarks as current, and what--orgcompares against.JWT_AUTH_LIFETIME; renewal stays unimplemented. Unchanged frommain, where the refresh path is commented out. The server rotates the refresh token on every refresh and treats a stale one as theft by revoking the session, with a 10s grace window, which several CLI processes sharing one vault entry cannot coordinate. The token is also no longer persisted, since nothing read it.initstops asking for an organization. It takes the profile's. The old prompt also silently re-scoped your session as a side effect.profile newandprofile set-orgperform their own exchange and prompt.--orgon an ordinary command runs inside credential resolution and cannot prompt, so it fails with a message naming the command that can.scan,agentorgatewaystay quiet even in a pinned terminal.Compatibility
Migration is lazy and needs no re-login: legacy
loggedInUserEmail/loggedInUsersbecome profiles named after the account email, which is already the keyring key, so existing sessions keep working.The legacy fields are only published for a profile named after its email. Migrated profiles still are, so a
mainbinary keeps working against them. A profile created by this branch is named<email>--<org-slug>, so amainbinary will ask for a fresh login rather than load some other profile's token against this profile's instance.Behavior changes for release notes:
initno longer prompts for an organization (--orgorprofile set-orgto cross).initnow fails instead of warning when--orgwould link a project that later commands, using the profile's own organization, could not find.login --profile <name>no longer creates a profile; uselogin --save-as <name>.resetnow revokes sessions server-side instead of only deleting local files.vault setclears the profile list along with the login it already cleared.login statusreports the profile, how it was selected, and the organization name;--jsongainsprofile,profileSourceandorganizationName.Verification
Automated script covering 20 behaviors (login and migration, targeted-login isolation, org-as-a-dimension including token-repointing,
profile newwith--use/--pin, the full precedence chain, pin's refusal to report false success, logout revocation including the shared-session case). Runs in a throwawayHOME, needs an account with two orgs. Script is attached in a comment below.On top of that, a 93-check matrix run against a local instance, one section per review comment, seeded with two root organizations, a third whose slug differs from its name, and a sub-organization with its own project. All 93 pass. It covers:
initon a sub-organization session listing and linking that organization's project, and a root-organization session still seeing only its own.--orgAPI call volume, counted from the access log: 0 calls for your own organization by id, slug or name including first use; 1 listing + 1 exchange for a new organization, then 0.--orgrefusal that points atprofile set-org.Done manually against a local instance as well: sub-org resolution by name and slug, single-profile output diffed byte-for-byte against a
mainbuild, two terminals pinned to different profiles, and per-profile instance routing.Worth a reviewer's own pass:
--domainFound while testing, pre-existing on
main, not fixed herecontext.code == "mfa_invalid", but the backend now returns a plain 500 with no code, so it silently re-prompts.user get tokenoverwrites the session error with the result of reading its own--plainflag, so a failed session surfaces as "invalid token format".🤖 Generated with Claude Code