Fix activity log indirect permissions

[Prangshuman Das (t-prda)]

Summary

Fix SaaS entitlement failures when Expense Agent or Expense Management users create lifecycle activity entries.

• Grant Expense Mgmt. Edit indirect insert/modify/delete permission on Expense Activity Log Entry.
• Grant Expense Agent - Data direct read plus indirect insert/modify/delete (Rimd).
• Keep all activity writes code-mediated through Expense Activity Log Mgt.; no caller receives direct insert, modify, or delete access.
• Add restrictive permission tests for a realistic D365 BASIC human user and the composite Expense Agent role.
• Assert both scenarios lack direct activity-log insert permission, proving the write remains code-mediated.

The existing permission hierarchy carries the fix to users who can create/edit/delete expense reports:

• Expense Mgmt. Admin includes Expense Mgmt. Edit.
• D365 Basic and Team Member permission extensions include Expense Mgmt. Edit.
• Full-access inherits the Expense Management admin role.
• Read-only users retain direct R only.

Issue

Fixes AB#647154

Validation

• Permission composition reviewed against the activity management codeunit (rimd).
• Added restrictive permission regression tests in codeunit 148338.
• Verified the previous BE/CZ failure came from testing Expense Mgmt. Edit without its Base Application role.
• Compile and run codeunit 148338 in Business Central.

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing}$

The new local procedure VerifyPermissionSetCanInsertActivity lowers the execution context with LibraryLowerPermissions.SetExactPermissionSet and only restores it afterward via SetOutsideO365Scope; Initialize() does not unconditionally reset permissions at the start of the run. If an assertion or the LogExpenseReportEvent call fails/errors between SetExactPermissionSet and SetOutsideO365Scope, the lowered permission set leaks into whatever test runs next in this codeunit, causing unrelated tests to fail for the wrong reason. The codebase's own established pattern for this (Expense Event Subs. Perm. Test.Initialize) calls RestoreFullPermissions()/SetOutsideO365Scope() unconditionally at the very start of Initialize(), before the IsInitialized early-exit, specifically to guard against this leak; ExpenseActivityLogTest.Initialize() was not updated to do the same when permission-lowering was introduced into this codeunit.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    local procedure Initialize()
    begin
        LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Activity Log Test");
        LibraryLowerPermissions.SetOutsideO365Scope();
        if IsInitialized then
            exit;

        LibraryExpense.SetupNumberSeriesInExpenseMgmt();
        IsInitialized := true;
        LibraryTestInitialize.OnAfterTestSuiteInitialize(Codeunit::"Expense Activity Log Test");
    end;

Knowledge:

• microsoft/knowledge/testing/permission-tests-must-lower-the-execution-context.md

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing}$

The new deletion-guard helpers (CreateExpenseForDeletionGuard, CreateExpenseReportForDeletionGuard, CreatePostedExpenseReportForDeletionGuard) hand-roll Expense, Expense Report Header, and Posted Expense Report Header fixtures with Init/Insert and ad-hoc GUID-derived keys instead of using supported library-backed creation flows (e.g. Library - Expense). This bypasses table validation and prerequisite setup that production-valid records go through, so the permission tests may pass against record shapes that would never be reachable through normal application flows, weakening the guarantee the tests provide.

Knowledge:

• microsoft/knowledge/testing/use-library-codeunits-for-test-fixtures.md

Posting this finding as an issue comment because inline comment placement failed.

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

[Prangshuman Das (t-prda)]

🟠 High Severity — Testing

The new local procedure VerifyPermissionSetCanInsertActivity lowers the execution context with LibraryLowerPermissions.SetExactPermissionSet and only restores it afterward via SetOutsideO365Scope; Initialize() does not unconditionally reset permissions at the start of the run. If an assertion or the LogExpenseReportEvent call fails/errors between SetExactPermissionSet and SetOutsideO365Scope, the lowered permission set leaks into whatever test runs next in this codeunit, causing unrelated tests to fail for the wrong reason. The codebase's own established pattern for this (Expense Event Subs. Perm. Test.Initialize) calls RestoreFullPermissions()/SetOutsideO365Scope() unconditionally at the very start of Initialize(), before the IsInitialized early-exit, specifically to guard against this leak; ExpenseActivityLogTest.Initialize() was not updated to do the same when permission-lowering was introduced into this codeunit.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    local procedure Initialize()
    begin
        LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Activity Log Test");
        LibraryLowerPermissions.SetOutsideO365Scope();
        if IsInitialized then
            exit;

        LibraryExpense.SetupNumberSeriesInExpenseMgmt();
        IsInitialized := true;
        LibraryTestInitialize.OnAfterTestSuiteInitialize(Codeunit::"Expense Activity Log Test");
    end;

Knowledge:

• microsoft/knowledge/testing/permission-tests-must-lower-the-execution-context.md

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4

Closed with RestoreFullPermissions();

[github-actions]

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

The new deletion-guard fixtures hand-roll business records with Init(), invented primary keys, and Insert(false) instead of building them through the test library/domain helpers. That makes the permission tests brittle: they can pass on records that skipped normal validation and will start failing for setup reasons when these tables gain required fields or validation logic.

Knowledge:

• microsoft/knowledge/testing/use-library-codeunits-for-test-fixtures.md

Posting this finding as an issue comment because inline comment placement failed.

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

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Accept with Suggestions

What this PR does

This PR grants indirect write permissions on Expense Activity Log Entry to the Expense Management edit path and to the Expense Agent data role. The main insert path is correct: callers still do not get direct write access, and Expense Activity Log Mgt. already carries the
imd permission needed to insert the entry through code.

Suggestions

S1 - Cover indirect modify and delete rights
The new grants include indirect modify and delete, but the restrictive tests only prove indirect insert. Please add coverage for ReassignExpenseReportEntriesToPosted and DeleteEntriesForSource, so the m and d rights are checked too.

Risk assessment and necessity

Risk: The direct write surface stays closed because the tests assert WritePermission() is false, and activity writes still go through Expense Activity Log Mgt.. The remaining risk is coverage: posting reassigns activity entries to posted reports, and delete triggers remove entries through the same table permissions.

Necessity: The change is needed to fix entitlement failures for users who are allowed to create expense lifecycle activity entries. The scope is small and matches the existing permission hierarchy for D365 BASIC, Expense Management edit, and the Expense Agent role.

---

[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10346 round=1 by=alexei-dobriansky at=2026-08-19T19:15:49Z lastSha=df52cd5b5b6c6b53bf23254d05f83b6cd96ff173 reviewKey=a88b7e6be5b46e42453e13575600cda3e9d95326bfb3450c2eb8ceef02d90e57 suggestions=S1@a67ce48a