Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send an email when client status changes #802

Merged
merged 8 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -55,6 +57,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;
Expand All @@ -72,11 +75,13 @@ 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;
Expand All @@ -86,6 +91,7 @@ public DefaultClientManagementService(Clock clock, ClientService clientService,
this.oidcTokenService = oidcTokenService;
this.tokenService = tokenService;
this.eventPublisher = aep;
this.notificationFactory = notificationFactory;
}

@Override
Expand Down Expand Up @@ -139,10 +145,19 @@ 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));
notificationFactory.createClientStatusChangedMessageFor(client, getClientOwners(clientId));
}

private List<IamAccount> getClientOwners(String clientId) {
return clientService.findClientOwners(clientId, PagingUtils.buildUnpagedPageRequest())
.getContent()
.stream()
.map(IamAccountClient::getAccount)
.collect(Collectors.toList());
}

@Validated(OnClientUpdate.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
*/
package it.infn.mw.iam.notification;

import java.util.List;
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;
Expand All @@ -39,4 +42,8 @@ public interface NotificationFactory {
IamEmailNotification createGroupMembershipApprovedMessage(IamGroupRequest groupRequest);

IamEmailNotification createGroupMembershipRejectedMessage(IamGroupRequest groupRequest);

IamEmailNotification createClientStatusChangedMessageFor(ClientDetailsEntity client,
List<IamAccount> accounts);

rmiccoli marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,26 @@
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;
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;
Expand All @@ -43,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 {

Expand All @@ -67,7 +70,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;
Expand Down Expand Up @@ -199,7 +203,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
Expand Down Expand Up @@ -249,6 +254,38 @@ public IamEmailNotification createGroupMembershipRejectedMessage(IamGroupRequest
return notification;
}

@Override
public IamEmailNotification createClientStatusChangedMessageFor(ClientDetailsEntity client,
List<IamAccount> accounts) {
Set<String> recipients = client.getContacts();

Map<String, Object> model = new HashMap<>();
model.put("clientId", client.getClientId());
model.put("clientName", client.getClientName());
model.put("isClientActive", client.isActive());
model.put(ORGANISATION_NAME, organisationName);

String subject = "Changed client status";

for (IamAccount a : accounts) {
recipients.add(a.getUserInfo().getEmail());
}

List<String> emails = new ArrayList<>(recipients);

if (emails.isEmpty()) {
LOG.warn("No email to send notification to for client {}", client.getClientId());
return null;
}

IamEmailNotification notification = createMessage("clientStatusChanged.ftl", model,
IamNotificationType.CLIENT_STATUS, subject, emails);

LOG.debug("Updated client status. Client id {}, active {}", client.getClientId(),
client.isActive());
return notification;
}

protected IamEmailNotification createMessage(String templateName, Map<String, Object> model,
IamNotificationType messageType, String subject, List<String> receiverAddress) {

Expand All @@ -265,8 +302,8 @@ protected IamEmailNotification createMessage(String templateName, Map<String, Ob
message.setCreationTime(new Date());
message.setDeliveryStatus(IamDeliveryStatus.PENDING);
message.setReceivers(receiverAddress.stream()
.map(a -> IamNotificationReceiver.forAddress(message, a))
.collect(Collectors.toList()));
.map(a -> IamNotificationReceiver.forAddress(message, a))
.collect(Collectors.toList()));

return message;
} catch (IOException | TemplateException e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Dear user,

this mail is to inform you that client ${clientName} with id ${clientId} has been <#if isClientActive>ACTIVATED<#else>SUSPENDED</#if>.

The ${organisationName} registration service
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public void testPagedClientLookup() {

Sort sort = Sort.by(Direction.ASC, "clientId");
Pageable pageable = PagingUtils.buildPageRequest(10, 1, 100, sort);

ListResponseDTO<RegisteredClientDTO> clients = managementService.retrieveAllClients(pageable);

assertThat(clients.getTotalResults(), is(19L));
Expand Down Expand Up @@ -249,8 +249,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");
Expand All @@ -261,7 +260,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));
Expand Down Expand Up @@ -296,8 +295,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
Expand Down Expand Up @@ -333,9 +331,8 @@ public void testClientOwnerAssignRemove() throws ParseException {
RegisteredClientDTO savedClient = managementService.saveNewClient(client);
assertThat(savedClient.getClientId(), is(client.getClientId()));
assertThat(savedClient.getClientSecret(), notNullValue());

ListResponseDTO<ScimUser> owners =
managementService.getClientOwners(savedClient.getClientId(),

ListResponseDTO<ScimUser> owners = managementService.getClientOwners(savedClient.getClientId(),
PagingUtils.buildUnpagedPageRequest());

assertThat(owners.getTotalResults(), is(0L));
Expand Down Expand Up @@ -417,4 +414,24 @@ public void testClientStatusChange() {
assertTrue(client.getStatusChangedOn().equals(Date.from(clock.instant())));
assertEquals("userUUID", client.getStatusChangedBy());
}

@Test
public void testClientStatusChangeWithContacts() {
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())));
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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
Loading