Skip to content

Enable Microsoft Entra ID sign-in for Private Marketplace access - #325331

Merged
Sandeep Somavarapu (sandy081) merged 43 commits into
microsoft:mainfrom
mcumming:mcumming-entra-id-vss-marketplace-review
Sep 1, 2026
Merged

Enable Microsoft Entra ID sign-in for Private Marketplace access#325331
Sandeep Somavarapu (sandy081) merged 43 commits into
microsoft:mainfrom
mcumming:mcumming-entra-id-vss-marketplace-review

Conversation

@mcumming

@mcumming Michael Cummings (MSFT) (mcumming) commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Addresses #280376Enable 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.jsonenableExtensionGalleryEntraAuth) 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 AI review requested due to automatic review settings July 10, 2026 17:15
@mcumming Michael Cummings (MSFT) (mcumming) changed the title Enable Microsoft Entra ID sign-in for Private Marketplace access (PR1) Enable Microsoft Entra ID sign-in for Private Marketplace access Jul 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts Outdated
Comment thread src/vs/workbench/contrib/extensions/browser/extensionsViewlet.ts Outdated
joshspicer
joshspicer previously approved these changes Jul 31, 2026
@joshspicer

Copy link
Copy Markdown
Contributor

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

@joshspicer

Copy link
Copy Markdown
Contributor

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

…ntext key

Introduce the `extensions.gallery.authProvider` policy that selects which
identity provider (github or microsoft) gates Private Marketplace access, and
register it in the exported policy data. Add the marketplace auth-provider
context key and the Entra ID resource scope constant used to acquire a
Private Marketplace-audienced token.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve the marketplace access strategy in the workbench gallery manifest
service: cache-first startup, provider-routed access handling, Microsoft
eligibility probing against the eligibility resource from the gallery
manifest, the GitHub DefaultAccount path, the marketplace auth-provider
context key, and access telemetry.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Surface a provider-aware sign-in prompt and access-denied state in the
extensions viewlet, driven by the marketplace auth-provider context key so
the correct identity provider is presented to the user.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Allow the built-in extensions gallery to silently use Microsoft (Entra ID)
authentication sessions for Private Marketplace access.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Cover provider selection, cache-first startup, Microsoft eligibility
handling, and the GitHub access path in the gallery manifest service.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ndling

Address rubber-duck review findings on the Entra ID marketplace path:

- Scope the cached access verdict to the marketplace it was computed against
  (authProvider + accountId + serviceUrl), rejecting stale caches on any mismatch.
- Guard cache application and background validation with a monotonic epoch so a
  session/account/config change mid-validation supersedes an in-flight result.
- Register session/account listeners before applying the cache, and the config
  listener before initial validation, closing startup TOCTOU windows.
- Route transient auth-service and marketplace-fetch failures to Unreachable
  instead of leaving a configured marketplace on a blank Unavailable view.
- Split 401 (missing/expired token -> RequiresSignIn, not cached) from 403
  (durable denial -> AccessDenied, cached ineligible).
- Never follow redirects on token-bearing requests; only send the Entra token to
  an HTTPS same-origin target; reject non-2xx and non-manifest 200 responses
  before parsing.
- Restore the galleryservice:custom:marketplace telemetry on the GitHub path and
  drop the unused server-provided eligibility reason from persisted cache.

Expand unit coverage to 45 tests across provider routing, eligibility, caching,
error classification, and the epoch race paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…g, resource validation, UX copy

- Policy: make the `extensions.gallery.authProvider` schema enum and enumDescriptions
  unconditional (`github`, `microsoft`). Gating the enum on the Entra product flag left the
  policy metadata exporting two enum descriptions against a single-value enum, which fails the
  policy-artifact generator's equal-length requirement on a clean export. The Entra gate is
  already enforced at runtime in getEffectiveAuthProvider(), and the setting is hidden
  (included: false), so this advertises nothing new in the UI.

- Cross-account authorization leak: on Microsoft session change and GitHub default-account
  change, revoke the active manifest (drop `Available`) before revalidating. Previously the
  active status stayed `Available`, so a transient index/eligibility failure on the new account
  preserved the prior account's access.

- Layering: move CONTEXT_MARKETPLACE_AUTH_PROVIDER down to the platform extensionGalleryManifest
  module so the workbench service no longer imports from a workbench/contrib module. The
  Extensions contribution re-exports it for existing consumers.

- Resource validation: reject a 200 service index whose `resources` entries are malformed
  (missing string `id`/`type`), not just a non-array `resources`. Endpoint discovery calls
  `resource.type.split()` outside the fetch try/catch, so an undefined `type` would crash
  initialization instead of surfacing `Unreachable`.

- UX: make the Microsoft AccessDenied welcome message generic. A bare 403 gives no typed reason,
  so asserting that an Entra ID account or Visual Studio Subscription is required could tell an
  already-signed-in user to obtain access they already have.

Adds a unit test covering the malformed-resources -> Unreachable path. All 47 gallery tests pass;
typecheck-client and valid-layers-check are clean.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace the two `as any` casts flagged by the local/code-no-any-casts
ESLint rule that failed hygiene: complete the stubbed
IProductService.extensionsGallery so it satisfies Partial<IProductService>
without a cast, and cast the entitlements literal to IEntitlementsData.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A marketplace can refuse the client outright — for example one enforcing a
minimum supported VS Code version replies 400 "Only VS Code clients version
1.104.2 or later are allowed". That is durable: retrying cannot help.

Such a response was classified as a transient failure and surfaced as
"The Extensions Marketplace is currently unavailable. Check your network
connection", which points the user at something that is not the problem. On main
any failed fetch of a configured marketplace reports AccessDenied ("please
contact your administrator"), so this was also a regression in what the user is
told.

Classify a non-401/403 4xx as a new MarketplaceClientRejectedError and map it to
a denial, restoring the message main gives. 5xx and network failures stay
transient and continue to report Unreachable. The denial is deliberately not
cached: it belongs to the client, not the account, and can change on upgrade.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
…ce the rest

Review feedback on the previous split: the account service should answer whether
there is a usable account, and nothing else. It was still fetching the service
index, so its verdict carried a manifest and it needed the marketplace URL —
neither of which is its concern.

Account service now resolves identity and entitlement only:

    readonly accountStatus: ExtensionGalleryAccountStatus;
    readonly onDidChangeAccountStatus: Event<ExtensionGalleryAccountStatus>;
    getAccount(): Promise<IExtensionGalleryAccount | undefined>;
    readonly onDidChangeAccount: Event<void>;
    setPreferredAccount(accountId: string): void;
    connectAuthentication(authenticationService: IAuthenticationService): void;

It no longer takes a serviceUrl or a CancellationToken, returns no manifest, and
owns no index cache. getAccount returns the signed-in account even when it is not
entitled, so callers can scope a durable denial to it; accountStatus says whether
it may be used. The marketplace auth-provider context key moves here too, which
also removes a second call to getEffectiveAuthProvider.

The manifest service now owns the marketplace side: the serviceUrl, the
non-HTTPS token-target check, the index fetch, resolution generations, and the
mapping from outcome to ExtensionGalleryManifestStatus.

The durable access verdict moves to a new ExtensionGalleryAccessCache rather than
into either service, so neither carries storage plumbing and the scoping rules —
a verdict is only honoured for the account, marketplace and auth provider it was
written for — live in one testable place. It resolves the effective auth provider
itself via getEffectiveAuthProvider, a pure function, rather than requiring a new
member on the account service.

Two ordering bugs surfaced while testing this and are fixed here: the
configuration-change listener was registered after the initial resolution, so a
change during a slow index fetch was missed entirely; and a transient failure to
resolve the account discarded the cached verdict that a later retry needs.

Behaviour is otherwise unchanged, verified against a deployed private marketplace
and by the existing suite.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
Review feedback: keep code comments minimal. The marketplace files carried long
explanatory blocks that restated what the code already says.

Trimmed every file this PR touches, keeping rationale only where the code cannot
show it — the service DI cycle, why an ineligible account is still returned, why
only a 403 is persisted, why a bearer is confined to a same-origin HTTPS target.
Net 107 lines removed with no functional change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155


// Fetches and memoizes the service index for the configured marketplace.
private readonly serviceIndexFetcher: ExtensionGalleryServiceIndexFetcher;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

private readonly serviceIndexFetcher: ExtensionGalleryServiceIndexFetcher;

// Durable "was this account allowed here?" verdicts, scoped to account + marketplace.
private readonly accessCache: ExtensionGalleryAccessCache;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment on lines +112 to +115
this.beginResolution();
this.serviceIndexFetcher.invalidate();
this.accessCache.clear();
this.requestRestart();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment on lines +121 to +123
// Registered before the initial resolution for the same reason: a sign-out or account
// switch mid-flight needs a live listener to supersede it.
this._register(this.galleryAccountService.onDidChangeAccount(() => this.resolve(configuredServiceUrl)));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please avoid verbose comments

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

this.update(null, ExtensionGalleryManifestStatus.AccessDenied);
return;
}
if (account.accessToken && !isSafeTokenTarget(configuredServiceUrl, configuredServiceUrl)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

// --- Status management ---

/** Publishes the manifest and reports successful custom-marketplace access (any auth provider). */
private renderAvailable(manifest: IExtensionGalleryManifest): void {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method name seems misleading - please rename it appropriately

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment on lines +248 to +250
// Fired for every successfully accessed serviceUrl-configured marketplace regardless of auth
// provider; the github/microsoft distinction is tracked separately by 'marketplace:auth:checked'.
this.telemetryService.publicLog2<

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please reduce noisy comments

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

The index fetcher only existed to hold a memo that could never be hit:
resolve() invalidated it immediately before its single read. With no
state left it returns to a private method on the manifest service.

The durable access cache is removed. Its `true` verdict was never read,
and the `false` written for a client-side ineligible account outlived
the condition it recorded - an account later granted entitlement stayed
AccessDenied until sign-out. Covered by a new regression test.

isSafeTokenTarget was called with the same URL for both arguments, so
only its HTTPS check ever ran; it becomes an inline guard. Redirects are
already handled by followRedirects: 0. MarketplaceMisconfiguredError was
never thrown and is gone.

The configuration listener returns to its shape on main, with the auth
provider key added. renderAvailable becomes setAvailable. Comment volume
across these files drops from 24% to 8%, against 4% in the surrounding
extensionManagement code.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
A private marketplace is account-scoped, but the success path skipped
publishing whenever the status was already Available. Switching to a
different eligible account therefore fetched that account's catalog and
then discarded it, leaving the previous account's in place until
restart. The manifest is now published on every success; the
custom-marketplace telemetry still fires only on the transition into
Available.

Removing the access cache orphaned IExtensionGalleryAccount.id - it
existed only to scope a cached verdict to an account - so it goes, along
with the comment describing the verdict it scoped.

That removal also took with it the only test asserting that an
already-available marketplace survives a transient failure. Restored for
both the fetch and the account-resolution paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
@sandy081

Copy link
Copy Markdown
Member

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

Takes c41e451 as-is, with two changes that restore main's outcome:

A failed manifest fetch is reported as denied, as on main, rather than
asking an already-signed-in user to sign in. This also keeps the
minimum-client-version rejection reading the way it does today.

A transient failure to resolve the account no longer retracts a
marketplace the user already has. On main that throw rejects the promise
and no status is published, so an available marketplace survives; here
the account service catches it, reports Unknown, and returned undefined
would otherwise be read as a sign-out.

Authentication moves to the follow-up PR: the bearer on the service
index, the HTTPS guard, redirect suppression, and 401/403 typing all go,
along with the Unreachable and Misconfigured statuses that only existed
to describe them, and their welcome content and badges.

Two defects that also reproduce on main are now separate PRs -
microsoft#331800 (a sign-out during an in-flight fetch) and
microsoft#331804 (a 200 carrying any JSON accepted as a service
index) - so the tests covering them move there.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
extensions.gallery.authProvider is now the only switch for the
Microsoft path. Also removes extensionGalleryAccess.ts, whose
remaining exports were already orphaned by the manifest service
adoption in e61ce45.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
/** A configured marketplace could not be reached — transient, unlike {@link Unavailable}. */
Unreachable = 'unreachable',
/** The deployment cannot work as configured — e.g. a non-HTTPS service index under Entra auth. */
Misconfigured = 'misconfigured'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets not introduce more status code unless necessay

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

export const ExtensionGalleryAuthProviderConfigKey = 'extensions.gallery.authProvider';

/** Standard OpenID Connect scopes — enough to identify the user for the eligibility check. */
export const PRIVATE_MARKETPLACE_SCOPES: string[] = ['openid', 'profile', 'email', 'offline_access'];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@mcumming Michael Cummings (MSFT) (mcumming) Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a65242bextensionsGallery.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).

content: localize('sign in microsoft', "[Sign in with your Microsoft account]({0}) to access the Extensions Marketplace.", `command:workbench.extensions.actions.gallery.signIn`),
when: ContextKeyExpr.and(
CONTEXT_EXTENSIONS_GALLERY_STATUS.isEqualTo(ExtensionGalleryManifestStatus.RequiresSignIn),
CONTEXT_MARKETPLACE_AUTH_PROVIDER.isEqualTo('microsoft')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

* Interactive Microsoft sign-in, registered in the Electron layer and invoked by id from the
* browser-layer action so it need not cross the layer boundary.
*/
export const ExtensionGalleryMicrosoftSignInCommandId = 'workbench.extensions.marketplace.signInWithMicrosoft';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment on lines +33 to +34
const MSA_TENANT_ID = '9188040d-6c67-4c5b-b112-36a304b66dad';
const MSA_PASSTHROUGH_TENANT_ID = 'f8cdef31-a31e-4b4a-93e4-5f571e91255a';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave this to TylerLeonhardt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


// A constructor dependency here would form a DI cycle: this → auth → extensionService → gallery
// → manifest → this, which aborts startup.
private authenticationService: IAuthenticationService | undefined;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

}

/** Entitlement decided locally from the token's tenant claim. The bearer travels with it. */
private async getMicrosoftAccount(): Promise<IExtensionGalleryAccount | undefined> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would request to get review from Tyler for this.

Adds extensionsGallery.accessScopes and drops the hardcoded
PRIVATE_MARKETPLACE_SCOPES, following defaultChatAgent.providerScopes.

Session lookup and interactive sign-in resolve the scopes through one
accessor so they cannot drift. A deployment that enables the Microsoft
path without configuring scopes now reports no account instead of
requesting a session with scopes it did not ask for.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
Follows DefaultAccountProvider: the auth-dependent half becomes an
IExtensionGalleryAccountProvider that a workbench contribution builds
and hands to the service, so the service no longer takes
IAuthenticationService at all and connectAuthentication is gone.

Sign-in is now a single signIn() on the service, which removes the
provider-specific command id and the cross-layer invoke-by-string. The
extensions view welcome content collapses back to one status-gated entry
labelled Sign In, leaving CONTEXT_MARKETPLACE_AUTH_PROVIDER with no
consumers.

The service interface moves to common/ so the browser layer can call it
directly. Both desktop entry points import the electron-browser module
explicitly: it is now only reachable for its registerSingleton side
effect, and without that the renderer fails to start.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
@sandy081

Copy link
Copy Markdown
Member

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

await chooseAccount(pick.account);
}

private readPreferredAccountId(): string | undefined {

@TylerLeonhardt TylerLeonhardt Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@IExtensionGalleryAccountService accountService: IExtensionGalleryAccountService,
) {
super();
const authProvider: ExtensionGalleryAccessProviderId = configurationService.getValue<string>(ExtensionGalleryAuthProviderConfigKey) === 'microsoft' ? 'microsoft' : 'github';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if this value changes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, now requests a restart on change.

The provider is selected once at startup, so changing
extensions.gallery.authProvider mid-session had no effect and gave no
indication that it had not been applied. The sibling serviceUrl setting
already prompts; this reuses that listener and dialog rather than
rebuilding the provider live.

Each setting keeps its own message: serviceUrl still reports a different
Marketplace, and the auth change reports a configuration change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
@sandy081
Sandeep Somavarapu (sandy081) merged commit 3d7597c into microsoft:main Sep 1, 2026
31 of 51 checks passed

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entra

@vs-code-engineering vs-code-engineering Bot added this to the 1.137.0 milestone Sep 1, 2026
Bhavya U (bhavyaus) pushed a commit that referenced this pull request Sep 1, 2026
…5331)

* Add extensions.gallery.authProvider policy, marketplace scope, and context key

Introduce the `extensions.gallery.authProvider` policy that selects which
identity provider (github or microsoft) gates Private Marketplace access, and
register it in the exported policy data. Add the marketplace auth-provider
context key and the Entra ID resource scope constant used to acquire a
Private Marketplace-audienced token.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Add Entra ID eligibility check to the gallery manifest service

Resolve the marketplace access strategy in the workbench gallery manifest
service: cache-first startup, provider-routed access handling, Microsoft
eligibility probing against the eligibility resource from the gallery
manifest, the GitHub DefaultAccount path, the marketplace auth-provider
context key, and access telemetry.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Add provider-aware marketplace sign-in and access-denied UX

Surface a provider-aware sign-in prompt and access-denied state in the
extensions viewlet, driven by the marketplace auth-provider context key so
the correct identity provider is presented to the user.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Add microsoft to trustedExtensionAuthAccess

Allow the built-in extensions gallery to silently use Microsoft (Entra ID)
authentication sessions for Private Marketplace access.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Add unit tests for marketplace provider routing and eligibility

Cover provider selection, cache-first startup, Microsoft eligibility
handling, and the GitHub access path in the gallery manifest service.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Harden Entra marketplace access: cache scoping, race guards, error handling

Address rubber-duck review findings on the Entra ID marketplace path:

- Scope the cached access verdict to the marketplace it was computed against
  (authProvider + accountId + serviceUrl), rejecting stale caches on any mismatch.
- Guard cache application and background validation with a monotonic epoch so a
  session/account/config change mid-validation supersedes an in-flight result.
- Register session/account listeners before applying the cache, and the config
  listener before initial validation, closing startup TOCTOU windows.
- Route transient auth-service and marketplace-fetch failures to Unreachable
  instead of leaving a configured marketplace on a blank Unavailable view.
- Split 401 (missing/expired token -> RequiresSignIn, not cached) from 403
  (durable denial -> AccessDenied, cached ineligible).
- Never follow redirects on token-bearing requests; only send the Entra token to
  an HTTPS same-origin target; reject non-2xx and non-manifest 200 responses
  before parsing.
- Restore the galleryservice:custom:marketplace telemetry on the GitHub path and
  drop the unused server-provided eligibility reason from persisted cache.

Expand unit coverage to 45 tests across provider routing, eligibility, caching,
error classification, and the epoch race paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Address Copilot PR review: policy export, cross-account leak, layering, resource validation, UX copy

- Policy: make the `extensions.gallery.authProvider` schema enum and enumDescriptions
  unconditional (`github`, `microsoft`). Gating the enum on the Entra product flag left the
  policy metadata exporting two enum descriptions against a single-value enum, which fails the
  policy-artifact generator's equal-length requirement on a clean export. The Entra gate is
  already enforced at runtime in getEffectiveAuthProvider(), and the setting is hidden
  (included: false), so this advertises nothing new in the UI.

- Cross-account authorization leak: on Microsoft session change and GitHub default-account
  change, revoke the active manifest (drop `Available`) before revalidating. Previously the
  active status stayed `Available`, so a transient index/eligibility failure on the new account
  preserved the prior account's access.

- Layering: move CONTEXT_MARKETPLACE_AUTH_PROVIDER down to the platform extensionGalleryManifest
  module so the workbench service no longer imports from a workbench/contrib module. The
  Extensions contribution re-exports it for existing consumers.

- Resource validation: reject a 200 service index whose `resources` entries are malformed
  (missing string `id`/`type`), not just a non-array `resources`. Endpoint discovery calls
  `resource.type.split()` outside the fetch try/catch, so an undefined `type` would crash
  initialization instead of surfacing `Unreachable`.

- UX: make the Microsoft AccessDenied welcome message generic. A bare 403 gives no typed reason,
  so asserting that an Entra ID account or Visual Studio Subscription is required could tell an
  already-signed-in user to obtain access they already have.

Adds a unit test covering the malformed-resources -> Unreachable path. All 47 gallery tests pass;
typecheck-client and valid-layers-check are clean.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Avoid `any` casts in extensionGalleryManifestService test

Replace the two `as any` casts flagged by the local/code-no-any-casts
ESLint rule that failed hygiene: complete the stubbed
IProductService.extensionsGallery so it satisfies Partial<IProductService>
without a cast, and cast the entitlements literal to IEntitlementsData.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Refactor Private Marketplace access validation into a provider strategy

Extract all eligibility/access-validation logic out of
WorkbenchExtensionGalleryManifestService into a dedicated, provider-agnostic
ExtensionGalleryAccessValidator, and split the GitHub-vs-Microsoft branching
into IExtensionGalleryAccessProvider strategy classes. This debloats the host
service (it now only builds a status sink and delegates) and isolates each
identity system's account resolution + eligibility check.

Replace the hand-rolled monotonic validationEpoch TOCTOU counter with a
CancellationTokenSource held in a MutableDisposable: assigning a new source
cancels/disposes the prior one, and each validation re-checks
token.isCancellationRequested immediately before mutating status/cache/manifest,
so a superseded in-flight validation cannot commit a stale verdict for an
account that is no longer current. Addresses reviewer feedback that the epoch
machinery bloated the service.

New files:
- extensionGalleryAccess.ts: shared leaf contracts (IExtensionGalleryAccessCore,
  IExtensionGalleryAccessProvider, IExtensionGalleryAccessSink, ICachedAccess,
  AccountResolution, ExtensionGalleryAccessProviderId, isSafeTokenTarget).
- extensionGalleryAccessProviders.ts: GitHub and Microsoft access providers.
- extensionGalleryAccessValidator.ts: provider-agnostic orchestrator.

Security invariants preserved: no microsoft->github fallback, cache scoped to
provider+serviceUrl, bearer only over HTTPS same-origin with followRedirects:0,
and the 401/403/transient status mappings are unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Surface AccessDenied instead of re-prompting sign-in on 401 for signed-in Microsoft accounts

When a signed-in Microsoft account made an authenticated Marketplace request that
returned 401, the previous logic mapped it to RequiresSignIn, which re-prompted the
same account whose token had just been rejected - producing an infinite sign-in loop.
Map both Microsoft 401 branches (service-index and eligibility) to AccessDenied so the
condition is surfaced to the user, and do not cache the 401 verdict (unlike a durable
403 denial) so a later config/account/session change re-evaluates cleanly. Lower the
MarketplaceAuthRequiredError log level to trace. First-time no-session flows are
unchanged (still RequiresSignIn).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Remove policy data from contributor PR

Keep the extensions.gallery.authProvider setting while moving its policy declaration and generated catalog entry to a separate maintainer-authored change.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 449a6246-235a-4c42-8d6d-ef65fd83a190

* Dissolve access validator into account + service-index services

Replace ExtensionGalleryAccessValidator and the provider/sink strategy
classes with two plain services and restore the manifest service toward
its upstream-main shape (minimal diff):

- ExtensionGalleryAccountService: mirrors IDefaultAccountService
  (getAccount/getCachedAccess/clearCache/onDidChangeAccount); owns
  GitHub + Microsoft account resolution, the eligibility check, and the
  ICachedAccess read/write/validate.
- ExtensionGalleryServiceIndexService: memoized service-index fetch.
- extensionGalleryManifestService: delegates all account/eligibility/
  index/cache work to the two services; keeps the added validation
  orchestration with a MutableDisposable<CancellationTokenSource> for
  the TOCTOU supersession guard.
- extensionGalleryAccess: trimmed leaf (removed orphaned sink/core
  interfaces), keeps shared helpers and error types.

The onDidChangeAccount subscription is registered before the initial
awaited validation so a sign-out mid-flight is observed. All 46
existing manifest-service tests pass unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* marketplace: thread CancellationToken guards, materialize index in cache path, restore logs, trim comments

Continue the Private Marketplace access refactor on the extracted services:

- Thread CancellationToken through the account service's cache mutations
  (denyFromAuthError and the eligible fast-path), guarding every write with
  token.isCancellationRequested so a superseded validation can never restore or
  persist a verdict for an account that is no longer current (TOCTOU guard).
- Materialize the service index inside the account service's cached-access path
  and add invalidateServiceIndexCache(), so the host maps a verdict to status
  without any further fetching and each validation generation re-fetches cleanly.
- Restore the [Marketplace] debug log messages (sign-in / access / SKU /
  enterprise) for parity with main's observability.
- Trim branch-added comments to why-only, leaving main's pre-existing comments
  untouched.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* marketplace: make getEffectiveAuthProvider dependency-free, cache resolved provider

Replace the DI-service parameters on getEffectiveAuthProvider with plain
primitives (configured provider string + Entra product flag) so the helper
never reaches into a service, and cache the resolved provider in a field on
the manifest service to avoid resolving it twice.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* marketplace: collapse duplicate access-denied welcome content into one entry

The microsoft and github/default access-denied welcome blocks carried
near-identical messages and together covered every provider state, so
replace them with a single entry gated on the AccessDenied status alone.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* marketplace: fix telemetry provider scoping

galleryservice:custom:marketplace was gated on the github provider, so
successful Microsoft/Entra marketplace access went uncounted. It now fires
for any successfully accessed serviceUrl-configured marketplace, restoring
its original meaning (custom-marketplace access, independent of provider).

The github-vs-microsoft distinction is instead tracked by
marketplace:auth:checked, which is now emitted from cacheAccess so every
definitive eligibility verdict reports its authProvider + eligible for both
providers (previously only the Microsoft 200 path emitted it).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* test: add gallery access unit and telemetry coverage

Add a dedicated extensionGalleryAccess.test.ts exercising the pure getEffectiveAuthProvider and isSafeTokenTarget helpers directly, and telemetry-assertion cases in the manifest service suite verifying galleryservice:custom:marketplace fires for both GitHub and Microsoft on eligible access, and marketplace:auth:checked reports the correct authProvider+eligible at each definitive verdict.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Remove CONTEXT_MARKETPLACE_AUTH_PROVIDER re-export

Import the context key directly from the platform extensionGalleryManifest
module in extensionsViewlet.ts (its only consumer) instead of re-exporting
it from contrib/extensions/common/extensions.ts, so there is a single import
source. Addresses PR review feedback.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Use Event.signal for onDidChangeAccount instead of an emitter relay

Assign onDidChangeAccount directly via Event.signal over the provider-specific
source instead of relaying through a private Emitter with Event.map. Removes the
now-unused Emitter import. Addresses PR review feedback.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Default extensions gallery auth provider to a valid enum member

Set the default for the marketplace auth-provider setting to 'github' instead
of the empty string, so the default is a member of the declared enum. Both
readers treat any non-'microsoft' value as the GitHub path, so behavior is
unchanged. Addresses PR review feedback.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Revert 'Add microsoft to trustedExtensionAuthAccess'

Drop the empty 'microsoft': [] placeholder from trustedExtensionAuthAccess in
product.json. It granted no silent access (no-op) and was local scaffolding for
the Entra path. Addresses PR review feedback.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Register account resolver as a Delayed singleton (Tyler #3, #4)

Replace the lazy `galleryAccountService: | undefined` field and its
`createInstance` in the manifest service with a proper
`InstantiationType.Delayed` singleton behind a new
`IExtensionGalleryAccountService` decorator, injected into the ctor.

The Delayed proxy makes ctor-time injection and the `onDidChangeAccount`
subscription non-instantiating, so the account service (and its
transitively-cyclic `IAuthenticationService` dependency) only materializes
on first non-event access. A `galleryAccountServiceActive` flag guards the
config-change handler so an unrelated config change never force-instantiates
the resolver when no private marketplace was configured. The ctor microtask
is kept: it defers the eager bootstrap's first access past ctor return so
the re-entry resolves the cached instance instead of throwing
"RECURSIVELY instantiating".

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Ground Private Marketplace account selection in a persisted slot

The Microsoft auth provider returns one session per signed-in account, so
picking sessions[0] was arbitrary when several accounts are signed in.

Persist a provider-scoped account slot (marketplace.account = { authProvider,
id }) and add a single getMicrosoftSession() selector that both the live check
and cache validation use: prefer the remembered account, adopt-and-persist a
lone account, and refuse to guess (require sign-in) when several accounts are
signed in with no remembered choice or the remembered one is gone.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Show account quick pick on Microsoft marketplace sign-in

Address PR review: the Microsoft sign-in action no longer blindly
creates a session. When multiple Microsoft accounts are signed in, a
quick pick lets the user choose one (with a "different account" escape
hatch); a single account is bound directly, and no accounts falls
through to interactive sign-in. The chosen account is persisted so
selection stays grounded across restarts.

The browser-layer sign-in action delegates to a command registered in
the electron-browser account service (mirroring the GitHub branch's
DEFAULT_ACCOUNT_SIGN_IN_COMMAND delegation), respecting the layer
boundary. Binding uses createSession({ account }) so an already
signed-in account is bound without a fresh login while still firing the
session-change event that drives marketplace re-validation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Clarify getAccount vs resolveCurrentAccount intent

Address PR review: the two methods read as similar. Add JSDoc on each
contrasting it with the other so the distinct responsibilities are clear
at the call site: getAccount is the heavier public eligibility verdict
(may hit the network), while resolveCurrentAccount is an identity-only
silent resolution used solely for cache validation. The overlapping
"current account" selection logic was already unified into the single
getMicrosoftSession() selector.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* Move Microsoft (Entra) marketplace eligibility check client-side

Decide Private Marketplace eligibility locally from the account's ID-token
tenant (`tid`) claim instead of round-tripping to a server-side
EligibilityService endpoint, mirroring how the GitHub path already gates
locally. A work/school (Entra) tenant is eligible; a personal Microsoft
Account (MSA) is not. The check runs before any index fetch, so an ineligible
account never touches the (possibly auth-gated) index, and fails closed on an
undecodable/opaque token or a token with no `tid`.

Removes the EligibilityService resource type, its URL discovery, the
same-origin token-target guard for it, and the IRequestService dependency and
POST round-trip in ExtensionGalleryAccountService. Adds an optional `tid`
claim to IAuthorizationJWTClaims and rewrites the surrounding docs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* marketplace: break account->auth DI cycle via orchestrator wiring

ExtensionGalleryAccountService injected IAuthenticationService, forming a
service DI cycle (account -> auth -> extensionService -> extensionGalleryService
-> manifest -> account) that the instantiation graph walker detects and aborts
startup on. `Delayed` does not help: the cycle graph is a static walk over the
@iService constructor decorators.

Remove the @IAuthenticationService constructor dependency and supply it
post-startup through a new connectAuthentication() init API, wired by a small
ExtensionGalleryAccountAuthenticationContribution at WorkbenchPhase.AfterRestored
(orchestrator wiring, per reviewer guidance - not a service-locator lookup).
Until connected the Microsoft path reports "no account"; connecting re-signals
onDidChangeAccount once so any verdict resolved in that window is re-validated.

Update the manifest service test to play the orchestrator role by calling
connectAuthentication after constructing the account service.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* marketplace: rename ExtensionGalleryServiceIndexService to ...Fetcher

The class carried a "Service" suffix but is a plain createInstance helper
owned by ExtensionGalleryAccountService (an owner-scoped memo cache), not a
DI-registered service. Rename the class to ExtensionGalleryServiceIndexFetcher
and the field indexService -> serviceIndexFetcher so the name no longer implies
a service registration it does not have, per reviewer feedback (either register
it properly or rename it).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c

* marketplace: drop obsolete microtask deferral in gallery manifest service

The deferral guarded against "RECURSIVELY instantiating service
'IAuthenticationService'" as introduced in bd44656, when this service
resolved authentication itself through instantiationService.invokeFunction
while still constructing.

af73002 moved access resolution into ExtensionGalleryAccountService and
removed 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 by launching a configured private marketplace with and without the
deferral on a clean build: both start normally, with no recursion error and
identical [Marketplace] trace output.

Also corrects the galleryAccountService comment, which described an
IAuthenticationService dependency that no longer exists and pointed at the
deferral removed here.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* marketplace: move access resolution into the gallery account service

The manifest service was orchestrating access validation rather than consuming
it: it drove the two-phase cached-then-live resolution, owned the validation
generations and cancellation tokens, and reached into the account service to
clear caches and invalidate the memoized service index. Roughly 125 of its 272
lines were access machinery, and four account-service internals had to be public
for it.

Move all of that behind the account service. It now resolves access itself
(cache first, then re-validating in the background) and publishes the outcome as
a verdict, so the manifest service only maps verdict to
ExtensionGalleryManifestStatus.

Interface changes:
- add onDidChangeAccess: Event<IExtensionGalleryAccessVerdict> and
  resolveAccess(serviceUrl), which needs no CancellationToken from the caller
- add reset() for the configuration-change path
- getAccount and getCachedAccess become private; clearCache becomes private;
  invalidateServiceIndexCache is deleted (unused once the caller moved)

No behaviour change: verdict classification, the "never downgrade an already
Available marketplace" rule, cancellation semantics and cache lifetimes are
preserved. extensionGalleryManifestService.ts drops from 272 to 203 lines and
its access-validation section from ~125 lines to a 40-line mapping.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* marketplace: include the service index error body in the failure

A non-2xx service index response was reported as only a status code, so a
marketplace that rejects the client and explains why in the body was
indistinguishable from an unreachable network. The workbench surfaces such a
failure as "The Extensions Marketplace is currently unavailable. Check your
network connection", which sends the user looking in the wrong place.

Append a best-effort, truncated response body to the error so the reason reaches
the log. For example a marketplace enforcing a minimum client version now
reports:

  Service index returned status 400: Access denied: Only VS Code clients
  version 1.104.2 or later are allowed.

Diagnostics only: the status mapping is unchanged, and reading the body never
throws so it cannot mask the status we already have.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* marketplace: pin that a signed-out user is never told to check the network

With no session the service index is never probed, so a failing marketplace
cannot turn RequiresSignIn into Unreachable and leave the user with a "check
your network connection" message and a reload link instead of a sign-in
affordance. That invariant was untested for the post-startup re-validation path,
which runs when authentication connects and re-signals an account change.

Adds a test covering that sequence: no session, an index that would reject the
client, and a session-change event after the initial resolution. Asserts the
status stays RequiresSignIn and that no index request is made.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* marketplace: report a rejected client as denied, not unreachable

A marketplace can refuse the client outright — for example one enforcing a
minimum supported VS Code version replies 400 "Only VS Code clients version
1.104.2 or later are allowed". That is durable: retrying cannot help.

Such a response was classified as a transient failure and surfaced as
"The Extensions Marketplace is currently unavailable. Check your network
connection", which points the user at something that is not the problem. On main
any failed fetch of a configured marketplace reports AccessDenied ("please
contact your administrator"), so this was also a regression in what the user is
told.

Classify a non-401/403 4xx as a new MarketplaceClientRejectedError and map it to
a denial, restoring the message main gives. 5xx and network failures stay
transient and continue to report Unreachable. The denial is deliberately not
cached: it belongs to the client, not the account, and can change on upgrade.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* marketplace: give the account service one job, and the manifest service the rest

Review feedback on the previous split: the account service should answer whether
there is a usable account, and nothing else. It was still fetching the service
index, so its verdict carried a manifest and it needed the marketplace URL —
neither of which is its concern.

Account service now resolves identity and entitlement only:

    readonly accountStatus: ExtensionGalleryAccountStatus;
    readonly onDidChangeAccountStatus: Event<ExtensionGalleryAccountStatus>;
    getAccount(): Promise<IExtensionGalleryAccount | undefined>;
    readonly onDidChangeAccount: Event<void>;
    setPreferredAccount(accountId: string): void;
    connectAuthentication(authenticationService: IAuthenticationService): void;

It no longer takes a serviceUrl or a CancellationToken, returns no manifest, and
owns no index cache. getAccount returns the signed-in account even when it is not
entitled, so callers can scope a durable denial to it; accountStatus says whether
it may be used. The marketplace auth-provider context key moves here too, which
also removes a second call to getEffectiveAuthProvider.

The manifest service now owns the marketplace side: the serviceUrl, the
non-HTTPS token-target check, the index fetch, resolution generations, and the
mapping from outcome to ExtensionGalleryManifestStatus.

The durable access verdict moves to a new ExtensionGalleryAccessCache rather than
into either service, so neither carries storage plumbing and the scoping rules —
a verdict is only honoured for the account, marketplace and auth provider it was
written for — live in one testable place. It resolves the effective auth provider
itself via getEffectiveAuthProvider, a pure function, rather than requiring a new
member on the account service.

Two ordering bugs surfaced while testing this and are fixed here: the
configuration-change listener was registered after the initial resolution, so a
change during a slow index fetch was missed entirely; and a transient failure to
resolve the account discarded the cached verdict that a later retry needs.

Behaviour is otherwise unchanged, verified against a deployed private marketplace
and by the existing suite.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* marketplace: cut comment volume across the PR

Review feedback: keep code comments minimal. The marketplace files carried long
explanatory blocks that restated what the code already says.

Trimmed every file this PR touches, keeping rationale only where the code cannot
show it — the service DI cycle, why an ineligible account is still returned, why
only a 403 is persisted, why a bearer is confined to a same-origin HTTPS target.
Net 107 lines removed with no functional change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* Fold the service index fetch back in and drop the access cache

The index fetcher only existed to hold a memo that could never be hit:
resolve() invalidated it immediately before its single read. With no
state left it returns to a private method on the manifest service.

The durable access cache is removed. Its `true` verdict was never read,
and the `false` written for a client-side ineligible account outlived
the condition it recorded - an account later granted entitlement stayed
AccessDenied until sign-out. Covered by a new regression test.

isSafeTokenTarget was called with the same URL for both arguments, so
only its HTTPS check ever ran; it becomes an inline guard. Redirects are
already handled by followRedirects: 0. MarketplaceMisconfiguredError was
never thrown and is gone.

The configuration listener returns to its shape on main, with the auth
provider key added. renderAvailable becomes setAvailable. Comment volume
across these files drops from 24% to 8%, against 4% in the surrounding
extensionManagement code.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* Publish the catalog on every successful resolve

A private marketplace is account-scoped, but the success path skipped
publishing whenever the status was already Available. Switching to a
different eligible account therefore fetched that account's catalog and
then discarded it, leaving the previous account's in place until
restart. The manifest is now published on every success; the
custom-marketplace telemetry still fires only on the transition into
Available.

Removing the access cache orphaned IExtensionGalleryAccount.id - it
existed only to scope a cached verdict to an account - so it goes, along
with the comment describing the verdict it scoped.

That removal also took with it the only test asserting that an
already-available marketplace survives a transient failure. Restored for
both the fetch and the account-resolution paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* refactor gallery manifest service to have minimal changes

* Adopt the reviewer's gallery manifest service

Takes c41e451 as-is, with two changes that restore main's outcome:

A failed manifest fetch is reported as denied, as on main, rather than
asking an already-signed-in user to sign in. This also keeps the
minimum-client-version rejection reading the way it does today.

A transient failure to resolve the account no longer retracts a
marketplace the user already has. On main that throw rejects the promise
and no status is published, so an available marketplace survives; here
the account service catches it, reports Unknown, and returned undefined
would otherwise be read as a sign-out.

Authentication moves to the follow-up PR: the bearer on the service
index, the HTTPS guard, redirect suppression, and 401/403 typing all go,
along with the Unreachable and Misconfigured statuses that only existed
to describe them, and their welcome content and badges.

Two defects that also reproduce on main are now separate PRs -
#331800 (a sign-out during an in-flight fetch) and
#331804 (a 200 carrying any JSON accepted as a service
index) - so the tests covering them move there.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* Remove the Entra auth product flag

extensions.gallery.authProvider is now the only switch for the
Microsoft path. Also removes extensionGalleryAccess.ts, whose
remaining exports were already orphaned by the manifest service
adoption in e61ce45.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* Read marketplace auth scopes from product.json

Adds extensionsGallery.accessScopes and drops the hardcoded
PRIVATE_MARKETPLACE_SCOPES, following defaultChatAgent.providerScopes.

Session lookup and interactive sign-in resolve the scopes through one
accessor so they cannot drift. A deployment that enables the Microsoft
path without configuring scopes now reports no account instead of
requesting a session with scopes it did not ask for.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* Route marketplace sign-in through the account service

Follows DefaultAccountProvider: the auth-dependent half becomes an
IExtensionGalleryAccountProvider that a workbench contribution builds
and hands to the service, so the service no longer takes
IAuthenticationService at all and connectAuthentication is gone.

Sign-in is now a single signIn() on the service, which removes the
provider-specific command id and the cross-layer invoke-by-string. The
extensions view welcome content collapses back to one status-gated entry
labelled Sign In, leaving CONTEXT_MARKETPLACE_AUTH_PROVIDER with no
consumers.

The service interface moves to common/ so the browser layer can call it
directly. Both desktop entry points import the electron-browser module
explicitly: it is now only reachable for its registerSingleton side
effect, and without that the renderer fails to start.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

* Prompt for restart when the marketplace auth provider changes

The provider is selected once at startup, so changing
extensions.gallery.authProvider mid-session had no effect and gave no
indication that it had not been applied. The sibling serviceUrl setting
already prompts; this reuses that listener and dialog rather than
rebuilding the provider live.

Each setting keeps its own message: serviceUrl still reports a different
Marketplace, and the auth change reports a configuration change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Josh Spicer <23246594+joshspicer@users.noreply.github.com>
Co-authored-by: Sandeep Somavarapu <sasomava@microsoft.com>
Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c
Copilot-Session: 4e27c63d-f29e-4ee0-a324-8adb523f5155
Michael Cummings (MSFT) (mcumming) added a commit to mcumming/vscode that referenced this pull request Sep 2, 2026
Resolve conflicts against the reworked service (Entra PR microsoft#325331 renamed
the account mechanism to IExtensionGalleryAccountService; microsoft#321044 made the
constructor re-publish to shared/remote channels on every manifest change).

Defect 1 (account listener registered after the initial await) is already
fixed upstream, and the stale-null-to-channels concern from review is moot
now that the constructor subscribes to onDidChangeExtensionGalleryManifest.
The lost race (defect 2) still exists in handleMarketplaceAccountAccess, so
the cancellation supersession is re-applied onto the new Eligible path and
the regression test is re-expressed in the new harness via the GitHub
default-account path.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants