diff --git a/src/System Application/App/Azure AD Plan/src/AzureADPlanImpl.Codeunit.al b/src/System Application/App/Azure AD Plan/src/AzureADPlanImpl.Codeunit.al index 515f5ae2d6..3dd072d237 100644 --- a/src/System Application/App/Azure AD Plan/src/AzureADPlanImpl.Codeunit.al +++ b/src/System Application/App/Azure AD Plan/src/AzureADPlanImpl.Codeunit.al @@ -662,7 +662,7 @@ codeunit 9018 "Azure AD Plan Impl." PlanIds: Codeunit "Plan Ids"; PlanConfigurationImpl: Codeunit "Plan Configuration Impl."; UserPermissions: Codeunit "User Permissions"; - UserGroupsAdded, ShouldRemoveSuper, HasPlans : Boolean; + UserGroupsAdded, ShouldRemoveSuper : Boolean; PlanId: Guid; begin case true of @@ -682,9 +682,6 @@ codeunit 9018 "Azure AD Plan Impl." Session.LogMessage('0000IC4', StrSubstNo(AssigningPlanForDelegatedRoleTxt, PlanId, not SkipUpdateUserAccess), Verbosity::Normal, DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, 'Category', UserSetupCategoryTxt); - // Check if the user have any plans assigned before removing - HasPlans := DoesUserHavePlans(UserSID); - // Delete any existing plans for the user UserPlan.SetRange("User Security ID", UserSID); UserPlan.DeleteAll(); @@ -695,16 +692,12 @@ codeunit 9018 "Azure AD Plan Impl." UserPlan."User Security ID" := UserSID; UserPlan.Insert(); - // Exit if user access should not be updated - if SkipUpdateUserAccess then - exit; - + // Fix for ADO #612420: Removed early-exit checks on SkipUpdateUserAccess and HasPlans + // that prevented access updates when a user's plan changed (e.g., guest to GDAP). + // The IsPlanAssignedToUser guard above already prevents re-processing unchanged plans. if not UserProperty.Get(UserSID) then exit; - if HasPlans then - exit; - // Assign user groups for the user AzureADPlan.OnUpdateUserAccessForSaaS(UserPlan."User Security ID", UserGroupsAdded); diff --git a/src/System Application/Test/Azure AD Plan/src/AzureADPlanTests.Codeunit.al b/src/System Application/Test/Azure AD Plan/src/AzureADPlanTests.Codeunit.al index dfdf47cb10..5524f94123 100644 --- a/src/System Application/Test/Azure AD Plan/src/AzureADPlanTests.Codeunit.al +++ b/src/System Application/Test/Azure AD Plan/src/AzureADPlanTests.Codeunit.al @@ -7,6 +7,7 @@ namespace System.Test.Azure.ActiveDirectory; using System; using System.Azure.Identity; +using System.Security.AccessControl; using System.Security.User; using System.TestLibraries.Azure.ActiveDirectory; using System.TestLibraries.Environment; @@ -17,6 +18,7 @@ using System.TestLibraries.Utilities; codeunit 132912 "Azure AD Plan Tests" { Subtype = Test; + Permissions = tabledata "User Property" = rimd; TestPermissions = Disabled; EventSubscriberInstance = Manual; @@ -697,6 +699,76 @@ codeunit 132912 "Azure AD Plan Tests" LibraryAssert.IsTrue(UserPermissions.IsSuper(UserSID), 'User should be SUPER'); end; + [Test] + [TransactionModel(TransactionModel::AutoRollback)] + [CommitBehavior(CommitBehavior::Ignore)] + [Scope('OnPrem')] + procedure GDAPUserWithPreviousGuestPlanGetsAccessUpdated() + var + UserPlan: Record "User Plan"; + UserProperty: Record "User Property"; + AzureADPlan: Codeunit "Azure AD Plan"; + UserPermissions: Codeunit "User Permissions"; + AzureADPlanTestLibraries: Codeunit "Azure AD Plan Test Library"; + AzureADUserTestLibrary: Codeunit "Azure AD User Test Library"; + PlanConfigurationLibrary: Codeunit "Plan Configuration Library"; + UserPermissionsLibrary: Codeunit "User Permissions Library"; + AzureAdPlanTest: Codeunit "Azure AD Plan Tests"; + PlanIds: Codeunit "Plan Ids"; + UserSID: Guid; + begin + // [Scenario] A GDAP delegated admin user who was previously a guest user gets the GDAP plan + // assigned on login even when SkipUpdateUserAccess = true. + // Fix for ADO #612420: The early-exit conditions in AssignPlanToUserWithDelegatedRole + // prevented the GDAP plan from replacing the guest plan for returning users. + + DeleteAllFromTablePlanAndUserPlan(); + PlanConfigurationLibrary.ClearPlanConfigurations(); + BindSubscription(AzureAdPlanTest); + EnvironmentInformationTestLibrary.SetTestabilitySoftwareAsAService(true); + + // [Given] A SUPER user + UserSID := UserPermissionsLibrary.CreateSuperUser('NEWUSER'); + UserPermissionsLibrary.CreateSuperUser('ANOTHERUSER'); + + // [Given] The Delegated Admin agent - Partner plan exists + AzureADPlanTestLibraries.CreatePlan(PlanIds.GetDelegatedAdminPlanId(), 'Delegated Admin agent - Partner', 9022, '7584DDCA-27B8-E911-BB26-000D3A2B005C'); + + // [Given] The user previously had a guest plan (simulating a former guest user) + AzureADPlanTestLibraries.AssignUserToPlan(UserSID, AzureADPlanTestLibraries.CreatePlan('Guest Plan')); + + // [Given] The user has a UserProperty record (required for access update path) + if not UserProperty.Get(UserSID) then begin + UserProperty.Init(); + UserProperty."User Security ID" := UserSID; + UserProperty."Authentication Object ID" := UserSID; + UserProperty.Insert(); + end; + + // [Given] The current user is a delegated admin + BindSubscription(AzureADUserTestLibrary); + AzureADUserTestLibrary.SetIsUserDelegatedAdmin(true); + + // [Given] The plan configuration is NOT customized (default permissions apply) + // This avoids the RemoveSuperPermissions path which requires a real Windows user + LibraryAssert.IsFalse(AzureADPlan.IsPlanAssignedToUser(PlanIds.GetDelegatedAdminPlanId(), UserSID), 'GDAP plan should not yet be assigned'); + + // [When] The plan is assigned per delegated role with SkipUpdateUserAccess = true + // (simulating a returning user logging in - the caller Run() passes UserLoggedInEnvironment as SkipUpdateUserAccess) + AzureADPlan.AssignPlanToUserWithDelegatedRole(UserSID, true); + + // [Then] The GDAP plan is assigned to the user (the old guest plan was replaced) + // Bug 612420: With the buggy code, the procedure exited early when SkipUpdateUserAccess = true + // and the user had existing plans, preventing the GDAP plan from being assigned. + UserPlan.SetRange("User Security ID", UserSID); + LibraryAssert.AreEqual(1, UserPlan.Count(), 'There should be exactly one plan assignment (guest plan replaced by GDAP plan)'); + LibraryAssert.IsTrue(UserPlan.FindFirst(), 'There should be a plan assigned'); + LibraryAssert.AreEqual(PlanIds.GetDelegatedAdminPlanId(), UserPlan."Plan ID", 'The delegated admin plan should be assigned, replacing the guest plan'); + + // [Then] SUPER is still assigned (plan config is not customized, so SUPER is not removed) + LibraryAssert.IsTrue(UserPermissions.IsSuper(UserSID), 'User should still be SUPER (plan config is not customized)'); + end; + [Test] [TransactionModel(TransactionModel::AutoRollback)] [Scope('OnPrem')]