Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implement one-time-salt use and add comprehensive tests #142

Merged
merged 5 commits into from
Oct 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,9 @@ public Boolean getModerateComments() {
}

public String getAnalyticsCode() {
return this.pojo.getAnalyticsCode();
return HTMLSanitizer.conditionallySanitize(this.pojo.getAnalyticsCode());
}


public Boolean getEmailComments() {
return this.pojo.getEmailComments();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
throws IOException, ServletException {

HttpServletRequest httpReq = (HttpServletRequest) request;
RollerSession rses = RollerSession.getRollerSession(httpReq);
String userId = rses != null && rses.getAuthenticatedUser() != null ? rses.getAuthenticatedUser().getId() : "";

SaltCache saltCache = SaltCache.getInstance();
String salt = RandomStringUtils.random(20, 0, 0, true, true, null, new SecureRandom());
saltCache.put(salt, userId);
httpReq.setAttribute("salt", salt);
RollerSession rollerSession = RollerSession.getRollerSession(httpReq);
if (rollerSession != null) {
String userId = rollerSession.getAuthenticatedUser() != null ? rollerSession.getAuthenticatedUser().getId() : "";
SaltCache saltCache = SaltCache.getInstance();
String salt = RandomStringUtils.random(20, 0, 0, true, true, null, new SecureRandom());
saltCache.put(salt, userId);
httpReq.setAttribute("salt", salt);
}

chain.doFilter(request, response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,31 @@ public void doFilter(ServletRequest request, ServletResponse response,
FilterChain chain) throws IOException, ServletException {
HttpServletRequest httpReq = (HttpServletRequest) request;

if ("POST".equals(httpReq.getMethod()) && !isIgnoredURL(httpReq.getServletPath())) {
RollerSession rses = RollerSession.getRollerSession(httpReq);
String userId = rses != null && rses.getAuthenticatedUser() != null ? rses.getAuthenticatedUser().getId() : "";
String requestURL = httpReq.getRequestURL().toString();
String queryString = httpReq.getQueryString();
if (queryString != null) {
requestURL += "?" + queryString;
}

if ("POST".equals(httpReq.getMethod()) && !isIgnoredURL(requestURL)) {
RollerSession rollerSession = RollerSession.getRollerSession(httpReq);
if (rollerSession != null) {
String userId = rollerSession.getAuthenticatedUser() != null ? rollerSession.getAuthenticatedUser().getId() : "";

String salt = httpReq.getParameter("salt");
SaltCache saltCache = SaltCache.getInstance();
if (salt == null || !Objects.equals(saltCache.get(salt), userId)) {
if (log.isDebugEnabled()) {
log.debug("Valid salt value not found on POST to URL : " + httpReq.getServletPath());
}
throw new ServletException("Security Violation");
}

String salt = httpReq.getParameter("salt");
SaltCache saltCache = SaltCache.getInstance();
if (salt == null || !Objects.equals(saltCache.get(salt), userId)) {
// Remove salt from cache after successful validation
saltCache.remove(salt);
if (log.isDebugEnabled()) {
log.debug("Valid salt value not found on POST to URL : " + httpReq.getServletPath());
log.debug("Salt used and invalidated: " + salt);
}
throw new ServletException("Security Violation");
}
}

Expand All @@ -70,8 +84,6 @@ public void doFilter(ServletRequest request, ServletResponse response,

@Override
public void init(FilterConfig filterConfig) throws ServletException {

// Construct our list of ignored urls
String urls = WebloggerConfig.getProperty("salt.ignored.urls");
ignored = Set.of(StringUtils.stripAll(StringUtils.split(urls, ",")));
}
Expand All @@ -82,16 +94,10 @@ public void destroy() {

/**
* Checks if this is an ignored url defined in the salt.ignored.urls property
* @param theUrl the the url
* @param theUrl the url
* @return true, if is ignored resource
*/
private boolean isIgnoredURL(String theUrl) {
int i = theUrl.lastIndexOf('/');

// If it's not a resource then don't ignore it
if (i <= 0 || i == theUrl.length() - 1) {
return false;
}
return ignored.contains(theUrl.substring(i + 1));
return ignored.contains(theUrl);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.roller.weblogger.business.WebloggerFactory;
import org.apache.roller.weblogger.business.WeblogEntryManager;
import org.apache.roller.weblogger.business.plugins.entry.WeblogEntryPlugin;
import org.apache.roller.weblogger.config.WebloggerConfig;
import org.apache.roller.weblogger.config.WebloggerRuntimeConfig;
import org.apache.roller.weblogger.pojos.Weblog;
import org.apache.roller.weblogger.pojos.WeblogCategory;
Expand Down Expand Up @@ -61,6 +62,8 @@ public class WeblogConfig extends UIAction {

// list of available plugins
private List<WeblogEntryPlugin> pluginsList = Collections.emptyList();

private final boolean weblogAdminsUntrusted = WebloggerConfig.getBooleanProperty("weblogAdminsUntrusted");


public WeblogConfig() {
Expand All @@ -71,7 +74,7 @@ public WeblogConfig() {

@Override
public void myPrepare() {

try {
WeblogEntryManager wmgr = WebloggerFactory.getWeblogger().getWeblogEntryManager();

Expand All @@ -88,10 +91,7 @@ public void myPrepare() {
// set plugins list
PluginManager ppmgr = WebloggerFactory.getWeblogger().getPluginManager();
Map<String, WeblogEntryPlugin> pluginsMap = ppmgr.getWeblogEntryPlugins(getActionWeblog());
List<WeblogEntryPlugin> plugins = new ArrayList<>();
for (WeblogEntryPlugin entryPlugin : pluginsMap.values()) {
plugins.add(entryPlugin);
}
List<WeblogEntryPlugin> plugins = new ArrayList<>(pluginsMap.values());

// sort
setPluginsList(plugins);
Expand Down Expand Up @@ -231,5 +231,8 @@ public List<WeblogEntryPlugin> getPluginsList() {
public void setPluginsList(List<WeblogEntryPlugin> pluginsList) {
this.pluginsList = pluginsList;
}


public boolean getWeblogAdminsUntrusted() {
return weblogAdminsUntrusted;
}
}
1 change: 1 addition & 0 deletions app/src/main/resources/ApplicationResources.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1759,6 +1759,7 @@ websiteSettings.formatting=Formatting
websiteSettings.spamPrevention=Spam Prevention
websiteSettings.ignoreUrls=List of words and regex expressions listed one per \
line to be added to the banned words list used to check comments, trackbacks and referrers.
websiteSettings.bannedWordsList=Words banned in comments (regex allowed)
websiteSettings.acceptedBannedwordslist=Accepted {0} string and {1} regex banned-words list rules
websiteSettings.error.processingBannedwordslist=Error processing banned-words list: {0}

Expand Down
4 changes: 2 additions & 2 deletions app/src/main/webapp/WEB-INF/jsps/editor/WeblogConfig.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,11 @@
<h3><s:text name="websiteSettings.spamPrevention"/></h3>

<s:textarea name="bean.bannedwordslist" rows="7" cols="40"
label="%{getText('websiteSettings.analyticsTrackingCode')}"/>
label="%{getText('websiteSettings.bannedWordsList')}"/>

<%-- ***** Web analytics settings ***** --%>

<s:if test="getBooleanProp('analytics.code.override.allowed')">
<s:if test="getBooleanProp('analytics.code.override.allowed') && !weblogAdminsUntrusted">
<h3><s:text name="configForm.webAnalytics"/></h3>

<s:textarea name="bean.analyticsCode" rows="10" cols="70"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package org.apache.roller.weblogger.ui.core.filters;

import org.apache.roller.weblogger.pojos.User;
import org.apache.roller.weblogger.ui.core.RollerSession;
import org.apache.roller.weblogger.ui.rendering.util.cache.SaltCache;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.MockitoAnnotations;

import javax.servlet.FilterChain;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import static org.mockito.Mockito.*;

public class LoadSaltFilterTest {

private LoadSaltFilter filter;

@Mock
private HttpServletRequest request;

@Mock
private HttpServletResponse response;

@Mock
private FilterChain chain;

@Mock
private RollerSession rollerSession;

@Mock
private SaltCache saltCache;

@BeforeEach
public void setUp() {
MockitoAnnotations.initMocks(this);
filter = new LoadSaltFilter();
}

@Test
public void testDoFilterGeneratesSalt() throws Exception {
try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class)) {

mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(rollerSession);
mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);

when(rollerSession.getAuthenticatedUser()).thenReturn(new TestUser("userId"));

filter.doFilter(request, response, chain);

verify(request).setAttribute(eq("salt"), anyString());
verify(saltCache).put(anyString(), eq("userId"));
verify(chain).doFilter(request, response);
}
}

@Test
public void testDoFilterWithNullRollerSession() throws Exception {
try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class)) {

mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(null);
mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);

filter.doFilter(request, response, chain);

verify(request, never()).setAttribute(eq("salt"), anyString());
verify(saltCache, never()).put(anyString(), anyString());
verify(chain).doFilter(request, response);
}
}

private static class TestUser extends User {
private final String id;

TestUser(String id) {
this.id = id;
}

public String getId() {
return id;
}
}
}
Loading
Loading