Enable Microsoft Entra ID sign-in for Private Marketplace access

[Michael Cummings (MSFT) (mcumming)]

Addresses #280376 — Enable VS Code sign-in with Microsoft Entra ID to connect to Private Marketplace.

Why

Today a configured Private Marketplace (extensions.gallery.serviceUrl) is gated only through the GitHub/default account path (enterprise flag or entitlement SKU). Customers who manage identity in Microsoft Entra ID have no way to sign in and be authorized against their marketplace. This change makes marketplace access provider-aware and adds a new Microsoft/Entra path with server-enforced eligibility, so an Entra-signed-in user who is verified as eligible gets the configured marketplace as their active Extension Gallery.

What changes

The new path is entirely behind a product flag (product.json → enableExtensionGalleryEntraAuth) and a per-marketplace setting/policy (extensions.gallery.authProvider, values github | microsoft). When the flag is off, microsoft is coerced back to github, so existing behavior is untouched.

• Provider routing (extensionGalleryManifestService.ts): getEffectiveAuthProvider() selects the access strategy. The GitHub path is preserved as-is; a new Microsoft path acquires an existing Entra session silently (getSessions, never prompts), reads the service index, discovers the manifest-advertised EligibilityService endpoint, and POSTs the token to it. The server's boolean verdict decides access.
• Access + error UX (extensionsViewlet.ts, extensions.contribution.ts, extensions.ts): provider-aware welcome view and activity badge for the new marketplace states, plus a provider-routed sign-in command (microsoft → Entra createSession, otherwise the existing default-account sign-in). A CONTEXT_MARKETPLACE_AUTH_PROVIDER context key backs the routing.
• Status model (extensionGalleryManifest.ts): adds Misconfigured and Unreachable states (alongside RequiresSignIn / AccessDenied), the EligibilityService resource type, PRIVATE_MARKETPLACE_SCOPES, the authProvider config key, and a typed MarketplaceAuthRequiredError.
• Policy / product (policyData.jsonc, product.ts, product.json): registers the authProvider admin policy and adds microsoft to trustedExtensionAuthAccess.

Security and correctness

• The Entra token is only ever sent to an HTTPS, exact same-origin target as the admin-configured service index (isSafeTokenTarget), so a compromised/misconfigured manifest can't redirect the token to a foreign or cleartext origin.
• Token-bearing requests never follow redirects (the request service would forward the Authorization header across hops).
• 401 vs 403 are distinct: 401 (missing/expired/wrong-audience token) → RequiresSignIn, never cached; 403 (identity accepted but forbidden) → AccessDenied, cached as ineligible.
• The cached verdict is scoped to authProvider + accountId + serviceUrl and dropped on any mismatch, so a stale allow/deny can't leak across accounts or marketplaces.
• A monotonic validation epoch guards every await, so a session/account/config change mid-validation supersedes an in-flight result instead of racing it.
• Transient failures (auth service down, marketplace unreachable, non-2xx, or a 200 that isn't a valid manifest) surface Unreachable rather than leaving a configured marketplace on a blank view or throwing.

Testing

• 45 unit tests in extensionGalleryManifestService.test.ts covering provider routing, the eligibility handshake, cache scoping/invalidation, 401/403/5xx/malformed classification, and the epoch race paths.
• npm run compile-check-ts-native, the gallery unit suite, and npm run valid-layers-check all pass.

[Copilot]

Pull request overview

Adds provider-aware Private Marketplace authentication using Microsoft Entra ID, including eligibility validation, secure token transport, caching, status UX, and policy configuration.

Changes:

• Adds Microsoft session and eligibility-service authentication.
• Adds provider-aware sign-in, marketplace statuses, and error UX.
• Adds configuration policy, product gating, and unit coverage.

Show a summary per file

File	Description
extensionGalleryManifestService.test.ts	Tests routing, eligibility, caching, and races.
extensionGalleryManifestService.ts	Implements authentication and eligibility flow.
extensions.ts	Defines marketplace provider context.
extensionsViewlet.ts	Adds status-specific marketplace UX.
extensions.contribution.ts	Registers policy and sign-in action.
extensionGalleryManifest.ts	Adds statuses, resource type, and scopes.
product.ts	Defines the Entra product gate.
product.json	Adds Microsoft authentication-provider metadata.
policyData.jsonc	Exports the new enterprise policy.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 8
• Review effort level: Medium

[joshspicer]

Please consider writing a TPI for us to validate these changes during testing https://github.com/microsoft/vscode/wiki/Writing-Test-Plan-Items

[joshspicer]

External contributors will be blocked from updating policy. After this lands, follow up with adding back the policy: #328982

[Sandeep Somavarapu (sandy081)]

Here's my first turn of comments. Since its a big PR - I would like to go service by service to keep services clean

[Sandeep Somavarapu (sandy081)]

May I know what you are trying to change here? What does the comment mean? Is this needed for this PR?

[Michael Cummings (MSFT) (mcumming)]

Fair question — and it's stale. Removed in cec950d.

It was needed when it was added (bd44656). At that point this service performed the Entra access check itself and resolved authentication through a lazy getter:

this.instantiationService.invokeFunction(accessor => accessor.get(IAuthenticationService))

Because the constructor called getExtensionGalleryManifest() synchronously, the Microsoft path hit that getter mid-construction and threw RECURSIVELY instantiating service 'IAuthenticationService', which locked the workbench on startup. Deferring the bootstrap to a microtask let the DI container finish caching this service first.

That is no longer how it works. af73002 moved access resolution into ExtensionGalleryAccountService and dropped authentication from that service graph — IAuthenticationService is now handed over after startup by ExtensionGalleryAccountAuthenticationContribution (WorkbenchPhase.AfterRestored), and the account service reports "no account" until then. Nothing reachable from this constructor can resolve authentication anymore, so the deferral was dead code.

Verified rather than assumed: launched a configured private marketplace (extensions.gallery.serviceUrl + authProvider: microsoft) with and without the deferral on a clean build. Both start normally, no recursion error, identical [Marketplace] trace output.

The same commit also corrects the comment above galleryAccountService, which was stale in the same way — it described an IAuthenticationService dependency that no longer exists and pointed at the deferral removed here.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

This service seems to be doing too much. It should use marketplace account service and ask for a vlaid account it can use for private marketplace. All access / entitlement checks should be done by the marketplace account service. Also there should be an event on the account service when account has changed.

[Michael Cummings (MSFT) (mcumming)]

Agreed. Restructured in bbfd837.

The manifest service was orchestrating access resolution rather than consuming it — driving the cached-then-live resolution, owning validation generations and cancellation tokens, and reaching in to clear caches. That's all in the account service now; the manifest service only maps verdict → ExtensionGalleryManifestStatus.

Entitlement checks were already in the account service — what leaked was the orchestration around them. The account-change event existed but was Event<void>, which is what pushed generations onto the host; it's now onDidChangeAccess: Event<IExtensionGalleryAccessVerdict> and carries the result.

readonly onDidChangeAccess: Event<IExtensionGalleryAccessVerdict>;
resolveAccess(configuredServiceUrl: string): Promise<IExtensionGalleryAccessVerdict>;
setPreferredAccount(accountId: string): void;
reset(): void;
connectAuthentication(authenticationService: IAuthenticationService): void;

getAccount / getCachedAccess / clearCache are private, invalidateServiceIndexCache is gone, and callers no longer pass CancellationTokens. 272 → 203 lines, with the access-validation section down from ~125 to 40. No behaviour change; existing tests pass.

Testing against a live private marketplace also turned up a regression, fixed separately:

• 7c18968 — non-2xx responses reported only a status code, so a marketplace that explains the rejection in its body looked like an unreachable network. Body now included.
• 8da36d1 — a marketplace refusing the client outright (mine returns 400 "Only VS Code clients version 1.104.2 or later are allowed") was classified as transient and shown as "check your network connection". main reports AccessDenied for a failed fetch, so this branch was regressing the guidance. Non-401/403 4xx now maps to a denial; 5xx and network failures stay Unreachable.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

please reduce verbose comments

[Michael Cummings (MSFT) (mcumming)]

Applied across the PR, not just this file — 61ac56e, −107 comment lines over all seven files it touches.

I kept rationale only where the code can't show it: the service DI cycle, why an ineligible account is still returned, why only a 403 is persisted, and why a bearer is confined to a same-origin HTTPS target. Everything that restated the code is gone.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Why is this reset needed? What is the scenario?

[Michael Cummings (MSFT) (mcumming)]

Scenario: extensions.gallery.serviceUrl or authProvider changes at runtime. That prompts for a restart, but the prompt is dismissable — so the process can keep running against the old configuration. Without superseding, an in-flight resolution can land afterwards and publish a manifest for the marketplace we just moved away from.

reset() is gone in 082b6c7. Most of what it did belongs to the manifest service now (cancel the resolution, drop the memoized index, clear the cached verdict), so it is explicit at the config-change site instead of a method on the account service.

Testing this also turned up a real bug: the config-change listener was registered after the initial resolution, so with a slow index fetch a change during startup was missed entirely. Fixed in the same commit.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Updating this context key should be moved to marketplace account service.

[Michael Cummings (MSFT) (mcumming)]

Moved in 082b6c7. It also removes a duplicate getEffectiveAuthProvider call — both services were computing the effective provider independently; now only the account service does.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Marketplace account service should just provide account verdict and should not return manfiest. Updating the manifest should be done by the manifest service itself

[Michael Cummings (MSFT) (mcumming)]

Agreed, fixed in 082b6c7. The account service no longer fetches the service index or returns a manifest; it reports accountStatus and hands back the account (with its token on the Entra path). The manifest service does the fetching and publishing.

One shape note: getAccount() returns the signed-in account even when it isn't entitled, so the caller can scope a durable denial to that account id. accountStatus is what says whether it may be used — the bearer is withheld when ineligible.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Why does account service need service URL for?

[Michael Cummings (MSFT) (mcumming)]

It doesn't — that was a consequence of it fetching the index. Entitlement is decided client-side (accessSKUs/enterprise flag for GitHub, the token's tid claim for Entra) and never used the URL. Removed in 082b6c7.

The one thing that genuinely was URL-scoped is the durable access verdict, which must not be reused after an admin repoints the client at a different marketplace. Rather than push that into either service, it now lives in a small ExtensionGalleryAccessCache that owns the storage key and the scoping rules.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Why do we need these many methods in this service? I would expect just following

• accountStatus
• onDidChangeAccountStatus
• getAccount
• onDidChangeAccount

[Michael Cummings (MSFT) (mcumming)]

Trimmed to identity and entitlement — 805 lines down to 438. serviceUrl, the cancellation token, index fetching, storage, and the manifest itself all moved out.

What's left is accountStatus + onDidChangeAccountStatus, getAccount() + onDidChangeAccount, and setPreferredAccount/connectAuthentication. The last two would be private if the sign-in command and the auth contribution didn't call them from outside.

Two deviations worth flagging:

• accountStatus has a fourth state, Unknown, for a transient auth failure. Without it a network blip is indistinguishable from a real SignedOut and the caller drops the cached verdict.
• getAccount() still returns the account when it's ineligible (with no bearer token). The caller needs the identity to scope a durable denial to it.

Happy to collapse either if you'd rather.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Why do we need a separate class to get service manifest. Retain the old method

[Michael Cummings (MSFT) (mcumming)]

The class existed only to memoize fetched indexes, and that memo could never be hit — resolve() called invalidate() immediately before the single getServiceIndex() call. With no state left it goes back to a private getExtensionGalleryManifestFromServiceUrl, and the file is deleted.

Carried over: the bearer header, followRedirects: 0 when a token is attached, and 401/403 typed apart from other 4xx.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Why do you need this access cache and what is it serving?

[Michael Cummings (MSFT) (mcumming)]

Removed, along with its storage key.

It wasn't serving much: write(…, true) was never read — the only read compared === false. And the false written for a client-side ineligible account outlived the condition it recorded. Grant that same account entitlement and the stale denial short-circuited, leaving them AccessDenied until they signed out. There is now a test for that case; it failed before this change and passes after.

The only real benefit left was skipping a re-probe after a server 403, which isn't worth a storage key.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

why are you trying to change this restart behaviour? Can this be left as before?

[Michael Cummings (MSFT) (mcumming)]

Yes — restored to main's version: registered after the initial resolution, and it only calls requestRestart(). The one addition is ExtensionGalleryAuthProviderConfigKey in the condition, since changing provider changes which identity gates access. The teardown went away with the memo and the cache.

One thing I did keep: the account listener is still registered before the first resolution. Moving it after (matching main) broke a test — a sign-out while the index fetch is in flight has no listener to supersede it, so the status never leaves Unavailable.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

please avoid verbose comments

[Michael Cummings (MSFT) (mcumming)]

These files were 24% comment lines against 4% in the surrounding extensionManagement code. They are now 8%, and what remains explains a constraint rather than restating the code.

This block is gone with the teardown.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

What is the purpose of isSafeTokenTarget method? Can you please explain what scenarios we are trying to handle by checking this?

[Michael Cummings (MSFT) (mcumming)]

The intent was to never attach a bearer to a URL that isn't HTTPS and isn't the admin-configured origin, so a tampered service index couldn't redirect the token somewhere else.

It didn't do that. The one call site passed the same URL for both arguments, so the same-origin half was always true and only the HTTPS check ran — on a URL we already control. Redirects are handled by followRedirects: 0 on the authenticated request.

Dropped for an inline HTTPS guard. MarketplaceMisconfiguredError beside it was never thrown — gone too.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

this method name seems misleading - please rename it appropriately

[Michael Cummings (MSFT) (mcumming)]

Renamed to setAvailable — it publishes and logs telemetry, renders nothing.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

please reduce noisy comments

[Michael Cummings (MSFT) (mcumming)]

Same pass as the other thread — 24% down to 8%, against 4% in the surrounding code.

This one is gone; the telemetry event name already says it.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Michael Cummings (MSFT) (@mcumming) I pushed a change to src/vs/workbench/services/extensionManagement/electron-browser/extensionGalleryManifestService.ts to show how I would expect this service should be. Please go through the changes and let me know if there is anything this service is missing. If so, lets discuss in teams to understand the missing pieces

[Sandeep Somavarapu (sandy081)]

Lets not introduce more status code unless necessay

[Michael Cummings (MSFT) (mcumming)]

Already done — Unreachable and Misconfigured came out in e61ce45, so the enum is back to the four values on main. The UI sites that switched on them went with it.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

move this scopes to product.json just like scopes for github

[Michael Cummings (MSFT) (mcumming)]

Done in a65242b — extensionsGallery.accessScopes, next to accessSKUs.

Followed defaultChatAgent.providerScopes exactly: no in-source fallback, so the product file is the only source. If a deployment turns on the Microsoft path without configuring scopes it now reports no account, rather than requesting a session it can't use.

These are plain OIDC scopes — we take the auth provider's default client id and organizations tenant, so no VSCODE_* overrides are involved. That's all the eligibility check needs: an ID token carrying a tid claim.

One asymmetry worth naming: providerScopes is required in the type, accessScopes can't be — extensionsGallery is itself optional, and requiring it would force scopes on GitHub-path deployments where they're meaningless. The fail-closed check covers that gap.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Avoid hardcoding provider ids. They should be read only at one place which should be gallery accout service. The sign in button should just say Sign In - why does user has to know if it is github or microsoft if they have to sign in anyway - the sign in page anyway shows right

[Michael Cummings (MSFT) (mcumming)]

Done in 3973ede — back to a single status-gated welcome entry, same shape as main.

You were right that the fork bought nothing: both blocks already invoked the same command and differed only in the label. Collapsing them left CONTEXT_MARKETPLACE_AUTH_PROVIDER with no consumers, so the context key is deleted too. The browser layer no longer references either provider id, and it also stopped importing DEFAULT_ACCOUNT_SIGN_IN_COMMAND.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

THere should be just one command for sign in and account service should handle signing in to microsoft or github

[Michael Cummings (MSFT) (mcumming)]

Done in 3973ede. ExtensionGalleryMicrosoftSignInCommandId is gone along with its CommandsRegistry.registerCommand, and the action is now just:

await accessor.get(IExtensionGalleryAccountService).signIn();

GitHub's signIn() delegates to defaultAccountService.signIn(); the Microsoft provider owns its account quick-pick. To make that callable I moved the service interface to services/extensionManagement/common/extensionGalleryAccount.ts, mirroring how IDefaultAccountService splits interface from implementation — the command id only existed because the browser layer couldn't reach electron-browser.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

Why are these hardcoded here? They should be read from product.json.

I would leave Tyler to review getting and checking microsoft account.

[Michael Cummings (MSFT) (mcumming)]

Two things to weigh before moving these, then happy either way.

The same two GUIDs are hardcoded in extensions/microsoft-authentication/src/common/scopeData.ts and cli/src/auth.rs — fixed Microsoft identity values rather than per-deployment config, which is why I matched that.

More importantly, this check decides whether the access token is released, so reading it from product.json would need an explicit closed default: absent values would otherwise make every account look eligible.

Probably the more useful precedent: scopeData.ts shows the customization path for Microsoft auth is the magic scope entries (VSCODE_CLIENT_ID:, VSCODE_TENANT:) rather than product.json, and DEFAULT_TENANT is already organizations, which excludes personal accounts. So this check is defence-in-depth against a session created elsewhere with common, not the only line.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

I would leave this to TylerLeonhardt

[TylerLeonhardt]

These values are well-known and never change. This is totally valid for detecting "not personal microsoft accounts"

[Sandeep Somavarapu (sandy081)]

I do not like this pattern. Instead check DefaultAccountProviderContribution how we handle this cyclic dependencies

[Michael Cummings (MSFT) (mcumming)]

Done in 3973ede — followed that pattern.

connectAuthentication is gone. There's now an IExtensionGalleryAccountProvider with two implementations, and a BlockStartup contribution builds the configured one via createInstance and calls setAccountProvider(...). The service holds no authentication dependency at all now rather than receiving it late, which is strictly better than what I had.

Two things fell out: setPreferredAccount was only ever called by the old sign-in command, so it's private now and off the service interface; and onDidChangeAccountStatus is load-bearing, carrying status from provider to service.

One behaviour change worth flagging: the GitHub path used to resolve at service construction and now also arrives via the contribution. I used BlockStartup to match DefaultAccountProviderContribution, and setAccountProvider fires onDidChangeAccount so anything that resolved early re-resolves.

---

_{🤖 This reply was drafted by an AI agent on behalf of Michael Cummings (MSFT) (@mcumming).}

[Sandeep Somavarapu (sandy081)]

I would request to get review from Tyler for this.

[Sandeep Somavarapu (sandy081)]

Changes look good to me. Thanks for incorporating all feedback.

Please wait for TylerLeonhardt 's feedback on Microsoft account provider implementation and then we are good to merge.

Thanks

[TylerLeonhardt]

Sandeep Somavarapu (@sandy081) you don't wanna align this with the account Settings Sync might be using?

[TylerLeonhardt]

And if this value changes?

[Michael Cummings (MSFT) (mcumming)]

Fixed, now requests a restart on change.