diff --git a/app/src/main/java/org/apache/roller/weblogger/business/FrontpageSettings.java b/app/src/main/java/org/apache/roller/weblogger/business/FrontpageSettings.java new file mode 100644 index 0000000000..18e5bdf2c6 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/business/FrontpageSettings.java @@ -0,0 +1,139 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF 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 + * + * 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. For additional + * information regarding copyright in this work, please see the NOTICE + * file in the top level directory of this distribution. + */ +package org.apache.roller.weblogger.business; + +import org.apache.commons.lang3.StringUtils; +import org.apache.roller.weblogger.WebloggerException; +import org.apache.roller.weblogger.pojos.RuntimeConfigProperty; +import org.apache.roller.weblogger.pojos.Weblog; +import org.apache.roller.weblogger.ui.rendering.util.cache.SiteWideCache; +import org.apache.roller.weblogger.ui.rendering.util.cache.WeblogFeedCache; +import org.apache.roller.weblogger.ui.rendering.util.cache.WeblogPageCache; + +/** + * Reads and writes the site frontpage weblog settings. + * + *

Two screens change these values: the one-time setup screen used to choose + * a frontpage while the site is being installed, and the global configuration + * screen used afterwards. Both go through here so that the handle is resolved + * and validated the same way, both properties move together, and the rendered + * page and feed caches are invalidated consistently. + */ +public final class FrontpageSettings { + + public static final String HANDLE_PROPERTY = "site.frontpage.weblog.handle"; + public static final String AGGREGATED_PROPERTY = "site.frontpage.weblog.aggregated"; + + private FrontpageSettings() { + } + + /** + * Resolves a submitted handle to a weblog that actually exists and is + * enabled. + * + * @return the weblog, or null when the handle is blank, unknown or refers + * to a disabled weblog + */ + public static Weblog resolveWeblog(String handle) throws WebloggerException { + if (StringUtils.isBlank(handle)) { + return null; + } + return WebloggerFactory.getWeblogger().getWeblogManager() + .getWeblogByHandle(handle.trim(), Boolean.TRUE); + } + + /** @return the configured frontpage handle, or null when none is set. */ + public static String getConfiguredHandle() throws WebloggerException { + RuntimeConfigProperty prop = WebloggerFactory.getWeblogger() + .getPropertiesManager().getProperty(HANDLE_PROPERTY); + if (prop == null || StringUtils.isBlank(prop.getValue())) { + return null; + } + return prop.getValue(); + } + + /** @return true when a frontpage weblog has already been chosen. */ + public static boolean isConfigured() throws WebloggerException { + return getConfiguredHandle() != null; + } + + /** + * Validates and stores the frontpage selection. + * + *

Both properties are written before the single flush so the pair cannot + * be left half-applied, and the canonical handle from the resolved weblog is + * stored rather than the submitted text. A missing aggregation value is + * treated as false, which is what an unchecked checkbox means. + * + * @param handle submitted weblog handle + * @param aggregated submitted aggregation flag; null means false + * @throws InvalidFrontpageWeblogException when the handle does not name an + * existing, enabled weblog + */ + public static void apply(String handle, Boolean aggregated) + throws WebloggerException { + + Weblog weblog = resolveWeblog(handle); + if (weblog == null) { + throw new InvalidFrontpageWeblogException(handle); + } + + PropertiesManager mgr = WebloggerFactory.getWeblogger().getPropertiesManager(); + + RuntimeConfigProperty handleProp = mgr.getProperty(HANDLE_PROPERTY); + handleProp.setValue(weblog.getHandle()); + mgr.saveProperty(handleProp); + + RuntimeConfigProperty aggregatedProp = mgr.getProperty(AGGREGATED_PROPERTY); + aggregatedProp.setValue(Boolean.toString(Boolean.TRUE.equals(aggregated))); + mgr.saveProperty(aggregatedProp); + + WebloggerFactory.getWeblogger().flush(); + + invalidateRenderedContent(); + } + + /** + * Drops the locally cached rendering of the front page. + * + *

The properties themselves are read through the properties manager on + * each request, but rendered pages and feeds are cached separately and would + * otherwise keep serving the previous weblog. Roller has no cross-node + * invalidation transport, so peers pick the change up when their own cache + * entries expire. + */ + private static void invalidateRenderedContent() { + SiteWideCache.getInstance().clear(); + WeblogPageCache.getInstance().clear(); + WeblogFeedCache.getInstance().clear(); + } + + /** Raised when a submitted frontpage handle cannot be used. */ + public static class InvalidFrontpageWeblogException extends WebloggerException { + private final String handle; + + public InvalidFrontpageWeblogException(String handle) { + super("Not an existing, enabled weblog handle: " + handle); + this.handle = handle; + } + + public String getHandle() { + return handle; + } + } +} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfig.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfig.java index 194337886c..b757615330 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfig.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/GlobalConfig.java @@ -25,6 +25,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.weblogger.WebloggerException; +import org.apache.roller.weblogger.business.FrontpageSettings; import org.apache.roller.weblogger.business.PropertiesManager; import org.apache.roller.weblogger.business.WeblogManager; import org.apache.roller.weblogger.business.WebloggerFactory; @@ -209,6 +210,22 @@ public String save() { Arrays.asList(propDesc, propName)); } + } else if ( FrontpageSettings.HANDLE_PROPERTY.equals(propertyDef.getName()) + && incomingProp != null ) { + // Declared as a plain string, but it names a weblog, so it + // is resolved through the same service as the setup path. The + // stored value is always a weblog that exists and is enabled. + try { + if (FrontpageSettings.resolveWeblog(incomingProp) == null) { + addError("frontpageConfig.invalidWeblog"); + } else { + updProp.setValue( incomingProp.trim() ); + } + } catch (WebloggerException ex) { + log.error("Error resolving frontpage weblog", ex); + addError("frontpageConfig.values.error"); + } + } else if ( incomingProp != null ){ updProp.setValue( incomingProp.trim() ); log.debug("Set something " + propName + " = " + incomingProp); diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/FrontpageSetup.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/FrontpageSetup.java new file mode 100644 index 0000000000..23a7585ac0 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/FrontpageSetup.java @@ -0,0 +1,122 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF 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 + * + * 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. For additional information regarding + * copyright in this work, please see the NOTICE file in the top level + * directory of this distribution. + */ + +package org.apache.roller.weblogger.ui.struts2.core; + +import java.util.Collections; +import java.util.List; + +import javax.servlet.http.HttpServletRequest; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.WebloggerException; +import org.apache.roller.weblogger.business.FrontpageSettings; +import org.apache.roller.weblogger.pojos.GlobalPermission; +import org.apache.roller.weblogger.ui.struts2.util.UIAction; +import org.apache.struts2.ServletActionContext; + +/** + * Chooses the site frontpage weblog for the first time. + * + *

This exists separately from {@link Setup} because the bootstrap page is + * reachable without a login while the site has no users. Here the caller must + * hold the global administrator permission, which the first registered user + * receives by default. + * + *

The action applies only to the initial choice. Once a frontpage weblog is + * set, later changes go through the global configuration screen, which is + * already administrator-only. + */ +public class FrontpageSetup extends UIAction { + + private static final Log LOG = LogFactory.getLog(FrontpageSetup.class); + + private String frontpageBlog; + private Boolean aggregated; + + public FrontpageSetup() { + this.pageTitle = "index.heading"; + } + + @Override + public boolean isWeblogRequired() { + return false; + } + + @Override + public List requiredGlobalPermissionActions() { + return Collections.singletonList(GlobalPermission.ADMIN); + } + + /** + * Stores the initial frontpage selection. + * + *

Reached only by POST, so the CSRF salt filter covers it, and only while + * no frontpage weblog has been chosen. + */ + public String save() { + + HttpServletRequest req = ServletActionContext.getRequest(); + if (!"POST".equalsIgnoreCase(req.getMethod())) { + return DENIED; + } + + try { + // Re-read immediately before writing so that a second submission + // arriving alongside the first cannot replace the winner. This + // narrows the window rather than closing it outright; the two + // submissions would have to interleave within this method, and the + // losing caller is told the choice is already made. + if (FrontpageSettings.isConfigured()) { + addError("frontpageConfig.alreadyConfigured"); + return "home"; + } + + FrontpageSettings.apply(frontpageBlog, aggregated); + addMessage("frontpageConfig.values.saved"); + + } catch (FrontpageSettings.InvalidFrontpageWeblogException ex) { + addError("frontpageConfig.invalidWeblog"); + return INPUT; + + } catch (WebloggerException ex) { + LOG.error("ERROR saving frontpage configuration", ex); + addError("frontpageConfig.values.error"); + return INPUT; + } + + return "home"; + } + + public String getFrontpageBlog() { + return frontpageBlog; + } + + public void setFrontpageBlog(String frontpageBlog) { + this.frontpageBlog = frontpageBlog; + } + + public Boolean getAggregated() { + return aggregated; + } + + public void setAggregated(Boolean aggregated) { + this.aggregated = aggregated; + } +} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Setup.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Setup.java index 00ab7e19c7..d224b65b57 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Setup.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Setup.java @@ -22,19 +22,25 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.roller.weblogger.WebloggerException; -import org.apache.roller.weblogger.business.PropertiesManager; +import org.apache.roller.weblogger.business.FrontpageSettings; import org.apache.roller.weblogger.business.WeblogManager; import org.apache.roller.weblogger.business.WebloggerFactory; -import org.apache.roller.weblogger.pojos.RuntimeConfigProperty; import org.apache.roller.weblogger.pojos.Weblog; import org.apache.roller.weblogger.ui.struts2.util.UIAction; -import org.apache.struts2.convention.annotation.AllowedMethods; /** * Page used to display Roller install instructions. + * + *

This page is reachable without a login because a brand new site has no + * users yet. While the site is empty it shows bootstrap guidance; once users + * exist it requires a global administrator, and once a frontpage weblog has + * been chosen it redirects home. + * + *

Choosing the initial frontpage weblog is {@link FrontpageSetup}, a + * separate global-administrator action; later changes go through the global + * configuration screen. */ -// TODO: make this work @AllowedMethods({"execute","save"}) public class Setup extends UIAction { private static final Log LOG = LogFactory.getLog(Setup.class); @@ -42,12 +48,12 @@ public class Setup extends UIAction { private long userCount = 0; private long blogCount = 0; - private String frontpageBlog; - private Boolean aggregated; - // weblogs for frontpage blog chooser private Collection weblogs; + // true while the site has no users and only bootstrap guidance is shown + private boolean bootstrap = false; + public Setup() { this.pageTitle = "index.heading"; } @@ -64,14 +70,6 @@ public boolean isWeblogRequired() { @Override public String execute() { - - try { - WeblogManager mgr = WebloggerFactory.getWeblogger().getWeblogManager(); - setWeblogs(mgr.getWeblogs(true, null, null, null, 0, -1)); - } catch (WebloggerException ex) { - LOG.error("Error getting weblogs", ex); - addError("frontpageConfig.weblogs.error"); - } try { setUserCount(WebloggerFactory.getWeblogger().getUserManager().getUserCount()); @@ -79,31 +77,42 @@ public String execute() { } catch (WebloggerException ex) { LOG.error("Error getting user/weblog counts", ex); } - - return SUCCESS; - } - public String save() { - PropertiesManager mgr = WebloggerFactory.getWeblogger().getPropertiesManager(); - try { - RuntimeConfigProperty frontpageBlogProp = mgr.getProperty("site.frontpage.weblog.handle"); - frontpageBlogProp.setValue(frontpageBlog); - mgr.saveProperty(frontpageBlogProp); - - RuntimeConfigProperty aggregatedProp = mgr.getProperty("site.frontpage.weblog.aggregated"); - aggregatedProp.setValue(aggregated.toString()); - mgr.saveProperty(aggregatedProp); + // A site with no users cannot have an administrator yet, so the + // bootstrap instructions are shown to anyone. Nothing about the site's + // contents is exposed here: registering the first user is the only + // thing that can usefully be done. + if (getUserCount() == 0) { + setBootstrap(true); + return SUCCESS; + } - WebloggerFactory.getWeblogger().flush(); + // Beyond that point this is a site configuration screen. + if (!isUserIsAdmin()) { + return DENIED; + } - addMessage("frontpageConfig.values.saved"); + try { + if (FrontpageSettings.isConfigured()) { + // Already chosen; later changes belong in global configuration. + return "home"; + } + } catch (WebloggerException ex) { + LOG.error("Error reading frontpage configuration", ex); + } + try { + WeblogManager mgr = WebloggerFactory.getWeblogger().getWeblogManager(); + setWeblogs(mgr.getWeblogs(true, null, null, null, 0, -1)); } catch (WebloggerException ex) { - LOG.error("ERROR saving frontpage configuration", ex); - addError("frontpageConfig.values.error"); + LOG.error("Error getting weblogs", ex); + addError("frontpageConfig.weblogs.error"); } - return "home"; + + return SUCCESS; } + + public long getUserCount() { return userCount; @@ -121,6 +130,14 @@ public void setBlogCount(long blogCount) { this.blogCount = blogCount; } + public boolean isBootstrap() { + return bootstrap; + } + + public void setBootstrap(boolean bootstrap) { + this.bootstrap = bootstrap; + } + public Collection getWeblogs() { return weblogs; } @@ -129,19 +146,4 @@ public void setWeblogs(Collection weblogs) { this.weblogs = weblogs; } - public String getFrontpageBlog() { - return frontpageBlog; - } - - public void setFrontpageBlog(String frontpageBlog) { - this.frontpageBlog = frontpageBlog; - } - - public Boolean getAggregated() { - return aggregated; - } - - public void setAggregated(Boolean aggregated) { - this.aggregated = aggregated; - } } diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index 66072c23f0..42943ffe2e 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -553,6 +553,8 @@ frontpageConfig.frontpageAggregated=Enable aggregated site-wide frontpage frontpageConfig.values.saved=Properties successfully saved frontpageConfig.values.error=Error saving properties frontpageConfig.weblogs.error=Unexpected error accessing Weblogs +frontpageConfig.invalidWeblog=Choose an existing, enabled weblog for the frontpage +frontpageConfig.alreadyConfigured=A frontpage weblog has already been chosen; change it from the global configuration page # --------------------------------------------------------------- Invite member diff --git a/app/src/main/resources/struts.xml b/app/src/main/resources/struts.xml index cc94ba6588..80a29e2f81 100644 --- a/app/src/main/resources/struts.xml +++ b/app/src/main/resources/struts.xml @@ -112,7 +112,14 @@ class="org.apache.roller.weblogger.ui.struts2.core.Setup"> .Setup home - activate,execute,save + execute + + + + .Setup + home + save

- - + - @@ -93,7 +93,8 @@ - + The setup screen is reachable without a login, because a site with no users + * has nobody who could log in. A page in that position should display bootstrap + * guidance and nothing more, so the frontpage write lives on a separate action + * that requires a global administrator. These tests pin that arrangement in + * place: the display page exposes no write method, the write action requires the + * permission, and both write paths validate through one service. + */ +public class FrontpageSetupAccessTest { + + private static final Path STRUTS_XML = Paths.get("src", "main", "resources", "struts.xml"); + private static final Path SETUP_JSP = + Paths.get("src", "main", "webapp", "WEB-INF", "jsps", "core", "Setup.jsp"); + + private String read(Path path) throws IOException { + assertTrue(Files.isReadable(path), + "cannot read " + path.toAbsolutePath() + " (run from the app module)"); + return new String(Files.readAllBytes(path), StandardCharsets.UTF_8); + } + + /** + * The mutation action requires the global administrator permission. This is + * the single check the whole fix rests on. + */ + @Test + public void frontpageSetupRequiresGlobalAdmin() { + List required = new FrontpageSetup().requiredGlobalPermissionActions(); + assertEquals(1, required.size(), "expected exactly one required permission"); + assertEquals(GlobalPermission.ADMIN, required.get(0), + "the frontpage write must require a global administrator"); + } + + /** + * The public setup page must not require a user, because it has to work on + * an empty site. That is precisely why it must not be able to write. + */ + @Test + public void publicSetupPageStillNeedsNoUserButCannotWrite() throws IOException { + Setup setup = new Setup(); + assertFalse(setup.isUserRequired(), + "the bootstrap page must stay reachable on a site with no users"); + + String struts = read(STRUTS_XML); + int setupIdx = struts.indexOf("name=\"setup\""); + assertTrue(setupIdx > 0, "setup action not found in struts.xml"); + String setupBlock = struts.substring(setupIdx, struts.indexOf("", setupIdx)); + assertFalse(setupBlock.contains("save"), + "the public setup action must expose no save method:\n" + setupBlock); + } + + /** The separate action exists and exposes only its save method. */ + @Test + public void frontpageSetupActionIsWiredAndSaveOnly() throws IOException { + String struts = read(STRUTS_XML); + int idx = struts.indexOf("name=\"frontpageSetup\""); + assertTrue(idx > 0, "frontpageSetup action not wired in struts.xml"); + String block = struts.substring(idx, struts.indexOf("", idx)); + assertTrue(block.contains("FrontpageSetup"), "wrong action class:\n" + block); + assertTrue(block.contains("save"), + "frontpageSetup must expose only save:\n" + block); + } + + /** The form must post to the administrator-only action, over POST. */ + @Test + public void setupFormPostsToTheAdminAction() throws IOException { + String jsp = read(SETUP_JSP); + assertFalse(jsp.contains("setup!save"), + "the form must no longer target the public setup action"); + assertTrue(jsp.contains("frontpageSetup!save"), + "the form must target the administrator-only action"); + assertTrue(jsp.contains("method=\"post\""), + "the form must POST so the CSRF salt filter applies"); + assertTrue(jsp.contains(""), + "the form must carry a CSRF salt"); + } + + /** + * Both write paths must resolve the handle through the shared service, so + * neither can store a weblog that does not exist. + */ + @Test + public void bothWritePathsValidateThroughTheSharedService() throws IOException { + String globalConfig = read(Paths.get("src", "main", "java", "org", "apache", "roller", + "weblogger", "ui", "struts2", "admin", "GlobalConfig.java")); + assertTrue(globalConfig.contains("FrontpageSettings.resolveWeblog"), + "the global configuration screen must validate the frontpage handle"); + + String frontpageSetup = read(Paths.get("src", "main", "java", "org", "apache", "roller", + "weblogger", "ui", "struts2", "core", "FrontpageSetup.java")); + assertTrue(frontpageSetup.contains("FrontpageSettings.apply"), + "the initial write must go through the shared service"); + assertTrue(frontpageSetup.contains("FrontpageSettings.isConfigured"), + "the initial write must apply only while no frontpage is set"); + } + + /** The write action must reject requests that are not HTTP POST. */ + @Test + public void frontpageSetupSaveEnforcesPost() throws IOException { + String frontpageSetup = read(Paths.get("src", "main", "java", "org", "apache", "roller", + "weblogger", "ui", "struts2", "core", "FrontpageSetup.java")); + assertTrue(frontpageSetup.contains("\"POST\".equalsIgnoreCase"), + "save() must reject non-POST requests"); + assertTrue(frontpageSetup.contains("getMethod()"), + "save() must inspect the request method"); + } + + /** A blank handle can never resolve, whatever the database contains. */ + @Test + public void blankHandlesNeverResolve() throws Exception { + assertEquals(null, FrontpageSettings.resolveWeblog(null)); + assertEquals(null, FrontpageSettings.resolveWeblog("")); + assertEquals(null, FrontpageSettings.resolveWeblog(" ")); + } +}