feat: support --organization-slug flag for user/browser SSO login#143
feat: support --organization-slug flag for user/browser SSO login#143devin-ai-integration[bot] wants to merge 4 commits intomainfrom
Conversation
Add support for scoping interactive SSO login to a specific sub-organization using the --organization-slug flag, similar to how machine identity login already works. Changes: - Add API types and function for accessible-with-sub-orgs endpoint - Add rescopeTokenToOrgBySlug function to resolve org slug to ID - After user login (browser or CLI), re-scope token if --organization-slug is set - Update flag description to reflect support for both user and machine identity login Co-Authored-By: ashwin <ashwin@infisical.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Greptile SummaryThis PR extends the existing Key changes:
Confidence Score: 4/5
Important Files Changed
Last reviewed commit: 8240907 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| newToken, rescopeErr := rescopeTokenToOrgBySlug(userCredentialsToBeStored.JTWToken, organizationSlug) | ||
| if rescopeErr != nil { | ||
| util.HandleError(rescopeErr, "Unable to scope login to the specified organization") | ||
| } | ||
| userCredentialsToBeStored.JTWToken = newToken | ||
| } |
There was a problem hiding this comment.
MFA bypass risk on re-scoped org token
rescopeTokenToOrgBySlug calls CallSelectOrganization and returns selectedOrgRes.Token, but ignores the MfaEnabled/MfaMethod fields from SelectOrganizationResponse. If the target organization (or sub-organization) enforces MFA, the returned token may be an incomplete/pre-MFA token that grants less access than expected — or one that silently bypasses MFA challenge handling that is performed elsewhere in the interactive login flow (e.g., in cliDefaultLogin).
The interactive CLI path (cliDefaultLogin) already handles MFA via a polling loop. If rescopeTokenToOrgBySlug returns a token that still requires MFA verification, subsequent API calls will silently fail with 401/403 rather than prompting the user for a code.
Consider inspecting the SelectOrganizationResponse for MFA fields and triggering the appropriate MFA challenge before storing the token:
selectedOrgRes, err := api.CallSelectOrganization(httpClient, api.SelectOrganizationRequest{OrganizationId: matchedOrgId})
if err != nil {
return "", fmt.Errorf("unable to select organization: %w", err)
}
if selectedOrgRes.MfaEnabled {
return "", fmt.Errorf("organization '%s' requires MFA; re-scoping via --organization-slug is not supported for MFA-protected orgs", organizationSlug)
}
return selectedOrgRes.Token, nilThere was a problem hiding this comment.
Addressed in 8240907. rescopeTokenToOrgBySlug now checks selectedOrgRes.MfaEnabled and returns a clear error if the target org requires MFA:
if selectedOrgRes.MfaEnabled {
return "", fmt.Errorf("organization '%s' requires MFA verification; please log in without --organization-slug and complete the MFA challenge during interactive org selection", organizationSlug)
}This ensures we never silently store a pre-MFA token.
There was a problem hiding this comment.
@greptile-apps can you re-run the review, taking into account all updated changes
There was a problem hiding this comment.
Great work addressing the previous feedback! The MFA safety check and error handling improvements are now in place. I've reviewed the complete PR and the code looks solid. Here's my assessment:
✅ Previous Issues Resolved
MFA Safety (8240907): The rescoping function now properly checks selectedOrgRes.MfaEnabled and returns a clear error when the target org requires MFA. This prevents storing pre-MFA tokens.
Error Handling (003476b): The flag conflict validation now properly handles errors with util.HandleError(orgIdErr) instead of discarding them.
✅ Code Quality
API Integration: The new CallGetAccessibleOrganizationsWithSubOrgs function follows existing patterns and has proper error handling.
Model Definitions: Clean, well-structured types for sub-organizations.
Search Logic: The nested loop to find matching slugs (root orgs → sub-orgs) correctly breaks out of both loops when a match is found.
Flag Conflict Validation: Properly prevents users from specifying both --organization-id and --organization-slug during direct login, with clear error messaging.
📝 Notes
Scope: The rescoping logic is correctly placed only in the user login path (if loginMethod == "user"). Machine identity login already has separate org-slug handling.
Timing: Rescoping happens after login but before storing credentials, which is the correct order - you need the initial token to query accessible orgs, and you want to store the rescoped token.
Backend Dependency: As noted in your PR description, this relies on the backend having the /v1/organization/accessible-with-sub-orgs endpoint. If it doesn't exist on older instances, users will get a clear error from the API call.
🚀 Ready to Merge
The implementation looks solid and all previous concerns have been addressed. The testing checklist in your PR description covers the important edge cases (backend endpoint existence, token permissions, MFA enforcement, flag conflicts).
One optional future consideration: if users report confusion when the backend endpoint doesn't exist (e.g., on older self-hosted instances), you could add a more helpful error message that suggests upgrading the Infisical instance. But that's not blocking for this PR.
| return &loginResponse, nil | ||
| } | ||
|
|
||
| // rescopeTokenToOrgBySlug resolves an organization slug to its ID using the accessible-with-sub-orgs | ||
| // endpoint and then calls selectOrganization to get a new token scoped to that org. | ||
| func rescopeTokenToOrgBySlug(currentToken string, organizationSlug string) (string, error) { | ||
| httpClient, err := util.GetRestyClientWithCustomHeaders() | ||
| if err != nil { | ||
| return "", fmt.Errorf("unable to get resty client with custom headers: %w", err) | ||
| } | ||
| httpClient.SetAuthToken(currentToken) | ||
|
|
||
| // Fetch all accessible organizations including sub-orgs | ||
| orgsResponse, err := api.CallGetAccessibleOrganizationsWithSubOrgs(httpClient) | ||
| if err != nil { | ||
| return "", fmt.Errorf("unable to fetch accessible organizations: %w", err) | ||
| } | ||
|
|
||
| // Search for the matching organization by slug (both root orgs and sub-orgs) | ||
| var matchedOrgId string | ||
| for _, org := range orgsResponse.Organizations { | ||
| if org.Slug == organizationSlug { | ||
| matchedOrgId = org.ID | ||
| break | ||
| } | ||
| for _, subOrg := range org.SubOrganizations { | ||
| if subOrg.Slug == organizationSlug { | ||
| matchedOrgId = subOrg.ID | ||
| break | ||
| } | ||
| } | ||
| if matchedOrgId != "" { | ||
| break | ||
| } | ||
| } | ||
|
|
||
| if matchedOrgId == "" { | ||
| return "", fmt.Errorf("organization with slug '%s' not found or not accessible", organizationSlug) | ||
| } | ||
|
|
||
| // Call selectOrganization to get a new token scoped to the matched org | ||
| selectedOrgRes, err := api.CallSelectOrganization(httpClient, api.SelectOrganizationRequest{OrganizationId: matchedOrgId}) | ||
| if err != nil { | ||
| return "", fmt.Errorf("unable to select organization: %w", err) | ||
| } | ||
|
|
||
| return selectedOrgRes.Token, nil | ||
| } | ||
|
|
||
| // Manages the browser login flow. | ||
| // Returns a UserCredentials object on success and an error on failure | ||
| func browserCliLogin() (models.UserCredentials, error) { |
There was a problem hiding this comment.
Feature discoverability — no documentation found
No documentation was found in the /docs folder (the folder does not exist in this repository). Could you clarify how users will discover the new --organization-slug behavior for user/browser SSO login? For example, is there a corresponding update to the Infisical docs site, a changelog entry, or a help-text reference users can follow?
The existing flag help text in init() does now mention both user and machine identity login, which is good — but external documentation for the browser SSO flow in particular would help users find and trust this feature.
There was a problem hiding this comment.
Addressed in 8240907. Added early validation that errors out when both --organization-id (or INFISICAL_ORGANIZATION_ID env) and --organization-slug are provided:
if organizationSlug != "" && isDirectUserLoginFlagsAndEnvsSet {
orgIdFlag, _ := util.GetCmdFlagOrEnv(cmd, "organization-id", []string{"INFISICAL_ORGANIZATION_ID"})
if orgIdFlag != "" {
util.PrintErrorMessageAndExit("Cannot use both --organization-id and --organization-slug at the same time. Please use only one to specify the target organization.")
}
}This prevents the silent override behavior. Note this conflict can only occur in direct user login mode (email/password via flags), since browser/interactive login doesn't use --organization-id.
There was a problem hiding this comment.
@greptile-apps please re-review with the latest changes.
There was a problem hiding this comment.
The latest changes look good! Both rounds of feedback have been properly addressed:
✅ Round 1 feedback (commit 8240907):
- MFA safety check: Correctly implemented at line 946-948. If the target org requires MFA, a clear error message is returned instead of storing a pre-MFA token.
- Flag conflict validation: Properly validates at lines 238-246 that both
--organization-idand--organization-slugcannot be used together.
✅ Round 2 feedback (commit 003476b):
- Error handling fix: The error from
util.GetCmdFlagOrEnvis now properly handled withutil.HandleError(orgIdErr)at line 240-242, instead of being discarded.
Code quality observations:
The rescopeTokenToOrgBySlug function (lines 905-950) is well-structured:
- Clean separation of concerns (fetch orgs → find match → select org → check MFA)
- Proper error wrapping with context
- Correct nested loop logic for searching both root orgs and sub-orgs
- Clear, actionable error messages
The integration point (lines 230-253) is logical:
- Conflict validation only runs for direct login mode (correct, since browser/interactive don't use
--organization-id) - Rescoping happens after login but before storing credentials (correct order)
- Errors are properly handled
No issues found. The implementation is solid and ready for the manual testing checklist outlined in the PR description.
…n-id/--organization-slug flags - Check MfaEnabled on SelectOrganizationResponse and return a clear error if the target org requires MFA verification - Error out early if both --organization-id and --organization-slug are provided to avoid silent override behavior Co-Authored-By: ashwin <ashwin@infisical.com>
|
Thanks for the thorough review! All three concerns have been addressed:
@greptile-apps please re-review with the latest changes. |
packages/cmd/login.go
Outdated
|
|
||
| // Validate that --organization-id and --organization-slug are not both set | ||
| if organizationSlug != "" && isDirectUserLoginFlagsAndEnvsSet { | ||
| orgIdFlag, _ := util.GetCmdFlagOrEnv(cmd, "organization-id", []string{"INFISICAL_ORGANIZATION_ID"}) |
There was a problem hiding this comment.
Error silently discarded on conflict check
The error return from util.GetCmdFlagOrEnv is discarded with _. If that call fails (e.g., because the flag is not registered or the env variable cannot be read), orgIdFlag will be an empty string and the conflict validation at line 239 will silently pass — meaning a user who has INFISICAL_ORGANIZATION_ID set and also provides --organization-slug will NOT see the expected error. Compare to the identical call at line 221–224 which correctly handles the error.
| orgIdFlag, _ := util.GetCmdFlagOrEnv(cmd, "organization-id", []string{"INFISICAL_ORGANIZATION_ID"}) | |
| orgIdFlag, err := util.GetCmdFlagOrEnv(cmd, "organization-id", []string{"INFISICAL_ORGANIZATION_ID"}) | |
| if err != nil { | |
| util.HandleError(err) | |
| } |
There was a problem hiding this comment.
Fixed in 003476b. The error is now properly handled instead of discarded:
orgIdFlag, orgIdErr := util.GetCmdFlagOrEnv(cmd, "organization-id", []string{"INFISICAL_ORGANIZATION_ID"})
if orgIdErr != nil {
util.HandleError(orgIdErr)
}This is now consistent with how the same call is handled at line 221-224.
@greptile-apps please re-review with the latest changes.
There was a problem hiding this comment.
✅ Fix confirmed
The error handling is now correct and consistent with the pattern at lines 221-224. The validation will properly fail if util.GetCmdFlagOrEnv encounters an error (e.g., flag not registered or environment variable issues), preventing the security concern where conflicts could be silently ignored.
Good catch and clean fix!
Instead of silently discarding the error with _, properly handle it to ensure the --organization-id/--organization-slug conflict check works correctly even if the flag/env cannot be read. Co-Authored-By: ashwin <ashwin@infisical.com>
…ation
Addresses security concern where an attacker could trick a user into
running a command with a malicious --organization-slug. Now the CLI
shows the matched organization name and slug, and requires the user
to explicitly confirm before re-scoping the token.
Prompt example:
You are about to scope your login to organization "Acme Sub-Org"
(slug: acme-sub-org). Do you want to continue?
> Yes
No
Co-Authored-By: ashwin <ashwin@infisical.com>
1 participant
feat: support --organization-slug for user/browser SSO login
Summary
Extends the existing
--organization-slugflag (previously only for machine identity login) to also work with user login methods, including interactive browser/SSO login. This allows users to scope their login session to a specific sub-organization without manually selecting it during the interactive flow.Approach: After any user login completes (browser SSO or CLI email/password), if
--organization-slugis provided, the CLI:GET /v1/organization/accessible-with-sub-orgsto fetch all accessible orgs and sub-orgsselectOrganizationto re-scope the JWT token to the matched organizationThis is a purely additive change — existing login behavior is unchanged when the flag is not provided.
Files changed:
packages/api/model.go— New response types for the accessible-with-sub-orgs endpointpackages/api/api.go— NewCallGetAccessibleOrganizationsWithSubOrgsAPI functionpackages/cmd/login.go—rescopeTokenToOrgBySlug()function + integration into user login flow + updated flag descriptionUpdates since initial revision
Addressed Greptile review feedback across two rounds, plus a security concern raised by an engineer:
Round 1 (
8240907):rescopeTokenToOrgBySlugnow inspectsMfaEnabledon theSelectOrganizationResponse. If the target org requires MFA, the CLI returns a clear error message instead of silently storing a pre-MFA token.--organization-idand--organization-slugare provided, preventing silent override of--organization-idby the slug rescoping logic. This only applies to direct email/password login (browser/interactive login doesn't use--organization-id).Round 2 (
003476b):util.GetCmdFlagOrEnvin the conflict validation was silently discarded with_. Now properly handled withutil.HandleError(orgIdErr), consistent with the rest of the function.Round 3 (
a108a6b) — Security hardening:--organization-slug, silently scoping their session to an attacker-controlled org. The CLI now displays the matched organization name and slug and requires explicit "Yes"/"No" confirmation viapromptui.Selectbefore re-scoping the token. If the user selects "No", the login is aborted with a clear message.Review & Testing Checklist for Human
GET /v1/organization/accessible-with-sub-orgs. Confirm this endpoint exists and returns{ organizations: [{ id, name, slug, subOrganizations: [{ id, name, slug }] }] }. If the response shape differs, the slug lookup will silently fail to match — this is the highest-risk item since the CLI changes were made without backend verification.accessible-with-sub-orgswith this token works, otherwise the rescoping flow will fail.promptui.Select, which requires interactive stdin. Verify that using--organization-slugin a non-interactive context (e.g., piped input, CI scripts with user login) either fails gracefully or consider whether a--yes/--forceflag is needed to skip the prompt for automation use cases.infisical login --organization-slug <sub-org-slug>with browser SSO, confirm the prompt shows the correct org name, select "Yes", and verify the stored token is scoped to the sub-org (e.g., by runninginfisical secrets getagainst a project in that sub-org).--organization-idand--organization-slug→ expect conflict error. (d) User selects "No" at confirmation prompt → expect clear cancellation message.infisical loginwithout--organization-slugand confirm the interactive org selection flow still works. Also verify machine identity login with--organization-slugis unaffected (machine identity should continue to work without prompts since it uses a different code path).Notes
--organization-slugflag was already registered on the login command (for machine identity use). This PR reuses it for user login without adding a new flag./docs; a docs update should follow separately.infisical login --organization-slug <slug>in CI/automation scripts will encounter an interactive prompt. If this becomes a common pattern, consider adding a--yesor--skip-promptflag to bypass confirmation for non-interactive environments.Link to Devin session: https://app.devin.ai/sessions/783a23defe5244d09b8b3ceb771e49c6
Requested by: ashwin