Fix API test authentication in BCApps

[Prangshuman Das (t-prda)]

Summary

Enable AL API tests in BCApps UserPassword containers while preserving existing Windows/SaaS authentication behavior. The solution combines pluggable request authentication with clean-tenant execution for tests that commit data.

AB#646383

Current update: ae9265e44c merges main b6ff7f07fa and resolves the Expense exclusion-list conflict while preserving upstream bug links and existing auth re-enablement. The PR is conflict-free; 104 affected PowerShell tests and PowerShell CI pass. Fresh runtime CI is running. Expense re-enablement remains a later step; earlier runtime results below do not validate this refreshed head.

Why both changes are needed

• Authentication: NAV's Windows-authenticated tests can use ambient credentials. A BCApps UserPassword container needs explicit credentials on the HTTP requests made by Library - Graph Mgt.; authenticating the AL test runner alone is insufficient.
• Isolation: an API request executes in a separate NST session. It must see committed fixtures, so ordinary test-transaction rollback cannot isolate the complete scenario.

Authentication design

AL test Initialize
  -> Graph instance selects API Test Authentication enum
  -> API Test Auth Provider configures API Test Auth Context
  -> context applies authentication to the HTTP request
  -> final customization event -> NST API

The extensible enum/interface keeps environment-specific credential handling out of individual tests. None is the default no-op provider.

The Microsoft provider leaves Windows/SaaS ambient behavior unchanged. Otherwise, it uses the current AL user's ID and the container password file, falling back to the existing Key Vault secret only when the file is absent. Invalid configured credentials fail explicitly. A successfully read password is cached as SecretText on the provider instance, not globally.

Tests select the provider in Initialize on the same retained Graph instance. Re-selecting it is idempotent. OnAfterInitializeWebRequestWithURL remains the final customization event. License-safe WorkDate setup runs before initialization guards where required, because the runner resets session WorkDate between methods.

The trust boundary is admitted OnPrem test code and environment credential access—not Internal or a destination-URL restriction.

Pipeline design

The existing test lanes remain; there is no separate API-test job.

1. Provision: create the multitenant test container and write its UserPassword credential directly into the shared mount. A protected file ACL is applied atomically before writing; there are no plaintext staging files. The writer encodes from scoped, zeroed credential buffers rather than creating another immutable plaintext string.
2. Prepare: compile/publish the selected apps. Discover RequiredTestIsolation = Disabled codeunits on a secondary tenant, then restore that worker so discovery cannot contaminate the primary fixture.
3. Execute: create a pristine database template and restore secondary workers before each clean codeunit, including retries. Isolation is per codeunit, not per method.
4. Rejoin: restore workers before ordinary tests and merge results into the normal JUnit output. Preserve app/type/exclusion selection and existing Legacy execution.
5. Clean up: the final workflow step stops remaining container consumers and removes the credential backing file before teardown, including failure/cancellation paths where the runner remains available.

Task Scheduler is explicitly enabled only for the existing Uncategorized profile that contains background-task polling tests. The clean-tenant scheduler does not override NST scheduling settings.

Scope and later Expense re-enablement

Unrelated fixes stay separately reviewable in the native stack: #11224, #11225, #11226, #11227, #11228, #11229, #11230, #11322, and Expense follow-ups #11451–#11454. This base retains the 47 API-business deferrals and 12 Expense cases owned by those follow-ups.

The existing Expense drafts already contain scoped re-enablement, but must be reconciled with newly merged #11333 before use:

PR	Owned cases	Current-main overlap
#11451 Permission observations/denial assertions	4	Capture/restore/assert fix still needed; three cases match AB#650245.
#11452 Negative and posted-report fixtures	5	Commits and posted-number lookup are upstream; its zero-amount deletion fixture must be isolated from new lifecycle tests. Two cases overlap AB#650277.
#11453 Exact approver membership	1	Filter fix remains needed.
#11454 Indirect header updates	2	Main now supplies header Modify permission; preserve the added regression coverage when reconciling the draft.

The base keeps 48 unique Expense exclusions, including upstream bug metadata. Six new posted/reapproval cases under AB#650277 and five new API cases are outside the old stack's ownership and remain excluded. The drafts and validation-only #11455 have not been blindly refreshed: their earlier green results are historical. #11340's three action-result assertion fixes are now upstream through #11333, but those page-action tests remain excluded. CZ prerequisite #11370 remains separate.

Prior-head validation

• Base f516524e10: runtime CI passed, PowerShell CI passed, and all 11 auth-contract/URL tests passed.
• Combined stack cfcf9b5993: CI passed. W1 verifies all 12 Expense fixes, six added regressions and the 11 contract/URL tests; complete affected suites pass 20/20, 56/56 and 31/31. Each run recorded successful credential removal/cleanup in all 113 final test jobs; disconnected first-attempt workers are not counted as successful cleanup.
• Both runtime runs passed after bot-triggered infrastructure retries. Validation-only [Validation only - do not merge] Combined API auth and Expense fix stack #11455 targets main to obtain AL-Go's warning baseline; it must not be merged. Earlier NAV validation passed on an older snapshot, not this current head.

Excluded/absent and tolerated-native failures are not counted as passes. IN/RU retain coverage gaps. Additional Microsoft-provider branch tests and broader work-date normalization remain deferred by agreement; existing failure-driven date fixes are preserved.

[Copilot]

Pull request overview

This PR centralizes Basic authentication injection for Library - Graph Mgt-based API tests so they can run in NavUserPassword (UserPassword) containers, and then re-enables several previously disabled API/E2E test suites. It replaces an app-specific Expense Agent auth helper with a shared TestLibraries subscriber and adds a container-side “credential bridge” file so the test runner can obtain the password without requiring Azure Key Vault.

Changes:

• Added Library - Graph Auth Mgt. as an event subscriber to inject Basic auth for non-Windows test users, sourcing the password from a container file or (fallback) Azure Key Vault.
• Removed the Expense Agent test-only auth helper + manual subscription binding; tests now rely on the shared subscriber.
• Re-enabled multiple API/E2E test suites by removing entries from various *.DisabledTest.json files (and deleting the APIV1/APIV2 exclusion lists), and updated container provisioning to create the API-test password bridge file.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/Layers/W1/Tests/TestLibraries/LibraryGraphAuthMgt.Codeunit.al	New shared subscriber that injects Basic auth into Library - Graph Mgt requests for NavUserPassword scenarios.
build/scripts/NewBcContainer.ps1	Writes the container password bridge file and sets ACLs so server-side AL can read it.
src/Apps/W1/ExpenseAgent/test/src/Helper/ExpenseAPITestAuthHelper.Codeunit.al	Removes app-specific auth injection helper in favor of shared TestLibraries implementation.
src/Apps/W1/ExpenseAgent/test/src/API/ExpenseUsersAPITest.Codeunit.al	Drops manual subscription binding to the removed helper.
src/Apps/W1/ExpenseAgent/test/src/API/ExpenseProjectsAPITest.Codeunit.al	Drops manual subscription binding to the removed helper.
src/Apps/W1/ExpenseAgent/test/src/API/ExpensePerDiemLocationsTest.Codeunit.al	Drops manual subscription binding to the removed helper.
src/Apps/W1/ExpenseAgent/test/src/API/ExpenseCapabilitiesAPITest.Codeunit.al	Drops manual subscription binding to the removed helper.
src/DisabledTests/Tests-Integration/Tests-Integration.DisabledTest.json	Re-enables specific integration API tests by removing disable entries.
src/DisabledTests/Tests-Graph/Tests-Graph.DisabledTest.json	Re-enables specific Graph E2E tests by removing disable entries.
src/DisabledTests/Sustainability_Tests/Sustainability_Tests.DisabledTest.json	Re-enables Sustainability API tests by removing disable entries.
src/DisabledTests/Quality_Management-Tests/Quality_Management-Tests.DisabledTest.json	Re-enables Quality Management API tests by removing disable entry.
src/DisabledTests/IRS_Forms_Tests/IRS_Forms_Tests.DisabledTest.json	Re-enables IRS 1099 API test by removing disable entry.
src/DisabledTests/E-Document_Core_Tests/E-Document Core Tests.DisabledTest.json	Re-enables E-Document API tests by removing disable entry.
src/DisabledTests/_Exclude_APIV2__Tests/_Exclude_APIV2__Tests.DisabledTest.json	Deletes the APIV2 exclusion list (re-enables APIV2 suite).
src/DisabledTests/_Exclude_APIV1__Tests/_Exclude_APIV1__Tests.DisabledTest.json	Deletes the APIV1 exclusion list (re-enables APIV1 suite).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Prangshuman Das (t-prda)]

Agentic PR Review - Round 1

Recommendation: Accept

What this PR does

This PR adds an explicit authentication bridge for API tests that use Library - Graph Mgt. in BCApps UserPassword containers. The subscriber is manual, each affected API test codeunit opts in, Windows-authenticated NAV gates keep their existing behavior, and local NAV UserPassword runs can use the existing Key Vault secret.

The change addresses the gate difference directly. It does not change application API behavior, and it avoids making partner test code automatically depend on the BCApps credential bridge.

Suggestions

None.

Risk assessment and necessity

Risk: The change touches shared test infrastructure and re-enables many existing suites, so CI isolation and concurrency failures may still need separate gate work. The authentication subscriber itself is manually scoped and internal.

Necessity: The change is required because BCApps runs these tests with UserPassword while NAV's normal uptake gates use Windows authentication. Without the bridge, the API suites fail with 401 responses and remain disabled.

---

[AI-PR-REVIEW] version=1 promptVersion=1 system=github pr=10085 round=1 by=t-prda at=2026-08-12T09:27:33Z lastSha=6fe7981474d813fcafab57bc6e6fa0c429df80a5 reviewKey=e0d301ad1af589367637a08b287af9af4fb26402df7532abc2876a0a48dbabc7 suggestions=none

[alexei-dobriansky]

Good Sense Reviewer - Round 15

Recommendation: Accept with Suggestions

What this PR does

This change adds shared authentication for API tests, prepares clean tenants for tests that require Disabled isolation, and re-enables affected API suites under UserPassword while keeping ambient-auth runs.

Since round 14, the only author commit adds a narrow set of temporary Expense test deferrals for confirmed permission and spend-request failures. That does not change product code and does not address the two older runner concerns, so those remain open.

Status of previous suggestions

ID	Title	Status	Author response
S1	Separate the production report change	Addressed	Still outside the current net diff.
S2	Cache the mock Key Vault setup	Addressed	The provider still caches only a successfully resolved password and keeps the container credential ahead of the fallback source.
S3	Keep the affected-app test exact	Addressed	The structural test still checks the exact Graph-requesting codeunits and the current scope.
S4	Clarify the E-Doc Graph variables	Addressed	The current diff does not reintroduce the unclear Graph variable pattern.
S5	Clean up the container password file	Addressed	The setup still records the backing file and the final cleanup stops consumers before deleting it.
S6	Narrow the transient error match	Not addressed	The classifier still treats a plain ClientSession State is InError as transient without the page 130455 or InteractionManager context.
S7	Use separate clean-test result files	Not addressed	Clean Disabled-isolation codeunits still start jobs with the tenant suffix, so later normal app jobs on the same tenant can reuse that result file.

New observations (commits since round 14)

None - the latest commit only adds specific temporary test deferrals and does not add a new changed-span issue in the runner.

S6 (🟠 Moderate): Narrow the transient error match
The transient detector still matches any ClientSession State is InError. That can re-run a real client or test failure as a transient platform issue. Please include the specific page 130455 or InteractionManager context in this match.

S7 (🟠 Moderate): Use separate clean-test result files
Clean Disabled-isolation codeunits still write through the normal tenant-suffixed result file path. A later normal job on that tenant can replace that XML before the final merge. Please use a distinct suffix for clean-codeunit jobs and merge it explicitly.

Risk assessment and necessity

Risk: Product behavior is unchanged, but the test runner path is broad. The latest change narrows the current failing Expense scope into named temporary deferrals, but the remaining runner risk is unchanged: a real failure may be retried as transient, or clean-codeunit result evidence may be overwritten by a later normal job on the same tenant.

Necessity: The API suites need a shared authentication path and clean Disabled-isolation execution to run reliably in UserPassword containers. The latest deferrals may be a practical way to keep this base focused, but they should stay temporary and the two runner issues above should still be fixed or accepted knowingly.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10085 round=15 by=alexei-dobriansky at=2026-09-15T00:04:15Z lastSha=76d18ba3135fa56c87aad1dbccc4c097cd01704e reviewKey=1673eb3b164ee66b9edfe7f466037e911d7b46f544f010ca7e6b7f925568f636 suggestions=S1@b79ed4f7:addressed,S2@373f05d4:addressed,S3@daa56c26:addressed,S4@7d6c760a:addressed,S5@227b86e9:addressed,S6@8ea4b299:notaddressed,S7@66cd1f4e:notaddressed parentRound=14

[Prangshuman Das (t-prda)]

Review follow-up at 76d18ba313: assessed all 13 open inline threads, replied individually, and resolved seven unsupported/by-design concerns. Six useful nonblocking improvements remain open; none was marked fixed without a code change. In particular, the empty first-subpage URL behavior is a real small regression, and host credential staging should establish restrictive access before the first plaintext write (actual cross-user exposure depends on runner ACLs).

For Good Sense round 15: S6 remains useful. A plain ClientSession State is InError is a generic client state, not proof of the known page/InteractionManager race. The one-retry cap limits impact but does not justify the broader classification. S7 is not established for the current AL-Go path: Run-AlPipeline supplies append-to-results flags, dispatch preserves them, and the container test helper loads existing XML before appending suites; final merge/deletion occurs after dispatch completes. The tenant file is intentionally reused, not merged/deleted between codeunits. Separate filenames would be optional hardening for callers without that append contract. This reasoning is based on the helper contract and source trace, not on treating green CI alone as proof against lost results.

No source changes, CI reruns, merges, or NAV selection changes were made in this comment-review pass.

[alexei-dobriansky]

Good Sense Reviewer - Reply

Thanks for the details. Responses to your points:

• S6 - Agree. The one-retry cap limits impact, but the current plain ClientSession State is InError match is still too broad, so this point remains useful.
• S7 - Agree. The current path preserves append flags, appends clean-codeunit suites into the tenant result file, and merges and deletes files only after dispatch completes. I withdraw this point; separate files would only be optional hardening outside that append contract.

---

[AI-PR-REPLY] version=1 promptVersion=4 system=github pr=10085 reviewRound=15 by=alexei-dobriansky at=2026-09-15T18:05:46Z respondsTo=2026-09-15T13:58:22Z reviewKey=1673eb3b164ee66b9edfe7f466037e911d7b46f544f010ca7e6b7f925568f636

[Prangshuman Das (t-prda)]

S6 is now addressed in 4c4b30f: removed the standalone ClientSession State is InError transient match. Focused negative tests cover plain InError and unrelated assertion/permission errors plus InError; positive known race fingerprints and the one-retry cap remain covered. All 102 tests in the affected PowerShell suites pass, and the new PowerShell CI checks pass. Fresh AL/container runtime validation is running. S7 remains withdrawn; no result-file naming changes were made.

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Style}$

The changed file name src/Apps/W1/APIV2/test/src/APIV2itemLedgEntriesE2E.Codeunit.al breaks the <ObjectName>.<ObjectType>.al convention: the object portion is not PascalCase-consistent with codeunit APIV2 - Item Ledg. Entries E2E. Rename it to APIV2ItemLedgEntriesE2E.Codeunit.al so the file-system name stays predictable for grep and tooling.

Knowledge:

• microsoft/knowledge/style/file-name-object-type-pattern.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6}

[alexei-dobriansky]

Good Sense Reviewer - Round 16

Recommendation: Accept

What this PR does

This change enables API tests in UserPassword containers by adding shared request authentication and clean-tenant execution for tests that need committed fixtures.

Since round 15, the new commits narrow transient-failure detection, avoid extra immutable plaintext credential copies, use the declared web-service option type in URL fixtures, and keep WorkDate changes within the agreed failure scope. The two remaining runner concerns are now resolved, and no new changed-span issue was found.

Status of previous suggestions

ID	Title	Status	Author response
S1	Separate the production report change	Addressed	The unrelated production report change remains outside the net diff.
S2	Cache the mock Key Vault setup	Addressed	The provider still returns the cached password first and caches only after a successful lookup.
S3	Keep the affected-app test exact	Addressed	The structural tests still check the exact Graph-requesting codeunits and enabled scope.
S4	Clarify the E-Doc Graph variables	Addressed	The unclear local Graph variable pattern is not present.
S5	Clean up the container password file	Addressed	Setup records the backing path, writes directly to the protected mount, and cleans up partial failures.
S6	Narrow the transient error match	Addressed	Plain ClientSession State is InError no longer triggers a retry; focused negative tests cover it.
S7	Use separate clean-test result files	Addressed	Clean and normal jobs append to the tenant result file, all jobs finish before merge, and the file is deleted only after merge. A separate suffix is not needed.

New observations (commits since round 15)

None - the latest commits address review feedback without adding a new changed-span issue.

Risk assessment and necessity

Risk: The regression surface remains broad test infrastructure: request authentication, tenant restore sequencing, result aggregation, and many API test initializers. The latest changes reduce the identified retry and credential-handling risks; the reported focused PowerShell suite covers those script paths, while fresh container and AL validation still governs the full integration surface.

Necessity: API tests need explicit request credentials in UserPassword containers and clean tenants when fixtures must be committed across NST sessions. The current scope provides both while keeping existing ambient-authentication paths unchanged.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10085 round=16 by=alexei-dobriansky at=2026-09-16T00:24:38Z lastSha=f516524e10b5acdb254b2f429b1789412c48d63a reviewKey=9ec078ab43d788022de7ad3222e4b027bb3a21550eb8ec0af1f4fc410abd9466 suggestions=S1@b79ed4f7:addressed,S2@373f05d4:addressed,S3@daa56c26:addressed,S4@7d6c760a:addressed,S5@227b86e9:addressed,S6@8ea4b299:addressed,S7@66cd1f4e:addressed parentRound=15

[alexei-dobriansky]

Good Sense Reviewer - Round 17

Recommendation: Request Changes

What this PR does

This change enables API tests in UserPassword containers by adding shared request authentication and clean-tenant execution for tests that need committed fixtures.

Since round 16, the head has only been refreshed with the current base and the Expense test deferrals were reconciled. The main design still looks targeted: API requests get explicit credentials only where ambient authentication is not enough, the credential file is written and cleaned up with a protected lifetime, and clean test codeunits run on restored secondary tenants. The runtime build is green, but the current static security check fails on a changed PowerShell test file, so the head is not clean yet.

Problem-solution fit

Fit: Strong

The change matches the reported need to run API tests in UserPassword containers while preserving existing Windows and SaaS behavior. The latest refresh does not expand the feature scope; it keeps unrelated Expense test failures deferred instead of counting them as part of this fix.

Status of previous suggestions

ID	Title	Status	Author response
S1	Separate the production report change	Addressed	The unrelated production report change remains outside the net diff.
S2	Cache the mock Key Vault setup	Addressed	The provider still returns the cached password first and caches only after a successful lookup.
S3	Keep the affected-app test exact	Addressed	The structural tests still check the exact Graph-requesting codeunits and enabled scope.
S4	Clarify the E-Doc Graph variables	Addressed	The unclear local Graph variable pattern is not present.
S5	Clean up the container password file	Addressed	Setup records the backing path, writes directly to the protected mount, and cleans up partial failures.
S6	Narrow the transient error match	Addressed	Plain ClientSession State is InError no longer triggers a retry; focused negative tests cover it.
S7	Use separate clean-test result files	Addressed	Clean and normal jobs append to the tenant result file, all jobs finish before merge, and the file is deleted only after merge.

New observations (commits since round 16)

S8 (🔴 High): Plaintext SecureString fixture fails code scanning
The current head has a PSScriptAnalyzer error on this line. Please build the synthetic credential without ConvertTo-SecureString -AsPlainText, or add a narrow test-only suppression, so the security check is clean.

Risk assessment and necessity

Risk: The regression surface remains broad test infrastructure: request authentication, credential lifetime, tenant restore sequencing, result aggregation, and many API test initializers. The runtime build passed at this head, and there is no BaseApp event-publisher dependency, but the changed PowerShell test file currently fails static security analysis.

Necessity: API tests need explicit request credentials in UserPassword containers and clean tenants when fixtures must be committed across NST sessions. The scope is still justified, but the static security check should be clean before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10085 round=17 by=alexei-dobriansky at=2026-09-19T00:13:27Z lastSha=ae9265e44ca7a53668d2a34f38bc41d7103d16af reviewKey=45f3a9eecb5b9f7baa69fb1ff9641eaef9cfc8de114fa721f807cb1ac9f5a965 suggestions=S1@b79ed4f7:addressed,S2@373f05d4:addressed,S3@daa56c26:addressed,S4@7d6c760a:addressed,S5@227b86e9:addressed,S6@8ea4b299:addressed,S7@66cd1f4e:addressed,S8@7052f386:new parentRound=16