Updated permissions handling. Additional logging.

pom.xml

@@ -22,9 +22,9 @@
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
 
     <parent>
-        <groupId>org.jasig.parent</groupId>
-        <artifactId>jasig-parent</artifactId>
-        <version>41</version>
+        <groupId>org.jasig.portlet</groupId>
+        <artifactId>uportal-portlet-parent</artifactId>
+        <version>42</version>
     </parent>
 
     <modelVersion>4.0.0</modelVersion>
@@ -182,7 +182,7 @@
             <dependency>
                 <groupId>org.apache.tomcat</groupId>
                 <artifactId>tomcat-jdbc</artifactId>
-                <version>${tomcat-jdbc.version}</version>
+                <version>${tomcat.version}</version>
             </dependency>
             <dependency>
                 <groupId>org.apache.httpcomponents</groupId>
@@ -669,23 +669,6 @@
 
     <build>
         <plugins>
-             <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-release-plugin</artifactId>
-                <version>3.1.1</version>
-                <dependencies>
-                    <dependency>
-                        <groupId>org.apache.maven.scm</groupId>
-                        <artifactId>maven-scm-provider-gitexe</artifactId>
-                        <version>2.0.1</version>
-                    </dependency>
-                </dependencies>
-                <configuration>
-                    <autoVersionSubmodules>true</autoVersionSubmodules>
-                    <localCheckout>true</localCheckout>
-                    <tagNameFormat>NewsReaderPortlet-@{project.version}</tagNameFormat>
-                </configuration>
-              </plugin>
               <plugin>
                 <groupId>org.jasig.resourceserver</groupId>
                 <artifactId>resource-server-plugin</artifactId>
@@ -722,44 +705,18 @@
                 <configuration>
                     <aggregate>true</aggregate>
                     <excludes>
-                        <exclude>.gitignore</exclude>
-                        <exclude>target/**</exclude>
-                        <exclude>bin/**</exclude>
-                        <exclude>pom.xml.*</exclude>
-                        <exclude>release.properties</exclude>
-                        <exclude>**/NOTICE</exclude>
-                        <exclude>NOTICE.template</exclude>
-                        <exclude>README</exclude>
-                        <exclude>LICENSE</exclude>
                         <exclude>src/main/webapp/rs/**</exclude>
                         <exclude>src/main/resources/antisamy/**</exclude>
                         <exclude>src/main/webapp/WEB-INF/dtd/**</exclude>
                         <exclude>src/main/webapp/WEB-INF/validator-rules.xml</exclude>
-                        <exclude>.idea/**</exclude> <!-- Exclude intelliJ files -->
-                        <exclude>overlays/**</exclude> <!-- Exclude intelliJ files -->
                     </excludes>
-                    <mapping>
-                        <channel>XML_STYLE</channel>
-                        <crn>XML_STYLE</crn>
-                        <less>SLASHSTAR_STYLE</less>
-                        <tag>DYNASCRIPT_STYLE</tag>
-                    </mapping>
-                </configuration>
-            </plugin>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-compiler-plugin</artifactId>
-                <configuration>
-                    <source>1.8</source>
-                    <target>1.8</target>
                 </configuration>
             </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-war-plugin</artifactId>
                 <configuration>
                     <attachClasses>true</attachClasses>
-                    <warName>NewsReaderPortlet</warName>
                     <overlays>
                         <overlay>
                             <groupId>org.jasig.resourceserver</groupId>
@@ -997,16 +954,4 @@
         </plugins>
     </build>
 
-    <reporting>
-        <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-checkstyle-plugin</artifactId>
-                <version>3.5.0</version>
-                <configuration>
-                    <configLocation>google_checks.xml</configLocation>
-                </configuration>
-            </plugin>
-        </plugins>
-    </reporting>
 </project>

src/main/java/org/jasig/portlet/newsreader/dao/HibernateNewsStore.java

@@ -356,6 +356,11 @@ public NewsSet getNewsSet(String userId, String setName) {
 	        NewsSet set = (NewsSet) q.uniqueResult();
             if (logger.isDebugEnabled()) {
                 logger.debug(this.getSessionFactory().getStatistics().toString());
+                if (set != null) {
+                    logger.debug("found " + set.getNewsConfigurations().size() + " news configurations");
+                } else {
+                    logger.debug("no news configurations found");
+                }
             }
             return set;
 

src/main/java/org/jasig/portlet/newsreader/mvc/portlet/reader/AdminNewsController.java

@@ -20,10 +20,16 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;
 
+import javax.portlet.ActionRequest;
+import javax.portlet.PortletRequest;
+import javax.portlet.PortletSecurityException;
 import javax.portlet.RenderRequest;
-import javax.portlet.RenderResponse;
+import javax.servlet.http.HttpServletRequest;
 
+import org.jasig.portlet.newsreader.mvc.AbstractNewsController;
+import org.jasig.portlet.newsreader.service.RolesService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.jasig.portlet.newsreader.PredefinedNewsDefinition;
@@ -35,6 +41,8 @@
 import org.springframework.web.portlet.ModelAndView;
 import org.springframework.web.portlet.bind.annotation.ActionMapping;
 import org.springframework.web.portlet.bind.annotation.RenderMapping;
+import org.springframework.web.portlet.context.PortletApplicationContextUtils;
+import org.springframework.web.portlet.util.PortletUtils;
 
 
 /**
@@ -54,7 +62,13 @@ public class AdminNewsController {
     private NewsStore newsStore;
 
     @RenderMapping(params="action=administration")
-    public ModelAndView getAdminView(RenderRequest request,RenderResponse response) {
+    public ModelAndView getAdminView(RenderRequest request) throws PortletSecurityException {
+        if (!request.isUserInRole(AbstractNewsController.NEWS_ADMIN_ROLE)) {
+            log.warn("User [ {} ] with IP [ {} ] tried to access news administration!",
+                    request.getRemoteUser(),
+                    request.getProperty("REMOTE_ADDR"));
+            throw new PortletSecurityException("User does not have required admin role");
+        }
 
         log.debug("Entering news admin");
 
@@ -67,7 +81,14 @@ public ModelAndView getAdminView(RenderRequest request,RenderResponse response)
     }
 
     @ActionMapping(params="action=deletePredefinedFeed")
-    public void deleteFeed(@RequestParam("id") Long id) {
+    public void deleteFeed(@RequestParam("id") Long id, ActionRequest request) throws PortletSecurityException {
+        if (!request.isUserInRole(AbstractNewsController.NEWS_ADMIN_ROLE)) {
+            log.warn("User [ {} ] with IP [ {} ] tried to access news administration!",
+                    request.getRemoteUser(),
+                    request.getProperty("REMOTE_ADDR"));
+            throw new PortletSecurityException("User does not have required admin role");
+        }
+
         PredefinedNewsDefinition def = newsStore.getPredefinedNewsDefinition(id);
         newsStore.deleteNewsDefinition(def);
     }

src/main/java/org/jasig/portlet/newsreader/mvc/portlet/reader/EditUserRomeController.java

@@ -21,12 +21,17 @@
 import javax.portlet.ActionRequest;
 import javax.portlet.ActionResponse;
 import javax.portlet.PortletRequest;
+import javax.portlet.PortletURL;
+import javax.portlet.RenderResponse;
 
+import org.apache.commons.lang.StringUtils;
 import org.jasig.portlet.newsreader.NewsConfiguration;
+import org.jasig.portlet.newsreader.PredefinedNewsDefinition;
 import org.jasig.portlet.newsreader.UserDefinedNewsConfiguration;
 import org.jasig.portlet.newsreader.UserDefinedNewsDefinition;
 import org.jasig.portlet.newsreader.adapter.RomeAdapter;
 import org.jasig.portlet.newsreader.dao.NewsStore;
+import org.jasig.portlet.newsreader.mvc.AbstractNewsController;
 import org.jasig.portlet.newsreader.mvc.NewsListingCommand;
 import org.jasig.portlet.newsreader.service.NewsSetResolvingService;
 import org.slf4j.Logger;
@@ -96,7 +101,32 @@ public NewsListingCommand getNewsForm(PortletRequest request) throws Exception {
     }
 
     @RenderMapping(params = "action=editUrl")
-    public String getUserEditView(PortletRequest request) {
+    public String getUserEditView(PortletRequest request, RenderResponse response) {
+        log.debug("Returning editNewsUrl view");
+
+        // get the to-be-edited news configuration id
+        String[] formIdValues = request.getParameterMap().get("id");
+        String formId = null;
+        if (formIdValues != null && formIdValues.length > 0) {
+            formId = formIdValues[0];
+        }
+
+        // if user doesn't have permissions, redirect
+        if (StringUtils.isNotBlank(formId)) {
+            long lFormId = Long.parseLong(formId);
+            if (lFormId > -1) {
+                if (!canEditNewsConfiguration(request, lFormId)) {
+                    log.warn("User [ {} ] with IP [ {} ] tried to edit news configuration [ {} ] without permission!",
+                            request.getRemoteUser(),
+                            request.getProperty("REMOTE_ADDR"),
+                            lFormId);
+                    PortletURL redirectUrl = response.createRenderURL();
+                    redirectUrl.setParameter("action", "editPreferences");
+                    request.setAttribute("redirectUrl", redirectUrl.toString());
+                }
+            }
+        }
+
         return "editNewsUrl";
     }
 
@@ -110,11 +140,19 @@ public void onSubmitAction(ActionRequest request, ActionResponse response,
 
         if (form.getId() > -1) {
 
-            config = (UserDefinedNewsConfiguration) newsStore.getNewsConfiguration(form.getId());
-            definition = (UserDefinedNewsDefinition) config.getNewsDefinition();
-            definition.addParameter("url", form.getUrl());
-            definition.setName(form.getName());
-            log.debug("Updating");
+            if (canEditNewsConfiguration(request, form.getId())) {
+                config = (UserDefinedNewsConfiguration) newsStore.getNewsConfiguration(form.getId());
+                log.debug("User [ {} ] is updating news", request.getRemoteUser());
+                definition = (UserDefinedNewsDefinition) config.getNewsDefinition();
+                definition.addParameter("url", form.getUrl());
+                definition.setName(form.getName());
+            } else {
+                log.warn("User [ {} ] with IP [ {} ] tried to edit news configuration [ {} ] without permission!",
+                        request.getRemoteUser(),
+                        request.getProperty("REMOTE_ADDR"),
+                        form.getId());
+                return;
+            }
 
         } else {
 
@@ -143,4 +181,19 @@ public void onSubmitAction(ActionRequest request, ActionResponse response,
 
     }
 
+    private boolean isPredefinedNewsConfiguration(NewsConfiguration newsConfiguration) {
+        return newsConfiguration.getNewsDefinition() instanceof PredefinedNewsDefinition;
+    }
+
+    private boolean canEditNewsConfiguration(PortletRequest request, long configurationId) {
+        boolean isAdmin = request.isUserInRole(AbstractNewsController.NEWS_ADMIN_ROLE);
+        NewsConfiguration configuration = newsStore.getNewsConfiguration(configurationId);
+        if (isPredefinedNewsConfiguration(configuration)) {
+            return isAdmin;
+        } else {
+            UserDefinedNewsConfiguration userConfiguration = (UserDefinedNewsConfiguration) configuration;
+            return isAdmin || userConfiguration.getNewsSet().getUserId().equals(request.getRemoteUser());
+        }
+    }
+
 }

src/main/java/org/jasig/portlet/newsreader/service/SharedNewsSetServiceImpl.java

@@ -77,13 +77,19 @@ public NewsSet getNewsSet(String fname, PortletRequest request) {
 		        set.setUserId(userId);
 		        set.setName(fname);
 		        newsStore.storeNewsSet(set);
+				//TODO: the persisted set (line above) isn't always available to the line below. Hibernate being lazy?
 				set = newsStore.getNewsSet(userId, fname); // get set_id
 			}
 
 			// Persistent set is now loaded but may still need re-initalising since last use.
 			// by adding setId to session, we signal that initialisation has taken place.
 			if (session.getAttribute("setId", PortletSession.PORTLET_SCOPE) == null) {
-				logger.debug("re-initalising loaded newsSet "+set.getName());
+				if (set != null) {
+					logger.debug("re-initalising loaded newsSet " + set.getName());
+				} else {
+					logger.debug("attempting to re-initialize loaded newsSet, but it is null");
+				}
+
 				@SuppressWarnings("unchecked")
 				final Set<String> roles = rolesService.getUserRoles(request);
 

src/main/webapp/WEB-INF/jsp/editNewsUrl.jsp

@@ -25,6 +25,12 @@
     <portlet:param name="action" value="editUrl"/>
 </portlet:actionURL>
 
+<c:if test="${not empty redirectUrl}">
+  <script type="text/javascript">
+    window.location.href = '${redirectUrl}';
+  </script>
+</c:if>
+
 <div class="container-fluid newsreader-container">
   <div class="row newsreader-portlet-toolbar">
     <div class="col-md-8 no-col-padding">