From 3264f4eaa648b84e63e24f945c2fc7164d422a99 Mon Sep 17 00:00:00 2001 From: Patrick Hobusch Date: Tue, 12 May 2026 17:43:14 +0800 Subject: [PATCH] Resolve effective name from map key in all bulk set services When a model's name field is null in a map-based request, fall back to the map key as the effective name. Also validate null models and throw BadRequestException with a descriptive message. Applies to groups, directories, application links, and applications. Covered by new unit tests for both cases in each service. --- .../service/AbstractDirectoriesService.java | 16 +++++-- .../DefaultApplicationLinksServiceImpl.java | 9 ++++ .../DefaultApplicationLinkServiceTest.java | 27 +++++++++++ .../service/ApplicationsServiceImpl.java | 12 ++++- .../crowd/service/GroupsServiceImpl.java | 46 +++++++++++++++---- .../service/ApplicationsServiceTest.java | 23 ++++++++++ .../crowd/service/DirectoriesServiceTest.java | 29 ++++++++++++ .../crowd/service/GroupsServiceTest.java | 35 +++++++++++++- 8 files changed, 183 insertions(+), 14 deletions(-) diff --git a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractDirectoriesService.java b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractDirectoriesService.java index 49a07cf0..373b0ec0 100644 --- a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractDirectoriesService.java +++ b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/AbstractDirectoriesService.java @@ -22,20 +22,30 @@ public abstract class AbstractDirectoriesService implements DirectoriesService { final Map resultDirectories = new LinkedHashMap<>(); for (Map.Entry directoryModelEntry : directoryModels.entrySet()) { + final String directoryName = directoryModelEntry.getKey(); final AbstractDirectoryModel directoryModel = directoryModelEntry.getValue(); + if (directoryModel == null) { + throw new BadRequestException(String.format( + "Cannot set directory '%s', a full directory model is required", directoryName)); + } + + if (directoryModel.getName() == null) { + directoryModel.setName(directoryName); + } + // Check if directoryModel is not an instance of any supported class if (getSupportedClassesForUpdate().stream().noneMatch(clazz -> clazz.isInstance(directoryModel))) { throw new BadRequestException(String.format("Updating directory type '%s' is not supported (yet)", directoryModel.getClass())); } - final AbstractDirectoryModel existingDirectoryModel = existingDirectoriesByName.get(directoryModelEntry.getKey()); + final AbstractDirectoryModel existingDirectoryModel = existingDirectoriesByName.get(directoryName); final AbstractDirectoryModel resultDirectoryModel; if (existingDirectoryModel != null) { - resultDirectoryModel = setDirectory(existingDirectoryModel.getId(), directoryModelEntry.getValue()); + resultDirectoryModel = setDirectory(existingDirectoryModel.getId(), directoryModel); } else { - resultDirectoryModel = addDirectory(directoryModelEntry.getValue()); + resultDirectoryModel = addDirectory(directoryModel); } resultDirectories.put(resultDirectoryModel.getName(), resultDirectoryModel); diff --git a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinksServiceImpl.java b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinksServiceImpl.java index e5eb1e69..8606ba83 100644 --- a/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinksServiceImpl.java +++ b/commons/src/main/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinksServiceImpl.java @@ -106,6 +106,15 @@ public Map setApplicationLinks( final String name = applicationLinkModelEntry.getKey(); final ApplicationLinkModel applicationLinkModel = applicationLinkModelEntry.getValue(); + if (applicationLinkModel == null) { + throw new BadRequestException(String.format( + "Cannot set application link '%s', a full application link model is required", name)); + } + + if (applicationLinkModel.getName() == null) { + applicationLinkModel.setName(name); + } + if (linkModelMap.containsKey(name)) { setApplicationLink(linkModelMap.get(name).getUuid(), applicationLinkModel); } else { diff --git a/commons/src/test/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinkServiceTest.java b/commons/src/test/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinkServiceTest.java index 407dcc61..7b9cfef2 100644 --- a/commons/src/test/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinkServiceTest.java +++ b/commons/src/test/java/com/deftdevs/bootstrapi/commons/service/DefaultApplicationLinkServiceTest.java @@ -108,6 +108,33 @@ void testGetApplicationLink() throws URISyntaxException, NoAccessException, NoSu assertEquals(applicationLinkModel, appLinkResponse); } + @Test + void testSetApplicationLinksNullModel() { + final Map applicationLinkModels = Collections.singletonMap("test", null); + assertThrows(BadRequestException.class, () -> applicationLinkService.setApplicationLinks(applicationLinkModels)); + } + + @Test + void testSetApplicationLinksNullNameUsesMapKey() + throws URISyntaxException, NoAccessException, NoSuchApplinkException, TypeNotInstalledException { + + final ApplicationLink applicationLink = createApplicationLink(); + final ApplicationLinkModel applicationLinkModel = createApplicationLinkModel(); + final String mapKey = applicationLinkModel.getName(); + applicationLinkModel.setName(null); + final Map applicationLinkModels = Collections.singletonMap(mapKey, applicationLinkModel); + doReturn(Collections.singletonList(applicationLink)).when(mutatingApplicationLinkService).getApplicationLinks(); + doReturn(applicationLink).when(mutatingApplicationLinkService).getApplicationLink(any()); + doReturn(applicationLink).when(mutatingApplicationLinkService).addApplicationLink(any(), any(), any()); + doReturn(new DefaultApplicationType()).when(typeAccessor).getApplicationType(any()); + doReturn(OAuthConfig.createDisabledConfig()).when(applicationLinksAuthConfigHelper).getOutgoingOAuthConfig(any()); + doReturn(OAuthConfig.createDisabledConfig()).when(applicationLinksAuthConfigHelper).getIncomingOAuthConfig(any()); + doReturn(createApplinkStatus(applicationLink, AVAILABLE)).when(applinkStatusService).getApplinkStatus(any()); + + applicationLinkService.setApplicationLinks(applicationLinkModels); + assertEquals(mapKey, applicationLinkModel.getName()); + } + @Test void testSetApplicationLinks() throws URISyntaxException, NoAccessException, NoSuchApplinkException, TypeNotInstalledException { diff --git a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceImpl.java b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceImpl.java index 44307f76..dcd0aae8 100644 --- a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceImpl.java +++ b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceImpl.java @@ -64,10 +64,20 @@ public Map setApplications( final Map resultApplicationModels = new LinkedHashMap<>(); for (Map.Entry applicationModelEntry : applicationModels.entrySet()) { + final String applicationName = applicationModelEntry.getKey(); final ApplicationModel applicationModel = applicationModelEntry.getValue(); + if (applicationModel == null) { + throw new BadRequestException(String.format( + "Cannot set application '%s', a full application model is required", applicationName)); + } + + if (applicationModel.getName() == null) { + applicationModel.setName(applicationName); + } + try { - final Application application = applicationManager.findByName(applicationModelEntry.getKey()); + final Application application = applicationManager.findByName(applicationName); final ApplicationModel updatedApplicationModel = setApplication(application.getId(), applicationModel); resultApplicationModels.put(updatedApplicationModel.getName(), updatedApplicationModel); } catch (ApplicationNotFoundException ignored) { diff --git a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceImpl.java b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceImpl.java index 2212aff7..32f9817a 100644 --- a/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceImpl.java +++ b/crowd/src/main/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceImpl.java @@ -49,17 +49,35 @@ public GroupModel createGroup( final long directoryId, final GroupModel groupModel) { - if (groupModel.getName() == null) { + if (groupModel == null) { + throw new BadRequestException("Cannot create group, a full group model is required"); + } + + return createGroup(directoryId, groupModel.getName(), groupModel); + } + + GroupModel createGroup( + final long directoryId, + final String groupName, + final GroupModel groupModel) { + + final String effectiveGroupName = groupModel.getName() != null ? groupModel.getName() : groupName; + + if (effectiveGroupName == null) { throw new BadRequestException("Cannot create group, group name is required"); } - final Group existingGroup = findGroup(directoryId, groupModel.getName()); + if (groupName != null && !effectiveGroupName.equals(groupName)) { + throw new BadRequestException("Cannot create group, two different group names provided"); + } + + final Group existingGroup = findGroup(directoryId, effectiveGroupName); if (existingGroup != null) { - throw new BadRequestException(String.format("Group '%s' already exists", groupModel.getName())); + throw new BadRequestException(String.format("Group '%s' already exists", effectiveGroupName)); } - final GroupTemplate groupTemplate = new GroupTemplate(groupModel.getName(), directoryId); + final GroupTemplate groupTemplate = new GroupTemplate(effectiveGroupName, directoryId); groupTemplate.setDescription(groupModel.getDescription()); groupTemplate.setActive(groupModel.getActive() == null || groupModel.getActive()); @@ -67,10 +85,10 @@ public GroupModel createGroup( return GroupModelUtil.toGroupModel(directoryManager.addGroup(directoryId, groupTemplate)); } catch (DirectoryPermissionException | InvalidGroupException e) { // A permission exception should only happen if we try adding the group - // a user in a read-only directory, so treat this as a bad request + // in a read-only directory, so treat this as a bad request throw new BadRequestException(e); } catch (com.atlassian.crowd.exception.DirectoryNotFoundException | OperationFailedException e) { - // At this point, we know the group exists, thus directory not found + // At this point, we know the directory exists, thus directory not found // should never happen, so if it does, treat it as an internal server error throw new InternalServerErrorException(e); } @@ -111,7 +129,7 @@ public GroupModel setGroup( if (groupModel == null) { throw new GroupNotFoundException(groupName); } - return createGroup(directoryId, groupModel); + return createGroup(directoryId, groupName, groupModel); } if (groupModel == null) { @@ -132,7 +150,19 @@ public Map setGroups( final Map resultGroupModels = new LinkedHashMap<>(); for (Map.Entry entry : groupModels.entrySet()) { - final GroupModel resultGroupModel = setGroup(directoryId, entry.getKey(), entry.getValue()); + final String groupName = entry.getKey(); + final GroupModel groupModel = entry.getValue(); + + if (groupModel == null) { + throw new BadRequestException(String.format( + "Cannot set group '%s', a full group model is required", groupName)); + } + + if (groupModel.getName() == null) { + groupModel.setName(groupName); + } + + final GroupModel resultGroupModel = setGroup(directoryId, groupName, groupModel); resultGroupModels.put(resultGroupModel.getName(), resultGroupModel); } return resultGroupModels; diff --git a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceTest.java b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceTest.java index 3f57b836..818a0cd1 100644 --- a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceTest.java +++ b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/ApplicationsServiceTest.java @@ -197,6 +197,29 @@ public void testSetApplicationEnsurePersistenceCalls() throws ApplicationNotFoun verify(spy).persistApplicationModelAutoAssignmentGroups(any(), any()); } + @Test + public void testSetApplicationsNullModel() { + final Map applicationModels = Collections.singletonMap("test", null); + assertThrows(BadRequestException.class, () -> applicationsService.setApplications(applicationModels)); + } + + @Test + public void testSetApplicationsNullNameUsesMapKey() throws ApplicationNotFoundException { + final String mapKey = EXAMPLE_2.getName(); + final ApplicationModel modelWithoutName = ApplicationModel.builder() + .type(EXAMPLE_2.getType()) + .build(); + final Map applicationModels = Collections.singletonMap(mapKey, modelWithoutName); + doThrow(new ApplicationNotFoundException("")).when(applicationManager).findByName(mapKey); + + final ApplicationsServiceImpl spy = spy(applicationsService); + doReturn(EXAMPLE_2).when(spy).addApplication(modelWithoutName); + + spy.setApplications(applicationModels); + assertEquals(mapKey, modelWithoutName.getName()); + verify(spy).addApplication(modelWithoutName); + } + @Test public void testSetApplications() throws ApplicationNotFoundException { final Application applicationExample1 = ImmutableApplication.builder(toApplication(EXAMPLE_1)) diff --git a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/DirectoriesServiceTest.java b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/DirectoriesServiceTest.java index 94fa7f75..6ebdafaf 100644 --- a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/DirectoriesServiceTest.java +++ b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/DirectoriesServiceTest.java @@ -82,6 +82,35 @@ public void testSetDirectoriesAddNew() { verify(spy).addDirectory(directoryModel); } + @Test + public void testSetDirectoriesNullModel() { + final Directory directoryInternal = getTestDirectoryInternal(); + final DirectoriesServiceImpl spy = spy(directoriesService); + doReturn(Collections.singletonList(directoryInternal)).when(spy).findAllDirectories(); + + final Map directoryModels = new LinkedHashMap<>(); + directoryModels.put("other", null); + assertThrows(BadRequestException.class, () -> spy.setDirectories(directoryModels)); + } + + @Test + public void testSetDirectoriesNullNameUsesMapKey() { + final Directory directoryInternal = getTestDirectoryInternal(); + final Directory directoryInternalNew = getTestDirectoryInternalOther(); + final DirectoriesServiceImpl spy = spy(directoriesService); + doReturn(Collections.singletonList(directoryInternal)).when(spy).findAllDirectories(); + + final AbstractDirectoryModel directoryModel = DirectoryModelUtil.toDirectoryModel(directoryInternalNew); + final String mapKey = directoryModel.getName(); + directoryModel.setName(null); + final Map directoryModels = Collections.singletonMap(mapKey, directoryModel); + doReturn(directoryModel).when(spy).addDirectory(any()); + + spy.setDirectories(directoryModels); + assertEquals(mapKey, directoryModel.getName()); + verify(spy).addDirectory(directoryModel); + } + @Test public void testSetDirectoriesAddNewUnsupportedType() { final Directory directoryInternal = getTestDirectoryInternal(); diff --git a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceTest.java b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceTest.java index dfddafb7..88b78a6f 100644 --- a/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceTest.java +++ b/crowd/src/test/java/com/deftdevs/bootstrapi/crowd/service/GroupsServiceTest.java @@ -107,6 +107,13 @@ public void testCreateGroupNoName() throws DirectoryNotFoundException, Operation }); } + @Test + public void testCreateGroupNullModel() { + assertThrows(BadRequestException.class, () -> { + groupsService.createGroup(0L, null); + }); + } + @Test public void testUpdateGroup() throws Exception { final Group group = getTestGroup(); @@ -157,10 +164,10 @@ public void testSetGroupAddNew() { final Group group = getTestGroup(); final GroupModel groupModel = GroupModel.EXAMPLE_1; final GroupsServiceImpl spy = spy(groupsService); - doReturn(groupModel).when(spy).createGroup(anyLong(), any()); + doReturn(groupModel).when(spy).createGroup(anyLong(), anyString(), any()); spy.setGroup(group.getDirectoryId(), groupModel.getName(), groupModel); - verify(spy).createGroup(anyLong(), any()); + verify(spy).createGroup(anyLong(), anyString(), any()); } @Test @@ -190,6 +197,30 @@ public void testSetGroups() { verify(spy, times(groupModels.size())).setGroup(anyLong(), any(), any()); } + @Test + public void testSetGroupsNullModel() { + final Map groupModels = new LinkedHashMap<>(); + groupModels.put("someGroup", null); + assertThrows(BadRequestException.class, () -> groupsService.setGroups(0L, groupModels)); + } + + @Test + public void testSetGroupsNullNameUsesMapKey() { + final String mapKey = GroupModel.EXAMPLE_1.getName(); + final GroupModel modelWithoutName = GroupModel.builder() + .active(GroupModel.EXAMPLE_1.getActive()) + .description(GroupModel.EXAMPLE_1.getDescription()) + .build(); + final Map groupModels = Collections.singletonMap(mapKey, modelWithoutName); + + final GroupsServiceImpl spy = spy(groupsService); + doReturn(modelWithoutName).when(spy).setGroup(anyLong(), anyString(), any()); + + spy.setGroups(0L, groupModels); + assertEquals(mapKey, modelWithoutName.getName()); + verify(spy).setGroup(anyLong(), eq(mapKey), eq(modelWithoutName)); + } + @Test public void testSetGroupNullModelAddNew() { final Group group = getTestGroup();