From 69fbf4d5f0ae2ab0d246afef6d50dd2371be2356 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Thu, 4 Aug 2022 09:33:23 -0500 Subject: [PATCH 01/10] basis create and retrieve of portlet-list shell --- .../portal/dao/portletlist/IPortletList.java | 25 ++ .../dao/portletlist/IPortletListDao.java | 18 ++ .../dao/portletlist/jpa/PortletList.java | 66 +++++ .../dao/portletlist/jpa/PortletListDao.java | 178 +++++++++++ .../PortletListRESTController.java | 252 ++++++++++++++++ .../portal/rest/utils/ErrorResponse.java | 14 + .../portletlist/IPortletListService.java | 21 ++ .../portletlist/PortletListService.java | 277 ++++++++++++++++++ .../resources/properties/db/hibernate.cfg.xml | 1 + 9 files changed, 852 insertions(+) create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/ErrorResponse.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java new file mode 100644 index 00000000000..59e6fe5f06b --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java @@ -0,0 +1,25 @@ +package org.apereo.portal.dao.portletlist; + +import org.dom4j.Element; + +public interface IPortletList { + + String getId(); + + String getUserId(); + + void setUserId(String userId); + + String getName(); + + void setName(String name); + + //Set getListItems(); + + //void setListItems(Set items); + + /** Supports exporting. */ + void toElement(Element parent); + + String toString(); +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java new file mode 100644 index 00000000000..925a3eb3cb5 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java @@ -0,0 +1,18 @@ +package org.apereo.portal.dao.portletlist; + +import java.util.List; +import java.util.Set; + +public interface IPortletListDao { + +// public IPersonAttributesGroupDefinition updatePersonAttributesGroupDefinition( +// IPersonAttributesGroupDefinition personAttributesGroupDefinition); +// +// public void deletePersonAttributesGroupDefinition(IPersonAttributesGroupDefinition definition); + + public List getPortletLists(String userId); + + public IPortletList getPortletList(String userId, String portletListUuid); + + public IPortletList createPortletList(IPortletList toCreate); +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java new file mode 100644 index 00000000000..86daed87f3b --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java @@ -0,0 +1,66 @@ +package org.apereo.portal.dao.portletlist.jpa; + +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; +import lombok.extern.slf4j.Slf4j; +import org.apereo.portal.dao.portletlist.IPortletList; +import org.dom4j.Element; +import org.hibernate.annotations.CacheConcurrencyStrategy; +import org.hibernate.annotations.GenericGenerator; +//import org.hibernate.annotations.NaturalIdCache; +import javax.persistence.*; +import java.util.UUID; + +@Getter +@Setter +@EqualsAndHashCode(onlyExplicitlyIncluded = true) +@ToString +@Slf4j +@Entity +@Table( + name = "UP_PORTLET_LIST", + uniqueConstraints = { @UniqueConstraint(columnNames = { "user_id", "name" }) }) +//@NaturalIdCache( +// region = +// "org.apereo.portal.groups.pags.dao.jpa.PersonAttributesGroupDefinitionImpl-NaturalId") +@Cacheable +@org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) +@SuppressWarnings("unused") +public class PortletList implements IPortletList { + + @Id + @GeneratedValue(generator = "UUID") + @GenericGenerator( + name = "UUID", + strategy = "org.hibernate.id.UUIDGenerator" + ) + @Column(name = "id", updatable = false, nullable = false) + private String id; + + @Column(name = "user_id", updatable = true, nullable = false) + private String userId; + + @Column(name = "name", updatable = true, nullable = false) + private String name; + + @Override + public void toElement(Element parent) { + if (parent == null) { + String msg = "Argument 'parent' cannot be null."; + throw new IllegalArgumentException(msg); + } + + parent.addElement("id").addText(this.getId().toString()); + parent.addElement("name").addText(this.getName()); + parent.addElement("userid").addText(this.getUserId()); +// if (!members.isEmpty()) { +// org.dom4j.Element elementMembers = DocumentHelper.createElement(new QName("members")); +// for (IPersonAttributesGroupDefinition member : members) { +// elementMembers.addElement("member-name").addText(member.getName()); +// } +// parent.add(elementMembers); +// } + } +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java new file mode 100644 index 00000000000..5a2977e9db3 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java @@ -0,0 +1,178 @@ +package org.apereo.portal.dao.portletlist.jpa; + +import com.google.common.base.Function; +import lombok.extern.slf4j.Slf4j; +import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.dao.portletlist.IPortletListDao; +import org.apereo.portal.groups.pags.dao.IPersonAttributesGroupDefinition; +import org.apereo.portal.groups.pags.dao.jpa.PersonAttributesGroupDefinitionImpl; +import org.apereo.portal.groups.pags.dao.jpa.PersonAttributesGroupDefinitionImpl_; +import org.apereo.portal.jpa.BasePortalJpaDao; +import org.dom4j.Element; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Repository; + +import javax.persistence.TypedQuery; +import javax.persistence.criteria.*; +import javax.portlet.Portlet; +import java.util.*; + +@Slf4j +@Repository("portletListDao") +public class PortletListDao extends BasePortalJpaDao implements IPortletListDao { + + private CriteriaQuery portletListsByUserIdQuery; + + private CriteriaQuery portletListByUserIdAndPortletListUuidQuery; + + private ParameterExpression userIdParameter; + + private ParameterExpression portletListUuidParameter; + + @Override + public void afterPropertiesSet() throws Exception { + this.userIdParameter = this.createParameterExpression(String.class, "user_id"); + this.portletListUuidParameter = this.createParameterExpression(String.class, "id"); + +// this.findAllDefinitionsQuery = +// this.createCriteriaQuery( +// new Function< +// CriteriaBuilder, +// CriteriaQuery>() { +// @Override +// public CriteriaQuery apply( +// CriteriaBuilder cb) { +// final CriteriaQuery +// criteriaQuery = +// cb.createQuery( +// PersonAttributesGroupDefinitionImpl.class); +// criteriaQuery.from(PersonAttributesGroupDefinitionImpl.class); +// return criteriaQuery; +// } +// }); + + this.portletListsByUserIdQuery = + this.createCriteriaQuery( + new Function< + CriteriaBuilder, + CriteriaQuery>() { + @Override + public CriteriaQuery apply( + CriteriaBuilder cb) { + final CriteriaQuery + criteriaQuery = + cb.createQuery( + PortletList.class); + Root root = + criteriaQuery.from( + PortletList.class); + criteriaQuery + .select(root) + .where(cb.equal(root.get("userId"), userIdParameter)); + return criteriaQuery; + } + }); + + this.portletListByUserIdAndPortletListUuidQuery = + this.createCriteriaQuery( + new Function< + CriteriaBuilder, + CriteriaQuery>() { + @Override + public CriteriaQuery apply( + CriteriaBuilder cb) { + final CriteriaQuery + criteriaQuery = + cb.createQuery( + PortletList.class); + Root root = + criteriaQuery.from( + PortletList.class); + criteriaQuery + .select(root) + .where(cb.and(cb.equal(root.get("userId"), userIdParameter), + cb.equal(root.get("id"), portletListUuidParameter))); + return criteriaQuery; + } + }); +// +// this.parentGroupDefinitionsQuery = +// this.createCriteriaQuery( +// new Function< +// CriteriaBuilder, +// CriteriaQuery>() { +// @Override +// public CriteriaQuery apply( +// CriteriaBuilder cb) { +// final CriteriaQuery +// criteriaQuery = +// cb.createQuery( +// PersonAttributesGroupDefinitionImpl.class); +// Root root = +// criteriaQuery.from( +// PersonAttributesGroupDefinitionImpl.class); +// Join< +// PersonAttributesGroupDefinitionImpl, +// PersonAttributesGroupDefinitionImpl> +// members = +// root.join( +// PersonAttributesGroupDefinitionImpl_ +// .members); +// criteriaQuery.where( +// cb.equal( +// members.get( +// PersonAttributesGroupDefinitionImpl_.name), +// nameParameter)); +// return criteriaQuery; +// } +// }); + } + + @PortalTransactionalReadOnly + @Override + public List getPortletLists( + String userId) { + TypedQuery query = + this.createCachedQuery(portletListsByUserIdQuery); + query.setParameter(userIdParameter, userId); + List entities = new ArrayList<>(query.getResultList()); + return entities; + } + + @PortalTransactionalReadOnly + @Override + public IPortletList getPortletList( + String userId, + String portletListUuid) { + TypedQuery query = + this.createCachedQuery(portletListByUserIdAndPortletListUuidQuery); + query.setParameter(userIdParameter, userId); + query.setParameter(portletListUuidParameter, portletListUuid); + + List lists = query.getResultList(); + if(lists.size() < 1) { + return null; + } else if(lists.size() > 1) { + log.error("Expected to up to 1 portlet list for user [{}] and portlet list uuid [{}], but found [{}].", userId, portletListUuid, lists.size()); + return null; + } + + return lists.get(0); + } + + @SuppressWarnings("unused") + @PortalTransactional + @Override + public IPortletList createPortletList(IPortletList toCreate) { + log.debug("Persisting portlet list [{}] for user [{}]", toCreate.getName(), toCreate.getUserId()); + try { + this.getEntityManager().persist(toCreate); + } catch (Exception e) { + log.debug("Failed to persist portlet list", e); + throw e; + } + log.debug("Finished persisting portlet list [{}] for user [{}]. ID = [{}]", toCreate.getName(), toCreate.getUserId(), toCreate.getId().toString()); + return toCreate; + } +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java new file mode 100644 index 00000000000..5db87fbb2a6 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java @@ -0,0 +1,252 @@ +package org.apereo.portal.rest.portletlist; + +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.extern.slf4j.Slf4j; +import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.dao.portletlist.jpa.PortletList; +import org.apereo.portal.rest.utils.ErrorResponse; +import org.apereo.portal.security.RuntimeAuthorizationException; +import org.apereo.portal.services.portletlist.IPortletListService; +import org.apereo.portal.security.IPerson; +import org.apereo.portal.security.IPersonManager; +import org.json.JSONObject; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.http.MediaType; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.*; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.util.List; + +import static org.springframework.web.bind.annotation.RequestMethod.GET; +import static org.springframework.web.bind.annotation.RequestMethod.POST; +import static org.springframework.web.bind.annotation.RequestMethod.PUT; + +/** PortletListRESTController provides a REST endpoint for interacting with portlet lists. */ + +@Controller +@Slf4j +public class PortletListRESTController { + public static final String CONTEXT = "/portlet-list/"; + @Autowired + private IPortletListService portletListService; + + @Autowired + private IPersonManager personManager; + + @Autowired private ObjectMapper objectMapper; + + /** Provide a JSON view of all portlet lists. */ +// @PreAuthorize( +// "hasPermission('ALL', 'java.lang.String', new org.apereo.portal.spring.security.evaluator.AuthorizableActivity('UP_PERMISSIONS', 'VIEW_PERMISSIONS'))") + @RequestMapping( + value = CONTEXT, + method = GET, + produces = MediaType.APPLICATION_JSON_VALUE) + public @ResponseBody String getPortletLists(HttpServletRequest request, HttpServletResponse response) { + final IPerson person = personManager.getPerson(request); + log.debug("Person.isGuest() = {}", person.isGuest()); + log.debug("Person.getUserName() = {}", person.getUserName()); + + if(person.isGuest()) { + log.warn("Guest is trying to access portlet-list API, which is not allowed."); + return respondPortletListJson(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + } + + List pLists = portletListService.getPortletLists(person); + return respondPortletListJson(response, pLists, null, HttpServletResponse.SC_OK); + } + + /** Provide a JSON view of all portlet lists. */ +// @PreAuthorize( +// "hasPermission('ALL', 'java.lang.String', new org.apereo.portal.spring.security.evaluator.AuthorizableActivity('UP_PERMISSIONS', 'VIEW_PERMISSIONS'))") + @RequestMapping( + value = CONTEXT + "{portletListUuid}", + method = GET, + produces = MediaType.APPLICATION_JSON_VALUE) + public @ResponseBody String getPortletList(HttpServletRequest request, HttpServletResponse response, @PathVariable String portletListUuid) { + final IPerson person = personManager.getPerson(request); + log.debug("Person.isGuest() = {}", person.isGuest()); + log.debug("Person.getUserName() = {}", person.getUserName()); + + if(person.isGuest()) { + log.warn("Guest is trying to access portlet-list API, which is not allowed."); + return respondPortletListJson(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + } + + IPortletList pList = portletListService.getPortletList(person, portletListUuid); + if(pList == null) { + return respondPortletListJson(response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); + } + return respondPortletListJson(response, pList, null, HttpServletResponse.SC_OK); + } + + @RequestMapping( + value = CONTEXT, + method = POST, + produces = MediaType.APPLICATION_JSON_VALUE) + public @ResponseBody String createPortletList( + HttpServletRequest request, + HttpServletResponse response, + @RequestBody String json) { + + final IPerson person = personManager.getPerson(request); + log.debug("createPortletList > Person.isGuest() = {}", person.isGuest()); + log.debug("createPortletList > Person.getUserName() = {}", person.getUserName()); + log.debug("createPortletList > JSON body is = {}", json); + + if(person.isGuest()) { + log.warn("createPortletList > Guest is trying to access portlet-list API, which is not allowed."); + return respondPortletListJson(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + + } + +// /* +// * This step is necessary; the incoming URLs will sometimes have '+' +// * characters for spaces, and the @PathVariable magic doesn't convert them. +// */ +// String name; +// try { +// name = URLDecoder.decode(parentGroupName, "UTF-8"); +// } catch (UnsupportedEncodingException e) { +// respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_BAD_REQUEST); +// } + + IPortletList input; + + try { + input = objectMapper.readValue(json, PortletList.class); + + // TODO This should be a default instead of a reset... + input.setUserId("" + person.getID()); + } catch (Exception e) { + return respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + + try { + final IPortletList created = portletListService.createPortletList( + person, input); + response.setHeader("Location", created.getId()); + return respondPortletListJson(response, null, null, HttpServletResponse.SC_CREATED); + } catch (RuntimeAuthorizationException rae) { + return respondPortletListJson(response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); + } catch (IllegalArgumentException iae) { + return respondPortletListJson(response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + } catch (DataIntegrityViolationException dive) { + log.warn("Attempted violation of data integrity when creating a portlet list {}", dive); + return respondPortletListJson(response, null, "Data integrity issue - likely tried to use a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); + } catch (Exception e) { + e.printStackTrace(); + log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName()); + return respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + } + + // WIP +// @RequestMapping( +// value = CONTEXT + "{portletListUuid}", +// method = PUT, +// produces = MediaType.APPLICATION_JSON_VALUE) +// public @ResponseBody String updatePortletList( +// HttpServletRequest request, +// HttpServletResponse response, +// @RequestBody String json, +// @PathVariable String portletListUuid) { +// +// final IPerson person = personManager.getPerson(request); +// log.debug("updatePortletList > Person.isGuest() = {}", person.isGuest()); +// log.debug("updatePortletList > Person.getUserName() = {}", person.getUserName()); +// log.debug("updatePortletList > JSON body is = {}", json); +// +// if(person.isGuest()) { +// log.warn("updatePortletList > Guest is trying to access portlet-list API, which is not allowed."); +// return respondPortletListJson(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); +// +// } +// +//// /* +//// * This step is necessary; the incoming URLs will sometimes have '+' +//// * characters for spaces, and the @PathVariable magic doesn't convert them. +//// */ +//// String name; +//// try { +//// name = URLDecoder.decode(parentGroupName, "UTF-8"); +//// } catch (UnsupportedEncodingException e) { +//// respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_BAD_REQUEST); +//// } +// +// IPortletList input; +// +// try { +// input = objectMapper.readValue(json, PortletList.class); +// +// // TODO This should be a default instead of a reset... +// input.setUserId("" + person.getID()); +// } catch (Exception e) { +// return respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); +// } +// +// try { +// final IPortletList created = portletListService.createPortletList( +// person, input); +// response.setHeader("Location", created.getId()); +// return respondPortletListJson(response, null, null, HttpServletResponse.SC_CREATED); +// } catch (RuntimeAuthorizationException rae) { +// return respondPortletListJson(response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); +// } catch (IllegalArgumentException iae) { +// return respondPortletListJson(response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); +// } catch (DataIntegrityViolationException dive) { +// log.warn("Attempted violation of data integrity when creating a portlet list {}", dive); +// return respondPortletListJson(response, null, "Data integrity issue - likely tried to use a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); +// } catch (Exception e) { +// e.printStackTrace(); +// log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName()); +// return respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); +// } +// } +// Maybe later: +// final EntityIdentifier ei = person.getEntityIdentifier(); +// final IAuthorizationPrincipal ap = +// AuthorizationServiceFacade.instance().newPrincipal(ei.getKey(), ei.getType()); +// if (!ap.hasPermission("UP_SYSTEM", "IMPORT_ENTITY", target)) { +// response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); +// return; +// } + + private String respondPortletListJson( + HttpServletResponse response, + Object returnPayload, + String errorMessage, + int status) { + response.setContentType(MediaType.APPLICATION_JSON_VALUE); + Object payloadToReturn = returnPayload; + int statusToReturn = status; + + if (returnPayload == null && errorMessage != null) { + payloadToReturn = new ErrorResponse(errorMessage); + } + + log.debug("returnPayload is null = {}, errorMessage is null = {}, final return payload is null = {}", + (returnPayload == null), + (errorMessage == null), + (payloadToReturn == null)); + + try { + response.setStatus(statusToReturn); + // If there is no payload, and no error, return a null body in the response + final String payloadAsString = ((returnPayload == null) && (errorMessage == null)) ? null : objectMapper.writeValueAsString(payloadToReturn); + log.debug("Prepared JSON Response - response code [{}], object type [{}], JSON as string: {}", + statusToReturn, + (payloadToReturn == null) ? "NULL" : payloadToReturn.getClass().getCanonicalName(), + payloadAsString); + return payloadAsString; + } catch (Exception e) { + log.error("Unable to write out payload object as JSON. Returning a 500.", e); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return null; + } + } +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/ErrorResponse.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/ErrorResponse.java new file mode 100644 index 00000000000..dcd6780a668 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/ErrorResponse.java @@ -0,0 +1,14 @@ +package org.apereo.portal.rest.utils; + +import lombok.Getter; +import lombok.Setter; + +@Getter +@Setter +public class ErrorResponse { + private String message; + + public ErrorResponse(String message) { + this.message = message; + } +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java new file mode 100644 index 00000000000..1886d788273 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java @@ -0,0 +1,21 @@ +package org.apereo.portal.services.portletlist; + +import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.security.IPerson; + +import java.util.List; +import java.util.Set; + +public interface IPortletListService { + public List getPortletLists(IPerson owner); + + /** + * Returns null if not found + * @param owner + * @param portletListUuid + * @return + */ + public IPortletList getPortletList(IPerson owner, String portletListUuid); + + public IPortletList createPortletList(IPerson owner, IPortletList toCreate); +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java new file mode 100644 index 00000000000..cf453fb0939 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java @@ -0,0 +1,277 @@ +package org.apereo.portal.services.portletlist; + +import org.apereo.portal.dao.portletlist.IPortletListDao; +import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.security.IPerson; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +import java.util.List; + +import lombok.extern.slf4j.Slf4j; +/** + * Service layer that sits atop the DAO layer and enforces permissions and/or business rules that + * apply to CRUD operations on portlet lists. External clients should interact with this service + * -- instead of the DAOs directly -- whenever the actions are undertaken on behalf of a specific + * user. + */ +@Service +@Slf4j +public final class PortletListService implements IPortletListService { + + @Autowired + private IPortletListDao portletListDao; + + /** + * All the definitions, filtered by the user's access rights. + * + * @param person + * @return + */ + @Override + public List getPortletLists(IPerson person) { + List rslt = portletListDao.getPortletLists("" + person.getID()); +// for (IPortletList pList : +// portletListDao.getPortletLists("" + person.getID())) { + // TODO - getPortletLists only returns lists for that user, so no permissions needed... however - need to implement admin access +// if (hasPermission( +// person, +// IPermission.VIEW_GROUP_ACTIVITY, +// def.getCompositeEntityIdentifierForGroup().getKey())) { +// rslt.add(def); +// } +// rslt.add(pList); +// } + log.debug("Returning portlet lists '{}' for user '{}'", rslt, person.getUserName()); + return rslt; + } + + /** + * All the definitions, filtered by the user's access rights. + * + * @param person + * @return + */ + @Override + public IPortletList getPortletList(IPerson person, String portletListUuid) { + IPortletList rslt = portletListDao.getPortletList("" + person.getID(), portletListUuid); + log.debug("Returning portlet list '{}' for user '{}'", rslt, person.getUserName()); + return rslt; + } + + /** TODO docs Verifies permissions and that the group doesn't already exist (case insensitive) */ + + @Override + public IPortletList createPortletList(IPerson owner, IPortletList toCreate) { +// // What's the target of the upcoming permissions check? +// String target = +// parent != null +// ? parent.getEntityIdentifier().getKey() +// : IPermission +// .ALL_GROUPS_TARGET; // Must have blanket permission to create one +// // w/o a parent +// +// // Verify permission +// if (!hasPermission(person, IPermission.CREATE_GROUP_ACTIVITY, target)) { +// throw new RuntimeAuthorizationException( +// person, IPermission.CREATE_GROUP_ACTIVITY, target); +// } +// +// // VALIDATION STEP: The group name & description are allowable +// if (StringUtils.isBlank(groupName)) { +// throw new IllegalArgumentException("Specified groupName is blank: " + groupName); +// } +// if (!GROUP_NAME_VALIDATOR_PATTERN.matcher(groupName).matches()) { +// throw new IllegalArgumentException( +// "Specified groupName is too long, too short, or contains invalid characters: " +// + groupName); +// } +// if (!StringUtils.isBlank(description)) { // Blank description is allowable +// if (!GROUP_DESC_VALIDATOR_PATTERN.matcher(description).matches()) { +// throw new IllegalArgumentException( +// "Specified description is too long or contains invalid characters: " +// + description); +// } +// } +// +// // VALIDATION STEP: We don't have a group by that name already +// EntityIdentifier[] people = +// GroupService.searchForGroups( +// groupName, IGroupConstants.SearchMethod.DISCRETE_CI, IPerson.class); +// EntityIdentifier[] portlets = +// GroupService.searchForGroups( +// groupName, +// IGroupConstants.SearchMethod.DISCRETE_CI, +// IPortletDefinition.class); +// if (people.length != 0 || portlets.length != 0) { +// throw new IllegalArgumentException("Specified groupName already in use: " + groupName); +// } + log.debug("Using DAO to create portlet list [{}] for user [{}]", toCreate.getName(), toCreate.getUserId()); + return portletListDao.createPortletList(toCreate); + } +// +// /** +// * Returns the specified definitions, provided (1) it exists and (2) the user may view it. +// * +// * @param person +// * @return +// */ +// public IPersonAttributesGroupDefinition getPagsDefinitionByName(IPerson person, String name) { +// IPersonAttributesGroupDefinition rslt = getPagsGroupDefByName(name); +// if (rslt == null) { +// // Better to produce exception? I'm thinking not, but open-minded. +// return null; +// } +// if (!hasPermission( +// person, +// IPermission.VIEW_GROUP_ACTIVITY, +// rslt.getCompositeEntityIdentifierForGroup().getKey())) { +// throw new RuntimeAuthorizationException(person, IPermission.VIEW_GROUP_ACTIVITY, name); +// } +// logger.debug("Returning PAGS definition '{}' for user '{}'", rslt, person.getUserName()); +// return rslt; +// } +// +// /** Verifies permissions and that the group doesn't already exist (case insensitive) */ +// public IPersonAttributesGroupDefinition createPagsDefinition( +// IPerson person, IEntityGroup parent, String groupName, String description) { +// +// // What's the target of the upcoming permissions check? +// String target = +// parent != null +// ? parent.getEntityIdentifier().getKey() +// : IPermission +// .ALL_GROUPS_TARGET; // Must have blanket permission to create one +// // w/o a parent +// +// // Verify permission +// if (!hasPermission(person, IPermission.CREATE_GROUP_ACTIVITY, target)) { +// throw new RuntimeAuthorizationException( +// person, IPermission.CREATE_GROUP_ACTIVITY, target); +// } +// +// // VALIDATION STEP: The group name & description are allowable +// if (StringUtils.isBlank(groupName)) { +// throw new IllegalArgumentException("Specified groupName is blank: " + groupName); +// } +// if (!GROUP_NAME_VALIDATOR_PATTERN.matcher(groupName).matches()) { +// throw new IllegalArgumentException( +// "Specified groupName is too long, too short, or contains invalid characters: " +// + groupName); +// } +// if (!StringUtils.isBlank(description)) { // Blank description is allowable +// if (!GROUP_DESC_VALIDATOR_PATTERN.matcher(description).matches()) { +// throw new IllegalArgumentException( +// "Specified description is too long or contains invalid characters: " +// + description); +// } +// } +// +// // VALIDATION STEP: We don't have a group by that name already +// EntityIdentifier[] people = +// GroupService.searchForGroups( +// groupName, IGroupConstants.SearchMethod.DISCRETE_CI, IPerson.class); +// EntityIdentifier[] portlets = +// GroupService.searchForGroups( +// groupName, +// IGroupConstants.SearchMethod.DISCRETE_CI, +// IPortletDefinition.class); +// if (people.length != 0 || portlets.length != 0) { +// throw new IllegalArgumentException("Specified groupName already in use: " + groupName); +// } +// +// IPersonAttributesGroupDefinition rslt = +// pagsGroupDefDao.createPersonAttributesGroupDefinition(groupName, description); +// if (parent != null) { +// // Should refactor this switch to instead choose a service and invoke a method on it +// switch (parent.getServiceName().toString()) { +// case SERVICE_NAME_LOCAL: +// IEntityGroup member = +// GroupService.findGroup( +// rslt.getCompositeEntityIdentifierForGroup().getKey()); +// if (member == null) { +// String msg = +// "The specified group was created, but is not present in the store: " +// + rslt.getName(); +// throw new RuntimeException(msg); +// } +// parent.addChild(member); +// parent.updateMembers(); +// break; +// case SERVICE_NAME_PAGS: +// IPersonAttributesGroupDefinition parentDef = +// getPagsGroupDefByName(parent.getName()); +// Set members = +// new HashSet<>(parentDef.getMembers()); +// members.add(rslt); +// parentDef.setMembers(members); +// pagsGroupDefDao.updatePersonAttributesGroupDefinition(parentDef); +// break; +// default: +// String msg = +// "The specified group service does not support adding members: " +// + parent.getServiceName(); +// throw new UnsupportedOperationException(msg); +// } +// } +// +// return rslt; +// } +// +// /** NOTE -- This method assumes that pagsDef is an existing JPA-managed entity. */ +// public IPersonAttributesGroupDefinition updatePagsDefinition( +// IPerson person, IPersonAttributesGroupDefinition pagsDef) { +// +// // Verify permission +// if (!hasPermission( +// person, +// IPermission.EDIT_GROUP_ACTIVITY, +// pagsDef.getCompositeEntityIdentifierForGroup().getKey())) { +// throw new RuntimeAuthorizationException( +// person, +// IPermission.EDIT_GROUP_ACTIVITY, +// pagsDef.getCompositeEntityIdentifierForGroup().getKey()); +// } +// +// IPersonAttributesGroupDefinition rslt = +// pagsGroupDefDao.updatePersonAttributesGroupDefinition(pagsDef); +// return rslt; +// } +// +// /** NOTE -- This method assumes that pagsDef is an existing JPA-managed entity. */ +// public void deletePagsDefinition(IPerson person, IPersonAttributesGroupDefinition pagsDef) { +// +// // Verify permission +// if (!hasPermission( +// person, +// IPermission.DELETE_GROUP_ACTIVITY, +// pagsDef.getCompositeEntityIdentifierForGroup().getKey())) { +// throw new RuntimeAuthorizationException( +// person, +// IPermission.DELETE_GROUP_ACTIVITY, +// pagsDef.getCompositeEntityIdentifierForGroup().getKey()); +// } +// +// pagsGroupDefDao.deletePersonAttributesGroupDefinition(pagsDef); +// } +// +// /* +// * Implementation +// */ +// +// private boolean hasPermission(IPerson person, String permission, String target) { +// EntityIdentifier ei = person.getEntityIdentifier(); +// IAuthorizationPrincipal ap = +// AuthorizationServiceFacade.instance().newPrincipal(ei.getKey(), ei.getType()); +// return ap.hasPermission(IPermission.PORTAL_GROUPS, permission, target); +// } +// +// private IPersonAttributesGroupDefinition getPagsGroupDefByName(String name) { +// Set pagsGroups = +// pagsGroupDefDao.getPersonAttributesGroupDefinitionByName(name); +// if (pagsGroups.size() > 1) { +// logger.error("More than one PAGS group with name {} found.", name); +// } +// return pagsGroups.isEmpty() ? null : pagsGroups.iterator().next(); +// } +} diff --git a/uPortal-webapp/src/main/resources/properties/db/hibernate.cfg.xml b/uPortal-webapp/src/main/resources/properties/db/hibernate.cfg.xml index ba60eeb2499..bd4e5c2a69b 100644 --- a/uPortal-webapp/src/main/resources/properties/db/hibernate.cfg.xml +++ b/uPortal-webapp/src/main/resources/properties/db/hibernate.cfg.xml @@ -80,5 +80,6 @@ + From d27e7a77ebe51502756ff13488d1f0ba4697c876 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Tue, 9 Aug 2022 14:21:40 -0500 Subject: [PATCH 02/10] WIP - portlet list items as entity --- .../portal/dao/portletlist/IPortletList.java | 9 + .../dao/portletlist/IPortletListDao.java | 7 +- .../dao/portletlist/IPortletListItem.java | 17 ++ .../dao/portletlist/jpa/PortletList.java | 44 ++- .../dao/portletlist/jpa/PortletListDao.java | 110 +++++--- .../dao/portletlist/jpa/PortletListItem.java | 47 ++++ .../portletlist/jpa/PortletListItemPK.java | 88 ++++++ .../PortletListRESTController.java | 263 ++++++++++-------- .../portletlist/IPortletListService.java | 13 +- .../portletlist/PortletListService.java | 55 ++-- .../resources/properties/db/hibernate.cfg.xml | 1 + 11 files changed, 458 insertions(+), 196 deletions(-) create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListItem.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItemPK.java diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java index 59e6fe5f06b..d5c50e8bcce 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java @@ -1,7 +1,10 @@ package org.apereo.portal.dao.portletlist; +import org.apereo.portal.dao.portletlist.jpa.PortletListItem; import org.dom4j.Element; +import java.util.List; + public interface IPortletList { String getId(); @@ -14,6 +17,10 @@ public interface IPortletList { void setName(String name); + List getItems(); + + void setItems(List items); + //Set getListItems(); //void setListItems(Set items); @@ -22,4 +29,6 @@ public interface IPortletList { void toElement(Element parent); String toString(); + + void overrideItems(List items); } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java index 925a3eb3cb5..5bffc734543 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java @@ -1,7 +1,6 @@ package org.apereo.portal.dao.portletlist; import java.util.List; -import java.util.Set; public interface IPortletListDao { @@ -12,7 +11,11 @@ public interface IPortletListDao { public List getPortletLists(String userId); - public IPortletList getPortletList(String userId, String portletListUuid); + public List getPortletLists(); + + public IPortletList getPortletList(String portletListUuid); public IPortletList createPortletList(IPortletList toCreate); + + public IPortletList updatePortletList(IPortletList toUpdate, String portletListUuid); } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListItem.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListItem.java new file mode 100644 index 00000000000..dc27f619310 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListItem.java @@ -0,0 +1,17 @@ +package org.apereo.portal.dao.portletlist; + +import org.apereo.portal.dao.portletlist.jpa.PortletListItemPK; +import org.dom4j.Element; + +public interface IPortletListItem { + + PortletListItemPK getPortletListItemPK(); + + void setPortletListItemPK(PortletListItemPK portletListItemPK); + + String getEntityId(); + + void setEntityId(String id); + + String toString(); +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java index 86daed87f3b..6c84e4b1898 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java @@ -6,12 +6,17 @@ import lombok.ToString; import lombok.extern.slf4j.Slf4j; import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.dao.portletlist.IPortletListItem; import org.dom4j.Element; -import org.hibernate.annotations.CacheConcurrencyStrategy; -import org.hibernate.annotations.GenericGenerator; -//import org.hibernate.annotations.NaturalIdCache; +import org.hibernate.annotations.*; + import javax.persistence.*; -import java.util.UUID; +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.OrderBy; +import javax.persistence.Table; +import java.util.ArrayList; +import java.util.List; @Getter @Setter @@ -22,9 +27,6 @@ @Table( name = "UP_PORTLET_LIST", uniqueConstraints = { @UniqueConstraint(columnNames = { "user_id", "name" }) }) -//@NaturalIdCache( -// region = -// "org.apereo.portal.groups.pags.dao.jpa.PersonAttributesGroupDefinitionImpl-NaturalId") @Cacheable @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) @SuppressWarnings("unused") @@ -45,6 +47,16 @@ public class PortletList implements IPortletList { @Column(name = "name", updatable = true, nullable = false) private String name; + @OneToMany( + targetEntity = PortletListItem.class, + cascade = CascadeType.REMOVE, + fetch = FetchType.EAGER, + mappedBy = "portletListItemPK.portletList") + @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) + @Fetch(FetchMode.SELECT) // FM JOIN does BAD things to collections that support duplicates + @OrderBy("LIST_ORDER ASC") + private List items = new ArrayList<>(); + @Override public void toElement(Element parent) { if (parent == null) { @@ -55,12 +67,16 @@ public void toElement(Element parent) { parent.addElement("id").addText(this.getId().toString()); parent.addElement("name").addText(this.getName()); parent.addElement("userid").addText(this.getUserId()); -// if (!members.isEmpty()) { -// org.dom4j.Element elementMembers = DocumentHelper.createElement(new QName("members")); -// for (IPersonAttributesGroupDefinition member : members) { -// elementMembers.addElement("member-name").addText(member.getName()); -// } -// parent.add(elementMembers); -// } + parent.addElement("items").addText("" + this.getItems().size()); + + } + + public void overrideItems(List items) { + this.items.clear(); + int order = 0; + for(PortletListItem item : items) { + item.setPortletListItemPK(new PortletListItemPK(this, order++)); + this.items.add(item); + } } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java index 5a2977e9db3..33bf70edc94 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java @@ -4,27 +4,22 @@ import lombok.extern.slf4j.Slf4j; import org.apereo.portal.dao.portletlist.IPortletList; import org.apereo.portal.dao.portletlist.IPortletListDao; -import org.apereo.portal.groups.pags.dao.IPersonAttributesGroupDefinition; -import org.apereo.portal.groups.pags.dao.jpa.PersonAttributesGroupDefinitionImpl; -import org.apereo.portal.groups.pags.dao.jpa.PersonAttributesGroupDefinitionImpl_; import org.apereo.portal.jpa.BasePortalJpaDao; -import org.dom4j.Element; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.stereotype.Repository; import javax.persistence.TypedQuery; import javax.persistence.criteria.*; -import javax.portlet.Portlet; import java.util.*; @Slf4j @Repository("portletListDao") public class PortletListDao extends BasePortalJpaDao implements IPortletListDao { + private CriteriaQuery portletListsQuery; + private CriteriaQuery portletListsByUserIdQuery; - private CriteriaQuery portletListByUserIdAndPortletListUuidQuery; + private CriteriaQuery portletListByPortletListUuidQuery; private ParameterExpression userIdParameter; @@ -52,6 +47,27 @@ public void afterPropertiesSet() throws Exception { // } // }); + this.portletListsQuery = + this.createCriteriaQuery( + new Function< + CriteriaBuilder, + CriteriaQuery>() { + @Override + public CriteriaQuery apply( + CriteriaBuilder cb) { + final CriteriaQuery + criteriaQuery = + cb.createQuery( + PortletList.class); + Root root = + criteriaQuery.from( + PortletList.class); + criteriaQuery + .select(root); + return criteriaQuery; + } + }); + this.portletListsByUserIdQuery = this.createCriteriaQuery( new Function< @@ -74,7 +90,7 @@ public CriteriaQuery apply( } }); - this.portletListByUserIdAndPortletListUuidQuery = + this.portletListByPortletListUuidQuery = this.createCriteriaQuery( new Function< CriteriaBuilder, @@ -91,42 +107,10 @@ public CriteriaQuery apply( PortletList.class); criteriaQuery .select(root) - .where(cb.and(cb.equal(root.get("userId"), userIdParameter), - cb.equal(root.get("id"), portletListUuidParameter))); + .where(cb.equal(root.get("id"), portletListUuidParameter)); return criteriaQuery; } }); -// -// this.parentGroupDefinitionsQuery = -// this.createCriteriaQuery( -// new Function< -// CriteriaBuilder, -// CriteriaQuery>() { -// @Override -// public CriteriaQuery apply( -// CriteriaBuilder cb) { -// final CriteriaQuery -// criteriaQuery = -// cb.createQuery( -// PersonAttributesGroupDefinitionImpl.class); -// Root root = -// criteriaQuery.from( -// PersonAttributesGroupDefinitionImpl.class); -// Join< -// PersonAttributesGroupDefinitionImpl, -// PersonAttributesGroupDefinitionImpl> -// members = -// root.join( -// PersonAttributesGroupDefinitionImpl_ -// .members); -// criteriaQuery.where( -// cb.equal( -// members.get( -// PersonAttributesGroupDefinitionImpl_.name), -// nameParameter)); -// return criteriaQuery; -// } -// }); } @PortalTransactionalReadOnly @@ -140,21 +124,28 @@ public List getPortletLists( return entities; } + @PortalTransactionalReadOnly + @Override + public List getPortletLists() { + TypedQuery query = + this.createCachedQuery(portletListsQuery); + List entities = new ArrayList<>(query.getResultList()); + return entities; + } + @PortalTransactionalReadOnly @Override public IPortletList getPortletList( - String userId, String portletListUuid) { TypedQuery query = - this.createCachedQuery(portletListByUserIdAndPortletListUuidQuery); - query.setParameter(userIdParameter, userId); + this.createCachedQuery(portletListByPortletListUuidQuery); query.setParameter(portletListUuidParameter, portletListUuid); List lists = query.getResultList(); if(lists.size() < 1) { return null; } else if(lists.size() > 1) { - log.error("Expected to up to 1 portlet list for user [{}] and portlet list uuid [{}], but found [{}].", userId, portletListUuid, lists.size()); + log.error("Expected to up to 1 portlet list for portlet list uuid [{}], but found [{}].", portletListUuid, lists.size()); return null; } @@ -175,4 +166,31 @@ public IPortletList createPortletList(IPortletList toCreate) { log.debug("Finished persisting portlet list [{}] for user [{}]. ID = [{}]", toCreate.getName(), toCreate.getUserId(), toCreate.getId().toString()); return toCreate; } + + @PortalTransactional + @Override + public IPortletList updatePortletList(IPortletList toUpdate, String portletListUuid) { + log.debug("Persisting changes for portlet list [{}] for user [{}]", toUpdate.getName(), toUpdate.getUserId()); + try { + IPortletList ref = this.getEntityManager().find(PortletList.class, portletListUuid); + if (ref == null) { + log.warn("Unable to find portlet list [{}] to update.", portletListUuid); + return null; + } + ref.overrideItems(toUpdate.getItems()); + +// List originalItems = toUpdate.getItems(); +// toUpdate.setItems(null); +// log.debug("Safe updating - removing items on portlet list [{}] for user [{}]", toUpdate.getName(), toUpdate.getUserId()); +// this.getEntityManager().persist(toUpdate); +// toUpdate.setItems(originalItems); + //log.debug("Safe updating - updating with current items on portlet list [{}] for user [{}]", toUpdate.getName(), toUpdate.getUserId()); + //this.getEntityManager().merge(toUpdate); + log.debug("Finished persisting changes for portlet list [{}] for user [{}]. ID = [{}]", toUpdate.getName(), toUpdate.getUserId(), portletListUuid); + } catch (Exception e) { + log.debug("Failed to persist changes for portlet list", e); + throw e; + } + return toUpdate; + } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java new file mode 100644 index 00000000000..d030b818605 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java @@ -0,0 +1,47 @@ +package org.apereo.portal.dao.portletlist.jpa; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; +import lombok.extern.slf4j.Slf4j; +import org.apereo.portal.dao.portletlist.IPortletListItem; +import org.hibernate.annotations.CacheConcurrencyStrategy; + +import javax.persistence.*; + +@Getter +@Setter +@EqualsAndHashCode(onlyExplicitlyIncluded = true) +@ToString +@Slf4j +@Entity +@Table( + // This is ONLY to be used as part of a portlet list, so not specifying a PK + name = "UP_PORTLET_LIST_ITEM", + uniqueConstraints = { + // These are sets of lists + @UniqueConstraint(columnNames = { "LIST_ID", "ENTITY_ID" }), + }) +@Cacheable +@org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) +@SuppressWarnings("unused") +public class PortletListItem implements IPortletListItem { + + @JsonIgnore + @EmbeddedId + private PortletListItemPK portletListItemPK; + + // This is generally the portlet fname, but could be adjusted in the future + @Column(name = "ENTITY_ID", updatable = true, nullable = false) + private String entityId; + + public PortletListItem() { + // No-arg constructor for JSON mapping + } + + public PortletListItem(String entityId) { + this.entityId = entityId; + } +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItemPK.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItemPK.java new file mode 100644 index 00000000000..4b58ba34cd1 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItemPK.java @@ -0,0 +1,88 @@ +/** + * Licensed to Apereo under one or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information regarding copyright ownership. Apereo + * licenses this file to you under the Apache License, Version 2.0 (the "License"); you may not use + * this file except in compliance with the License. You may obtain a copy of the License at the + * following location: + * + *

http://www.apache.org/licenses/LICENSE-2.0 + * + *

Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apereo.portal.dao.portletlist.jpa; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import lombok.Getter; +import lombok.Setter; +import org.apereo.portal.dao.portletlist.IPortletList; +import org.hibernate.annotations.Cascade; + +import javax.persistence.*; +import java.io.Serializable; + +@Getter +@Setter +@SecondaryTable(name = "UP_PORTLET_LIST") +@Embeddable +@JsonIgnoreProperties({ "portletList" }) +public class PortletListItemPK implements Serializable { + + @ManyToOne(optional = false) + @JoinColumn( + name = "LIST_ID", + referencedColumnName = "ID") + protected PortletList portletList; + + @Column( + name = "LIST_ORDER", + unique = false, + nullable = false, + insertable = true, + updatable = true) + protected int listOrder; + + /** Empty constructor is needed for Serializable */ + public PortletListItemPK() {}; + + public PortletListItemPK(PortletList portletList, int listOrder) { + this.portletList = portletList; + this.listOrder = listOrder; + } + +// @Override +// public boolean equals(Object obj) { +// if (obj == null || !(obj instanceof org.apereo.portal.portlet.dao.jpa.MarketplaceRatingPK)) { +// return false; +// } else if (obj == this) { +// return true; +// } +// org.apereo.portal.portlet.dao.jpa.MarketplaceRatingPK tempRating = (org.apereo.portal.portlet.dao.jpa.MarketplaceRatingPK) obj; +// return new EqualsBuilder() +// .append(userName, tempRating.userName) +// .append(portletDefinition, tempRating.portletDefinition) +// .isEquals(); +// } +// +// @Override +// public int hashCode() { +// return new HashCodeBuilder(17, 31) +// . // two randomly chosen prime numbers +// append(userName) +// .append(portletDefinition) +// .toHashCode(); +// } +// +// @Override +// public String toString() { +// StringBuilder builder = new StringBuilder(); +// builder.append("User: "); +// builder.append(this.userName); +// builder.append("\n"); +// builder.append("Portlet: "); +// builder.append(this.portletDefinition); +// return builder.toString(); +// } +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java index 5db87fbb2a6..e1fdfde017c 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java @@ -2,28 +2,32 @@ import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang.ArrayUtils; import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.dao.portletlist.IPortletListItem; import org.apereo.portal.dao.portletlist.jpa.PortletList; +import org.apereo.portal.dao.portletlist.jpa.PortletListItem; +import org.apereo.portal.dao.portletlist.jpa.PortletListItemPK; import org.apereo.portal.rest.utils.ErrorResponse; import org.apereo.portal.security.RuntimeAuthorizationException; import org.apereo.portal.services.portletlist.IPortletListService; import org.apereo.portal.security.IPerson; import org.apereo.portal.security.IPersonManager; -import org.json.JSONObject; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.MediaType; import org.springframework.stereotype.Controller; +import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.*; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.util.ArrayList; import java.util.List; +import java.util.Set; -import static org.springframework.web.bind.annotation.RequestMethod.GET; -import static org.springframework.web.bind.annotation.RequestMethod.POST; -import static org.springframework.web.bind.annotation.RequestMethod.PUT; +import static org.springframework.web.bind.annotation.RequestMethod.*; /** PortletListRESTController provides a REST endpoint for interacting with portlet lists. */ @@ -39,49 +43,61 @@ public class PortletListRESTController { @Autowired private ObjectMapper objectMapper; - /** Provide a JSON view of all portlet lists. */ -// @PreAuthorize( -// "hasPermission('ALL', 'java.lang.String', new org.apereo.portal.spring.security.evaluator.AuthorizableActivity('UP_PERMISSIONS', 'VIEW_PERMISSIONS'))") + /** + * Provide a JSON view of all portlet lists. + * + * If an administrator makes this call, ALL portlet lists will be returned. + * Otherwise, only the portlet lists that the requester owns will be returned. + */ @RequestMapping( value = CONTEXT, method = GET, produces = MediaType.APPLICATION_JSON_VALUE) public @ResponseBody String getPortletLists(HttpServletRequest request, HttpServletResponse response) { final IPerson person = personManager.getPerson(request); - log.debug("Person.isGuest() = {}", person.isGuest()); - log.debug("Person.getUserName() = {}", person.getUserName()); + debugPerson("getAllPortletLists", person); if(person.isGuest()) { log.warn("Guest is trying to access portlet-list API, which is not allowed."); - return respondPortletListJson(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } - List pLists = portletListService.getPortletLists(person); - return respondPortletListJson(response, pLists, null, HttpServletResponse.SC_OK); + List pLists = portletListService.isPortletListAdmin(person) ? + portletListService.getPortletLists() : portletListService.getPortletLists(person); + return prepareResponse(response, pLists, null, HttpServletResponse.SC_OK); } - /** Provide a JSON view of all portlet lists. */ -// @PreAuthorize( -// "hasPermission('ALL', 'java.lang.String', new org.apereo.portal.spring.security.evaluator.AuthorizableActivity('UP_PERMISSIONS', 'VIEW_PERMISSIONS'))") + /** + * Provide a JSON view of a given portlet list. + * + * If an administrator makes the request, the portlet list will be returned, regardless of ownership. + * If anyone else makes the request, the portlet list will only be returned if the requester is the owner. + */ @RequestMapping( value = CONTEXT + "{portletListUuid}", method = GET, produces = MediaType.APPLICATION_JSON_VALUE) public @ResponseBody String getPortletList(HttpServletRequest request, HttpServletResponse response, @PathVariable String portletListUuid) { final IPerson person = personManager.getPerson(request); - log.debug("Person.isGuest() = {}", person.isGuest()); - log.debug("Person.getUserName() = {}", person.getUserName()); + debugPerson("getSpecificPortletList", person); if(person.isGuest()) { log.warn("Guest is trying to access portlet-list API, which is not allowed."); - return respondPortletListJson(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } - IPortletList pList = portletListService.getPortletList(person, portletListUuid); - if(pList == null) { - return respondPortletListJson(response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); + IPortletList pList = portletListService.getPortletList(portletListUuid); + + if(pList == null) { + log.warn("User [{}] tried to access portlet-list [{}], but list was not found.", person.getUserName(), portletListUuid); + return prepareResponse(response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); + } else if(!portletListService.isPortletListAdmin(person) && !pList.getUserId().equals("" + person.getID())) { + // Not an admin, and not the owner + log.warn("Non-admin user [{}][{}] tried to access portlet-list [{}] with owner [{}], but was blocked since they aren't the owner.", person.getID(), person.getUserName(), portletListUuid, pList.getUserId()); + return prepareResponse(response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); } - return respondPortletListJson(response, pList, null, HttpServletResponse.SC_OK); + + return prepareResponse(response, pList, null, HttpServletResponse.SC_OK); } @RequestMapping( @@ -94,129 +110,142 @@ public class PortletListRESTController { @RequestBody String json) { final IPerson person = personManager.getPerson(request); - log.debug("createPortletList > Person.isGuest() = {}", person.isGuest()); - log.debug("createPortletList > Person.getUserName() = {}", person.getUserName()); + debugPerson("createPortletList", person); log.debug("createPortletList > JSON body is = {}", json); if(person.isGuest()) { log.warn("createPortletList > Guest is trying to access portlet-list API, which is not allowed."); - return respondPortletListJson(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); - + return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } -// /* -// * This step is necessary; the incoming URLs will sometimes have '+' -// * characters for spaces, and the @PathVariable magic doesn't convert them. -// */ -// String name; -// try { -// name = URLDecoder.decode(parentGroupName, "UTF-8"); -// } catch (UnsupportedEncodingException e) { -// respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_BAD_REQUEST); -// } - - IPortletList input; + PortletList input; try { input = objectMapper.readValue(json, PortletList.class); - - // TODO This should be a default instead of a reset... - input.setUserId("" + person.getID()); + if(!portletListService.isPortletListAdmin(person)) { + // Default - non-admins can only create lists for themselves. + input.setUserId("" + person.getID()); + } else if (StringUtils.isEmpty(input.getUserId())) { + // Default - admins don't have to specify a user name + input.setUserId("" + person.getID()); + } } catch (Exception e) { - return respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + log.warn("User [{}][{}] tried to create a portlet-list with bad json - {}", person.getID(), person.getUserName(), e.getMessage()); + return prepareResponse(response, null, "Unparsable portlet-list JSON", HttpServletResponse.SC_BAD_REQUEST); } try { + // Sets the parent-child relationships + input.overrideItems(input.getItems()); final IPortletList created = portletListService.createPortletList( person, input); response.setHeader("Location", created.getId()); - return respondPortletListJson(response, null, null, HttpServletResponse.SC_CREATED); + return prepareResponse(response, null, null, HttpServletResponse.SC_CREATED); } catch (RuntimeAuthorizationException rae) { - return respondPortletListJson(response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); + log.warn("RuntimeAuthorizationException thrown - {}", rae.getMessage(), rae); + return prepareResponse(response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); } catch (IllegalArgumentException iae) { - return respondPortletListJson(response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + log.warn("IllegalArgumentException thrown - {}", iae.getMessage(), iae); + return prepareResponse(response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); } catch (DataIntegrityViolationException dive) { - log.warn("Attempted violation of data integrity when creating a portlet list {}", dive); - return respondPortletListJson(response, null, "Data integrity issue - likely tried to use a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); + log.warn("Attempted violation of data integrity when creating a portlet list {}", dive.getMessage(), dive); + return prepareResponse(response, null, "Data integrity issue - such as specifying a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); } catch (Exception e) { - e.printStackTrace(); - log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName()); - return respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName(), e); + return prepareResponse(response, null, "Something unexpected occurred. Please check with your System Administrator", HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } } - // WIP -// @RequestMapping( -// value = CONTEXT + "{portletListUuid}", -// method = PUT, -// produces = MediaType.APPLICATION_JSON_VALUE) -// public @ResponseBody String updatePortletList( -// HttpServletRequest request, -// HttpServletResponse response, -// @RequestBody String json, -// @PathVariable String portletListUuid) { -// -// final IPerson person = personManager.getPerson(request); -// log.debug("updatePortletList > Person.isGuest() = {}", person.isGuest()); -// log.debug("updatePortletList > Person.getUserName() = {}", person.getUserName()); -// log.debug("updatePortletList > JSON body is = {}", json); -// -// if(person.isGuest()) { -// log.warn("updatePortletList > Guest is trying to access portlet-list API, which is not allowed."); -// return respondPortletListJson(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + @RequestMapping( + value = CONTEXT + "{portletListUuid}", + method = PUT, + produces = MediaType.APPLICATION_JSON_VALUE) + public @ResponseBody String updatePortletList( + HttpServletRequest request, + HttpServletResponse response, + @RequestBody String json, + @PathVariable String portletListUuid) { + + final IPerson person = personManager.getPerson(request); + debugPerson("updatePortletList", person); + log.debug("updatePortletList > JSON body is = {}", json); + + if(person.isGuest()) { + log.warn("updatePortletList > Guest is trying to access portlet-list API, which is not allowed."); + return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + } + + IPortletList input; + try { + input = objectMapper.readValue(json, PortletList.class); + } catch (Exception e) { + log.warn("User [{}][{}] tried to create a portlet-list with bad json - {}", person.getID(), person.getUserName(), e.getMessage()); + return prepareResponse(response, null, "Unparsable portlet-list JSON", HttpServletResponse.SC_BAD_REQUEST); + } + +// // Overlay changes onto a known entity, and then persist that entity. +// IPortletList toUpdate = portletListService.getPortletList(portletListUuid); // +// if(toUpdate == null) { +// return prepareResponse(response, null, "Unknown portlet-list", HttpServletResponse.SC_NOT_FOUND); // } // -//// /* -//// * This step is necessary; the incoming URLs will sometimes have '+' -//// * characters for spaces, and the @PathVariable magic doesn't convert them. -//// */ -//// String name; -//// try { -//// name = URLDecoder.decode(parentGroupName, "UTF-8"); -//// } catch (UnsupportedEncodingException e) { -//// respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_BAD_REQUEST); -//// } +// if(portletListService.isPortletListAdmin(person)) { +// // If admin, allow admin-level changes +// if(!StringUtils.isEmpty(input.getUserId())) { +// toUpdate.setUserId(input.getUserId()); +// } // -// IPortletList input; +// if(!StringUtils.isEmpty(input.getName())) { +// toUpdate.setName(input.getName()); +// } +// } else if (toUpdate.getUserId().equals("" + person.getID())) { +// // If owner of portlet-list, allow only owner-level changes // -// try { -// input = objectMapper.readValue(json, PortletList.class); +// if(!StringUtils.isEmpty(input.getUserId())) { +// log.warn("updatePortletList - non-admin user [{}][{}] tried to update portlet-list [{}][{}] with a new owner [{}], which is not allowed.", person.getID(), person.getUserName(), toUpdate.getId(), toUpdate.getName(), input.getUserId()); +// return prepareResponse(response, null, "Non-admin user cannot change portlet-list owner", HttpServletResponse.SC_BAD_REQUEST); +// } +// } else { +// // Otherwise, disallow changes +// log.warn("updatePortletList - user [{}][{}] tried to update portlet-list [{}][{}], but was not the owner.", person.getID(), person.getUserName(), toUpdate.getId(), toUpdate.getName()); +// return prepareResponse(response, null, "Unknown portlet-list", HttpServletResponse.SC_UNAUTHORIZED); +// } // -// // TODO This should be a default instead of a reset... -// input.setUserId("" + person.getID()); -// } catch (Exception e) { -// return respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); +// // Either an owner or an admin. allow general-level changes: +// if(!StringUtils.isEmpty(input.getName())) { +// toUpdate.setName(input.getName()); // } // -// try { -// final IPortletList created = portletListService.createPortletList( -// person, input); -// response.setHeader("Location", created.getId()); -// return respondPortletListJson(response, null, null, HttpServletResponse.SC_CREATED); -// } catch (RuntimeAuthorizationException rae) { -// return respondPortletListJson(response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); -// } catch (IllegalArgumentException iae) { -// return respondPortletListJson(response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); -// } catch (DataIntegrityViolationException dive) { -// log.warn("Attempted violation of data integrity when creating a portlet list {}", dive); -// return respondPortletListJson(response, null, "Data integrity issue - likely tried to use a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); -// } catch (Exception e) { -// e.printStackTrace(); -// log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName()); -// return respondPortletListJson(response, null, e.getMessage(), HttpServletResponse.SC_INTERNAL_SERVER_ERROR); +// if(input.getItems() != null) { +// toUpdate.overrideItems(input.getItems()); // } -// } -// Maybe later: -// final EntityIdentifier ei = person.getEntityIdentifier(); -// final IAuthorizationPrincipal ap = -// AuthorizationServiceFacade.instance().newPrincipal(ei.getKey(), ei.getType()); -// if (!ap.hasPermission("UP_SYSTEM", "IMPORT_ENTITY", target)) { -// response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); -// return; -// } - - private String respondPortletListJson( + + try { + final IPortletList updated = portletListService.updatePortletList( + person, input, portletListUuid); + if(updated == null) { + log.warn("update returned null for portlet-list uuid [{}]. Failing request.", portletListUuid); + return prepareResponse(response, null, "Unable to update portlet list.", HttpServletResponse.SC_BAD_REQUEST); + } else { + return prepareResponse(response, null, null, HttpServletResponse.SC_OK); + } + } catch (RuntimeAuthorizationException rae) { + log.warn("RuntimeAuthorizationException thrown - {}", rae.getMessage(), rae); + return prepareResponse(response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); + } catch (IllegalArgumentException iae) { + log.warn("IllegalArgumentException thrown - {}", iae.getMessage(), iae); + return prepareResponse(response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + } catch (DataIntegrityViolationException dive) { + log.warn("Attempted violation of data integrity when updating a portlet list {}", dive.getMessage(), dive); + return prepareResponse(response, null, "Data integrity issue - such as specifying a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); + } catch (Exception e) { + log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName(), e); + return prepareResponse(response, null, "Something unexpected occurred. Please check with your System Administrator", HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + } + + private String prepareResponse( HttpServletResponse response, Object returnPayload, String errorMessage, @@ -249,4 +278,12 @@ private String respondPortletListJson( return null; } } + + private void debugPerson(String flow, IPerson person) { + log.debug("{} > Current user: username={}, isGuest={}, isAdmin={}", + flow, + person.getUserName(), + person.isGuest(), + portletListService.isPortletListAdmin(person)); + } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java index 1886d788273..5e1152b796a 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java @@ -1,21 +1,28 @@ package org.apereo.portal.services.portletlist; import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.security.IAuthorizationPrincipal; import org.apereo.portal.security.IPerson; import java.util.List; import java.util.Set; public interface IPortletListService { + public List getPortletLists(); + public List getPortletLists(IPerson owner); /** * Returns null if not found - * @param owner * @param portletListUuid * @return */ - public IPortletList getPortletList(IPerson owner, String portletListUuid); + public IPortletList getPortletList(String portletListUuid); + + public IPortletList createPortletList(IPerson creater, IPortletList toCreate); + + public IPortletList updatePortletList(IPerson updater, IPortletList toCreate, String portletListUuid); + + public boolean isPortletListAdmin(IPerson person); - public IPortletList createPortletList(IPerson owner, IPortletList toCreate); } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java index cf453fb0939..15ea84cb699 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java @@ -2,6 +2,9 @@ import org.apereo.portal.dao.portletlist.IPortletListDao; import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.security.AuthorizationPrincipalHelper; +import org.apereo.portal.security.IAuthorizationService; +import org.apereo.portal.security.IPermission; import org.apereo.portal.security.IPerson; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; @@ -22,6 +25,29 @@ public final class PortletListService implements IPortletListService { @Autowired private IPortletListDao portletListDao; + @Autowired + private IAuthorizationService authorizationService; + + public boolean isPortletListAdmin(IPerson person) { + return authorizationService.doesPrincipalHavePermission( + AuthorizationPrincipalHelper.principalFromUser(person), + IPermission.PORTAL_SYSTEM, + IPermission.ALL_PERMISSIONS_ACTIVITY, + IPermission.ALL_TARGET); + } + + /** + * All the definitions, filtered by the user's access rights. + * + * @return + */ + @Override + public List getPortletLists() { + List rslt = portletListDao.getPortletLists(); + log.debug("Returning {} portlet lists", rslt.size()); + return rslt; + } + /** * All the definitions, filtered by the user's access rights. * @@ -31,38 +57,25 @@ public final class PortletListService implements IPortletListService { @Override public List getPortletLists(IPerson person) { List rslt = portletListDao.getPortletLists("" + person.getID()); -// for (IPortletList pList : -// portletListDao.getPortletLists("" + person.getID())) { - // TODO - getPortletLists only returns lists for that user, so no permissions needed... however - need to implement admin access -// if (hasPermission( -// person, -// IPermission.VIEW_GROUP_ACTIVITY, -// def.getCompositeEntityIdentifierForGroup().getKey())) { -// rslt.add(def); -// } -// rslt.add(pList); -// } log.debug("Returning portlet lists '{}' for user '{}'", rslt, person.getUserName()); return rslt; } /** - * All the definitions, filtered by the user's access rights. + * Retrieve a specific portlet list * - * @param person + * @param portletListUuid * @return */ @Override - public IPortletList getPortletList(IPerson person, String portletListUuid) { - IPortletList rslt = portletListDao.getPortletList("" + person.getID(), portletListUuid); - log.debug("Returning portlet list '{}' for user '{}'", rslt, person.getUserName()); - return rslt; + public IPortletList getPortletList(String portletListUuid) { + return portletListDao.getPortletList(portletListUuid); } /** TODO docs Verifies permissions and that the group doesn't already exist (case insensitive) */ @Override - public IPortletList createPortletList(IPerson owner, IPortletList toCreate) { + public IPortletList createPortletList(IPerson creater, IPortletList toCreate) { // // What's the target of the upcoming permissions check? // String target = // parent != null @@ -109,6 +122,12 @@ public IPortletList createPortletList(IPerson owner, IPortletList toCreate) { log.debug("Using DAO to create portlet list [{}] for user [{}]", toCreate.getName(), toCreate.getUserId()); return portletListDao.createPortletList(toCreate); } + + @Override + public IPortletList updatePortletList(IPerson updater, IPortletList toUpdate, String portletListUuid) { + log.debug("Using DAO to update portlet list [{}] for user [{}]", portletListUuid, updater.getUserName()); + return portletListDao.updatePortletList(toUpdate, portletListUuid); + } // // /** // * Returns the specified definitions, provided (1) it exists and (2) the user may view it. diff --git a/uPortal-webapp/src/main/resources/properties/db/hibernate.cfg.xml b/uPortal-webapp/src/main/resources/properties/db/hibernate.cfg.xml index bd4e5c2a69b..540c851191d 100644 --- a/uPortal-webapp/src/main/resources/properties/db/hibernate.cfg.xml +++ b/uPortal-webapp/src/main/resources/properties/db/hibernate.cfg.xml @@ -81,5 +81,6 @@ + From d170611a6202878b504b5ca9d76288583f9767e5 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Fri, 12 Aug 2022 19:33:55 -0500 Subject: [PATCH 03/10] Begin hardening --- .../portal/dao/portletlist/IPortletList.java | 14 +- .../dao/portletlist/IPortletListDao.java | 17 +- .../dao/portletlist/IPortletListItem.java | 11 +- .../dao/portletlist/jpa/PortletList.java | 107 +++++++- .../dao/portletlist/jpa/PortletListDao.java | 105 ++++---- .../dao/portletlist/jpa/PortletListItem.java | 36 ++- .../portletlist/jpa/PortletListItemPK.java | 88 ------- .../PortletListRESTController.java | 154 ++++++----- .../portal/rest/utils/InputValidator.java | 23 ++ .../portletlist/IPortletListService.java | 8 +- .../portletlist/PortletListService.java | 240 ++---------------- 11 files changed, 353 insertions(+), 450 deletions(-) delete mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItemPK.java create mode 100644 uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java index d5c50e8bcce..99e19d5fd5a 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java @@ -1,6 +1,7 @@ package org.apereo.portal.dao.portletlist; import org.apereo.portal.dao.portletlist.jpa.PortletListItem; +import org.apereo.portal.security.IPerson; import org.dom4j.Element; import java.util.List; @@ -9,9 +10,9 @@ public interface IPortletList { String getId(); - String getUserId(); + String getOwnerUsername(); - void setUserId(String userId); + void setOwnerUsername(String username); String getName(); @@ -19,16 +20,15 @@ public interface IPortletList { List getItems(); + // Don't use this setter - instead use clearAndSetItems(...) to let hibernate handle its own List management void setItems(List items); - //Set getListItems(); - - //void setListItems(Set items); - /** Supports exporting. */ void toElement(Element parent); String toString(); - void overrideItems(List items); + void clearAndSetItems(List items); + + void prepareForPersistence(IPerson requester); } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java index 5bffc734543..5b6014fe837 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java @@ -1,21 +1,22 @@ package org.apereo.portal.dao.portletlist; +import org.apereo.portal.security.IPerson; + import java.util.List; public interface IPortletListDao { -// public IPersonAttributesGroupDefinition updatePersonAttributesGroupDefinition( -// IPersonAttributesGroupDefinition personAttributesGroupDefinition); -// -// public void deletePersonAttributesGroupDefinition(IPersonAttributesGroupDefinition definition); - - public List getPortletLists(String userId); + public List getPortletLists(String ownerUsername); public List getPortletLists(); public IPortletList getPortletList(String portletListUuid); - public IPortletList createPortletList(IPortletList toCreate); + public IPortletList createPortletList(IPortletList toCreate, IPerson requester); + + public IPortletList updatePortletList(IPortletList toUpdate, IPerson requester); + + public boolean removePortletListAsAdmin(String portletListUuid, IPerson adminRequester); - public IPortletList updatePortletList(IPortletList toUpdate, String portletListUuid); + public boolean removePortletListAsOwner(String portletListUuid, IPerson ownerRequester); } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListItem.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListItem.java index dc27f619310..ab9723fabc7 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListItem.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListItem.java @@ -1,13 +1,16 @@ package org.apereo.portal.dao.portletlist; -import org.apereo.portal.dao.portletlist.jpa.PortletListItemPK; -import org.dom4j.Element; +import org.apereo.portal.dao.portletlist.jpa.PortletList; public interface IPortletListItem { - PortletListItemPK getPortletListItemPK(); + PortletList getPortletList(); - void setPortletListItemPK(PortletListItemPK portletListItemPK); + void setPortletList(PortletList portletList); + + int getListOrder(); + + void setListOrder(int listOrder); String getEntityId(); diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java index 6c84e4b1898..38a683da608 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java @@ -1,21 +1,32 @@ package org.apereo.portal.dao.portletlist.jpa; +import com.fasterxml.jackson.annotation.JsonFormat; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Setter; import lombok.ToString; import lombok.extern.slf4j.Slf4j; import org.apereo.portal.dao.portletlist.IPortletList; -import org.apereo.portal.dao.portletlist.IPortletListItem; +import org.apereo.portal.rest.utils.InputValidator; +import org.apereo.portal.security.IPerson; import org.dom4j.Element; import org.hibernate.annotations.*; +import org.springframework.util.StringUtils; import javax.persistence.*; import javax.persistence.CascadeType; import javax.persistence.Entity; import javax.persistence.OrderBy; import javax.persistence.Table; +import javax.portlet.Portlet; +import java.sql.Timestamp; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneId; import java.util.ArrayList; +import java.util.Date; +import java.util.HashMap; import java.util.List; @Getter @@ -26,32 +37,56 @@ @Entity @Table( name = "UP_PORTLET_LIST", - uniqueConstraints = { @UniqueConstraint(columnNames = { "user_id", "name" }) }) + uniqueConstraints = { @UniqueConstraint(columnNames = { "OWNER_USERNAME", "NAME" }) }) @Cacheable @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) @SuppressWarnings("unused") public class PortletList implements IPortletList { + private static final ZoneId tz = ZoneId.systemDefault(); + private static final String AUDIT_DATE_FORMAT = "yyyy-MM-dd hh:mm:ss Z"; + @JsonProperty(access = JsonProperty.Access.READ_ONLY) @Id @GeneratedValue(generator = "UUID") @GenericGenerator( name = "UUID", strategy = "org.hibernate.id.UUIDGenerator" ) - @Column(name = "id", updatable = false, nullable = false) + @Column(name = "ID", updatable = false, nullable = false) private String id; - @Column(name = "user_id", updatable = true, nullable = false) - private String userId; + @Column(name = "OWNER_USERNAME", updatable = true, nullable = false) + private String ownerUsername; - @Column(name = "name", updatable = true, nullable = false) + @Column(name = "NAME", updatable = true, nullable = false) private String name; + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @Column(name = "CREATED_BY", updatable = false, nullable = false) + private String createdBy; + + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @JsonFormat + (shape = JsonFormat.Shape.STRING, pattern = AUDIT_DATE_FORMAT) + @Column(name = "CREATED_ON", updatable = false, nullable = false) + private Timestamp createdOn; + + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @Column(name = "UPDATED_BY", updatable = true, nullable = false) + private String updatedBy; + + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @JsonFormat + (shape = JsonFormat.Shape.STRING, pattern = AUDIT_DATE_FORMAT) + @Column(name = "UPDATED_ON", updatable = true, nullable = false) + private Timestamp updatedOn; + @OneToMany( targetEntity = PortletListItem.class, - cascade = CascadeType.REMOVE, + cascade = CascadeType.ALL, fetch = FetchType.EAGER, - mappedBy = "portletListItemPK.portletList") + mappedBy = "portletList", + orphanRemoval = true) @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) @Fetch(FetchMode.SELECT) // FM JOIN does BAD things to collections that support duplicates @OrderBy("LIST_ORDER ASC") @@ -66,17 +101,63 @@ public void toElement(Element parent) { parent.addElement("id").addText(this.getId().toString()); parent.addElement("name").addText(this.getName()); - parent.addElement("userid").addText(this.getUserId()); + parent.addElement("ownerUsername").addText(this.getOwnerUsername()); parent.addElement("items").addText("" + this.getItems().size()); } - public void overrideItems(List items) { + public void clearAndSetItems(List items) { + // Index all current items + HashMap existingItems = new HashMap<>(); + for(PortletListItem existingItem : this.items) { + existingItems.put(existingItem.getEntityId(), existingItem); + } + this.items.clear(); - int order = 0; + for(PortletListItem item : items) { - item.setPortletListItemPK(new PortletListItemPK(this, order++)); - this.items.add(item); + PortletListItem existingItem = existingItems.get(item.getEntityId()); + if(existingItem != null) { + // If any item specific attributes are configured, specifically copy them over here. + // Order will be set in prepareForPersistence() + existingItem.setListOrder(-1); + this.items.add(existingItem); + } else { + this.items.add(item); + } } } + + /** + * Final step before letting the object be persisted or merged via the entity manager. + * + * Validation is in part, a fail-safe to ensure SQL injections are checked. + * @param requester + */ + public void prepareForPersistence(IPerson requester) { + if(!StringUtils.isEmpty(this.name)) { + InputValidator.validateAsWordCharacters(this.name, "name"); + } + + if(!StringUtils.isEmpty(this.ownerUsername)) { + InputValidator.validateAsWordCharacters(this.ownerUsername, "ownerUsername"); + } + + int order = 0; + for(PortletListItem item : this.items) { + InputValidator.validateAsWordCharacters(item.getEntityId(), "items > entityId"); + item.setPortletList(this); + item.setListOrder(order++); + } + + // Set / Update audit fields + if(this.createdOn == null) { + this.createdOn = this.updatedOn = Timestamp.valueOf(LocalDateTime.now(tz)); + this.createdBy = this.updatedBy = requester.getUserName(); + } else { + this.updatedOn = Timestamp.valueOf(LocalDateTime.now(tz)); + this.updatedBy = requester.getUserName(); + } + + } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java index 33bf70edc94..78def733945 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java @@ -5,6 +5,7 @@ import org.apereo.portal.dao.portletlist.IPortletList; import org.apereo.portal.dao.portletlist.IPortletListDao; import org.apereo.portal.jpa.BasePortalJpaDao; +import org.apereo.portal.security.IPerson; import org.springframework.stereotype.Repository; import javax.persistence.TypedQuery; @@ -21,31 +22,14 @@ public class PortletListDao extends BasePortalJpaDao implements IPortletListDao private CriteriaQuery portletListByPortletListUuidQuery; - private ParameterExpression userIdParameter; + private ParameterExpression ownerUsernameParameter; private ParameterExpression portletListUuidParameter; @Override public void afterPropertiesSet() throws Exception { - this.userIdParameter = this.createParameterExpression(String.class, "user_id"); - this.portletListUuidParameter = this.createParameterExpression(String.class, "id"); - -// this.findAllDefinitionsQuery = -// this.createCriteriaQuery( -// new Function< -// CriteriaBuilder, -// CriteriaQuery>() { -// @Override -// public CriteriaQuery apply( -// CriteriaBuilder cb) { -// final CriteriaQuery -// criteriaQuery = -// cb.createQuery( -// PersonAttributesGroupDefinitionImpl.class); -// criteriaQuery.from(PersonAttributesGroupDefinitionImpl.class); -// return criteriaQuery; -// } -// }); + this.ownerUsernameParameter = this.createParameterExpression(String.class, "OWNER_USERNAME"); + this.portletListUuidParameter = this.createParameterExpression(String.class, "ID"); this.portletListsQuery = this.createCriteriaQuery( @@ -85,7 +69,7 @@ public CriteriaQuery apply( PortletList.class); criteriaQuery .select(root) - .where(cb.equal(root.get("userId"), userIdParameter)); + .where(cb.equal(root.get("ownerUsername"), ownerUsernameParameter)); return criteriaQuery; } }); @@ -116,10 +100,10 @@ public CriteriaQuery apply( @PortalTransactionalReadOnly @Override public List getPortletLists( - String userId) { + String ownerUsername) { TypedQuery query = this.createCachedQuery(portletListsByUserIdQuery); - query.setParameter(userIdParameter, userId); + query.setParameter(ownerUsernameParameter, ownerUsername); List entities = new ArrayList<>(query.getResultList()); return entities; } @@ -145,7 +129,7 @@ public IPortletList getPortletList( if(lists.size() < 1) { return null; } else if(lists.size() > 1) { - log.error("Expected to up to 1 portlet list for portlet list uuid [{}], but found [{}].", portletListUuid, lists.size()); + log.error("Expected up to 1 portlet list for portlet list uuid [{}], but found [{}].", portletListUuid, lists.size()); return null; } @@ -155,42 +139,77 @@ public IPortletList getPortletList( @SuppressWarnings("unused") @PortalTransactional @Override - public IPortletList createPortletList(IPortletList toCreate) { - log.debug("Persisting portlet list [{}] for user [{}]", toCreate.getName(), toCreate.getUserId()); + public IPortletList createPortletList(IPortletList toCreate, IPerson requester) { + log.debug("Persisting portlet list [{}] with owner [{}]", toCreate.getName(), toCreate.getOwnerUsername()); try { + toCreate.prepareForPersistence(requester); this.getEntityManager().persist(toCreate); } catch (Exception e) { log.debug("Failed to persist portlet list", e); throw e; } - log.debug("Finished persisting portlet list [{}] for user [{}]. ID = [{}]", toCreate.getName(), toCreate.getUserId(), toCreate.getId().toString()); + log.debug("Finished persisting portlet list [{}] for owner [{}]. ID = [{}]", toCreate.getName(), toCreate.getOwnerUsername(), toCreate.getId().toString()); return toCreate; } @PortalTransactional @Override - public IPortletList updatePortletList(IPortletList toUpdate, String portletListUuid) { - log.debug("Persisting changes for portlet list [{}] for user [{}]", toUpdate.getName(), toUpdate.getUserId()); + public IPortletList updatePortletList(IPortletList toUpdate, IPerson requester) { + log.debug("Persisting changes for portlet list [{}]", toUpdate.getId()); try { - IPortletList ref = this.getEntityManager().find(PortletList.class, portletListUuid); - if (ref == null) { - log.warn("Unable to find portlet list [{}] to update.", portletListUuid); - return null; + toUpdate.prepareForPersistence(requester); + if(log.isDebugEnabled()) { + StringBuffer sb = new StringBuffer(); + sb.append("PortletList items to update: "); + if(toUpdate.getItems().size() < 1) { + sb.append("[Currently no items]"); + } else { + final int size = toUpdate.getItems().size(); + for (int i = 0; i < size; i++) { + PortletListItem item = toUpdate.getItems().get(i); + sb.append(item); + if(i < size - 1) { + sb.append("; "); + } + } + } + log.debug(sb.toString()); } - ref.overrideItems(toUpdate.getItems()); - -// List originalItems = toUpdate.getItems(); -// toUpdate.setItems(null); -// log.debug("Safe updating - removing items on portlet list [{}] for user [{}]", toUpdate.getName(), toUpdate.getUserId()); -// this.getEntityManager().persist(toUpdate); -// toUpdate.setItems(originalItems); - //log.debug("Safe updating - updating with current items on portlet list [{}] for user [{}]", toUpdate.getName(), toUpdate.getUserId()); - //this.getEntityManager().merge(toUpdate); - log.debug("Finished persisting changes for portlet list [{}] for user [{}]. ID = [{}]", toUpdate.getName(), toUpdate.getUserId(), portletListUuid); + this.getEntityManager().merge(toUpdate); + log.debug("Finished persisting changes for portlet list [{}]", toUpdate.getId()); } catch (Exception e) { log.debug("Failed to persist changes for portlet list", e); throw e; } return toUpdate; } + + @PortalTransactional + @Override + public boolean removePortletListAsAdmin(String portletListUuid, IPerson requester) { + IPortletList list = this.getPortletList(portletListUuid); + if(list == null) { + log.warn("Admin user [{}] tried to remove a non-existent list [{}]. Failing request.", requester.getUserName(), portletListUuid); + return false; + } + this.getEntityManager().remove(list); + return true; + } + + @PortalTransactional + @Override + public boolean removePortletListAsOwner(String portletListUuid, IPerson requester) { + IPortletList list = this.getPortletList(portletListUuid); + if(list == null) { + log.warn("Non-admin user [{}] tried to remove a non-existent list [{}]. Failing request.", requester.getUserName(), portletListUuid); + return false; + } else if (!list.getOwnerUsername().equals(requester.getUserName())) { + log.warn("Non-admin user [{}] tried to remove a list they didn't own [{}]. Failing request.", requester.getUserName(), portletListUuid); + return false; + } + + log.debug("Non-admin user [{}] requested to remove a list they own [{}]. Allowing request.", requester.getUserName(), portletListUuid); + this.getEntityManager().remove(list); + return true; + } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java index d030b818605..0f4ff8f625d 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java @@ -8,6 +8,7 @@ import lombok.extern.slf4j.Slf4j; import org.apereo.portal.dao.portletlist.IPortletListItem; import org.hibernate.annotations.CacheConcurrencyStrategy; +import org.hibernate.annotations.GenericGenerator; import javax.persistence.*; @@ -22,7 +23,7 @@ name = "UP_PORTLET_LIST_ITEM", uniqueConstraints = { // These are sets of lists - @UniqueConstraint(columnNames = { "LIST_ID", "ENTITY_ID" }), + @UniqueConstraint(columnNames = { "LIST_ID", "LIST_ORDER", "ENTITY_ID" }), }) @Cacheable @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) @@ -30,8 +31,29 @@ public class PortletListItem implements IPortletListItem { @JsonIgnore - @EmbeddedId - private PortletListItemPK portletListItemPK; + @Id + @GeneratedValue(generator = "UUID") + @GenericGenerator( + name = "UUID", + strategy = "org.hibernate.id.UUIDGenerator" + ) + @Column(name = "ID", updatable = false, nullable = false) + private String id; + + @JsonIgnore + @ManyToOne(optional = false) + @JoinColumn( + name = "LIST_ID", + referencedColumnName = "ID") + private PortletList portletList; + + @JsonIgnore + @Column( + name = "LIST_ORDER", + unique = false, + nullable = false, + updatable = true) + private int listOrder; // This is generally the portlet fname, but could be adjusted in the future @Column(name = "ENTITY_ID", updatable = true, nullable = false) @@ -44,4 +66,12 @@ public PortletListItem() { public PortletListItem(String entityId) { this.entityId = entityId; } + + public String toString() { + return "PortletListItem: id=[" + id + + "], portlet-list=[" + (portletList == null ? "NULL" : portletList.getId()) + + "], order=[" + listOrder + + "], entity-id=[" + entityId + + "]"; + } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItemPK.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItemPK.java deleted file mode 100644 index 4b58ba34cd1..00000000000 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItemPK.java +++ /dev/null @@ -1,88 +0,0 @@ -/** - * Licensed to Apereo under one or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information regarding copyright ownership. Apereo - * licenses this file to you under the Apache License, Version 2.0 (the "License"); you may not use - * this file except in compliance with the License. You may obtain a copy of the License at the - * following location: - * - *

http://www.apache.org/licenses/LICENSE-2.0 - * - *

Unless required by applicable law or agreed to in writing, software distributed under the - * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apereo.portal.dao.portletlist.jpa; - -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import lombok.Getter; -import lombok.Setter; -import org.apereo.portal.dao.portletlist.IPortletList; -import org.hibernate.annotations.Cascade; - -import javax.persistence.*; -import java.io.Serializable; - -@Getter -@Setter -@SecondaryTable(name = "UP_PORTLET_LIST") -@Embeddable -@JsonIgnoreProperties({ "portletList" }) -public class PortletListItemPK implements Serializable { - - @ManyToOne(optional = false) - @JoinColumn( - name = "LIST_ID", - referencedColumnName = "ID") - protected PortletList portletList; - - @Column( - name = "LIST_ORDER", - unique = false, - nullable = false, - insertable = true, - updatable = true) - protected int listOrder; - - /** Empty constructor is needed for Serializable */ - public PortletListItemPK() {}; - - public PortletListItemPK(PortletList portletList, int listOrder) { - this.portletList = portletList; - this.listOrder = listOrder; - } - -// @Override -// public boolean equals(Object obj) { -// if (obj == null || !(obj instanceof org.apereo.portal.portlet.dao.jpa.MarketplaceRatingPK)) { -// return false; -// } else if (obj == this) { -// return true; -// } -// org.apereo.portal.portlet.dao.jpa.MarketplaceRatingPK tempRating = (org.apereo.portal.portlet.dao.jpa.MarketplaceRatingPK) obj; -// return new EqualsBuilder() -// .append(userName, tempRating.userName) -// .append(portletDefinition, tempRating.portletDefinition) -// .isEquals(); -// } -// -// @Override -// public int hashCode() { -// return new HashCodeBuilder(17, 31) -// . // two randomly chosen prime numbers -// append(userName) -// .append(portletDefinition) -// .toHashCode(); -// } -// -// @Override -// public String toString() { -// StringBuilder builder = new StringBuilder(); -// builder.append("User: "); -// builder.append(this.userName); -// builder.append("\n"); -// builder.append("Portlet: "); -// builder.append(this.portletDefinition); -// return builder.toString(); -// } -} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java index e1fdfde017c..2936d5b537a 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java @@ -2,12 +2,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang.ArrayUtils; import org.apereo.portal.dao.portletlist.IPortletList; -import org.apereo.portal.dao.portletlist.IPortletListItem; import org.apereo.portal.dao.portletlist.jpa.PortletList; -import org.apereo.portal.dao.portletlist.jpa.PortletListItem; -import org.apereo.portal.dao.portletlist.jpa.PortletListItemPK; import org.apereo.portal.rest.utils.ErrorResponse; import org.apereo.portal.security.RuntimeAuthorizationException; import org.apereo.portal.services.portletlist.IPortletListService; @@ -23,9 +19,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.util.ArrayList; import java.util.List; -import java.util.Set; import static org.springframework.web.bind.annotation.RequestMethod.*; @@ -55,7 +49,9 @@ public class PortletListRESTController { produces = MediaType.APPLICATION_JSON_VALUE) public @ResponseBody String getPortletLists(HttpServletRequest request, HttpServletResponse response) { final IPerson person = personManager.getPerson(request); - debugPerson("getAllPortletLists", person); + if(log.isDebugEnabled()) { + debugPerson("getAllPortletLists", person); + } if(person.isGuest()) { log.warn("Guest is trying to access portlet-list API, which is not allowed."); @@ -79,7 +75,9 @@ public class PortletListRESTController { produces = MediaType.APPLICATION_JSON_VALUE) public @ResponseBody String getPortletList(HttpServletRequest request, HttpServletResponse response, @PathVariable String portletListUuid) { final IPerson person = personManager.getPerson(request); - debugPerson("getSpecificPortletList", person); + if(log.isDebugEnabled()) { + debugPerson("getSpecificPortletList", person); + } if(person.isGuest()) { log.warn("Guest is trying to access portlet-list API, which is not allowed."); @@ -91,9 +89,9 @@ public class PortletListRESTController { if(pList == null) { log.warn("User [{}] tried to access portlet-list [{}], but list was not found.", person.getUserName(), portletListUuid); return prepareResponse(response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); - } else if(!portletListService.isPortletListAdmin(person) && !pList.getUserId().equals("" + person.getID())) { + } else if(!portletListService.isPortletListAdmin(person) && !pList.getOwnerUsername().equals(person.getUserName())) { // Not an admin, and not the owner - log.warn("Non-admin user [{}][{}] tried to access portlet-list [{}] with owner [{}], but was blocked since they aren't the owner.", person.getID(), person.getUserName(), portletListUuid, pList.getUserId()); + log.warn("Non-admin user [{}] tried to access portlet-list [{}] with owner [{}], but was blocked since they aren't the owner.", person.getUserName(), portletListUuid, pList.getOwnerUsername()); return prepareResponse(response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); } @@ -110,8 +108,10 @@ public class PortletListRESTController { @RequestBody String json) { final IPerson person = personManager.getPerson(request); - debugPerson("createPortletList", person); - log.debug("createPortletList > JSON body is = {}", json); + if(log.isDebugEnabled()) { + debugPerson("createPortletList", person); + log.debug("createPortletList > JSON body is = {}", json); + } if(person.isGuest()) { log.warn("createPortletList > Guest is trying to access portlet-list API, which is not allowed."); @@ -124,19 +124,17 @@ public class PortletListRESTController { input = objectMapper.readValue(json, PortletList.class); if(!portletListService.isPortletListAdmin(person)) { // Default - non-admins can only create lists for themselves. - input.setUserId("" + person.getID()); - } else if (StringUtils.isEmpty(input.getUserId())) { + input.setOwnerUsername(person.getUserName()); + } else if (StringUtils.isEmpty(input.getOwnerUsername())) { // Default - admins don't have to specify a user name - input.setUserId("" + person.getID()); + input.setOwnerUsername(person.getUserName()); } } catch (Exception e) { - log.warn("User [{}][{}] tried to create a portlet-list with bad json - {}", person.getID(), person.getUserName(), e.getMessage()); + log.warn("User [{}] tried to create a portlet-list with bad json - {}", person.getUserName(), e.getMessage()); return prepareResponse(response, null, "Unparsable portlet-list JSON", HttpServletResponse.SC_BAD_REQUEST); } try { - // Sets the parent-child relationships - input.overrideItems(input.getItems()); final IPortletList created = portletListService.createPortletList( person, input); response.setHeader("Location", created.getId()); @@ -167,11 +165,13 @@ public class PortletListRESTController { @PathVariable String portletListUuid) { final IPerson person = personManager.getPerson(request); - debugPerson("updatePortletList", person); - log.debug("updatePortletList > JSON body is = {}", json); + if(log.isDebugEnabled()) { + debugPerson("updatePortletList", person); + log.debug("updatePortletList > JSON body is = {}", json); + } if(person.isGuest()) { - log.warn("updatePortletList > Guest is trying to access portlet-list API, which is not allowed."); + log.warn("Guest user is trying to access portlet-list PUT API, which is not allowed."); return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } @@ -179,54 +179,48 @@ public class PortletListRESTController { try { input = objectMapper.readValue(json, PortletList.class); } catch (Exception e) { - log.warn("User [{}][{}] tried to create a portlet-list with bad json - {}", person.getID(), person.getUserName(), e.getMessage()); + log.warn("User [{}] tried to update a portlet-list with bad json - {}", person.getUserName(), e.getMessage()); return prepareResponse(response, null, "Unparsable portlet-list JSON", HttpServletResponse.SC_BAD_REQUEST); } -// // Overlay changes onto a known entity, and then persist that entity. -// IPortletList toUpdate = portletListService.getPortletList(portletListUuid); -// -// if(toUpdate == null) { -// return prepareResponse(response, null, "Unknown portlet-list", HttpServletResponse.SC_NOT_FOUND); -// } -// -// if(portletListService.isPortletListAdmin(person)) { -// // If admin, allow admin-level changes -// if(!StringUtils.isEmpty(input.getUserId())) { -// toUpdate.setUserId(input.getUserId()); -// } -// -// if(!StringUtils.isEmpty(input.getName())) { -// toUpdate.setName(input.getName()); -// } -// } else if (toUpdate.getUserId().equals("" + person.getID())) { -// // If owner of portlet-list, allow only owner-level changes -// -// if(!StringUtils.isEmpty(input.getUserId())) { -// log.warn("updatePortletList - non-admin user [{}][{}] tried to update portlet-list [{}][{}] with a new owner [{}], which is not allowed.", person.getID(), person.getUserName(), toUpdate.getId(), toUpdate.getName(), input.getUserId()); -// return prepareResponse(response, null, "Non-admin user cannot change portlet-list owner", HttpServletResponse.SC_BAD_REQUEST); -// } -// } else { -// // Otherwise, disallow changes -// log.warn("updatePortletList - user [{}][{}] tried to update portlet-list [{}][{}], but was not the owner.", person.getID(), person.getUserName(), toUpdate.getId(), toUpdate.getName()); -// return prepareResponse(response, null, "Unknown portlet-list", HttpServletResponse.SC_UNAUTHORIZED); -// } -// -// // Either an owner or an admin. allow general-level changes: -// if(!StringUtils.isEmpty(input.getName())) { -// toUpdate.setName(input.getName()); -// } -// -// if(input.getItems() != null) { -// toUpdate.overrideItems(input.getItems()); -// } + // Overlay changes onto a known entity, and then persist that entity. + IPortletList toUpdate = portletListService.getPortletList(portletListUuid); + + if(toUpdate == null) { + return prepareResponse(response, null, "Unknown portlet-list", HttpServletResponse.SC_NOT_FOUND); + } + + if(portletListService.isPortletListAdmin(person)) { + // If admin, allow admin-level changes + if(!StringUtils.isEmpty(input.getOwnerUsername())) { + toUpdate.setOwnerUsername(input.getOwnerUsername()); + } + } else if (toUpdate.getOwnerUsername().equals(person.getUserName())) { + // If owner of portlet-list, allow only owner-level changes + if(!StringUtils.isEmpty(input.getOwnerUsername())) { + log.warn("non-admin user [{}] tried to update portlet-list [{}][{}] with a new owner [{}], which is not allowed.", person.getUserName(), toUpdate.getId(), toUpdate.getName(), input.getOwnerUsername()); + return prepareResponse(response, null, "Non-admin user cannot change portlet-list owner", HttpServletResponse.SC_BAD_REQUEST); + } + } else { + // Otherwise, disallow changes + log.warn("user [{}] tried to update portlet-list [{}][{}], but was not the owner nor an admin.", person.getUserName(), toUpdate.getId(), toUpdate.getName()); + return prepareResponse(response, null, "Unknown portlet-list", HttpServletResponse.SC_UNAUTHORIZED); + } + + // Either an owner or an admin. allow general-level changes: + if(!StringUtils.isEmpty(input.getName())) { + toUpdate.setName(input.getName()); + } + + if(input.getItems() != null) { + toUpdate.clearAndSetItems(input.getItems()); + } try { - final IPortletList updated = portletListService.updatePortletList( - person, input, portletListUuid); + final IPortletList updated = portletListService.updatePortletList(person, toUpdate); if(updated == null) { log.warn("update returned null for portlet-list uuid [{}]. Failing request.", portletListUuid); - return prepareResponse(response, null, "Unable to update portlet list.", HttpServletResponse.SC_BAD_REQUEST); + return prepareResponse(response, null, "Error occurred while updating portlet list. Please check your System Administrator", HttpServletResponse.SC_BAD_REQUEST); } else { return prepareResponse(response, null, null, HttpServletResponse.SC_OK); } @@ -241,7 +235,41 @@ public class PortletListRESTController { return prepareResponse(response, null, "Data integrity issue - such as specifying a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); } catch (Exception e) { log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName(), e); - return prepareResponse(response, null, "Something unexpected occurred. Please check with your System Administrator", HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return prepareResponse(response, null, "Something unexpected occurred. Please check with your System Administrator.", HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + } + + /** + * Provide a JSON view of a given portlet list. + * + * If an administrator makes the request, the portlet list will be returned, regardless of ownership. + * If anyone else makes the request, the portlet list will only be returned if the requester is the owner. + */ + @RequestMapping( + value = CONTEXT + "{portletListUuid}", + method = DELETE, + produces = MediaType.APPLICATION_JSON_VALUE) + public @ResponseBody String removePortletList(HttpServletRequest request, HttpServletResponse response, @PathVariable String portletListUuid) { + final IPerson person = personManager.getPerson(request); + if(log.isDebugEnabled()) { + debugPerson("removePortletList", person); + } + + if(person.isGuest()) { + log.warn("Guest is trying to access portlet-list API, which is not allowed."); + return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + } + + try { + if(portletListService.removePortletList(person, portletListUuid)) { + return prepareResponse(response, null, null, HttpServletResponse.SC_OK); + } else { + return prepareResponse(response, null, "Unable to remove portlet list. Please check with your System Administrator.", HttpServletResponse.SC_BAD_REQUEST); + } + } catch (Exception e) { + log.error("Unable to delete portlet list. Returning a 500.", e); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return null; } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java new file mode 100644 index 00000000000..f4bfb329653 --- /dev/null +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java @@ -0,0 +1,23 @@ +package org.apereo.portal.rest.utils; + +import java.util.regex.Pattern; + +public class InputValidator { + private static final String ALPHANUMERIC_AND_DASH_VALIDATOR_REGEX = "^[\\w\\-]{1,500}$"; // 1-500 characters + private static final Pattern ALPHANUMERIC_AND_DASH_VALIDATOR_PATTERN = + Pattern.compile(ALPHANUMERIC_AND_DASH_VALIDATOR_REGEX); + + /** + * + * @param s value to validate + * @param inputType not sanitized and will be returned in json. + * @return The validated value + */ + public static String validateAsWordCharacters(String s, String inputType) { + if (!ALPHANUMERIC_AND_DASH_VALIDATOR_PATTERN.matcher(s).matches()) { + throw new IllegalArgumentException( + "Specified " + inputType + " is not the correct length or has invalid characters."); + } + return s; + } +} diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java index 5e1152b796a..b4bc03ccd7f 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java @@ -10,7 +10,9 @@ public interface IPortletListService { public List getPortletLists(); - public List getPortletLists(IPerson owner); + public List getPortletLists(IPerson requester); + + public boolean removePortletList(IPerson requester, String portletListUuid); /** * Returns null if not found @@ -19,9 +21,9 @@ public interface IPortletListService { */ public IPortletList getPortletList(String portletListUuid); - public IPortletList createPortletList(IPerson creater, IPortletList toCreate); + public IPortletList createPortletList(IPerson requester, IPortletList toCreate); - public IPortletList updatePortletList(IPerson updater, IPortletList toCreate, String portletListUuid); + public IPortletList updatePortletList(IPerson requester, IPortletList toUpdate); public boolean isPortletListAdmin(IPerson person); diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java index 15ea84cb699..2c127900d0e 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java @@ -1,7 +1,9 @@ package org.apereo.portal.services.portletlist; +import jdk.internal.util.xml.impl.Input; import org.apereo.portal.dao.portletlist.IPortletListDao; import org.apereo.portal.dao.portletlist.IPortletList; +import org.apereo.portal.rest.utils.InputValidator; import org.apereo.portal.security.AuthorizationPrincipalHelper; import org.apereo.portal.security.IAuthorizationService; import org.apereo.portal.security.IPermission; @@ -51,13 +53,13 @@ public List getPortletLists() { /** * All the definitions, filtered by the user's access rights. * - * @param person + * @param requester * @return */ @Override - public List getPortletLists(IPerson person) { - List rslt = portletListDao.getPortletLists("" + person.getID()); - log.debug("Returning portlet lists '{}' for user '{}'", rslt, person.getUserName()); + public List getPortletLists(IPerson requester) { + List rslt = portletListDao.getPortletLists(requester.getUserName()); + log.debug("Returning portlet lists '{}' for user '{}'", rslt, requester.getUserName()); return rslt; } @@ -72,225 +74,27 @@ public IPortletList getPortletList(String portletListUuid) { return portletListDao.getPortletList(portletListUuid); } + @Override + public boolean removePortletList(IPerson requester, String portletListUuid) { + if(isPortletListAdmin(requester)) { + return portletListDao.removePortletListAsAdmin(portletListUuid, requester); + } else { + return portletListDao.removePortletListAsOwner(portletListUuid, requester); + } + } + /** TODO docs Verifies permissions and that the group doesn't already exist (case insensitive) */ @Override - public IPortletList createPortletList(IPerson creater, IPortletList toCreate) { -// // What's the target of the upcoming permissions check? -// String target = -// parent != null -// ? parent.getEntityIdentifier().getKey() -// : IPermission -// .ALL_GROUPS_TARGET; // Must have blanket permission to create one -// // w/o a parent -// -// // Verify permission -// if (!hasPermission(person, IPermission.CREATE_GROUP_ACTIVITY, target)) { -// throw new RuntimeAuthorizationException( -// person, IPermission.CREATE_GROUP_ACTIVITY, target); -// } -// -// // VALIDATION STEP: The group name & description are allowable -// if (StringUtils.isBlank(groupName)) { -// throw new IllegalArgumentException("Specified groupName is blank: " + groupName); -// } -// if (!GROUP_NAME_VALIDATOR_PATTERN.matcher(groupName).matches()) { -// throw new IllegalArgumentException( -// "Specified groupName is too long, too short, or contains invalid characters: " -// + groupName); -// } -// if (!StringUtils.isBlank(description)) { // Blank description is allowable -// if (!GROUP_DESC_VALIDATOR_PATTERN.matcher(description).matches()) { -// throw new IllegalArgumentException( -// "Specified description is too long or contains invalid characters: " -// + description); -// } -// } -// -// // VALIDATION STEP: We don't have a group by that name already -// EntityIdentifier[] people = -// GroupService.searchForGroups( -// groupName, IGroupConstants.SearchMethod.DISCRETE_CI, IPerson.class); -// EntityIdentifier[] portlets = -// GroupService.searchForGroups( -// groupName, -// IGroupConstants.SearchMethod.DISCRETE_CI, -// IPortletDefinition.class); -// if (people.length != 0 || portlets.length != 0) { -// throw new IllegalArgumentException("Specified groupName already in use: " + groupName); -// } - log.debug("Using DAO to create portlet list [{}] for user [{}]", toCreate.getName(), toCreate.getUserId()); - return portletListDao.createPortletList(toCreate); + public IPortletList createPortletList(IPerson requester, IPortletList toCreate) { + log.debug("Using DAO to create portlet list [{}] for user [{}]", toCreate.getName(), toCreate.getOwnerUsername()); + return portletListDao.createPortletList(toCreate, requester); } @Override - public IPortletList updatePortletList(IPerson updater, IPortletList toUpdate, String portletListUuid) { - log.debug("Using DAO to update portlet list [{}] for user [{}]", portletListUuid, updater.getUserName()); - return portletListDao.updatePortletList(toUpdate, portletListUuid); + public IPortletList updatePortletList(IPerson requester, IPortletList toUpdate) { + log.debug("Using DAO to update portlet list [{}] for user [{}]", toUpdate.getId(), requester.getUserName()); + return portletListDao.updatePortletList(toUpdate, requester); } -// -// /** -// * Returns the specified definitions, provided (1) it exists and (2) the user may view it. -// * -// * @param person -// * @return -// */ -// public IPersonAttributesGroupDefinition getPagsDefinitionByName(IPerson person, String name) { -// IPersonAttributesGroupDefinition rslt = getPagsGroupDefByName(name); -// if (rslt == null) { -// // Better to produce exception? I'm thinking not, but open-minded. -// return null; -// } -// if (!hasPermission( -// person, -// IPermission.VIEW_GROUP_ACTIVITY, -// rslt.getCompositeEntityIdentifierForGroup().getKey())) { -// throw new RuntimeAuthorizationException(person, IPermission.VIEW_GROUP_ACTIVITY, name); -// } -// logger.debug("Returning PAGS definition '{}' for user '{}'", rslt, person.getUserName()); -// return rslt; -// } -// -// /** Verifies permissions and that the group doesn't already exist (case insensitive) */ -// public IPersonAttributesGroupDefinition createPagsDefinition( -// IPerson person, IEntityGroup parent, String groupName, String description) { -// -// // What's the target of the upcoming permissions check? -// String target = -// parent != null -// ? parent.getEntityIdentifier().getKey() -// : IPermission -// .ALL_GROUPS_TARGET; // Must have blanket permission to create one -// // w/o a parent -// -// // Verify permission -// if (!hasPermission(person, IPermission.CREATE_GROUP_ACTIVITY, target)) { -// throw new RuntimeAuthorizationException( -// person, IPermission.CREATE_GROUP_ACTIVITY, target); -// } -// -// // VALIDATION STEP: The group name & description are allowable -// if (StringUtils.isBlank(groupName)) { -// throw new IllegalArgumentException("Specified groupName is blank: " + groupName); -// } -// if (!GROUP_NAME_VALIDATOR_PATTERN.matcher(groupName).matches()) { -// throw new IllegalArgumentException( -// "Specified groupName is too long, too short, or contains invalid characters: " -// + groupName); -// } -// if (!StringUtils.isBlank(description)) { // Blank description is allowable -// if (!GROUP_DESC_VALIDATOR_PATTERN.matcher(description).matches()) { -// throw new IllegalArgumentException( -// "Specified description is too long or contains invalid characters: " -// + description); -// } -// } -// -// // VALIDATION STEP: We don't have a group by that name already -// EntityIdentifier[] people = -// GroupService.searchForGroups( -// groupName, IGroupConstants.SearchMethod.DISCRETE_CI, IPerson.class); -// EntityIdentifier[] portlets = -// GroupService.searchForGroups( -// groupName, -// IGroupConstants.SearchMethod.DISCRETE_CI, -// IPortletDefinition.class); -// if (people.length != 0 || portlets.length != 0) { -// throw new IllegalArgumentException("Specified groupName already in use: " + groupName); -// } -// -// IPersonAttributesGroupDefinition rslt = -// pagsGroupDefDao.createPersonAttributesGroupDefinition(groupName, description); -// if (parent != null) { -// // Should refactor this switch to instead choose a service and invoke a method on it -// switch (parent.getServiceName().toString()) { -// case SERVICE_NAME_LOCAL: -// IEntityGroup member = -// GroupService.findGroup( -// rslt.getCompositeEntityIdentifierForGroup().getKey()); -// if (member == null) { -// String msg = -// "The specified group was created, but is not present in the store: " -// + rslt.getName(); -// throw new RuntimeException(msg); -// } -// parent.addChild(member); -// parent.updateMembers(); -// break; -// case SERVICE_NAME_PAGS: -// IPersonAttributesGroupDefinition parentDef = -// getPagsGroupDefByName(parent.getName()); -// Set members = -// new HashSet<>(parentDef.getMembers()); -// members.add(rslt); -// parentDef.setMembers(members); -// pagsGroupDefDao.updatePersonAttributesGroupDefinition(parentDef); -// break; -// default: -// String msg = -// "The specified group service does not support adding members: " -// + parent.getServiceName(); -// throw new UnsupportedOperationException(msg); -// } -// } -// -// return rslt; -// } -// -// /** NOTE -- This method assumes that pagsDef is an existing JPA-managed entity. */ -// public IPersonAttributesGroupDefinition updatePagsDefinition( -// IPerson person, IPersonAttributesGroupDefinition pagsDef) { -// -// // Verify permission -// if (!hasPermission( -// person, -// IPermission.EDIT_GROUP_ACTIVITY, -// pagsDef.getCompositeEntityIdentifierForGroup().getKey())) { -// throw new RuntimeAuthorizationException( -// person, -// IPermission.EDIT_GROUP_ACTIVITY, -// pagsDef.getCompositeEntityIdentifierForGroup().getKey()); -// } -// -// IPersonAttributesGroupDefinition rslt = -// pagsGroupDefDao.updatePersonAttributesGroupDefinition(pagsDef); -// return rslt; -// } -// -// /** NOTE -- This method assumes that pagsDef is an existing JPA-managed entity. */ -// public void deletePagsDefinition(IPerson person, IPersonAttributesGroupDefinition pagsDef) { -// -// // Verify permission -// if (!hasPermission( -// person, -// IPermission.DELETE_GROUP_ACTIVITY, -// pagsDef.getCompositeEntityIdentifierForGroup().getKey())) { -// throw new RuntimeAuthorizationException( -// person, -// IPermission.DELETE_GROUP_ACTIVITY, -// pagsDef.getCompositeEntityIdentifierForGroup().getKey()); -// } -// -// pagsGroupDefDao.deletePersonAttributesGroupDefinition(pagsDef); -// } -// -// /* -// * Implementation -// */ -// -// private boolean hasPermission(IPerson person, String permission, String target) { -// EntityIdentifier ei = person.getEntityIdentifier(); -// IAuthorizationPrincipal ap = -// AuthorizationServiceFacade.instance().newPrincipal(ei.getKey(), ei.getType()); -// return ap.hasPermission(IPermission.PORTAL_GROUPS, permission, target); -// } -// -// private IPersonAttributesGroupDefinition getPagsGroupDefByName(String name) { -// Set pagsGroups = -// pagsGroupDefDao.getPersonAttributesGroupDefinitionByName(name); -// if (pagsGroups.size() > 1) { -// logger.error("More than one PAGS group with name {} found.", name); -// } -// return pagsGroups.isEmpty() ? null : pagsGroups.iterator().next(); -// } + } From 91750a2d253134ea2a0ef2e80ab9bd0b75ceadf3 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Wed, 17 Aug 2022 13:14:06 -0500 Subject: [PATCH 04/10] restrict favorite keyword --- .../dao/portletlist/jpa/PortletList.java | 42 +++++++++++++++---- .../dao/portletlist/jpa/PortletListDao.java | 18 +------- .../PortletListRESTController.java | 39 ++++++++++++++--- 3 files changed, 69 insertions(+), 30 deletions(-) diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java index 38a683da608..2154885cca4 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java @@ -32,7 +32,6 @@ @Getter @Setter @EqualsAndHashCode(onlyExplicitlyIncluded = true) -@ToString @Slf4j @Entity @Table( @@ -90,7 +89,7 @@ public class PortletList implements IPortletList { @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) @Fetch(FetchMode.SELECT) // FM JOIN does BAD things to collections that support duplicates @OrderBy("LIST_ORDER ASC") - private List items = new ArrayList<>(); + private List items; @Override public void toElement(Element parent) { @@ -107,6 +106,10 @@ public void toElement(Element parent) { } public void clearAndSetItems(List items) { + if(this.items == null) { + this.items = new ArrayList<>(); + } + // Index all current items HashMap existingItems = new HashMap<>(); for(PortletListItem existingItem : this.items) { @@ -143,11 +146,13 @@ public void prepareForPersistence(IPerson requester) { InputValidator.validateAsWordCharacters(this.ownerUsername, "ownerUsername"); } - int order = 0; - for(PortletListItem item : this.items) { - InputValidator.validateAsWordCharacters(item.getEntityId(), "items > entityId"); - item.setPortletList(this); - item.setListOrder(order++); + if(this.items != null) { + int order = 0; + for (PortletListItem item : this.items) { + InputValidator.validateAsWordCharacters(item.getEntityId(), "items > entityId"); + item.setPortletList(this); + item.setListOrder(order++); + } } // Set / Update audit fields @@ -158,6 +163,29 @@ public void prepareForPersistence(IPerson requester) { this.updatedOn = Timestamp.valueOf(LocalDateTime.now(tz)); this.updatedBy = requester.getUserName(); } + } + public String toString() { + StringBuffer sb = new StringBuffer(); + sb.append("PortletList id=["); + sb.append(id); + sb.append("], name=["); + sb.append(name); + sb.append("], owner=["); + sb.append(ownerUsername); + sb.append("], items=: "); + if(this.items.size() < 1) { + sb.append("[Currently no items]"); + } else { + final int size = items.size(); + for (int i = 0; i < size; i++) { + PortletListItem item = items.get(i); + sb.append(item); + if(i < size - 1) { + sb.append("; "); + } + } + } + return sb.toString(); } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java index 78def733945..13c885506a0 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java @@ -158,23 +158,7 @@ public IPortletList updatePortletList(IPortletList toUpdate, IPerson requester) log.debug("Persisting changes for portlet list [{}]", toUpdate.getId()); try { toUpdate.prepareForPersistence(requester); - if(log.isDebugEnabled()) { - StringBuffer sb = new StringBuffer(); - sb.append("PortletList items to update: "); - if(toUpdate.getItems().size() < 1) { - sb.append("[Currently no items]"); - } else { - final int size = toUpdate.getItems().size(); - for (int i = 0; i < size; i++) { - PortletListItem item = toUpdate.getItems().get(i); - sb.append(item); - if(i < size - 1) { - sb.append("; "); - } - } - } - log.debug(sb.toString()); - } + log.debug("Portlet List to update: {}", toUpdate); this.getEntityManager().merge(toUpdate); log.debug("Finished persisting changes for portlet list [{}]", toUpdate.getId()); } catch (Exception e) { diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java index 2936d5b537a..ccb5e8774e9 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java @@ -29,6 +29,7 @@ @Slf4j public class PortletListRESTController { public static final String CONTEXT = "/portlet-list/"; + public static final String FAVORITES_KEYWORD = "favorites"; @Autowired private IPortletListService portletListService; @@ -122,13 +123,29 @@ public class PortletListRESTController { try { input = objectMapper.readValue(json, PortletList.class); - if(!portletListService.isPortletListAdmin(person)) { - // Default - non-admins can only create lists for themselves. - input.setOwnerUsername(person.getUserName()); - } else if (StringUtils.isEmpty(input.getOwnerUsername())) { - // Default - admins don't have to specify a user name - input.setOwnerUsername(person.getUserName()); + + if(portletListService.isPortletListAdmin(person)) { + if (StringUtils.isEmpty(input.getOwnerUsername())) { + // Default - admins don't have to specify a user name + input.setOwnerUsername(person.getUserName()); + } + } else { + if (StringUtils.isEmpty(input.getOwnerUsername())) { + // non-admins can only create lists for themselves. + input.setOwnerUsername(person.getUserName()); + } else { + if(!StringUtils.isEmpty(input.getOwnerUsername()) && !person.getUserName().equals(input.getOwnerUsername())) { + log.warn("non-admin user [{}] tried to create a portlet-list [{}] with a different owner [{}], which is not allowed.", person.getUserName(), input.getName(), input.getOwnerUsername()); + return prepareResponse(response, null, "Non-admin user cannot set portlet-list owner", HttpServletResponse.SC_BAD_REQUEST); + } + } + + if(FAVORITES_KEYWORD.equals(input.getName())) { + log.warn("non-admin user [{}] tried to create a portlet-list [{}], which is a reserved keyword, which is not allowed.", person.getUserName(), input.getName(), FAVORITES_KEYWORD); + return prepareResponse(response, null, "Non-admin user cannot set portlet-list name to " + FAVORITES_KEYWORD, HttpServletResponse.SC_BAD_REQUEST); + } } + } catch (Exception e) { log.warn("User [{}] tried to create a portlet-list with bad json - {}", person.getUserName(), e.getMessage()); return prepareResponse(response, null, "Unparsable portlet-list JSON", HttpServletResponse.SC_BAD_REQUEST); @@ -201,6 +218,12 @@ public class PortletListRESTController { log.warn("non-admin user [{}] tried to update portlet-list [{}][{}] with a new owner [{}], which is not allowed.", person.getUserName(), toUpdate.getId(), toUpdate.getName(), input.getOwnerUsername()); return prepareResponse(response, null, "Non-admin user cannot change portlet-list owner", HttpServletResponse.SC_BAD_REQUEST); } + + // Only admins can change a portlet list name to the reserved keyword 'favorites'. + if(FAVORITES_KEYWORD.equals(input.getName()) && !FAVORITES_KEYWORD.equals(toUpdate.getName())) { + log.warn("non-admin user [{}] tried to update portlet-list [{}][{}] with a name to the reserved keyword of [{}], which is not allowed.", person.getUserName(), toUpdate.getId(), toUpdate.getName(), FAVORITES_KEYWORD); + return prepareResponse(response, null, "Non-admin user cannot change portlet-list name to " + FAVORITES_KEYWORD, HttpServletResponse.SC_BAD_REQUEST); + } } else { // Otherwise, disallow changes log.warn("user [{}] tried to update portlet-list [{}][{}], but was not the owner nor an admin.", person.getUserName(), toUpdate.getId(), toUpdate.getName()); @@ -213,7 +236,11 @@ public class PortletListRESTController { } if(input.getItems() != null) { + log.debug("Updating portlet list {} with new list of items, number of items: {}", toUpdate, input.getItems().size()); toUpdate.clearAndSetItems(input.getItems()); + log.debug("Updated portlet list {} with new list of items", toUpdate); + } else { + log.debug("Not updating portlet list items (request items were null: {}", toUpdate); } try { From f445f5750c7df3472d7ebc636e4353e7ab80fc6f Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Wed, 17 Aug 2022 16:00:32 -0500 Subject: [PATCH 05/10] auto formatted code --- .../portal/dao/portletlist/IPortletList.java | 6 +- .../dao/portletlist/IPortletListDao.java | 3 +- .../dao/portletlist/jpa/PortletList.java | 76 ++-- .../dao/portletlist/jpa/PortletListDao.java | 168 ++++---- .../dao/portletlist/jpa/PortletListItem.java | 44 +- .../PortletListRESTController.java | 377 ++++++++++++------ .../portal/rest/utils/InputValidator.java | 14 +- .../portletlist/IPortletListService.java | 7 +- .../portletlist/PortletListService.java | 46 +-- 9 files changed, 417 insertions(+), 324 deletions(-) diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java index 99e19d5fd5a..afc3c3cdb2d 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java @@ -1,11 +1,10 @@ package org.apereo.portal.dao.portletlist; +import java.util.List; import org.apereo.portal.dao.portletlist.jpa.PortletListItem; import org.apereo.portal.security.IPerson; import org.dom4j.Element; -import java.util.List; - public interface IPortletList { String getId(); @@ -20,7 +19,8 @@ public interface IPortletList { List getItems(); - // Don't use this setter - instead use clearAndSetItems(...) to let hibernate handle its own List management + // Don't use this setter - instead use clearAndSetItems(...) to let hibernate handle its own + // List management void setItems(List items); /** Supports exporting. */ diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java index 5b6014fe837..64fd444a345 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletListDao.java @@ -1,8 +1,7 @@ package org.apereo.portal.dao.portletlist; -import org.apereo.portal.security.IPerson; - import java.util.List; +import org.apereo.portal.security.IPerson; public interface IPortletListDao { diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java index 2154885cca4..9b267d4878e 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java @@ -2,10 +2,20 @@ import com.fasterxml.jackson.annotation.JsonFormat; import com.fasterxml.jackson.annotation.JsonProperty; +import java.sql.Timestamp; +import java.time.LocalDateTime; +import java.time.ZoneId; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import javax.persistence.*; +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.OrderBy; +import javax.persistence.Table; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Setter; -import lombok.ToString; import lombok.extern.slf4j.Slf4j; import org.apereo.portal.dao.portletlist.IPortletList; import org.apereo.portal.rest.utils.InputValidator; @@ -14,29 +24,14 @@ import org.hibernate.annotations.*; import org.springframework.util.StringUtils; -import javax.persistence.*; -import javax.persistence.CascadeType; -import javax.persistence.Entity; -import javax.persistence.OrderBy; -import javax.persistence.Table; -import javax.portlet.Portlet; -import java.sql.Timestamp; -import java.time.Instant; -import java.time.LocalDateTime; -import java.time.ZoneId; -import java.util.ArrayList; -import java.util.Date; -import java.util.HashMap; -import java.util.List; - @Getter @Setter @EqualsAndHashCode(onlyExplicitlyIncluded = true) @Slf4j @Entity @Table( - name = "UP_PORTLET_LIST", - uniqueConstraints = { @UniqueConstraint(columnNames = { "OWNER_USERNAME", "NAME" }) }) + name = "UP_PORTLET_LIST", + uniqueConstraints = {@UniqueConstraint(columnNames = {"OWNER_USERNAME", "NAME"})}) @Cacheable @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) @SuppressWarnings("unused") @@ -47,10 +42,7 @@ public class PortletList implements IPortletList { @JsonProperty(access = JsonProperty.Access.READ_ONLY) @Id @GeneratedValue(generator = "UUID") - @GenericGenerator( - name = "UUID", - strategy = "org.hibernate.id.UUIDGenerator" - ) + @GenericGenerator(name = "UUID", strategy = "org.hibernate.id.UUIDGenerator") @Column(name = "ID", updatable = false, nullable = false) private String id; @@ -65,8 +57,7 @@ public class PortletList implements IPortletList { private String createdBy; @JsonProperty(access = JsonProperty.Access.READ_ONLY) - @JsonFormat - (shape = JsonFormat.Shape.STRING, pattern = AUDIT_DATE_FORMAT) + @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = AUDIT_DATE_FORMAT) @Column(name = "CREATED_ON", updatable = false, nullable = false) private Timestamp createdOn; @@ -75,17 +66,16 @@ public class PortletList implements IPortletList { private String updatedBy; @JsonProperty(access = JsonProperty.Access.READ_ONLY) - @JsonFormat - (shape = JsonFormat.Shape.STRING, pattern = AUDIT_DATE_FORMAT) + @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = AUDIT_DATE_FORMAT) @Column(name = "UPDATED_ON", updatable = true, nullable = false) private Timestamp updatedOn; @OneToMany( - targetEntity = PortletListItem.class, - cascade = CascadeType.ALL, - fetch = FetchType.EAGER, - mappedBy = "portletList", - orphanRemoval = true) + targetEntity = PortletListItem.class, + cascade = CascadeType.ALL, + fetch = FetchType.EAGER, + mappedBy = "portletList", + orphanRemoval = true) @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) @Fetch(FetchMode.SELECT) // FM JOIN does BAD things to collections that support duplicates @OrderBy("LIST_ORDER ASC") @@ -102,25 +92,24 @@ public void toElement(Element parent) { parent.addElement("name").addText(this.getName()); parent.addElement("ownerUsername").addText(this.getOwnerUsername()); parent.addElement("items").addText("" + this.getItems().size()); - } public void clearAndSetItems(List items) { - if(this.items == null) { + if (this.items == null) { this.items = new ArrayList<>(); } // Index all current items HashMap existingItems = new HashMap<>(); - for(PortletListItem existingItem : this.items) { + for (PortletListItem existingItem : this.items) { existingItems.put(existingItem.getEntityId(), existingItem); } this.items.clear(); - for(PortletListItem item : items) { + for (PortletListItem item : items) { PortletListItem existingItem = existingItems.get(item.getEntityId()); - if(existingItem != null) { + if (existingItem != null) { // If any item specific attributes are configured, specifically copy them over here. // Order will be set in prepareForPersistence() existingItem.setListOrder(-1); @@ -134,19 +123,20 @@ public void clearAndSetItems(List items) { /** * Final step before letting the object be persisted or merged via the entity manager. * - * Validation is in part, a fail-safe to ensure SQL injections are checked. + *

Validation is in part, a fail-safe to ensure SQL injections are checked. + * * @param requester */ public void prepareForPersistence(IPerson requester) { - if(!StringUtils.isEmpty(this.name)) { + if (!StringUtils.isEmpty(this.name)) { InputValidator.validateAsWordCharacters(this.name, "name"); } - if(!StringUtils.isEmpty(this.ownerUsername)) { + if (!StringUtils.isEmpty(this.ownerUsername)) { InputValidator.validateAsWordCharacters(this.ownerUsername, "ownerUsername"); } - if(this.items != null) { + if (this.items != null) { int order = 0; for (PortletListItem item : this.items) { InputValidator.validateAsWordCharacters(item.getEntityId(), "items > entityId"); @@ -156,7 +146,7 @@ public void prepareForPersistence(IPerson requester) { } // Set / Update audit fields - if(this.createdOn == null) { + if (this.createdOn == null) { this.createdOn = this.updatedOn = Timestamp.valueOf(LocalDateTime.now(tz)); this.createdBy = this.updatedBy = requester.getUserName(); } else { @@ -174,14 +164,14 @@ public String toString() { sb.append("], owner=["); sb.append(ownerUsername); sb.append("], items=: "); - if(this.items.size() < 1) { + if (this.items.size() < 1) { sb.append("[Currently no items]"); } else { final int size = items.size(); for (int i = 0; i < size; i++) { PortletListItem item = items.get(i); sb.append(item); - if(i < size - 1) { + if (i < size - 1) { sb.append("; "); } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java index 13c885506a0..aab0a8104c8 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListDao.java @@ -1,6 +1,9 @@ package org.apereo.portal.dao.portletlist.jpa; import com.google.common.base.Function; +import java.util.*; +import javax.persistence.TypedQuery; +import javax.persistence.criteria.*; import lombok.extern.slf4j.Slf4j; import org.apereo.portal.dao.portletlist.IPortletList; import org.apereo.portal.dao.portletlist.IPortletListDao; @@ -8,10 +11,6 @@ import org.apereo.portal.security.IPerson; import org.springframework.stereotype.Repository; -import javax.persistence.TypedQuery; -import javax.persistence.criteria.*; -import java.util.*; - @Slf4j @Repository("portletListDao") public class PortletListDao extends BasePortalJpaDao implements IPortletListDao { @@ -28,81 +27,61 @@ public class PortletListDao extends BasePortalJpaDao implements IPortletListDao @Override public void afterPropertiesSet() throws Exception { - this.ownerUsernameParameter = this.createParameterExpression(String.class, "OWNER_USERNAME"); + this.ownerUsernameParameter = + this.createParameterExpression(String.class, "OWNER_USERNAME"); this.portletListUuidParameter = this.createParameterExpression(String.class, "ID"); this.portletListsQuery = - this.createCriteriaQuery( - new Function< - CriteriaBuilder, - CriteriaQuery>() { - @Override - public CriteriaQuery apply( - CriteriaBuilder cb) { - final CriteriaQuery - criteriaQuery = - cb.createQuery( - PortletList.class); - Root root = - criteriaQuery.from( - PortletList.class); - criteriaQuery - .select(root); - return criteriaQuery; - } - }); + this.createCriteriaQuery( + new Function>() { + @Override + public CriteriaQuery apply(CriteriaBuilder cb) { + final CriteriaQuery criteriaQuery = + cb.createQuery(PortletList.class); + Root root = criteriaQuery.from(PortletList.class); + criteriaQuery.select(root); + return criteriaQuery; + } + }); this.portletListsByUserIdQuery = - this.createCriteriaQuery( - new Function< - CriteriaBuilder, - CriteriaQuery>() { - @Override - public CriteriaQuery apply( - CriteriaBuilder cb) { - final CriteriaQuery - criteriaQuery = - cb.createQuery( - PortletList.class); - Root root = - criteriaQuery.from( - PortletList.class); - criteriaQuery - .select(root) - .where(cb.equal(root.get("ownerUsername"), ownerUsernameParameter)); - return criteriaQuery; - } - }); + this.createCriteriaQuery( + new Function>() { + @Override + public CriteriaQuery apply(CriteriaBuilder cb) { + final CriteriaQuery criteriaQuery = + cb.createQuery(PortletList.class); + Root root = criteriaQuery.from(PortletList.class); + criteriaQuery + .select(root) + .where( + cb.equal( + root.get("ownerUsername"), + ownerUsernameParameter)); + return criteriaQuery; + } + }); this.portletListByPortletListUuidQuery = - this.createCriteriaQuery( - new Function< - CriteriaBuilder, - CriteriaQuery>() { - @Override - public CriteriaQuery apply( - CriteriaBuilder cb) { - final CriteriaQuery - criteriaQuery = - cb.createQuery( - PortletList.class); - Root root = - criteriaQuery.from( - PortletList.class); - criteriaQuery - .select(root) - .where(cb.equal(root.get("id"), portletListUuidParameter)); - return criteriaQuery; - } - }); + this.createCriteriaQuery( + new Function>() { + @Override + public CriteriaQuery apply(CriteriaBuilder cb) { + final CriteriaQuery criteriaQuery = + cb.createQuery(PortletList.class); + Root root = criteriaQuery.from(PortletList.class); + criteriaQuery + .select(root) + .where(cb.equal(root.get("id"), portletListUuidParameter)); + return criteriaQuery; + } + }); } @PortalTransactionalReadOnly @Override - public List getPortletLists( - String ownerUsername) { - TypedQuery query = - this.createCachedQuery(portletListsByUserIdQuery); + public List getPortletLists(String ownerUsername) { + TypedQuery query = this.createCachedQuery(portletListsByUserIdQuery); query.setParameter(ownerUsernameParameter, ownerUsername); List entities = new ArrayList<>(query.getResultList()); return entities; @@ -111,25 +90,25 @@ public List getPortletLists( @PortalTransactionalReadOnly @Override public List getPortletLists() { - TypedQuery query = - this.createCachedQuery(portletListsQuery); + TypedQuery query = this.createCachedQuery(portletListsQuery); List entities = new ArrayList<>(query.getResultList()); return entities; } @PortalTransactionalReadOnly @Override - public IPortletList getPortletList( - String portletListUuid) { - TypedQuery query = - this.createCachedQuery(portletListByPortletListUuidQuery); + public IPortletList getPortletList(String portletListUuid) { + TypedQuery query = this.createCachedQuery(portletListByPortletListUuidQuery); query.setParameter(portletListUuidParameter, portletListUuid); List lists = query.getResultList(); - if(lists.size() < 1) { + if (lists.size() < 1) { return null; - } else if(lists.size() > 1) { - log.error("Expected up to 1 portlet list for portlet list uuid [{}], but found [{}].", portletListUuid, lists.size()); + } else if (lists.size() > 1) { + log.error( + "Expected up to 1 portlet list for portlet list uuid [{}], but found [{}].", + portletListUuid, + lists.size()); return null; } @@ -140,7 +119,10 @@ public IPortletList getPortletList( @PortalTransactional @Override public IPortletList createPortletList(IPortletList toCreate, IPerson requester) { - log.debug("Persisting portlet list [{}] with owner [{}]", toCreate.getName(), toCreate.getOwnerUsername()); + log.debug( + "Persisting portlet list [{}] with owner [{}]", + toCreate.getName(), + toCreate.getOwnerUsername()); try { toCreate.prepareForPersistence(requester); this.getEntityManager().persist(toCreate); @@ -148,7 +130,11 @@ public IPortletList createPortletList(IPortletList toCreate, IPerson requester) log.debug("Failed to persist portlet list", e); throw e; } - log.debug("Finished persisting portlet list [{}] for owner [{}]. ID = [{}]", toCreate.getName(), toCreate.getOwnerUsername(), toCreate.getId().toString()); + log.debug( + "Finished persisting portlet list [{}] for owner [{}]. ID = [{}]", + toCreate.getName(), + toCreate.getOwnerUsername(), + toCreate.getId().toString()); return toCreate; } @@ -172,8 +158,11 @@ public IPortletList updatePortletList(IPortletList toUpdate, IPerson requester) @Override public boolean removePortletListAsAdmin(String portletListUuid, IPerson requester) { IPortletList list = this.getPortletList(portletListUuid); - if(list == null) { - log.warn("Admin user [{}] tried to remove a non-existent list [{}]. Failing request.", requester.getUserName(), portletListUuid); + if (list == null) { + log.warn( + "Admin user [{}] tried to remove a non-existent list [{}]. Failing request.", + requester.getUserName(), + portletListUuid); return false; } this.getEntityManager().remove(list); @@ -184,15 +173,24 @@ public boolean removePortletListAsAdmin(String portletListUuid, IPerson requeste @Override public boolean removePortletListAsOwner(String portletListUuid, IPerson requester) { IPortletList list = this.getPortletList(portletListUuid); - if(list == null) { - log.warn("Non-admin user [{}] tried to remove a non-existent list [{}]. Failing request.", requester.getUserName(), portletListUuid); + if (list == null) { + log.warn( + "Non-admin user [{}] tried to remove a non-existent list [{}]. Failing request.", + requester.getUserName(), + portletListUuid); return false; } else if (!list.getOwnerUsername().equals(requester.getUserName())) { - log.warn("Non-admin user [{}] tried to remove a list they didn't own [{}]. Failing request.", requester.getUserName(), portletListUuid); + log.warn( + "Non-admin user [{}] tried to remove a list they didn't own [{}]. Failing request.", + requester.getUserName(), + portletListUuid); return false; } - log.debug("Non-admin user [{}] requested to remove a list they own [{}]. Allowing request.", requester.getUserName(), portletListUuid); + log.debug( + "Non-admin user [{}] requested to remove a list they own [{}]. Allowing request.", + requester.getUserName(), + portletListUuid); this.getEntityManager().remove(list); return true; } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java index 0f4ff8f625d..2aa8f66b01d 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java @@ -1,6 +1,7 @@ package org.apereo.portal.dao.portletlist.jpa; import com.fasterxml.jackson.annotation.JsonIgnore; +import javax.persistence.*; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Setter; @@ -10,8 +11,6 @@ import org.hibernate.annotations.CacheConcurrencyStrategy; import org.hibernate.annotations.GenericGenerator; -import javax.persistence.*; - @Getter @Setter @EqualsAndHashCode(onlyExplicitlyIncluded = true) @@ -19,12 +18,12 @@ @Slf4j @Entity @Table( - // This is ONLY to be used as part of a portlet list, so not specifying a PK - name = "UP_PORTLET_LIST_ITEM", - uniqueConstraints = { - // These are sets of lists - @UniqueConstraint(columnNames = { "LIST_ID", "LIST_ORDER", "ENTITY_ID" }), - }) + // This is ONLY to be used as part of a portlet list, so not specifying a PK + name = "UP_PORTLET_LIST_ITEM", + uniqueConstraints = { + // These are sets of lists + @UniqueConstraint(columnNames = {"LIST_ID", "LIST_ORDER", "ENTITY_ID"}), + }) @Cacheable @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) @SuppressWarnings("unused") @@ -33,26 +32,17 @@ public class PortletListItem implements IPortletListItem { @JsonIgnore @Id @GeneratedValue(generator = "UUID") - @GenericGenerator( - name = "UUID", - strategy = "org.hibernate.id.UUIDGenerator" - ) + @GenericGenerator(name = "UUID", strategy = "org.hibernate.id.UUIDGenerator") @Column(name = "ID", updatable = false, nullable = false) private String id; @JsonIgnore @ManyToOne(optional = false) - @JoinColumn( - name = "LIST_ID", - referencedColumnName = "ID") + @JoinColumn(name = "LIST_ID", referencedColumnName = "ID") private PortletList portletList; @JsonIgnore - @Column( - name = "LIST_ORDER", - unique = false, - nullable = false, - updatable = true) + @Column(name = "LIST_ORDER", unique = false, nullable = false, updatable = true) private int listOrder; // This is generally the portlet fname, but could be adjusted in the future @@ -68,10 +58,14 @@ public PortletListItem(String entityId) { } public String toString() { - return "PortletListItem: id=[" + id + - "], portlet-list=[" + (portletList == null ? "NULL" : portletList.getId()) + - "], order=[" + listOrder + - "], entity-id=[" + entityId + - "]"; + return "PortletListItem: id=[" + + id + + "], portlet-list=[" + + (portletList == null ? "NULL" : portletList.getId()) + + "], order=[" + + listOrder + + "], entity-id=[" + + entityId + + "]"; } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java index ccb5e8774e9..bce68888d31 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java @@ -1,14 +1,19 @@ package org.apereo.portal.rest.portletlist; +import static org.springframework.web.bind.annotation.RequestMethod.*; + import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.List; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; import org.apereo.portal.dao.portletlist.IPortletList; import org.apereo.portal.dao.portletlist.jpa.PortletList; import org.apereo.portal.rest.utils.ErrorResponse; -import org.apereo.portal.security.RuntimeAuthorizationException; -import org.apereo.portal.services.portletlist.IPortletListService; import org.apereo.portal.security.IPerson; import org.apereo.portal.security.IPersonManager; +import org.apereo.portal.security.RuntimeAuthorizationException; +import org.apereo.portal.services.portletlist.IPortletListService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.MediaType; @@ -16,107 +21,110 @@ import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.*; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import java.util.List; - -import static org.springframework.web.bind.annotation.RequestMethod.*; - /** PortletListRESTController provides a REST endpoint for interacting with portlet lists. */ - @Controller @Slf4j public class PortletListRESTController { public static final String CONTEXT = "/portlet-list/"; public static final String FAVORITES_KEYWORD = "favorites"; - @Autowired - private IPortletListService portletListService; + @Autowired private IPortletListService portletListService; - @Autowired - private IPersonManager personManager; + @Autowired private IPersonManager personManager; @Autowired private ObjectMapper objectMapper; /** * Provide a JSON view of all portlet lists. * - * If an administrator makes this call, ALL portlet lists will be returned. - * Otherwise, only the portlet lists that the requester owns will be returned. + *

If an administrator makes this call, ALL portlet lists will be returned. Otherwise, only + * the portlet lists that the requester owns will be returned. */ - @RequestMapping( - value = CONTEXT, - method = GET, - produces = MediaType.APPLICATION_JSON_VALUE) - public @ResponseBody String getPortletLists(HttpServletRequest request, HttpServletResponse response) { + @RequestMapping(value = CONTEXT, method = GET, produces = MediaType.APPLICATION_JSON_VALUE) + public @ResponseBody String getPortletLists( + HttpServletRequest request, HttpServletResponse response) { final IPerson person = personManager.getPerson(request); - if(log.isDebugEnabled()) { + if (log.isDebugEnabled()) { debugPerson("getAllPortletLists", person); } - if(person.isGuest()) { + if (person.isGuest()) { log.warn("Guest is trying to access portlet-list API, which is not allowed."); - return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + return prepareResponse( + response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } - List pLists = portletListService.isPortletListAdmin(person) ? - portletListService.getPortletLists() : portletListService.getPortletLists(person); + List pLists = + portletListService.isPortletListAdmin(person) + ? portletListService.getPortletLists() + : portletListService.getPortletLists(person); return prepareResponse(response, pLists, null, HttpServletResponse.SC_OK); } /** * Provide a JSON view of a given portlet list. * - * If an administrator makes the request, the portlet list will be returned, regardless of ownership. - * If anyone else makes the request, the portlet list will only be returned if the requester is the owner. + *

If an administrator makes the request, the portlet list will be returned, regardless of + * ownership. If anyone else makes the request, the portlet list will only be returned if the + * requester is the owner. */ @RequestMapping( - value = CONTEXT + "{portletListUuid}", - method = GET, - produces = MediaType.APPLICATION_JSON_VALUE) - public @ResponseBody String getPortletList(HttpServletRequest request, HttpServletResponse response, @PathVariable String portletListUuid) { + value = CONTEXT + "{portletListUuid}", + method = GET, + produces = MediaType.APPLICATION_JSON_VALUE) + public @ResponseBody String getPortletList( + HttpServletRequest request, + HttpServletResponse response, + @PathVariable String portletListUuid) { final IPerson person = personManager.getPerson(request); - if(log.isDebugEnabled()) { + if (log.isDebugEnabled()) { debugPerson("getSpecificPortletList", person); } - if(person.isGuest()) { + if (person.isGuest()) { log.warn("Guest is trying to access portlet-list API, which is not allowed."); - return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + return prepareResponse( + response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } IPortletList pList = portletListService.getPortletList(portletListUuid); - if(pList == null) { - log.warn("User [{}] tried to access portlet-list [{}], but list was not found.", person.getUserName(), portletListUuid); - return prepareResponse(response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); - } else if(!portletListService.isPortletListAdmin(person) && !pList.getOwnerUsername().equals(person.getUserName())) { + if (pList == null) { + log.warn( + "User [{}] tried to access portlet-list [{}], but list was not found.", + person.getUserName(), + portletListUuid); + return prepareResponse( + response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); + } else if (!portletListService.isPortletListAdmin(person) + && !pList.getOwnerUsername().equals(person.getUserName())) { // Not an admin, and not the owner - log.warn("Non-admin user [{}] tried to access portlet-list [{}] with owner [{}], but was blocked since they aren't the owner.", person.getUserName(), portletListUuid, pList.getOwnerUsername()); - return prepareResponse(response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); + log.warn( + "Non-admin user [{}] tried to access portlet-list [{}] with owner [{}], but was blocked since they aren't the owner.", + person.getUserName(), + portletListUuid, + pList.getOwnerUsername()); + return prepareResponse( + response, null, "Entity not found", HttpServletResponse.SC_NOT_FOUND); } return prepareResponse(response, pList, null, HttpServletResponse.SC_OK); } - @RequestMapping( - value = CONTEXT, - method = POST, - produces = MediaType.APPLICATION_JSON_VALUE) + @RequestMapping(value = CONTEXT, method = POST, produces = MediaType.APPLICATION_JSON_VALUE) public @ResponseBody String createPortletList( - HttpServletRequest request, - HttpServletResponse response, - @RequestBody String json) { + HttpServletRequest request, HttpServletResponse response, @RequestBody String json) { final IPerson person = personManager.getPerson(request); - if(log.isDebugEnabled()) { + if (log.isDebugEnabled()) { debugPerson("createPortletList", person); log.debug("createPortletList > JSON body is = {}", json); } - if(person.isGuest()) { - log.warn("createPortletList > Guest is trying to access portlet-list API, which is not allowed."); - return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + if (person.isGuest()) { + log.warn( + "createPortletList > Guest is trying to access portlet-list API, which is not allowed."); + return prepareResponse( + response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } PortletList input; @@ -124,7 +132,7 @@ public class PortletListRESTController { try { input = objectMapper.readValue(json, PortletList.class); - if(portletListService.isPortletListAdmin(person)) { + if (portletListService.isPortletListAdmin(person)) { if (StringUtils.isEmpty(input.getOwnerUsername())) { // Default - admins don't have to specify a user name input.setOwnerUsername(person.getUserName()); @@ -134,109 +142,181 @@ public class PortletListRESTController { // non-admins can only create lists for themselves. input.setOwnerUsername(person.getUserName()); } else { - if(!StringUtils.isEmpty(input.getOwnerUsername()) && !person.getUserName().equals(input.getOwnerUsername())) { - log.warn("non-admin user [{}] tried to create a portlet-list [{}] with a different owner [{}], which is not allowed.", person.getUserName(), input.getName(), input.getOwnerUsername()); - return prepareResponse(response, null, "Non-admin user cannot set portlet-list owner", HttpServletResponse.SC_BAD_REQUEST); + if (!StringUtils.isEmpty(input.getOwnerUsername()) + && !person.getUserName().equals(input.getOwnerUsername())) { + log.warn( + "non-admin user [{}] tried to create a portlet-list [{}] with a different owner [{}], which is not allowed.", + person.getUserName(), + input.getName(), + input.getOwnerUsername()); + return prepareResponse( + response, + null, + "Non-admin user cannot set portlet-list owner", + HttpServletResponse.SC_BAD_REQUEST); } } - if(FAVORITES_KEYWORD.equals(input.getName())) { - log.warn("non-admin user [{}] tried to create a portlet-list [{}], which is a reserved keyword, which is not allowed.", person.getUserName(), input.getName(), FAVORITES_KEYWORD); - return prepareResponse(response, null, "Non-admin user cannot set portlet-list name to " + FAVORITES_KEYWORD, HttpServletResponse.SC_BAD_REQUEST); + if (FAVORITES_KEYWORD.equals(input.getName())) { + log.warn( + "non-admin user [{}] tried to create a portlet-list [{}], which is a reserved keyword, which is not allowed.", + person.getUserName(), + input.getName(), + FAVORITES_KEYWORD); + return prepareResponse( + response, + null, + "Non-admin user cannot set portlet-list name to " + FAVORITES_KEYWORD, + HttpServletResponse.SC_BAD_REQUEST); } } } catch (Exception e) { - log.warn("User [{}] tried to create a portlet-list with bad json - {}", person.getUserName(), e.getMessage()); - return prepareResponse(response, null, "Unparsable portlet-list JSON", HttpServletResponse.SC_BAD_REQUEST); + log.warn( + "User [{}] tried to create a portlet-list with bad json - {}", + person.getUserName(), + e.getMessage()); + return prepareResponse( + response, + null, + "Unparsable portlet-list JSON", + HttpServletResponse.SC_BAD_REQUEST); } try { - final IPortletList created = portletListService.createPortletList( - person, input); + final IPortletList created = portletListService.createPortletList(person, input); response.setHeader("Location", created.getId()); return prepareResponse(response, null, null, HttpServletResponse.SC_CREATED); } catch (RuntimeAuthorizationException rae) { log.warn("RuntimeAuthorizationException thrown - {}", rae.getMessage(), rae); - return prepareResponse(response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); + return prepareResponse( + response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); } catch (IllegalArgumentException iae) { log.warn("IllegalArgumentException thrown - {}", iae.getMessage(), iae); - return prepareResponse(response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + return prepareResponse( + response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); } catch (DataIntegrityViolationException dive) { - log.warn("Attempted violation of data integrity when creating a portlet list {}", dive.getMessage(), dive); - return prepareResponse(response, null, "Data integrity issue - such as specifying a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); + log.warn( + "Attempted violation of data integrity when creating a portlet list {}", + dive.getMessage(), + dive); + return prepareResponse( + response, + null, + "Data integrity issue - such as specifying a non-unique name.", + HttpServletResponse.SC_BAD_REQUEST); } catch (Exception e) { log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName(), e); - return prepareResponse(response, null, "Something unexpected occurred. Please check with your System Administrator", HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return prepareResponse( + response, + null, + "Something unexpected occurred. Please check with your System Administrator", + HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } } @RequestMapping( - value = CONTEXT + "{portletListUuid}", - method = PUT, - produces = MediaType.APPLICATION_JSON_VALUE) + value = CONTEXT + "{portletListUuid}", + method = PUT, + produces = MediaType.APPLICATION_JSON_VALUE) public @ResponseBody String updatePortletList( - HttpServletRequest request, - HttpServletResponse response, - @RequestBody String json, - @PathVariable String portletListUuid) { + HttpServletRequest request, + HttpServletResponse response, + @RequestBody String json, + @PathVariable String portletListUuid) { final IPerson person = personManager.getPerson(request); - if(log.isDebugEnabled()) { + if (log.isDebugEnabled()) { debugPerson("updatePortletList", person); log.debug("updatePortletList > JSON body is = {}", json); } - if(person.isGuest()) { + if (person.isGuest()) { log.warn("Guest user is trying to access portlet-list PUT API, which is not allowed."); - return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + return prepareResponse( + response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } IPortletList input; try { input = objectMapper.readValue(json, PortletList.class); } catch (Exception e) { - log.warn("User [{}] tried to update a portlet-list with bad json - {}", person.getUserName(), e.getMessage()); - return prepareResponse(response, null, "Unparsable portlet-list JSON", HttpServletResponse.SC_BAD_REQUEST); + log.warn( + "User [{}] tried to update a portlet-list with bad json - {}", + person.getUserName(), + e.getMessage()); + return prepareResponse( + response, + null, + "Unparsable portlet-list JSON", + HttpServletResponse.SC_BAD_REQUEST); } // Overlay changes onto a known entity, and then persist that entity. IPortletList toUpdate = portletListService.getPortletList(portletListUuid); - if(toUpdate == null) { - return prepareResponse(response, null, "Unknown portlet-list", HttpServletResponse.SC_NOT_FOUND); + if (toUpdate == null) { + return prepareResponse( + response, null, "Unknown portlet-list", HttpServletResponse.SC_NOT_FOUND); } - if(portletListService.isPortletListAdmin(person)) { + if (portletListService.isPortletListAdmin(person)) { // If admin, allow admin-level changes - if(!StringUtils.isEmpty(input.getOwnerUsername())) { + if (!StringUtils.isEmpty(input.getOwnerUsername())) { toUpdate.setOwnerUsername(input.getOwnerUsername()); } } else if (toUpdate.getOwnerUsername().equals(person.getUserName())) { // If owner of portlet-list, allow only owner-level changes - if(!StringUtils.isEmpty(input.getOwnerUsername())) { - log.warn("non-admin user [{}] tried to update portlet-list [{}][{}] with a new owner [{}], which is not allowed.", person.getUserName(), toUpdate.getId(), toUpdate.getName(), input.getOwnerUsername()); - return prepareResponse(response, null, "Non-admin user cannot change portlet-list owner", HttpServletResponse.SC_BAD_REQUEST); + if (!StringUtils.isEmpty(input.getOwnerUsername())) { + log.warn( + "non-admin user [{}] tried to update portlet-list [{}][{}] with a new owner [{}], which is not allowed.", + person.getUserName(), + toUpdate.getId(), + toUpdate.getName(), + input.getOwnerUsername()); + return prepareResponse( + response, + null, + "Non-admin user cannot change portlet-list owner", + HttpServletResponse.SC_BAD_REQUEST); } // Only admins can change a portlet list name to the reserved keyword 'favorites'. - if(FAVORITES_KEYWORD.equals(input.getName()) && !FAVORITES_KEYWORD.equals(toUpdate.getName())) { - log.warn("non-admin user [{}] tried to update portlet-list [{}][{}] with a name to the reserved keyword of [{}], which is not allowed.", person.getUserName(), toUpdate.getId(), toUpdate.getName(), FAVORITES_KEYWORD); - return prepareResponse(response, null, "Non-admin user cannot change portlet-list name to " + FAVORITES_KEYWORD, HttpServletResponse.SC_BAD_REQUEST); + if (FAVORITES_KEYWORD.equals(input.getName()) + && !FAVORITES_KEYWORD.equals(toUpdate.getName())) { + log.warn( + "non-admin user [{}] tried to update portlet-list [{}][{}] with a name to the reserved keyword of [{}], which is not allowed.", + person.getUserName(), + toUpdate.getId(), + toUpdate.getName(), + FAVORITES_KEYWORD); + return prepareResponse( + response, + null, + "Non-admin user cannot change portlet-list name to " + FAVORITES_KEYWORD, + HttpServletResponse.SC_BAD_REQUEST); } } else { // Otherwise, disallow changes - log.warn("user [{}] tried to update portlet-list [{}][{}], but was not the owner nor an admin.", person.getUserName(), toUpdate.getId(), toUpdate.getName()); - return prepareResponse(response, null, "Unknown portlet-list", HttpServletResponse.SC_UNAUTHORIZED); + log.warn( + "user [{}] tried to update portlet-list [{}][{}], but was not the owner nor an admin.", + person.getUserName(), + toUpdate.getId(), + toUpdate.getName()); + return prepareResponse( + response, null, "Unknown portlet-list", HttpServletResponse.SC_UNAUTHORIZED); } // Either an owner or an admin. allow general-level changes: - if(!StringUtils.isEmpty(input.getName())) { + if (!StringUtils.isEmpty(input.getName())) { toUpdate.setName(input.getName()); } - if(input.getItems() != null) { - log.debug("Updating portlet list {} with new list of items, number of items: {}", toUpdate, input.getItems().size()); + if (input.getItems() != null) { + log.debug( + "Updating portlet list {} with new list of items, number of items: {}", + toUpdate, + input.getItems().size()); toUpdate.clearAndSetItems(input.getItems()); log.debug("Updated portlet list {} with new list of items", toUpdate); } else { @@ -245,53 +325,81 @@ public class PortletListRESTController { try { final IPortletList updated = portletListService.updatePortletList(person, toUpdate); - if(updated == null) { - log.warn("update returned null for portlet-list uuid [{}]. Failing request.", portletListUuid); - return prepareResponse(response, null, "Error occurred while updating portlet list. Please check your System Administrator", HttpServletResponse.SC_BAD_REQUEST); + if (updated == null) { + log.warn( + "update returned null for portlet-list uuid [{}]. Failing request.", + portletListUuid); + return prepareResponse( + response, + null, + "Error occurred while updating portlet list. Please check your System Administrator", + HttpServletResponse.SC_BAD_REQUEST); } else { return prepareResponse(response, null, null, HttpServletResponse.SC_OK); } } catch (RuntimeAuthorizationException rae) { log.warn("RuntimeAuthorizationException thrown - {}", rae.getMessage(), rae); - return prepareResponse(response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); + return prepareResponse( + response, null, "not authorized", HttpServletResponse.SC_FORBIDDEN); } catch (IllegalArgumentException iae) { log.warn("IllegalArgumentException thrown - {}", iae.getMessage(), iae); - return prepareResponse(response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + return prepareResponse( + response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); } catch (DataIntegrityViolationException dive) { - log.warn("Attempted violation of data integrity when updating a portlet list {}", dive.getMessage(), dive); - return prepareResponse(response, null, "Data integrity issue - such as specifying a non-unique name.", HttpServletResponse.SC_BAD_REQUEST); + log.warn( + "Attempted violation of data integrity when updating a portlet list {}", + dive.getMessage(), + dive); + return prepareResponse( + response, + null, + "Data integrity issue - such as specifying a non-unique name.", + HttpServletResponse.SC_BAD_REQUEST); } catch (Exception e) { log.warn("Just hit an exception of type {}", e.getClass().getCanonicalName(), e); - return prepareResponse(response, null, "Something unexpected occurred. Please check with your System Administrator.", HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + return prepareResponse( + response, + null, + "Something unexpected occurred. Please check with your System Administrator.", + HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } } /** * Provide a JSON view of a given portlet list. * - * If an administrator makes the request, the portlet list will be returned, regardless of ownership. - * If anyone else makes the request, the portlet list will only be returned if the requester is the owner. + *

If an administrator makes the request, the portlet list will be returned, regardless of + * ownership. If anyone else makes the request, the portlet list will only be returned if the + * requester is the owner. */ @RequestMapping( - value = CONTEXT + "{portletListUuid}", - method = DELETE, - produces = MediaType.APPLICATION_JSON_VALUE) - public @ResponseBody String removePortletList(HttpServletRequest request, HttpServletResponse response, @PathVariable String portletListUuid) { + value = CONTEXT + "{portletListUuid}", + method = DELETE, + produces = MediaType.APPLICATION_JSON_VALUE) + public @ResponseBody String removePortletList( + HttpServletRequest request, + HttpServletResponse response, + @PathVariable String portletListUuid) { final IPerson person = personManager.getPerson(request); - if(log.isDebugEnabled()) { + if (log.isDebugEnabled()) { debugPerson("removePortletList", person); } - if(person.isGuest()) { + if (person.isGuest()) { log.warn("Guest is trying to access portlet-list API, which is not allowed."); - return prepareResponse(response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); + return prepareResponse( + response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } try { - if(portletListService.removePortletList(person, portletListUuid)) { + if (portletListService.removePortletList(person, portletListUuid)) { return prepareResponse(response, null, null, HttpServletResponse.SC_OK); } else { - return prepareResponse(response, null, "Unable to remove portlet list. Please check with your System Administrator.", HttpServletResponse.SC_BAD_REQUEST); + return prepareResponse( + response, + null, + "Unable to remove portlet list. Please check with your System Administrator.", + HttpServletResponse.SC_BAD_REQUEST); } } catch (Exception e) { log.error("Unable to delete portlet list. Returning a 500.", e); @@ -301,10 +409,7 @@ public class PortletListRESTController { } private String prepareResponse( - HttpServletResponse response, - Object returnPayload, - String errorMessage, - int status) { + HttpServletResponse response, Object returnPayload, String errorMessage, int status) { response.setContentType(MediaType.APPLICATION_JSON_VALUE); Object payloadToReturn = returnPayload; int statusToReturn = status; @@ -313,19 +418,26 @@ private String prepareResponse( payloadToReturn = new ErrorResponse(errorMessage); } - log.debug("returnPayload is null = {}, errorMessage is null = {}, final return payload is null = {}", - (returnPayload == null), - (errorMessage == null), - (payloadToReturn == null)); + log.debug( + "returnPayload is null = {}, errorMessage is null = {}, final return payload is null = {}", + (returnPayload == null), + (errorMessage == null), + (payloadToReturn == null)); try { response.setStatus(statusToReturn); // If there is no payload, and no error, return a null body in the response - final String payloadAsString = ((returnPayload == null) && (errorMessage == null)) ? null : objectMapper.writeValueAsString(payloadToReturn); - log.debug("Prepared JSON Response - response code [{}], object type [{}], JSON as string: {}", - statusToReturn, - (payloadToReturn == null) ? "NULL" : payloadToReturn.getClass().getCanonicalName(), - payloadAsString); + final String payloadAsString = + ((returnPayload == null) && (errorMessage == null)) + ? null + : objectMapper.writeValueAsString(payloadToReturn); + log.debug( + "Prepared JSON Response - response code [{}], object type [{}], JSON as string: {}", + statusToReturn, + (payloadToReturn == null) + ? "NULL" + : payloadToReturn.getClass().getCanonicalName(), + payloadAsString); return payloadAsString; } catch (Exception e) { log.error("Unable to write out payload object as JSON. Returning a 500.", e); @@ -335,10 +447,11 @@ private String prepareResponse( } private void debugPerson(String flow, IPerson person) { - log.debug("{} > Current user: username={}, isGuest={}, isAdmin={}", - flow, - person.getUserName(), - person.isGuest(), - portletListService.isPortletListAdmin(person)); + log.debug( + "{} > Current user: username={}, isGuest={}, isAdmin={}", + flow, + person.getUserName(), + person.isGuest(), + portletListService.isPortletListAdmin(person)); } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java index f4bfb329653..39e0b511e46 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java @@ -3,21 +3,23 @@ import java.util.regex.Pattern; public class InputValidator { - private static final String ALPHANUMERIC_AND_DASH_VALIDATOR_REGEX = "^[\\w\\-]{1,500}$"; // 1-500 characters + private static final String ALPHANUMERIC_AND_DASH_VALIDATOR_REGEX = + "^[\\w\\-]{1,500}$"; // 1-500 characters private static final Pattern ALPHANUMERIC_AND_DASH_VALIDATOR_PATTERN = - Pattern.compile(ALPHANUMERIC_AND_DASH_VALIDATOR_REGEX); + Pattern.compile(ALPHANUMERIC_AND_DASH_VALIDATOR_REGEX); /** - * * @param s value to validate * @param inputType not sanitized and will be returned in json. * @return The validated value */ public static String validateAsWordCharacters(String s, String inputType) { if (!ALPHANUMERIC_AND_DASH_VALIDATOR_PATTERN.matcher(s).matches()) { - throw new IllegalArgumentException( - "Specified " + inputType + " is not the correct length or has invalid characters."); - } + throw new IllegalArgumentException( + "Specified " + + inputType + + " is not the correct length or has invalid characters."); + } return s; } } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java index b4bc03ccd7f..221bdb92870 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/IPortletListService.java @@ -1,12 +1,9 @@ package org.apereo.portal.services.portletlist; +import java.util.List; import org.apereo.portal.dao.portletlist.IPortletList; -import org.apereo.portal.security.IAuthorizationPrincipal; import org.apereo.portal.security.IPerson; -import java.util.List; -import java.util.Set; - public interface IPortletListService { public List getPortletLists(); @@ -16,6 +13,7 @@ public interface IPortletListService { /** * Returns null if not found + * * @param portletListUuid * @return */ @@ -26,5 +24,4 @@ public interface IPortletListService { public IPortletList updatePortletList(IPerson requester, IPortletList toUpdate); public boolean isPortletListAdmin(IPerson person); - } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java index 2c127900d0e..deb77bce65d 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java @@ -1,9 +1,9 @@ package org.apereo.portal.services.portletlist; -import jdk.internal.util.xml.impl.Input; -import org.apereo.portal.dao.portletlist.IPortletListDao; +import java.util.List; +import lombok.extern.slf4j.Slf4j; import org.apereo.portal.dao.portletlist.IPortletList; -import org.apereo.portal.rest.utils.InputValidator; +import org.apereo.portal.dao.portletlist.IPortletListDao; import org.apereo.portal.security.AuthorizationPrincipalHelper; import org.apereo.portal.security.IAuthorizationService; import org.apereo.portal.security.IPermission; @@ -11,31 +11,25 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; -import java.util.List; - -import lombok.extern.slf4j.Slf4j; /** * Service layer that sits atop the DAO layer and enforces permissions and/or business rules that - * apply to CRUD operations on portlet lists. External clients should interact with this service - * -- instead of the DAOs directly -- whenever the actions are undertaken on behalf of a specific - * user. + * apply to CRUD operations on portlet lists. External clients should interact with this service -- + * instead of the DAOs directly -- whenever the actions are undertaken on behalf of a specific user. */ @Service @Slf4j public final class PortletListService implements IPortletListService { - @Autowired - private IPortletListDao portletListDao; + @Autowired private IPortletListDao portletListDao; - @Autowired - private IAuthorizationService authorizationService; + @Autowired private IAuthorizationService authorizationService; public boolean isPortletListAdmin(IPerson person) { return authorizationService.doesPrincipalHavePermission( - AuthorizationPrincipalHelper.principalFromUser(person), - IPermission.PORTAL_SYSTEM, - IPermission.ALL_PERMISSIONS_ACTIVITY, - IPermission.ALL_TARGET); + AuthorizationPrincipalHelper.principalFromUser(person), + IPermission.PORTAL_SYSTEM, + IPermission.ALL_PERMISSIONS_ACTIVITY, + IPermission.ALL_TARGET); } /** @@ -76,25 +70,31 @@ public IPortletList getPortletList(String portletListUuid) { @Override public boolean removePortletList(IPerson requester, String portletListUuid) { - if(isPortletListAdmin(requester)) { + if (isPortletListAdmin(requester)) { return portletListDao.removePortletListAsAdmin(portletListUuid, requester); } else { return portletListDao.removePortletListAsOwner(portletListUuid, requester); } } - /** TODO docs Verifies permissions and that the group doesn't already exist (case insensitive) */ - + /** + * TODO docs Verifies permissions and that the group doesn't already exist (case insensitive) + */ @Override public IPortletList createPortletList(IPerson requester, IPortletList toCreate) { - log.debug("Using DAO to create portlet list [{}] for user [{}]", toCreate.getName(), toCreate.getOwnerUsername()); + log.debug( + "Using DAO to create portlet list [{}] for user [{}]", + toCreate.getName(), + toCreate.getOwnerUsername()); return portletListDao.createPortletList(toCreate, requester); } @Override public IPortletList updatePortletList(IPerson requester, IPortletList toUpdate) { - log.debug("Using DAO to update portlet list [{}] for user [{}]", toUpdate.getId(), requester.getUserName()); + log.debug( + "Using DAO to update portlet list [{}] for user [{}]", + toUpdate.getId(), + requester.getUserName()); return portletListDao.updatePortletList(toUpdate, requester); } - } From 301f1b52ac97d09554c9ed498377ab661f24c8dd Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Wed, 17 Aug 2022 16:19:42 -0500 Subject: [PATCH 06/10] remove some debug statements --- .../portal/rest/portletlist/PortletListRESTController.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java index bce68888d31..de8df74ba34 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java @@ -117,7 +117,6 @@ public class PortletListRESTController { final IPerson person = personManager.getPerson(request); if (log.isDebugEnabled()) { debugPerson("createPortletList", person); - log.debug("createPortletList > JSON body is = {}", json); } if (person.isGuest()) { @@ -228,7 +227,6 @@ public class PortletListRESTController { final IPerson person = personManager.getPerson(request); if (log.isDebugEnabled()) { debugPerson("updatePortletList", person); - log.debug("updatePortletList > JSON body is = {}", json); } if (person.isGuest()) { From b7b96281bd069fae859cbca3015414b2daeff677 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Fri, 19 Aug 2022 10:38:17 -0500 Subject: [PATCH 07/10] Clarified comment --- .../java/org/apereo/portal/dao/portletlist/IPortletList.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java index afc3c3cdb2d..0976f3a340d 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java @@ -19,7 +19,8 @@ public interface IPortletList { List getItems(); - // Don't use this setter - instead use clearAndSetItems(...) to let hibernate handle its own + // Don't use this setter directly - instead use clearAndSetItems(...). Hibernate uses this + // method to handle its own // List management void setItems(List items); From e861ce3f5472b497256bc3d426a7be98e416bc63 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Tue, 23 Aug 2022 11:15:15 -0500 Subject: [PATCH 08/10] minor fixes --- .../portal/dao/portletlist/IPortletList.java | 3 +-- .../portal/dao/portletlist/jpa/PortletList.java | 4 ++++ .../dao/portletlist/jpa/PortletListItem.java | 4 +++- .../portal/rest/utils/InputValidator.java | 17 +++++++++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java index 0976f3a340d..01b92bf0680 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java @@ -20,8 +20,7 @@ public interface IPortletList { List getItems(); // Don't use this setter directly - instead use clearAndSetItems(...). Hibernate uses this - // method to handle its own - // List management + // method to handle its own List management void setItems(List items); /** Supports exporting. */ diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java index 9b267d4878e..dd8aec04f42 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java @@ -109,6 +109,7 @@ public void clearAndSetItems(List items) { for (PortletListItem item : items) { PortletListItem existingItem = existingItems.get(item.getEntityId()); + if (existingItem != null) { // If any item specific attributes are configured, specifically copy them over here. // Order will be set in prepareForPersistence() @@ -140,6 +141,9 @@ public void prepareForPersistence(IPerson requester) { int order = 0; for (PortletListItem item : this.items) { InputValidator.validateAsWordCharacters(item.getEntityId(), "items > entityId"); + + InputValidator.validateUniqueEntityId(this.items, item); + item.setPortletList(this); item.setListOrder(order++); } diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java index 2aa8f66b01d..20fededb18d 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java @@ -21,8 +21,10 @@ // This is ONLY to be used as part of a portlet list, so not specifying a PK name = "UP_PORTLET_LIST_ITEM", uniqueConstraints = { - // These are sets of lists + // Only allow sets of lists @UniqueConstraint(columnNames = {"LIST_ID", "LIST_ORDER", "ENTITY_ID"}), + // Only allow sets of portlets in the list + @UniqueConstraint(columnNames = {"LIST_ID", "ENTITY_ID"}), }) @Cacheable @org.hibernate.annotations.Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java index 39e0b511e46..ce36538f6be 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/utils/InputValidator.java @@ -1,6 +1,9 @@ package org.apereo.portal.rest.utils; +import java.util.List; import java.util.regex.Pattern; +import java.util.stream.Collectors; +import org.apereo.portal.dao.portletlist.jpa.PortletListItem; public class InputValidator { private static final String ALPHANUMERIC_AND_DASH_VALIDATOR_REGEX = @@ -22,4 +25,18 @@ public static String validateAsWordCharacters(String s, String inputType) { } return s; } + + /** + * @param items list of portlet list items + * @param item a given portlet list item (assumed to be in the list at least once) + */ + public static void validateUniqueEntityId(List items, PortletListItem item) { + if (items.stream() + .filter(o -> item.getEntityId().equals(o.getEntityId())) + .collect(Collectors.toList()) + .size() + > 1) { + throw new IllegalArgumentException("entity IDs must be unique in the items list"); + } + } } From adf18e4af84399a69031e525ab968b86a3eb4e4e Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Tue, 23 Aug 2022 12:02:48 -0500 Subject: [PATCH 09/10] clean up annotations and comments --- .../portal/dao/portletlist/IPortletList.java | 4 ---- .../dao/portletlist/jpa/PortletList.java | 16 +------------ .../dao/portletlist/jpa/PortletListItem.java | 2 +- .../PortletListRESTController.java | 23 ++++++++++++++----- .../portletlist/PortletListService.java | 20 ---------------- 5 files changed, 19 insertions(+), 46 deletions(-) diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java index 01b92bf0680..3741d34f0fb 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/IPortletList.java @@ -3,7 +3,6 @@ import java.util.List; import org.apereo.portal.dao.portletlist.jpa.PortletListItem; import org.apereo.portal.security.IPerson; -import org.dom4j.Element; public interface IPortletList { @@ -23,9 +22,6 @@ public interface IPortletList { // method to handle its own List management void setItems(List items); - /** Supports exporting. */ - void toElement(Element parent); - String toString(); void clearAndSetItems(List items); diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java index dd8aec04f42..dac9aea7e16 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletList.java @@ -20,13 +20,12 @@ import org.apereo.portal.dao.portletlist.IPortletList; import org.apereo.portal.rest.utils.InputValidator; import org.apereo.portal.security.IPerson; -import org.dom4j.Element; import org.hibernate.annotations.*; import org.springframework.util.StringUtils; @Getter @Setter -@EqualsAndHashCode(onlyExplicitlyIncluded = true) +@EqualsAndHashCode @Slf4j @Entity @Table( @@ -81,19 +80,6 @@ public class PortletList implements IPortletList { @OrderBy("LIST_ORDER ASC") private List items; - @Override - public void toElement(Element parent) { - if (parent == null) { - String msg = "Argument 'parent' cannot be null."; - throw new IllegalArgumentException(msg); - } - - parent.addElement("id").addText(this.getId().toString()); - parent.addElement("name").addText(this.getName()); - parent.addElement("ownerUsername").addText(this.getOwnerUsername()); - parent.addElement("items").addText("" + this.getItems().size()); - } - public void clearAndSetItems(List items) { if (this.items == null) { this.items = new ArrayList<>(); diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java index 20fededb18d..93a1c299c12 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/dao/portletlist/jpa/PortletListItem.java @@ -13,7 +13,7 @@ @Getter @Setter -@EqualsAndHashCode(onlyExplicitlyIncluded = true) +@EqualsAndHashCode @ToString @Slf4j @Entity diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java index de8df74ba34..4159455946e 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java @@ -34,7 +34,7 @@ public class PortletListRESTController { @Autowired private ObjectMapper objectMapper; /** - * Provide a JSON view of all portlet lists. + * Provide a JSON view of all portlet lists * *

If an administrator makes this call, ALL portlet lists will be returned. Otherwise, only * the portlet lists that the requester owns will be returned. @@ -61,7 +61,7 @@ public class PortletListRESTController { } /** - * Provide a JSON view of a given portlet list. + * Provide a JSON view of a given portlet list * *

If an administrator makes the request, the portlet list will be returned, regardless of * ownership. If anyone else makes the request, the portlet list will only be returned if the @@ -110,6 +110,12 @@ public class PortletListRESTController { return prepareResponse(response, pList, null, HttpServletResponse.SC_OK); } + /** + * Create a portlet list + * + *

If an administrator makes the request: - The owner can be specified. Owner will default to + * the current logged in user. - The name can be 'favorites' + */ @RequestMapping(value = CONTEXT, method = POST, produces = MediaType.APPLICATION_JSON_VALUE) public @ResponseBody String createPortletList( HttpServletRequest request, HttpServletResponse response, @RequestBody String json) { @@ -214,6 +220,12 @@ public class PortletListRESTController { } } + /** + * Update a portlet list. + * + *

If an administrator makes the request: - the owner can be specified - the name can be + * updated to 'favorites' - any list in the system can be updated + */ @RequestMapping( value = CONTEXT + "{portletListUuid}", method = PUT, @@ -364,11 +376,10 @@ public class PortletListRESTController { } /** - * Provide a JSON view of a given portlet list. + * Remove a portlet list * - *

If an administrator makes the request, the portlet list will be returned, regardless of - * ownership. If anyone else makes the request, the portlet list will only be returned if the - * requester is the owner. + *

If an administrator makes the request, they can remove any portlet list. Otherwise, only + * an owner can remove their portlet list. */ @RequestMapping( value = CONTEXT + "{portletListUuid}", diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java index deb77bce65d..898395e516f 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/services/portletlist/PortletListService.java @@ -32,11 +32,6 @@ public boolean isPortletListAdmin(IPerson person) { IPermission.ALL_TARGET); } - /** - * All the definitions, filtered by the user's access rights. - * - * @return - */ @Override public List getPortletLists() { List rslt = portletListDao.getPortletLists(); @@ -44,12 +39,6 @@ public List getPortletLists() { return rslt; } - /** - * All the definitions, filtered by the user's access rights. - * - * @param requester - * @return - */ @Override public List getPortletLists(IPerson requester) { List rslt = portletListDao.getPortletLists(requester.getUserName()); @@ -57,12 +46,6 @@ public List getPortletLists(IPerson requester) { return rslt; } - /** - * Retrieve a specific portlet list - * - * @param portletListUuid - * @return - */ @Override public IPortletList getPortletList(String portletListUuid) { return portletListDao.getPortletList(portletListUuid); @@ -77,9 +60,6 @@ public boolean removePortletList(IPerson requester, String portletListUuid) { } } - /** - * TODO docs Verifies permissions and that the group doesn't already exist (case insensitive) - */ @Override public IPortletList createPortletList(IPerson requester, IPortletList toCreate) { log.debug( From 242962abd912ebc4dc1834f3543dfbea55d74b44 Mon Sep 17 00:00:00 2001 From: Chris Beach Date: Wed, 24 Aug 2022 10:12:34 -0500 Subject: [PATCH 10/10] PR revisions --- .../PortletListRESTController.java | 58 +++++++++++++++++-- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java index 4159455946e..ff6fb64499c 100644 --- a/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java +++ b/uPortal-api/uPortal-api-rest/src/main/java/org/apereo/portal/rest/portletlist/PortletListRESTController.java @@ -10,6 +10,7 @@ import org.apereo.portal.dao.portletlist.IPortletList; import org.apereo.portal.dao.portletlist.jpa.PortletList; import org.apereo.portal.rest.utils.ErrorResponse; +import org.apereo.portal.rest.utils.InputValidator; import org.apereo.portal.security.IPerson; import org.apereo.portal.security.IPersonManager; import org.apereo.portal.security.RuntimeAuthorizationException; @@ -86,6 +87,16 @@ public class PortletListRESTController { response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } + // Input validation prior to logging any values to protect against logging security attacks + try { + if (!StringUtils.isEmpty(portletListUuid)) + InputValidator.validateAsWordCharacters(portletListUuid, "Portlet List UUID"); + } catch (IllegalArgumentException iae) { + log.warn("IllegalArgumentException thrown - {}", iae.getMessage(), iae); + return prepareResponse( + response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + } + IPortletList pList = portletListService.getPortletList(portletListUuid); if (pList == null) { @@ -137,6 +148,20 @@ public class PortletListRESTController { try { input = objectMapper.readValue(json, PortletList.class); + // Input validation prior to logging any values to protect against logging security + // attacks + try { + if (!StringUtils.isEmpty(input.getOwnerUsername())) + InputValidator.validateAsWordCharacters( + input.getOwnerUsername(), "ownerUsername"); + if (!StringUtils.isEmpty(input.getName())) + InputValidator.validateAsWordCharacters(input.getName(), "name"); + } catch (IllegalArgumentException iae) { + log.warn("IllegalArgumentException thrown - {}", iae.getMessage(), iae); + return prepareResponse( + response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + } + if (portletListService.isPortletListAdmin(person)) { if (StringUtils.isEmpty(input.getOwnerUsername())) { // Default - admins don't have to specify a user name @@ -178,9 +203,9 @@ public class PortletListRESTController { } catch (Exception e) { log.warn( - "User [{}] tried to create a portlet-list with bad json - {}", + "User [{}] tried to create a portlet-list with bad json.", person.getUserName(), - e.getMessage()); + e); return prepareResponse( response, null, @@ -190,6 +215,7 @@ public class PortletListRESTController { try { final IPortletList created = portletListService.createPortletList(person, input); + // Safe soft redirect since the id is system generated response.setHeader("Location", created.getId()); return prepareResponse(response, null, null, HttpServletResponse.SC_CREATED); } catch (RuntimeAuthorizationException rae) { @@ -252,9 +278,9 @@ public class PortletListRESTController { input = objectMapper.readValue(json, PortletList.class); } catch (Exception e) { log.warn( - "User [{}] tried to update a portlet-list with bad json - {}", + "User [{}] tried to update a portlet-list with bad json", person.getUserName(), - e.getMessage()); + e); return prepareResponse( response, null, @@ -262,6 +288,20 @@ public class PortletListRESTController { HttpServletResponse.SC_BAD_REQUEST); } + // Input validation prior to logging any values to protect against logging security attacks + try { + if (!StringUtils.isEmpty(portletListUuid)) + InputValidator.validateAsWordCharacters(portletListUuid, "Portlet List UUID"); + if (!StringUtils.isEmpty(input.getOwnerUsername())) + InputValidator.validateAsWordCharacters(input.getOwnerUsername(), "ownerUsername"); + if (!StringUtils.isEmpty(input.getName())) + InputValidator.validateAsWordCharacters(input.getName(), "name"); + } catch (IllegalArgumentException iae) { + log.warn("IllegalArgumentException thrown - {}", iae.getMessage(), iae); + return prepareResponse( + response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + } + // Overlay changes onto a known entity, and then persist that entity. IPortletList toUpdate = portletListService.getPortletList(portletListUuid); @@ -400,6 +440,16 @@ public class PortletListRESTController { response, null, "Not authorized", HttpServletResponse.SC_UNAUTHORIZED); } + // Input validation prior to logging any values to protect against logging security attacks + try { + if (!StringUtils.isEmpty(portletListUuid)) + InputValidator.validateAsWordCharacters(portletListUuid, "Portlet List UUID"); + } catch (IllegalArgumentException iae) { + log.warn("IllegalArgumentException thrown - {}", iae.getMessage(), iae); + return prepareResponse( + response, null, iae.getMessage(), HttpServletResponse.SC_CONFLICT); + } + try { if (portletListService.removePortletList(person, portletListUuid)) { return prepareResponse(response, null, null, HttpServletResponse.SC_OK);