Add extension gallery auth provider policy - #328982
Draft
joshspicer wants to merge 21 commits into
Draft
Conversation
…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>
…ss-marketplace-review
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
…d-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 trailing whitespace on the blank line after the enableExtensionGalleryEntraAuth property, which tripped the Compile & Hygiene CI check (File not formatted). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c
Resolve conflict in build/lib/policies/policyData.jsonc by taking main's regenerated policy catalog and re-adding the ExtensionGalleryAuthProvider policy immediately after ExtensionGalleryServiceUrl (matching source registration order in extensions.contribution.ts). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c
The ExtensionGalleryAuthProvider policy block was hand-inserted after ExtensionGalleryServiceUrl during merge conflict resolution, but the generator sorts policies alphabetically by key. Regenerated via the canonical export so authProvider correctly precedes serviceUrl, matching the PolicyExport integration test's byte-for-byte check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe244b45-6362-4563-880d-fa3d9a27a07c
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
Register the maintainer-owned ExtensionGalleryAuthProvider policy for the setting introduced by PR microsoft#325331 and regenerate the policy catalog.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 449a6246-235a-4c42-8d6d-ef65fd83a190
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an enterprise policy for selecting the Extension Marketplace authentication provider, alongside the dependent Entra ID marketplace-access implementation.
Changes:
- Registers and exports the
ExtensionGalleryAuthProviderpolicy. - Adds GitHub/Microsoft access validation, caching, and eligibility checks.
- Adds provider-aware marketplace status and sign-in UX.
Show a summary per file
| File | Description |
|---|---|
build/lib/policies/policyData.jsonc |
Adds generated policy metadata. |
product.json |
Registers Microsoft trusted-auth access. |
src/vs/base/common/product.ts |
Defines the Entra feature gate. |
src/vs/platform/extensionManagement/common/extensionGalleryManifest.ts |
Adds auth configuration, scopes, resources, and statuses. |
src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts |
Registers the policy and sign-in action. |
src/vs/workbench/contrib/extensions/browser/extensionsViewlet.ts |
Adds provider-aware marketplace UX. |
src/vs/workbench/contrib/extensions/common/extensions.ts |
Re-exports the provider context key. |
src/vs/workbench/services/extensionManagement/electron-browser/extensionGalleryAccess.ts |
Defines shared access contracts and safety checks. |
src/vs/workbench/services/extensionManagement/electron-browser/extensionGalleryAccessProviders.ts |
Implements GitHub and Microsoft validation. |
src/vs/workbench/services/extensionManagement/electron-browser/extensionGalleryAccessValidator.ts |
Coordinates validation, cancellation, and caching. |
src/vs/workbench/services/extensionManagement/electron-browser/extensionGalleryManifestService.ts |
Integrates validation with manifest state. |
src/vs/workbench/services/extensionManagement/test/electron-browser/extensionGalleryManifestService.test.ts |
Tests routing, security, caching, and races. |
Review details
Suppressed comments (1)
src/vs/workbench/services/extensionManagement/electron-browser/extensionGalleryAccessProviders.ts:338
- The linked PR explicitly specifies eligibility 401 as
RequiresSignInand 403 asAccessDenied, but this branch maps both to denial. A stale/wrong-audience token therefore produces the wrong welcome message and state. Route 401 toRequiresSignInand update the matching test.
// condition. Surface AccessDenied so it is communicated. We do NOT cache a
// negative verdict: unlike a 403 a 401 is not a durable per-identity denial, so
// a later config/account/session change re-evaluates cleanly.
this._core.clearCache();
this._core.sink.update(null, ExtensionGalleryManifestStatus.AccessDenied);
- Files reviewed: 12/12 changed files
- Comments generated: 5
- Review effort level: Balanced
Comment on lines
+132
to
+136
| * Only standard OpenID Connect sign-in scopes are requested — enough to obtain a | ||
| * Microsoft session that identifies the user. This intentionally does NOT request a | ||
| * resource-scoped token (e.g. `api://<client-id>/access_as_user`). Acquiring resource | ||
| * tokens for Private Marketplace API calls, per the server's Protected Resource | ||
| * Metadata (RFC 9728), is deferred to a follow-up change. |
Comment on lines
+253
to
+257
| // We do NOT cache a negative verdict: unlike a 403 a 401 is not a durable | ||
| // per-identity denial, so a later config/account/session change re-evaluates | ||
| // cleanly. | ||
| this._core.clearCache(); | ||
| this._core.sink.update(null, ExtensionGalleryManifestStatus.AccessDenied); |
Comment on lines
+180
to
+183
| cancel(): void { | ||
| this._validationTokenSource.value?.cancel(); | ||
| this._validationTokenSource.clear(); | ||
| } |
| // A newer validation superseded this one while we awaited — discard. | ||
| return; | ||
| } | ||
| const session = sessions[0]; |
Comment on lines
+115
to
+119
| // 3. Validate (foreground if no cache, background if cache was applied) | ||
| if (cached) { | ||
| validateAccess(); | ||
| } else { | ||
| await validateAccess(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
!! REBASE on origin/main once #325331 merges !!
The final commit moves the
ExtensionGalleryAuthProviderpolicy declaration and generatedpolicyData.jsoncentry into a maintainer-authored PR while leaving the setting and the rest of the Private Marketplace work in the contributor PR. Once #325331 merges, this PR reduces to the policy-only change.The policy catalog was regenerated with
npm run export-policy-data, and the policy uses the current minimum VS Code version (1.133).