[API tests] Default to web service key authentication

[Prangshuman Das (t-prda)]

Review focus: MicrosoftTestAuthProvider (about 60 lines) and the enum default; the build/ changes are deletions plus the anti-SSRF line in NewBcContainer.ps1.

Defaults API test authentication (Library - Graph Mgt) to the current user's web service key. API tests no longer need per-app auth uptake or a container password.

• API Test Authentication enum: DefaultImplementation = Microsoft Test Auth Provider. Removes No API Test Auth Provider and the explicit Microsoft Test Environment value.
• Microsoft Test Auth Provider: on-premises with NavUserPassword, sends Basic auth with UserId() and the web service key. When the key is missing or expired, a background session (API Test Web Service Key, CU 131025) creates it with a 24h expiry and commits it, because the API request runs in a separate server session. The caller's transaction is never committed; the provider waits up to 60 s for the session and then re-reads the key.
• Library - Graph Mgt: adds the SetLicenseSafeWorkDate and AddFieldToIgnoreIfExists helpers used by later layers.
• Build: removes the container password and Key Vault credential plumbing (ApiTestCredential.psm1, Remove-ApiTestPassword.ps1, PipelineFinalize hooks and their tests). Keeps the clean-tenant execution logic. Sets NavHttpClientAntiSSRFEnabled=false in the test container, as NAV RunALTests.ps1 does: since Migrate API test libraries off Http Web Request Mgt. #12323, API tests call the container's own endpoint through AL HttpClient, which anti-SSRF blocks.
• CU 139494 API Test Auth Provider Tests: covers key creation and renewal. Declares TestHttpRequestPolicy = AllowAllOutboundRequests (NAV blocks outbound requests by default; Allow API tests to issue local HTTP requests #12694 covered the other API tests).

Related: AB#646383, AB#653393

Stack (each PR shows only its own layer; merge bottom first)

1. [API tests] Default to web service key authentication #12674 A: web service key auth (base: main)
2. [Expense tests] Re-enable Expense Agent API tests #12675 B: Expense API tests + clean test codeunit execution
3. [API tests] Re-enable APIV1 and APIV2 API tests #12676 C: re-enable APIV1/APIV2 API tests
4. Area fixes: [API tests] Ensure VAT posting setup in API document fixtures #12677 D1 VAT fixtures → [API tests] Ensure cancellation reason codes in API sales document tests #12678 D2 cancellation reasons → [API tests] Handle localized purchase invoice payment discounts in APIV1 tests #12679 D3 localized discounts → [API tests] Set sales quote document and posting dates in API tests #12680 D4 quote dates → [API tests] Assign a number series to API vendor payment journals #12681 D5 journal numbering → [API tests] Ignore localized fields in API document comparisons #12682 D6 field comparison → [API tests] Return shipment PDF selection and pdfDocument test isolation #12683 D7 PDF documents → [API tests] Improve APIV1 fixture and verification reliability #12684 D8 APIV1 reliability

Experiments (on D8, do not merge): #12685 E single tenant, #12686 F no clean execution. Combined validation (do not merge): #12687, same tree as D8.

Platform: main already pins BCPlatform 30.0.56112.0, which includes the SQL connection-pool fix (AB#653457).

Note for CI: AL-Go's new-warning check needs a CI/CD baseline from the base branch, and features/* bases have none, so a PR build there fails with a 404. The current CI runs were triggered while these PRs targeted main, so their results are valid for the cumulative change.

[alexei-dobriansky]

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This change makes API test requests use the current user's web service key for NavUserPassword containers, removes the pipeline-managed password files, and allows calls to the container's local endpoint. The key reuse and expiry checks match the authentication goal, but the provider commits the caller's transaction and the enum update removes public symbols, so the current implementation can give wrong test results and break dependent test apps.

Problem-solution fit

Fit: Partial

The change removes the external credential dependency and covers key creation and renewal. The transaction side effect and compatibility break mean it does not yet provide a safe default for all existing API tests.

Suggestions

S1 (🔴 High): Preserve the existing enum values
Removing None and Microsoft Test Environment breaks test apps that reference these public enum values. Keep them, add Default as a new value, and initialize Library - Graph Mgt to the new value.

S2 (🔴 High): Move key creation outside request transactions
The Commit() can run inside an asserterror API call, so the test can catch the COMMIT error and pass without sending the expected HTTP request. Provision or renew the key at a runner or fixture boundary, or use an isolated session that does not commit the caller's transaction.

S3 (🟠 Moderate): Avoid persisting current user key changes
These tests clear and replace the current user's key with isolation disabled, and they never restore a safe state. Use a disposable test user or fixture cleanup so later tests and local sessions do not inherit an expired or rotated credential.

Risk assessment and necessity

Risk: The default provider is used by every Library - Graph Mgt request. A missing or expired key can therefore change transaction behavior across API tests, including negative tests, while the enum removals can stop dependent test apps from compiling. The container setting also broadens local-address access for all tests in these containers, although it is limited to test infrastructure.

Necessity: Removing the pipeline-managed password and making API authentication self-contained is needed to restore reliable API coverage. The core direction is sound, but the key must be provisioned without committing arbitrary test work and the existing public enum surface should remain compatible.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=12674 round=1 by=alexei-dobriansky at=2026-10-13T22:37:30Z lastSha=3a7dc6e06903f884d14c5c61d17023cb47c1197d reviewKey=ff722cc559b094078eadacc16983b5923c98300222c57c1e6421daff98959135 suggestions=S1@a2702056,S2@e0f4b9a0,S3@d68606b9

[Prangshuman Das (t-prda)]

Thanks for the review. Replies per suggestion:

S1 (enum values) — Not a compatibility break in practice. None and Microsoft Test Environment were introduced in #10085 (merged 2026-09-29) and are not in any release branch (checked releases/26.x and releases/29.0). The only reference is API Test Auth Provider Tests (CU 139494), which this PR updates, and there are no references in the NAV repo. Since no shipped version exposes those values, we are simplifying the enum to a single Default value before it ships rather than carrying two obsolete values forever.

S2 (Commit during key creation) — Agreed this is a real concern; discussing the approach and will follow up here.

S3 (current user key) — In CI, CU 139494 runs with RequiredTestIsolation = Disabled, so it executes on a disposable clean tenant copied from the template, and every test leaves a valid key behind. Locally it does rotate the developer's key; we will correct the test comments that call it a "disposable test user" but will not add restore logic.

[Prangshuman Das (t-prda)]

S2 (Commit during key creation): addressed in ee1c82a. Microsoft Test Auth Provider no longer calls Commit(). When the key is missing or expired, it starts a background session (new internal CU 131025 API Test Web Service Key) that creates the key (24h expiry). The key is committed by that session's own transaction, so the caller's transaction, including any asserterror block, is never committed. The provider waits for the session to end (60 s timeout, then StopSession and a clear error), calls SelectLatestVersion(), re-reads the key, and fails if it is still missing or expired. This follows the existing StartSession + Active Session wait pattern in CreateCompanyTests. CU 139494 still covers creation, reuse and renewal.

S3: the three test comments now say "current user" instead of "disposable test user". No restore logic, as discussed above.

[github-actions]

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

The public CreateTargetURLWithTwoSubpages procedure in the library codeunit has no XML documentation. Add a summary describing the URL construction and a param tag for each of its six parameters.

Knowledge:

• microsoft/knowledge/style/xmldoc-for-public-library-procedures.md

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

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

[alexei-dobriansky]

Good Sense Reviewer - Round 2

Recommendation: Request Changes

What this PR does

The update moves web service key creation to a separate background session, so it no longer commits the caller's transaction. That fixes the original transaction problem, but the default provider can now fail before the HTTP request when it is called from a normally isolated test codeunit.

Status of previous suggestions

ID	Title	Status	Author response
S1	Preserve the existing enum values	Disputed	The values are absent from release branches, and repository searches found no remaining consumers.
S2	Move key creation outside request transactions	Addressed	Key creation now commits in a separate background session.
S3	Avoid persisting current user key changes	Disputed	CI uses a clean tenant; a local run intentionally rotates the current user's key.

New observations (commits since round 1)

S4 (🔴 High): Create the key before isolated tests run
Library - Graph Mgt uses this provider from test codeunits that run with codeunit isolation. Background sessions cannot start under that isolation, so a missing or expired key makes those API tests fail before the request; provision the key at the runner or clean-tenant boundary, or use a path that does not start a session from the test transaction.

Risk assessment and necessity

Risk: The default provider is shared by API test callers. Several callers use the normal test runner with codeunit isolation, while the new provider test disables isolation, so the added tests do not cover the failing caller context. The separate session otherwise removes the caller-transaction commit risk, and the enum values are not present in release branches.

Necessity: Self-contained API authentication is needed to remove the pipeline credential dependency and restore API coverage. The key must be provisioned outside an isolated test transaction so the default works for existing API test codeunits.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=12674 round=2 by=alexei-dobriansky at=2026-10-14T13:15:58Z lastSha=ee1c82a8e98fdd32cb135fdde3f67ab08eb1e067 reviewKey=356aa577066b5799a6768e96d0411c2a4b9884f95d7a8101e7b5e08219a42bff suggestions=S1@a2702056:disputed,S2@e0f4b9a0:addressed,S3@d68606b9:disputed,S4@7c56c3c0:new parentRound=1

[Prangshuman Das (t-prda)]

S4: agreed, and CI confirmed it: TestGetApprovalEntries failed in the IntegrationTests lane with "Sessions can only be started in tests that are run by a TestRunner that has TestIsolation set to Disabled". Fixed in 0bfb1b2, pushed after the commit this round reviewed (ee1c82a):

• The provider first tries StartSession inside a [TryFunction]. Under a non-isolating runner (RequiredTestIsolation = Disabled codeunits, CU 139494) the key is still created in the background session, so S2 stays addressed there.
• If the runner does not allow sessions, it runs the same codeunit (API Test Web Service Key) in the caller's session and commits, which is the pre-round-1 behavior that was green in these lanes. An isolating runner rolls back the test's changes when it ends, including committed ones, so the caller-transaction concern from S2 does not apply in that context.
• Coverage: CU 139494 covers the background-session path. The isolated path is covered by the existing API test codeunits in the IntegrationTests and Uncategorized lanes (for example TestGetApprovalEntries), which run on the current CI.

On provisioning at the runner or clean-tenant boundary: each clean tenant is copied from a template taken before any test runs, so a key provisioned in the pipeline would remove the in-test creation in CI. I kept the library self-contained instead, so local runs and NAV need no pipeline setup. It can be added later as an optimization if wanted.

[alexei-dobriansky]

Good Sense Reviewer - Round 3

Recommendation: Request Changes

What this PR does

The latest change handles the test-isolation StartSession failure by creating the key in the caller's session and committing it. This lets ordinary isolated API tests proceed, but it reopens the caller-transaction problem for API requests inside asserterror: they can fail on COMMIT before any HTTP request is sent.

Status of previous suggestions

ID	Title	Status	Author response
S1	Preserve the existing enum values	Disputed	The values have not shipped, and no remaining consumers were found.
S2	Move key creation outside request transactions	Disputed	The isolated runner rolls back committed changes, so the caller-side commit is considered safe.
S3	Avoid persisting current user key changes	Disputed	CI uses clean tenants, and local key rotation remains intentional.
S4	Create the key before isolated tests run	Addressed	The fallback creates the key when test isolation blocks the background session.

New observations (commits since round 2)

None - the latest change addresses S4 but reopens S2.

Risk assessment and necessity

Risk: The default provider is shared by API test codeunits, and several requests run inside asserterror. If the key is first created there, the caller-side Commit() can raise the wrong runtime error before the HTTP request. A negative test can then pass without exercising the endpoint. The background-session path remains safe, and ordinary isolated requests can now proceed.

Necessity: Self-contained API authentication is needed to remove pipeline credential handling and restore portable API coverage. Key creation must still avoid committing the caller's assertion transaction.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=12674 round=3 by=alexei-dobriansky at=2026-10-14T20:03:30.3100570Z lastSha=0bfb1b2e10b82212212e6f40d64c5894f9d6f183 reviewKey=4ddb062182dcd6818c90a57cdfc2d111e89328186879be356021af3d23b90393 suggestions=S1@a2702056:disputed,S2@e0f4b9a0:disputed,S3@d68606b9:disputed,S4@7c56c3c0:addressed parentRound=2