Skip to content

Fix effective permission filtering #9692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
darjoo wants to merge 4 commits into
base: main
Choose a base branch
from
+119 −2
Open

Fix effective permission filtering #9692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
952ad44
Fix effective permission filtering
darjoo Jul 23, 2026
81bde8f
Refactor expanded permission lookup
darjoo Jul 23, 2026
f051f5c
Revert expanded permission lookup refactor
darjoo Jul 23, 2026
31fbfdd
Merge origin/main into darjoo-fix-effective-permissions-filters
darjoo Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/Layers/W1/BaseApp/System/Permissions/EffectivePermissionsMgt.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,12 @@ codeunit 9852 "Effective Permissions Mgt."
ExpandedPermission: Record "Expanded Permission";
PermissionSetBuffer: Record "Permission Set Buffer";
AssignedRead, AssignedInsert, AssignedModify, AssignedDelete, AssignedExecute : Integer;
ExpandedPermissionFound: Boolean;
begin
PermissionBuffer.Reset();
PermissionBuffer.DeleteAll();

ExpandedPermission.SetRange("Object Type", PassedObjectType);
ExpandedPermission.SetFilter("Object ID", '%1|%2', 0, PassedObjectId);

// find permissions from all permission sets for this user
AccessControl.SetFilter("User Security ID", GetAccessControlFilterForUser(PassedUserID));
Expand All @@ -327,7 +327,16 @@ codeunit 9852 "Effective Permissions Mgt."

ExpandedPermission.SetRange("App ID", AccessControl."App ID");
ExpandedPermission.SetRange("Role ID", AccessControl."Role ID");
if ExpandedPermission.FindFirst() then begin

// Specific object permissions override the wildcard entry.
ExpandedPermission.SetRange("Object ID", PassedObjectId);
ExpandedPermissionFound := ExpandedPermission.FindFirst();
if not ExpandedPermissionFound then begin
ExpandedPermission.SetRange("Object ID", 0);
ExpandedPermissionFound := ExpandedPermission.FindFirst();
end;

if ExpandedPermissionFound then begin
FillPermissionBufferFromExpandedPermission(PermissionBuffer, ExpandedPermission);
SetHighestAssignedPermission(PermissionBuffer, AssignedRead, AssignedInsert, AssignedModify, AssignedDelete, AssignedExecute);
PermissionBuffer.Order := PermissionBuffer.Source;
Expand Down
108 changes: 108 additions & 0 deletions src/Layers/W1/Tests/Permissions/PermissionsTest.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,114 @@ codeunit 139400 "Permissions Test"
RecRef.Delete();
end;

[Test]
[TransactionModel(TransactionModel::AutoRollback)]
[Scope('OnPrem')]
procedure PopulatePermissionBufferPrefersSpecificObjectOverWildcard()
var
TenantPermissionSet: Record "Tenant Permission Set";
TenantPermission: Record "Tenant Permission";
AccessControl: Record "Access Control";
PermissionBuffer: Record "Permission Buffer" temporary;

@github-actions github-actions Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The temporary Record variable in the new test PopulatePermissionBufferPrefersSpecificObjectOverWildcard is named PermissionBuffer instead of TempPermissionBuffer. The file already follows the Temp-prefix convention elsewhere (e.g. TempPermissionSetBuffer), so this new declaration and its five usages in the method (var declaration and the calls to PopulatePermissionBuffer, SetRange, FindFirst, and the Assert.AreEqual comparisons) should be renamed to make clear it is an in-memory buffer, not a database write.

Knowledge:

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

EffectivePermissionsMgt: Codeunit "Effective Permissions Mgt.";
SpecificPageId: Integer;
begin
// [SCENARIO 615963] A specific object permission is displayed instead of the wildcard permission
// [GIVEN] A permission set with an indirect wildcard permission
SpecificPageId := Page::"Customer Card";
LibraryPermissions.CreateTenantPermissionSet(TenantPermissionSet, LibraryUtility.GenerateGUID(), NullGuid);

TenantPermission.Init();
TenantPermission."App ID" := NullGuid;
TenantPermission."Role ID" := TenantPermissionSet."Role ID";
TenantPermission."Object Type" := TenantPermission."Object Type"::Page;
TenantPermission."Object ID" := 0;
TenantPermission."Execute Permission" := TenantPermission."Execute Permission"::Indirect;
TenantPermission.Insert(true);

// [GIVEN] The same permission set grants direct access to a specific page
TenantPermission.Init();
TenantPermission."App ID" := NullGuid;
TenantPermission."Role ID" := TenantPermissionSet."Role ID";
TenantPermission."Object Type" := TenantPermission."Object Type"::Page;
TenantPermission."Object ID" := SpecificPageId;
TenantPermission."Execute Permission" := TenantPermission."Execute Permission"::Yes;
TenantPermission.Insert(true);

// [GIVEN] The permission set is assigned to the current user
AccessControl.Init();
AccessControl."User Security ID" := UserSecurityId();
AccessControl."Role ID" := TenantPermissionSet."Role ID";
AccessControl.Scope := AccessControl.Scope::Tenant;
AccessControl."Company Name" := CompanyName();
AccessControl.Insert(true);

// [WHEN] The permission buffer is populated for the specific page
EffectivePermissionsMgt.PopulatePermissionBuffer(
PermissionBuffer, UserSecurityId(), CompanyName(), TenantPermission."Object Type"::Page, SpecificPageId);

// [THEN] The specific page permission is displayed
PermissionBuffer.SetRange("Permission Set", TenantPermissionSet."Role ID");
Assert.IsTrue(PermissionBuffer.FindFirst(), 'Permission buffer should contain the permission set.');
Assert.AreEqual(
PermissionBuffer."Execute Permission"::Yes, PermissionBuffer."Execute Permission",
'The specific page permission should override the wildcard permission.');
end;

[Test]
[TransactionModel(TransactionModel::AutoRollback)]
[Scope('OnPrem')]
procedure PopulatePermissionBufferPrefersSpecificEmptyOverWildcard()
var
TenantPermissionSet: Record "Tenant Permission Set";
TenantPermission: Record "Tenant Permission";
AccessControl: Record "Access Control";
PermissionBuffer: Record "Permission Buffer" temporary;

@github-actions github-actions Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The temporary Record variable in the new test PopulatePermissionBufferPrefersSpecificEmptyOverWildcard is named PermissionBuffer instead of TempPermissionBuffer. The file already follows the Temp-prefix convention elsewhere (e.g. TempPermissionSetBuffer), so this new declaration and its five usages in the method should be renamed to make clear it is an in-memory buffer, not a database write.

Knowledge:

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

EffectivePermissionsMgt: Codeunit "Effective Permissions Mgt.";
SpecificPageId: Integer;
begin
// [SCENARIO 615963] A specific exclusion is displayed instead of the wildcard permission
// [GIVEN] A permission set with a wildcard permission
SpecificPageId := Page::"Customer Card";
LibraryPermissions.CreateTenantPermissionSet(TenantPermissionSet, LibraryUtility.GenerateGUID(), NullGuid);

TenantPermission.Init();
TenantPermission."App ID" := NullGuid;
TenantPermission."Role ID" := TenantPermissionSet."Role ID";
TenantPermission."Object Type" := TenantPermission."Object Type"::Page;
TenantPermission."Object ID" := 0;
TenantPermission."Execute Permission" := TenantPermission."Execute Permission"::Yes;
TenantPermission.Insert(true);

// [GIVEN] The same permission set excludes a specific page
TenantPermission.Init();
TenantPermission."App ID" := NullGuid;
TenantPermission."Role ID" := TenantPermissionSet."Role ID";
TenantPermission."Object Type" := TenantPermission."Object Type"::Page;
TenantPermission."Object ID" := SpecificPageId;
TenantPermission."Execute Permission" := TenantPermission."Execute Permission"::" ";
TenantPermission.Insert(true);

// [GIVEN] The permission set is assigned to the current user
AccessControl.Init();
AccessControl."User Security ID" := UserSecurityId();
AccessControl."Role ID" := TenantPermissionSet."Role ID";
AccessControl.Scope := AccessControl.Scope::Tenant;
AccessControl."Company Name" := CompanyName();
AccessControl.Insert(true);

// [WHEN] The permission buffer is populated for the specific page
EffectivePermissionsMgt.PopulatePermissionBuffer(
PermissionBuffer, UserSecurityId(), CompanyName(), TenantPermission."Object Type"::Page, SpecificPageId);

// [THEN] The specific page exclusion is displayed
PermissionBuffer.SetRange("Permission Set", TenantPermissionSet."Role ID");
Assert.IsTrue(PermissionBuffer.FindFirst(), 'Permission buffer should contain the permission set.');
Assert.AreEqual(
PermissionBuffer."Execute Permission"::" ", PermissionBuffer."Execute Permission",
'The specific page exclusion should override the wildcard permission.');
end;

//[Test] ignore 426467
[HandlerFunctions('SendResolveNotificationHandler')]
[TransactionModel(TransactionModel::AutoRollback)]
Expand Down
Loading