Migrate Recommended Apps & Connectivity Apps to Official Marketplace Catalog API#8999
Migrate Recommended Apps & Connectivity Apps to Official Marketplace Catalog API#8999Groenbech96 wants to merge 4 commits into
Conversation
…Catalog API Replace the undocumented appsource.microsoft.com/view/app endpoint (now blocked by Azure WAF) with the official Marketplace Catalog API at catalogapi.azure.com/products. Changes: - Fetch the X-API-Key from Azure Key Vault (secret: MarketplaceCatalogApi-Key) - Call GET https://catalogapi.azure.com/products/PUBID.{pub}|AID.{aid}|PAPPID.{pappid} with X-API-Key header instead of spoofing a browser User-Agent - Parse largeIconUri directly from the top-level JSON response (was nested under detailInformation.LargeIconUri in the old endpoint) - Remove fragile User-Agent spoofing workaround - Update error messages and telemetry labels to reflect new API - Update test assertions for new error message format AB#622276 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3c56b93 to
6faaa68
Compare
Copilot PR ReviewIteration 5 · Outcome: completed Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf Findings by domainFindings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).
Totals: 3 knowledge-backed · 0 agent findings. Orchestrator pre-filter (2 file(s) excluded)
Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Use the full PUBID.{pub}|AID.{aid}|PAPPID.{pappid} product identifier when calling the Marketplace Catalog API (matches the documented endpoint), which also resolves the AA0137 unused-variable compile errors for AId/PAppId and removes the now-unused LanguageCode parameter from CheckIfURLExistsAndDownloadLogo in Recommended Apps.
Replace the placeholder telemetry event ID '0000NEW' with a registered unique ID '0000I4S' in Connectivity Apps (addresses PR review comment).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 306d1b5a-91d0-407c-b347-aff98e270fe0
This comment has been minimized.
This comment has been minimized.
|
- RecommendedAppsImpl: pass label + args directly to Error (preserve RT0030 telemetry label identity) instead of pre-building with StrSubstNo - RecommendedAppsImpl: raise the missing Key Vault key as ErrorType::Internal so infrastructure detail goes to telemetry, not the user dialog - ConnectivityAppsLogoMgt: log Catalog API logo-download failure (0000I4J) at Verbosity::Warning instead of Normal - RecommendedAppsTests: restore full expected error (incl. 404 status) so the negative test pins the not-found failure Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| HttpClient.DefaultRequestHeaders().Add('User-Agent', UserAgentLbl); | ||
| HttpClient.Get(StrSubstNo(AppExistURLLbl, PubId, AId, PAppId), HttpResponseMessage); | ||
| if not AzureKeyVault.GetAzureKeyVaultSecret(CatalogApiKeyVaultSecretNameLbl, ApiKey) then begin | ||
| Session.LogMessage('0000I4S', CannotGetApiKeyFromKeyVaultLbl, Verbosity::Error, DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, 'Category', TelemetryCategoryLbl); |
There was a problem hiding this comment.
The new Session.LogMessage call for the Key Vault retrieval failure reuses event ID '0000I4S', which is already used by an unrelated event in src/System Application/App/Extension Management/src/ExtensionMarketplace.Codeunit.al (line 246, 'MissingAppIdErr').
Telemetry event IDs must be unique so Application Insights queries and alert rules can distinguish events; reusing this ID makes the two unrelated events indistinguishable in telemetry. Pick an unused event ID for the new Key Vault failure log.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
| exit(false); | ||
| end; | ||
|
|
||
| HttpClient.DefaultRequestHeaders().Add('X-API-Key', ApiKey); |
There was a problem hiding this comment.
CheckIfURLExistsAndDownloadLogo now authenticates to the Marketplace Catalog API with a static API key retrieved from Azure Key Vault ('X-API-Key' header) instead of an unauthenticated call.
This is a real improvement over the prior unauthenticated scrape, but per BCQuality guidance an API key is the weaker of the two credential mechanisms: it cannot be rotated automatically, and a single disclosure of the 'MarketplaceCatalogApi-Key' secret affects every tenant running this app. If catalogapi.azure.com supports an OAuth2 client-credentials flow, prefer AcquireTokenWithClientCredentials over a long-lived shared API key.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
| Error(KeyVaultErrorInfo); | ||
| end; | ||
|
|
||
| HttpClient.DefaultRequestHeaders().Add('X-API-Key', ApiKey); |
There was a problem hiding this comment.
CheckIfURLExistsAndDownloadLogo now authenticates to the Marketplace Catalog API with a static API key retrieved from Azure Key Vault ('X-API-Key' header) instead of an unauthenticated call.
Same concern as the parallel Connectivity Apps implementation: an API key cannot be rotated automatically and a single disclosure of 'MarketplaceCatalogApi-Key' affects every tenant. If catalogapi.azure.com offers an OAuth2 client-credentials flow, prefer that over the shared key.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
RecommendedAppsImpl.CheckIfURLExistsAndDownloadLogo now calls AzureKeyVault.GetAzureKeyVaultSecret before any HTTP call, and raises an ErrorInfo('Cannot retrieve the Marketplace Catalog API key from Azure Key Vault.') when that TryFunction returns false.RecommendedAppsTests.Codeunit.al was updated to expect the old 404-not-reachable message but never calls 'Azure Key Vault Test Library'.SetAzureKeyVaultSecretProvider to mock a secret. Without a mocked provider, AzureKeyVaultImpl.GetSecretFromClient returns an empty secret and the TryFunction fails, so every test that reaches InsertApp (TestInsertAppWhenURLAppInfoAreWrong, and — via the InsertSingleApp helper — TestGetApp, TestRefreshImage, TestDeleteApp, TestGetAppURL, TestGetAppURLWrongAppId) will now raise 'Cannot retrieve the Marketplace Catalog API key from Azure Key Vault.' instead of reaching the HTTP call, so Assert.ExpectedError(Err1Msg) and the other assertions will not match the actual error. If this were run, the actual impact is every test in the codeunit failing (a major/blocker-level break), but per BCQuality's agent-finding cap this is reported as minor; add a call to the Azure Key Vault Test Library's SetAzureKeyVaultSecretProvider (mocking 'MarketplaceCatalogApi-Key') in each test, or in a shared setup, before this can be treated as resolved. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why |
2 participants
Summary
Replaces the undocumented ^Gppsource.microsoft.com/view/app/\ endpoint (now blocked by Azure WAF) with the official Marketplace Catalog API at \catalogapi.azure.com/products/.
Changes
Files Changed
Prerequisites for deployment
The secret *\MarketplaceCatalogApi-Key* must be provisioned in the environment's Azure Key Vault with the API key obtained from the Marketplace Catalog Team.
Benefits
AB#622276
Testing
Verified the new endpoint end-to-end against live AppSource apps. A
GETto the Marketplace Catalog API (with theX-API-Keyfrom Key Vault) returns200 OKand thelargeIconUrifield at the top level of the response, matching the updated parsing. Malformed requests behave as expected (wrong path → 404, missingapi-version→ 400, invalid key → 403).