Skip to content

Commit f8f1445

Browse files
committed
azrepos: rebuild binding manager around scopes and home account id
The pre-pivot binding manager had one stored shape — `(orgName, userName)` — and one verb pair to manipulate it (`bind`/`unbind` with `--local`). It couldn't distinguish "use account A for any org in tenant T" from "use account A for org O", and the stored value was a UPN, so a rename outside GCM silently broke the binding. Rebuild it around two changes: - **Scope union.** A binding now lives at one of two scopes: `Tenant(id)` ("any organization backed by this Microsoft Entra tenant") or `Org(name)` ("this specific organization"). Each case carries an `IsLocal` flag that picks per-clone vs. machine-wide storage. Resolution is most-specific-first: local Org → global Org → local Tenant → global Tenant → nothing. - **Home account id storage.** The persisted value for typical bindings is the MSAL `HomeAccountId`, which never changes when an account is renamed or its UPN suffix shifts. When a HomeAccountId isn't available (because the user hasn't signed in yet, or because they're pre-staging a binding), the manager falls back to storing the UPN. Storage key picks itself from the value's shape: values containing `@` go in `.username`, everything else (presumed HomeAccountId) goes in `.accountid`. `Bind` and `Unbind` clear the opposing key at the same scope so the two never drift; if they ever are both present at the same scope, `GetAccountId` prefers `.accountid` and emits a trace warning telling the user to run `bind`/`unbind` to clean up. `bind`/`unbind` are reshaped to match: azure-repos bind <account> (--tenant <id> | --org <name>) [--local] azure-repos unbind (--tenant <id> | --org <name>) [--local] `<account>` is resolved against the MSAL cache: if a cached account matches by UPN, we persist its HomeAccountId; otherwise we warn and persist the value verbatim. That keeps a working `login`-then-`bind` flow precise while still letting users pre-stage bindings before the matching login happens. On the credential paths: - `get` now derives a tenant id from the resolved Microsoft authentication authority (parsing the `login.microsoftonline.com/<tenant>` URL form) and feeds it into `ResolveForOrg`. When no binding matches but exactly one cached MSAL account belongs to the resolved tenant, the provider auto-routes to it silently — covering the consultant /guest-tenant case without any explicit `bind`. The lookup result (a HomeAccountId for new-style bindings or a UPN for legacy `.username` bindings) is wrapped in an `IMicrosoftAccount` and passed directly to `IMicrosoftAuthentication.GetTokenForUserAsync`, whose HomeAccountId-first / UPN-fallback resolution handles either shape natively. - `store` checks the MSAL cache for an account matching the incoming UPN and persists the cached account's HomeAccountId when found (UPN as-is otherwise); the binding manager picks `.accountid` or `.username` storage from the value's shape. - `erase` clears whichever level (local first, else global) had a binding for the org. Tenant bindings are left alone — they cover orgs other than the failing one. `list-bindings` learns the new shape: bindings group under a heading derived from the scope (`dev.azure.com/<org>` for Org, `login.microsoftonline.com/<tenant-id>` for Tenant) with a `(global)`/`(local)` row each. Stored HomeAccountIds are displayed as UPNs when the MSAL cache resolves them, otherwise as the raw id. The `--show-remotes` and `--verbose` flags keep their existing semantics. The `NoInherit` sentinel from the old binding manager is dropped. It was a workaround for "I want this clone to never inherit my global binding"; with bindings now resolved by an explicit hierarchy and the auto-route fallback handling the single-tenant case, a per-clone "always prompt" toggle is better delivered through the picker UI work rather than as a magic empty string. Tests cover the new storage shape (format-aware key selection, both directions of legacy↔modern overwrite), legacy `.username` read fallback at both Tenant and Org scopes, drift detection when both keys are present, and the resolution hierarchy across both scopes and levels. The four existing `GetCredentialAsync` tests are updated for the new mock surface (`GetAccountId(scope)` instead of `GetBinding(orgName)`). Assisted-by: Claude Opus 4.7 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
1 parent 8586c51 commit f8f1445

6 files changed

Lines changed: 830 additions & 1181 deletions

File tree

0 commit comments

Comments
 (0)