-
Notifications
You must be signed in to change notification settings - Fork 406
Update proposal document for GetManagedIdentityCapabilitiesAsync #6040
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+220
−0
Merged
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
525eaf3
Update proposal document for GetManagedIdentityCapabilitiesAsync
gladjohn b184f3d
Update proposal-capabilities-and-minstrength.md
gladjohn 5ca4ec6
Address PR #6040 review feedback
gladjohn 9dfbad4
Keep IsMtlsPopSupportedByHost on ManagedIdentityCapabilities
gladjohn 254eb3b
MtlsBindingStrength lives in Microsoft.Identity.Client.AppConfig
gladjohn c7ef9d0
Adopt WithMtlsProofOfPossession(PoPOptions) per Bogdan's suggestion
gladjohn d7b22e7
Update spec to agreed plan: phased rollout, derived bool, ImdsV2->Imds
Robbie-Microsoft 8204c71
Merge branch 'main' into gladjohn-patch-17
Robbie-Microsoft 2d16e4b
Merge branch 'main' into gladjohn-patch-17
Robbie-Microsoft baf878b
Potential fix for pull request finding
gladjohn a33cac6
Merge branch 'main' into gladjohn-patch-17
gladjohn 0aa0902
Address Bogdan review: drop history/phasing, add superseded-names app…
GladwinJohnson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| # Proposal: `GetManagedIdentityCapabilitiesAsync` + `WithMinStrength` | ||
|
|
||
| **Status:** Proposal | ||
| **Related:** host capability detection | ||
|
|
||
| --- | ||
|
|
||
| ## Why | ||
|
|
||
| `ManagedIdentityApplication.GetManagedIdentitySourceAsync` is already a shipped public API. Its return type was meant to be a thin wrapper over `ManagedIdentitySource` — but it's grown: | ||
|
|
||
| - **today:** `Source` + `ImdsV1FailureReason` + `ImdsV2FailureReason` | ||
| - **after PR #6026:** + `IsMtlsPopSupportedByHost` | ||
| - **next:** + `SupportedBindingStrengths` (for the MinStrength feature) | ||
|
|
||
| The method name `GetManagedIdentitySource…` no longer describes what callers get back. Callers using it for capability decisions (Azure SDK, AKV SDK) read it as "tell me what this host can do," which is what we want — and the name should match. | ||
|
|
||
| Separately, service teams (Dragos's ask) need a way to **assert** the minimum binding strength their app requires, so a CVM/TVM app accidentally deployed on a software-only host fails at build/request time instead of silently downgrading. | ||
|
gladjohn marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## What | ||
|
|
||
| ### 1. New discovery API (capability-shaped name) | ||
|
|
||
| ```csharp | ||
| namespace Microsoft.Identity.Client.ManagedIdentity | ||
| { | ||
| public class ManagedIdentityApplication : IManagedIdentityApplication | ||
| { | ||
| public Task<ManagedIdentityCapabilities> GetManagedIdentityCapabilitiesAsync( | ||
|
gladjohn marked this conversation as resolved.
Outdated
|
||
| CancellationToken cancellationToken); | ||
| } | ||
|
gladjohn marked this conversation as resolved.
Outdated
gladjohn marked this conversation as resolved.
|
||
|
|
||
| public class ManagedIdentityCapabilities | ||
| { | ||
| public ManagedIdentitySource Source { get; } | ||
|
|
||
| // Existing probe-failure detail (carried over) | ||
| public string ImdsV1FailureReason { get; set; } | ||
|
gladjohn marked this conversation as resolved.
Outdated
|
||
| public string ImdsV2FailureReason { get; set; } | ||
|
|
||
| // Host capability surface | ||
| public bool IsMtlsPopSupportedByHost { get; } | ||
| public IReadOnlyList<MtlsBindingStrength> SupportedBindingStrengths { get; } | ||
|
gladjohn marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| public enum MtlsBindingStrength | ||
| { | ||
| Bearer = 0, // .NET 4.6.2 (no PoP) | ||
| EphemeralSoftware = 1, // in-memory RSA, process lifetime | ||
|
gladjohn marked this conversation as resolved.
Outdated
|
||
| // 2 reserved for future (TPM-backed, etc.) | ||
| KeyGuard = 3, // VBS-isolated (CVM/TVM with VBS enabled) | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### 2. Old API stays, marked Obsolete | ||
|
|
||
| ```csharp | ||
| [Obsolete( | ||
|
gladjohn marked this conversation as resolved.
Outdated
|
||
| "Use GetManagedIdentityCapabilitiesAsync instead. " + | ||
| "The returned ManagedIdentityCapabilities exposes the same Source plus host capability.", | ||
| error: false)] | ||
| public Task<ManagedIdentitySourceResult> GetManagedIdentitySourceAsync( | ||
| CancellationToken cancellationToken); | ||
| ``` | ||
|
|
||
| - `ManagedIdentitySourceResult` continues to exist for back-compat; no new members added to it. | ||
| - Implementation of the obsolete method delegates to the new one and projects the result. | ||
|
|
||
| ### 3. `WithMinStrength` — floor assertion on the request | ||
|
gladjohn marked this conversation as resolved.
Outdated
|
||
|
|
||
| ```csharp | ||
| public static class ManagedIdentityPopExtensions | ||
| { | ||
| // Existing | ||
| public static AcquireTokenForManagedIdentityParameterBuilder WithMtlsProofOfPossession( | ||
| this AcquireTokenForManagedIdentityParameterBuilder builder); | ||
|
|
||
| // New | ||
| public static AcquireTokenForManagedIdentityParameterBuilder WithMinStrength( | ||
| this AcquireTokenForManagedIdentityParameterBuilder builder, | ||
| MtlsBindingStrength minStrength); | ||
| } | ||
|
|
||
| // Symmetry for confidential client | ||
| public class AcquireTokenForClientParameterBuilder | ||
| { | ||
| public AcquireTokenForClientParameterBuilder WithMtlsProofOfPossession(); // existing | ||
| public AcquireTokenForClientParameterBuilder WithMinStrength(MtlsBindingStrength); // new | ||
| } | ||
|
Robbie-Microsoft marked this conversation as resolved.
|
||
| ``` | ||
|
|
||
| #### Semantics — floor, not selector | ||
|
|
||
| | Host actual | `WithMinStrength(...)` | Behavior | | ||
| |---|---|---| | ||
| | KeyGuard available | `KeyGuard` | Uses KeyGuard. ✅ | | ||
| | KeyGuard available | `EphemeralSoftware` | Uses KeyGuard (host max, never downgrades). | | ||
| | Software only | `KeyGuard` | **Throws `MsalClientException` at AcquireToken.** | | ||
| | Software only | `EphemeralSoftware` | Uses ephemeral software. ✅ | | ||
| | .NET 4.6.2 | any non-Bearer | **Throws.** | | ||
| | Not chained with `WithMtlsProofOfPossession()` | any | **Throws** — `WithMinStrength` only meaningful with PoP. | | ||
|
|
||
| - `WithMinStrength` is a **floor assertion**, never a downgrade dial. | ||
| - MSAL always picks the host's max binding strength; `WithMinStrength` just adds a tripwire that fails the request if the host can't meet the floor. | ||
| - Failure is `MsalClientException` with a new error code: `MsalError.MinStrengthNotMet`. | ||
|
|
||
| ## Why this shape | ||
|
|
||
| - **Name reflects content.** `GetManagedIdentityCapabilities` is honest about returning a capability bag, not just a source enum. | ||
|
gladjohn marked this conversation as resolved.
|
||
| - **No silent downgrade.** A CVM-bound service that asserts `KeyGuard` will fail fast on the wrong host. No production drift. | ||
| - **No developer downgrade lever.** There's no way to use `WithMinStrength` to opt for *less* binding than the host provides. The dial only enforces a floor. | ||
| - **Discovery + enforcement composed cleanly:** call `GetManagedIdentityCapabilitiesAsync` to inspect, then `WithMinStrength` to enforce. | ||
|
|
||
| ## Sample usage | ||
|
|
||
| ```csharp | ||
| var mi = ManagedIdentityApplicationBuilder | ||
| .Create(ManagedIdentityId.SystemAssigned) | ||
| .Build(); | ||
|
|
||
| // 1. Inspect what this host can do (optional — for diagnostics / DefaultAzureCredential chain) | ||
| var caps = await ((ManagedIdentityApplication)mi) | ||
| .GetManagedIdentityCapabilitiesAsync(ct); | ||
|
|
||
|
Robbie-Microsoft marked this conversation as resolved.
|
||
| if (!caps.SupportedBindingStrengths.Contains(MtlsBindingStrength.KeyGuard)) | ||
| { | ||
| logger.LogWarning("This host cannot meet KeyGuard requirement; service will refuse to start."); | ||
| } | ||
|
gladjohn marked this conversation as resolved.
|
||
|
|
||
| // 2. Assert the floor when acquiring | ||
| var result = await mi.AcquireTokenForManagedIdentity("https://vault.azure.net/.default") | ||
| .WithMtlsProofOfPossession() | ||
| .WithMinStrength(MtlsBindingStrength.KeyGuard) // throws if host can't deliver | ||
| .ExecuteAsync(ct); | ||
| ``` | ||
|
|
||
| ## API surface impact | ||
|
|
||
| `PublicAPI.Unshipped.txt` additions: | ||
| - `ManagedIdentityApplication.GetManagedIdentityCapabilitiesAsync(CancellationToken)` | ||
| - `ManagedIdentityCapabilities` (class + members) | ||
| - `MtlsBindingStrength` (enum + members) | ||
| - `ManagedIdentityPopExtensions.WithMinStrength(...)` | ||
| - `AcquireTokenForClientParameterBuilder.WithMinStrength(...)` | ||
| - `MsalError.MinStrengthNotMet` | ||
|
|
||
| `PublicAPI.Shipped.txt` change: | ||
| - `GetManagedIdentitySourceAsync` gets `[Obsolete]` (does not remove from shipped surface). | ||
|
|
||
| No binary-breaking changes. No semver bump. | ||
|
|
||
| ## Acceptance | ||
|
|
||
| - [ ] `MtlsBindingStrength` enum added. | ||
| - [ ] `ManagedIdentityCapabilities` class added. | ||
| - [ ] `GetManagedIdentityCapabilitiesAsync` added; populates `SupportedBindingStrengths` by combining IMDS hint + KeyGuard probe + .NET TFM. | ||
| - [ ] Old `GetManagedIdentitySourceAsync` marked `[Obsolete]` (non-error), delegates to new method. | ||
| - [ ] `WithMinStrength` added on `AcquireTokenForManagedIdentityParameterBuilder` and `AcquireTokenForClientParameterBuilder`. | ||
| - [ ] `MsalError.MinStrengthNotMet` thrown when host can't meet floor; error message names host actual strength + required floor. | ||
| - [ ] `WithMinStrength` without `WithMtlsProofOfPossession` throws at request time. | ||
| - [ ] Tests cover: KeyGuard host passes floor, software host fails floor, .NET 4.6.2 fails floor, ConfClient parity. | ||
|
|
||
| ## Open questions for the thread | ||
|
|
||
| 1. Bogdan — keep `IsMtlsPopSupportedByHost` (boolean) on the new `ManagedIdentityCapabilities`, or drop it in favor of `SupportedBindingStrengths.Count > 0`? | ||
| 2. Dragos — does `WithMinStrength` as a floor-only assertion meet the no-downgrade goal? Or do you want the discovery API alone (no enforcement helper)? | ||
| 3. Should `MtlsBindingStrength` live in `Microsoft.Identity.Client.ManagedIdentity` or `Microsoft.Identity.Client.AppConfig`? It's shared by both MI and ConfClient. | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.