From 2c671ee8d6a5418c2e1256dc706bb0a5b2a15dc8 Mon Sep 17 00:00:00 2001 From: Pooya Kharamesh Date: Tue, 6 Oct 2026 10:11:24 +0200 Subject: [PATCH 1/4] Harden extension management authorization Backport the corrected permission-set checks and regression tests, including the namespace and test compilation repairs from Bug 651703. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../App/Extension Management/app.json | 6 + .../ExtenMgtAdmin.PermissionSet.al | 2 +- .../src/ExtensionInstallationImpl.Codeunit.al | 51 ++++++++- .../ExtMgtNonAdmin.PermissionSet.al | 15 +++ .../src/ExtensionManagementTest.Codeunit.al | 105 +++++++++++++++++- 5 files changed, 174 insertions(+), 5 deletions(-) create mode 100644 src/System Application/Test/Extension Management/permissions/ExtMgtNonAdmin.PermissionSet.al diff --git a/src/System Application/App/Extension Management/app.json b/src/System Application/App/Extension Management/app.json index 248d8748d2c..98a56e23c73 100644 --- a/src/System Application/App/Extension Management/app.json +++ b/src/System Application/App/Extension Management/app.json @@ -70,6 +70,12 @@ "name": "Control Add-Ins", "publisher": "Microsoft", "version": "24.3.0.0" + }, + { + "id": "c56e3ef4-7ab0-4636-ae87-013a62f12213", + "name": "User Permissions", + "publisher": "Microsoft", + "version": "24.3.0.0" } ], "internalsVisibleTo": [ diff --git a/src/System Application/App/Extension Management/permissions/ExtenMgtAdmin.PermissionSet.al b/src/System Application/App/Extension Management/permissions/ExtenMgtAdmin.PermissionSet.al index 720da8efa42..620828d9b05 100644 --- a/src/System Application/App/Extension Management/permissions/ExtenMgtAdmin.PermissionSet.al +++ b/src/System Application/App/Extension Management/permissions/ExtenMgtAdmin.PermissionSet.al @@ -17,7 +17,7 @@ permissionset 2502 "Exten. Mgt. - Admin" "Guided Experience - View"; Permissions = tabledata "Application Dependency" = Rimd, - tabledata "Application Object Metadata" = Rimd, // r needed for check CanManageExtensions + tabledata "Application Object Metadata" = Rimd, tabledata "Application Resource" = Rimd, tabledata "Extension Pending Setup" = rimd, tabledata "Inplace Installed Application" = Rimd, diff --git a/src/System Application/App/Extension Management/src/ExtensionInstallationImpl.Codeunit.al b/src/System Application/App/Extension Management/src/ExtensionInstallationImpl.Codeunit.al index b295c9e5d27..26624cf1805 100644 --- a/src/System Application/App/Extension Management/src/ExtensionInstallationImpl.Codeunit.al +++ b/src/System Application/App/Extension Management/src/ExtensionInstallationImpl.Codeunit.al @@ -5,6 +5,8 @@ namespace System.Apps; +using System.Security.AccessControl; +using System.Security.User; using System.Utilities; using System.Environment.Configuration; @@ -178,11 +180,55 @@ codeunit 2500 "Extension Installation Impl" Error(NotSufficientPermissionErr); end; + procedure CheckPermissions(UserSecurityIdToCheck: Guid) + begin + if not CanManageExtensions(UserSecurityIdToCheck) then + Error(NotSufficientPermissionErr); + end; + procedure CanManageExtensions(): Boolean var - ApplicationObjectMetadata: Record "Application Object Metadata"; + UserPermissions: Codeunit "User Permissions"; + Result: Boolean; + begin + OnCanManageExtensions(Result); + if Result then + exit(true); + + if UserPermissions.IsSuper(UserSecurityId()) then + exit(true); + + exit(CanManageExtensions(UserSecurityId())); + end; + + procedure CanManageExtensions(UserSecurityIdToCheck: Guid): Boolean + var + AccessControl: Record "Access Control"; + UserPermissions: Codeunit "User Permissions"; + CurrentModuleInfo: ModuleInfo; + ExtensionManagementAdminTok: Label 'Exten. Mgt. - Admin', Locked = true; + SuperTok: Label 'SUPER', Locked = true; + NullGuid: Guid; + begin + if UserPermissions.HasUserPermissionSetAssigned( + UserSecurityIdToCheck, '', SuperTok, AccessControl.Scope::System, NullGuid) + then + exit(true); + + NavApp.GetCurrentModuleInfo(CurrentModuleInfo); + exit(UserPermissions.HasUserPermissionSetAssigned( + UserSecurityIdToCheck, '', ExtensionManagementAdminTok, AccessControl.Scope::System, CurrentModuleInfo.Id())); + end; + + /// + /// Allows tests to authorize the current test session without modifying platform user records. + /// + /// + /// This event is for testing only and must not be used in production. + /// + [InternalEvent(false)] + local procedure OnCanManageExtensions(var Result: Boolean) begin - exit(ApplicationObjectMetadata.ReadPermission()); end; procedure UninstallExtension(PackageID: Guid; IsUIEnabled: Boolean): Boolean @@ -365,4 +411,3 @@ codeunit 2500 "Extension Installation Impl" exit(ExtensionDetails.Editable()); end; } - diff --git a/src/System Application/Test/Extension Management/permissions/ExtMgtNonAdmin.PermissionSet.al b/src/System Application/Test/Extension Management/permissions/ExtMgtNonAdmin.PermissionSet.al new file mode 100644 index 00000000000..e9856758419 --- /dev/null +++ b/src/System Application/Test/Extension Management/permissions/ExtMgtNonAdmin.PermissionSet.al @@ -0,0 +1,15 @@ +// ------------------------------------------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. +// ------------------------------------------------------------------------------------------------ + +namespace System.Test.Apps; + +using System.Security.AccessControl; + +permissionset 133100 "Ext. Mgt. Nonadmin" +{ + Assignable = true; + + Permissions = tabledata "Access Control" = R; +} \ No newline at end of file diff --git a/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al b/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al index 636abc61c4c..65e1b801b08 100644 --- a/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al +++ b/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al @@ -5,6 +5,7 @@ namespace System.Test.Apps; +using System.Security.AccessControl; using System.Environment.Configuration; using System.Apps; using System.Media; @@ -28,6 +29,7 @@ codeunit 133100 "Extension Management Test" var ExtensionManagement: Codeunit "Extension Management"; + ExtensionInstallationImpl: Codeunit "Extension Installation Impl"; Assert: Codeunit "Library Assert"; PermissionsMock: Codeunit "Permissions Mock"; MainAppId: Guid; @@ -40,6 +42,7 @@ codeunit 133100 "Extension Management Test" ExtensionNotInstalledErr: Label 'Extension should be installed.'; PackageIdExistsErr: Label 'The returned extension pakage does not exist'; NullPackageIdErr: Label 'There should not be an extension corresponding to the returned package ID'; + NotSufficientPermissionErr: Label 'You do not have sufficient permissions to manage extensions. Please contact your administrator.'; PackageIdExtensionVersionErr: Label 'The package Id does not poin to the correct extension version'; local procedure SetNavAppIds() @@ -48,6 +51,44 @@ codeunit 133100 "Extension Management Test" DependingAppId := 'c4123d81-a537-4062-bdd4-7b9882bcc319'; end; + local procedure CreateUserWithPermissionSet(RoleId: Code[20]; Company: Text): Guid + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + UserSecurityId: Guid; + begin + UserSecurityId := CreateGuid(); + + AggregatePermissionSet.SetRange("Role ID", RoleId); + AggregatePermissionSet.FindFirst(); + + AccessControl.Init(); + AccessControl."User Security ID" := UserSecurityId; + AccessControl."Role ID" := AggregatePermissionSet."Role ID"; + AccessControl."Company Name" := CopyStr(Company, 1, MaxStrLen(AccessControl."Company Name")); + AccessControl.Scope := AggregatePermissionSet.Scope; + AccessControl."App ID" := AggregatePermissionSet."App ID"; + AccessControl.Insert(); + + exit(UserSecurityId); + end; + + local procedure CreateSuperUser(): Guid + var + AccessControl: Record "Access Control"; + UserSecurityId: Guid; + begin + UserSecurityId := CreateGuid(); + + AccessControl.Init(); + AccessControl."User Security ID" := UserSecurityId; + AccessControl."Role ID" := 'SUPER'; + AccessControl.Scope := AccessControl.Scope::System; + AccessControl.Insert(); + + exit(UserSecurityId); + end; + local procedure InitializeExtensions() var NAVAppInstalledApp: Record "NAV App Installed App"; @@ -58,6 +99,69 @@ codeunit 133100 "Extension Management Test" ExtensionManagement.UninstallExtension(NAVAppInstalledApp."Package ID", false); end; + [Test] + [TransactionModel(TransactionModel::AutoRollback)] + procedure NonAdminCannotManageExtensions() + var + UserSecurityId: Guid; + begin + UserSecurityId := CreateUserWithPermissionSet('Ext. Mgt. Nonadmin', ''); + + Assert.IsFalse(ExtensionInstallationImpl.CanManageExtensions(UserSecurityId), 'A non-admin permission set must not grant extension management permission.'); + end; + + [Test] + [TransactionModel(TransactionModel::AutoRollback)] + procedure NonAdminCannotPassExtensionPermissionCheck() + var + UserSecurityId: Guid; + begin + UserSecurityId := CreateUserWithPermissionSet('Ext. Mgt. Nonadmin', ''); + + asserterror ExtensionInstallationImpl.CheckPermissions(UserSecurityId); + + Assert.ExpectedError(NotSufficientPermissionErr); + end; + + [Test] + [TransactionModel(TransactionModel::AutoRollback)] + procedure ExtensionManagementAdminCanManageExtensions() + var + UserSecurityId: Guid; + begin + UserSecurityId := CreateUserWithPermissionSet('Exten. Mgt. - Admin', ''); + + Assert.IsTrue(ExtensionInstallationImpl.CanManageExtensions(UserSecurityId), 'Exten. Mgt. - Admin must grant extension management permission.'); + end; + + [Test] + [TransactionModel(TransactionModel::AutoRollback)] + procedure SuperCanManageExtensions() + var + UserSecurityId: Guid; + begin + UserSecurityId := CreateSuperUser(); + + Assert.IsTrue(ExtensionInstallationImpl.CanManageExtensions(UserSecurityId), 'SUPER must grant extension management permission.'); + end; + + + [Test] + [TransactionModel(TransactionModel::AutoRollback)] + procedure CompanyScopedExtensionManagementAdminCannotManageExtensions() + var + UserSecurityId: Guid; + begin + UserSecurityId := CreateUserWithPermissionSet('Exten. Mgt. - Admin', CompanyName()); + + Assert.IsFalse(ExtensionInstallationImpl.CanManageExtensions(UserSecurityId), 'Company-scoped Exten. Mgt. - Admin must not grant tenant-wide extension management permission.'); + end; + [EventSubscriber(ObjectType::Codeunit, Codeunit::"Extension Installation Impl", OnCanManageExtensions, '', false, false)] + local procedure AuthorizeTestSession(var Result: Boolean) + begin + Result := true; + end; + [Test] [Scope('OnPrem')] procedure InstallUninstallExtension() @@ -571,4 +675,3 @@ codeunit 133100 "Extension Management Test" Reply := true; end; } - From e40f93d804c5230fec1a9161124e2913227d923f Mon Sep 17 00:00:00 2001 From: Pooya Kharamesh Date: Wed, 7 Oct 2026 13:06:02 +0200 Subject: [PATCH 2/4] Fix Extension Management test build Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/ExtensionInstallationImpl.Codeunit.al | 2 +- .../src/ExtensionManagementTest.Codeunit.al | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/System Application/App/Extension Management/src/ExtensionInstallationImpl.Codeunit.al b/src/System Application/App/Extension Management/src/ExtensionInstallationImpl.Codeunit.al index 26624cf1805..6e02d04305f 100644 --- a/src/System Application/App/Extension Management/src/ExtensionInstallationImpl.Codeunit.al +++ b/src/System Application/App/Extension Management/src/ExtensionInstallationImpl.Codeunit.al @@ -5,10 +5,10 @@ namespace System.Apps; +using System.Environment.Configuration; using System.Security.AccessControl; using System.Security.User; using System.Utilities; -using System.Environment.Configuration; codeunit 2500 "Extension Installation Impl" { diff --git a/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al b/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al index 65e1b801b08..06b94317823 100644 --- a/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al +++ b/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al @@ -5,15 +5,16 @@ namespace System.Test.Apps; -using System.Security.AccessControl; -using System.Environment.Configuration; using System.Apps; +using System.Environment.Configuration; using System.Media; -using System.TestLibraries.Utilities; +using System.Security.AccessControl; using System.TestLibraries.Security.AccessControl; +using System.TestLibraries.Utilities; codeunit 133100 "Extension Management Test" { + EventSubscriberInstance = Manual; // For the purpose of this test, two sample extensions are pre-published. // The condition for the tests to pass is that one of the extensions is holding a dependency on the other one, and they are both 1st Party Extensions, version 1.0. @@ -29,6 +30,7 @@ codeunit 133100 "Extension Management Test" var ExtensionManagement: Codeunit "Extension Management"; + ExtensionManagementTest: Codeunit "Extension Management Test"; ExtensionInstallationImpl: Codeunit "Extension Installation Impl"; Assert: Codeunit "Library Assert"; PermissionsMock: Codeunit "Permissions Mock"; @@ -93,6 +95,8 @@ codeunit 133100 "Extension Management Test" var NAVAppInstalledApp: Record "NAV App Installed App"; begin + BindSubscription(ExtensionManagementTest); + if NAVAppInstalledApp.Get(MainAppId) then ExtensionManagement.UninstallExtension(NAVAppInstalledApp."Package ID", false); if NAVAppInstalledApp.Get(DependingAppId) then From f82a60251038d4aeb003775fae20d64048964fcb Mon Sep 17 00:00:00 2001 From: Pooya Kharamesh Date: Wed, 7 Oct 2026 15:28:29 +0200 Subject: [PATCH 3/4] Fix Extension Management test subscriber lifecycle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/ExtensionManagementTest.Codeunit.al | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al b/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al index 06b94317823..c6f44d785df 100644 --- a/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al +++ b/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al @@ -31,7 +31,8 @@ codeunit 133100 "Extension Management Test" var ExtensionManagement: Codeunit "Extension Management"; ExtensionManagementTest: Codeunit "Extension Management Test"; - ExtensionInstallationImpl: Codeunit "Extension Installation Impl"; + ExtensionInstallationImplTest: Codeunit "Extension Installation Impl"; + ExtensionManagementTestBound: Boolean; Assert: Codeunit "Library Assert"; PermissionsMock: Codeunit "Permissions Mock"; MainAppId: Guid; @@ -95,7 +96,10 @@ codeunit 133100 "Extension Management Test" var NAVAppInstalledApp: Record "NAV App Installed App"; begin - BindSubscription(ExtensionManagementTest); + if not ExtensionManagementTestBound then begin + BindSubscription(ExtensionManagementTest); + ExtensionManagementTestBound := true; + end; if NAVAppInstalledApp.Get(MainAppId) then ExtensionManagement.UninstallExtension(NAVAppInstalledApp."Package ID", false); @@ -111,7 +115,7 @@ codeunit 133100 "Extension Management Test" begin UserSecurityId := CreateUserWithPermissionSet('Ext. Mgt. Nonadmin', ''); - Assert.IsFalse(ExtensionInstallationImpl.CanManageExtensions(UserSecurityId), 'A non-admin permission set must not grant extension management permission.'); + Assert.IsFalse(ExtensionInstallationImplTest.CanManageExtensions(UserSecurityId), 'A non-admin permission set must not grant extension management permission.'); end; [Test] @@ -122,7 +126,7 @@ codeunit 133100 "Extension Management Test" begin UserSecurityId := CreateUserWithPermissionSet('Ext. Mgt. Nonadmin', ''); - asserterror ExtensionInstallationImpl.CheckPermissions(UserSecurityId); + asserterror ExtensionInstallationImplTest.CheckPermissions(UserSecurityId); Assert.ExpectedError(NotSufficientPermissionErr); end; @@ -135,7 +139,7 @@ codeunit 133100 "Extension Management Test" begin UserSecurityId := CreateUserWithPermissionSet('Exten. Mgt. - Admin', ''); - Assert.IsTrue(ExtensionInstallationImpl.CanManageExtensions(UserSecurityId), 'Exten. Mgt. - Admin must grant extension management permission.'); + Assert.IsTrue(ExtensionInstallationImplTest.CanManageExtensions(UserSecurityId), 'Exten. Mgt. - Admin must grant extension management permission.'); end; [Test] @@ -146,7 +150,7 @@ codeunit 133100 "Extension Management Test" begin UserSecurityId := CreateSuperUser(); - Assert.IsTrue(ExtensionInstallationImpl.CanManageExtensions(UserSecurityId), 'SUPER must grant extension management permission.'); + Assert.IsTrue(ExtensionInstallationImplTest.CanManageExtensions(UserSecurityId), 'SUPER must grant extension management permission.'); end; @@ -158,7 +162,7 @@ codeunit 133100 "Extension Management Test" begin UserSecurityId := CreateUserWithPermissionSet('Exten. Mgt. - Admin', CompanyName()); - Assert.IsFalse(ExtensionInstallationImpl.CanManageExtensions(UserSecurityId), 'Company-scoped Exten. Mgt. - Admin must not grant tenant-wide extension management permission.'); + Assert.IsFalse(ExtensionInstallationImplTest.CanManageExtensions(UserSecurityId), 'Company-scoped Exten. Mgt. - Admin must not grant tenant-wide extension management permission.'); end; [EventSubscriber(ObjectType::Codeunit, Codeunit::"Extension Installation Impl", OnCanManageExtensions, '', false, false)] local procedure AuthorizeTestSession(var Result: Boolean) From ba3710124efdcb12547b0a1574519278f065b9cf Mon Sep 17 00:00:00 2001 From: Pooya Kharamesh Date: Wed, 7 Oct 2026 17:18:17 +0200 Subject: [PATCH 4/4] Order Extension Management test variables Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/ExtensionManagementTest.Codeunit.al | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al b/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al index c6f44d785df..2ab86fef953 100644 --- a/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al +++ b/src/System Application/Test/Extension Management/src/ExtensionManagementTest.Codeunit.al @@ -32,9 +32,9 @@ codeunit 133100 "Extension Management Test" ExtensionManagement: Codeunit "Extension Management"; ExtensionManagementTest: Codeunit "Extension Management Test"; ExtensionInstallationImplTest: Codeunit "Extension Installation Impl"; - ExtensionManagementTestBound: Boolean; Assert: Codeunit "Library Assert"; PermissionsMock: Codeunit "Permissions Mock"; + ExtensionManagementTestBound: Boolean; MainAppId: Guid; DependingAppId: Guid; NotInstalledSuccErr: Label 'Extension was not installed succesfully';