Entra ID integration API proposal for discussion#14668
Entra ID integration API proposal for discussion#14668jmprieur wants to merge 11 commits intomicrosoft:mainfrom
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14668Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14668" |
…be mandatory in the EntraIdStoreCertificateCredentials
…anagedCertificate creds
IEvangelist
left a comment
There was a problem hiding this comment.
This looks really good so far. A few minor nits, while I looked at the code quick.
Co-authored-by: David Pine <david.pine@microsoft.com>
There was a problem hiding this comment.
Pull request overview
This PR proposes a new Aspire Hosting integration surface for modeling Microsoft Entra ID application registrations in the AppHost model, enabling other “surfaces” (provisioning skill/MCP/CLI) to consume a single source of truth.
Changes:
- Adds
EntraIdApplicationResource+ credential subtypes to represent Entra ID app registration configuration in the application model. - Adds
EntraIdResourceExtensionsfluent APIs (AddEntraIdApplication,AsExisting, credential helpers, andWithReferenceenv-var injection). - Introduces a new hosting integration README and a new test project with builder/unit tests; wires projects into
Aspire.slnx.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting.Azure.EntraId/EntraIdApplicationResource.cs | New resource + credential model types for Entra ID app registrations. |
| src/Aspire.Hosting.Azure.EntraId/EntraIdResourceExtensions.cs | New fluent builder APIs and WithReference env-var injection logic. |
| src/Aspire.Hosting.Azure.EntraId/README.md | New hosting integration README describing intended usage. |
| src/Aspire.Hosting.Azure.EntraId/Aspire.Hosting.Azure.EntraId.csproj | New packable hosting integration project + InternalsVisibleTo for tests. |
| tests/Aspire.Hosting.Azure.EntraId.Tests/EntraIdResourceBuilderTests.cs | New unit tests for resource/builder behavior. |
| tests/Aspire.Hosting.Azure.EntraId.Tests/Aspire.Hosting.Azure.EntraId.Tests.csproj | New test project referencing the integration package and shared hosting tests infra. |
| Aspire.slnx | Adds the new project and test project to the solution. |
|
|
||
| if (DistinguishedName is not null) |
There was a problem hiding this comment.
EmitEnvironmentVariables will emit a thumbprint when set, and (in the following block) also emits a distinguished name when set. If both properties are set, both values will be emitted even though only one identifier should be used for the selected SourceType. Consider validating/normalizing so only the intended identifier is emitted.
| if (DistinguishedName is not null) | |
| else if (DistinguishedName is not null) |
| public static IResourceBuilder<EntraIdApplicationResource> WithClientSecret( | ||
| this IResourceBuilder<EntraIdApplicationResource> builder, | ||
| IResourceBuilder<ParameterResource> clientSecret) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(builder); |
There was a problem hiding this comment.
WithClientSecret accepts any ParameterResource, but this value represents a credential and will be injected into consuming services. Other integrations validate that sensitive inputs are marked secret: true (e.g., OpenAI WithApiKey). Consider checking clientSecret.Resource.Secret and throwing if it isn’t secret to prevent accidental exposure of non-secret parameters as credentials.
| public void WithReference_InjectsEnvironmentVariables() | ||
| { | ||
| using var appBuilder = TestDistributedApplicationBuilder.Create(); | ||
|
|
||
| var secret = appBuilder.AddParameter("EntraSecret", secret: true); |
There was a problem hiding this comment.
WithReference_InjectsEnvironmentVariables only asserts that an EnvironmentCallbackAnnotation exists, but it doesn’t execute the callback or verify any emitted keys/values. Consider invoking the callback(s) with an EnvironmentCallbackContext (as done in other hosting tests) and asserting on a representative set of variables so regressions in the env-var mapping are caught.
| throw new InvalidOperationException( | ||
| $"Either {nameof(Thumbprint)} or {nameof(DistinguishedName)} must be set on {nameof(EntraIdStoreCertificateCredential)}."); | ||
| } | ||
|
|
||
| return Thumbprint is not null ? "StoreWithThumbprint" : "StoreWithDistinguishedName"; |
There was a problem hiding this comment.
EntraIdStoreCertificateCredential is documented as “set either Thumbprint or DistinguishedName (not both)”, but SourceType currently picks StoreWithThumbprint whenever Thumbprint is set even if DistinguishedName is also set. Consider throwing when both are set (or otherwise enforcing mutual exclusivity) to avoid silently producing inconsistent configuration.
| public class EntraIdApplicationResource(string name, string configSectionName = "AzureAd") | ||
| : Resource(name), IResourceWithEnvironment | ||
| { | ||
| private const string DefaultInstance = "https://login.microsoftonline.com/"; | ||
|
|
There was a problem hiding this comment.
EntraIdApplicationResource is public and can be constructed directly with a configSectionName, but there’s no validation to prevent null/empty values. If ConfigSectionName is null/empty, WithReference will emit malformed environment variable keys. Consider validating configSectionName in the constructor so the resource is always in a valid state.
| public static IResourceBuilder<EntraIdApplicationResource> WithCertificateThumbprint( | ||
| this IResourceBuilder<EntraIdApplicationResource> builder, | ||
| string storePath, | ||
| string thumbprint) | ||
| { |
There was a problem hiding this comment.
WithCertificateThumbprint introduces behavior/config shape but there’s no unit test validating the resulting EntraIdStoreCertificateCredential state (including SourceType, StorePath, and Thumbprint). Adding a test similar to the distinguished-name test would help prevent regressions (especially given the SourceType selection logic).
| In your service projects, install Microsoft.Identity.Web: | ||
|
|
||
| ```dotnetcli | ||
| dotnet add package Microsoft.Identity.Web | ||
| ``` |
There was a problem hiding this comment.
This hosting integration README includes client-project guidance (installing Microsoft.Identity.Web). The repo’s hosting integration README guidance typically keeps these READMEs focused on AppHost usage (Add* / WithReference) and links out for client integration details. Consider moving this client-package installation guidance to a client integration README or “Additional documentation”.
| In the API project's _Program.cs_, use Microsoft.Identity.Web directly — the configuration is automatically available via the `AzureAd` section: | ||
|
|
||
| ```csharp | ||
| builder.Services.AddAuthentication() | ||
| .AddMicrosoftIdentityWebApi(builder.Configuration.GetSection("AzureAd")); | ||
| ``` |
There was a problem hiding this comment.
The README includes service Program.cs authentication wiring examples (AddMicrosoftIdentityWebApi / AddMicrosoftIdentityWebApp). For hosting integration READMEs, we generally try to avoid DI/container setup details and instead focus on the AppHost surface and what WithReference injects. Consider moving these snippets to a dedicated client integration doc and keep this README centered on the AppHost model API.
| internal List<EntraIdClientCredential> ClientCredentials { get; } = []; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the client capabilities (e.g., <c>"cp1"</c> for Continuous Access Evaluation). |
There was a problem hiding this comment.
The XML doc for ClientCapabilities says “Gets or sets…”, but the property is getter-only. Consider updating the summary to “Gets the client capabilities…” to keep the docs accurate.
| /// Gets or sets the client capabilities (e.g., <c>"cp1"</c> for Continuous Access Evaluation). | |
| /// Gets the client capabilities (e.g., <c>"cp1"</c> for Continuous Access Evaluation). |
Description
AppHost model to integrate Entra ID applications. This is the source of truth that all surfaces read (provisioning Skill, MCP, Aspire CLI). It creates the model that makes intelligent provisioning possible.
classDiagram class Resource { <<Aspire.Hosting>> +string Name } class IResourceWithEnvironment { <<interface>> } class EntraIdApplicationResource { +string ConfigSectionName +string Instance +ParameterResource? TenantIdParameter +string? TenantId +ParameterResource? ClientIdParameter +string? ClientId +string? AppHomeTenantId +string? AzureRegion +bool AllowWebApiToBeAuthorizedByACL ~List~EntraIdClientCredential~ ClientCredentials ~List~string~ ClientCapabilities ~List~string~ Audiences ~Dictionary~string,string~ ExtraQueryParameters } class EntraIdClientCredential { <<abstract>> +string SourceType* ~EmitEnvironmentVariables()* } class EntraIdClientSecretCredential { +SourceType = "ClientSecret" +ParameterResource ClientSecret } class EntraIdFederatedIdentityCredential { +SourceType = "SignedAssertionFromManagedIdentity" +string? ManagedIdentityClientId +string? TokenExchangeUrl +string? TokenExchangeAuthority } class EntraIdKeyVaultCertificateCredential { +SourceType = "KeyVault" +string KeyVaultUrl +string CertificateNameInKeyVault } class EntraIdStoreCertificateCredential { +SourceType = "StoreWith..." +string StorePath +string? Thumbprint +string? DistinguishedName } class EntraIdFileCertificateCredential { +SourceType = "Path" +string FilePath +string? Password } class EntraIdSignedAssertionFileCredential { +SourceType = "SignedAssertionFilePath" +string? FilePath } class EntraIdResourceExtensions { <<static>> +AddEntraIdApplication()$ +AsExisting(tenantId, clientId)$ +WithClientSecret()$ +WithFicMsi()$ +WithCertificateFromKeyVault()$ +WithCertificateThumbprint()$ +WithCertificateDistinguishedName()$ +WithCredential()$ +WithInstance()$ +WithAppHomeTenantId()$ +WithClientCapability()$ +WithAzureRegion()$ +WithAllowWebApiToBeAuthorizedByACL()$ +WithExtraQueryParameter()$ +WithAudience()$ +WithEntraIdAuthentication()$ } Resource <|-- EntraIdApplicationResource IResourceWithEnvironment <|.. EntraIdApplicationResource EntraIdApplicationResource *-- "0..*" EntraIdClientCredential : ClientCredentials EntraIdClientCredential <|-- EntraIdClientSecretCredential EntraIdClientCredential <|-- EntraIdFederatedIdentityCredential EntraIdClientCredential <|-- EntraIdKeyVaultCertificateCredential EntraIdClientCredential <|-- EntraIdStoreCertificateCredential EntraIdClientCredential <|-- EntraIdFileCertificateCredential EntraIdClientCredential <|-- EntraIdSignedAssertionFileCredential EntraIdResourceExtensions ..> EntraIdApplicationResource : creates & configures note for EntraIdResourceExtensions "WithReference() injects\nenv vars like AzureAd__TenantId\ninto consuming services" note for EntraIdClientCredential "Each subtype emits its own\nenv vars via EmitEnvironmentVariables()"Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: