Skip to content

Commit 960f69c

Browse files
Naenynbjagg
Naenyn
and
bjagg
authored
Updated permissions handling. Additional logging. (#397)
* Updated permissions handling. Additional logging. Also includes a smidge of logging / TODO notes around hibernate / cache weirdness experienced while working on this commit. * Fixed imports. My IDE's settings for switching direct imports to '*' was set too low, so I undid the '*' import but forgot to fix the missing direct imports. * chore: change pom parent (#377) Co-authored-by: Bill Smith <wsmith@unicon.net> * Updated permissions handling. Additional logging. Also includes a smidge of logging / TODO notes around hibernate / cache weirdness experienced while working on this commit. * Fixed imports. My IDE's settings for switching direct imports to '*' was set too low, so I undid the '*' import but forgot to fix the missing direct imports. --------- Co-authored-by: Benito Gonzalez <bgonzalez@unicon.net>
1 parent 4bcda5a commit 960f69c

File tree

5 files changed

+101
-10
lines changed

5 files changed

+101
-10
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,11 @@ public NewsSet getNewsSet(String userId, String setName) {
356356
NewsSet set = (NewsSet) q.uniqueResult();
357357
if (logger.isDebugEnabled()) {
358358
logger.debug(this.getSessionFactory().getStatistics().toString());
359+
if (set != null) {
360+
logger.debug("found " + set.getNewsConfigurations().size() + " news configurations");
361+
} else {
362+
logger.debug("no news configurations found");
363+
}
359364
}
360365
return set;
361366

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,16 @@
2020

2121
import java.util.HashMap;
2222
import java.util.Map;
23+
import java.util.Set;
2324

25+
import javax.portlet.ActionRequest;
26+
import javax.portlet.PortletRequest;
27+
import javax.portlet.PortletSecurityException;
2428
import javax.portlet.RenderRequest;
25-
import javax.portlet.RenderResponse;
29+
import javax.servlet.http.HttpServletRequest;
2630

31+
import org.jasig.portlet.newsreader.mvc.AbstractNewsController;
32+
import org.jasig.portlet.newsreader.service.RolesService;
2733
import org.slf4j.Logger;
2834
import org.slf4j.LoggerFactory;
2935
import org.jasig.portlet.newsreader.PredefinedNewsDefinition;
@@ -35,6 +41,8 @@
3541
import org.springframework.web.portlet.ModelAndView;
3642
import org.springframework.web.portlet.bind.annotation.ActionMapping;
3743
import org.springframework.web.portlet.bind.annotation.RenderMapping;
44+
import org.springframework.web.portlet.context.PortletApplicationContextUtils;
45+
import org.springframework.web.portlet.util.PortletUtils;
3846

3947

4048
/**
@@ -54,7 +62,13 @@ public class AdminNewsController {
5462
private NewsStore newsStore;
5563

5664
@RenderMapping(params="action=administration")
57-
public ModelAndView getAdminView(RenderRequest request,RenderResponse response) {
65+
public ModelAndView getAdminView(RenderRequest request) throws PortletSecurityException {
66+
if (!request.isUserInRole(AbstractNewsController.NEWS_ADMIN_ROLE)) {
67+
log.warn("User [ {} ] with IP [ {} ] tried to access news administration!",
68+
request.getRemoteUser(),
69+
request.getProperty("REMOTE_ADDR"));
70+
throw new PortletSecurityException("User does not have required admin role");
71+
}
5872

5973
log.debug("Entering news admin");
6074

@@ -67,7 +81,14 @@ public ModelAndView getAdminView(RenderRequest request,RenderResponse response)
6781
}
6882

6983
@ActionMapping(params="action=deletePredefinedFeed")
70-
public void deleteFeed(@RequestParam("id") Long id) {
84+
public void deleteFeed(@RequestParam("id") Long id, ActionRequest request) throws PortletSecurityException {
85+
if (!request.isUserInRole(AbstractNewsController.NEWS_ADMIN_ROLE)) {
86+
log.warn("User [ {} ] with IP [ {} ] tried to access news administration!",
87+
request.getRemoteUser(),
88+
request.getProperty("REMOTE_ADDR"));
89+
throw new PortletSecurityException("User does not have required admin role");
90+
}
91+
7192
PredefinedNewsDefinition def = newsStore.getPredefinedNewsDefinition(id);
7293
newsStore.deleteNewsDefinition(def);
7394
}

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

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,17 @@
2121
import javax.portlet.ActionRequest;
2222
import javax.portlet.ActionResponse;
2323
import javax.portlet.PortletRequest;
24+
import javax.portlet.PortletURL;
25+
import javax.portlet.RenderResponse;
2426

27+
import org.apache.commons.lang.StringUtils;
2528
import org.jasig.portlet.newsreader.NewsConfiguration;
29+
import org.jasig.portlet.newsreader.PredefinedNewsDefinition;
2630
import org.jasig.portlet.newsreader.UserDefinedNewsConfiguration;
2731
import org.jasig.portlet.newsreader.UserDefinedNewsDefinition;
2832
import org.jasig.portlet.newsreader.adapter.RomeAdapter;
2933
import org.jasig.portlet.newsreader.dao.NewsStore;
34+
import org.jasig.portlet.newsreader.mvc.AbstractNewsController;
3035
import org.jasig.portlet.newsreader.mvc.NewsListingCommand;
3136
import org.jasig.portlet.newsreader.service.NewsSetResolvingService;
3237
import org.slf4j.Logger;
@@ -96,7 +101,32 @@ public NewsListingCommand getNewsForm(PortletRequest request) throws Exception {
96101
}
97102

98103
@RenderMapping(params = "action=editUrl")
99-
public String getUserEditView(PortletRequest request) {
104+
public String getUserEditView(PortletRequest request, RenderResponse response) {
105+
log.debug("Returning editNewsUrl view");
106+
107+
// get the to-be-edited news configuration id
108+
String[] formIdValues = request.getParameterMap().get("id");
109+
String formId = null;
110+
if (formIdValues != null && formIdValues.length > 0) {
111+
formId = formIdValues[0];
112+
}
113+
114+
// if user doesn't have permissions, redirect
115+
if (StringUtils.isNotBlank(formId)) {
116+
long lFormId = Long.parseLong(formId);
117+
if (lFormId > -1) {
118+
if (!canEditNewsConfiguration(request, lFormId)) {
119+
log.warn("User [ {} ] with IP [ {} ] tried to edit news configuration [ {} ] without permission!",
120+
request.getRemoteUser(),
121+
request.getProperty("REMOTE_ADDR"),
122+
lFormId);
123+
PortletURL redirectUrl = response.createRenderURL();
124+
redirectUrl.setParameter("action", "editPreferences");
125+
request.setAttribute("redirectUrl", redirectUrl.toString());
126+
}
127+
}
128+
}
129+
100130
return "editNewsUrl";
101131
}
102132

@@ -110,11 +140,19 @@ public void onSubmitAction(ActionRequest request, ActionResponse response,
110140

111141
if (form.getId() > -1) {
112142

113-
config = (UserDefinedNewsConfiguration) newsStore.getNewsConfiguration(form.getId());
114-
definition = (UserDefinedNewsDefinition) config.getNewsDefinition();
115-
definition.addParameter("url", form.getUrl());
116-
definition.setName(form.getName());
117-
log.debug("Updating");
143+
if (canEditNewsConfiguration(request, form.getId())) {
144+
config = (UserDefinedNewsConfiguration) newsStore.getNewsConfiguration(form.getId());
145+
log.debug("User [ {} ] is updating news", request.getRemoteUser());
146+
definition = (UserDefinedNewsDefinition) config.getNewsDefinition();
147+
definition.addParameter("url", form.getUrl());
148+
definition.setName(form.getName());
149+
} else {
150+
log.warn("User [ {} ] with IP [ {} ] tried to edit news configuration [ {} ] without permission!",
151+
request.getRemoteUser(),
152+
request.getProperty("REMOTE_ADDR"),
153+
form.getId());
154+
return;
155+
}
118156

119157
} else {
120158

@@ -143,4 +181,19 @@ public void onSubmitAction(ActionRequest request, ActionResponse response,
143181

144182
}
145183

184+
private boolean isPredefinedNewsConfiguration(NewsConfiguration newsConfiguration) {
185+
return newsConfiguration.getNewsDefinition() instanceof PredefinedNewsDefinition;
186+
}
187+
188+
private boolean canEditNewsConfiguration(PortletRequest request, long configurationId) {
189+
boolean isAdmin = request.isUserInRole(AbstractNewsController.NEWS_ADMIN_ROLE);
190+
NewsConfiguration configuration = newsStore.getNewsConfiguration(configurationId);
191+
if (isPredefinedNewsConfiguration(configuration)) {
192+
return isAdmin;
193+
} else {
194+
UserDefinedNewsConfiguration userConfiguration = (UserDefinedNewsConfiguration) configuration;
195+
return isAdmin || userConfiguration.getNewsSet().getUserId().equals(request.getRemoteUser());
196+
}
197+
}
198+
146199
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,19 @@ public NewsSet getNewsSet(String fname, PortletRequest request) {
7777
set.setUserId(userId);
7878
set.setName(fname);
7979
newsStore.storeNewsSet(set);
80+
//TODO: the persisted set (line above) isn't always available to the line below. Hibernate being lazy?
8081
set = newsStore.getNewsSet(userId, fname); // get set_id
8182
}
8283

8384
// Persistent set is now loaded but may still need re-initalising since last use.
8485
// by adding setId to session, we signal that initialisation has taken place.
8586
if (session.getAttribute("setId", PortletSession.PORTLET_SCOPE) == null) {
86-
logger.debug("re-initalising loaded newsSet "+set.getName());
87+
if (set != null) {
88+
logger.debug("re-initalising loaded newsSet " + set.getName());
89+
} else {
90+
logger.debug("attempting to re-initialize loaded newsSet, but it is null");
91+
}
92+
8793
@SuppressWarnings("unchecked")
8894
final Set<String> roles = rolesService.getUserRoles(request);
8995

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@
2525
<portlet:param name="action" value="editUrl"/>
2626
</portlet:actionURL>
2727

28+
<c:if test="${not empty redirectUrl}">
29+
<script type="text/javascript">
30+
window.location.href = '${redirectUrl}';
31+
</script>
32+
</c:if>
33+
2834
<div class="container-fluid newsreader-container">
2935
<div class="row newsreader-portlet-toolbar">
3036
<div class="col-md-8 no-col-padding">

0 commit comments

Comments
 (0)