Skip to content

Commit 0d5cfcb

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 three 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. - **`IMicrosoftAccount`-typed API.** The manager traffics in `IMicrosoftAccount` rather than opaque strings, so the storage boundary owns the UPN-vs-HomeAccountId distinction and the provider doesn't have to encode-then-translate at the cache boundary. `Bind` takes an account with at least one of `HomeAccountId` or `UserName` populated; `GetAccount` returns whichever fields the stored binding has. - **Dual-key storage.** `.accountid` always holds the MSAL `HomeAccountId`, `.username` always holds the UPN. Both are written atomically when both are known; either alone is written when only one is available (legacy `.username`-only bindings continue to read back as a UPN-only account). A HomeAccountId never changes when an account is renamed or its UPN suffix shifts, so credential lookups survive identity drift; the colocated UPN gives `list-bindings` something to display when MSAL hasn't seen the account recently. No format detection, no drift warning: with atomic dual-key writes both keys present is the normal state for a fully-resolved binding. User-typed identifiers (CLI `<account>` arguments, URL/credential user names) classify themselves through a new factory on `MicrosoftAccount`: MicrosoftAccount.FromIdentifier(string) `FromIdentifier` recognises an MSAL Entra `HomeAccountId` shape positively — an `<object-id>.<tenant-id>` pair split on the LAST `.` (mirroring `Microsoft.Identity.Client.AccountId.ParseFromString`, which permits an object id to contain dots in B2C / guest scenarios) where the tenant-id suffix is a well-formed GUID. The object-id prefix is only required to be non-empty, since MSAL itself doesn't validate it as a GUID and exotic-but-real object ids exist. Anything else (including bare words and ADFS-shape single tokens, which are indistinguishable from a username) routes into the `UserName` slot. Keeping the heuristic on the account type lets the provider stay out of the classification business and gives future consumers (eg. a picker UI) one place to reuse. Tests on the factory cover both classic Entra (GUID.GUID) and non-GUID-object-id shapes, the ambiguous-as-UPN cases, malformed-tenant rejection, and null/whitespace rejection. `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 or HomeAccountId, we persist both fields of that cached account; otherwise we warn and persist the value classified by `MicrosoftAccount.FromIdentifier`. 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` derives a tenant id from the resolved Microsoft authentication authority (parsing the `login.microsoftonline.com/<tenant>` URL form) and feeds it into `ResolveAccountBinding`. The returned `IMicrosoftAccount` is passed directly to `IMicrosoftAuthentication.GetTokenForUserAsync`, whose HomeAccountId-first / UPN-fallback resolution handles either shape natively. When no binding matches the result is `null` — the existing interactive sign-in path handles it from there. Inline URL/credential-supplied user names go through `MicrosoftAccount.FromIdentifier` so a user can pin a clone to a specific MSAL account by either UPN or HomeAccountId. A future PR (state[]/continue) will layer optimistic auto-routing on top, with a safe loop-breaking story. - `store` matches the incoming user name against the MSAL cache by either UPN or HomeAccountId and binds the resolved account; on a cache miss it falls back to a single-field binding classified the same way. - `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. The bound account is displayed UPN-first, falling back to HomeAccountId for HomeAccountId-only bindings. 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, 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 (HomeAccountId-only, UPN-only, both fields), legacy `.username`-only read fallback at both Tenant and Org scopes, scope/level resolution precedence, and local-vs-global enumeration boundaries. The existing `GetCredentialAsync` tests are updated for the new mock surface (`GetAccount(scope)` returning an `IMicrosoftAccount` instead of `GetBinding(orgName)` returning a string), and a new `DevAzureUrlHomeAccountId` test pins the URL-user classification. Assisted-by: Claude Opus 4.7 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
1 parent cdbd055 commit 0d5cfcb

8 files changed

Lines changed: 936 additions & 1190 deletions

File tree

src/shared/Core.Tests/Authentication/MicrosoftAuthenticationTests.cs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,69 @@ public void MicrosoftAuthentication_GetManagedIdentity_Invalid_ThrowsArgumentExc
7070
Assert.Throws<ArgumentException>(() => MicrosoftAuthentication.GetManagedIdentity(str));
7171
}
7272
}
73+
74+
public class MicrosoftAccountTests
75+
{
76+
[Theory]
77+
// Classic Entra ID: two GUIDs
78+
[InlineData("00000000-0000-0000-0000-000000000002.00000000-0000-0000-0000-000000000001")]
79+
[InlineData("12345678-1234-1234-1234-123456789abc.fedcba98-7654-3210-fedc-ba9876543210")]
80+
// Non-GUID object id; tenant id is still a GUID — split on LAST dot
81+
[InlineData("contoso.onmicrosoft.com.00000000-0000-0000-0000-000000000001")]
82+
[InlineData("b2c-policy.tenant.00000000-0000-0000-0000-000000000001")]
83+
public void MicrosoftAccount_IsHomeAccountIdShape_ObjectIdAnyAndGuidTenantId_True(string value)
84+
{
85+
Assert.True(MicrosoftAccount.IsHomeAccountIdShape(value));
86+
}
87+
88+
[Theory]
89+
[InlineData(null)]
90+
[InlineData("")]
91+
[InlineData(" ")]
92+
[InlineData("alice@example.com")]
93+
[InlineData("alice")] // ADFS-shape / bare word
94+
[InlineData("00000000-0000-0000-0000-000000000002")] // single guid, no dot
95+
[InlineData("00000000-0000-0000-0000-000000000002.")] // trailing dot
96+
[InlineData(".00000000-0000-0000-0000-000000000001")] // leading dot
97+
[InlineData("00000000-0000-0000-0000-000000000002.not-a-guid")] // tid is not a GUID
98+
[InlineData("alice@example.com.contoso")] // tid is not a GUID
99+
public void MicrosoftAccount_IsHomeAccountIdShape_NotEntraHomeAccountId_False(string value)
100+
{
101+
Assert.False(MicrosoftAccount.IsHomeAccountIdShape(value));
102+
}
103+
104+
[Fact]
105+
public void MicrosoftAccount_FromIdentifier_HomeAccountIdShape_PopulatesHomeAccountId()
106+
{
107+
const string id = "00000000-0000-0000-0000-000000000002.00000000-0000-0000-0000-000000000001";
108+
109+
MicrosoftAccount account = MicrosoftAccount.FromIdentifier(id);
110+
111+
Assert.Equal(id, account.HomeAccountId);
112+
Assert.Null(account.UserName);
113+
}
114+
115+
[Theory]
116+
[InlineData("alice@example.com")]
117+
[InlineData("alice@contoso.onmicrosoft.com")]
118+
[InlineData("alice")] // ambiguous → UserName
119+
[InlineData("contoso")] // ambiguous → UserName
120+
[InlineData("00000000-0000-0000-0000-000000000002")] // single guid, not HomeAccountId-shaped
121+
public void MicrosoftAccount_FromIdentifier_NonHomeAccountIdShape_PopulatesUserName(string id)
122+
{
123+
MicrosoftAccount account = MicrosoftAccount.FromIdentifier(id);
124+
125+
Assert.Null(account.HomeAccountId);
126+
Assert.Equal(id, account.UserName);
127+
}
128+
129+
[Theory]
130+
[InlineData(null)]
131+
[InlineData("")]
132+
[InlineData(" ")]
133+
public void MicrosoftAccount_FromIdentifier_NullOrWhitespace_Throws(string id)
134+
{
135+
Assert.ThrowsAny<ArgumentException>(() => MicrosoftAccount.FromIdentifier(id));
136+
}
137+
}
73138
}

src/shared/Core/Authentication/MicrosoftAuthentication.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,55 @@ internal static MicrosoftAccount FromMsalAccount(IAccount msalAccount)
184184
return new MicrosoftAccount(msalAccount.HomeAccountId.Identifier, msalAccount.Username);
185185
}
186186

187+
/// <summary>
188+
/// Construct an account from a single identifier whose shape isn't known by the caller
189+
/// (typically supplied by the user via URL credential-helper input, the
190+
/// <c>credential.username</c> git-config key, or a command-line argument). The identifier
191+
/// is structurally classified and placed in the matching slot:
192+
/// <list type="bullet">
193+
/// <item><description>An MSAL <see cref="HomeAccountId"/> shape
194+
/// (<c>&lt;object-id&gt;.&lt;tenant-id-guid&gt;</c>) is placed in
195+
/// <see cref="HomeAccountId"/>.</description></item>
196+
/// <item><description>Anything else is placed in <see cref="UserName"/>. This includes
197+
/// well-formed UPNs (<c>local@domain</c>), ADFS-style HomeAccountIds (which
198+
/// carry no tenant id and are indistinguishable from a bare username), and
199+
/// unrecognised values — MSAL's UPN-fallback resolution then either
200+
/// fuzzy-matches it or rejects it.</description></item>
201+
/// </list>
202+
/// The heuristics never reject an input. Throws <see cref="ArgumentException"/> only when
203+
/// the identifier itself is null or whitespace.
204+
/// </summary>
205+
public static MicrosoftAccount FromIdentifier(string identifier)
206+
{
207+
EnsureArgument.NotNullOrWhiteSpace(identifier, nameof(identifier));
208+
209+
return IsHomeAccountIdShape(identifier)
210+
? new MicrosoftAccount(homeAccountId: identifier, userName: null)
211+
: new MicrosoftAccount(homeAccountId: null, userName: identifier);
212+
}
213+
214+
/// <summary>
215+
/// True when <paramref name="value"/> structurally matches an MSAL Microsoft Entra
216+
/// <c>HomeAccountId</c>: an <c>&lt;object-id&gt;.&lt;tenant-id&gt;</c> pair where the
217+
/// <c>tenant-id</c> suffix is a well-formed RFC 4122 GUID (a strong invariant for
218+
/// Entra ID) and the <c>object-id</c> prefix is non-empty.
219+
/// </summary>
220+
/// <remarks>
221+
/// Splits on the last <c>.</c> to mirror <c>Microsoft.Identity.Client.AccountId.ParseFromString</c>,
222+
/// which permits an object id to itself contain dots in edge scenarios (B2C, some
223+
/// guest accounts). Object ids are not required to be GUIDs; only the tenant id
224+
/// is checked. Does not validate that the resulting pair refers to a real account,
225+
/// and intentionally does not recognise the ADFS shape (single token, no dot) since
226+
/// it is indistinguishable from a bare username.
227+
/// </remarks>
228+
public static bool IsHomeAccountIdShape(string value)
229+
{
230+
if (string.IsNullOrWhiteSpace(value)) return false;
231+
int dot = value.LastIndexOf('.');
232+
if (dot <= 0 || dot >= value.Length - 1) return false;
233+
return Guid.TryParse(value.AsSpan(dot + 1), out _);
234+
}
235+
187236
public MicrosoftAccount(string homeAccountId, string userName)
188237
{
189238
UserName = userName;

0 commit comments

Comments
 (0)