[PM-34601] Bump Group.RevisionDate on edits and access changes

bitwarden_license/src/Scim/Groups/PatchGroupCommand.cs

@@ -22,19 +22,22 @@ public class PatchGroupCommand : IPatchGroupCommand
     private readonly IUpdateGroupCommand _updateGroupCommand;
     private readonly ILogger<PatchGroupCommand> _logger;
     private readonly IOrganizationRepository _organizationRepository;
+    private readonly TimeProvider _timeProvider;
 
     public PatchGroupCommand(
         IGroupRepository groupRepository,
         IGroupService groupService,
         IUpdateGroupCommand updateGroupCommand,
         ILogger<PatchGroupCommand> logger,
-        IOrganizationRepository organizationRepository)
+        IOrganizationRepository organizationRepository,
+        TimeProvider timeProvider)
     {
         _groupRepository = groupRepository;
         _groupService = groupService;
         _updateGroupCommand = updateGroupCommand;
         _logger = logger;
         _organizationRepository = organizationRepository;
+        _timeProvider = timeProvider;
     }
 
     public async Task PatchGroupAsync(Group group, ScimPatchModel model)
@@ -53,7 +56,7 @@ private async Task HandleOperationAsync(Group group, ScimPatchModel.OperationMod
             case PatchOps.Replace when operation.Path?.ToLowerInvariant() == PatchPaths.Members:
                 {
                     var ids = GetOperationValueIds(operation.Value);
-                    await _groupRepository.UpdateUsersAsync(group.Id, ids);
+                    await _groupRepository.UpdateUsersAsync(group.Id, ids, _timeProvider.GetUtcNow().UtcDateTime);
                     break;
                 }
 
@@ -122,7 +125,7 @@ private async Task HandleOperationAsync(Group group, ScimPatchModel.OperationMod
                     {
                         orgUserIds.Remove(v);
                     }
-                    await _groupRepository.UpdateUsersAsync(group.Id, orgUserIds);
+                    await _groupRepository.UpdateUsersAsync(group.Id, orgUserIds, _timeProvider.GetUtcNow().UtcDateTime);
                     break;
                 }
 
@@ -146,7 +149,7 @@ private async Task AddMembersAsync(Group group, HashSet<Guid> usersToAdd)
             return;
         }
 
-        await _groupRepository.AddGroupUsersByIdAsync(group.Id, usersToAdd);
+        await _groupRepository.AddGroupUsersByIdAsync(group.Id, usersToAdd, _timeProvider.GetUtcNow().UtcDateTime);
     }
 
     private static HashSet<Guid> GetOperationValueIds(JsonElement objArray)

bitwarden_license/src/Scim/Groups/PostGroupCommand.cs

@@ -62,6 +62,6 @@ private async Task UpdateGroupMembersAsync(Group group, ScimGroupRequestModel mo
             return;
         }
 
-        await _groupRepository.UpdateUsersAsync(group.Id, memberIds);
+        await _groupRepository.UpdateUsersAsync(group.Id, memberIds, group.RevisionDate);
     }
 }

bitwarden_license/src/Scim/Groups/PutGroupCommand.cs

@@ -52,6 +52,6 @@ private async Task UpdateGroupMembersAsync(Group group, ScimGroupRequestModel mo
             }
         }
 
-        await _groupRepository.UpdateUsersAsync(group.Id, memberIds);
+        await _groupRepository.UpdateUsersAsync(group.Id, memberIds, group.RevisionDate);
     }
 }

bitwarden_license/test/Scim.Test/Groups/PatchGroupCommandTests.cs

@@ -13,6 +13,7 @@
 using Bit.Test.Common.AutoFixture;
 using Bit.Test.Common.AutoFixture.Attributes;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Time.Testing;
 using NSubstitute;
 using Xunit;
 
@@ -21,11 +22,14 @@ namespace Bit.Scim.Test.Groups;
 [SutProviderCustomize]
 public class PatchGroupCommandTests
 {
+    private static readonly DateTime _expectedRevisionDate = DateTime.UtcNow.AddYears(1);
+
     [Theory]
     [BitAutoData]
-    public async Task PatchGroup_ReplaceListMembers_Success(SutProvider<PatchGroupCommand> sutProvider,
+    public async Task PatchGroup_ReplaceListMembers_Success(
         Organization organization, Group group, IEnumerable<Guid> userIds)
     {
+        var sutProvider = SetupSutProvider();
         group.OrganizationId = organization.Id;
 
         var scimPatchModel = new ScimPatchModel
@@ -48,7 +52,8 @@ await sutProvider.GetDependency<IGroupRepository>().Received(1).UpdateUsersAsync
             group.Id,
             Arg.Is<IEnumerable<Guid>>(arg =>
                 arg.Count() == userIds.Count() &&
-                arg.ToHashSet().SetEquals(userIds)));
+                arg.ToHashSet().SetEquals(userIds)),
+            Arg.Is<DateTime>(d => d == _expectedRevisionDate));
     }
 
     [Theory]
@@ -168,8 +173,9 @@ public async Task PatchGroup_ReplaceDisplayNameFromValueObject_MissingOrganizati
 
     [Theory]
     [BitAutoData]
-    public async Task PatchGroup_AddSingleMember_Success(SutProvider<PatchGroupCommand> sutProvider, Organization organization, Group group, ICollection<Guid> existingMembers, Guid userId)
+    public async Task PatchGroup_AddSingleMember_Success(Organization organization, Group group, ICollection<Guid> existingMembers, Guid userId)
     {
+        var sutProvider = SetupSutProvider();
         group.OrganizationId = organization.Id;
 
         sutProvider.GetDependency<IGroupRepository>()
@@ -193,7 +199,8 @@ public async Task PatchGroup_AddSingleMember_Success(SutProvider<PatchGroupComma
 
         await sutProvider.GetDependency<IGroupRepository>().Received(1).AddGroupUsersByIdAsync(
             group.Id,
-            Arg.Is<IEnumerable<Guid>>(arg => arg.Single() == userId));
+            Arg.Is<IEnumerable<Guid>>(arg => arg.Single() == userId),
+            Arg.Is<DateTime>(d => d == _expectedRevisionDate));
     }
 
     [Theory]
@@ -229,13 +236,14 @@ public async Task PatchGroup_AddSingleMember_ReturnsEarlyIfAlreadyInGroup(
 
         await sutProvider.GetDependency<IGroupRepository>()
             .DidNotReceiveWithAnyArgs()
-            .AddGroupUsersByIdAsync(default, default);
+            .AddGroupUsersByIdAsync(default, default, default);
     }
 
     [Theory]
     [BitAutoData]
-    public async Task PatchGroup_AddListMembers_Success(SutProvider<PatchGroupCommand> sutProvider, Organization organization, Group group, ICollection<Guid> existingMembers, ICollection<Guid> userIds)
+    public async Task PatchGroup_AddListMembers_Success(Organization organization, Group group, ICollection<Guid> existingMembers, ICollection<Guid> userIds)
     {
+        var sutProvider = SetupSutProvider();
         group.OrganizationId = organization.Id;
 
         sutProvider.GetDependency<IGroupRepository>()
@@ -262,15 +270,18 @@ await sutProvider.GetDependency<IGroupRepository>().Received(1).AddGroupUsersByI
             group.Id,
             Arg.Is<IEnumerable<Guid>>(arg =>
                 arg.Count() == userIds.Count &&
-                arg.ToHashSet().SetEquals(userIds)));
+                arg.ToHashSet().SetEquals(userIds)),
+            Arg.Is<DateTime>(d => d == _expectedRevisionDate));
     }
 
     [Theory]
     [BitAutoData]
     public async Task PatchGroup_AddListMembers_IgnoresDuplicatesInRequest(
-        SutProvider<PatchGroupCommand> sutProvider, Organization organization, Group group,
+        Organization organization, Group group,
         ICollection<Guid> existingMembers)
     {
+        var sutProvider = SetupSutProvider();
+
         // Create 3 userIds
         var fixture = new Fixture { RepeatCount = 3 };
         var userIds = fixture.CreateMany<Guid>().ToList();
@@ -308,17 +319,19 @@ await sutProvider.GetDependency<IGroupRepository>().Received(1).AddGroupUsersByI
             group.Id,
             Arg.Is<IEnumerable<Guid>>(arg =>
                 arg.Count() == 3 &&
-                arg.ToHashSet().SetEquals(userIds)));
+                arg.ToHashSet().SetEquals(userIds)),
+            Arg.Is<DateTime>(d => d == _expectedRevisionDate));
     }
 
     [Theory]
     [BitAutoData]
     public async Task PatchGroup_AddListMembers_SuccessIfOnlySomeUsersAreInGroup(
-        SutProvider<PatchGroupCommand> sutProvider,
         Organization organization, Group group,
         ICollection<Guid> existingMembers,
         ICollection<Guid> userIds)
     {
+        var sutProvider = SetupSutProvider();
+
         // A user is already in the group, but some still need to be added
         userIds.Add(existingMembers.First());
 
@@ -350,7 +363,8 @@ await sutProvider.GetDependency<IGroupRepository>()
                 group.Id,
                 Arg.Is<IEnumerable<Guid>>(arg =>
                     arg.Count() == userIds.Count &&
-                    arg.ToHashSet().SetEquals(userIds)));
+                    arg.ToHashSet().SetEquals(userIds)),
+                Arg.Is<DateTime>(d => d == _expectedRevisionDate));
     }
 
     [Theory]
@@ -379,9 +393,10 @@ public async Task PatchGroup_RemoveSingleMember_Success(SutProvider<PatchGroupCo
 
     [Theory]
     [BitAutoData]
-    public async Task PatchGroup_RemoveListMembers_Success(SutProvider<PatchGroupCommand> sutProvider,
+    public async Task PatchGroup_RemoveListMembers_Success(
         Organization organization, Group group, ICollection<Guid> existingMembers)
     {
+        var sutProvider = SetupSutProvider();
         List<Guid> usersToRemove = [existingMembers.First(), existingMembers.Skip(1).First()];
         group.OrganizationId = organization.Id;
 
@@ -412,7 +427,8 @@ await sutProvider.GetDependency<IGroupRepository>()
                 group.Id,
                 Arg.Is<IEnumerable<Guid>>(arg =>
                     arg.Count() == expectedRemainingUsers.Count &&
-                    arg.ToHashSet().SetEquals(expectedRemainingUsers)));
+                    arg.ToHashSet().SetEquals(expectedRemainingUsers)),
+                Arg.Is<DateTime>(d => d == _expectedRevisionDate));
     }
 
     [Theory]
@@ -430,7 +446,7 @@ public async Task PatchGroup_InvalidOperation_Success(SutProvider<PatchGroupComm
         await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel);
 
         // Assert: no operation performed
-        await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default);
+        await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default, default);
         await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().GetManyUserIdsByIdAsync(default);
         await sutProvider.GetDependency<IUpdateGroupCommand>().DidNotReceiveWithAnyArgs().UpdateGroupAsync(default, default);
         await sutProvider.GetDependency<IGroupService>().DidNotReceiveWithAnyArgs().DeleteUserAsync(default, default);
@@ -454,9 +470,18 @@ public async Task PatchGroup_NoOperation_Success(
 
         await sutProvider.Sut.PatchGroupAsync(group, scimPatchModel);
 
-        await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default);
+        await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default, default);
         await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().GetManyUserIdsByIdAsync(default);
         await sutProvider.GetDependency<IUpdateGroupCommand>().DidNotReceiveWithAnyArgs().UpdateGroupAsync(default, default);
         await sutProvider.GetDependency<IGroupService>().DidNotReceiveWithAnyArgs().DeleteUserAsync(default, default);
     }
+
+    private static SutProvider<PatchGroupCommand> SetupSutProvider()
+    {
+        var sutProvider = new SutProvider<PatchGroupCommand>()
+            .WithFakeTimeProvider()
+            .Create();
+        sutProvider.GetDependency<FakeTimeProvider>().SetUtcNow(_expectedRevisionDate);
+        return sutProvider;
+    }
 }

bitwarden_license/test/Scim.Test/Groups/PostGroupCommandTests.cs

@@ -43,7 +43,7 @@ public async Task PostGroup_Success(SutProvider<PostGroupCommand> sutProvider, s
         var group = await sutProvider.Sut.PostGroupAsync(organization, scimGroupRequestModel);
 
         await sutProvider.GetDependency<ICreateGroupCommand>().Received(1).CreateGroupAsync(group, organization, EventSystemUser.SCIM, null);
-        await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default);
+        await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default, default);
 
         AssertHelper.AssertPropertyEqual(expectedResult, group, "Id", "CreationDate", "RevisionDate");
     }
@@ -74,7 +74,7 @@ public async Task PostGroup_WithMembers_Success(SutProvider<PostGroupCommand> su
         var group = await sutProvider.Sut.PostGroupAsync(organization, scimGroupRequestModel);
 
         await sutProvider.GetDependency<ICreateGroupCommand>().Received(1).CreateGroupAsync(group, organization, EventSystemUser.SCIM, null);
-        await sutProvider.GetDependency<IGroupRepository>().Received(1).UpdateUsersAsync(Arg.Any<Guid>(), Arg.Is<IEnumerable<Guid>>(arg => arg.All(id => membersUserIds.Contains(id))));
+        await sutProvider.GetDependency<IGroupRepository>().Received(1).UpdateUsersAsync(group.Id, Arg.Is<IEnumerable<Guid>>(arg => arg.All(id => membersUserIds.Contains(id))), group.RevisionDate);
 
         AssertHelper.AssertPropertyEqual(expectedResult, group, "Id", "CreationDate", "RevisionDate");
     }

bitwarden_license/test/Scim.Test/Groups/PutGroupCommandTests.cs

@@ -47,7 +47,7 @@ public async Task PutGroup_Success(SutProvider<PutGroupCommand> sutProvider, Org
         Assert.Equal(displayName, group.Name);
 
         await sutProvider.GetDependency<IUpdateGroupCommand>().Received(1).UpdateGroupAsync(group, organization, EventSystemUser.SCIM);
-        await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default);
+        await sutProvider.GetDependency<IGroupRepository>().DidNotReceiveWithAnyArgs().UpdateUsersAsync(default, default, default);
     }
 
     [Theory]
@@ -81,7 +81,7 @@ public async Task PutGroup_ChangeMembers_Success(SutProvider<PutGroupCommand> su
         Assert.Equal(displayName, group.Name);
 
         await sutProvider.GetDependency<IUpdateGroupCommand>().Received(1).UpdateGroupAsync(group, organization, EventSystemUser.SCIM);
-        await sutProvider.GetDependency<IGroupRepository>().Received(1).UpdateUsersAsync(group.Id, Arg.Is<IEnumerable<Guid>>(arg => arg.All(id => membersUserIds.Contains(id))));
+        await sutProvider.GetDependency<IGroupRepository>().Received(1).UpdateUsersAsync(group.Id, Arg.Is<IEnumerable<Guid>>(arg => arg.All(id => membersUserIds.Contains(id))), group.RevisionDate);
     }
 
     [Theory]

src/Api/AdminConsole/Public/Controllers/GroupsController.cs

@@ -23,19 +23,22 @@ public class GroupsController : Controller
     private readonly ICurrentContext _currentContext;
     private readonly ICreateGroupCommand _createGroupCommand;
     private readonly IUpdateGroupCommand _updateGroupCommand;
+    private readonly TimeProvider _timeProvider;
 
     public GroupsController(
         IGroupRepository groupRepository,
         IOrganizationRepository organizationRepository,
         ICurrentContext currentContext,
         ICreateGroupCommand createGroupCommand,
-        IUpdateGroupCommand updateGroupCommand)
+        IUpdateGroupCommand updateGroupCommand,
+        TimeProvider timeProvider)
     {
         _groupRepository = groupRepository;
         _organizationRepository = organizationRepository;
         _currentContext = currentContext;
         _createGroupCommand = createGroupCommand;
         _updateGroupCommand = updateGroupCommand;
+        _timeProvider = timeProvider;
     }
 
     /// <summary>
@@ -168,7 +171,7 @@ public async Task<IActionResult> PutMemberIds(Guid id, [FromBody] UpdateMemberId
         {
             return new NotFoundResult();
         }
-        await _groupRepository.UpdateUsersAsync(existingGroup.Id, model.MemberIds);
+        await _groupRepository.UpdateUsersAsync(existingGroup.Id, model.MemberIds, _timeProvider.GetUtcNow().UtcDateTime);
         return new OkResult();
     }
 

src/Core/AdminConsole/OrganizationFeatures/Groups/CreateGroupCommand.cs

@@ -17,16 +17,19 @@ public class CreateGroupCommand : ICreateGroupCommand
     private readonly IEventService _eventService;
     private readonly IGroupRepository _groupRepository;
     private readonly IOrganizationUserRepository _organizationUserRepository;
+    private readonly TimeProvider _timeProvider;
 
     public CreateGroupCommand(
         IEventService eventService,
         IGroupRepository groupRepository,
-        IOrganizationUserRepository organizationUserRepository
+        IOrganizationUserRepository organizationUserRepository,
+        TimeProvider timeProvider
         )
     {
         _eventService = eventService;
         _groupRepository = groupRepository;
         _organizationUserRepository = organizationUserRepository;
+        _timeProvider = timeProvider;
     }
 
     public async Task CreateGroupAsync(Group group, Organization organization,
@@ -61,7 +64,8 @@ public async Task CreateGroupAsync(Group group, Organization organization, Event
 
     private async Task GroupRepositoryCreateGroupAsync(Group group, Organization organization, IEnumerable<CollectionAccessSelection> collections = null)
     {
-        group.CreationDate = group.RevisionDate = DateTime.UtcNow;
+        var now = _timeProvider.GetUtcNow().UtcDateTime;
+        group.CreationDate = group.RevisionDate = now;
 
         if (collections == null)
         {
@@ -78,10 +82,10 @@ private async Task GroupRepositoryUpdateUsersAsync(Group group, IEnumerable<Guid
     {
         var usersToAddToGroup = userIds as Guid[] ?? userIds.ToArray();
 
-        await _groupRepository.UpdateUsersAsync(group.Id, usersToAddToGroup);
+        await _groupRepository.UpdateUsersAsync(group.Id, usersToAddToGroup, group.RevisionDate);
 
         var users = await _organizationUserRepository.GetManyAsync(usersToAddToGroup);
-        var eventDate = DateTime.UtcNow;
+        var eventDate = group.RevisionDate;
 
         if (systemUser.HasValue)
         {