From 9de0037634682fdc191473f0c0d0f59137eaf095 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Wed, 9 Sep 2026 19:57:04 +0200 Subject: [PATCH 01/24] [Bug 640454] Scope Expense Agent Entra permissions by company Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentAPIValidation.Codeunit.al | 25 +-- .../src/Integration/EAHttpClient.Codeunit.al | 4 +- .../ExpenseAgentEntraAppMgt.Codeunit.al | 146 ++++++++++++++++++ .../Pages/ExpenseAgentSetupWizard.Page.al | 63 +------- .../src/ExpensePermissionsTest.Codeunit.al | 113 ++++++++++++++ 5 files changed, 271 insertions(+), 80 deletions(-) create mode 100644 src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al diff --git a/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al index ba65a32fc36..837981d9518 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al @@ -6,7 +6,6 @@ namespace Microsoft.ExpenseAgent; using System.AI; using System.Environment; -using System.Environment.Configuration; codeunit 6993 "Expense Agent API Validation" { @@ -17,7 +16,6 @@ codeunit 6993 "Expense Agent API Validation" var AgentNotEnabledErr: Label 'Expense Agent is not enabled. Please contact your administrator.'; CapabilityNotEnabledErr: Label 'The "%1" capability is not enabled. Please contact your administrator to enable the capability.', Comment = '%1 = a capability name, such as Expense Agent'; - ExpenseAgentAadAppIdTxt: Label 'ee1eb5fd-719b-44f2-97d0-0efd34bc4148', Locked = true; procedure VerifyAgentAccess() begin @@ -45,33 +43,18 @@ codeunit 6993 "Expense Agent API Validation" Error(CapabilityNotEnabledErr, Enum::"Copilot Capability"::"Expense Agent"); end; - procedure GetAadAppId(): Text - begin - exit(ExpenseAgentAadAppIdTxt); - end; - procedure IsCurrentUserExpenseAgent(): Boolean var - AADApplication: Record "AAD Application"; - EnvironmentInfo: Codeunit "Environment Information"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - if not EnvironmentInfo.IsSaaSInfrastructure() then - exit(true); - - if not AADApplication.Get(ExpenseAgentAadAppIdTxt) then - exit(false); - - exit(AADApplication."User ID" = UserSecurityId()); + exit(ExpenseAgentEntraApp.IsCurrentUserExpenseAgent()); end; [TryFunction] internal procedure TryGetExpenseAgentUserId(var ExpenseAgentUserId: Guid) var - AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - AadApplication.Get(ExpenseAgentAadAppIdTxt); - AadApplication.TestField(State, AadApplication.State::Enabled); - - ExpenseAgentUserId := AadApplication."User ID"; + ExpenseAgentUserId := ExpenseAgentEntraApp.GetEnabledExpenseAgentUserId(); end; } diff --git a/src/Apps/W1/ExpenseAgent/app/src/Integration/EAHttpClient.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Integration/EAHttpClient.Codeunit.al index d1a1c1334e1..fd611079291 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Integration/EAHttpClient.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Integration/EAHttpClient.Codeunit.al @@ -589,13 +589,13 @@ codeunit 6941 "EA Http Client" var ExpenseAgentSetup: Record "Expense Agent Setup"; AzureADMgt: Codeunit "Azure AD Mgt."; - ExpenseAgentAPIValidation: Codeunit "Expense Agent API Validation"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; OAuth2: Codeunit OAuth2; Scopes: List of [Text]; OAuthScope: Text; OAuthScopePatternLbl: Label 'api://%1/', Locked = true; begin - OAuthScope := StrSubstNo(OAuthScopePatternLbl, ExpenseAgentAPIValidation.GetAadAppId()); + OAuthScope := StrSubstNo(OAuthScopePatternLbl, ExpenseAgentEntraApp.GetAadAppId()); Scopes.Add(OAuthScope + 'Expenses.ReadWrite.All'); AccessToken := AzureADMgt.GetAccessTokenAsSecretText(OAuthScope, '', false); if AccessToken.IsEmpty() then begin diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al new file mode 100644 index 00000000000..f9cbdeeed85 --- /dev/null +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -0,0 +1,146 @@ +// ------------------------------------------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. +// ------------------------------------------------------------------------------------------------ +namespace Microsoft.ExpenseAgent; + +using System.Environment; +using System.Environment.Configuration; +using System.Security.AccessControl; + +codeunit 6913 "Expense Agent Entra App Mgt." +{ + Access = Internal; + InherentEntitlements = X; + InherentPermissions = X; + + internal procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean + var + AccessControl: Record "Access Control"; + begin + SetExpenseAgentPermissionFilters(AccessControl, AadApplication); + AccessControl.SetRange("Company Name", CompanyName()); + exit(not AccessControl.IsEmpty()); + end; + + internal procedure HasAnyPermission(AadApplication: Record "AAD Application"): Boolean + var + AccessControl: Record "Access Control"; + begin + SetExpenseAgentPermissionFilters(AccessControl, AadApplication); + exit(not AccessControl.IsEmpty()); + end; + + internal procedure AddPermissionForCurrentCompany(AadApplication: Record "AAD Application") + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + begin + AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetLbl); + if not AggregatePermissionSet.FindFirst() then + exit; + + AccessControl.Init(); + AccessControl.Validate("User Security ID", AadApplication."User ID"); + AccessControl.Validate("Role ID", ExpenseAgentPermissionSetLbl); + AccessControl.Validate("App ID", AggregatePermissionSet."App ID"); + AccessControl.Validate("Company Name", CompanyName()); + AccessControl.Insert(true); + end; + + internal procedure RemovePermissionForCurrentCompany(AadApplication: Record "AAD Application") + var + AccessControl: Record "Access Control"; + begin + SetExpenseAgentPermissionFilters(AccessControl, AadApplication); + AccessControl.SetRange("Company Name", CompanyName()); + AccessControl.DeleteAll(true); + end; + + internal procedure EnableAadApplicationForCurrentCompany() + var + AadApplication: Record "AAD Application"; + begin + if not GetAadApplication(AadApplication) then + exit; + + // Enabling creates the application user. Disable it again before changing permissions. + if AadApplication.State <> AadApplication.State::Enabled then begin + AadApplication.Validate(State, AadApplication.State::Enabled); + AadApplication.Modify(true); + end; + + if HasPermissionForCurrentCompany(AadApplication) then + exit; + + AadApplication.Validate(State, AadApplication.State::Disabled); + AadApplication.Modify(true); + + AddPermissionForCurrentCompany(AadApplication); + + AadApplication.Validate(State, AadApplication.State::Enabled); + AadApplication.Modify(true); + end; + + internal procedure DisableAadApplicationForCurrentCompany() + var + AadApplication: Record "AAD Application"; + begin + if not GetAadApplication(AadApplication) then + exit; + + RemovePermissionForCurrentCompany(AadApplication); + if HasAnyPermission(AadApplication) then + exit; + if AadApplication.State = AadApplication.State::Disabled then + exit; + + AadApplication.Validate(State, AadApplication.State::Disabled); + AadApplication.Modify(true); + end; + + internal procedure IsCurrentUserExpenseAgent(): Boolean + var + AadApplication: Record "AAD Application"; + EnvironmentInfo: Codeunit "Environment Information"; + begin + if not EnvironmentInfo.IsSaaSInfrastructure() then + exit(true); + + if not AadApplication.Get(GetAadAppId()) then + exit(false); + + exit(AadApplication."User ID" = UserSecurityId()); + end; + + internal procedure GetEnabledExpenseAgentUserId(): Guid + var + AadApplication: Record "AAD Application"; + begin + AadApplication.Get(GetAadAppId()); + AadApplication.TestField(State, AadApplication.State::Enabled); + + exit(AadApplication."User ID"); + end; + + internal procedure GetAadAppId(): Text + begin + exit(ExpenseAgentAadAppIdTxt); + end; + + local procedure GetAadApplication(var AadApplication: Record "AAD Application"): Boolean + begin + AadApplication.SetRange("Client Id", GetAadAppId()); + exit(AadApplication.FindFirst()); + end; + + local procedure SetExpenseAgentPermissionFilters(var AccessControl: Record "Access Control"; AadApplication: Record "AAD Application") + begin + AccessControl.SetRange("User Security ID", AadApplication."User ID"); + AccessControl.SetRange("Role ID", ExpenseAgentPermissionSetLbl); + end; + + var + ExpenseAgentAadAppIdTxt: Label 'ee1eb5fd-719b-44f2-97d0-0efd34bc4148', Locked = true; + ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; +} diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al index 0c67fe11c53..6e035307803 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al @@ -11,7 +11,6 @@ using System.AI; using System.Email; using System.Environment; using System.Environment.Configuration; -using System.Security.AccessControl; using System.Telemetry; using System.Utilities; #pragma warning disable AS0031 @@ -998,7 +997,6 @@ page 6991 "Expense Agent Setup Wizard" IncludeCategoriesForRulesQst: Label 'Default management rules require default expense categories. Do you want to add them to the configuration?'; IncludeCategoriesAndPostingGroupsForRulesQst: Label 'Default management rules require default expense categories and posting groups. Do you want to add them to the configuration?'; PrivacyNoticeNotAcceptedMsg: Label 'To use the Expense Agent, you must first accept the privacy notice. Please accept the privacy notice and try again.'; - ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; NoExpenseUsersErr: Label 'You must first specify who can access.'; NoSystemUsersErr: Label 'You must first specify a user in Business Central as expense user.'; NotAuthorizedToViewSetupErr: Label 'You do not have permission to view the Expense Agent setup. Contact your administrator to be granted agent management rights.'; @@ -1422,6 +1420,8 @@ page 6991 "Expense Agent Setup Wizard" end; local procedure ActivateAgent(): Boolean + var + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin ValidatePrivacyNoticeApproval(); ValidateCapabilityIsEnabled(); @@ -1430,7 +1430,7 @@ page 6991 "Expense Agent Setup Wizard" Error(ApprovalWorkflowConflictErr, Rec.FieldCaption("Enable Approval Workflow")); EnsureCurrentUserHasAccess(); - EnableAadApplication(); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); Commit(); if not RegisterErpConfiguration() then exit(false); @@ -1439,11 +1439,14 @@ page 6991 "Expense Agent Setup Wizard" end; local procedure DeactivateAgent(): Boolean + var + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin if not Rec.ShowDeactivationAccessWarning() then exit(false); if not UnregisterErpConfiguration() then exit(false); + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); Rec.LogAgentDisabledTelemetry(); exit(true); end; @@ -1505,60 +1508,6 @@ page 6991 "Expense Agent Setup Wizard" Error(CapabilityDisabledErr, Enum::"Copilot Capability"::"Expense Agent"); end; - local procedure EnableAadApplication() - var - AadApplication: Record "AAD Application"; - ExpenseAgentApiValidation: Codeunit "Expense Agent API Validation"; - begin - AadApplication.SetRange("Client Id", ExpenseAgentApiValidation.GetAadAppId()); - if not AadApplication.FindFirst() then - exit; - - // We need to enable the AAD application first because enabling creates the user record. - // Once the user exists, we disable it, add the permission set, and re-enable it. - if AadApplication.State <> AadApplication.State::Enabled then begin - AadApplication.Validate(State, AadApplication.State::Enabled); - AadApplication.Modify(true); - end; - - if HasExpenseAgentPermissionSet(AadApplication) then - exit; - - AadApplication.Validate(State, AadApplication.State::Disabled); - AadApplication.Modify(true); - - AddExpenseAgentPermissionSet(AadApplication); - - AadApplication.Validate(State, AadApplication.State::Enabled); - AadApplication.Modify(true); - end; - - local procedure HasExpenseAgentPermissionSet(AadApplication: Record "AAD Application"): Boolean - var - AccessControl: Record "Access Control"; - begin - AccessControl.SetRange("User Security ID", AadApplication."User ID"); - AccessControl.SetRange("Role ID", ExpenseAgentPermissionSetLbl); - exit(not AccessControl.IsEmpty()); - end; - - local procedure AddExpenseAgentPermissionSet(AadApplication: Record "AAD Application") - var - AccessControl: Record "Access Control"; - AggregatePermissionSet: Record "Aggregate Permission Set"; - begin - AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetLbl); - if not AggregatePermissionSet.FindFirst() then - exit; - - AccessControl.Init(); - AccessControl.Validate("User Security ID", AadApplication."User ID"); - AccessControl.Validate("Role ID", ExpenseAgentPermissionSetLbl); - AccessControl.Validate("App ID", AggregatePermissionSet."App ID"); - AccessControl.Validate("Company Name", CopyStr(CompanyName(), 1, MaxStrLen(AccessControl."Company Name"))); - AccessControl.Insert(true); - end; - local procedure OnAssistEditMailbox() var PrevEmailAddress: Text[250]; diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al index 9c4888740da..e6d74861040 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al @@ -8,6 +8,8 @@ using Microsoft.ExpenseAgent; using Microsoft.Finance.SpendRequest; using Microsoft.HumanResources.Employee; using Microsoft.HumanResources.Setup; +using System.Environment.Configuration; +using System.TestLibraries.Security.AccessControl; codeunit 148338 "Expense Permissions Test" { @@ -21,6 +23,7 @@ codeunit 148338 "Expense Permissions Test" LibraryLowerPermissions: Codeunit "Library - Lower Permissions"; LibraryRandom: Codeunit "Library - Random"; LibraryTestInitialize: Codeunit "Library - Test Initialize"; + UserPermissionsLibrary: Codeunit "User Permissions Library"; IsInitialized: Boolean; EmployeeOnlyPermissionSetTok: Label 'Exp. Emp. Only Test', Locked = true; HREditPermissionSetTok: Label 'Exp. HR Edit Test', Locked = true; @@ -150,6 +153,116 @@ codeunit 148338 "Expense Permissions Test" Assert.AreEqual(SpendRequest.Status::Released, SpendRequest.Status, 'A denied approval must preserve the request status.'); ExpenseReportHeader.SetRange("Spend Request No.", SpendRequest."No."); Assert.RecordIsEmpty(ExpenseReportHeader); + procedure EntraAppPermissionFromOtherCompanyDoesNotApplyToCurrentCompany() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + OtherCompanyName: Text[30]; + PermissionExists: Boolean; + begin + // [FEATURE] [AI TEST] + // [SCENARIO 640454] An Entra app permission for another company does not satisfy the current company + Initialize(); + + // [GIVEN] Entra app user "EA" has the Expense Agent permission only for company "B" + AadApplication."User ID" := CreateGuid(); + OtherCompanyName := CopyStr('Other ' + CompanyName(), 1, MaxStrLen(OtherCompanyName)); + UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, OtherCompanyName); + + // [WHEN] Checking whether "EA" has the permission for the current company + PermissionExists := ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication); + + // [THEN] The permission is not considered assigned for the current company + Assert.IsFalse(PermissionExists, 'A permission assigned to another company must not satisfy the current company.'); + end; + + [Test] + procedure EntraAppPermissionFromCurrentCompanyAppliesToCurrentCompany() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + PermissionExists: Boolean; + begin + // [FEATURE] [AI TEST] + // [SCENARIO 640454] An Entra app permission for the current company satisfies the current company + Initialize(); + + // [GIVEN] Entra app user "EA" has the Expense Agent permission for the current company + AadApplication."User ID" := CreateGuid(); + UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, CompanyName()); + + // [WHEN] Checking whether "EA" has the permission for the current company + PermissionExists := ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication); + + // [THEN] The permission is considered assigned for the current company + Assert.IsTrue(PermissionExists, 'A permission assigned to the current company must satisfy the current company.'); + end; + + [Test] + procedure AddingEntraAppPermissionCreatesCurrentCompanyPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI TEST] + // [SCENARIO 640454] Adding the Entra app permission creates it for the current company + Initialize(); + + // [GIVEN] Entra app user "EA" has no Expense Agent permission for the current company + AadApplication."User ID" := UserSecurityId(); + ExpenseAgentEntraApp.RemovePermissionForCurrentCompany(AadApplication); + + // [WHEN] Adding the permission for the current company + ExpenseAgentEntraApp.AddPermissionForCurrentCompany(AadApplication); + + // [THEN] The current company permission exists + Assert.IsTrue(ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication), 'The current company permission must be added.'); + end; + + [Test] + procedure RemovingCurrentCompanyPermissionKeepsOtherCompanyPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + OtherCompanyName: Text[30]; + begin + // [FEATURE] [AI TEST] + // [SCENARIO 640454] Removing the current company permission preserves another company's permission + Initialize(); + + // [GIVEN] Entra app user "EA" has the Expense Agent permission for the current company and company "B" + AadApplication."User ID" := CreateGuid(); + OtherCompanyName := CopyStr('Other ' + CompanyName(), 1, MaxStrLen(OtherCompanyName)); + UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, CompanyName()); + UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, OtherCompanyName); + + // [WHEN] Removing the permission for the current company + ExpenseAgentEntraApp.RemovePermissionForCurrentCompany(AadApplication); + + // [THEN] The current company permission is removed and another company permission remains + Assert.IsFalse(ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication), 'The current company permission must be removed.'); + Assert.IsTrue(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'Another company permission must be preserved.'); + end; + + [Test] + procedure RemovingLastCompanyPermissionLeavesNoPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI TEST] + // [SCENARIO 640454] Removing the last company permission leaves no Expense Agent permission + Initialize(); + + // [GIVEN] Entra app user "EA" has the Expense Agent permission only for the current company + AadApplication."User ID" := CreateGuid(); + UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, CompanyName()); + + // [WHEN] Removing the permission for the current company + ExpenseAgentEntraApp.RemovePermissionForCurrentCompany(AadApplication); + + // [THEN] No Expense Agent permission remains + Assert.IsFalse(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'No Expense Agent permission must remain.'); end; [Test] From c45dda7f2ec56665b4f5a2a169af35238e8d84a2 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 00:33:14 +0200 Subject: [PATCH 02/24] Address Expense Agent Entra lifecycle feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../src/ExpensePermissionsTest.Codeunit.al | 70 +++++++++++++++++-- 1 file changed, 65 insertions(+), 5 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al index e6d74861040..b5ce613601f 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al @@ -9,6 +9,7 @@ using Microsoft.Finance.SpendRequest; using Microsoft.HumanResources.Employee; using Microsoft.HumanResources.Setup; using System.Environment.Configuration; +using System.Security.AccessControl; using System.TestLibraries.Security.AccessControl; codeunit 148338 "Expense Permissions Test" @@ -160,7 +161,6 @@ codeunit 148338 "Expense Permissions Test" OtherCompanyName: Text[30]; PermissionExists: Boolean; begin - // [FEATURE] [AI TEST] // [SCENARIO 640454] An Entra app permission for another company does not satisfy the current company Initialize(); @@ -183,7 +183,6 @@ codeunit 148338 "Expense Permissions Test" ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; PermissionExists: Boolean; begin - // [FEATURE] [AI TEST] // [SCENARIO 640454] An Entra app permission for the current company satisfies the current company Initialize(); @@ -204,7 +203,6 @@ codeunit 148338 "Expense Permissions Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI TEST] // [SCENARIO 640454] Adding the Entra app permission creates it for the current company Initialize(); @@ -226,7 +224,6 @@ codeunit 148338 "Expense Permissions Test" ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; OtherCompanyName: Text[30]; begin - // [FEATURE] [AI TEST] // [SCENARIO 640454] Removing the current company permission preserves another company's permission Initialize(); @@ -250,7 +247,6 @@ codeunit 148338 "Expense Permissions Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI TEST] // [SCENARIO 640454] Removing the last company permission leaves no Expense Agent permission Initialize(); @@ -265,6 +261,54 @@ codeunit 148338 "Expense Permissions Test" Assert.IsFalse(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'No Expense Agent permission must remain.'); end; + [Test] + procedure DisablingEntraAppWithOtherCompanyPermissionKeepsApplicationEnabled() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + OtherCompanyName: Text[30]; + begin + // [SCENARIO 640454] Deactivation keeps the Entra application enabled when another company permission remains + Initialize(); + + // [GIVEN] The Expense Agent Entra application is enabled for the current company and company "B" + PrepareExpenseAgentAadApplication(AadApplication); + ExpenseAgentEntraApp.AddPermissionForCurrentCompany(AadApplication); + OtherCompanyName := CopyStr('Other ' + CompanyName(), 1, MaxStrLen(OtherCompanyName)); + UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, OtherCompanyName); + + // [WHEN] Disabling the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] The Entra application remains enabled for company "B" + AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); + Assert.AreEqual(AadApplication.State::Enabled, AadApplication.State, 'The Entra application must remain enabled for another company.'); + Assert.IsFalse(ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication), 'The current company permission must be removed.'); + Assert.IsTrue(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'Another company permission must remain.'); + end; + + [Test] + procedure DisablingEntraAppAfterLastCompanyPermissionDisablesApplication() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [SCENARIO 640454] Deactivation disables the Entra application after its last company permission is removed + Initialize(); + + // [GIVEN] The Expense Agent Entra application is enabled only for the current company + PrepareExpenseAgentAadApplication(AadApplication); + ExpenseAgentEntraApp.AddPermissionForCurrentCompany(AadApplication); + + // [WHEN] Disabling the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] The Entra application is disabled and no Expense Agent permission remains + AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); + Assert.AreEqual(AadApplication.State::Disabled, AadApplication.State, 'The Entra application must be disabled after its last company permission is removed.'); + Assert.IsFalse(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'No Expense Agent permission must remain.'); + end; + [Test] procedure CompanyEmailSyncsWithEmployeeOnlyPermissions() begin @@ -544,6 +588,22 @@ codeunit 148338 "Expense Permissions Test" PostedExpenseReportHeader.Insert(false); end; + local procedure PrepareExpenseAgentAadApplication(var AadApplication: Record "AAD Application") + var + AccessControl: Record "Access Control"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); + if AadApplication.State <> AadApplication.State::Enabled then begin + AadApplication.Validate(State, AadApplication.State::Enabled); + AadApplication.Modify(true); + end; + + AccessControl.SetRange("User Security ID", AadApplication."User ID"); + AccessControl.SetRange("Role ID", ExpenseAgentPermissionSetTok); + AccessControl.DeleteAll(true); + end; + local procedure Initialize() begin LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Permissions Test"); From eb2c9879b9d955b74776d2dd4f8c4b955f5d8ef9 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 01:10:14 +0200 Subject: [PATCH 03/24] Test Expense Agent Entra lifecycle scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 10 +- src/Apps/W1/ExpenseAgent/test/app.json | 4 + .../src/ExpenseAgentConfigTest.Codeunit.al | 244 ++++++++++++++++++ .../src/ExpensePermissionsTest.Codeunit.al | 173 ------------- 4 files changed, 253 insertions(+), 178 deletions(-) create mode 100644 src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index f9cbdeeed85..7c112abe124 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -14,7 +14,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." InherentEntitlements = X; InherentPermissions = X; - internal procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean + local procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean var AccessControl: Record "Access Control"; begin @@ -23,7 +23,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(not AccessControl.IsEmpty()); end; - internal procedure HasAnyPermission(AadApplication: Record "AAD Application"): Boolean + local procedure HasAnyExpenseAgentPermission(AadApplication: Record "AAD Application"): Boolean var AccessControl: Record "Access Control"; begin @@ -31,7 +31,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(not AccessControl.IsEmpty()); end; - internal procedure AddPermissionForCurrentCompany(AadApplication: Record "AAD Application") + local procedure AddPermissionForCurrentCompany(AadApplication: Record "AAD Application") var AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; @@ -48,7 +48,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl.Insert(true); end; - internal procedure RemovePermissionForCurrentCompany(AadApplication: Record "AAD Application") + local procedure RemovePermissionForCurrentCompany(AadApplication: Record "AAD Application") var AccessControl: Record "Access Control"; begin @@ -90,7 +90,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit; RemovePermissionForCurrentCompany(AadApplication); - if HasAnyPermission(AadApplication) then + if HasAnyExpenseAgentPermission(AadApplication) then exit; if AadApplication.State = AadApplication.State::Disabled then exit; diff --git a/src/Apps/W1/ExpenseAgent/test/app.json b/src/Apps/W1/ExpenseAgent/test/app.json index 0fca4afe54f..272bc182e1a 100644 --- a/src/Apps/W1/ExpenseAgent/test/app.json +++ b/src/Apps/W1/ExpenseAgent/test/app.json @@ -47,6 +47,10 @@ { "from": 148330, "to": 148350 + }, + { + "from": 148361, + "to": 148361 } ], "features": [ diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al new file mode 100644 index 00000000000..861fc1253a9 --- /dev/null +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -0,0 +1,244 @@ +// ------------------------------------------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. +// ------------------------------------------------------------------------------------------------ +namespace Microsoft.Test.ExpenseAgent; + +using Microsoft.ExpenseAgent; +using System.Environment.Configuration; +using System.Security.AccessControl; +using System.TestLibraries.Security.AccessControl; + +codeunit 148361 "Expense Agent Config. Test" +{ + Subtype = Test; + TestType = UnitTest; + TestPermissions = Restrictive; + Permissions = + tabledata "AAD Application" = rm, + tabledata "Access Control" = rimd; + + var + Assert: Codeunit Assert; + LibraryTestInitialize: Codeunit "Library - Test Initialize"; + UserPermissionsLibrary: Codeunit "User Permissions Library"; + ExpenseAgentPermissionSetTok: Label 'Expense Agent', Locked = true; + UnrelatedPermissionSetTok: Label 'D365 BASIC', Locked = true; + + [Test] + procedure ActivatingWithOtherCompanyPermissionAddsCurrentCompanyPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + OtherCompanyName: Text[30]; + begin + // [SCENARIO 640454] Activating with another company permission adds the current company permission + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has the Expense Agent permission only for company "B" + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + OtherCompanyName := GetOtherCompanyName(); + AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); + + // [WHEN] Activating the Expense Agent for the current company + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [THEN] "EA" remains enabled and has Expense Agent permissions for both companies + VerifyAadApplicationState(AadApplication.State::Enabled); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); + end; + + [Test] + procedure ActivatingDisabledAppWithCurrentPermissionDoesNotDuplicatePermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [SCENARIO 640454] Activating a disabled Entra app preserves its existing current company permission + Initialize(); + + // [GIVEN] Disabled Entra app "EA" already has the Expense Agent permission for the current company + PrepareAadApplication(AadApplication, AadApplication.State::Disabled); + AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + + // [WHEN] Activating the Expense Agent for the current company + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [THEN] "EA" is enabled with one current company permission + VerifyAadApplicationState(AadApplication.State::Enabled); + VerifyPermissionCount(AadApplication, ExpenseAgentPermissionSetTok, CompanyName(), 1); + end; + + [Test] + procedure ActivatingWithUnrelatedPermissionPreservesPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [SCENARIO 640454] Activating preserves an unrelated permission assigned to the Entra app user + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has an unrelated permission but no Expense Agent permission + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + AssignPermission(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + + // [WHEN] Activating the Expense Agent for the current company + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [THEN] "EA" remains enabled with both the unrelated and current Expense Agent permissions + VerifyAadApplicationState(AadApplication.State::Enabled); + VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + end; + + [Test] + procedure DeactivatingWithOtherCompanyPermissionKeepsApplicationEnabled() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + OtherCompanyName: Text[30]; + begin + // [SCENARIO 640454] Deactivating preserves another company permission and keeps the Entra app enabled + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has Expense Agent permissions for the current company and company "B" + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + OtherCompanyName := GetOtherCompanyName(); + AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] "EA" remains enabled with only company "B" Expense Agent permission + VerifyAadApplicationState(AadApplication.State::Enabled); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); + end; + + [Test] + procedure DeactivatingLastExpensePermissionDisablesApplication() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [SCENARIO 640454] Deactivating the last Expense Agent company disables the Entra app + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has the Expense Agent permission only for the current company + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] "EA" is disabled with no Expense Agent permission + VerifyAadApplicationState(AadApplication.State::Disabled); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + end; + + [Test] + procedure DeactivatingLastExpensePermissionPreservesUnrelatedPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [SCENARIO 640454] Deactivating the last Expense Agent company preserves unrelated permissions + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has current Expense Agent and unrelated permissions + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + AssignPermission(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] "EA" is disabled, the Expense Agent permission is removed, and the unrelated permission remains + VerifyAadApplicationState(AadApplication.State::Disabled); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + end; + + [Test] + procedure DeactivatingWithoutExpensePermissionDisablesApplication() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [SCENARIO 640454] Deactivating disables the Entra app when no Expense Agent company permission exists + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has only an unrelated permission + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + AssignPermission(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] "EA" is disabled and the unrelated permission remains + VerifyAadApplicationState(AadApplication.State::Disabled); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + end; + + local procedure Initialize() + begin + LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Agent Config. Test"); + end; + + local procedure PrepareAadApplication(var AadApplication: Record "AAD Application"; State: Option) + var + AccessControl: Record "Access Control"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); + if AadApplication.State <> State then begin + AadApplication.Validate(State, State); + AadApplication.Modify(true); + end; + + AccessControl.SetRange("User Security ID", AadApplication."User ID"); + AccessControl.DeleteAll(true); + end; + + local procedure AssignPermission(AadApplication: Record "AAD Application"; PermissionSetId: Code[20]; CompanyNameValue: Text[30]) + begin + UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", PermissionSetId, CompanyNameValue); + end; + + local procedure GetOtherCompanyName(): Text[30] + begin + exit(CopyStr(Format(CreateGuid()), 1, 30)); + end; + + local procedure VerifyAadApplicationState(ExpectedState: Option) + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); + Assert.AreEqual(ExpectedState, AadApplication.State, 'The Entra application state is incorrect.'); + end; + + local procedure VerifyPermissionExists(AadApplication: Record "AAD Application"; PermissionSetId: Code[20]; CompanyNameValue: Text[30]) + begin + VerifyPermissionCount(AadApplication, PermissionSetId, CompanyNameValue, 1); + end; + + local procedure VerifyPermissionDoesNotExist(AadApplication: Record "AAD Application"; PermissionSetId: Code[20]; CompanyNameValue: Text[30]) + begin + VerifyPermissionCount(AadApplication, PermissionSetId, CompanyNameValue, 0); + end; + + local procedure VerifyPermissionCount(AadApplication: Record "AAD Application"; PermissionSetId: Code[20]; CompanyNameValue: Text[30]; ExpectedCount: Integer) + var + AccessControl: Record "Access Control"; + begin + AccessControl.SetRange("User Security ID", AadApplication."User ID"); + AccessControl.SetRange("Role ID", PermissionSetId); + AccessControl.SetRange("Company Name", CompanyNameValue); + Assert.AreEqual(ExpectedCount, AccessControl.Count(), 'The number of matching Entra app permissions is incorrect.'); + end; +} diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al index b5ce613601f..9c4888740da 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al @@ -8,9 +8,6 @@ using Microsoft.ExpenseAgent; using Microsoft.Finance.SpendRequest; using Microsoft.HumanResources.Employee; using Microsoft.HumanResources.Setup; -using System.Environment.Configuration; -using System.Security.AccessControl; -using System.TestLibraries.Security.AccessControl; codeunit 148338 "Expense Permissions Test" { @@ -24,7 +21,6 @@ codeunit 148338 "Expense Permissions Test" LibraryLowerPermissions: Codeunit "Library - Lower Permissions"; LibraryRandom: Codeunit "Library - Random"; LibraryTestInitialize: Codeunit "Library - Test Initialize"; - UserPermissionsLibrary: Codeunit "User Permissions Library"; IsInitialized: Boolean; EmployeeOnlyPermissionSetTok: Label 'Exp. Emp. Only Test', Locked = true; HREditPermissionSetTok: Label 'Exp. HR Edit Test', Locked = true; @@ -154,159 +150,6 @@ codeunit 148338 "Expense Permissions Test" Assert.AreEqual(SpendRequest.Status::Released, SpendRequest.Status, 'A denied approval must preserve the request status.'); ExpenseReportHeader.SetRange("Spend Request No.", SpendRequest."No."); Assert.RecordIsEmpty(ExpenseReportHeader); - procedure EntraAppPermissionFromOtherCompanyDoesNotApplyToCurrentCompany() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - OtherCompanyName: Text[30]; - PermissionExists: Boolean; - begin - // [SCENARIO 640454] An Entra app permission for another company does not satisfy the current company - Initialize(); - - // [GIVEN] Entra app user "EA" has the Expense Agent permission only for company "B" - AadApplication."User ID" := CreateGuid(); - OtherCompanyName := CopyStr('Other ' + CompanyName(), 1, MaxStrLen(OtherCompanyName)); - UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, OtherCompanyName); - - // [WHEN] Checking whether "EA" has the permission for the current company - PermissionExists := ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication); - - // [THEN] The permission is not considered assigned for the current company - Assert.IsFalse(PermissionExists, 'A permission assigned to another company must not satisfy the current company.'); - end; - - [Test] - procedure EntraAppPermissionFromCurrentCompanyAppliesToCurrentCompany() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - PermissionExists: Boolean; - begin - // [SCENARIO 640454] An Entra app permission for the current company satisfies the current company - Initialize(); - - // [GIVEN] Entra app user "EA" has the Expense Agent permission for the current company - AadApplication."User ID" := CreateGuid(); - UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, CompanyName()); - - // [WHEN] Checking whether "EA" has the permission for the current company - PermissionExists := ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication); - - // [THEN] The permission is considered assigned for the current company - Assert.IsTrue(PermissionExists, 'A permission assigned to the current company must satisfy the current company.'); - end; - - [Test] - procedure AddingEntraAppPermissionCreatesCurrentCompanyPermission() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Adding the Entra app permission creates it for the current company - Initialize(); - - // [GIVEN] Entra app user "EA" has no Expense Agent permission for the current company - AadApplication."User ID" := UserSecurityId(); - ExpenseAgentEntraApp.RemovePermissionForCurrentCompany(AadApplication); - - // [WHEN] Adding the permission for the current company - ExpenseAgentEntraApp.AddPermissionForCurrentCompany(AadApplication); - - // [THEN] The current company permission exists - Assert.IsTrue(ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication), 'The current company permission must be added.'); - end; - - [Test] - procedure RemovingCurrentCompanyPermissionKeepsOtherCompanyPermission() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - OtherCompanyName: Text[30]; - begin - // [SCENARIO 640454] Removing the current company permission preserves another company's permission - Initialize(); - - // [GIVEN] Entra app user "EA" has the Expense Agent permission for the current company and company "B" - AadApplication."User ID" := CreateGuid(); - OtherCompanyName := CopyStr('Other ' + CompanyName(), 1, MaxStrLen(OtherCompanyName)); - UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, CompanyName()); - UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, OtherCompanyName); - - // [WHEN] Removing the permission for the current company - ExpenseAgentEntraApp.RemovePermissionForCurrentCompany(AadApplication); - - // [THEN] The current company permission is removed and another company permission remains - Assert.IsFalse(ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication), 'The current company permission must be removed.'); - Assert.IsTrue(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'Another company permission must be preserved.'); - end; - - [Test] - procedure RemovingLastCompanyPermissionLeavesNoPermission() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Removing the last company permission leaves no Expense Agent permission - Initialize(); - - // [GIVEN] Entra app user "EA" has the Expense Agent permission only for the current company - AadApplication."User ID" := CreateGuid(); - UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, CompanyName()); - - // [WHEN] Removing the permission for the current company - ExpenseAgentEntraApp.RemovePermissionForCurrentCompany(AadApplication); - - // [THEN] No Expense Agent permission remains - Assert.IsFalse(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'No Expense Agent permission must remain.'); - end; - - [Test] - procedure DisablingEntraAppWithOtherCompanyPermissionKeepsApplicationEnabled() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - OtherCompanyName: Text[30]; - begin - // [SCENARIO 640454] Deactivation keeps the Entra application enabled when another company permission remains - Initialize(); - - // [GIVEN] The Expense Agent Entra application is enabled for the current company and company "B" - PrepareExpenseAgentAadApplication(AadApplication); - ExpenseAgentEntraApp.AddPermissionForCurrentCompany(AadApplication); - OtherCompanyName := CopyStr('Other ' + CompanyName(), 1, MaxStrLen(OtherCompanyName)); - UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", ExpenseAgentPermissionSetTok, OtherCompanyName); - - // [WHEN] Disabling the Expense Agent for the current company - ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - - // [THEN] The Entra application remains enabled for company "B" - AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); - Assert.AreEqual(AadApplication.State::Enabled, AadApplication.State, 'The Entra application must remain enabled for another company.'); - Assert.IsFalse(ExpenseAgentEntraApp.HasPermissionForCurrentCompany(AadApplication), 'The current company permission must be removed.'); - Assert.IsTrue(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'Another company permission must remain.'); - end; - - [Test] - procedure DisablingEntraAppAfterLastCompanyPermissionDisablesApplication() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Deactivation disables the Entra application after its last company permission is removed - Initialize(); - - // [GIVEN] The Expense Agent Entra application is enabled only for the current company - PrepareExpenseAgentAadApplication(AadApplication); - ExpenseAgentEntraApp.AddPermissionForCurrentCompany(AadApplication); - - // [WHEN] Disabling the Expense Agent for the current company - ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - - // [THEN] The Entra application is disabled and no Expense Agent permission remains - AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); - Assert.AreEqual(AadApplication.State::Disabled, AadApplication.State, 'The Entra application must be disabled after its last company permission is removed.'); - Assert.IsFalse(ExpenseAgentEntraApp.HasAnyPermission(AadApplication), 'No Expense Agent permission must remain.'); end; [Test] @@ -588,22 +431,6 @@ codeunit 148338 "Expense Permissions Test" PostedExpenseReportHeader.Insert(false); end; - local procedure PrepareExpenseAgentAadApplication(var AadApplication: Record "AAD Application") - var - AccessControl: Record "Access Control"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); - if AadApplication.State <> AadApplication.State::Enabled then begin - AadApplication.Validate(State, AadApplication.State::Enabled); - AadApplication.Modify(true); - end; - - AccessControl.SetRange("User Security ID", AadApplication."User ID"); - AccessControl.SetRange("Role ID", ExpenseAgentPermissionSetTok); - AccessControl.DeleteAll(true); - end; - local procedure Initialize() begin LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Permissions Test"); From a70355a964fb3e9a265d8a11f4f224fffa7d97dd Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 01:32:50 +0200 Subject: [PATCH 04/24] Refactor Expense Agent lifecycle tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 86 +++++++++---------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 7c112abe124..4fbd2638a61 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -14,49 +14,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." InherentEntitlements = X; InherentPermissions = X; - local procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean - var - AccessControl: Record "Access Control"; - begin - SetExpenseAgentPermissionFilters(AccessControl, AadApplication); - AccessControl.SetRange("Company Name", CompanyName()); - exit(not AccessControl.IsEmpty()); - end; - - local procedure HasAnyExpenseAgentPermission(AadApplication: Record "AAD Application"): Boolean - var - AccessControl: Record "Access Control"; - begin - SetExpenseAgentPermissionFilters(AccessControl, AadApplication); - exit(not AccessControl.IsEmpty()); - end; - - local procedure AddPermissionForCurrentCompany(AadApplication: Record "AAD Application") - var - AccessControl: Record "Access Control"; - AggregatePermissionSet: Record "Aggregate Permission Set"; - begin - AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetLbl); - if not AggregatePermissionSet.FindFirst() then - exit; - - AccessControl.Init(); - AccessControl.Validate("User Security ID", AadApplication."User ID"); - AccessControl.Validate("Role ID", ExpenseAgentPermissionSetLbl); - AccessControl.Validate("App ID", AggregatePermissionSet."App ID"); - AccessControl.Validate("Company Name", CompanyName()); - AccessControl.Insert(true); - end; - - local procedure RemovePermissionForCurrentCompany(AadApplication: Record "AAD Application") - var - AccessControl: Record "Access Control"; - begin - SetExpenseAgentPermissionFilters(AccessControl, AadApplication); - AccessControl.SetRange("Company Name", CompanyName()); - AccessControl.DeleteAll(true); - end; - internal procedure EnableAadApplicationForCurrentCompany() var AadApplication: Record "AAD Application"; @@ -128,6 +85,49 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(ExpenseAgentAadAppIdTxt); end; + local procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean + var + AccessControl: Record "Access Control"; + begin + SetExpenseAgentPermissionFilters(AccessControl, AadApplication); + AccessControl.SetRange("Company Name", CompanyName()); + exit(not AccessControl.IsEmpty()); + end; + + local procedure HasAnyExpenseAgentPermission(AadApplication: Record "AAD Application"): Boolean + var + AccessControl: Record "Access Control"; + begin + SetExpenseAgentPermissionFilters(AccessControl, AadApplication); + exit(not AccessControl.IsEmpty()); + end; + + local procedure AddPermissionForCurrentCompany(AadApplication: Record "AAD Application") + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + begin + AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetLbl); + if not AggregatePermissionSet.FindFirst() then + exit; + + AccessControl.Init(); + AccessControl.Validate("User Security ID", AadApplication."User ID"); + AccessControl.Validate("Role ID", ExpenseAgentPermissionSetLbl); + AccessControl.Validate("App ID", AggregatePermissionSet."App ID"); + AccessControl.Validate("Company Name", CompanyName()); + AccessControl.Insert(true); + end; + + local procedure RemovePermissionForCurrentCompany(AadApplication: Record "AAD Application") + var + AccessControl: Record "Access Control"; + begin + SetExpenseAgentPermissionFilters(AccessControl, AadApplication); + AccessControl.SetRange("Company Name", CompanyName()); + AccessControl.DeleteAll(true); + end; + local procedure GetAadApplication(var AadApplication: Record "AAD Application"): Boolean begin AadApplication.SetRange("Client Id", GetAadAppId()); From 457683da7e022e3f6683667b3f8184610cb100ae Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 12:13:38 +0200 Subject: [PATCH 05/24] Address Expense Agent permission review findings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 62 +++++++++--- .../src/ExpenseAgentConfigTest.Codeunit.al | 97 +++++++++++++++---- 2 files changed, 123 insertions(+), 36 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 4fbd2638a61..dc77faec6df 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -42,12 +42,14 @@ codeunit 6913 "Expense Agent Entra App Mgt." internal procedure DisableAadApplicationForCurrentCompany() var AadApplication: Record "AAD Application"; + HasOtherCompanyPermission: Boolean; begin if not GetAadApplication(AadApplication) then exit; + HasOtherCompanyPermission := HasExpenseAgentPermissionForOtherCompany(AadApplication); RemovePermissionForCurrentCompany(AadApplication); - if HasAnyExpenseAgentPermission(AadApplication) then + if HasOtherCompanyPermission then exit; if AadApplication.State = AadApplication.State::Disabled then exit; @@ -88,17 +90,26 @@ codeunit 6913 "Expense Agent Entra App Mgt." local procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean var AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; begin - SetExpenseAgentPermissionFilters(AccessControl, AadApplication); - AccessControl.SetRange("Company Name", CompanyName()); + if not GetExpenseAgentPermissionSet(AggregatePermissionSet) then + exit(false); + + SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); + AccessControl.SetRange("Company Name", GetCurrentCompanyName()); exit(not AccessControl.IsEmpty()); end; - local procedure HasAnyExpenseAgentPermission(AadApplication: Record "AAD Application"): Boolean + local procedure HasExpenseAgentPermissionForOtherCompany(AadApplication: Record "AAD Application"): Boolean var AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; begin - SetExpenseAgentPermissionFilters(AccessControl, AadApplication); + if not GetExpenseAgentPermissionSet(AggregatePermissionSet) then + exit(false); + + SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); + AccessControl.SetFilter("Company Name", '<>%1', GetCurrentCompanyName()); exit(not AccessControl.IsEmpty()); end; @@ -107,40 +118,61 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; begin - AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetLbl); - if not AggregatePermissionSet.FindFirst() then + if not GetExpenseAgentPermissionSet(AggregatePermissionSet) then exit; AccessControl.Init(); AccessControl.Validate("User Security ID", AadApplication."User ID"); - AccessControl.Validate("Role ID", ExpenseAgentPermissionSetLbl); + AccessControl.Validate("Role ID", AggregatePermissionSet."Role ID"); AccessControl.Validate("App ID", AggregatePermissionSet."App ID"); - AccessControl.Validate("Company Name", CompanyName()); + AccessControl.Validate(Scope, AggregatePermissionSet.Scope); + AccessControl.Validate("Company Name", GetCurrentCompanyName()); AccessControl.Insert(true); end; local procedure RemovePermissionForCurrentCompany(AadApplication: Record "AAD Application") var AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; begin - SetExpenseAgentPermissionFilters(AccessControl, AadApplication); - AccessControl.SetRange("Company Name", CompanyName()); + if not GetExpenseAgentPermissionSet(AggregatePermissionSet) then + exit; + + SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); + AccessControl.SetRange("Company Name", GetCurrentCompanyName()); AccessControl.DeleteAll(true); end; local procedure GetAadApplication(var AadApplication: Record "AAD Application"): Boolean begin - AadApplication.SetRange("Client Id", GetAadAppId()); - exit(AadApplication.FindFirst()); + exit(AadApplication.Get(GetAadAppId())); + end; + + local procedure GetCurrentCompanyName(): Text[30] + begin + exit(CopyStr(CompanyName(), 1, 30)); + end; + + local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set"): Boolean + var + ExpenseAgentAppId: Guid; + begin + Evaluate(ExpenseAgentAppId, ExpenseAgentAppIdTxt); + AggregatePermissionSet.SetRange("App ID", ExpenseAgentAppId); + AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetLbl); + exit(AggregatePermissionSet.FindFirst()); end; - local procedure SetExpenseAgentPermissionFilters(var AccessControl: Record "Access Control"; AadApplication: Record "AAD Application") + local procedure SetExpenseAgentPermissionFilters(var AccessControl: Record "Access Control"; AadApplication: Record "AAD Application"; AggregatePermissionSet: Record "Aggregate Permission Set") begin AccessControl.SetRange("User Security ID", AadApplication."User ID"); - AccessControl.SetRange("Role ID", ExpenseAgentPermissionSetLbl); + AccessControl.SetRange("Role ID", AggregatePermissionSet."Role ID"); + AccessControl.SetRange(Scope, AggregatePermissionSet.Scope); + AccessControl.SetRange("App ID", AggregatePermissionSet."App ID"); end; var ExpenseAgentAadAppIdTxt: Label 'ee1eb5fd-719b-44f2-97d0-0efd34bc4148', Locked = true; + ExpenseAgentAppIdTxt: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; } diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al index 861fc1253a9..4a385531fbd 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -22,6 +22,7 @@ codeunit 148361 "Expense Agent Config. Test" Assert: Codeunit Assert; LibraryTestInitialize: Codeunit "Library - Test Initialize"; UserPermissionsLibrary: Codeunit "User Permissions Library"; + ExpenseAgentAppIdTok: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseAgentPermissionSetTok: Label 'Expense Agent', Locked = true; UnrelatedPermissionSetTok: Label 'D365 BASIC', Locked = true; @@ -45,7 +46,7 @@ codeunit 148361 "Expense Agent Config. Test" // [THEN] "EA" remains enabled and has Expense Agent permissions for both companies VerifyAadApplicationState(AadApplication.State::Enabled); - VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); end; @@ -60,14 +61,15 @@ codeunit 148361 "Expense Agent Config. Test" // [GIVEN] Disabled Entra app "EA" already has the Expense Agent permission for the current company PrepareAadApplication(AadApplication, AadApplication.State::Disabled); - AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + SetAadApplicationState(AadApplication.State::Disabled); // [WHEN] Activating the Expense Agent for the current company ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); // [THEN] "EA" is enabled with one current company permission VerifyAadApplicationState(AadApplication.State::Enabled); - VerifyPermissionCount(AadApplication, ExpenseAgentPermissionSetTok, CompanyName(), 1); + VerifyPermissionCount(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName(), 1); end; [Test] @@ -81,39 +83,38 @@ codeunit 148361 "Expense Agent Config. Test" // [GIVEN] Enabled Entra app "EA" has an unrelated permission but no Expense Agent permission PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - AssignPermission(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + AssignPermission(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); // [WHEN] Activating the Expense Agent for the current company ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); // [THEN] "EA" remains enabled with both the unrelated and current Expense Agent permissions VerifyAadApplicationState(AadApplication.State::Enabled); - VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, CompanyName()); - VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); end; [Test] - procedure DeactivatingWithOtherCompanyPermissionKeepsApplicationEnabled() + procedure DeactivatingPreservesOtherCompanyPermission() var AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; OtherCompanyName: Text[30]; begin - // [SCENARIO 640454] Deactivating preserves another company permission and keeps the Entra app enabled + // [SCENARIO 640454] Deactivating preserves another company permission Initialize(); // [GIVEN] Enabled Entra app "EA" has Expense Agent permissions for the current company and company "B" PrepareAadApplication(AadApplication, AadApplication.State::Enabled); OtherCompanyName := GetOtherCompanyName(); - AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); // [WHEN] Deactivating the Expense Agent for the current company ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - // [THEN] "EA" remains enabled with only company "B" Expense Agent permission - VerifyAadApplicationState(AadApplication.State::Enabled); - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + // [THEN] "EA" keeps only company "B" Expense Agent permission + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); end; @@ -128,14 +129,14 @@ codeunit 148361 "Expense Agent Config. Test" // [GIVEN] Enabled Entra app "EA" has the Expense Agent permission only for the current company PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); // [WHEN] Deactivating the Expense Agent for the current company ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); // [THEN] "EA" is disabled with no Expense Agent permission VerifyAadApplicationState(AadApplication.State::Disabled); - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); end; [Test] @@ -149,16 +150,16 @@ codeunit 148361 "Expense Agent Config. Test" // [GIVEN] Enabled Entra app "EA" has current Expense Agent and unrelated permissions PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); - AssignPermission(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + AssignPermission(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); // [WHEN] Deactivating the Expense Agent for the current company ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); // [THEN] "EA" is disabled, the Expense Agent permission is removed, and the unrelated permission remains VerifyAadApplicationState(AadApplication.State::Disabled); - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); - VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); + VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); end; [Test] @@ -172,15 +173,15 @@ codeunit 148361 "Expense Agent Config. Test" // [GIVEN] Enabled Entra app "EA" has only an unrelated permission PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - AssignPermission(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + AssignPermission(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); // [WHEN] Deactivating the Expense Agent for the current company ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); // [THEN] "EA" is disabled and the unrelated permission remains VerifyAadApplicationState(AadApplication.State::Disabled); - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, CompanyName()); - VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, CompanyName()); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); + VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); end; local procedure Initialize() @@ -204,15 +205,42 @@ codeunit 148361 "Expense Agent Config. Test" end; local procedure AssignPermission(AadApplication: Record "AAD Application"; PermissionSetId: Code[20]; CompanyNameValue: Text[30]) + var + AggregatePermissionSet: Record "Aggregate Permission Set"; begin + if PermissionSetId = ExpenseAgentPermissionSetTok then begin + GetExpenseAgentPermissionSet(AggregatePermissionSet); + AssignPermission(AadApplication, AggregatePermissionSet, CompanyNameValue); + SelectLatestVersion(); + exit; + end; + UserPermissionsLibrary.AssignPermissionSetToUser(AadApplication."User ID", PermissionSetId, CompanyNameValue); end; + local procedure AssignPermission(AadApplication: Record "AAD Application"; AggregatePermissionSet: Record "Aggregate Permission Set"; CompanyNameValue: Text[30]) + var + AccessControl: Record "Access Control"; + begin + AccessControl.Init(); + AccessControl."User Security ID" := AadApplication."User ID"; + AccessControl."Role ID" := AggregatePermissionSet."Role ID"; + AccessControl."Company Name" := CompanyNameValue; + AccessControl.Scope := AggregatePermissionSet.Scope; + AccessControl."App ID" := AggregatePermissionSet."App ID"; + AccessControl.Insert(true); + end; + local procedure GetOtherCompanyName(): Text[30] begin exit(CopyStr(Format(CreateGuid()), 1, 30)); end; + local procedure GetCurrentCompanyName(): Text[30] + begin + exit(CopyStr(CompanyName(), 1, 30)); + end; + local procedure VerifyAadApplicationState(ExpectedState: Option) var AadApplication: Record "AAD Application"; @@ -222,6 +250,16 @@ codeunit 148361 "Expense Agent Config. Test" Assert.AreEqual(ExpectedState, AadApplication.State, 'The Entra application state is incorrect.'); end; + local procedure SetAadApplicationState(State: Option) + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); + AadApplication.Validate(State, State); + AadApplication.Modify(true); + end; + local procedure VerifyPermissionExists(AadApplication: Record "AAD Application"; PermissionSetId: Code[20]; CompanyNameValue: Text[30]) begin VerifyPermissionCount(AadApplication, PermissionSetId, CompanyNameValue, 1); @@ -235,10 +273,27 @@ codeunit 148361 "Expense Agent Config. Test" local procedure VerifyPermissionCount(AadApplication: Record "AAD Application"; PermissionSetId: Code[20]; CompanyNameValue: Text[30]; ExpectedCount: Integer) var AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; begin + AadApplication.Get(AadApplication."Client Id"); AccessControl.SetRange("User Security ID", AadApplication."User ID"); + if PermissionSetId = ExpenseAgentPermissionSetTok then begin + GetExpenseAgentPermissionSet(AggregatePermissionSet); + AccessControl.SetRange(Scope, AggregatePermissionSet.Scope); + AccessControl.SetRange("App ID", AggregatePermissionSet."App ID"); + end; AccessControl.SetRange("Role ID", PermissionSetId); AccessControl.SetRange("Company Name", CompanyNameValue); Assert.AreEqual(ExpectedCount, AccessControl.Count(), 'The number of matching Entra app permissions is incorrect.'); end; + + local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") + var + ExpenseAgentAppId: Guid; + begin + Evaluate(ExpenseAgentAppId, ExpenseAgentAppIdTok); + AggregatePermissionSet.SetRange("App ID", ExpenseAgentAppId); + AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetTok); + AggregatePermissionSet.FindFirst(); + end; } From ce5d47576e19a9fa058820e4287cd7b54e0d79e9 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 13:35:01 +0200 Subject: [PATCH 06/24] Harden Expense Agent Entra permission lifecycle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 16 ++++- .../src/ExpenseAgentConfigTest.Codeunit.al | 67 ++++++++++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index dc77faec6df..796c204eefc 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -4,6 +4,7 @@ // ------------------------------------------------------------------------------------------------ namespace Microsoft.ExpenseAgent; +using System.Agents; using System.Environment; using System.Environment.Configuration; using System.Security.AccessControl; @@ -18,6 +19,8 @@ codeunit 6913 "Expense Agent Entra App Mgt." var AadApplication: Record "AAD Application"; begin + EnsureCurrentUserCanManageExpenseAgent(); + if not GetAadApplication(AadApplication) then exit; @@ -44,6 +47,8 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication: Record "AAD Application"; HasOtherCompanyPermission: Boolean; begin + EnsureCurrentUserCanManageExpenseAgent(); + if not GetAadApplication(AadApplication) then exit; @@ -96,7 +101,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(false); SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); - AccessControl.SetRange("Company Name", GetCurrentCompanyName()); + AccessControl.SetFilter("Company Name", '%1|''''', GetCurrentCompanyName()); exit(not AccessControl.IsEmpty()); end; @@ -153,6 +158,14 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(CopyStr(CompanyName(), 1, 30)); end; + local procedure EnsureCurrentUserCanManageExpenseAgent() + var + AgentSystemPermissions: Codeunit "Agent System Permissions"; + begin + if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then + Error(NotAuthorizedToManageExpenseAgentErr); + end; + local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set"): Boolean var ExpenseAgentAppId: Guid; @@ -175,4 +188,5 @@ codeunit 6913 "Expense Agent Entra App Mgt." ExpenseAgentAadAppIdTxt: Label 'ee1eb5fd-719b-44f2-97d0-0efd34bc4148', Locked = true; ExpenseAgentAppIdTxt: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; + NotAuthorizedToManageExpenseAgentErr: Label 'You do not have permission to manage the Expense Agent.'; } diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al index 4a385531fbd..46bf2b5f3fd 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -16,7 +16,7 @@ codeunit 148361 "Expense Agent Config. Test" TestPermissions = Restrictive; Permissions = tabledata "AAD Application" = rm, - tabledata "Access Control" = rimd; + tabledata "Access Control" = rid; var Assert: Codeunit Assert; @@ -94,6 +94,29 @@ codeunit 148361 "Expense Agent Config. Test" VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); end; + [Test] + procedure ActivatingWithGlobalPermissionDoesNotAddCompanyPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [SCENARIO 640454] Activating with a global Expense Agent permission does not add a company permission + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has a global Expense Agent permission + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + MakeCurrentExpenseAgentPermissionGlobal(AadApplication); + + // [WHEN] Activating the Expense Agent for the current company + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [THEN] "EA" remains enabled with only the global Expense Agent permission + VerifyAadApplicationState(AadApplication.State::Enabled); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, ''); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); + end; + [Test] procedure DeactivatingPreservesOtherCompanyPermission() var @@ -184,6 +207,28 @@ codeunit 148361 "Expense Agent Config. Test" VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); end; + [Test] + procedure DeactivatingWithGlobalPermissionPreservesApplication() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [SCENARIO 640454] Deactivating preserves a global Expense Agent permission and the Entra app state + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has a global Expense Agent permission + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + MakeCurrentExpenseAgentPermissionGlobal(AadApplication); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] "EA" remains enabled with its global Expense Agent permission + VerifyAadApplicationState(AadApplication.State::Enabled); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, ''); + end; + local procedure Initialize() begin LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Agent Config. Test"); @@ -296,4 +341,24 @@ codeunit 148361 "Expense Agent Config. Test" AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetTok); AggregatePermissionSet.FindFirst(); end; + + local procedure MakeCurrentExpenseAgentPermissionGlobal(AadApplication: Record "AAD Application") + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + begin + GetExpenseAgentPermissionSet(AggregatePermissionSet); + AccessControl.Get( + AadApplication."User ID", + AggregatePermissionSet."Role ID", + GetCurrentCompanyName(), + AggregatePermissionSet.Scope, + AggregatePermissionSet."App ID"); + AccessControl.Rename( + AadApplication."User ID", + AggregatePermissionSet."Role ID", + '', + AggregatePermissionSet.Scope, + AggregatePermissionSet."App ID"); + end; } From bba15205ea09aca4c9833e8c21a9f642df5c9f01 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 17:38:58 +0200 Subject: [PATCH 07/24] Fail on missing Expense Agent prerequisites Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 796c204eefc..1a24ad74034 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -20,9 +20,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication: Record "AAD Application"; begin EnsureCurrentUserCanManageExpenseAgent(); - - if not GetAadApplication(AadApplication) then - exit; + GetAadApplication(AadApplication); // Enabling creates the application user. Disable it again before changing permissions. if AadApplication.State <> AadApplication.State::Enabled then begin @@ -48,9 +46,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." HasOtherCompanyPermission: Boolean; begin EnsureCurrentUserCanManageExpenseAgent(); - - if not GetAadApplication(AadApplication) then - exit; + GetAadApplication(AadApplication); HasOtherCompanyPermission := HasExpenseAgentPermissionForOtherCompany(AadApplication); RemovePermissionForCurrentCompany(AadApplication); @@ -97,8 +93,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; begin - if not GetExpenseAgentPermissionSet(AggregatePermissionSet) then - exit(false); + GetExpenseAgentPermissionSet(AggregatePermissionSet); SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); AccessControl.SetFilter("Company Name", '%1|''''', GetCurrentCompanyName()); @@ -110,8 +105,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; begin - if not GetExpenseAgentPermissionSet(AggregatePermissionSet) then - exit(false); + GetExpenseAgentPermissionSet(AggregatePermissionSet); SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); AccessControl.SetFilter("Company Name", '<>%1', GetCurrentCompanyName()); @@ -123,8 +117,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; begin - if not GetExpenseAgentPermissionSet(AggregatePermissionSet) then - exit; + GetExpenseAgentPermissionSet(AggregatePermissionSet); AccessControl.Init(); AccessControl.Validate("User Security ID", AadApplication."User ID"); @@ -140,17 +133,17 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; begin - if not GetExpenseAgentPermissionSet(AggregatePermissionSet) then - exit; + GetExpenseAgentPermissionSet(AggregatePermissionSet); SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); AccessControl.SetRange("Company Name", GetCurrentCompanyName()); AccessControl.DeleteAll(true); end; - local procedure GetAadApplication(var AadApplication: Record "AAD Application"): Boolean + local procedure GetAadApplication(var AadApplication: Record "AAD Application") begin - exit(AadApplication.Get(GetAadAppId())); + if not AadApplication.Get(GetAadAppId()) then + Error(AadApplicationMissingErr); end; local procedure GetCurrentCompanyName(): Text[30] @@ -166,14 +159,15 @@ codeunit 6913 "Expense Agent Entra App Mgt." Error(NotAuthorizedToManageExpenseAgentErr); end; - local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set"): Boolean + local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") var ExpenseAgentAppId: Guid; begin Evaluate(ExpenseAgentAppId, ExpenseAgentAppIdTxt); AggregatePermissionSet.SetRange("App ID", ExpenseAgentAppId); AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetLbl); - exit(AggregatePermissionSet.FindFirst()); + if not AggregatePermissionSet.FindFirst() then + Error(ExpenseAgentPermissionSetMissingErr); end; local procedure SetExpenseAgentPermissionFilters(var AccessControl: Record "Access Control"; AadApplication: Record "AAD Application"; AggregatePermissionSet: Record "Aggregate Permission Set") @@ -188,5 +182,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." ExpenseAgentAadAppIdTxt: Label 'ee1eb5fd-719b-44f2-97d0-0efd34bc4148', Locked = true; ExpenseAgentAppIdTxt: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; + AadApplicationMissingErr: Label 'The Expense Agent Microsoft Entra application is not configured.'; + ExpenseAgentPermissionSetMissingErr: Label 'The Expense Agent permission set is not available.'; NotAuthorizedToManageExpenseAgentErr: Label 'You do not have permission to manage the Expense Agent.'; } From deaffd785dbd8583443984a5b50eb40a9f03556a Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 22:03:02 +0200 Subject: [PATCH 08/24] Preflight Expense Agent security permissions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 25 +++++++++++-------- .../Pages/ExpenseAgentSetupWizard.Page.al | 1 + 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 1a24ad74034..79767fd7d52 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -8,6 +8,7 @@ using System.Agents; using System.Environment; using System.Environment.Configuration; using System.Security.AccessControl; +using System.Security.User; codeunit 6913 "Expense Agent Entra App Mgt." { @@ -19,7 +20,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." var AadApplication: Record "AAD Application"; begin - EnsureCurrentUserCanManageExpenseAgent(); + VerifyCurrentUserCanManageExpenseAgent(); GetAadApplication(AadApplication); // Enabling creates the application user. Disable it again before changing permissions. @@ -45,7 +46,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication: Record "AAD Application"; HasOtherCompanyPermission: Boolean; begin - EnsureCurrentUserCanManageExpenseAgent(); + VerifyCurrentUserCanManageExpenseAgent(); GetAadApplication(AadApplication); HasOtherCompanyPermission := HasExpenseAgentPermissionForOtherCompany(AadApplication); @@ -88,6 +89,17 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(ExpenseAgentAadAppIdTxt); end; + internal procedure VerifyCurrentUserCanManageExpenseAgent() + var + AgentSystemPermissions: Codeunit "Agent System Permissions"; + UserPermissions: Codeunit "User Permissions"; + begin + if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then + Error(NotAuthorizedToManageExpenseAgentErr); + if not UserPermissions.CanManageUsersOnTenant(UserSecurityId()) then + Error(SecurityPermissionRequiredErr); + end; + local procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean var AccessControl: Record "Access Control"; @@ -151,14 +163,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(CopyStr(CompanyName(), 1, 30)); end; - local procedure EnsureCurrentUserCanManageExpenseAgent() - var - AgentSystemPermissions: Codeunit "Agent System Permissions"; - begin - if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then - Error(NotAuthorizedToManageExpenseAgentErr); - end; - local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") var ExpenseAgentAppId: Guid; @@ -185,4 +189,5 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplicationMissingErr: Label 'The Expense Agent Microsoft Entra application is not configured.'; ExpenseAgentPermissionSetMissingErr: Label 'The Expense Agent permission set is not available.'; NotAuthorizedToManageExpenseAgentErr: Label 'You do not have permission to manage the Expense Agent.'; + SecurityPermissionRequiredErr: Label 'You must be assigned either the SUPER permission set or the SECURITY permission set directly in the current company to manage the Expense Agent Microsoft Entra application.'; } diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al index 6e035307803..53743c72add 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al @@ -1442,6 +1442,7 @@ page 6991 "Expense Agent Setup Wizard" var ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin + ExpenseAgentEntraApp.VerifyCurrentUserCanManageExpenseAgent(); if not Rec.ShowDeactivationAccessWarning() then exit(false); if not UnregisterErpConfiguration() then From e503922ccac0d7463a9857dcabeae0b59da8fcd7 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 22:32:40 +0200 Subject: [PATCH 09/24] Preflight Expense Agent Entra authorization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 34 ++++++++++++++++--- .../Pages/ExpenseAgentSetupWizard.Page.al | 2 +- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 79767fd7d52..22766572a40 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -46,7 +46,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication: Record "AAD Application"; HasOtherCompanyPermission: Boolean; begin - VerifyCurrentUserCanManageExpenseAgent(); + VerifyCanDisableAadApplicationForCurrentCompany(); GetAadApplication(AadApplication); HasOtherCompanyPermission := HasExpenseAgentPermissionForOtherCompany(AadApplication); @@ -89,15 +89,38 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(ExpenseAgentAadAppIdTxt); end; - internal procedure VerifyCurrentUserCanManageExpenseAgent() + internal procedure VerifyCanDisableAadApplicationForCurrentCompany() + var + AadApplication: Record "AAD Application"; + begin + VerifyCurrentUserCanManageExpenseAgent(); + GetAadApplication(AadApplication); + end; + + local procedure VerifyCurrentUserCanManageExpenseAgent() var AgentSystemPermissions: Codeunit "Agent System Permissions"; + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; UserPermissions: Codeunit "User Permissions"; + NullGuid: Guid; begin if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then Error(NotAuthorizedToManageExpenseAgentErr); - if not UserPermissions.CanManageUsersOnTenant(UserSecurityId()) then + + GetExpenseAgentPermissionSet(AggregatePermissionSet); + if UserPermissions.HasUserPermissionSetAssigned(UserSecurityId(), GetCurrentCompanyName(), SuperPermissionSetTok, AccessControl.Scope::System, NullGuid) then + exit; + if not UserPermissions.HasUserPermissionSetAssigned(UserSecurityId(), GetCurrentCompanyName(), SecurityPermissionSetTok, AccessControl.Scope::System, NullGuid) then Error(SecurityPermissionRequiredErr); + if not UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), + GetCurrentCompanyName(), + AggregatePermissionSet."Role ID", + AggregatePermissionSet.Scope, + AggregatePermissionSet."App ID") + then + Error(ExpenseAgentPermissionRequiredErr); end; local procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean @@ -186,8 +209,11 @@ codeunit 6913 "Expense Agent Entra App Mgt." ExpenseAgentAadAppIdTxt: Label 'ee1eb5fd-719b-44f2-97d0-0efd34bc4148', Locked = true; ExpenseAgentAppIdTxt: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; + ExpenseAgentPermissionRequiredErr: Label 'You don''t have rights to manage the Expense Agent permission set on users. The SECURITY permission set only grants you rights to manage those permission sets that are also assigned to your account.'; AadApplicationMissingErr: Label 'The Expense Agent Microsoft Entra application is not configured.'; ExpenseAgentPermissionSetMissingErr: Label 'The Expense Agent permission set is not available.'; NotAuthorizedToManageExpenseAgentErr: Label 'You do not have permission to manage the Expense Agent.'; - SecurityPermissionRequiredErr: Label 'You must be assigned either the SUPER permission set or the SECURITY permission set directly in the current company to manage the Expense Agent Microsoft Entra application.'; + SecurityPermissionRequiredErr: Label 'You must be assigned either the SUPER or SECURITY permission set to manage the Expense Agent Microsoft Entra application.'; + SecurityPermissionSetTok: Label 'SECURITY', Locked = true; + SuperPermissionSetTok: Label 'SUPER', Locked = true; } diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al index 53743c72add..00fba63a99a 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al @@ -1442,7 +1442,7 @@ page 6991 "Expense Agent Setup Wizard" var ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - ExpenseAgentEntraApp.VerifyCurrentUserCanManageExpenseAgent(); + ExpenseAgentEntraApp.VerifyCanDisableAadApplicationForCurrentCompany(); if not Rec.ShowDeactivationAccessWarning() then exit(false); if not UnregisterErpConfiguration() then From 728248eb15b3f304522a3f8f830ef78c088158e9 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Thu, 10 Sep 2026 22:44:23 +0200 Subject: [PATCH 10/24] Align Expense Agent preflight with platform security Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 22766572a40..7f8ae55bdf5 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -99,19 +99,15 @@ codeunit 6913 "Expense Agent Entra App Mgt." local procedure VerifyCurrentUserCanManageExpenseAgent() var - AgentSystemPermissions: Codeunit "Agent System Permissions"; - AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; + AgentSystemPermissions: Codeunit "Agent System Permissions"; UserPermissions: Codeunit "User Permissions"; - NullGuid: Guid; begin if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then Error(NotAuthorizedToManageExpenseAgentErr); GetExpenseAgentPermissionSet(AggregatePermissionSet); - if UserPermissions.HasUserPermissionSetAssigned(UserSecurityId(), GetCurrentCompanyName(), SuperPermissionSetTok, AccessControl.Scope::System, NullGuid) then - exit; - if not UserPermissions.HasUserPermissionSetAssigned(UserSecurityId(), GetCurrentCompanyName(), SecurityPermissionSetTok, AccessControl.Scope::System, NullGuid) then + if not UserPermissions.CanManageUsersOnTenant(UserSecurityId()) then Error(SecurityPermissionRequiredErr); if not UserPermissions.HasUserPermissionSetAssigned( UserSecurityId(), @@ -214,6 +210,4 @@ codeunit 6913 "Expense Agent Entra App Mgt." ExpenseAgentPermissionSetMissingErr: Label 'The Expense Agent permission set is not available.'; NotAuthorizedToManageExpenseAgentErr: Label 'You do not have permission to manage the Expense Agent.'; SecurityPermissionRequiredErr: Label 'You must be assigned either the SUPER or SECURITY permission set to manage the Expense Agent Microsoft Entra application.'; - SecurityPermissionSetTok: Label 'SECURITY', Locked = true; - SuperPermissionSetTok: Label 'SUPER', Locked = true; } From 86c9a406d46e4ae7710551ec03b0181d9b01a451 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Fri, 11 Sep 2026 11:03:41 +0200 Subject: [PATCH 11/24] Narrow Expense Agent fix to activation scope Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 72 +---------- .../Pages/ExpenseAgentSetupWizard.Page.al | 4 - .../src/ExpenseAgentConfigTest.Codeunit.al | 112 ------------------ 3 files changed, 2 insertions(+), 186 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 7f8ae55bdf5..54a1b678254 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -8,7 +8,6 @@ using System.Agents; using System.Environment; using System.Environment.Configuration; using System.Security.AccessControl; -using System.Security.User; codeunit 6913 "Expense Agent Entra App Mgt." { @@ -20,7 +19,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." var AadApplication: Record "AAD Application"; begin - VerifyCurrentUserCanManageExpenseAgent(); + VerifyCurrentUserCanManageAgents(); GetAadApplication(AadApplication); // Enabling creates the application user. Disable it again before changing permissions. @@ -41,25 +40,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication.Modify(true); end; - internal procedure DisableAadApplicationForCurrentCompany() - var - AadApplication: Record "AAD Application"; - HasOtherCompanyPermission: Boolean; - begin - VerifyCanDisableAadApplicationForCurrentCompany(); - GetAadApplication(AadApplication); - - HasOtherCompanyPermission := HasExpenseAgentPermissionForOtherCompany(AadApplication); - RemovePermissionForCurrentCompany(AadApplication); - if HasOtherCompanyPermission then - exit; - if AadApplication.State = AadApplication.State::Disabled then - exit; - - AadApplication.Validate(State, AadApplication.State::Disabled); - AadApplication.Modify(true); - end; - internal procedure IsCurrentUserExpenseAgent(): Boolean var AadApplication: Record "AAD Application"; @@ -89,34 +69,12 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(ExpenseAgentAadAppIdTxt); end; - internal procedure VerifyCanDisableAadApplicationForCurrentCompany() - var - AadApplication: Record "AAD Application"; - begin - VerifyCurrentUserCanManageExpenseAgent(); - GetAadApplication(AadApplication); - end; - - local procedure VerifyCurrentUserCanManageExpenseAgent() + local procedure VerifyCurrentUserCanManageAgents() var - AggregatePermissionSet: Record "Aggregate Permission Set"; AgentSystemPermissions: Codeunit "Agent System Permissions"; - UserPermissions: Codeunit "User Permissions"; begin if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then Error(NotAuthorizedToManageExpenseAgentErr); - - GetExpenseAgentPermissionSet(AggregatePermissionSet); - if not UserPermissions.CanManageUsersOnTenant(UserSecurityId()) then - Error(SecurityPermissionRequiredErr); - if not UserPermissions.HasUserPermissionSetAssigned( - UserSecurityId(), - GetCurrentCompanyName(), - AggregatePermissionSet."Role ID", - AggregatePermissionSet.Scope, - AggregatePermissionSet."App ID") - then - Error(ExpenseAgentPermissionRequiredErr); end; local procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean @@ -131,18 +89,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(not AccessControl.IsEmpty()); end; - local procedure HasExpenseAgentPermissionForOtherCompany(AadApplication: Record "AAD Application"): Boolean - var - AccessControl: Record "Access Control"; - AggregatePermissionSet: Record "Aggregate Permission Set"; - begin - GetExpenseAgentPermissionSet(AggregatePermissionSet); - - SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); - AccessControl.SetFilter("Company Name", '<>%1', GetCurrentCompanyName()); - exit(not AccessControl.IsEmpty()); - end; - local procedure AddPermissionForCurrentCompany(AadApplication: Record "AAD Application") var AccessControl: Record "Access Control"; @@ -159,18 +105,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl.Insert(true); end; - local procedure RemovePermissionForCurrentCompany(AadApplication: Record "AAD Application") - var - AccessControl: Record "Access Control"; - AggregatePermissionSet: Record "Aggregate Permission Set"; - begin - GetExpenseAgentPermissionSet(AggregatePermissionSet); - - SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); - AccessControl.SetRange("Company Name", GetCurrentCompanyName()); - AccessControl.DeleteAll(true); - end; - local procedure GetAadApplication(var AadApplication: Record "AAD Application") begin if not AadApplication.Get(GetAadAppId()) then @@ -205,9 +139,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." ExpenseAgentAadAppIdTxt: Label 'ee1eb5fd-719b-44f2-97d0-0efd34bc4148', Locked = true; ExpenseAgentAppIdTxt: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; - ExpenseAgentPermissionRequiredErr: Label 'You don''t have rights to manage the Expense Agent permission set on users. The SECURITY permission set only grants you rights to manage those permission sets that are also assigned to your account.'; AadApplicationMissingErr: Label 'The Expense Agent Microsoft Entra application is not configured.'; ExpenseAgentPermissionSetMissingErr: Label 'The Expense Agent permission set is not available.'; NotAuthorizedToManageExpenseAgentErr: Label 'You do not have permission to manage the Expense Agent.'; - SecurityPermissionRequiredErr: Label 'You must be assigned either the SUPER or SECURITY permission set to manage the Expense Agent Microsoft Entra application.'; } diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al index 00fba63a99a..f030c0a889d 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al @@ -1439,15 +1439,11 @@ page 6991 "Expense Agent Setup Wizard" end; local procedure DeactivateAgent(): Boolean - var - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - ExpenseAgentEntraApp.VerifyCanDisableAadApplicationForCurrentCompany(); if not Rec.ShowDeactivationAccessWarning() then exit(false); if not UnregisterErpConfiguration() then exit(false); - ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); Rec.LogAgentDisabledTelemetry(); exit(true); end; diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al index 46bf2b5f3fd..ef96ab822aa 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -117,118 +117,6 @@ codeunit 148361 "Expense Agent Config. Test" VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); end; - [Test] - procedure DeactivatingPreservesOtherCompanyPermission() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - OtherCompanyName: Text[30]; - begin - // [SCENARIO 640454] Deactivating preserves another company permission - Initialize(); - - // [GIVEN] Enabled Entra app "EA" has Expense Agent permissions for the current company and company "B" - PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - OtherCompanyName := GetOtherCompanyName(); - ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); - - // [WHEN] Deactivating the Expense Agent for the current company - ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - - // [THEN] "EA" keeps only company "B" Expense Agent permission - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); - VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); - end; - - [Test] - procedure DeactivatingLastExpensePermissionDisablesApplication() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Deactivating the last Expense Agent company disables the Entra app - Initialize(); - - // [GIVEN] Enabled Entra app "EA" has the Expense Agent permission only for the current company - PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - - // [WHEN] Deactivating the Expense Agent for the current company - ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - - // [THEN] "EA" is disabled with no Expense Agent permission - VerifyAadApplicationState(AadApplication.State::Disabled); - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); - end; - - [Test] - procedure DeactivatingLastExpensePermissionPreservesUnrelatedPermission() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Deactivating the last Expense Agent company preserves unrelated permissions - Initialize(); - - // [GIVEN] Enabled Entra app "EA" has current Expense Agent and unrelated permissions - PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - AssignPermission(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); - ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - - // [WHEN] Deactivating the Expense Agent for the current company - ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - - // [THEN] "EA" is disabled, the Expense Agent permission is removed, and the unrelated permission remains - VerifyAadApplicationState(AadApplication.State::Disabled); - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); - VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); - end; - - [Test] - procedure DeactivatingWithoutExpensePermissionDisablesApplication() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Deactivating disables the Entra app when no Expense Agent company permission exists - Initialize(); - - // [GIVEN] Enabled Entra app "EA" has only an unrelated permission - PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - AssignPermission(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); - - // [WHEN] Deactivating the Expense Agent for the current company - ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - - // [THEN] "EA" is disabled and the unrelated permission remains - VerifyAadApplicationState(AadApplication.State::Disabled); - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); - VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); - end; - - [Test] - procedure DeactivatingWithGlobalPermissionPreservesApplication() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Deactivating preserves a global Expense Agent permission and the Entra app state - Initialize(); - - // [GIVEN] Enabled Entra app "EA" has a global Expense Agent permission - PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - MakeCurrentExpenseAgentPermissionGlobal(AadApplication); - - // [WHEN] Deactivating the Expense Agent for the current company - ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - - // [THEN] "EA" remains enabled with its global Expense Agent permission - VerifyAadApplicationState(AadApplication.State::Enabled); - VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, ''); - end; - local procedure Initialize() begin LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Agent Config. Test"); From 756ca7d0e8d8f462710779aee7e779e3dc85e7d4 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 15:35:10 +0200 Subject: [PATCH 12/24] Complete Expense Agent company lifecycle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 247 +++++++++++++- .../Pages/ExpenseAgentSetupWizard.Page.al | 59 +++- .../src/ExpenseAgentConfigTest.Codeunit.al | 192 ++++++++++- .../src/ExpensePermissionsTest.Codeunit.al | 320 +++++++++++++++++- 4 files changed, 782 insertions(+), 36 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 54a1b678254..129ba3d30bd 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -8,18 +8,30 @@ using System.Agents; using System.Environment; using System.Environment.Configuration; using System.Security.AccessControl; +using System.Security.User; codeunit 6913 "Expense Agent Entra App Mgt." { Access = Internal; InherentEntitlements = X; InherentPermissions = X; + Permissions = + tabledata "AAD Application" = rm, + tabledata "Access Control" = rimd, + tabledata Company = r, + tabledata "Expense Agent Setup" = r; internal procedure EnableAadApplicationForCurrentCompany() + begin + VerifyCurrentUserCanManageExpenseAgent(); + EnableAadApplicationForCurrentCompanyWithoutAuthorization(); + end; + + local procedure EnableAadApplicationForCurrentCompanyWithoutAuthorization() var AadApplication: Record "AAD Application"; begin - VerifyCurrentUserCanManageAgents(); + AadApplication.LockTable(); GetAadApplication(AadApplication); // Enabling creates the application user. Disable it again before changing permissions. @@ -28,18 +40,67 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication.Modify(true); end; - if HasPermissionForCurrentCompany(AadApplication) then + if HasPermissionForCurrentCompany(AadApplication) and not HasGlobalPermission(AadApplication) then exit; AadApplication.Validate(State, AadApplication.State::Disabled); AadApplication.Modify(true); - AddPermissionForCurrentCompany(AadApplication); + NormalizeGlobalPermission(AadApplication, true, GetCurrentCompanyName()); + AddPermissionForCompany(AadApplication, GetCurrentCompanyName()); AadApplication.Validate(State, AadApplication.State::Enabled); AadApplication.Modify(true); end; + [EventSubscriber(ObjectType::Page, Page::"Expense Agent Setup Wizard", OnReconcileExpenseAgentEntraApplication, '', false, false)] + local procedure OnReconcileExpenseAgentEntraApplication(EnableAgent: Boolean) + begin + if EnableAgent then + EnableAadApplicationForCurrentCompanyWithoutAuthorization() + else + DisableAadApplicationForCompany(GetCurrentCompanyName()); + end; + + internal procedure DisableAadApplicationForCurrentCompany() + begin + VerifyCanDisableAadApplicationForCurrentCompany(); + DisableAadApplicationForCompany(GetCurrentCompanyName()); + end; + + local procedure DisableAadApplicationForCompany(CompanyNameValue: Text[30]) + var + AadApplication: Record "AAD Application"; + HasOtherEnabledCompany: Boolean; + begin + AadApplication.LockTable(); + if not AadApplication.Get(GetAadAppId()) then + exit; + + HasOtherEnabledCompany := HasEnabledExpenseAgentForOtherCompany(CompanyNameValue); + if HasGlobalPermission(AadApplication) then begin + if AadApplication.State <> AadApplication.State::Disabled then begin + AadApplication.Validate(State, AadApplication.State::Disabled); + AadApplication.Modify(true); + end; + NormalizeGlobalPermission(AadApplication, false, CompanyNameValue); + end; + RemovePermissionForCompany(AadApplication, CompanyNameValue); + if HasOtherEnabledCompany then begin + GetAadApplication(AadApplication); + if AadApplication.State <> AadApplication.State::Enabled then begin + AadApplication.Validate(State, AadApplication.State::Enabled); + AadApplication.Modify(true); + end; + exit; + end; + if AadApplication.State = AadApplication.State::Disabled then + exit; + + AadApplication.Validate(State, AadApplication.State::Disabled); + AadApplication.Modify(true); + end; + internal procedure IsCurrentUserExpenseAgent(): Boolean var AadApplication: Record "AAD Application"; @@ -69,15 +130,92 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(ExpenseAgentAadAppIdTxt); end; - local procedure VerifyCurrentUserCanManageAgents() + internal procedure VerifyCanDisableAadApplicationForCurrentCompany() + var + AadApplication: Record "AAD Application"; + begin + VerifyCurrentUserCanManageExpenseAgent(); + GetAadApplication(AadApplication); + end; + + internal procedure LockAadApplicationForReconciliation() + var + AadApplication: Record "AAD Application"; + begin + AadApplication.LockTable(); + GetAadApplication(AadApplication); + end; + + local procedure VerifyCurrentUserCanManageExpenseAgent() var + AggregatePermissionSet: Record "Aggregate Permission Set"; AgentSystemPermissions: Codeunit "Agent System Permissions"; + UserPermissions: Codeunit "User Permissions"; begin + GetAgentAdminPermissionSet(AggregatePermissionSet); + if not UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), + GetCurrentCompanyName(), + AggregatePermissionSet."Role ID", + AggregatePermissionSet.Scope, + AggregatePermissionSet."App ID") + then + Error(AgentAdminPermissionRequiredErr); + if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then - Error(NotAuthorizedToManageExpenseAgentErr); + Error(AgentAdminPermissionRequiredErr); + + GetExpenseManagementAdminPermissionSet(AggregatePermissionSet); + if not UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), + GetCurrentCompanyName(), + AggregatePermissionSet."Role ID", + AggregatePermissionSet.Scope, + AggregatePermissionSet."App ID") + then + Error(ExpenseManagementAdminPermissionRequiredErr); + + if not HasSecurityPermission(UserPermissions) then + Error(SecurityPermissionRequiredErr); + + GetExpenseAgentPermissionSet(AggregatePermissionSet); + if not UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), + GetCurrentCompanyName(), + AggregatePermissionSet."Role ID", + AggregatePermissionSet.Scope, + AggregatePermissionSet."App ID") + then + Error(ExpenseAgentPermissionRequiredErr); + end; + + local procedure HasSecurityPermission(UserPermissions: Codeunit "User Permissions"): Boolean + var + AccessControl: Record "Access Control"; + NullGuid: Guid; + begin + if UserPermissions.IsSuper(UserSecurityId()) then + exit(true); + + exit(UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), + GetCurrentCompanyName(), + SecurityPermissionSetLbl, + AccessControl.Scope::System, + NullGuid)); end; local procedure HasPermissionForCurrentCompany(AadApplication: Record "AAD Application"): Boolean + begin + exit(HasPermissionForCompany(AadApplication, GetCurrentCompanyName())); + end; + + local procedure HasGlobalPermission(AadApplication: Record "AAD Application"): Boolean + begin + exit(HasPermissionForCompany(AadApplication, '')); + end; + + local procedure HasPermissionForCompany(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]): Boolean var AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; @@ -85,15 +223,58 @@ codeunit 6913 "Expense Agent Entra App Mgt." GetExpenseAgentPermissionSet(AggregatePermissionSet); SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); - AccessControl.SetFilter("Company Name", '%1|''''', GetCurrentCompanyName()); + AccessControl.SetRange("Company Name", CompanyNameValue); exit(not AccessControl.IsEmpty()); end; - local procedure AddPermissionForCurrentCompany(AadApplication: Record "AAD Application") + local procedure HasEnabledExpenseAgentForOtherCompany(CompanyNameValue: Text[30]): Boolean + var + Company: Record Company; + ExpenseAgentSetup: Record "Expense Agent Setup"; + begin + Company.SecurityFiltering(SecurityFilter::Ignored); + Company.SetFilter(Name, '<>%1', CompanyNameValue); + if Company.FindSet() then + repeat + ExpenseAgentSetup.ChangeCompany(Company.Name); + if ExpenseAgentSetup.Get() and ExpenseAgentSetup."Enable Agent" then + exit(true); + until Company.Next() = 0; + + exit(false); + end; + + local procedure NormalizeGlobalPermission(AadApplication: Record "AAD Application"; IncludeCurrentCompany: Boolean; CurrentCompanyNameValue: Text[30]) + var + Company: Record Company; + ExpenseAgentSetup: Record "Expense Agent Setup"; + begin + if not HasGlobalPermission(AadApplication) then + exit; + + if IncludeCurrentCompany then + AddPermissionForCompany(AadApplication, CurrentCompanyNameValue); + + Company.SecurityFiltering(SecurityFilter::Ignored); + Company.SetFilter(Name, '<>%1', CurrentCompanyNameValue); + if Company.FindSet() then + repeat + ExpenseAgentSetup.ChangeCompany(Company.Name); + if ExpenseAgentSetup.Get() and ExpenseAgentSetup."Enable Agent" then + AddPermissionForCompany(AadApplication, Company.Name); + until Company.Next() = 0; + + RemovePermissionForCompany(AadApplication, ''); + end; + + local procedure AddPermissionForCompany(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]) var AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; begin + if HasPermissionForCompany(AadApplication, CompanyNameValue) then + exit; + GetExpenseAgentPermissionSet(AggregatePermissionSet); AccessControl.Init(); @@ -101,10 +282,22 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl.Validate("Role ID", AggregatePermissionSet."Role ID"); AccessControl.Validate("App ID", AggregatePermissionSet."App ID"); AccessControl.Validate(Scope, AggregatePermissionSet.Scope); - AccessControl.Validate("Company Name", GetCurrentCompanyName()); + AccessControl.Validate("Company Name", CompanyNameValue); AccessControl.Insert(true); end; + local procedure RemovePermissionForCompany(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]) + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + begin + GetExpenseAgentPermissionSet(AggregatePermissionSet); + + SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); + AccessControl.SetRange("Company Name", CompanyNameValue); + AccessControl.DeleteAll(true); + end; + local procedure GetAadApplication(var AadApplication: Record "AAD Application") begin if not AadApplication.Get(GetAadAppId()) then @@ -117,14 +310,37 @@ codeunit 6913 "Expense Agent Entra App Mgt." end; local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") + begin + GetPermissionSet(AggregatePermissionSet, ExpenseAgentPermissionSetLbl); + end; + + local procedure GetAgentAdminPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") + var + BaseApplicationAppId: Guid; + begin + Evaluate(BaseApplicationAppId, BaseApplicationAppIdTxt); + AggregatePermissionSet.Reset(); + AggregatePermissionSet.SetRange("App ID", BaseApplicationAppId); + AggregatePermissionSet.SetRange("Role ID", AgentAdminPermissionSetLbl); + if not AggregatePermissionSet.FindFirst() then + Error(PermissionSetMissingErr, AgentAdminPermissionSetLbl); + end; + + local procedure GetExpenseManagementAdminPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") + begin + GetPermissionSet(AggregatePermissionSet, ExpenseManagementAdminPermissionSetLbl); + end; + + local procedure GetPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set"; PermissionSetId: Code[20]) var ExpenseAgentAppId: Guid; begin + AggregatePermissionSet.Reset(); Evaluate(ExpenseAgentAppId, ExpenseAgentAppIdTxt); AggregatePermissionSet.SetRange("App ID", ExpenseAgentAppId); - AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetLbl); + AggregatePermissionSet.SetRange("Role ID", PermissionSetId); if not AggregatePermissionSet.FindFirst() then - Error(ExpenseAgentPermissionSetMissingErr); + Error(PermissionSetMissingErr, PermissionSetId); end; local procedure SetExpenseAgentPermissionFilters(var AccessControl: Record "Access Control"; AadApplication: Record "AAD Application"; AggregatePermissionSet: Record "Aggregate Permission Set") @@ -138,8 +354,15 @@ codeunit 6913 "Expense Agent Entra App Mgt." var ExpenseAgentAadAppIdTxt: Label 'ee1eb5fd-719b-44f2-97d0-0efd34bc4148', Locked = true; ExpenseAgentAppIdTxt: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; + BaseApplicationAppIdTxt: Label '437dbf0e-84ff-417a-965d-ed2bb9650972', Locked = true; + AgentAdminPermissionSetLbl: Label 'Agent - Admin', Locked = true; ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; + ExpenseManagementAdminPermissionSetLbl: Label 'Expense Mgmt. Admin', Locked = true; + SecurityPermissionSetLbl: Label 'SECURITY', Locked = true; + AgentAdminPermissionRequiredErr: Label 'You must be assigned the Agent - Admin permission set to manage the Expense Agent Microsoft Entra application.'; AadApplicationMissingErr: Label 'The Expense Agent Microsoft Entra application is not configured.'; - ExpenseAgentPermissionSetMissingErr: Label 'The Expense Agent permission set is not available.'; - NotAuthorizedToManageExpenseAgentErr: Label 'You do not have permission to manage the Expense Agent.'; + ExpenseAgentPermissionRequiredErr: Label 'You must be assigned the Expense Agent permission set to manage the Expense Agent Microsoft Entra application.'; + ExpenseManagementAdminPermissionRequiredErr: Label 'You must be assigned the Expense Management - Admin permission set to manage the Expense Agent Microsoft Entra application.'; + PermissionSetMissingErr: Label 'The %1 permission set is not available.', Comment = '%1 = permission set ID'; + SecurityPermissionRequiredErr: Label 'You must be assigned either the SUPER or SECURITY permission set to manage the Expense Agent Microsoft Entra application.'; } diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al index f030c0a889d..fb51618a9d4 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al @@ -931,6 +931,7 @@ page 6991 "Expense Agent Setup Wizard" exit(false); VerifySchedulingMailboxAccess(); + SynchronizePersistedAgentState(); if AgentBeingEnabled() and StateChanged() then if not ActivateAgent() then @@ -940,7 +941,11 @@ page 6991 "Expense Agent Setup Wizard" if not DeactivateAgent() then exit(false); + if StateChanged() then + LockExpenseAgentEntraApplicationForReconciliation(); PersistAgentState(); + if StateChanged() then + ReconcileExpenseAgentEntraApplication(); ApplyScheduleChange(); exit(true); @@ -1001,6 +1006,7 @@ page 6991 "Expense Agent Setup Wizard" NoSystemUsersErr: Label 'You must first specify a user in Business Central as expense user.'; NotAuthorizedToViewSetupErr: Label 'You do not have permission to view the Expense Agent setup. Contact your administrator to be granted agent management rights.'; ApprovalWorkflowConflictErr: Label 'You must turn off "%1" in Expense Agent Setup to enable Expense Agent.', Comment = '%1 = Field Caption'; + AgentStateChangedErr: Label 'The Expense Agent state was changed by another session. Reopen the setup and try again.'; ActivatePolicyEvalQst: Label 'You are about to activate automated policy evaluation. By doing this, you acknowledge that this feature will consume additional AI credits. Continue?'; AgentUserNameLbl: Label 'Expense Agent', Locked = true; AgentDisplayNameLbl: Label 'Expense Agent', MaxLength = 80; @@ -1350,8 +1356,11 @@ page 6991 "Expense Agent Setup Wizard" exit; Rec.ClearMailboxAndDependents(); - if Rec."Enable Agent" then + if Rec."Enable Agent" then begin Rec.Validate("Enable Agent", false); + AgentSetupBuffer.Validate(State, AgentSetupBuffer.State::Disabled); + CurrPage.AgentSetupPart.Page.SetAgentSetupBuffer(AgentSetupBuffer); + end; Rec.Modify(); end; @@ -1439,11 +1448,15 @@ page 6991 "Expense Agent Setup Wizard" end; local procedure DeactivateAgent(): Boolean + var + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin + ExpenseAgentEntraApp.VerifyCanDisableAadApplicationForCurrentCompany(); if not Rec.ShowDeactivationAccessWarning() then exit(false); if not UnregisterErpConfiguration() then exit(false); + ReconcileExpenseAgentEntraApplication(); Rec.LogAgentDisabledTelemetry(); exit(true); end; @@ -1467,6 +1480,50 @@ page 6991 "Expense Agent Setup Wizard" ApplyDefaultsIfRequested(); end; + local procedure ReconcileExpenseAgentEntraApplication() + begin + OnReconcileExpenseAgentEntraApplication(AgentBeingEnabled()); + end; + + local procedure LockExpenseAgentEntraApplicationForReconciliation() + var + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + ExpenseAgentEntraApp.LockAadApplicationForReconciliation(); + end; + + local procedure SynchronizePersistedAgentState() + var + ExpenseAgentSetup: Record "Expense Agent Setup"; + PersistedState: Option; + begin + if StateChanged() then + exit; + + LockExpenseAgentEntraApplicationForReconciliation(); + ExpenseAgentSetup.ReadIsolation := IsolationLevel::UpdLock; + if not ExpenseAgentSetup.Get() then + exit; + + if ExpenseAgentSetup."Enable Agent" then + PersistedState := AgentSetupBuffer.State::Enabled + else + PersistedState := AgentSetupBuffer.State::Disabled; + if AgentSetupBuffer.State = PersistedState then + exit; + if PersistedState = AgentSetupBuffer.State::Enabled then + Error(AgentStateChangedErr); + + AgentSetupBuffer.Validate(State, PersistedState); + InitialState := AgentSetupBuffer.State; + CurrPage.AgentSetupPart.Page.SetAgentSetupBuffer(AgentSetupBuffer); + end; + + [IntegrationEvent(false, false)] + local procedure OnReconcileExpenseAgentEntraApplication(EnableAgent: Boolean) + begin + end; + local procedure ApplyScheduleChange() begin if not ScheduleAffectingChange() then diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al index ef96ab822aa..69be33c53ff 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -7,6 +7,7 @@ namespace Microsoft.Test.ExpenseAgent; using Microsoft.ExpenseAgent; using System.Environment.Configuration; using System.Security.AccessControl; +using System.Security.User; using System.TestLibraries.Security.AccessControl; codeunit 148361 "Expense Agent Config. Test" @@ -22,8 +23,10 @@ codeunit 148361 "Expense Agent Config. Test" Assert: Codeunit Assert; LibraryTestInitialize: Codeunit "Library - Test Initialize"; UserPermissionsLibrary: Codeunit "User Permissions Library"; + AgentAdminPermissionSetTok: Label 'Agent - Admin', Locked = true; ExpenseAgentAppIdTok: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseAgentPermissionSetTok: Label 'Expense Agent', Locked = true; + ExpenseManagementAdminPermissionSetTok: Label 'Expense Mgmt. Admin', Locked = true; UnrelatedPermissionSetTok: Label 'D365 BASIC', Locked = true; [Test] @@ -33,6 +36,7 @@ codeunit 148361 "Expense Agent Config. Test" ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; OtherCompanyName: Text[30]; begin + // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activating with another company permission adds the current company permission Initialize(); @@ -56,6 +60,7 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin + // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activating a disabled Entra app preserves its existing current company permission Initialize(); @@ -78,6 +83,7 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin + // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activating preserves an unrelated permission assigned to the Entra app user Initialize(); @@ -95,12 +101,13 @@ codeunit 148361 "Expense Agent Config. Test" end; [Test] - procedure ActivatingWithGlobalPermissionDoesNotAddCompanyPermission() + procedure ActivatingWithGlobalPermissionAddsCurrentCompanyPermission() var AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [SCENARIO 640454] Activating with a global Expense Agent permission does not add a company permission + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Activating with a global Expense Agent permission adds the current company permission Initialize(); // [GIVEN] Enabled Entra app "EA" has a global Expense Agent permission @@ -111,15 +118,184 @@ codeunit 148361 "Expense Agent Config. Test" // [WHEN] Activating the Expense Agent for the current company ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - // [THEN] "EA" remains enabled with only the global Expense Agent permission + // [THEN] The global grant is replaced by an explicit current-company grant VerifyAadApplicationState(AadApplication.State::Enabled); - VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, ''); + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, ''); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); + end; + + [Test] + procedure DeactivatingPreservesOtherCompanyPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + OtherCompanyName: Text[30]; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Deactivating preserves another company permission + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has Expense Agent permissions for the current company and company "B" + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + OtherCompanyName := GetOtherCompanyName(); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + AssignPermission(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] The current permission is removed while company "B" is preserved + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); + end; + + [Test] + procedure DeactivatingRemovesCurrentCompanyPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Deactivating removes the current-company Expense Agent permission + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has the Expense Agent permission only for the current company + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] The current-company Expense Agent permission is removed + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); + end; + + [Test] + procedure DeactivatingPreservesUnrelatedPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Deactivating preserves unrelated permissions + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has current Expense Agent and unrelated permissions + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + AssignPermission(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] The Expense Agent permission is removed and the unrelated permission remains + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); + VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); + end; + + [Test] + procedure DeactivatingWithoutExpensePermissionPreservesUnrelatedPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Deactivating without a current grant preserves unrelated permissions + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has only an unrelated permission + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + AssignPermission(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] The unrelated permission remains + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); + VerifyPermissionExists(AadApplication, UnrelatedPermissionSetTok, GetCurrentCompanyName()); + end; + + [Test] + procedure DeactivatingRemovesGlobalPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Deactivating removes a legacy global Expense Agent grant + Initialize(); + + // [GIVEN] Enabled Entra app "EA" has global and current-company Expense Agent permissions + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + MakeCurrentExpenseAgentPermissionGlobal(AadApplication); + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [WHEN] Deactivating the Expense Agent for the current company + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] Current and global Expense Agent grants are removed + VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, ''); VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); end; local procedure Initialize() begin LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Agent Config. Test"); + EnsureCurrentUserAgentAdminPermissionSetAssigned(); + EnsureCurrentUserPermissionSetAssigned(ExpenseManagementAdminPermissionSetTok); + EnsureCurrentUserPermissionSetAssigned(ExpenseAgentPermissionSetTok); + end; + + local procedure EnsureCurrentUserAgentAdminPermissionSetAssigned() + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + UserPermissions: Codeunit "User Permissions"; + begin + AggregatePermissionSet.SetRange("Role ID", AgentAdminPermissionSetTok); + AggregatePermissionSet.FindFirst(); + if UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), + GetCurrentCompanyName(), + AggregatePermissionSet."Role ID", + AggregatePermissionSet.Scope, + AggregatePermissionSet."App ID") + then + exit; + + AccessControl.Init(); + AccessControl."User Security ID" := UserSecurityId(); + AccessControl."Role ID" := AggregatePermissionSet."Role ID"; + AccessControl.Scope := AggregatePermissionSet.Scope; + AccessControl."App ID" := AggregatePermissionSet."App ID"; + AccessControl.Insert(true); + end; + + local procedure EnsureCurrentUserPermissionSetAssigned(PermissionSetId: Code[20]) + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + UserPermissions: Codeunit "User Permissions"; + begin + GetPermissionSet(AggregatePermissionSet, PermissionSetId); + if UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), + GetCurrentCompanyName(), + AggregatePermissionSet."Role ID", + AggregatePermissionSet.Scope, + AggregatePermissionSet."App ID") + then + exit; + + AccessControl.Init(); + AccessControl."User Security ID" := UserSecurityId(); + AccessControl."Role ID" := AggregatePermissionSet."Role ID"; + AccessControl."Company Name" := GetCurrentCompanyName(); + AccessControl.Scope := AggregatePermissionSet.Scope; + AccessControl."App ID" := AggregatePermissionSet."App ID"; + AccessControl.Insert(true); end; local procedure PrepareAadApplication(var AadApplication: Record "AAD Application"; State: Option) @@ -155,6 +331,7 @@ codeunit 148361 "Expense Agent Config. Test" var AccessControl: Record "Access Control"; begin + AadApplication.Get(AadApplication."Client Id"); AccessControl.Init(); AccessControl."User Security ID" := AadApplication."User ID"; AccessControl."Role ID" := AggregatePermissionSet."Role ID"; @@ -221,12 +398,17 @@ codeunit 148361 "Expense Agent Config. Test" end; local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") + begin + GetPermissionSet(AggregatePermissionSet, ExpenseAgentPermissionSetTok); + end; + + local procedure GetPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set"; PermissionSetId: Code[20]) var ExpenseAgentAppId: Guid; begin Evaluate(ExpenseAgentAppId, ExpenseAgentAppIdTok); AggregatePermissionSet.SetRange("App ID", ExpenseAgentAppId); - AggregatePermissionSet.SetRange("Role ID", ExpenseAgentPermissionSetTok); + AggregatePermissionSet.SetRange("Role ID", PermissionSetId); AggregatePermissionSet.FindFirst(); end; diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al index 9c4888740da..7222a69a21d 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al @@ -8,6 +8,9 @@ using Microsoft.ExpenseAgent; using Microsoft.Finance.SpendRequest; using Microsoft.HumanResources.Employee; using Microsoft.HumanResources.Setup; +using System.Environment.Configuration; +using System.Security.AccessControl; +using System.TestLibraries.Security.AccessControl; codeunit 148338 "Expense Permissions Test" { @@ -21,12 +24,19 @@ codeunit 148338 "Expense Permissions Test" LibraryLowerPermissions: Codeunit "Library - Lower Permissions"; LibraryRandom: Codeunit "Library - Random"; LibraryTestInitialize: Codeunit "Library - Test Initialize"; + UserPermissionsLibrary: Codeunit "User Permissions Library"; IsInitialized: Boolean; + AgentAdminPermissionSetTok: Label 'Agent - Admin', Locked = true; EmployeeOnlyPermissionSetTok: Label 'Exp. Emp. Only Test', Locked = true; HREditPermissionSetTok: Label 'Exp. HR Edit Test', Locked = true; AutomationPermissionSetTok: Label 'Exp. Auto Test', Locked = true; D365BasicPermissionSetTok: Label 'D365 BASIC', Locked = true; ExpenseAgentPermissionSetTok: Label 'Expense Agent', Locked = true; + ExpenseAgentAppIdTok: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; + ExpenseMgmtAdminPermissionSetTok: Label 'Expense Mgmt. Admin', Locked = true; + SecurityPermissionSetTok: Label 'SECURITY', Locked = true; + ExpenseAgentPermissionRequiredErr: Label 'You must be assigned the Expense Agent permission set to manage the Expense Agent Microsoft Entra application.'; + ExpenseMgmtAdminPermissionRequiredErr: Label 'You must be assigned the Expense Management - Admin permission set to manage the Expense Agent Microsoft Entra application.'; PermissionDeniedErr: Label 'You do not have the following permissions', Locked = true; CannotDeleteEmployeeWithExpenseErr: Label 'You cannot delete Employee %1 because they have active expense.', Comment = '%1 = Employee No.'; CannotDeleteEmployeeWithExpenseReportErr: Label 'You cannot delete Employee %1 because they have active expense report.', Comment = '%1 = Employee No.'; @@ -300,6 +310,298 @@ codeunit 148338 "Expense Permissions Test" RestoreFullPermissions(); end; + [Test] + procedure RequiredPermissionsCanActivateExpenseAgentEntraApplication() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] The required administrator permissions can activate the Expense Agent Entra application + Initialize(); + + // [GIVEN] Disabled Entra app "EA" without an Expense Agent permission + PrepareAadApplication(AadApplication, AadApplication.State::Disabled); + + // [GIVEN] User "U" has Agent Admin, Expense Management Admin, SECURITY, and Expense Agent + SetRequiredExpenseAgentManagementPermissions(); + + // [WHEN] "U" activates "EA" + ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + RestoreExpenseAgentManagementPermissions(); + + // [THEN] "EA" is enabled with one current-company Expense Agent permission + VerifyAadApplicationState(AadApplication.State::Enabled); + VerifyExpenseAgentPermissionCount(AadApplication, GetCurrentCompanyName(), 1); + end; + + [Test] + procedure RequiredPermissionsCanDeactivateExpenseAgentEntraApplication() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] The required administrator permissions can deactivate the Expense Agent Entra application + Initialize(); + + // [GIVEN] Enabled Entra app "EA" with the current-company Expense Agent permission + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + AssignExpenseAgentPermission(AadApplication, GetCurrentCompanyName()); + + // [GIVEN] User "U" has Agent Admin, Expense Management Admin, SECURITY, and Expense Agent + SetRequiredExpenseAgentManagementPermissions(); + + // [WHEN] "U" deactivates "EA" + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + RestoreExpenseAgentManagementPermissions(); + + // [THEN] The current-company Expense Agent permission is removed + VerifyExpenseAgentPermissionCount(AadApplication, GetCurrentCompanyName(), 0); + end; + + [Test] + procedure ActivationRequiresExpenseManagementAdminPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Activation requires Expense Management Admin + Initialize(); + + // [GIVEN] Disabled Entra app "EA" and user "U" without Expense Management Admin + PrepareAadApplication(AadApplication, AadApplication.State::Disabled); + SetExpenseAgentManagementPermissions(true, false, true, true); + + // [WHEN] "U" activates "EA" + asserterror ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [THEN] Activation is rejected + Assert.ExpectedError(ExpenseMgmtAdminPermissionRequiredErr); + Assert.ExpectedErrorCode('Dialog'); + RestoreExpenseAgentManagementPermissions(); + end; + + [Test] + procedure ActivationRequiresExpenseAgentPermission() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Activation requires the administrator to hold the Expense Agent permission + Initialize(); + + // [GIVEN] Disabled Entra app "EA" and user "U" without Expense Agent + PrepareAadApplication(AadApplication, AadApplication.State::Disabled); + SetExpenseAgentManagementPermissions(true, true, true, false); + + // [WHEN] "U" activates "EA" + asserterror ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); + + // [THEN] Activation is rejected + Assert.ExpectedError(ExpenseAgentPermissionRequiredErr); + Assert.ExpectedErrorCode('Dialog'); + RestoreExpenseAgentManagementPermissions(); + end; + + [Test] + procedure DeactivationRequiresExpenseAgentPermissionBeforeMutation() + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + // [FEATURE] [AI test 1.0] + // [SCENARIO 640454] Deactivation requires the administrator to hold the Expense Agent permission + Initialize(); + + // [GIVEN] Enabled Entra app "EA" with a current-company grant and user "U" without Expense Agent + PrepareAadApplication(AadApplication, AadApplication.State::Enabled); + AssignExpenseAgentPermission(AadApplication, GetCurrentCompanyName()); + SetExpenseAgentManagementPermissions(true, true, true, false); + + // [WHEN] "U" deactivates "EA" + asserterror ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); + + // [THEN] Deactivation is rejected + Assert.ExpectedError(ExpenseAgentPermissionRequiredErr); + Assert.ExpectedErrorCode('Dialog'); + RestoreExpenseAgentManagementPermissions(); + end; + + local procedure Initialize() + begin + LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Permissions Test"); + RestoreFullPermissions(); + LibraryExpense.CleanTransactionalData(); + LibraryExpense.CleanUpBeforeTesting(); + if IsInitialized then + exit; + + LibraryTestInitialize.OnBeforeTestSuiteInitialize(Codeunit::"Expense Permissions Test"); + EnsureSetupRecordsExist(); + LibraryExpense.SetupNumberSeriesInExpenseMgmt(); + LibraryExpense.UpdateEnableApprovalWorkflowInAgentSetup(false); + IsInitialized := true; + Commit(); + LibraryTestInitialize.OnAfterTestSuiteInitialize(Codeunit::"Expense Permissions Test"); + end; + + local procedure SetRequiredExpenseAgentManagementPermissions() + begin + SetExpenseAgentManagementPermissions(true, true, true, true); + end; + + local procedure SetExpenseAgentManagementPermissions(IncludeAgentAdmin: Boolean; IncludeExpenseMgmtAdmin: Boolean; IncludeSecurity: Boolean; IncludeExpenseAgent: Boolean) + begin + PrepareCurrentUserPermissionAssignments(); + + if IncludeAgentAdmin then + AssignCurrentUserPermissionSet(AgentAdminPermissionSetTok); + if IncludeExpenseMgmtAdmin then + AssignCurrentUserExpensePermissionSet(ExpenseMgmtAdminPermissionSetTok); + if IncludeSecurity then + UserPermissionsLibrary.AssignPermissionSetToUser(UserSecurityId(), SecurityPermissionSetTok); + if IncludeExpenseAgent then + AssignCurrentUserExpensePermissionSet(ExpenseAgentPermissionSetTok); + + LibraryLowerPermissions.SetExactPermissionSet(D365BasicPermissionSetTok); + if IncludeAgentAdmin then + LibraryLowerPermissions.AddPermissionSet(AgentAdminPermissionSetTok); + if IncludeExpenseMgmtAdmin then + LibraryLowerPermissions.AddPermissionSet(ExpenseMgmtAdminPermissionSetTok); + if IncludeSecurity then + LibraryLowerPermissions.AddSecurity(); + if IncludeExpenseAgent then + LibraryLowerPermissions.AddPermissionSet(ExpenseAgentPermissionSetTok); + end; + + local procedure PrepareCurrentUserPermissionAssignments() + begin + RemoveCurrentUserPermissionSet(AgentAdminPermissionSetTok); + RemoveCurrentUserPermissionSet(ExpenseMgmtAdminPermissionSetTok); + RemoveCurrentUserPermissionSet(SecurityPermissionSetTok); + RemoveCurrentUserPermissionSet(ExpenseAgentPermissionSetTok); + end; + + local procedure RestoreExpenseAgentManagementPermissions() + begin + RestoreFullPermissions(); + end; + + local procedure RemoveCurrentUserPermissionSet(PermissionSetId: Code[20]) + var + AccessControl: Record "Access Control"; + begin + AccessControl.SetRange("User Security ID", UserSecurityId()); + AccessControl.SetRange("Role ID", PermissionSetId); + AccessControl.DeleteAll(true); + end; + + local procedure PrepareAadApplication(var AadApplication: Record "AAD Application"; State: Option) + var + AccessControl: Record "Access Control"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); + if AadApplication.State <> State then begin + AadApplication.Validate(State, State); + AadApplication.Modify(true); + end; + + AccessControl.SetRange("User Security ID", AadApplication."User ID"); + AccessControl.DeleteAll(true); + end; + + local procedure AssignExpenseAgentPermission(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]) + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + begin + GetExpenseAgentPermissionSet(AggregatePermissionSet); + AccessControl.Init(); + AccessControl."User Security ID" := AadApplication."User ID"; + AccessControl."Role ID" := AggregatePermissionSet."Role ID"; + AccessControl."Company Name" := CompanyNameValue; + AccessControl.Scope := AggregatePermissionSet.Scope; + AccessControl."App ID" := AggregatePermissionSet."App ID"; + AccessControl.Insert(true); + end; + + local procedure AssignCurrentUserExpensePermissionSet(PermissionSetId: Code[20]) + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + begin + GetExpensePermissionSet(AggregatePermissionSet, PermissionSetId); + AccessControl.Init(); + AccessControl."User Security ID" := UserSecurityId(); + AccessControl."Role ID" := AggregatePermissionSet."Role ID"; + AccessControl."Company Name" := GetCurrentCompanyName(); + AccessControl.Scope := AggregatePermissionSet.Scope; + AccessControl."App ID" := AggregatePermissionSet."App ID"; + AccessControl.Insert(true); + end; + + local procedure AssignCurrentUserPermissionSet(PermissionSetId: Code[20]) + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + begin + AggregatePermissionSet.SetRange("Role ID", PermissionSetId); + AggregatePermissionSet.FindFirst(); + AccessControl.Init(); + AccessControl."User Security ID" := UserSecurityId(); + AccessControl."Role ID" := AggregatePermissionSet."Role ID"; + AccessControl.Scope := AggregatePermissionSet.Scope; + AccessControl."App ID" := AggregatePermissionSet."App ID"; + AccessControl.Insert(true); + end; + + local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") + begin + GetExpensePermissionSet(AggregatePermissionSet, ExpenseAgentPermissionSetTok); + end; + + local procedure GetExpensePermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set"; PermissionSetId: Code[20]) + var + ExpenseAgentAppId: Guid; + begin + Evaluate(ExpenseAgentAppId, ExpenseAgentAppIdTok); + AggregatePermissionSet.SetRange("App ID", ExpenseAgentAppId); + AggregatePermissionSet.SetRange("Role ID", PermissionSetId); + AggregatePermissionSet.FindFirst(); + end; + + local procedure GetCurrentCompanyName(): Text[30] + begin + exit(CopyStr(CompanyName(), 1, 30)); + end; + + local procedure VerifyAadApplicationState(ExpectedState: Option) + var + AadApplication: Record "AAD Application"; + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + AadApplication.Get(ExpenseAgentEntraApp.GetAadAppId()); + Assert.AreEqual(ExpectedState, AadApplication.State, 'The Entra application state is incorrect.'); + end; + + local procedure VerifyExpenseAgentPermissionCount(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]; ExpectedCount: Integer) + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + begin + GetExpenseAgentPermissionSet(AggregatePermissionSet); + AccessControl.SetRange("User Security ID", AadApplication."User ID"); + AccessControl.SetRange("Role ID", AggregatePermissionSet."Role ID"); + AccessControl.SetRange("Company Name", CompanyNameValue); + AccessControl.SetRange(Scope, AggregatePermissionSet.Scope); + AccessControl.SetRange("App ID", AggregatePermissionSet."App ID"); + Assert.AreEqual(ExpectedCount, AccessControl.Count(), 'The number of matching Expense Agent permissions is incorrect.'); + end; + local procedure VerifyExpenseMgmtPermissions(PermissionSetId: Code[20]; CanEdit: Boolean) var SpendRequest: Record "Spend Request"; @@ -431,24 +733,6 @@ codeunit 148338 "Expense Permissions Test" PostedExpenseReportHeader.Insert(false); end; - local procedure Initialize() - begin - LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Permissions Test"); - RestoreFullPermissions(); - LibraryExpense.CleanTransactionalData(); - LibraryExpense.CleanUpBeforeTesting(); - if IsInitialized then - exit; - - LibraryTestInitialize.OnBeforeTestSuiteInitialize(Codeunit::"Expense Permissions Test"); - EnsureSetupRecordsExist(); - LibraryExpense.SetupNumberSeriesInExpenseMgmt(); - LibraryExpense.UpdateEnableApprovalWorkflowInAgentSetup(false); - IsInitialized := true; - Commit(); - LibraryTestInitialize.OnAfterTestSuiteInitialize(Codeunit::"Expense Permissions Test"); - end; - local procedure VerifyPermissionSetCanInsertActivity(PermissionSetId: Code[20]) var ExpenseUser: Record "Expense User"; From 2e4ae4367787f549634ca788a2bf3ddedf6dbb93 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 16:09:04 +0200 Subject: [PATCH 13/24] Reorder Expense Agent manager procedures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 102 +++++++++--------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 129ba3d30bd..2accd04723d 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -27,6 +27,57 @@ codeunit 6913 "Expense Agent Entra App Mgt." EnableAadApplicationForCurrentCompanyWithoutAuthorization(); end; + internal procedure DisableAadApplicationForCurrentCompany() + begin + VerifyCanDisableAadApplicationForCurrentCompany(); + DisableAadApplicationForCompany(GetCurrentCompanyName()); + end; + + internal procedure IsCurrentUserExpenseAgent(): Boolean + var + AadApplication: Record "AAD Application"; + EnvironmentInfo: Codeunit "Environment Information"; + begin + if not EnvironmentInfo.IsSaaSInfrastructure() then + exit(true); + + if not AadApplication.Get(GetAadAppId()) then + exit(false); + + exit(AadApplication."User ID" = UserSecurityId()); + end; + + internal procedure GetEnabledExpenseAgentUserId(): Guid + var + AadApplication: Record "AAD Application"; + begin + AadApplication.Get(GetAadAppId()); + AadApplication.TestField(State, AadApplication.State::Enabled); + + exit(AadApplication."User ID"); + end; + + internal procedure GetAadAppId(): Text + begin + exit(ExpenseAgentAadAppIdTxt); + end; + + internal procedure VerifyCanDisableAadApplicationForCurrentCompany() + var + AadApplication: Record "AAD Application"; + begin + VerifyCurrentUserCanManageExpenseAgent(); + GetAadApplication(AadApplication); + end; + + internal procedure LockAadApplicationForReconciliation() + var + AadApplication: Record "AAD Application"; + begin + AadApplication.LockTable(); + GetAadApplication(AadApplication); + end; + local procedure EnableAadApplicationForCurrentCompanyWithoutAuthorization() var AadApplication: Record "AAD Application"; @@ -62,12 +113,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." DisableAadApplicationForCompany(GetCurrentCompanyName()); end; - internal procedure DisableAadApplicationForCurrentCompany() - begin - VerifyCanDisableAadApplicationForCurrentCompany(); - DisableAadApplicationForCompany(GetCurrentCompanyName()); - end; - local procedure DisableAadApplicationForCompany(CompanyNameValue: Text[30]) var AadApplication: Record "AAD Application"; @@ -101,51 +146,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication.Modify(true); end; - internal procedure IsCurrentUserExpenseAgent(): Boolean - var - AadApplication: Record "AAD Application"; - EnvironmentInfo: Codeunit "Environment Information"; - begin - if not EnvironmentInfo.IsSaaSInfrastructure() then - exit(true); - - if not AadApplication.Get(GetAadAppId()) then - exit(false); - - exit(AadApplication."User ID" = UserSecurityId()); - end; - - internal procedure GetEnabledExpenseAgentUserId(): Guid - var - AadApplication: Record "AAD Application"; - begin - AadApplication.Get(GetAadAppId()); - AadApplication.TestField(State, AadApplication.State::Enabled); - - exit(AadApplication."User ID"); - end; - - internal procedure GetAadAppId(): Text - begin - exit(ExpenseAgentAadAppIdTxt); - end; - - internal procedure VerifyCanDisableAadApplicationForCurrentCompany() - var - AadApplication: Record "AAD Application"; - begin - VerifyCurrentUserCanManageExpenseAgent(); - GetAadApplication(AadApplication); - end; - - internal procedure LockAadApplicationForReconciliation() - var - AadApplication: Record "AAD Application"; - begin - AadApplication.LockTable(); - GetAadApplication(AadApplication); - end; - local procedure VerifyCurrentUserCanManageExpenseAgent() var AggregatePermissionSet: Record "Aggregate Permission Set"; From 569b361a600c75f30201ed3cb2155bf819bae3d2 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 16:21:26 +0200 Subject: [PATCH 14/24] Simplify Expense Agent lifecycle flow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 19 ------- .../Pages/ExpenseAgentSetupWizard.Page.al | 57 +------------------ 2 files changed, 2 insertions(+), 74 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 2accd04723d..2ba53c3b950 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -70,19 +70,10 @@ codeunit 6913 "Expense Agent Entra App Mgt." GetAadApplication(AadApplication); end; - internal procedure LockAadApplicationForReconciliation() - var - AadApplication: Record "AAD Application"; - begin - AadApplication.LockTable(); - GetAadApplication(AadApplication); - end; - local procedure EnableAadApplicationForCurrentCompanyWithoutAuthorization() var AadApplication: Record "AAD Application"; begin - AadApplication.LockTable(); GetAadApplication(AadApplication); // Enabling creates the application user. Disable it again before changing permissions. @@ -104,21 +95,11 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication.Modify(true); end; - [EventSubscriber(ObjectType::Page, Page::"Expense Agent Setup Wizard", OnReconcileExpenseAgentEntraApplication, '', false, false)] - local procedure OnReconcileExpenseAgentEntraApplication(EnableAgent: Boolean) - begin - if EnableAgent then - EnableAadApplicationForCurrentCompanyWithoutAuthorization() - else - DisableAadApplicationForCompany(GetCurrentCompanyName()); - end; - local procedure DisableAadApplicationForCompany(CompanyNameValue: Text[30]) var AadApplication: Record "AAD Application"; HasOtherEnabledCompany: Boolean; begin - AadApplication.LockTable(); if not AadApplication.Get(GetAadAppId()) then exit; diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al index fb51618a9d4..00fba63a99a 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Pages/ExpenseAgentSetupWizard.Page.al @@ -931,7 +931,6 @@ page 6991 "Expense Agent Setup Wizard" exit(false); VerifySchedulingMailboxAccess(); - SynchronizePersistedAgentState(); if AgentBeingEnabled() and StateChanged() then if not ActivateAgent() then @@ -941,11 +940,7 @@ page 6991 "Expense Agent Setup Wizard" if not DeactivateAgent() then exit(false); - if StateChanged() then - LockExpenseAgentEntraApplicationForReconciliation(); PersistAgentState(); - if StateChanged() then - ReconcileExpenseAgentEntraApplication(); ApplyScheduleChange(); exit(true); @@ -1006,7 +1001,6 @@ page 6991 "Expense Agent Setup Wizard" NoSystemUsersErr: Label 'You must first specify a user in Business Central as expense user.'; NotAuthorizedToViewSetupErr: Label 'You do not have permission to view the Expense Agent setup. Contact your administrator to be granted agent management rights.'; ApprovalWorkflowConflictErr: Label 'You must turn off "%1" in Expense Agent Setup to enable Expense Agent.', Comment = '%1 = Field Caption'; - AgentStateChangedErr: Label 'The Expense Agent state was changed by another session. Reopen the setup and try again.'; ActivatePolicyEvalQst: Label 'You are about to activate automated policy evaluation. By doing this, you acknowledge that this feature will consume additional AI credits. Continue?'; AgentUserNameLbl: Label 'Expense Agent', Locked = true; AgentDisplayNameLbl: Label 'Expense Agent', MaxLength = 80; @@ -1356,11 +1350,8 @@ page 6991 "Expense Agent Setup Wizard" exit; Rec.ClearMailboxAndDependents(); - if Rec."Enable Agent" then begin + if Rec."Enable Agent" then Rec.Validate("Enable Agent", false); - AgentSetupBuffer.Validate(State, AgentSetupBuffer.State::Disabled); - CurrPage.AgentSetupPart.Page.SetAgentSetupBuffer(AgentSetupBuffer); - end; Rec.Modify(); end; @@ -1456,7 +1447,7 @@ page 6991 "Expense Agent Setup Wizard" exit(false); if not UnregisterErpConfiguration() then exit(false); - ReconcileExpenseAgentEntraApplication(); + ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); Rec.LogAgentDisabledTelemetry(); exit(true); end; @@ -1480,50 +1471,6 @@ page 6991 "Expense Agent Setup Wizard" ApplyDefaultsIfRequested(); end; - local procedure ReconcileExpenseAgentEntraApplication() - begin - OnReconcileExpenseAgentEntraApplication(AgentBeingEnabled()); - end; - - local procedure LockExpenseAgentEntraApplicationForReconciliation() - var - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - ExpenseAgentEntraApp.LockAadApplicationForReconciliation(); - end; - - local procedure SynchronizePersistedAgentState() - var - ExpenseAgentSetup: Record "Expense Agent Setup"; - PersistedState: Option; - begin - if StateChanged() then - exit; - - LockExpenseAgentEntraApplicationForReconciliation(); - ExpenseAgentSetup.ReadIsolation := IsolationLevel::UpdLock; - if not ExpenseAgentSetup.Get() then - exit; - - if ExpenseAgentSetup."Enable Agent" then - PersistedState := AgentSetupBuffer.State::Enabled - else - PersistedState := AgentSetupBuffer.State::Disabled; - if AgentSetupBuffer.State = PersistedState then - exit; - if PersistedState = AgentSetupBuffer.State::Enabled then - Error(AgentStateChangedErr); - - AgentSetupBuffer.Validate(State, PersistedState); - InitialState := AgentSetupBuffer.State; - CurrPage.AgentSetupPart.Page.SetAgentSetupBuffer(AgentSetupBuffer); - end; - - [IntegrationEvent(false, false)] - local procedure OnReconcileExpenseAgentEntraApplication(EnableAgent: Boolean) - begin - end; - local procedure ApplyScheduleChange() begin if not ScheduleAffectingChange() then From d868c8229293f792e516a7c3e1f919dcf39b3c8b Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 17:29:00 +0200 Subject: [PATCH 15/24] Simplify company permission handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 81 +++++-------------- .../src/ExpenseAgentConfigTest.Codeunit.al | 16 ++-- 2 files changed, 27 insertions(+), 70 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 2ba53c3b950..b5b9ad71c17 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -17,9 +17,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." InherentPermissions = X; Permissions = tabledata "AAD Application" = rm, - tabledata "Access Control" = rimd, - tabledata Company = r, - tabledata "Expense Agent Setup" = r; + tabledata "Access Control" = rimd; internal procedure EnableAadApplicationForCurrentCompany() begin @@ -82,13 +80,12 @@ codeunit 6913 "Expense Agent Entra App Mgt." AadApplication.Modify(true); end; - if HasPermissionForCurrentCompany(AadApplication) and not HasGlobalPermission(AadApplication) then + if HasPermissionForCurrentCompany(AadApplication) then exit; AadApplication.Validate(State, AadApplication.State::Disabled); AadApplication.Modify(true); - NormalizeGlobalPermission(AadApplication, true, GetCurrentCompanyName()); AddPermissionForCompany(AadApplication, GetCurrentCompanyName()); AadApplication.Validate(State, AadApplication.State::Enabled); @@ -98,25 +95,18 @@ codeunit 6913 "Expense Agent Entra App Mgt." local procedure DisableAadApplicationForCompany(CompanyNameValue: Text[30]) var AadApplication: Record "AAD Application"; - HasOtherEnabledCompany: Boolean; + OtherCompanyPermissionExists: Boolean; begin if not AadApplication.Get(GetAadAppId()) then exit; - HasOtherEnabledCompany := HasEnabledExpenseAgentForOtherCompany(CompanyNameValue); - if HasGlobalPermission(AadApplication) then begin - if AadApplication.State <> AadApplication.State::Disabled then begin - AadApplication.Validate(State, AadApplication.State::Disabled); - AadApplication.Modify(true); - end; - NormalizeGlobalPermission(AadApplication, false, CompanyNameValue); - end; - RemovePermissionForCompany(AadApplication, CompanyNameValue); - if HasOtherEnabledCompany then begin - GetAadApplication(AadApplication); + OtherCompanyPermissionExists := HasOtherCompanyPermission(AadApplication, CompanyNameValue); + RemovePermissionForCompany(AadApplication, CompanyNameValue, not OtherCompanyPermissionExists); + if OtherCompanyPermissionExists then begin + AadApplication.Get(GetAadAppId()); if AadApplication.State <> AadApplication.State::Enabled then begin - AadApplication.Validate(State, AadApplication.State::Enabled); - AadApplication.Modify(true); + AadApplication.State := AadApplication.State::Enabled; + AadApplication.Modify(false); end; exit; end; @@ -191,11 +181,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(HasPermissionForCompany(AadApplication, GetCurrentCompanyName())); end; - local procedure HasGlobalPermission(AadApplication: Record "AAD Application"): Boolean - begin - exit(HasPermissionForCompany(AadApplication, '')); - end; - local procedure HasPermissionForCompany(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]): Boolean var AccessControl: Record "Access Control"; @@ -208,44 +193,16 @@ codeunit 6913 "Expense Agent Entra App Mgt." exit(not AccessControl.IsEmpty()); end; - local procedure HasEnabledExpenseAgentForOtherCompany(CompanyNameValue: Text[30]): Boolean - var - Company: Record Company; - ExpenseAgentSetup: Record "Expense Agent Setup"; - begin - Company.SecurityFiltering(SecurityFilter::Ignored); - Company.SetFilter(Name, '<>%1', CompanyNameValue); - if Company.FindSet() then - repeat - ExpenseAgentSetup.ChangeCompany(Company.Name); - if ExpenseAgentSetup.Get() and ExpenseAgentSetup."Enable Agent" then - exit(true); - until Company.Next() = 0; - - exit(false); - end; - - local procedure NormalizeGlobalPermission(AadApplication: Record "AAD Application"; IncludeCurrentCompany: Boolean; CurrentCompanyNameValue: Text[30]) + local procedure HasOtherCompanyPermission(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]): Boolean var - Company: Record Company; - ExpenseAgentSetup: Record "Expense Agent Setup"; + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; begin - if not HasGlobalPermission(AadApplication) then - exit; - - if IncludeCurrentCompany then - AddPermissionForCompany(AadApplication, CurrentCompanyNameValue); - - Company.SecurityFiltering(SecurityFilter::Ignored); - Company.SetFilter(Name, '<>%1', CurrentCompanyNameValue); - if Company.FindSet() then - repeat - ExpenseAgentSetup.ChangeCompany(Company.Name); - if ExpenseAgentSetup.Get() and ExpenseAgentSetup."Enable Agent" then - AddPermissionForCompany(AadApplication, Company.Name); - until Company.Next() = 0; - - RemovePermissionForCompany(AadApplication, ''); + GetExpenseAgentPermissionSet(AggregatePermissionSet); + SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); + AccessControl.SecurityFiltering(SecurityFilter::Ignored); + AccessControl.SetFilter("Company Name", '<>''''&<>%1', CompanyNameValue); + exit(not AccessControl.IsEmpty()); end; local procedure AddPermissionForCompany(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]) @@ -267,7 +224,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl.Insert(true); end; - local procedure RemovePermissionForCompany(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]) + local procedure RemovePermissionForCompany(AadApplication: Record "AAD Application"; CompanyNameValue: Text[30]; RunTrigger: Boolean) var AccessControl: Record "Access Control"; AggregatePermissionSet: Record "Aggregate Permission Set"; @@ -276,7 +233,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." SetExpenseAgentPermissionFilters(AccessControl, AadApplication, AggregatePermissionSet); AccessControl.SetRange("Company Name", CompanyNameValue); - AccessControl.DeleteAll(true); + AccessControl.DeleteAll(RunTrigger); end; local procedure GetAadApplication(var AadApplication: Record "AAD Application") diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al index 69be33c53ff..055b920191a 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -118,9 +118,9 @@ codeunit 148361 "Expense Agent Config. Test" // [WHEN] Activating the Expense Agent for the current company ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - // [THEN] The global grant is replaced by an explicit current-company grant + // [THEN] The global grant remains and an explicit current-company grant is added VerifyAadApplicationState(AadApplication.State::Enabled); - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, ''); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, ''); VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); end; @@ -145,7 +145,7 @@ codeunit 148361 "Expense Agent Config. Test" // [WHEN] Deactivating the Expense Agent for the current company ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - // [THEN] The current permission is removed while company "B" is preserved + // [THEN] The current permission is removed and company "B" is preserved VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, OtherCompanyName); end; @@ -217,13 +217,13 @@ codeunit 148361 "Expense Agent Config. Test" end; [Test] - procedure DeactivatingRemovesGlobalPermission() + procedure DeactivatingWithGlobalPermissionDisablesApplication() var AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin // [FEATURE] [AI test 1.0] - // [SCENARIO 640454] Deactivating removes a legacy global Expense Agent grant + // [SCENARIO 640454] A global grant does not keep the Entra app enabled after the last company is deactivated Initialize(); // [GIVEN] Enabled Entra app "EA" has global and current-company Expense Agent permissions @@ -235,8 +235,9 @@ codeunit 148361 "Expense Agent Config. Test" // [WHEN] Deactivating the Expense Agent for the current company ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - // [THEN] Current and global Expense Agent grants are removed - VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, ''); + // [THEN] "EA" is disabled, the current grant is removed, and the global grant remains + VerifyAadApplicationState(AadApplication.State::Disabled); + VerifyPermissionExists(AadApplication, ExpenseAgentPermissionSetTok, ''); VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); end; @@ -320,7 +321,6 @@ codeunit 148361 "Expense Agent Config. Test" if PermissionSetId = ExpenseAgentPermissionSetTok then begin GetExpenseAgentPermissionSet(AggregatePermissionSet); AssignPermission(AadApplication, AggregatePermissionSet, CompanyNameValue); - SelectLatestVersion(); exit; end; From 4e7932b9a40794693101517fc040b056ab80bdce Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 17:48:31 +0200 Subject: [PATCH 16/24] Polish Expense Agent lifecycle tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al | 8 +------- .../test/src/ExpenseAgentConfigTest.Codeunit.al | 9 --------- .../test/src/ExpensePermissionsTest.Codeunit.al | 5 ----- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index b5b9ad71c17..e70d81e445e 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -102,14 +102,8 @@ codeunit 6913 "Expense Agent Entra App Mgt." OtherCompanyPermissionExists := HasOtherCompanyPermission(AadApplication, CompanyNameValue); RemovePermissionForCompany(AadApplication, CompanyNameValue, not OtherCompanyPermissionExists); - if OtherCompanyPermissionExists then begin - AadApplication.Get(GetAadAppId()); - if AadApplication.State <> AadApplication.State::Enabled then begin - AadApplication.State := AadApplication.State::Enabled; - AadApplication.Modify(false); - end; + if OtherCompanyPermissionExists then exit; - end; if AadApplication.State = AadApplication.State::Disabled then exit; diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al index 055b920191a..fb0f32ede1d 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -36,7 +36,6 @@ codeunit 148361 "Expense Agent Config. Test" ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; OtherCompanyName: Text[30]; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activating with another company permission adds the current company permission Initialize(); @@ -60,7 +59,6 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activating a disabled Entra app preserves its existing current company permission Initialize(); @@ -83,7 +81,6 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activating preserves an unrelated permission assigned to the Entra app user Initialize(); @@ -106,7 +103,6 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activating with a global Expense Agent permission adds the current company permission Initialize(); @@ -131,7 +127,6 @@ codeunit 148361 "Expense Agent Config. Test" ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; OtherCompanyName: Text[30]; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Deactivating preserves another company permission Initialize(); @@ -156,7 +151,6 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Deactivating removes the current-company Expense Agent permission Initialize(); @@ -177,7 +171,6 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Deactivating preserves unrelated permissions Initialize(); @@ -200,7 +193,6 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Deactivating without a current grant preserves unrelated permissions Initialize(); @@ -222,7 +214,6 @@ codeunit 148361 "Expense Agent Config. Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] A global grant does not keep the Entra app enabled after the last company is deactivated Initialize(); diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al index 7222a69a21d..99f5286e613 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al @@ -316,7 +316,6 @@ codeunit 148338 "Expense Permissions Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] The required administrator permissions can activate the Expense Agent Entra application Initialize(); @@ -341,7 +340,6 @@ codeunit 148338 "Expense Permissions Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] The required administrator permissions can deactivate the Expense Agent Entra application Initialize(); @@ -366,7 +364,6 @@ codeunit 148338 "Expense Permissions Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activation requires Expense Management Admin Initialize(); @@ -389,7 +386,6 @@ codeunit 148338 "Expense Permissions Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Activation requires the administrator to hold the Expense Agent permission Initialize(); @@ -412,7 +408,6 @@ codeunit 148338 "Expense Permissions Test" AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [FEATURE] [AI test 1.0] // [SCENARIO 640454] Deactivation requires the administrator to hold the Expense Agent permission Initialize(); From f0ab337ca19155129bd82f70b30a57f8cb5d0dd9 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 18:27:58 +0200 Subject: [PATCH 17/24] Preserve Expense Agent API compatibility Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../app/src/Common/ExpenseAgentAPIValidation.Codeunit.al | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al index 837981d9518..1a0c27a6e1b 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al @@ -43,6 +43,14 @@ codeunit 6993 "Expense Agent API Validation" Error(CapabilityNotEnabledErr, Enum::"Copilot Capability"::"Expense Agent"); end; + [Obsolete('Use codeunit "Expense Agent Entra App Mgt.".GetAadAppId() instead.', '30.0')] + procedure GetAadAppId(): Text + var + ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; + begin + exit(ExpenseAgentEntraApp.GetAadAppId()); + end; + procedure IsCurrentUserExpenseAgent(): Boolean var ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; From 79d0ef84e081aaf603dfaf1b23022b8bb2586d28 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 19:03:40 +0200 Subject: [PATCH 18/24] Refine Expense Agent deactivation preflight Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index e70d81e445e..025a5d204a2 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -61,11 +61,8 @@ codeunit 6913 "Expense Agent Entra App Mgt." end; internal procedure VerifyCanDisableAadApplicationForCurrentCompany() - var - AadApplication: Record "AAD Application"; begin VerifyCurrentUserCanManageExpenseAgent(); - GetAadApplication(AadApplication); end; local procedure EnableAadApplicationForCurrentCompanyWithoutAuthorization() @@ -255,7 +252,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AggregatePermissionSet.SetRange("App ID", BaseApplicationAppId); AggregatePermissionSet.SetRange("Role ID", AgentAdminPermissionSetLbl); if not AggregatePermissionSet.FindFirst() then - Error(PermissionSetMissingErr, AgentAdminPermissionSetLbl); + ErrorPermissionSetMissing(AgentAdminPermissionSetLbl); end; local procedure GetExpenseManagementAdminPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") @@ -272,7 +269,17 @@ codeunit 6913 "Expense Agent Entra App Mgt." AggregatePermissionSet.SetRange("App ID", ExpenseAgentAppId); AggregatePermissionSet.SetRange("Role ID", PermissionSetId); if not AggregatePermissionSet.FindFirst() then - Error(PermissionSetMissingErr, PermissionSetId); + ErrorPermissionSetMissing(PermissionSetId); + end; + + local procedure ErrorPermissionSetMissing(PermissionSetId: Code[20]) + var + MissingPermissionSetErrorInfo: ErrorInfo; + begin + MissingPermissionSetErrorInfo.ErrorType := ErrorType::Internal; + MissingPermissionSetErrorInfo.DataClassification := DataClassification::SystemMetadata; + MissingPermissionSetErrorInfo.Message := StrSubstNo(PermissionSetMissingErr, PermissionSetId); + Error(MissingPermissionSetErrorInfo); end; local procedure SetExpenseAgentPermissionFilters(var AccessControl: Record "Access Control"; AadApplication: Record "AAD Application"; AggregatePermissionSet: Record "Aggregate Permission Set") From cbefdd59b02661a66cdee5e696f5c3aed3a3fc68 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 19:22:10 +0200 Subject: [PATCH 19/24] Verify Expense Agent final deactivation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al index fb0f32ede1d..f3ad784250f 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -161,7 +161,8 @@ codeunit 148361 "Expense Agent Config. Test" // [WHEN] Deactivating the Expense Agent for the current company ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - // [THEN] The current-company Expense Agent permission is removed + // [THEN] The current-company Expense Agent permission is removed and "EA" is disabled + VerifyAadApplicationState(AadApplication.State::Disabled); VerifyPermissionDoesNotExist(AadApplication, ExpenseAgentPermissionSetTok, GetCurrentCompanyName()); end; From 79d13771eafbe3f9cb5ce340c248c744aa6c0f6b Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 20:12:25 +0200 Subject: [PATCH 20/24] Align Expense Agent obsolete tag Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../app/src/Common/ExpenseAgentAPIValidation.Codeunit.al | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al index 1a0c27a6e1b..aceff55bd6a 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al @@ -43,7 +43,7 @@ codeunit 6993 "Expense Agent API Validation" Error(CapabilityNotEnabledErr, Enum::"Copilot Capability"::"Expense Agent"); end; - [Obsolete('Use codeunit "Expense Agent Entra App Mgt.".GetAadAppId() instead.', '30.0')] + [Obsolete('Use codeunit "Expense Agent Entra App Mgt.".GetAadAppId() instead.', '99.9')] procedure GetAadAppId(): Text var ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; From 63f161ae892613e874e3ce16c019a8ff6efcb3e9 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Mon, 14 Sep 2026 20:29:12 +0200 Subject: [PATCH 21/24] Keep Expense Agent compatibility delegate Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../app/src/Common/ExpenseAgentAPIValidation.Codeunit.al | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al index aceff55bd6a..6f7f6ba9027 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Common/ExpenseAgentAPIValidation.Codeunit.al @@ -43,7 +43,6 @@ codeunit 6993 "Expense Agent API Validation" Error(CapabilityNotEnabledErr, Enum::"Copilot Capability"::"Expense Agent"); end; - [Obsolete('Use codeunit "Expense Agent Entra App Mgt.".GetAadAppId() instead.', '99.9')] procedure GetAadAppId(): Text var ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; From 7343b0485191da431f24363c6cee00cba235e42b Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Tue, 15 Sep 2026 15:17:32 +0200 Subject: [PATCH 22/24] Allow SUPER to manage Expense Agent Entra app Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../ExpenseAgentEntraAppMgt.Codeunit.al | 18 ++- .../src/ExpensePermissionsTest.Codeunit.al | 131 +++++++----------- 2 files changed, 57 insertions(+), 92 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al index 025a5d204a2..1d64396fe2d 100644 --- a/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/app/src/Setup/Codeunits/ExpenseAgentEntraAppMgt.Codeunit.al @@ -114,6 +114,9 @@ codeunit 6913 "Expense Agent Entra App Mgt." AgentSystemPermissions: Codeunit "Agent System Permissions"; UserPermissions: Codeunit "User Permissions"; begin + if UserPermissions.IsSuper(UserSecurityId()) then + exit; + GetAgentAdminPermissionSet(AggregatePermissionSet); if not UserPermissions.HasUserPermissionSetAssigned( UserSecurityId(), @@ -122,10 +125,10 @@ codeunit 6913 "Expense Agent Entra App Mgt." AggregatePermissionSet.Scope, AggregatePermissionSet."App ID") then - Error(AgentAdminPermissionRequiredErr); + Error(PermissionSetRequiredErr, AggregatePermissionSet.Name); if not AgentSystemPermissions.CurrentUserHasCanManageAllAgentsPermission() then - Error(AgentAdminPermissionRequiredErr); + Error(PermissionSetRequiredErr, AggregatePermissionSet.Name); GetExpenseManagementAdminPermissionSet(AggregatePermissionSet); if not UserPermissions.HasUserPermissionSetAssigned( @@ -135,7 +138,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AggregatePermissionSet.Scope, AggregatePermissionSet."App ID") then - Error(ExpenseManagementAdminPermissionRequiredErr); + Error(PermissionSetRequiredErr, AggregatePermissionSet.Name); if not HasSecurityPermission(UserPermissions) then Error(SecurityPermissionRequiredErr); @@ -148,7 +151,7 @@ codeunit 6913 "Expense Agent Entra App Mgt." AggregatePermissionSet.Scope, AggregatePermissionSet."App ID") then - Error(ExpenseAgentPermissionRequiredErr); + Error(PermissionSetRequiredErr, AggregatePermissionSet.Name); end; local procedure HasSecurityPermission(UserPermissions: Codeunit "User Permissions"): Boolean @@ -156,9 +159,6 @@ codeunit 6913 "Expense Agent Entra App Mgt." AccessControl: Record "Access Control"; NullGuid: Guid; begin - if UserPermissions.IsSuper(UserSecurityId()) then - exit(true); - exit(UserPermissions.HasUserPermissionSetAssigned( UserSecurityId(), GetCurrentCompanyName(), @@ -298,10 +298,8 @@ codeunit 6913 "Expense Agent Entra App Mgt." ExpenseAgentPermissionSetLbl: Label 'Expense Agent', Locked = true; ExpenseManagementAdminPermissionSetLbl: Label 'Expense Mgmt. Admin', Locked = true; SecurityPermissionSetLbl: Label 'SECURITY', Locked = true; - AgentAdminPermissionRequiredErr: Label 'You must be assigned the Agent - Admin permission set to manage the Expense Agent Microsoft Entra application.'; AadApplicationMissingErr: Label 'The Expense Agent Microsoft Entra application is not configured.'; - ExpenseAgentPermissionRequiredErr: Label 'You must be assigned the Expense Agent permission set to manage the Expense Agent Microsoft Entra application.'; - ExpenseManagementAdminPermissionRequiredErr: Label 'You must be assigned the Expense Management - Admin permission set to manage the Expense Agent Microsoft Entra application.'; + PermissionSetRequiredErr: Label 'You must be assigned the %1 permission set to manage the Expense Agent Microsoft Entra application.', Comment = '%1 = permission set name'; PermissionSetMissingErr: Label 'The %1 permission set is not available.', Comment = '%1 = permission set ID'; SecurityPermissionRequiredErr: Label 'You must be assigned either the SUPER or SECURITY permission set to manage the Expense Agent Microsoft Entra application.'; } diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al index 99f5286e613..5ca02c6efe6 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al @@ -10,6 +10,7 @@ using Microsoft.HumanResources.Employee; using Microsoft.HumanResources.Setup; using System.Environment.Configuration; using System.Security.AccessControl; +using System.Security.User; using System.TestLibraries.Security.AccessControl; codeunit 148338 "Expense Permissions Test" @@ -27,6 +28,7 @@ codeunit 148338 "Expense Permissions Test" UserPermissionsLibrary: Codeunit "User Permissions Library"; IsInitialized: Boolean; AgentAdminPermissionSetTok: Label 'Agent - Admin', Locked = true; + BaseApplicationAppIdTok: Label '437dbf0e-84ff-417a-965d-ed2bb9650972', Locked = true; EmployeeOnlyPermissionSetTok: Label 'Exp. Emp. Only Test', Locked = true; HREditPermissionSetTok: Label 'Exp. HR Edit Test', Locked = true; AutomationPermissionSetTok: Label 'Exp. Auto Test', Locked = true; @@ -35,8 +37,6 @@ codeunit 148338 "Expense Permissions Test" ExpenseAgentAppIdTok: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseMgmtAdminPermissionSetTok: Label 'Expense Mgmt. Admin', Locked = true; SecurityPermissionSetTok: Label 'SECURITY', Locked = true; - ExpenseAgentPermissionRequiredErr: Label 'You must be assigned the Expense Agent permission set to manage the Expense Agent Microsoft Entra application.'; - ExpenseMgmtAdminPermissionRequiredErr: Label 'You must be assigned the Expense Management - Admin permission set to manage the Expense Agent Microsoft Entra application.'; PermissionDeniedErr: Label 'You do not have the following permissions', Locked = true; CannotDeleteEmployeeWithExpenseErr: Label 'You cannot delete Employee %1 because they have active expense.', Comment = '%1 = Employee No.'; CannotDeleteEmployeeWithExpenseReportErr: Label 'You cannot delete Employee %1 because they have active expense report.', Comment = '%1 = Employee No.'; @@ -311,19 +311,20 @@ codeunit 148338 "Expense Permissions Test" end; [Test] - procedure RequiredPermissionsCanActivateExpenseAgentEntraApplication() + procedure SuperCanActivateWithoutAdditionalPermissionSets() var AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [SCENARIO 640454] The required administrator permissions can activate the Expense Agent Entra application + // [SCENARIO 640454] SUPER can activate without additional Expense Agent administrator permission sets Initialize(); // [GIVEN] Disabled Entra app "EA" without an Expense Agent permission PrepareAadApplication(AadApplication, AadApplication.State::Disabled); - // [GIVEN] User "U" has Agent Admin, Expense Management Admin, SECURITY, and Expense Agent - SetRequiredExpenseAgentManagementPermissions(); + // [GIVEN] SUPER user "U" has none of the additional Expense Agent administrator permission sets + SetExpenseAgentManagementPermissions(false, false, false, false); + VerifySuperWithoutAdditionalExpenseAgentPermissionSets(); // [WHEN] "U" activates "EA" ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); @@ -335,20 +336,21 @@ codeunit 148338 "Expense Permissions Test" end; [Test] - procedure RequiredPermissionsCanDeactivateExpenseAgentEntraApplication() + procedure SuperCanDeactivateWithoutAdditionalPermissionSets() var AadApplication: Record "AAD Application"; ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; begin - // [SCENARIO 640454] The required administrator permissions can deactivate the Expense Agent Entra application + // [SCENARIO 640454] SUPER can deactivate without additional Expense Agent administrator permission sets Initialize(); // [GIVEN] Enabled Entra app "EA" with the current-company Expense Agent permission PrepareAadApplication(AadApplication, AadApplication.State::Enabled); AssignExpenseAgentPermission(AadApplication, GetCurrentCompanyName()); - // [GIVEN] User "U" has Agent Admin, Expense Management Admin, SECURITY, and Expense Agent - SetRequiredExpenseAgentManagementPermissions(); + // [GIVEN] SUPER user "U" has none of the additional Expense Agent administrator permission sets + SetExpenseAgentManagementPermissions(false, false, false, false); + VerifySuperWithoutAdditionalExpenseAgentPermissionSets(); // [WHEN] "U" deactivates "EA" ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); @@ -358,73 +360,6 @@ codeunit 148338 "Expense Permissions Test" VerifyExpenseAgentPermissionCount(AadApplication, GetCurrentCompanyName(), 0); end; - [Test] - procedure ActivationRequiresExpenseManagementAdminPermission() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Activation requires Expense Management Admin - Initialize(); - - // [GIVEN] Disabled Entra app "EA" and user "U" without Expense Management Admin - PrepareAadApplication(AadApplication, AadApplication.State::Disabled); - SetExpenseAgentManagementPermissions(true, false, true, true); - - // [WHEN] "U" activates "EA" - asserterror ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - - // [THEN] Activation is rejected - Assert.ExpectedError(ExpenseMgmtAdminPermissionRequiredErr); - Assert.ExpectedErrorCode('Dialog'); - RestoreExpenseAgentManagementPermissions(); - end; - - [Test] - procedure ActivationRequiresExpenseAgentPermission() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Activation requires the administrator to hold the Expense Agent permission - Initialize(); - - // [GIVEN] Disabled Entra app "EA" and user "U" without Expense Agent - PrepareAadApplication(AadApplication, AadApplication.State::Disabled); - SetExpenseAgentManagementPermissions(true, true, true, false); - - // [WHEN] "U" activates "EA" - asserterror ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - - // [THEN] Activation is rejected - Assert.ExpectedError(ExpenseAgentPermissionRequiredErr); - Assert.ExpectedErrorCode('Dialog'); - RestoreExpenseAgentManagementPermissions(); - end; - - [Test] - procedure DeactivationRequiresExpenseAgentPermissionBeforeMutation() - var - AadApplication: Record "AAD Application"; - ExpenseAgentEntraApp: Codeunit "Expense Agent Entra App Mgt."; - begin - // [SCENARIO 640454] Deactivation requires the administrator to hold the Expense Agent permission - Initialize(); - - // [GIVEN] Enabled Entra app "EA" with a current-company grant and user "U" without Expense Agent - PrepareAadApplication(AadApplication, AadApplication.State::Enabled); - AssignExpenseAgentPermission(AadApplication, GetCurrentCompanyName()); - SetExpenseAgentManagementPermissions(true, true, true, false); - - // [WHEN] "U" deactivates "EA" - asserterror ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - - // [THEN] Deactivation is rejected - Assert.ExpectedError(ExpenseAgentPermissionRequiredErr); - Assert.ExpectedErrorCode('Dialog'); - RestoreExpenseAgentManagementPermissions(); - end; - local procedure Initialize() begin LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Permissions Test"); @@ -443,11 +378,6 @@ codeunit 148338 "Expense Permissions Test" LibraryTestInitialize.OnAfterTestSuiteInitialize(Codeunit::"Expense Permissions Test"); end; - local procedure SetRequiredExpenseAgentManagementPermissions() - begin - SetExpenseAgentManagementPermissions(true, true, true, true); - end; - local procedure SetExpenseAgentManagementPermissions(IncludeAgentAdmin: Boolean; IncludeExpenseMgmtAdmin: Boolean; IncludeSecurity: Boolean; IncludeExpenseAgent: Boolean) begin PrepareCurrentUserPermissionAssignments(); @@ -480,6 +410,43 @@ codeunit 148338 "Expense Permissions Test" RemoveCurrentUserPermissionSet(ExpenseAgentPermissionSetTok); end; + local procedure VerifySuperWithoutAdditionalExpenseAgentPermissionSets() + var + AccessControl: Record "Access Control"; + AggregatePermissionSet: Record "Aggregate Permission Set"; + UserPermissions: Codeunit "User Permissions"; + BaseApplicationAppId: Guid; + NullGuid: Guid; + begin + Assert.IsTrue(UserPermissions.IsSuper(UserSecurityId()), 'The test user must be SUPER.'); + + Evaluate(BaseApplicationAppId, BaseApplicationAppIdTok); + AggregatePermissionSet.SetRange("App ID", BaseApplicationAppId); + AggregatePermissionSet.SetRange("Role ID", AgentAdminPermissionSetTok); + AggregatePermissionSet.FindFirst(); + Assert.IsFalse( + UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), GetCurrentCompanyName(), AggregatePermissionSet."Role ID", AggregatePermissionSet.Scope, AggregatePermissionSet."App ID"), + 'Agent - Admin must not be assigned.'); + + GetExpensePermissionSet(AggregatePermissionSet, ExpenseMgmtAdminPermissionSetTok); + Assert.IsFalse( + UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), GetCurrentCompanyName(), AggregatePermissionSet."Role ID", AggregatePermissionSet.Scope, AggregatePermissionSet."App ID"), + 'Expense Mgmt. Admin must not be assigned.'); + + Assert.IsFalse( + UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), GetCurrentCompanyName(), SecurityPermissionSetTok, AccessControl.Scope::System, NullGuid), + 'SECURITY must not be assigned.'); + + GetExpenseAgentPermissionSet(AggregatePermissionSet); + Assert.IsFalse( + UserPermissions.HasUserPermissionSetAssigned( + UserSecurityId(), GetCurrentCompanyName(), AggregatePermissionSet."Role ID", AggregatePermissionSet.Scope, AggregatePermissionSet."App ID"), + 'Expense Agent must not be assigned.'); + end; + local procedure RestoreExpenseAgentManagementPermissions() begin RestoreFullPermissions(); From 0ae5941f636a0dff05f561c06dd111078baab662 Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Wed, 16 Sep 2026 14:48:01 +0200 Subject: [PATCH 23/24] Run SUPER tests under SUPER permissions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../src/ExpensePermissionsTest.Codeunit.al | 67 +------------------ 1 file changed, 2 insertions(+), 65 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al index 5ca02c6efe6..aae9d12e946 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpensePermissionsTest.Codeunit.al @@ -11,7 +11,6 @@ using Microsoft.HumanResources.Setup; using System.Environment.Configuration; using System.Security.AccessControl; using System.Security.User; -using System.TestLibraries.Security.AccessControl; codeunit 148338 "Expense Permissions Test" { @@ -25,7 +24,6 @@ codeunit 148338 "Expense Permissions Test" LibraryLowerPermissions: Codeunit "Library - Lower Permissions"; LibraryRandom: Codeunit "Library - Random"; LibraryTestInitialize: Codeunit "Library - Test Initialize"; - UserPermissionsLibrary: Codeunit "User Permissions Library"; IsInitialized: Boolean; AgentAdminPermissionSetTok: Label 'Agent - Admin', Locked = true; BaseApplicationAppIdTok: Label '437dbf0e-84ff-417a-965d-ed2bb9650972', Locked = true; @@ -323,12 +321,11 @@ codeunit 148338 "Expense Permissions Test" PrepareAadApplication(AadApplication, AadApplication.State::Disabled); // [GIVEN] SUPER user "U" has none of the additional Expense Agent administrator permission sets - SetExpenseAgentManagementPermissions(false, false, false, false); + PrepareCurrentUserPermissionAssignments(); VerifySuperWithoutAdditionalExpenseAgentPermissionSets(); // [WHEN] "U" activates "EA" ExpenseAgentEntraApp.EnableAadApplicationForCurrentCompany(); - RestoreExpenseAgentManagementPermissions(); // [THEN] "EA" is enabled with one current-company Expense Agent permission VerifyAadApplicationState(AadApplication.State::Enabled); @@ -349,12 +346,11 @@ codeunit 148338 "Expense Permissions Test" AssignExpenseAgentPermission(AadApplication, GetCurrentCompanyName()); // [GIVEN] SUPER user "U" has none of the additional Expense Agent administrator permission sets - SetExpenseAgentManagementPermissions(false, false, false, false); + PrepareCurrentUserPermissionAssignments(); VerifySuperWithoutAdditionalExpenseAgentPermissionSets(); // [WHEN] "U" deactivates "EA" ExpenseAgentEntraApp.DisableAadApplicationForCurrentCompany(); - RestoreExpenseAgentManagementPermissions(); // [THEN] The current-company Expense Agent permission is removed VerifyExpenseAgentPermissionCount(AadApplication, GetCurrentCompanyName(), 0); @@ -378,30 +374,6 @@ codeunit 148338 "Expense Permissions Test" LibraryTestInitialize.OnAfterTestSuiteInitialize(Codeunit::"Expense Permissions Test"); end; - local procedure SetExpenseAgentManagementPermissions(IncludeAgentAdmin: Boolean; IncludeExpenseMgmtAdmin: Boolean; IncludeSecurity: Boolean; IncludeExpenseAgent: Boolean) - begin - PrepareCurrentUserPermissionAssignments(); - - if IncludeAgentAdmin then - AssignCurrentUserPermissionSet(AgentAdminPermissionSetTok); - if IncludeExpenseMgmtAdmin then - AssignCurrentUserExpensePermissionSet(ExpenseMgmtAdminPermissionSetTok); - if IncludeSecurity then - UserPermissionsLibrary.AssignPermissionSetToUser(UserSecurityId(), SecurityPermissionSetTok); - if IncludeExpenseAgent then - AssignCurrentUserExpensePermissionSet(ExpenseAgentPermissionSetTok); - - LibraryLowerPermissions.SetExactPermissionSet(D365BasicPermissionSetTok); - if IncludeAgentAdmin then - LibraryLowerPermissions.AddPermissionSet(AgentAdminPermissionSetTok); - if IncludeExpenseMgmtAdmin then - LibraryLowerPermissions.AddPermissionSet(ExpenseMgmtAdminPermissionSetTok); - if IncludeSecurity then - LibraryLowerPermissions.AddSecurity(); - if IncludeExpenseAgent then - LibraryLowerPermissions.AddPermissionSet(ExpenseAgentPermissionSetTok); - end; - local procedure PrepareCurrentUserPermissionAssignments() begin RemoveCurrentUserPermissionSet(AgentAdminPermissionSetTok); @@ -447,11 +419,6 @@ codeunit 148338 "Expense Permissions Test" 'Expense Agent must not be assigned.'); end; - local procedure RestoreExpenseAgentManagementPermissions() - begin - RestoreFullPermissions(); - end; - local procedure RemoveCurrentUserPermissionSet(PermissionSetId: Code[20]) var AccessControl: Record "Access Control"; @@ -491,36 +458,6 @@ codeunit 148338 "Expense Permissions Test" AccessControl.Insert(true); end; - local procedure AssignCurrentUserExpensePermissionSet(PermissionSetId: Code[20]) - var - AccessControl: Record "Access Control"; - AggregatePermissionSet: Record "Aggregate Permission Set"; - begin - GetExpensePermissionSet(AggregatePermissionSet, PermissionSetId); - AccessControl.Init(); - AccessControl."User Security ID" := UserSecurityId(); - AccessControl."Role ID" := AggregatePermissionSet."Role ID"; - AccessControl."Company Name" := GetCurrentCompanyName(); - AccessControl.Scope := AggregatePermissionSet.Scope; - AccessControl."App ID" := AggregatePermissionSet."App ID"; - AccessControl.Insert(true); - end; - - local procedure AssignCurrentUserPermissionSet(PermissionSetId: Code[20]) - var - AccessControl: Record "Access Control"; - AggregatePermissionSet: Record "Aggregate Permission Set"; - begin - AggregatePermissionSet.SetRange("Role ID", PermissionSetId); - AggregatePermissionSet.FindFirst(); - AccessControl.Init(); - AccessControl."User Security ID" := UserSecurityId(); - AccessControl."Role ID" := AggregatePermissionSet."Role ID"; - AccessControl.Scope := AggregatePermissionSet.Scope; - AccessControl."App ID" := AggregatePermissionSet."App ID"; - AccessControl.Insert(true); - end; - local procedure GetExpenseAgentPermissionSet(var AggregatePermissionSet: Record "Aggregate Permission Set") begin GetExpensePermissionSet(AggregatePermissionSet, ExpenseAgentPermissionSetTok); From bb2182e6df08b7462ee2d243812b1c9f0482251a Mon Sep 17 00:00:00 2001 From: Prangshuman Das Date: Wed, 16 Sep 2026 15:18:04 +0200 Subject: [PATCH 24/24] Make Expense Agent lifecycle test permissions explicit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 520d66e2-67cf-44bb-8863-5c195128151b --- .../src/ExpenseAgentConfigTest.Codeunit.al | 58 +------------------ 1 file changed, 1 insertion(+), 57 deletions(-) diff --git a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al index f3ad784250f..dea0f478eae 100644 --- a/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al +++ b/src/Apps/W1/ExpenseAgent/test/src/ExpenseAgentConfigTest.Codeunit.al @@ -7,14 +7,13 @@ namespace Microsoft.Test.ExpenseAgent; using Microsoft.ExpenseAgent; using System.Environment.Configuration; using System.Security.AccessControl; -using System.Security.User; using System.TestLibraries.Security.AccessControl; codeunit 148361 "Expense Agent Config. Test" { Subtype = Test; TestType = UnitTest; - TestPermissions = Restrictive; + TestPermissions = Disabled; Permissions = tabledata "AAD Application" = rm, tabledata "Access Control" = rid; @@ -23,10 +22,8 @@ codeunit 148361 "Expense Agent Config. Test" Assert: Codeunit Assert; LibraryTestInitialize: Codeunit "Library - Test Initialize"; UserPermissionsLibrary: Codeunit "User Permissions Library"; - AgentAdminPermissionSetTok: Label 'Agent - Admin', Locked = true; ExpenseAgentAppIdTok: Label '66efe10c-8033-403b-a86d-77c0887178ba', Locked = true; ExpenseAgentPermissionSetTok: Label 'Expense Agent', Locked = true; - ExpenseManagementAdminPermissionSetTok: Label 'Expense Mgmt. Admin', Locked = true; UnrelatedPermissionSetTok: Label 'D365 BASIC', Locked = true; [Test] @@ -236,59 +233,6 @@ codeunit 148361 "Expense Agent Config. Test" local procedure Initialize() begin LibraryTestInitialize.OnTestInitialize(Codeunit::"Expense Agent Config. Test"); - EnsureCurrentUserAgentAdminPermissionSetAssigned(); - EnsureCurrentUserPermissionSetAssigned(ExpenseManagementAdminPermissionSetTok); - EnsureCurrentUserPermissionSetAssigned(ExpenseAgentPermissionSetTok); - end; - - local procedure EnsureCurrentUserAgentAdminPermissionSetAssigned() - var - AccessControl: Record "Access Control"; - AggregatePermissionSet: Record "Aggregate Permission Set"; - UserPermissions: Codeunit "User Permissions"; - begin - AggregatePermissionSet.SetRange("Role ID", AgentAdminPermissionSetTok); - AggregatePermissionSet.FindFirst(); - if UserPermissions.HasUserPermissionSetAssigned( - UserSecurityId(), - GetCurrentCompanyName(), - AggregatePermissionSet."Role ID", - AggregatePermissionSet.Scope, - AggregatePermissionSet."App ID") - then - exit; - - AccessControl.Init(); - AccessControl."User Security ID" := UserSecurityId(); - AccessControl."Role ID" := AggregatePermissionSet."Role ID"; - AccessControl.Scope := AggregatePermissionSet.Scope; - AccessControl."App ID" := AggregatePermissionSet."App ID"; - AccessControl.Insert(true); - end; - - local procedure EnsureCurrentUserPermissionSetAssigned(PermissionSetId: Code[20]) - var - AccessControl: Record "Access Control"; - AggregatePermissionSet: Record "Aggregate Permission Set"; - UserPermissions: Codeunit "User Permissions"; - begin - GetPermissionSet(AggregatePermissionSet, PermissionSetId); - if UserPermissions.HasUserPermissionSetAssigned( - UserSecurityId(), - GetCurrentCompanyName(), - AggregatePermissionSet."Role ID", - AggregatePermissionSet.Scope, - AggregatePermissionSet."App ID") - then - exit; - - AccessControl.Init(); - AccessControl."User Security ID" := UserSecurityId(); - AccessControl."Role ID" := AggregatePermissionSet."Role ID"; - AccessControl."Company Name" := GetCurrentCompanyName(); - AccessControl.Scope := AggregatePermissionSet.Scope; - AccessControl."App ID" := AggregatePermissionSet."App ID"; - AccessControl.Insert(true); end; local procedure PrepareAadApplication(var AadApplication: Record "AAD Application"; State: Option)