diff --git a/src/main/java/uk/ac/sanger/sccp/stan/GraphQLMutation.java b/src/main/java/uk/ac/sanger/sccp/stan/GraphQLMutation.java index 8a1284e7..f0112f17 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/GraphQLMutation.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/GraphQLMutation.java @@ -6,10 +6,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; -import org.springframework.security.core.Authentication; import org.springframework.stereotype.Component; -import uk.ac.sanger.sccp.stan.config.SessionConfig; import uk.ac.sanger.sccp.stan.model.*; import uk.ac.sanger.sccp.stan.repo.UserRepo; import uk.ac.sanger.sccp.stan.request.*; @@ -32,9 +29,8 @@ import uk.ac.sanger.sccp.stan.service.work.WorkService; import uk.ac.sanger.sccp.stan.service.work.WorkTypeService; -import java.util.*; +import java.util.List; import java.util.function.BiFunction; -import java.util.function.Function; import static java.util.Objects.requireNonNull; import static uk.ac.sanger.sccp.utils.BasicUtils.repr; @@ -45,8 +41,7 @@ @Component public class GraphQLMutation extends BaseGraphQLResource { Logger log = LoggerFactory.getLogger(GraphQLMutation.class); - final LDAPService ldapService; - final SessionConfig sessionConfig; + final AuthService authService; final IRegisterService registerService; final IRegisterService sectionRegisterService; final PlanService planService; @@ -107,7 +102,7 @@ public class GraphQLMutation extends BaseGraphQLResource { @Autowired public GraphQLMutation(ObjectMapper objectMapper, AuthenticationComponent authComp, - LDAPService ldapService, SessionConfig sessionConfig, + AuthService authService, IRegisterService registerService, IRegisterService sectionRegisterService, PlanService planService, LabelPrintService labelPrintService, @@ -138,8 +133,7 @@ public GraphQLMutation(ObjectMapper objectMapper, AuthenticationComponent authCo ReactivateService reactivateService, LibraryPrepService libraryPrepService, UserAdminService userAdminService) { super(objectMapper, authComp, userRepo); - this.ldapService = ldapService; - this.sessionConfig = sessionConfig; + this.authService = authService; this.registerService = registerService; this.sectionRegisterService = sectionRegisterService; this.planService = planService; @@ -206,48 +200,22 @@ private void logRequest(String name, User user, Object request) { } public DataFetcher logIn() { - return dataFetchingEnvironment -> { - String username = dataFetchingEnvironment.getArgument("username"); - if (log.isInfoEnabled()) { - log.info("Login attempt by {}", repr(username)); - } - Optional optUser = userRepo.findByUsername(username); - if (optUser.isEmpty()) { - return new LoginResult("Username not in database.", null); - } - User user = optUser.get(); - if (user.getRole()==User.Role.disabled) { - return new LoginResult("Username is disabled.", null); - } - String password = dataFetchingEnvironment.getArgument("password"); - if (!ldapService.verifyCredentials(username, password)) { - return new LoginResult("Login failed", null); - } - Authentication authentication = new UsernamePasswordAuthenticationToken(user, password, new ArrayList<>()); - authComp.setAuthentication(authentication, sessionConfig.getMaxInactiveMinutes()); - log.info("Login succeeded for user {}", user); - return new LoginResult("OK", user); + return dfe -> { + String username = dfe.getArgument("username"); + String password = dfe.getArgument("password"); + return authService.logIn(username, password); }; } - private String loggedInUsername() { - var auth = authComp.getAuthentication(); - if (auth != null) { - var princ = auth.getPrincipal(); - if (princ instanceof User) { - return ((User) princ).getUsername(); - } - } - return null; + public DataFetcher logOut() { + return dfe -> authService.logOut(); } - public DataFetcher logOut() { - return dataFetchingEnvironment -> { - if (log.isInfoEnabled()) { - log.info("Logout requested by {}", repr(loggedInUsername())); - } - authComp.setAuthentication(null, 0); - return "OK"; + public DataFetcher userSelfRegister(final User.Role role) { + return dfe -> { + String username = dfe.getArgument("username"); + String password = dfe.getArgument("password"); + return authService.selfRegister(username, password, role); }; } @@ -470,7 +438,7 @@ public DataFetcher setSpeciesEnabled() { } public DataFetcher addProject() { - return adminAdd(projectService::addNew, "AddProject", "name"); + return adminAdd(projectService::addNew, "AddProject", "name", User.Role.enduser); } public DataFetcher setProjectEnabled() { @@ -486,7 +454,7 @@ public DataFetcher setProgramEnabled() { } public DataFetcher addCostCode() { - return adminAdd(costCodeService::addNew, "AddCostCode", "code"); + return adminAdd(costCodeService::addNew, "AddCostCode", "code", User.Role.enduser); } public DataFetcher setCostCodeEnabled() { @@ -907,7 +875,7 @@ public DataFetcher libraryPrep() { } public DataFetcher addUser() { - return adminAdd(userAdminService::addUser, "AddUser", "username"); + return adminAdd(userAdminService::addNormalUser, "AddUser", "username"); } public DataFetcher setUserRole() { @@ -920,12 +888,16 @@ public DataFetcher setUserRole() { }; } - private DataFetcher adminAdd(Function addFunction, String functionName, String argName) { + private DataFetcher adminAdd(BiFunction addFunction, String functionName, String argName) { + return adminAdd(addFunction, functionName, argName, User.Role.admin); + } + + private DataFetcher adminAdd(BiFunction addFunction, String functionName, String argName, User.Role role) { return dfe -> { - User user = checkUser(dfe, User.Role.admin); + User user = checkUser(dfe, role); String arg = dfe.getArgument(argName); logRequest(functionName, user, repr(arg)); - return addFunction.apply(arg); + return addFunction.apply(user, arg); }; } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/GraphQLProvider.java b/src/main/java/uk/ac/sanger/sccp/stan/GraphQLProvider.java index 48c9bf18..6862f740 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/GraphQLProvider.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/GraphQLProvider.java @@ -11,6 +11,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.stereotype.Component; +import uk.ac.sanger.sccp.stan.model.User; import javax.annotation.PostConstruct; import java.io.IOException; @@ -134,6 +135,7 @@ private RuntimeWiring buildWiring() { .dataFetcher("version", graphQLDataFetchers.versionInfo()) ) .type(newTypeWiring("Mutation") + .dataFetcher("registerAsEndUser", graphQLMutation.userSelfRegister(User.Role.enduser)) // internal transaction .dataFetcher("login", graphQLMutation.logIn()) .dataFetcher("logout", graphQLMutation.logOut()) .dataFetcher("register", transact(graphQLMutation.register())) @@ -153,34 +155,34 @@ private RuntimeWiring buildWiring() { .dataFetcher("addEquipment", transact(graphQLMutation.addEquipment())) .dataFetcher("setEquipmentEnabled", transact(graphQLMutation.setEquipmentEnabled())) .dataFetcher("renameEquipment", transact(graphQLMutation.renameEquipment())) - .dataFetcher("addHmdmc", transact(graphQLMutation.addHmdmc())) + .dataFetcher("addHmdmc", graphQLMutation.addHmdmc()) // internal transaction .dataFetcher("setHmdmcEnabled", transact(graphQLMutation.setHmdmcEnabled())) - .dataFetcher("addDestructionReason", transact(graphQLMutation.addDestructionReason())) + .dataFetcher("addDestructionReason", graphQLMutation.addDestructionReason()) // internal transaction .dataFetcher("setDestructionReasonEnabled", transact(graphQLMutation.setDestructionReasonEnabled())) - .dataFetcher("addReleaseDestination", transact(graphQLMutation.addReleaseDestination())) + .dataFetcher("addReleaseDestination", graphQLMutation.addReleaseDestination()) // internal transaction .dataFetcher("setReleaseDestinationEnabled", transact(graphQLMutation.setReleaseDestinationEnabled())) .dataFetcher("addReleaseRecipient", transact(graphQLMutation.addReleaseRecipient())) .dataFetcher("updateReleaseRecipientFullName", transact(graphQLMutation.updateReleaseRecipientFullName())) .dataFetcher("setReleaseRecipientEnabled", transact(graphQLMutation.setReleaseRecipientEnabled())) - .dataFetcher("addSpecies", transact(graphQLMutation.addSpecies())) + .dataFetcher("addSpecies", graphQLMutation.addSpecies()) // internal transaction .dataFetcher("setSpeciesEnabled", transact(graphQLMutation.setSpeciesEnabled())) - .dataFetcher("addProject", transact(graphQLMutation.addProject())) + .dataFetcher("addProject", graphQLMutation.addProject()) // internal transaction .dataFetcher("setProjectEnabled", transact(graphQLMutation.setProjectEnabled())) - .dataFetcher("addProgram", transact(graphQLMutation.addProgram())) + .dataFetcher("addProgram", graphQLMutation.addProgram()) // internal transaction .dataFetcher("setProgramEnabled", transact(graphQLMutation.setProgramEnabled())) - .dataFetcher("addCostCode", transact(graphQLMutation.addCostCode())) + .dataFetcher("addCostCode", graphQLMutation.addCostCode()) // internal transaction .dataFetcher("setCostCodeEnabled", transact(graphQLMutation.setCostCodeEnabled())) - .dataFetcher("addFixative", transact(graphQLMutation.addFixative())) + .dataFetcher("addFixative", graphQLMutation.addFixative()) // internal transaction .dataFetcher("setFixativeEnabled", transact(graphQLMutation.setFixativeEnabled())) - .dataFetcher("addSolution", transact(graphQLMutation.addSolution())) + .dataFetcher("addSolution", graphQLMutation.addSolution()) // internal transaction .dataFetcher("setSolutionEnabled", transact(graphQLMutation.setSolutionEnabled())) - .dataFetcher("addOmeroProject", transact(graphQLMutation.addOmeroProject())) + .dataFetcher("addOmeroProject", graphQLMutation.addOmeroProject()) // internal transaction .dataFetcher("setOmeroProjectEnabled", transact(graphQLMutation.setOmeroProjectEnabled())) - .dataFetcher("addSlotRegion", transact(graphQLMutation.addSlotRegion())) + .dataFetcher("addSlotRegion", graphQLMutation.addSlotRegion()) // internal transaction .dataFetcher("setSlotRegionEnabled", transact(graphQLMutation.setSlotRegionEnabled())) - .dataFetcher("addProbePanel", transact(graphQLMutation.addProbePanel())) + .dataFetcher("addProbePanel", graphQLMutation.addProbePanel()) // internal transaction .dataFetcher("setProbePanelEnabled", transact(graphQLMutation.setProbePanelEnabled())) - .dataFetcher("addWorkType", transact(graphQLMutation.addWorkType())) + .dataFetcher("addWorkType", graphQLMutation.addWorkType()) // internal transaction .dataFetcher("setWorkTypeEnabled", transact(graphQLMutation.setWorkTypeEnabled())) .dataFetcher("createWork", transact(graphQLMutation.createWork())) .dataFetcher("updateWorkStatus", transact(graphQLMutation.updateWorkStatus())) diff --git a/src/main/java/uk/ac/sanger/sccp/stan/config/MailConfig.java b/src/main/java/uk/ac/sanger/sccp/stan/config/MailConfig.java index 45967899..03572a82 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/config/MailConfig.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/config/MailConfig.java @@ -1,7 +1,11 @@ package uk.ac.sanger.sccp.stan.config; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Configuration; +import uk.ac.sanger.sccp.utils.UCMap; + +import java.util.Map; /** * Some config related to emails @@ -21,6 +25,8 @@ public class MailConfig { @Value("${stan.mail.release_cc}") String releaseCC; + private UCMap adminNotifications; + /** The value to put in the "from" field of emails */ public String getSender() { return this.sender; @@ -40,4 +46,13 @@ public String getServiceDescription() { public String getReleaseCC() { return this.releaseCC; } + + public boolean isAdminNotificationEnabled(String name) { + return this.adminNotifications.getOrDefault(name, Boolean.TRUE); + } + + @Autowired + public void setAdminNotifications(@Value("#{${stan.mail.admin_notify}}") Map notifications) { + this.adminNotifications = (notifications==null ? new UCMap<>(0) : new UCMap<>(notifications)); + } } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/repo/UserRepo.java b/src/main/java/uk/ac/sanger/sccp/stan/repo/UserRepo.java index 8d575131..1ff14531 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/repo/UserRepo.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/repo/UserRepo.java @@ -4,6 +4,7 @@ import uk.ac.sanger.sccp.stan.model.User; import javax.persistence.EntityNotFoundException; +import java.util.List; import java.util.Optional; import static uk.ac.sanger.sccp.utils.BasicUtils.repr; @@ -14,4 +15,6 @@ public interface UserRepo extends CrudRepository { default User getByUsername(String username) throws EntityNotFoundException { return findByUsername(username).orElseThrow(() -> new EntityNotFoundException("User not found: "+repr(username))); } + + List findAllByRole(User.Role role); } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/request/LoginResult.java b/src/main/java/uk/ac/sanger/sccp/stan/request/LoginResult.java index 6545be30..06826a12 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/request/LoginResult.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/request/LoginResult.java @@ -11,12 +11,18 @@ public class LoginResult { private String message; private User user; + private boolean created; public LoginResult() {} public LoginResult(String message, User user) { + this(message, user, false); + } + + public LoginResult(String message, User user, boolean created) { this.message = message; this.user = user; + this.created = created; } public String getMessage() { @@ -35,13 +41,23 @@ public void setUser(User user) { this.user = user; } + /** Has a new user been created? This is used internally to trigger a notification email. */ + public boolean isCreated() { + return this.created; + } + + public void setCreated(boolean created) { + this.created = created; + } + @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; LoginResult that = (LoginResult) o; return (Objects.equals(this.message, that.message) - && Objects.equals(this.user, that.user)); + && Objects.equals(this.user, that.user) + && this.created==that.created); } @Override @@ -54,6 +70,7 @@ public String toString() { return MoreObjects.toStringHelper(this) .add("message", message) .add("user", user) + .add("created", created) .toString(); } } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/AdminNotifyService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/AdminNotifyService.java new file mode 100644 index 00000000..455c8982 --- /dev/null +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/AdminNotifyService.java @@ -0,0 +1,32 @@ +package uk.ac.sanger.sccp.stan.service; + +/** + * Service for sending email notifications to admin users + * @author dr6 + */ +public interface AdminNotifyService { + /** + * Is the indicated admin notification enabled? + * @param name the name of the notification + * @return true if it is enabled; false if it is disabled + */ + boolean isNotificationEnabled(String name); + + /** + * Replaces {@code %service} with the name of the service, for clearer email messages. + * @param string the string to modify + * @return the modified string + */ + String substitute(String string); + + /** + * Issues the specified notification to admin users, if it is enabled. + * {@link #substitute} is used on the {@code heading} and {@code body} arguments. + * @param name the name of the notification + * @param heading the email heading + * @param body the text of the email + * @return true if the email was sent; false otherwise + */ + boolean issue(String name, String heading, String body); + +} diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/AdminNotifyServiceImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/AdminNotifyServiceImp.java new file mode 100644 index 00000000..12edbfe1 --- /dev/null +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/AdminNotifyServiceImp.java @@ -0,0 +1,60 @@ +package uk.ac.sanger.sccp.stan.service; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.config.MailConfig; +import uk.ac.sanger.sccp.stan.model.User; +import uk.ac.sanger.sccp.stan.repo.UserRepo; + +import java.util.List; + +import static uk.ac.sanger.sccp.utils.BasicUtils.nullOrEmpty; + +/** + * @author dr6 + */ +@Service +public class AdminNotifyServiceImp implements AdminNotifyService { + private final MailConfig mailConfig; + private final UserRepo userRepo; + private final EmailService emailService; + + @Autowired + public AdminNotifyServiceImp(MailConfig mailConfig, UserRepo userRepo, EmailService emailService) { + this.mailConfig = mailConfig; + this.userRepo = userRepo; + this.emailService = emailService; + } + + @Override + public boolean isNotificationEnabled(String name) { + return this.mailConfig.isAdminNotificationEnabled(name); + } + + @Override + public String substitute(String string) { + if (!nullOrEmpty(string)) { + string = string.replace("%service", mailConfig.getServiceDescription()); + } + return string; + } + + @Override + public boolean issue(String name, String heading, String body) { + if (!isNotificationEnabled(name)) { + return false; + } + List admins = userRepo.findAllByRole(User.Role.admin); + if (admins.isEmpty()) { + return false; + } + List recipients = admins.stream().map(User::getUsername).toList(); + return sendNotification(recipients, heading, body); + } + + public boolean sendNotification(List recipients, String heading, String body) { + heading = substitute(heading); + body = substitute(body); + return emailService.tryEmail(recipients, heading, body); + } +} diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/AuthService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/AuthService.java new file mode 100644 index 00000000..57497563 --- /dev/null +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/AuthService.java @@ -0,0 +1,32 @@ +package uk.ac.sanger.sccp.stan.service; + +import uk.ac.sanger.sccp.stan.model.User; +import uk.ac.sanger.sccp.stan.request.LoginResult; + +/** + * Service dealing with user authentication + */ +public interface AuthService { + /** + * Logs in + * @param username the username to log in + * @param password the password to authenticate the user + * @return a login result describing the result of the login + */ + LoginResult logIn(String username, String password); + + /** + * Logs out the current logged in user, if any + * @return a description of the result of logging out + */ + String logOut(); + + /** + * Creates a Stan user authenticated with the given credentials + * @param username the username to create + * @param password the password to authenticate the user + * @param role the role to create the user with + * @return the result of attempting to log in with the given credentials + */ + LoginResult selfRegister(String username, String password, User.Role role); +} diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/AuthServiceImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/AuthServiceImp.java new file mode 100644 index 00000000..e00c2159 --- /dev/null +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/AuthServiceImp.java @@ -0,0 +1,145 @@ +package uk.ac.sanger.sccp.stan.service; + +import org.jetbrains.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.AuthenticationComponent; +import uk.ac.sanger.sccp.stan.Transactor; +import uk.ac.sanger.sccp.stan.config.SessionConfig; +import uk.ac.sanger.sccp.stan.model.User; +import uk.ac.sanger.sccp.stan.repo.UserRepo; +import uk.ac.sanger.sccp.stan.request.LoginResult; + +import java.util.ArrayList; +import java.util.Optional; + +import static uk.ac.sanger.sccp.utils.BasicUtils.repr; + +/** + * @author dr6 + */ +@Service +public class AuthServiceImp implements AuthService { + Logger log = LoggerFactory.getLogger(AuthServiceImp.class); + private final SessionConfig sessionConfig; + private final UserRepo userRepo; + private final AuthenticationComponent authComp; + private final LDAPService ldapService; + private final AdminNotifyService adminNotifyService; + private final UserAdminService userAdminService; + private final Transactor transactor; + + @Autowired + public AuthServiceImp(SessionConfig sessionConfig, + UserRepo userRepo, AuthenticationComponent authComp, + LDAPService ldapService, AdminNotifyService adminNotifyService, + UserAdminService userAdminService, + Transactor transactor) { + this.sessionConfig = sessionConfig; + this.userRepo = userRepo; + this.authComp = authComp; + this.ldapService = ldapService; + this.adminNotifyService = adminNotifyService; + this.userAdminService = userAdminService; + this.transactor = transactor; + } + + /** + * Gets the username of the logged in user, if any + * @return the logged in username, or null + */ + @Nullable + public String loggedInUsername() { + var auth = authComp.getAuthentication(); + if (auth != null) { + var princ = auth.getPrincipal(); + if (princ instanceof User) { + return ((User) princ).getUsername(); + } + } + return null; + } + + @Override + public LoginResult logIn(String username, String password) { + if (log.isInfoEnabled()) { + log.info("Login attempt by {}", repr(username)); + } + Optional optUser = userRepo.findByUsername(username); + if (optUser.isEmpty()) { + return new LoginResult("Username not in database.", null); + } + User user = optUser.get(); + if (user.getRole()==User.Role.disabled) { + return new LoginResult("Username is disabled.", null); + } + if (!ldapService.verifyCredentials(username, password)) { + return new LoginResult("Login failed.", null); + } + Authentication authentication = new UsernamePasswordAuthenticationToken(user, password, new ArrayList<>()); + authComp.setAuthentication(authentication, sessionConfig.getMaxInactiveMinutes()); + log.info("Login succeeded for user {}", user); + return new LoginResult("OK", user); + } + + @Override + public String logOut() { + if (log.isInfoEnabled()) { + log.info("Logout requested by {}", repr(loggedInUsername())); + } + authComp.setAuthentication(null, 0); + return "OK"; + } + + public LoginResult selfRegisterInTransaction(String username, String password, User.Role role) { + if (log.isInfoEnabled()) { + log.info("selfRegister attempt by {}", repr(username)); + } + username = userAdminService.validateUsername(username); + if (!ldapService.verifyCredentials(username, password)) { + return new LoginResult("Authentication failed.", null); + } + Optional optUser = userRepo.findByUsername(username); + User user; + boolean created; + if (optUser.isEmpty()) { + user = userAdminService.addUser(username, role); + log.info("Login succeeded as new user {}", user); + created = true; + } else { + user = optUser.get(); + if (user.getRole()== User.Role.disabled) { + return new LoginResult("Username is disabled.", null); + } + log.info("Login succeeded for existing user {}", user); + created = false; + } + + Authentication authentication = new UsernamePasswordAuthenticationToken(user, password, new ArrayList<>()); + authComp.setAuthentication(authentication, sessionConfig.getMaxInactiveMinutes()); + return new LoginResult("OK", user, created); + } + + @Override + public LoginResult selfRegister(String username, String password, User.Role role) { + LoginResult result = transactor.transact("selfRegister", + () -> selfRegisterInTransaction(username, password, role)); + if (result.isCreated()) { + sendNewUserEmail(result.getUser()); + } + return result; + } + + /** + * Tries to send an email to admin users about the new user being created. + * @param user the new user + */ + public void sendNewUserEmail(User user) { + String body = "User "+user.getUsername()+" has registered themself as "+user.getRole()+" on %service."; + adminNotifyService.issue("user", "New user created on %service", body); + } +} diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/BaseAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/BaseAdminService.java index 25a2a593..61b7e238 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/BaseAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/BaseAdminService.java @@ -1,7 +1,9 @@ package uk.ac.sanger.sccp.stan.service; import org.springframework.data.repository.CrudRepository; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.HasEnabled; +import uk.ac.sanger.sccp.stan.model.User; import javax.persistence.EntityExistsException; import javax.persistence.EntityNotFoundException; @@ -18,32 +20,58 @@ public abstract class BaseAdminService stringValidator; + final Transactor transactor; + final AdminNotifyService notifyService; - protected BaseAdminService(R repo, String entityTypeName, String stringFieldName, Validator stringValidator) { + protected BaseAdminService(R repo, String entityTypeName, String stringFieldName, + Validator stringValidator, Transactor transactor, + AdminNotifyService notifyService) { this.repo = repo; this.entityTypeName = entityTypeName; this.missingFieldMessage = stringFieldName+" not supplied."; this.stringValidator = stringValidator; + this.transactor = transactor; + this.notifyService = notifyService; } /** - * Creates a new item representing the given string - * @param string the string. This will be trimmed before it is used. + * Creates a new item representing the given string. + * If the creating user is an {@link User.Role#enduser enduser} then a notification is sent by email to admin users + * @param creator the user responsible for creating the new item + * @param string the string. This will be trimmed before it is used * @return the new item * @exception IllegalArgumentException if the string is blank or null, or fails validation * @exception EntityExistsException A matching item already exists */ - public E addNew(String string) { - return repo.save(newEntity(validateEntity(string))); + public E addNew(User creator, String string) { + E newValue = transactor.transact("Add "+entityTypeName, + () -> repo.save(newEntity(validateEntity(string)))); + if (creator != null && creator.getRole() == User.Role.enduser) { + sendNewEntityEmail(creator, newValue); + } + return newValue; } - public String validateEntity(String identifier) { + /** + * Validates the given identifier for its value and uniqueness. + * @param identifier the identifier for a new entity + * @return the sanitised version of the identifier + * @exception IllegalArgumentException if validation fails + * @exception EntityExistsException if the identifier is already in use + */ + public String validateEntity(String identifier) throws IllegalArgumentException, EntityExistsException { identifier = validateIdentifier(identifier); validateUniqueness(identifier); return identifier; } - public String validateIdentifier(String identifier) { + /** + * Validates the given identifier for its value + * @param identifier the identifier for a new entity + * @return the sanitised version of the identifier + * @exception IllegalArgumentException if validation fails + */ + public String validateIdentifier(String identifier) throws IllegalArgumentException { identifier = trimAndRequire(identifier, missingFieldMessage); if (this.stringValidator!=null) { this.stringValidator.checkArgument(identifier); @@ -51,7 +79,12 @@ public String validateIdentifier(String identifier) { return identifier; } - private void validateUniqueness(String identifier) { + /** + * Checks if an entity with the given identifier exists already + * @param identifier the new identifier + * @exception EntityExistsException if such an entity already exists + */ + private void validateUniqueness(String identifier) throws EntityExistsException { Optional entity = findEntity(repo, identifier); if (entity.isPresent()) { throw new EntityExistsException(entityTypeName+" already exists: "+identifier); @@ -68,7 +101,7 @@ private void validateUniqueness(String identifier) { * @exception IllegalArgumentException if the string is blank or null * @exception EntityNotFoundException if no such entity is found */ - public E setEnabled(String string, boolean enabled) { + public E setEnabled(String string, boolean enabled) throws IllegalArgumentException, EntityNotFoundException { final String stringValue = trimAndRequire(string, missingFieldMessage); E entity = findEntity(repo, stringValue).orElseThrow(() -> new EntityNotFoundException(entityTypeName+" not found: "+stringValue)); if (entity.isEnabled()==enabled) { @@ -78,6 +111,20 @@ public E setEnabled(String string, boolean enabled) { return repo.save(entity); } + /** + * Sends an email about the creation of a given item to admin users. + * @param creator the user who created the item + * @param item the new item created + */ + public void sendNewEntityEmail(User creator, E item) { + String notification = this.notificationName(); + if (notification!=null) { + String body = String.format("User %s has created a new %s on %%service: %s", + creator.getUsername(), entityTypeName, item); + notifyService.issue(notification, "%service new "+entityTypeName, body); + } + } + /** * Returns a new unpersisted object * @param string the string for the new object @@ -92,4 +139,13 @@ public E setEnabled(String string, boolean enabled) { * @return an optional that will contain the entity if it is found */ protected abstract Optional findEntity(R repo, String string); + + /** + * If this returns a string, it will be used as the name of the notification sent to admin users. + * By default it returns null, and no notification will be sent. + * @return the name of the notification + */ + public String notificationName() { + return null; + } } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/CostCodeService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/CostCodeService.java index 2cafc3b7..fbd49583 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/CostCodeService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/CostCodeService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.CostCode; import uk.ac.sanger.sccp.stan.repo.CostCodeRepo; @@ -16,8 +17,14 @@ public class CostCodeService extends BaseAdminService { @Autowired public CostCodeService(CostCodeRepo costCodeRepo, - @Qualifier("costCodeValidator") Validator costCodeValidator) { - super(costCodeRepo, "CostCode", "Code", costCodeValidator); + @Qualifier("costCodeValidator") Validator costCodeValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(costCodeRepo, "CostCode", "Code", costCodeValidator, transactor, notifyService); + } + + @Override + public String notificationName() { + return "costcode"; } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/DestructionReasonAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/DestructionReasonAdminService.java index a70f4d64..a7cbd021 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/DestructionReasonAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/DestructionReasonAdminService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.DestructionReason; import uk.ac.sanger.sccp.stan.repo.DestructionReasonRepo; @@ -16,8 +17,10 @@ public class DestructionReasonAdminService extends BaseAdminService { @Autowired public DestructionReasonAdminService(DestructionReasonRepo destructionReasonRepo, - @Qualifier("destructionReasonValidator") Validator destructionReasonValidator) { - super(destructionReasonRepo, "Destruction reason", "Text", destructionReasonValidator); + @Qualifier("destructionReasonValidator") Validator destructionReasonValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(destructionReasonRepo, "Destruction reason", "Text", + destructionReasonValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/EmailService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/EmailService.java index 4739dc2b..db2f3be9 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/EmailService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/EmailService.java @@ -10,6 +10,7 @@ import uk.ac.sanger.sccp.stan.config.MailConfig; import uk.ac.sanger.sccp.stan.service.store.StoreService; +import java.util.Collection; import java.util.List; import static uk.ac.sanger.sccp.utils.BasicUtils.nullOrEmpty; @@ -88,13 +89,10 @@ public String getServiceDescription() { * @return an array of cc recipients, or null */ public String[] releaseEmailCCs(List ccList) { - String releaseCc = mailConfig.getReleaseCC(); + String releaseCc = usernameToEmail(mailConfig.getReleaseCC()); if (nullOrEmpty(releaseCc)) { return nullOrEmpty(ccList) ? null : ccList.toArray(String[]::new); } - if (releaseCc.indexOf('@') < 0) { - releaseCc += "@sanger.ac.uk"; - } if (nullOrEmpty(ccList)) { return new String[] { releaseCc }; } @@ -133,4 +131,38 @@ public boolean tryReleaseEmail(String recipient, List ccList, List recipients, String heading, String text) { + String[] emailRecs = recipients.stream() + .map(this::usernameToEmail) + .toArray(String[]::new); + try { + send(heading, text, emailRecs, null); + return true; + } catch (Exception e) { + log.error("Failed to send email", e); + return false; + } + } } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/FixativeService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/FixativeService.java index df2d8e0d..83cfa749 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/FixativeService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/FixativeService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.Fixative; import uk.ac.sanger.sccp.stan.repo.FixativeRepo; @@ -16,8 +17,9 @@ public class FixativeService extends BaseAdminService { @Autowired public FixativeService(FixativeRepo fixativeRepo, - @Qualifier("fixativeNameValidator") Validator fixativeNameValidator) { - super(fixativeRepo, "Fixative", "Name", fixativeNameValidator); + @Qualifier("fixativeNameValidator") Validator fixativeNameValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(fixativeRepo, "Fixative", "Name", fixativeNameValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/HmdmcAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/HmdmcAdminService.java index b1dc6aba..30d46275 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/HmdmcAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/HmdmcAdminService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.Hmdmc; import uk.ac.sanger.sccp.stan.repo.HmdmcRepo; @@ -15,8 +16,9 @@ @Service public class HmdmcAdminService extends BaseAdminService { @Autowired - public HmdmcAdminService(HmdmcRepo repo, @Qualifier("hmdmcValidator") Validator hmdmcValidator) { - super(repo, "HuMFre", "HuMFre", hmdmcValidator); + public HmdmcAdminService(HmdmcRepo repo, @Qualifier("hmdmcValidator") Validator hmdmcValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(repo, "HuMFre", "HuMFre", hmdmcValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/OmeroProjectAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/OmeroProjectAdminService.java index 08806521..96732899 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/OmeroProjectAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/OmeroProjectAdminService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.OmeroProject; import uk.ac.sanger.sccp.stan.repo.OmeroProjectRepo; @@ -16,8 +17,9 @@ public class OmeroProjectAdminService extends BaseAdminService { @Autowired public OmeroProjectAdminService(OmeroProjectRepo repo, - @Qualifier("omeroProjectNameValidator") Validator omeroProjectValidator) { - super(repo, "Omero project", "Name", omeroProjectValidator); + @Qualifier("omeroProjectNameValidator") Validator omeroProjectValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(repo, "Omero project", "Name", omeroProjectValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/ProbePanelService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/ProbePanelService.java index 3e4dcac2..fa43b40d 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/ProbePanelService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/ProbePanelService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.CostCode; import uk.ac.sanger.sccp.stan.model.ProbePanel; import uk.ac.sanger.sccp.stan.repo.ProbePanelRepo; @@ -17,8 +18,9 @@ public class ProbePanelService extends BaseAdminService { @Autowired public ProbePanelService(ProbePanelRepo repo, - @Qualifier("probePanelNameValidator") Validator probePanelValidator) { - super(repo, "ProbePanel", "Name", probePanelValidator); + @Qualifier("probePanelNameValidator") Validator probePanelValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(repo, "ProbePanel", "Name", probePanelValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/ProgramService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/ProgramService.java index 94a93521..fb925ef8 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/ProgramService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/ProgramService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.Program; import uk.ac.sanger.sccp.stan.repo.ProgramRepo; @@ -16,8 +17,9 @@ public class ProgramService extends BaseAdminService { @Autowired public ProgramService(ProgramRepo ProgramRepo, - @Qualifier("programNameValidator") Validator ProgramNameValidator) { - super(ProgramRepo, "Program", "Name", ProgramNameValidator); + @Qualifier("programNameValidator") Validator ProgramNameValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(ProgramRepo, "Program", "Name", ProgramNameValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/ProjectService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/ProjectService.java index ebad11fa..e7cd16b1 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/ProjectService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/ProjectService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.Project; import uk.ac.sanger.sccp.stan.repo.ProjectRepo; @@ -16,8 +17,14 @@ public class ProjectService extends BaseAdminService { @Autowired public ProjectService(ProjectRepo projectRepo, - @Qualifier("projectNameValidator") Validator projectNameValidator) { - super(projectRepo, "Project", "Name", projectNameValidator); + @Qualifier("projectNameValidator") Validator projectNameValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(projectRepo, "Project", "Name", projectNameValidator, transactor, notifyService); + } + + @Override + public String notificationName() { + return "project"; } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseDestinationAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseDestinationAdminService.java index 47442c1c..73c7eefe 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseDestinationAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseDestinationAdminService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.ReleaseDestination; import uk.ac.sanger.sccp.stan.repo.ReleaseDestinationRepo; @@ -16,8 +17,9 @@ public class ReleaseDestinationAdminService extends BaseAdminService { @Autowired public ReleaseDestinationAdminService(ReleaseDestinationRepo repo, - @Qualifier("releaseDestinationValidator") Validator releaseDestinationValidator) { - super(repo, "Release destination", "Name", releaseDestinationValidator); + @Qualifier("releaseDestinationValidator") Validator releaseDestinationValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(repo, "Release destination", "Name", releaseDestinationValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseRecipientAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseRecipientAdminService.java index 045d7711..dd0a11ef 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseRecipientAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseRecipientAdminService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.ReleaseRecipient; import uk.ac.sanger.sccp.stan.repo.ReleaseRecipientRepo; @@ -21,8 +22,9 @@ public class ReleaseRecipientAdminService extends BaseAdminService { @Autowired public ReleaseRecipientAdminService(ReleaseRecipientRepo repo, - @Qualifier("releaseRecipientValidator") Validator releaseRecipientValidator) { - super(repo, "Release recipient", "Username", releaseRecipientValidator); + @Qualifier("releaseRecipientValidator") Validator releaseRecipientValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(repo, "Release recipient", "Username", releaseRecipientValidator, transactor, notifyService); } public ReleaseRecipient addNew(String username, String fullName) { diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseServiceImp.java b/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseServiceImp.java index 0f253ec3..246e4afe 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseServiceImp.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/ReleaseServiceImp.java @@ -276,28 +276,28 @@ public void validateLabware(Collection labware) { List emptyLabwareBarcodes = labware.stream() .filter(Labware::isEmpty) .map(Labware::getBarcode) - .collect(toList()); + .toList(); if (!emptyLabwareBarcodes.isEmpty()) { throw new IllegalArgumentException("Cannot release empty labware: "+emptyLabwareBarcodes); } List releasedLabwareBarcodes = labware.stream() .filter(Labware::isReleased) .map(Labware::getBarcode) - .collect(toList()); + .toList(); if (!releasedLabwareBarcodes.isEmpty()) { throw new IllegalArgumentException("Labware has already been released: "+releasedLabwareBarcodes); } List destroyedLabwareBarcodes = labware.stream() .filter(Labware::isDestroyed) .map(Labware::getBarcode) - .collect(toList()); + .toList(); if (!destroyedLabwareBarcodes.isEmpty()) { throw new IllegalArgumentException("Labware cannot be released because it is destroyed: "+destroyedLabwareBarcodes); } List discardedLabwareBarcodes = labware.stream() .filter(Labware::isDiscarded) .map(Labware::getBarcode) - .collect(toList()); + .toList(); if (!discardedLabwareBarcodes.isEmpty()) { throw new IllegalArgumentException("Labware cannot be released because it is discarded: "+discardedLabwareBarcodes); } diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/SlotRegionAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/SlotRegionAdminService.java index c8a73e44..6a6c5bf0 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/SlotRegionAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/SlotRegionAdminService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.SlotRegion; import uk.ac.sanger.sccp.stan.repo.SlotRegionRepo; @@ -15,8 +16,10 @@ @Service public class SlotRegionAdminService extends BaseAdminService { @Autowired - public SlotRegionAdminService(SlotRegionRepo repo, @Qualifier("slotRegionNameValidator") Validator nameValidator) { - super(repo, "SlotRegion", "name", nameValidator); + public SlotRegionAdminService(SlotRegionRepo repo, + @Qualifier("slotRegionNameValidator") Validator nameValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(repo, "SlotRegion", "name", nameValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/SolutionAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/SolutionAdminService.java index 5c02ae97..f91cf4cb 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/SolutionAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/SolutionAdminService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.Solution; import uk.ac.sanger.sccp.stan.repo.SolutionRepo; @@ -16,8 +17,9 @@ public class SolutionAdminService extends BaseAdminService { @Autowired public SolutionAdminService(SolutionRepo solutionRepo, - @Qualifier("solutionValidator") Validator solutionValidator) { - super(solutionRepo, "Solution", "Name", solutionValidator); + @Qualifier("solutionValidator") Validator solutionValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(solutionRepo, "Solution", "Name", solutionValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/SpeciesAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/SpeciesAdminService.java index b3b50fb3..0411b6bb 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/SpeciesAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/SpeciesAdminService.java @@ -3,6 +3,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.Species; import uk.ac.sanger.sccp.stan.repo.SpeciesRepo; @@ -15,8 +16,10 @@ @Service public class SpeciesAdminService extends BaseAdminService { @Autowired - public SpeciesAdminService(SpeciesRepo repo, @Qualifier("speciesValidator") Validator speciesValidator) { - super(repo, "Species", "Name", speciesValidator); + public SpeciesAdminService(SpeciesRepo repo, + @Qualifier("speciesValidator") Validator speciesValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(repo, "Species", "Name", speciesValidator, transactor, notifyService); } @Override diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/UserAdminService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/UserAdminService.java index 4baf8fa2..c8241d9c 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/UserAdminService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/UserAdminService.java @@ -27,21 +27,47 @@ public UserAdminService(UserRepo userRepo, @Qualifier("usernameValidator") Valid this.usernameValidator = usernameValidator; } + /** + * Validates a given username + * @param username the username to validate + * @return the name in a sanitised form + * @exception IllegalArgumentException if the username is unsuitable + */ + public String validateUsername(String username) { + username = trimAndRequire(username, "Username not supplied.").toLowerCase(); + usernameValidator.checkArgument(username); + return username; + } + /** * Creates a new user with the given username. * The username will be stripped and made lower case. * @param username the username + * @param role the role for the new user * @return the newly created user * @exception IllegalArgumentException if the username is unsuitable * @exception EntityExistsException if the user already exists */ - public User addUser(String username) { - username = trimAndRequire(username, "Username not supplied.").toLowerCase(); - usernameValidator.checkArgument(username); + public User addUser(String username, User.Role role) { + username = validateUsername(username); + requireNonNull(role, "User role is null"); if (userRepo.findByUsername(username).isPresent()) { throw new EntityExistsException("User already exists: "+username); } - return userRepo.save(new User(null, username, User.Role.normal)); + return userRepo.save(new User(null, username, role)); + } + + /** + * Creates a new user with the given username. + * The username will be stripped and made lower case. + * @param creator the user responsible for creating this new user + * @param username the username + * @return the newly created user + * @exception IllegalArgumentException if the username is unsuitable + * @exception EntityExistsException if the user already exists + */ + public User addNormalUser(User creator, String username) { + return addUser(username, User.Role.normal); } /** diff --git a/src/main/java/uk/ac/sanger/sccp/stan/service/work/WorkTypeService.java b/src/main/java/uk/ac/sanger/sccp/stan/service/work/WorkTypeService.java index d5f167a9..4c6d5967 100644 --- a/src/main/java/uk/ac/sanger/sccp/stan/service/work/WorkTypeService.java +++ b/src/main/java/uk/ac/sanger/sccp/stan/service/work/WorkTypeService.java @@ -3,10 +3,10 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.WorkType; import uk.ac.sanger.sccp.stan.repo.WorkTypeRepo; -import uk.ac.sanger.sccp.stan.service.BaseAdminService; -import uk.ac.sanger.sccp.stan.service.Validator; +import uk.ac.sanger.sccp.stan.service.*; import java.util.Optional; @@ -18,8 +18,9 @@ public class WorkTypeService extends BaseAdminService { @Autowired public WorkTypeService(WorkTypeRepo workTypeRepo, - @Qualifier("workTypeNameValidator") Validator nameValidator) { - super(workTypeRepo, "WorkType", "Name", nameValidator); + @Qualifier("workTypeNameValidator") Validator nameValidator, + Transactor transactor, AdminNotifyService notifyService) { + super(workTypeRepo, "WorkType", "Name", nameValidator, transactor, notifyService); } @Override diff --git a/src/main/resources/application-dev.properties b/src/main/resources/application-dev.properties index dc0ad7f4..3305718a 100644 --- a/src/main/resources/application-dev.properties +++ b/src/main/resources/application-dev.properties @@ -27,6 +27,7 @@ stan.mail.sender=Stan dev stan.mail.alert_recipients= stan.mail.service_description=Stan dev stan.mail.release_cc= +stan.mail.admin_notify={} stan.root=http://localhost:8080/ uk.ac.sanger.sccp.stan.apikeys=${STAN_APIKEYS:{'devapikey':'patch'}} stan.store.root=${HOME}/stan_files diff --git a/src/main/resources/application-test.properties b/src/main/resources/application-test.properties index 07486cbe..e54d04d6 100644 --- a/src/main/resources/application-test.properties +++ b/src/main/resources/application-test.properties @@ -23,6 +23,7 @@ storelight.host=${STORELIGHT_HOST:http://localhost:8081/graphql} storelight.apikey=${STORELIGHT_APIKEY:devel} stan.mail.sender=Stan test stan.mail.alert_recipients= +stan.mail.admin_notify={'apple':true, 'banana':false} stan.mail.service_description=Stan test stan.mail.release_cc=beagledev@sanger.ac.uk stan.root=stantestroot/ diff --git a/src/main/resources/schema.graphqls b/src/main/resources/schema.graphqls index 61b17e5c..4264c6cd 100644 --- a/src/main/resources/schema.graphqls +++ b/src/main/resources/schema.graphqls @@ -1973,6 +1973,8 @@ Send information to the application. These typically require a user with the suitable permission for the particular request. """ type Mutation { + """Log in with your Sanger id and create an end user account.""" + registerAsEndUser(username: String!, password: String!): LoginResult! """Log in with the given credentials.""" login(username: String!, password: String!): LoginResult! """Log out; end the current login session.""" diff --git a/src/test/java/uk/ac/sanger/sccp/stan/TestLogin.java b/src/test/java/uk/ac/sanger/sccp/stan/TestLogin.java index bf01f9b4..4049080f 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/TestLogin.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/TestLogin.java @@ -10,6 +10,7 @@ import org.springframework.test.context.ActiveProfiles; import uk.ac.sanger.sccp.stan.model.User; import uk.ac.sanger.sccp.stan.repo.UserRepo; +import uk.ac.sanger.sccp.stan.service.AdminNotifyServiceImp; import uk.ac.sanger.sccp.stan.service.LDAPService; import java.util.*; @@ -17,6 +18,8 @@ import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.*; +import static uk.ac.sanger.sccp.stan.integrationtest.IntegrationTestUtils.chainGet; + /** * @author dr6 */ @@ -30,6 +33,8 @@ public class TestLogin { private LDAPService mockLdapService; @MockBean private UserRepo mockUserRepo; + @MockBean + private AdminNotifyServiceImp mockNotifyService; @Autowired private GraphQLTester tester; @@ -65,4 +70,29 @@ public void testGetUser() throws Exception { Map>> userResult = tester.post(query); assertEquals(Map.of("data", Map.of("user", Map.of("username", "dr6", "role", "normal"))), userResult); } + + @Test + public void testSelfRegister() throws Exception { + final String mutation = "mutation { registerAsEndUser(username: \"user1\", password: \"42\") " + + "{ user { username role } } }"; + User newUser = new User(100, "user1", User.Role.enduser); + when(mockNotifyService.issue(any(), any(), any())).thenReturn(true); + when(mockUserRepo.findByUsername(any())).thenReturn(Optional.empty()); + when(mockUserRepo.save(any())).thenReturn(newUser); + when(mockLdapService.verifyCredentials("user1", "42")).thenReturn(true); + when(mockNotifyService.issue(any(), any(), any())).thenReturn(true); + + Object response = tester.post(mutation); + Map userData = chainGet(response, "data", "registerAsEndUser", "user"); + assertEquals("user1", userData.get("username")); + assertEquals("enduser", userData.get("role")); + verify(mockUserRepo, atLeastOnce()).findByUsername(eq("user1")); + verify(mockUserRepo).save(new User("user1", User.Role.enduser)); + verify(mockLdapService).verifyCredentials("user1", "42"); + verify(mockNotifyService).issue("user", "New user created on %service", + "User user1 has registered themself as enduser on %service."); + + verify(tester.mockAuthComp).setAuthentication(eq(new UsernamePasswordAuthenticationToken(newUser, "42", new ArrayList<>())), anyInt()); + + } } diff --git a/src/test/java/uk/ac/sanger/sccp/stan/config/TestMailConfig.java b/src/test/java/uk/ac/sanger/sccp/stan/config/TestMailConfig.java new file mode 100644 index 00000000..03e9b7be --- /dev/null +++ b/src/test/java/uk/ac/sanger/sccp/stan/config/TestMailConfig.java @@ -0,0 +1,27 @@ +package uk.ac.sanger.sccp.stan.config; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ActiveProfiles; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +@SpringBootTest +@ActiveProfiles(profiles = "test") +class TestMailConfig { + @Autowired + MailConfig mailConfig; + + @ParameterizedTest + @CsvSource({"Apple, true", + "APPLE, true", + "banana, false", + "BANANA, false", + "orange, true", + }) + public void testIsAdminNotificationEnabled(String name, boolean expected) { + assertEquals(expected, mailConfig.isAdminNotificationEnabled(name)); + } +} \ No newline at end of file diff --git a/src/test/java/uk/ac/sanger/sccp/stan/repo/TestUserRepo.java b/src/test/java/uk/ac/sanger/sccp/stan/repo/TestUserRepo.java new file mode 100644 index 00000000..d24cab1b --- /dev/null +++ b/src/test/java/uk/ac/sanger/sccp/stan/repo/TestUserRepo.java @@ -0,0 +1,65 @@ +package uk.ac.sanger.sccp.stan.repo; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ActiveProfiles; +import uk.ac.sanger.sccp.stan.model.User; + +import javax.persistence.EntityNotFoundException; +import javax.transaction.Transactional; +import java.util.stream.IntStream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** Tests {@link UserRepo} */ +@SpringBootTest +@ActiveProfiles(profiles = "test") +class TestUserRepo { + @Autowired + UserRepo userRepo; + + @ParameterizedTest + @ValueSource(booleans={false,true}) + @Transactional + void testFindByUsername(boolean exists) { + if (exists) { + User user = userRepo.save(new User("bananas", User.Role.normal)); + assertThat(userRepo.findByUsername("BANANAS")).contains(user); + } else { + assertThat(userRepo.findByUsername("BANANAS")).isEmpty(); + } + } + + @ParameterizedTest + @ValueSource(booleans={false,true}) + @Transactional + void testGetByUsername(boolean exists) { + if (exists) { + User user = userRepo.save(new User("bananas", User.Role.normal)); + assertEquals(user, userRepo.getByUsername("BANANAS")); + } else { + assertThat(assertThrows(EntityNotFoundException.class, () -> userRepo.getByUsername("BANANAS"))) + .hasMessage("User not found: \"BANANAS\""); + } + } + + @Test + @Transactional + void testFindByRole() { + userRepo.deleteAll(); + var adminUsers = userRepo.saveAll(IntStream.rangeClosed(1,2) + .mapToObj(i -> new User("admin"+i, User.Role.admin)) + .toList()); + var normalUsers = userRepo.saveAll(IntStream.rangeClosed(1,2) + .mapToObj(i -> new User("user"+i, User.Role.normal)) + .toList()); + assertThat(userRepo.findAllByRole(User.Role.admin)).containsExactlyInAnyOrderElementsOf(adminUsers); + assertThat(userRepo.findAllByRole(User.Role.normal)).containsExactlyInAnyOrderElementsOf(normalUsers); + assertThat(userRepo.findAllByRole(User.Role.disabled)).isEmpty(); + } +} \ No newline at end of file diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/AdminServiceTestUtils.java b/src/test/java/uk/ac/sanger/sccp/stan/service/AdminServiceTestUtils.java index 51c04979..a4f489d6 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/AdminServiceTestUtils.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/AdminServiceTestUtils.java @@ -1,10 +1,14 @@ package uk.ac.sanger.sccp.stan.service; import org.apache.commons.lang3.function.TriFunction; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.function.Executable; import org.junit.jupiter.params.provider.Arguments; import org.springframework.data.repository.CrudRepository; +import uk.ac.sanger.sccp.stan.EntityFactory; +import uk.ac.sanger.sccp.stan.Transactor; import uk.ac.sanger.sccp.stan.model.HasEnabled; +import uk.ac.sanger.sccp.stan.model.User; import javax.persistence.EntityExistsException; import javax.persistence.EntityNotFoundException; @@ -16,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import static uk.ac.sanger.sccp.stan.Matchers.mockTransactor; /** * Utils class for tests of admin services @@ -30,6 +35,9 @@ public abstract class AdminServiceTestUtils newEntityFunction, BiFunction> repoFindFunction, String missingStringMessage) { @@ -39,22 +47,34 @@ protected AdminServiceTestUtils(String entityTypeName, BiFunction serviceAddFunction, + protected void genericTestAddNew(TriFunction serviceAddFunction, String string, String existingEntityString, Exception expectedException, String expectedResultString) { + mockTransactor(mockTransactor); when(repoFindFunction.apply(mockRepo, string)).thenReturn(Optional.ofNullable(existingEntityString).map(s -> newEntity(14, s))); + User user = EntityFactory.getUser(); if (expectedException != null) { - assertException(expectedException, () -> serviceAddFunction.apply(service, string)); + assertException(expectedException, () -> serviceAddFunction.apply(service, user, string)); verify(mockRepo, never()).save(any()); return; } E expectedResult = newEntity(20, expectedResultString); when(mockRepo.save(any())).thenReturn(expectedResult); - assertSame(expectedResult, serviceAddFunction.apply(service, string)); + assertSame(expectedResult, serviceAddFunction.apply(service, user, string)); verify(mockRepo).save(newEntity(null, expectedResultString)); + if (user.getRole()!= User.Role.enduser) { + verifyNoInteractions(mockNotifyService); + } + verify(mockTransactor).transact(any(), any()); } protected String expectedMessage(Exception expectedException) { diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestAdminNotifyService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestAdminNotifyService.java new file mode 100644 index 00000000..8960f84e --- /dev/null +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestAdminNotifyService.java @@ -0,0 +1,90 @@ +package uk.ac.sanger.sccp.stan.service; + +import org.junit.jupiter.api.*; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.*; +import uk.ac.sanger.sccp.stan.config.MailConfig; +import uk.ac.sanger.sccp.stan.model.User; +import uk.ac.sanger.sccp.stan.repo.UserRepo; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.*; + +/** Tests {@link AdminNotifyServiceImp} */ +class TestAdminNotifyService { + @Mock + MailConfig mockMailConfig; + @Mock + UserRepo mockUserRepo; + @Mock + EmailService mockEmailService; + + @InjectMocks + AdminNotifyServiceImp service; + + private AutoCloseable mocking; + + @BeforeEach + void setup() { + mocking = MockitoAnnotations.openMocks(this); + service = spy(service); + } + + @AfterEach + void cleanup() throws Exception { + mocking.close(); + } + + @ParameterizedTest + @ValueSource(booleans={false,true}) + void isNotificationEnabled(boolean enabled) { + final String name = "Banana"; + when(mockMailConfig.isAdminNotificationEnabled(name)).thenReturn(enabled); + assertEquals(enabled, service.isNotificationEnabled(name)); + } + + @Test + void substitute() { + when(mockMailConfig.getServiceDescription()).thenReturn("Stan test"); + assertEquals("Alpha Stan test beta", service.substitute("Alpha %service beta")); + assertEquals("", service.substitute("")); + assertNull(service.substitute(null)); + } + + @ParameterizedTest + @ValueSource(ints={0,1,2,3}) + void issue(int phase) { + // 0 disabled + // 1 no admin + // 2 send fails + // 3 success + final String name = "alpha"; + doReturn(phase!=0).when(service).isNotificationEnabled(name); + List admins = (phase==1 ? List.of() : List.of(new User(1, "admin1", User.Role.admin), + new User(2, "admin2", User.Role.admin))); + when(mockUserRepo.findAllByRole(User.Role.admin)).thenReturn(admins); + doReturn(phase!=2).when(service).sendNotification(any(), any(), any()); + + assertEquals(phase==3, service.issue(name, "%service heading", "Body %service")); + + if (phase >= 2) { + verify(service).sendNotification(List.of("admin1", "admin2"), "%service heading", "Body %service"); + } else { + verify(service, never()).sendNotification(any(), any(), any()); + } + } + + @ParameterizedTest + @ValueSource(booleans={false,true}) + void sendNotification(boolean result) { + when(mockMailConfig.getServiceDescription()).thenReturn("Stan test"); + when(mockEmailService.tryEmail(any(), any(), any())).thenReturn(result); + List recipients = List.of("admin1", "admin2"); + assertEquals(result, service.sendNotification(recipients, "%service heading", "Body %service")); + verify(mockEmailService).tryEmail(recipients, "Stan test heading", "Body Stan test"); + } +} \ No newline at end of file diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestAuthService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestAuthService.java new file mode 100644 index 00000000..a2f04b92 --- /dev/null +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestAuthService.java @@ -0,0 +1,186 @@ +package uk.ac.sanger.sccp.stan.service; + +import org.junit.jupiter.api.*; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.*; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; +import uk.ac.sanger.sccp.stan.AuthenticationComponent; +import uk.ac.sanger.sccp.stan.Transactor; +import uk.ac.sanger.sccp.stan.config.SessionConfig; +import uk.ac.sanger.sccp.stan.model.User; +import uk.ac.sanger.sccp.stan.repo.UserRepo; +import uk.ac.sanger.sccp.stan.request.LoginResult; + +import java.util.ArrayList; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.*; +import static uk.ac.sanger.sccp.stan.Matchers.mockTransactor; + +/** Test {@link AuthServiceImp} */ +class TestAuthService { + @Mock + private SessionConfig mockSessionConfig; + @Mock + private UserRepo mockUserRepo; + @Mock + private AuthenticationComponent mockAuthComp; + @Mock + private LDAPService mockLdapService; + @Mock + private UserAdminService mockUserAdminService; + @Mock + private AdminNotifyService mockNotifyService; + @Mock + private Transactor mockTransactor; + + @InjectMocks + private AuthServiceImp service; + + private AutoCloseable mocking; + + @BeforeEach + void setup() { + mocking = MockitoAnnotations.openMocks(this); + service = spy(service); + } + + @AfterEach + void cleanup() throws Exception { + mocking.close(); + } + + @ParameterizedTest + @ValueSource(booleans={true,false}) + void testLoggedInUsername(boolean loggedIn) { + String username; + if (loggedIn) { + username = "user1"; + User user = new User(100, username, User.Role.normal); + Authentication auth = mock(Authentication.class); + when(auth.getPrincipal()).thenReturn(user); + when(mockAuthComp.getAuthentication()).thenReturn(auth); + } else { + username = null; + when(mockAuthComp.getAuthentication()).thenReturn(null); + } + assertEquals(username, service.loggedInUsername()); + } + + @ParameterizedTest + @CsvSource({"normal,true", + ",true", + "disabled,true", + "normal,false", + }) + void testLogin(User.Role role, boolean valid) { + String username = "un"; + String password = "pw"; + final int maxInactiveMinutes = 10; + User foundUser = role==null ? null : new User(100, username, role); + when(mockLdapService.verifyCredentials(username, password)).thenReturn(valid); + when(mockUserRepo.findByUsername(username)).thenReturn(Optional.ofNullable(foundUser)); + when(mockSessionConfig.getMaxInactiveMinutes()).thenReturn(maxInactiveMinutes); + LoginResult result = service.logIn(username, password); + String expectedMessage; + User expectedUser = null; + if (role==null) { + expectedMessage = "Username not in database."; + } else if (role== User.Role.disabled) { + expectedMessage = "Username is disabled."; + } else if (!valid) { + expectedMessage = "Login failed."; + } else { + expectedUser = foundUser; + expectedMessage = "OK"; + } + assertEquals(expectedMessage, result.getMessage()); + assertSame(expectedUser, result.getUser()); + if (result.getUser()!=null) { + Authentication authentication = new UsernamePasswordAuthenticationToken(foundUser, password, new ArrayList<>()); + verify(mockAuthComp).setAuthentication(authentication, maxInactiveMinutes); + } else { + verifyNoInteractions(mockAuthComp); + } + } + + @Test + void testLogOut() { + service.logOut(); + verify(mockAuthComp).setAuthentication(null, 0); + } + + @ParameterizedTest + @CsvSource({ + ", true", + "normal, true", + "disabled, true", + ", false", + }) + void testSelfRegister(User.Role existingRole, boolean valid) { + final int maxInactiveMinutes = 10; + final String sanUsername = "user1"; + final String password = "pw"; + final String inputUsername = " USER1"; + final User.Role suppliedRole = User.Role.enduser; + mockTransactor(mockTransactor); + when(mockSessionConfig.getMaxInactiveMinutes()).thenReturn(maxInactiveMinutes); + User existingUser = (existingRole==null ? null : new User(100, sanUsername, existingRole)); + when(mockUserAdminService.validateUsername(inputUsername)).thenReturn(sanUsername); + when(mockLdapService.verifyCredentials(sanUsername, password)).thenReturn(valid); + when(mockUserRepo.findByUsername(sanUsername)).thenReturn(Optional.ofNullable(existingUser)); + User newUser; + if (existingRole==null && valid) { + newUser = new User(200, sanUsername, suppliedRole); + when(mockUserAdminService.addUser(sanUsername, suppliedRole)).thenReturn(newUser); + } else { + newUser = null; + } + doNothing().when(service).sendNewUserEmail(any()); + User expectedUser; + String expectedMessage; + LoginResult result = service.selfRegister(inputUsername, password, suppliedRole); + if (newUser!=null) { + expectedUser = newUser; + expectedMessage = "OK"; + } else if (valid && existingRole != User.Role.disabled) { + expectedUser = existingUser; + expectedMessage = "OK"; + } else { + expectedUser = null; + expectedMessage = valid ? "Username is disabled." : "Authentication failed."; + } + + assertSame(expectedUser, result.getUser()); + assertEquals(expectedMessage, result.getMessage()); + + if (expectedUser!=null) { + Authentication authentication = new UsernamePasswordAuthenticationToken(expectedUser, password, new ArrayList<>()); + verify(mockAuthComp).setAuthentication(authentication, maxInactiveMinutes); + } else { + verifyNoInteractions(mockAuthComp); + } + if (newUser!=null) { + verify(service).sendNewUserEmail(newUser); + } else { + verify(service, never()).sendNewUserEmail(any()); + } + verify(mockTransactor).transact(any(), any()); + } + + + @Test + void testSendNewUserEmail() { + User newUser = new User(100, "user1", User.Role.enduser); + + service.sendNewUserEmail(newUser); + + verify(mockNotifyService).issue("user", "New user created on %service", + "User user1 has registered themself as enduser on %service."); + } +} \ No newline at end of file diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestCostCodeService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestCostCodeService.java index 0d1bcb29..7d6d758a 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestCostCodeService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestCostCodeService.java @@ -1,18 +1,23 @@ package uk.ac.sanger.sccp.stan.service; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import uk.ac.sanger.sccp.stan.model.CostCode; +import uk.ac.sanger.sccp.stan.model.User; import uk.ac.sanger.sccp.stan.repo.CostCodeRepo; -import static org.mockito.Mockito.mock; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.Mockito.*; +import static uk.ac.sanger.sccp.stan.Matchers.mockTransactor; /** * Tests {@link CostCodeService} * @author dr6 */ public class TestCostCodeService extends AdminServiceTestUtils { + public TestCostCodeService() { super("CostCode", CostCode::new, CostCodeRepo::findByCode, "Code not supplied."); @@ -21,7 +26,7 @@ public TestCostCodeService() { @BeforeEach void setup() { mockRepo = mock(CostCodeRepo.class); - service = new CostCodeService(mockRepo, simpleValidator()); + service = spy(new CostCodeService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService)); } @ParameterizedTest @@ -31,6 +36,28 @@ public void testAddNew(String string, String existingString, Exception expectedE string, existingString, expectedException, expectedResultString); } + @Test + public void testAddNewByEndUser() { + mockTransactor(mockTransactor); + User creator = new User(100, "user1", User.Role.enduser); + CostCode cc = new CostCode(200, "BANANA"); + when(mockRepo.save(any())).thenReturn(cc); + doNothing().when(service).sendNewEntityEmail(any(), any()); + assertSame(cc, service.addNew(creator, "banana")); + verify(mockTransactor).transact(eq("Add CostCode"), any()); + verify(service).sendNewEntityEmail(creator, cc); + } + + @Test + public void testSendNewEntityEmail() { + CostCode item = new CostCode(100, "BANANAS"); + User creator = new User(1, "jeff", User.Role.enduser); + + service.sendNewEntityEmail(creator, item); + verify(mockNotifyService).issue("costcode", "%service new CostCode", + "User jeff has created a new CostCode on %service: BANANAS"); + } + @ParameterizedTest @MethodSource("setEnabledArgs") public void testSetEnabled(String string, boolean newValue, Boolean oldValue, Exception expectedException) { diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestDestructionReasonAdminService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestDestructionReasonAdminService.java index e429dbd7..7e31069b 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestDestructionReasonAdminService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestDestructionReasonAdminService.java @@ -21,7 +21,7 @@ public TestDestructionReasonAdminService() { @BeforeEach void setup() { mockRepo = mock(DestructionReasonRepo.class); - service = new DestructionReasonAdminService(mockRepo, simpleValidator()); + service = new DestructionReasonAdminService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestEmailService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestEmailService.java index 5628ad29..c6d0a734 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestEmailService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestEmailService.java @@ -148,4 +148,30 @@ public void testTryReleaseEmail(boolean success, boolean anyWork) { verify(service).send(desc+" release", body + path, new String[] { recipient }, ccArray); } + + @ParameterizedTest + @CsvSource({",", + "alpha, alpha@sanger.ac.uk", + "beta@gamma, beta@gamma", + }) + public void testUsernameToEmail(String input, String expected) { + assertEquals(expected, service.usernameToEmail(input)); + } + + @ParameterizedTest + @ValueSource(booleans={false,true}) + public void testTryEmail(boolean succeeds) { + if (succeeds) { + doNothing().when(service).send(any(), any(), any(), any()); + } else { + doThrow(RuntimeException.class).when(service).send(any(), any(), any(), any()); + } + List recipients = List.of("alpha", "beta@gamma"); + assertEquals(succeeds, service.tryEmail(recipients, "Email header", + "Email body.")); + verify(service).send("Email header", + "Email body.", + new String[] { "alpha@sanger.ac.uk", "beta@gamma" }, + null); + } } diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestFixativeService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestFixativeService.java index 390abdf4..b0778d57 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestFixativeService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestFixativeService.java @@ -21,7 +21,7 @@ public TestFixativeService() { @BeforeEach void setup() { mockRepo = mock(FixativeRepo.class); - service = new FixativeService(mockRepo, simpleValidator()); + service = new FixativeService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestHmdmcAdminService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestHmdmcAdminService.java index 1c38ca69..43f4fe32 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestHmdmcAdminService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestHmdmcAdminService.java @@ -20,7 +20,7 @@ public TestHmdmcAdminService() { @BeforeEach void setup() { mockRepo = mock(HmdmcRepo.class); - service = new HmdmcAdminService(mockRepo, simpleValidator()); + service = new HmdmcAdminService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestOmeroProjectAdminService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestOmeroProjectAdminService.java index 5db8ed94..19ea48dc 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestOmeroProjectAdminService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestOmeroProjectAdminService.java @@ -20,7 +20,7 @@ public TestOmeroProjectAdminService() { @BeforeEach void setup() { mockRepo = mock(OmeroProjectRepo.class); - service = new OmeroProjectAdminService(mockRepo, simpleValidator()); + service = new OmeroProjectAdminService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestProbePanelService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestProbePanelService.java index 7be32a0b..3f162ce0 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestProbePanelService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestProbePanelService.java @@ -20,7 +20,7 @@ public TestProbePanelService() { @BeforeEach void setup() { mockRepo = mock(ProbePanelRepo.class); - service = new ProbePanelService(mockRepo, simpleValidator()); + service = new ProbePanelService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestProjectService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestProjectService.java index 36710524..d1a8cd95 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestProjectService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestProjectService.java @@ -1,12 +1,17 @@ package uk.ac.sanger.sccp.stan.service; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import uk.ac.sanger.sccp.stan.model.Project; +import uk.ac.sanger.sccp.stan.model.User; import uk.ac.sanger.sccp.stan.repo.ProjectRepo; -import static org.mockito.Mockito.mock; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; +import static uk.ac.sanger.sccp.stan.Matchers.mockTransactor; /** * Tests {@link ProjectService} @@ -21,7 +26,7 @@ public TestProjectService() { @BeforeEach void setup() { mockRepo = mock(ProjectRepo.class); - service = new ProjectService(mockRepo, simpleValidator()); + service = spy(new ProjectService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService)); } @ParameterizedTest @@ -31,6 +36,28 @@ public void testAddNew(String string, String existingString, Exception expectedE string, existingString, expectedException, expectedResultString); } + @Test + public void testAddNewByEndUser() { + mockTransactor(mockTransactor); + User creator = new User(100, "user1", User.Role.enduser); + Project project = new Project(200, "Bananas"); + when(mockRepo.save(any())).thenReturn(project); + doNothing().when(service).sendNewEntityEmail(any(), any()); + assertSame(project, service.addNew(creator, "Bananas")); + verify(service).sendNewEntityEmail(creator, project); + verify(mockTransactor).transact(eq("Add Project"), any()); + } + + @Test + public void testSendNewEntityEmail() { + Project item = new Project(100, "Bananas"); + User creator = new User(1, "jeff", User.Role.enduser); + + service.sendNewEntityEmail(creator, item); + verify(mockNotifyService).issue("project", "%service new Project", + "User jeff has created a new Project on %service: Bananas"); + } + @ParameterizedTest @MethodSource("setEnabledArgs") public void testSetEnabled(String string, boolean newValue, Boolean oldValue, Exception expectedException) { diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestReleaseDestinationAdminService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestReleaseDestinationAdminService.java index 53af66fc..0403ccdc 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestReleaseDestinationAdminService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestReleaseDestinationAdminService.java @@ -20,7 +20,7 @@ public TestReleaseDestinationAdminService() { @BeforeEach void setup() { mockRepo = mock(ReleaseDestinationRepo.class); - service = new ReleaseDestinationAdminService(mockRepo, simpleValidator()); + service = new ReleaseDestinationAdminService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestReleaseRecipientAdminService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestReleaseRecipientAdminService.java index adbba1a6..ee0ae2ef 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestReleaseRecipientAdminService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestReleaseRecipientAdminService.java @@ -27,7 +27,7 @@ public TestReleaseRecipientAdminService() { @BeforeEach void setup() { mockRepo = mock(ReleaseRecipientRepo.class); - service = new ReleaseRecipientAdminService(mockRepo, simpleValidator()); + service = new ReleaseRecipientAdminService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestSolutionAdminService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestSolutionAdminService.java index 1534b035..2d502fd9 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestSolutionAdminService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestSolutionAdminService.java @@ -21,7 +21,7 @@ public TestSolutionAdminService() { @BeforeEach void setup() { mockRepo = mock(SolutionRepo.class); - service = new SolutionAdminService(mockRepo, simpleValidator()); + service = new SolutionAdminService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestSpeciesAdminService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestSpeciesAdminService.java index 8471f907..59d5bccb 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestSpeciesAdminService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestSpeciesAdminService.java @@ -20,7 +20,7 @@ public TestSpeciesAdminService() { @BeforeEach void setup() { mockRepo = mock(SpeciesRepo.class); - service = new SpeciesAdminService(mockRepo, simpleValidator()); + service = new SpeciesAdminService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/TestUserAdminService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/TestUserAdminService.java index 4163ec02..fedaf90c 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/TestUserAdminService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/TestUserAdminService.java @@ -5,6 +5,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import uk.ac.sanger.sccp.stan.EntityFactory; import uk.ac.sanger.sccp.stan.Matchers; import uk.ac.sanger.sccp.stan.model.User; import uk.ac.sanger.sccp.stan.repo.UserRepo; @@ -35,8 +36,8 @@ void setup() { } @ParameterizedTest - @MethodSource("addUserArgs") - public void testAddUser(String username, String sanitisedUsername, Exception expectedException) { + @MethodSource("addNormalUserArgs") + public void testAddNormalUser(User creator, String username, String sanitisedUsername, Exception expectedException) { User existingUser; if (expectedException instanceof EntityExistsException) { existingUser = new User(13, sanitisedUsername, User.Role.normal); @@ -45,24 +46,25 @@ public void testAddUser(String username, String sanitisedUsername, Exception exp } when(mockUserRepo.findByUsername(sanitisedUsername)).thenReturn(Optional.ofNullable(existingUser)); if (expectedException != null) { - assertException(expectedException, () -> service.addUser(username)); + assertException(expectedException, () -> service.addNormalUser(creator, username)); verify(mockUserRepo, never()).save(any()); return; } User newUser = new User(14, sanitisedUsername, User.Role.normal); when(mockUserRepo.save(any())).thenReturn(newUser); - assertSame(newUser, service.addUser(username)); + assertSame(newUser, service.addNormalUser(creator, username)); verify(mockUserRepo).save(new User(null, sanitisedUsername, User.Role.normal)); } - static Stream addUserArgs() { + static Stream addNormalUserArgs() { + User creator = EntityFactory.getUser(); return Stream.of( - Arguments.of("Alpha", "alpha", null), - Arguments.of(" Alpha\t \n", "alpha", null), - Arguments.of("!Alpha", "!alpha", new IllegalArgumentException("username \"!alpha\" contains invalid characters \"!\".")), - Arguments.of(null, null, new IllegalArgumentException("Username not supplied.")), - Arguments.of(" \t\n ", null, new IllegalArgumentException("Username not supplied.")), - Arguments.of(" ALPHA ", "alpha", new EntityExistsException("User already exists: alpha")) + Arguments.of(creator, "Alpha", "alpha", null), + Arguments.of(creator, " Alpha\t \n", "alpha", null), + Arguments.of(creator, "!Alpha", "!alpha", new IllegalArgumentException("username \"!alpha\" contains invalid characters \"!\".")), + Arguments.of(creator, null, null, new IllegalArgumentException("Username not supplied.")), + Arguments.of(creator, " \t\n ", null, new IllegalArgumentException("Username not supplied.")), + Arguments.of(creator, " ALPHA ", "alpha", new EntityExistsException("User already exists: alpha")) ); } diff --git a/src/test/java/uk/ac/sanger/sccp/stan/service/work/TestWorkTypeService.java b/src/test/java/uk/ac/sanger/sccp/stan/service/work/TestWorkTypeService.java index 95122bf0..a810ca9e 100644 --- a/src/test/java/uk/ac/sanger/sccp/stan/service/work/TestWorkTypeService.java +++ b/src/test/java/uk/ac/sanger/sccp/stan/service/work/TestWorkTypeService.java @@ -22,7 +22,7 @@ public TestWorkTypeService() { @BeforeEach void setup() { mockRepo = mock(WorkTypeRepo.class); - service = new WorkTypeService(mockRepo, simpleValidator()); + service = new WorkTypeService(mockRepo, simpleValidator(), mockTransactor, mockNotifyService); } @ParameterizedTest