From 12a821706fc5c3e18955e9692d9ebe486dcbb051 Mon Sep 17 00:00:00 2001 From: rmiccoli Date: Tue, 2 Jul 2024 17:06:24 +0200 Subject: [PATCH 1/8] Send an email when client status changes to client contacts --- .../DefaultClientManagementService.java | 8 +++++- .../iam/notification/NotificationFactory.java | 4 +++ .../TransientNotificationFactory.java | 27 +++++++++++++++++++ .../email-templates/clientStatusChanged.ftl | 5 ++++ .../infn/mw/iam/core/IamNotificationType.java | 2 +- 5 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 iam-login-service/src/main/resources/email-templates/clientStatusChanged.ftl diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java index 25c12c870..6a16a0fc7 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java @@ -55,6 +55,7 @@ import it.infn.mw.iam.audit.events.client.ClientStatusChangedEvent; import it.infn.mw.iam.audit.events.client.ClientUpdatedEvent; import it.infn.mw.iam.core.IamTokenService; +import it.infn.mw.iam.notification.NotificationFactory; import it.infn.mw.iam.persistence.model.IamAccount; import it.infn.mw.iam.persistence.model.IamAccountClient; import it.infn.mw.iam.persistence.repository.IamAccountRepository; @@ -72,11 +73,12 @@ public class DefaultClientManagementService implements ClientManagementService { private final OIDCTokenService oidcTokenService; private final IamTokenService tokenService; private final ApplicationEventPublisher eventPublisher; + private final NotificationFactory notificationFactory; public DefaultClientManagementService(Clock clock, ClientService clientService, ClientConverter converter, ClientDefaultsService defaultsService, UserConverter userConverter, IamAccountRepository accountRepo, OIDCTokenService oidcTokenService, - IamTokenService tokenService, ApplicationEventPublisher aep) { + IamTokenService tokenService, ApplicationEventPublisher aep, NotificationFactory notificationFactory) { this.clock = clock; this.clientService = clientService; this.converter = converter; @@ -86,6 +88,7 @@ public DefaultClientManagementService(Clock clock, ClientService clientService, this.oidcTokenService = oidcTokenService; this.tokenService = tokenService; this.eventPublisher = aep; + this.notificationFactory = notificationFactory; } @Override @@ -143,6 +146,9 @@ public void updateClientStatus(String clientId, boolean status, String userId) { client = clientService.updateClientStatus(client, status, userId); String message = "Client " + (status?"enabled":"disabled"); eventPublisher.publishEvent(new ClientStatusChangedEvent(this, client, message)); + if (!client.getContacts().isEmpty()) { + notificationFactory.createClientStatusChangedMessage(client); + } } @Validated(OnClientUpdate.class) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java index 035218299..efe81590a 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java @@ -17,6 +17,8 @@ import java.util.Optional; +import org.mitre.oauth2.model.ClientDetailsEntity; + import it.infn.mw.iam.persistence.model.IamAccount; import it.infn.mw.iam.persistence.model.IamEmailNotification; import it.infn.mw.iam.persistence.model.IamGroupRequest; @@ -39,4 +41,6 @@ public interface NotificationFactory { IamEmailNotification createGroupMembershipApprovedMessage(IamGroupRequest groupRequest); IamEmailNotification createGroupMembershipRejectedMessage(IamGroupRequest groupRequest); + + IamEmailNotification createClientStatusChangedMessage(ClientDetailsEntity client); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java index ea0b74e66..7bfaf7057 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java @@ -18,16 +18,20 @@ import static java.util.Arrays.asList; import java.io.IOException; +import java.util.ArrayList; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; import freemarker.template.Configuration; import freemarker.template.TemplateException; + +import org.mitre.oauth2.model.ClientDetailsEntity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -249,6 +253,29 @@ public IamEmailNotification createGroupMembershipRejectedMessage(IamGroupRequest return notification; } + @Override + public IamEmailNotification createClientStatusChangedMessage(ClientDetailsEntity client) { + Set recipients = client.getContacts(); + + Map model = new HashMap<>(); + model.put("clientId", client.getClientId()); + model.put("clientName", client.getClientName()); + model.put("clientStatus", client.isActive()); + model.put(ORGANISATION_NAME, organisationName); + + String subject = + String.format("Changed client status"); + + List contacts = new ArrayList(recipients); + + IamEmailNotification notification = + createMessage("clientStatusChanged.ftl", model, IamNotificationType.CLIENT_STATUS, + subject, contacts); + + LOG.debug("Updated client status. Client id {}, active {}", client.getClientId(), client.isActive()); + return notification; + } + protected IamEmailNotification createMessage(String templateName, Map model, IamNotificationType messageType, String subject, List receiverAddress) { diff --git a/iam-login-service/src/main/resources/email-templates/clientStatusChanged.ftl b/iam-login-service/src/main/resources/email-templates/clientStatusChanged.ftl new file mode 100644 index 000000000..91727dbc9 --- /dev/null +++ b/iam-login-service/src/main/resources/email-templates/clientStatusChanged.ftl @@ -0,0 +1,5 @@ +Dear user, + +this mail is to inform you that client ${clientName} with id ${clientId} has been <#if clientStatus>ACTIVATED<#else>SUSPENDED. + +The ${organisationName} registration service \ No newline at end of file diff --git a/iam-persistence/src/main/java/it/infn/mw/iam/core/IamNotificationType.java b/iam-persistence/src/main/java/it/infn/mw/iam/core/IamNotificationType.java index 971ae61be..16ec24c16 100644 --- a/iam-persistence/src/main/java/it/infn/mw/iam/core/IamNotificationType.java +++ b/iam-persistence/src/main/java/it/infn/mw/iam/core/IamNotificationType.java @@ -16,5 +16,5 @@ package it.infn.mw.iam.core; public enum IamNotificationType { - CONFIRMATION, RESETPASSWD, ACTIVATED, REJECTED, GROUP_MEMBERSHIP + CONFIRMATION, RESETPASSWD, ACTIVATED, REJECTED, GROUP_MEMBERSHIP, CLIENT_STATUS } From 7a64e673353762921a0403d253b1f1bd0986baa7 Mon Sep 17 00:00:00 2001 From: rmiccoli Date: Tue, 2 Jul 2024 17:41:42 +0200 Subject: [PATCH 2/8] Send an email to client owners too if they exist --- .../DefaultClientManagementService.java | 17 ++++++-- .../iam/notification/NotificationFactory.java | 3 ++ .../TransientNotificationFactory.java | 41 ++++++++++++++----- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java index 6a16a0fc7..13d9ad80c 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java @@ -23,6 +23,7 @@ import java.text.ParseException; import java.time.Clock; import java.util.Date; +import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -44,6 +45,7 @@ import it.infn.mw.iam.api.client.service.ClientService; import it.infn.mw.iam.api.client.util.ClientSuppliers; import it.infn.mw.iam.api.common.ListResponseDTO; +import it.infn.mw.iam.api.common.PagingUtils; import it.infn.mw.iam.api.common.client.RegisteredClientDTO; import it.infn.mw.iam.api.scim.converter.UserConverter; import it.infn.mw.iam.api.scim.model.ScimUser; @@ -78,7 +80,8 @@ public class DefaultClientManagementService implements ClientManagementService { public DefaultClientManagementService(Clock clock, ClientService clientService, ClientConverter converter, ClientDefaultsService defaultsService, UserConverter userConverter, IamAccountRepository accountRepo, OIDCTokenService oidcTokenService, - IamTokenService tokenService, ApplicationEventPublisher aep, NotificationFactory notificationFactory) { + IamTokenService tokenService, ApplicationEventPublisher aep, + NotificationFactory notificationFactory) { this.clock = clock; this.clientService = clientService; this.converter = converter; @@ -142,13 +145,21 @@ public void deleteClientByClientId(String clientId) { public void updateClientStatus(String clientId, boolean status, String userId) { ClientDetailsEntity client = clientService.findClientByClientId(clientId) - .orElseThrow(ClientSuppliers.clientNotFound(clientId)); + .orElseThrow(ClientSuppliers.clientNotFound(clientId)); client = clientService.updateClientStatus(client, status, userId); - String message = "Client " + (status?"enabled":"disabled"); + String message = "Client " + (status ? "enabled" : "disabled"); eventPublisher.publishEvent(new ClientStatusChangedEvent(this, client, message)); if (!client.getContacts().isEmpty()) { notificationFactory.createClientStatusChangedMessage(client); } + List owners = + getClientOwners(client.getClientId(), PagingUtils.buildUnpagedPageRequest()).getResources(); + if (!owners.isEmpty()) { + for (ScimUser owner : owners) { + notificationFactory.createClientStatusChangedMessageForClientOwners(client, + owner.getEmails().get(0).getValue()); + } + } } @Validated(OnClientUpdate.class) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java index efe81590a..a27817e9b 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java @@ -43,4 +43,7 @@ public interface NotificationFactory { IamEmailNotification createGroupMembershipRejectedMessage(IamGroupRequest groupRequest); IamEmailNotification createClientStatusChangedMessage(ClientDetailsEntity client); + + IamEmailNotification createClientStatusChangedMessageForClientOwners(ClientDetailsEntity client, + String email); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java index 7bfaf7057..c16a29cb9 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java @@ -71,7 +71,8 @@ public class TransientNotificationFactory implements NotificationFactory { private final Configuration freeMarkerConfiguration; @Autowired - public TransientNotificationFactory(Configuration fm, NotificationProperties np, AdminNotificationDeliveryStrategy ands, GroupManagerNotificationDeliveryStrategy gmds) { + public TransientNotificationFactory(Configuration fm, NotificationProperties np, + AdminNotificationDeliveryStrategy ands, GroupManagerNotificationDeliveryStrategy gmds) { this.freeMarkerConfiguration = fm; this.properties = np; this.adminNotificationDeliveryStrategy = ands; @@ -203,7 +204,8 @@ public IamEmailNotification createAdminHandleGroupRequestMessage(IamGroupRequest LOG.debug("Create group membership admin notification for request {}", groupRequest.getUuid()); return createMessage("adminHandleGroupRequest.ftl", model, IamNotificationType.GROUP_MEMBERSHIP, - subject, groupManagerDeliveryStrategy.resolveGroupManagersEmailAddresses(groupRequest.getGroup())); + subject, + groupManagerDeliveryStrategy.resolveGroupManagersEmailAddresses(groupRequest.getGroup())); } @Override @@ -263,16 +265,35 @@ public IamEmailNotification createClientStatusChangedMessage(ClientDetailsEntity model.put("clientStatus", client.isActive()); model.put(ORGANISATION_NAME, organisationName); - String subject = - String.format("Changed client status"); + String subject = String.format("Changed client status"); List contacts = new ArrayList(recipients); - IamEmailNotification notification = - createMessage("clientStatusChanged.ftl", model, IamNotificationType.CLIENT_STATUS, - subject, contacts); + IamEmailNotification notification = createMessage("clientStatusChanged.ftl", model, + IamNotificationType.CLIENT_STATUS, subject, contacts); + + LOG.debug("Updated client status. Client id {}, active {}", client.getClientId(), + client.isActive()); + return notification; + } + + @Override + public IamEmailNotification createClientStatusChangedMessageForClientOwners( + ClientDetailsEntity client, String email) { + + Map model = new HashMap<>(); + model.put("clientId", client.getClientId()); + model.put("clientName", client.getClientName()); + model.put("clientStatus", client.isActive()); + model.put(ORGANISATION_NAME, organisationName); + + String subject = String.format("Changed client status"); + + IamEmailNotification notification = createMessage("clientStatusChanged.ftl", model, + IamNotificationType.CLIENT_STATUS, subject, asList(email)); - LOG.debug("Updated client status. Client id {}, active {}", client.getClientId(), client.isActive()); + LOG.debug("Updated client status. Client id {}, active {}", client.getClientId(), + client.isActive()); return notification; } @@ -292,8 +313,8 @@ protected IamEmailNotification createMessage(String templateName, Map IamNotificationReceiver.forAddress(message, a)) - .collect(Collectors.toList())); + .map(a -> IamNotificationReceiver.forAddress(message, a)) + .collect(Collectors.toList())); return message; } catch (IOException | TemplateException e) { From 9c0d7e00f89437cceb684f572ba137cdaaa53331 Mon Sep 17 00:00:00 2001 From: rmiccoli Date: Tue, 2 Jul 2024 17:57:43 +0200 Subject: [PATCH 3/8] Fix code smell --- .../mw/iam/notification/TransientNotificationFactory.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java index c16a29cb9..4d6d62d40 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java @@ -265,9 +265,9 @@ public IamEmailNotification createClientStatusChangedMessage(ClientDetailsEntity model.put("clientStatus", client.isActive()); model.put(ORGANISATION_NAME, organisationName); - String subject = String.format("Changed client status"); + String subject = "Changed client status"; - List contacts = new ArrayList(recipients); + List contacts = new ArrayList<>(recipients); IamEmailNotification notification = createMessage("clientStatusChanged.ftl", model, IamNotificationType.CLIENT_STATUS, subject, contacts); @@ -287,7 +287,7 @@ public IamEmailNotification createClientStatusChangedMessageForClientOwners( model.put("clientStatus", client.isActive()); model.put(ORGANISATION_NAME, organisationName); - String subject = String.format("Changed client status"); + String subject = "Changed client status"; IamEmailNotification notification = createMessage("clientStatusChanged.ftl", model, IamNotificationType.CLIENT_STATUS, subject, asList(email)); From c62a1966cafa7f623bae0328ef157dae963d58f7 Mon Sep 17 00:00:00 2001 From: rmiccoli Date: Wed, 3 Jul 2024 10:24:54 +0200 Subject: [PATCH 4/8] Try to improve the coverage --- .../client/ClientManagementServiceTests.java | 41 +++++++++++++++---- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java index 1711e80da..87867bde0 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java @@ -37,6 +37,8 @@ import java.text.ParseException; import java.time.Clock; import java.util.Date; +import java.util.HashSet; +import java.util.Set; import javax.validation.ConstraintViolationException; @@ -105,7 +107,7 @@ public void testPagedClientLookup() { Sort sort = Sort.by(Direction.ASC, "clientId"); Pageable pageable = PagingUtils.buildPageRequest(10, 1, 100, sort); - + ListResponseDTO clients = managementService.retrieveAllClients(pageable); assertThat(clients.getTotalResults(), is(19L)); @@ -249,8 +251,7 @@ public void testBasicClientValidation() { } @Test - public void testDynamicallyRegisteredClientCanBeUpdated() - throws ParseException { + public void testDynamicallyRegisteredClientCanBeUpdated() throws ParseException { userAuth = Mockito.mock(UsernamePasswordAuthenticationToken.class); when(userAuth.getName()).thenReturn("test"); @@ -261,7 +262,7 @@ public void testDynamicallyRegisteredClientCanBeUpdated() request.setGrantTypes(Sets.newHashSet(AuthorizationGrantType.CLIENT_CREDENTIALS)); RegisteredClientDTO response = registrationService.registerClient(request, userAuth); - + String clientId = response.getClientId(); ClientDetailsEntity entity = clientService.findClientByClientId(clientId).orElseThrow(); assertThat(entity.isDynamicallyRegistered(), is(true)); @@ -296,8 +297,7 @@ public void testSecretRotation() throws ParseException { RegisteredClientDTO updatedClient = managementService.retrieveClientByClientId(client.getClientId()).orElseThrow(); - assertThat(updatedClient.getClientSecret(), - not(equalTo(savedClient.getClientSecret()))); + assertThat(updatedClient.getClientSecret(), not(equalTo(savedClient.getClientSecret()))); } @Test @@ -333,9 +333,8 @@ public void testClientOwnerAssignRemove() throws ParseException { RegisteredClientDTO savedClient = managementService.saveNewClient(client); assertThat(savedClient.getClientId(), is(client.getClientId())); assertThat(savedClient.getClientSecret(), notNullValue()); - - ListResponseDTO owners = - managementService.getClientOwners(savedClient.getClientId(), + + ListResponseDTO owners = managementService.getClientOwners(savedClient.getClientId(), PagingUtils.buildUnpagedPageRequest()); assertThat(owners.getTotalResults(), is(0L)); @@ -417,4 +416,28 @@ public void testClientStatusChange() { assertTrue(client.getStatusChangedOn().equals(Date.from(clock.instant()))); assertEquals("userUUID", client.getStatusChangedBy()); } + + @Test + public void testClientStatusChangeWithContacts() throws ParseException { + managementService.updateClientStatus("client", false, "userUUID"); + RegisteredClientDTO client = managementService.retrieveClientByClientId("client").get(); + Set contacts = new HashSet(); + contacts.add("test@example.com"); + client.setContacts(contacts); + managementService.updateClient(client.getClientId(), client); + + assertFalse(client.isActive()); + assertTrue(client.getStatusChangedOn().equals(Date.from(clock.instant()))); + assertEquals("userUUID", client.getStatusChangedBy()); + } + + @Test + public void testClientStatusChangeWithoutOwners() { + managementService.updateClientStatus("client-cred", false, "userUUID"); + RegisteredClientDTO client = managementService.retrieveClientByClientId("client-cred").get(); + + assertFalse(client.isActive()); + assertTrue(client.getStatusChangedOn().equals(Date.from(clock.instant()))); + assertEquals("userUUID", client.getStatusChangedBy()); + } } From 63ab10da362d8f4bacf50839c80e8ed6f1fc02ad Mon Sep 17 00:00:00 2001 From: rmiccoli Date: Wed, 3 Jul 2024 11:23:35 +0200 Subject: [PATCH 5/8] Improve coverage --- .../test/service/client/ClientManagementServiceTests.java | 8 ++------ .../resources/db/migration/test/V100000___test_data.sql | 3 +++ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java index 87867bde0..2e552ea77 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java @@ -419,12 +419,8 @@ public void testClientStatusChange() { @Test public void testClientStatusChangeWithContacts() throws ParseException { - managementService.updateClientStatus("client", false, "userUUID"); - RegisteredClientDTO client = managementService.retrieveClientByClientId("client").get(); - Set contacts = new HashSet(); - contacts.add("test@example.com"); - client.setContacts(contacts); - managementService.updateClient(client.getClientId(), client); + managementService.updateClientStatus("device-code-client", false, "userUUID"); + RegisteredClientDTO client = managementService.retrieveClientByClientId("device-code-client").get(); assertFalse(client.isActive()); assertTrue(client.getStatusChangedOn().equals(Date.from(clock.instant()))); diff --git a/iam-persistence/src/main/resources/db/migration/test/V100000___test_data.sql b/iam-persistence/src/main/resources/db/migration/test/V100000___test_data.sql index 468928784..0d2b46151 100644 --- a/iam-persistence/src/main/resources/db/migration/test/V100000___test_data.sql +++ b/iam-persistence/src/main/resources/db/migration/test/V100000___test_data.sql @@ -186,6 +186,9 @@ INSERT INTO client_grant_type (owner_id, grant_type) VALUES (18, 'urn:ietf:params:oauth:grant-type:device_code'), (18, 'authorization_code'), (19, 'client_credentials'); + +INSERT INTO client_contact (owner_id, contact) VALUES + (12, 'test@example.com'); INSERT INTO iam_user_info(ID, GIVENNAME, FAMILYNAME, EMAIL, EMAILVERIFIED, BIRTHDATE, GENDER, NICKNAME) VALUES (2, 'Test', 'User', 'test@iam.test', true, '1950-01-01','M','test'), From 04a6db427fce418456e8b2b6cff88894ab0780c0 Mon Sep 17 00:00:00 2001 From: rmiccoli Date: Wed, 3 Jul 2024 11:34:55 +0200 Subject: [PATCH 6/8] Fix code smell --- .../iam/test/service/client/ClientManagementServiceTests.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java index 2e552ea77..b9f8df669 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/service/client/ClientManagementServiceTests.java @@ -37,8 +37,6 @@ import java.text.ParseException; import java.time.Clock; import java.util.Date; -import java.util.HashSet; -import java.util.Set; import javax.validation.ConstraintViolationException; @@ -418,7 +416,7 @@ public void testClientStatusChange() { } @Test - public void testClientStatusChangeWithContacts() throws ParseException { + public void testClientStatusChangeWithContacts() { managementService.updateClientStatus("device-code-client", false, "userUUID"); RegisteredClientDTO client = managementService.retrieveClientByClientId("device-code-client").get(); From c89bbeed8034950d7be991f385ae843bc50a6961 Mon Sep 17 00:00:00 2001 From: rmiccoli Date: Fri, 12 Jul 2024 12:12:20 +0200 Subject: [PATCH 7/8] Fix requested changes --- .../DefaultClientManagementService.java | 20 ++++----- .../iam/notification/NotificationFactory.java | 6 +-- .../TransientNotificationFactory.java | 43 +++++++------------ .../email-templates/clientStatusChanged.ftl | 2 +- 4 files changed, 29 insertions(+), 42 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java index 13d9ad80c..1055aca9c 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/management/service/DefaultClientManagementService.java @@ -149,17 +149,15 @@ public void updateClientStatus(String clientId, boolean status, String userId) { client = clientService.updateClientStatus(client, status, userId); String message = "Client " + (status ? "enabled" : "disabled"); eventPublisher.publishEvent(new ClientStatusChangedEvent(this, client, message)); - if (!client.getContacts().isEmpty()) { - notificationFactory.createClientStatusChangedMessage(client); - } - List owners = - getClientOwners(client.getClientId(), PagingUtils.buildUnpagedPageRequest()).getResources(); - if (!owners.isEmpty()) { - for (ScimUser owner : owners) { - notificationFactory.createClientStatusChangedMessageForClientOwners(client, - owner.getEmails().get(0).getValue()); - } - } + notificationFactory.createClientStatusChangedMessageFor(client, getClientOwners(clientId)); + } + + private List getClientOwners(String clientId) { + return clientService.findClientOwners(clientId, PagingUtils.buildUnpagedPageRequest()) + .getContent() + .stream() + .map(IamAccountClient::getAccount) + .collect(Collectors.toList()); } @Validated(OnClientUpdate.class) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java index a27817e9b..7d2536a11 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/NotificationFactory.java @@ -15,6 +15,7 @@ */ package it.infn.mw.iam.notification; +import java.util.List; import java.util.Optional; import org.mitre.oauth2.model.ClientDetailsEntity; @@ -42,8 +43,7 @@ public interface NotificationFactory { IamEmailNotification createGroupMembershipRejectedMessage(IamGroupRequest groupRequest); - IamEmailNotification createClientStatusChangedMessage(ClientDetailsEntity client); + IamEmailNotification createClientStatusChangedMessageFor(ClientDetailsEntity client, + List accounts); - IamEmailNotification createClientStatusChangedMessageForClientOwners(ClientDetailsEntity client, - String email); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java index 4d6d62d40..289c4d0fa 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java @@ -28,15 +28,16 @@ import java.util.UUID; import java.util.stream.Collectors; -import freemarker.template.Configuration; -import freemarker.template.TemplateException; - import org.mitre.oauth2.model.ClientDetailsEntity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.ui.freemarker.FreeMarkerTemplateUtils; +import freemarker.template.Configuration; +import freemarker.template.Template; +import freemarker.template.TemplateException; import it.infn.mw.iam.api.account.password_reset.PasswordResetController; import it.infn.mw.iam.core.IamDeliveryStatus; import it.infn.mw.iam.core.IamNotificationType; @@ -47,8 +48,6 @@ import it.infn.mw.iam.persistence.model.IamGroupRequest; import it.infn.mw.iam.persistence.model.IamNotificationReceiver; import it.infn.mw.iam.persistence.model.IamRegistrationRequest; -import org.springframework.ui.freemarker.FreeMarkerTemplateUtils; -import freemarker.template.Template; public class TransientNotificationFactory implements NotificationFactory { @@ -256,41 +255,31 @@ public IamEmailNotification createGroupMembershipRejectedMessage(IamGroupRequest } @Override - public IamEmailNotification createClientStatusChangedMessage(ClientDetailsEntity client) { + public IamEmailNotification createClientStatusChangedMessageFor(ClientDetailsEntity client, + List accounts) { Set recipients = client.getContacts(); Map model = new HashMap<>(); model.put("clientId", client.getClientId()); model.put("clientName", client.getClientName()); - model.put("clientStatus", client.isActive()); + model.put("isClientActive", client.isActive()); model.put(ORGANISATION_NAME, organisationName); String subject = "Changed client status"; - List contacts = new ArrayList<>(recipients); - - IamEmailNotification notification = createMessage("clientStatusChanged.ftl", model, - IamNotificationType.CLIENT_STATUS, subject, contacts); - - LOG.debug("Updated client status. Client id {}, active {}", client.getClientId(), - client.isActive()); - return notification; - } - - @Override - public IamEmailNotification createClientStatusChangedMessageForClientOwners( - ClientDetailsEntity client, String email) { + for (IamAccount a : accounts) { + recipients.add(a.getUserInfo().getEmail()); + } - Map model = new HashMap<>(); - model.put("clientId", client.getClientId()); - model.put("clientName", client.getClientName()); - model.put("clientStatus", client.isActive()); - model.put(ORGANISATION_NAME, organisationName); + List emails = new ArrayList<>(recipients); - String subject = "Changed client status"; + if (emails.isEmpty()) { + LOG.warn("No emails to send notification to for client id {}", client.getClientId()); + return null; + } IamEmailNotification notification = createMessage("clientStatusChanged.ftl", model, - IamNotificationType.CLIENT_STATUS, subject, asList(email)); + IamNotificationType.CLIENT_STATUS, subject, emails); LOG.debug("Updated client status. Client id {}, active {}", client.getClientId(), client.isActive()); diff --git a/iam-login-service/src/main/resources/email-templates/clientStatusChanged.ftl b/iam-login-service/src/main/resources/email-templates/clientStatusChanged.ftl index 91727dbc9..995e42329 100644 --- a/iam-login-service/src/main/resources/email-templates/clientStatusChanged.ftl +++ b/iam-login-service/src/main/resources/email-templates/clientStatusChanged.ftl @@ -1,5 +1,5 @@ Dear user, -this mail is to inform you that client ${clientName} with id ${clientId} has been <#if clientStatus>ACTIVATED<#else>SUSPENDED. +this mail is to inform you that client ${clientName} with id ${clientId} has been <#if isClientActive>ACTIVATED<#else>SUSPENDED. The ${organisationName} registration service \ No newline at end of file From 46534a0b8ee06cd9639f29391b9a71ab2a19cf81 Mon Sep 17 00:00:00 2001 From: rmiccoli Date: Tue, 16 Jul 2024 17:53:47 +0200 Subject: [PATCH 8/8] Fix log message --- .../infn/mw/iam/notification/TransientNotificationFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java index 289c4d0fa..b25dcec60 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/notification/TransientNotificationFactory.java @@ -274,7 +274,7 @@ public IamEmailNotification createClientStatusChangedMessageFor(ClientDetailsEnt List emails = new ArrayList<>(recipients); if (emails.isEmpty()) { - LOG.warn("No emails to send notification to for client id {}", client.getClientId()); + LOG.warn("No email to send notification to for client {}", client.getClientId()); return null; }