PPSC-1230: fix post-setup hint printing IdP slug as tenant ID - #294
Open
dmitrysmirnov-armis wants to merge 1 commit into
Open
Conversation
|
dmitrysmirnov-armis
marked this pull request as ready for review
August 7, 2026 18:31
Test Coverage Reporttotal: (statements) 72.3% Coverage by function |
yiftach-armis
approved these changes
Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Type of Change
Problem
armis-cli auth setupprints environment variables at the end for the admin to deploy via MDM. TheARMIS_TENANT_IDline printed the IdP config'stenant_id— a human-readable slug the admin typed into the form (e.g.acme) — instead of the actual Armis credential-derived tenant identifier (e.g.69382fa8c6be76dc4390cbae, the hex value shown byarmis-cli auth whoami). Admins deploying this value via MDM would set the wrongARMIS_TENANT_IDon developer machines, causingarmis-cli auth loginto fail because it sends the wrong tenant to the device authorization endpoint.Root cause:
printPostSetupHintininternal/cmd/auth_setup.gowas called withreqCfg.TenantID/existing.TenantID(the IdP config slug) instead of thedetectedTenantalready resolved from the admin's own credentials viaprovider.GetTenantID().Solution
Threaded
detectedTenantthrough the full call chain —runConfigFileSetup,runInteractiveCreate,runInteractiveUpdate,sendCreate,sendUpdate— so everyprintPostSetupHintcall site now uses it instead of the config's tenant slug. This covers both the interactive flow and the--confignon-interactive (MDM/CI) path. Other uses of the tenant slug (success messages, the 409-conflict confirmation prompt) are left as-is since they correctly refer to the IdP config's tenant, not theARMIS_TENANT_IDenv var value.Testing
Automated Tests
Added
TestAuthSetupPostSetupHintUsesDetectedTenant, which configures a distinct credential tenant (acme, via Basic auth) vs. IdP config tenant slug (acme-slug) and asserts the printed hint uses the credential tenant. Verified it fails against the pre-fix code (prints the slug) and passes against the fix. Full suite (go test ./...,go vet ./...,gofmt) passes.Manual Testing
Traced both the interactive and
--configcode paths to confirmdetectedTenant(resolved once inrunAuthSetupfromprovider.GetTenantID()) reaches everyprintPostSetupHintcall site.Reviewer Notes
No fallback logic was added for an empty
detectedTenant:detectIdentityonly returns""whenprovider.GetTenantIDfails, and every code path that reachesprintPostSetupHinthas already made a successful authenticated admin-API call with that same provider — so a failure there could not have gotten this far.Checklist