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..5f75c8ebe5 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/business/FrontpageSettings.java @@ -0,0 +1,132 @@ +/* + * 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; + +/** + * 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 and both properties move together. + */ +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) || !isValidHandle(handle.trim())) { + 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 resolveWeblog(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 boolean applyInitial(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); + String currentValue = handleProp == null ? null : handleProp.getValue(); + if (resolveWeblog(currentValue) != null + || !mgr.compareAndSetProperty(HANDLE_PROPERTY, currentValue, weblog.getHandle())) { + return false; + } + + RuntimeConfigProperty aggregatedProp = mgr.getProperty(AGGREGATED_PROPERTY); + aggregatedProp.setValue(Boolean.toString(Boolean.TRUE.equals(aggregated))); + mgr.saveProperty(aggregatedProp); + + WebloggerFactory.getWeblogger().flush(); + + return true; + } + + private static boolean isValidHandle(String handle) { + for (int i = 0; i < handle.length(); i++) { + if (!Character.isLetterOrDigit(handle.charAt(i)) && handle.charAt(i) != '_') { + return false; + } + } + return true; + } + + /** 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/business/PropertiesManager.java b/app/src/main/java/org/apache/roller/weblogger/business/PropertiesManager.java index 5b0fef784b..d0ed156441 100644 --- a/app/src/main/java/org/apache/roller/weblogger/business/PropertiesManager.java +++ b/app/src/main/java/org/apache/roller/weblogger/business/PropertiesManager.java @@ -52,6 +52,15 @@ public interface PropertiesManager { * Save a list of properties */ void saveProperties(Map properties) throws WebloggerException; + + + /** + * Replace a property's value only when it still has the expected value. + * + * @return true when the property was updated, otherwise false + */ + boolean compareAndSetProperty(String name, String expectedValue, String newValue) + throws WebloggerException; /** diff --git a/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAPropertiesManagerImpl.java b/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAPropertiesManagerImpl.java index 255ee047f8..b576d6aa74 100644 --- a/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAPropertiesManagerImpl.java +++ b/app/src/main/java/org/apache/roller/weblogger/business/jpa/JPAPropertiesManagerImpl.java @@ -21,6 +21,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; + +import jakarta.persistence.LockModeType; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -144,6 +147,19 @@ public void saveProperties(Map properties) throws this.strategy.store(prop); } } + + + @Override + public boolean compareAndSetProperty(String name, String expectedValue, String newValue) + throws WebloggerException { + RuntimeConfigProperty property = strategy.getEntityManager(true).find( + RuntimeConfigProperty.class, name, LockModeType.PESSIMISTIC_WRITE); + if (property == null || !Objects.equals(expectedValue, property.getValue())) { + return false; + } + property.setValue(newValue); + return true; + } /** 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..4cd235084d 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; @@ -37,6 +38,9 @@ import org.apache.roller.weblogger.pojos.GlobalPermission; 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; import org.apache.roller.weblogger.ui.struts2.util.UIAction; import org.apache.roller.weblogger.util.Utilities; import org.apache.struts2.dispatcher.HttpParameters; @@ -156,6 +160,9 @@ public String save() { return ERROR; } + String oldFrontpageHandle = propertyValue(FrontpageSettings.HANDLE_PROPERTY); + String oldFrontpageAggregated = propertyValue(FrontpageSettings.AGGREGATED_PROPERTY); + // only set values for properties that are already defined RuntimeConfigProperty updProp; String incomingProp; @@ -209,6 +216,23 @@ 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 { + Weblog weblog = FrontpageSettings.resolveWeblog(incomingProp); + if (weblog == null) { + addError("frontpageConfig.invalidWeblog"); + } else { + updProp.setValue(weblog.getHandle()); + } + } 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); @@ -240,6 +264,12 @@ public String save() { mgr.saveProperties(getProperties()); WebloggerFactory.getWeblogger().flush(); + if (!Objects.equals(oldFrontpageHandle, propertyValue(FrontpageSettings.HANDLE_PROPERTY)) + || !Objects.equals(oldFrontpageAggregated, + propertyValue(FrontpageSettings.AGGREGATED_PROPERTY))) { + invalidateRenderedContent(); + } + // notify user of our success addMessage("generic.changes.saved"); @@ -251,6 +281,17 @@ public String save() { return SUCCESS; } + private String propertyValue(String name) { + RuntimeConfigProperty property = getProperties().get(name); + return property == null ? null : property.getValue(); + } + + private void invalidateRenderedContent() { + SiteWideCache.getInstance().clear(); + WeblogPageCache.getInstance().clear(); + WeblogFeedCache.getInstance().clear(); + } + @Override public void setParameters(HttpParameters parameters) { 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..cac83ba4e1 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/FrontpageSetup.java @@ -0,0 +1,138 @@ +/* + * 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.rendering.util.cache.SiteWideCache; +import org.apache.roller.weblogger.ui.rendering.util.cache.WeblogFeedCache; +import org.apache.roller.weblogger.ui.rendering.util.cache.WeblogPageCache; +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 Setup { + + 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 boolean isUserRequired() { + return true; + } + + @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() { + + if (!isPostRequest()) { + return DENIED; + } + + try { + if (!FrontpageSettings.applyInitial(frontpageBlog, aggregated)) { + addError("frontpageConfig.alreadyConfigured"); + loadSetupModel(); + setFrontpageConfigured(true); + return INPUT; + } + + invalidateRenderedContent(); + addMessage("frontpageConfig.values.saved"); + + } catch (FrontpageSettings.InvalidFrontpageWeblogException ex) { + addError("frontpageConfig.invalidWeblog"); + loadSetupModel(); + return INPUT; + + } catch (WebloggerException ex) { + LOG.error("ERROR saving frontpage configuration", ex); + addError("frontpageConfig.values.error"); + loadSetupModel(); + return INPUT; + } + + return "home"; + } + + protected boolean isPostRequest() { + HttpServletRequest req = ServletActionContext.getRequest(); + return req != null && "POST".equalsIgnoreCase(req.getMethod()); + } + + private void invalidateRenderedContent() { + SiteWideCache.getInstance().clear(); + WeblogPageCache.getInstance().clear(); + WeblogFeedCache.getInstance().clear(); + } + + 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..77db586404 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 remains useful to everyone, but only a global administrator sees + * the frontpage chooser. + * + *

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,15 @@ 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; + + // true when a valid frontpage weblog has already been selected + private boolean frontpageConfigured = false; + public Setup() { this.pageTitle = "index.heading"; } @@ -64,46 +73,57 @@ 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"); + + loadSetupModel(); + + if (isBootstrap()) { + return SUCCESS; } + if (isFrontpageConfigured()) { + return "home"; + } + + return SUCCESS; + } + + /** Loads the model used by both the public page and failed save results. */ + protected void loadSetupModel() { + try { setUserCount(WebloggerFactory.getWeblogger().getUserManager().getUserCount()); setBlogCount(WebloggerFactory.getWeblogger().getWeblogManager().getWeblogCount()); } 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); - - WebloggerFactory.getWeblogger().flush(); - - addMessage("frontpageConfig.values.saved"); + // 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; + } + try { + setFrontpageConfigured(FrontpageSettings.isConfigured()); } catch (WebloggerException ex) { - LOG.error("ERROR saving frontpage configuration", ex); - addError("frontpageConfig.values.error"); + LOG.error("Error reading frontpage configuration", ex); + } + + if (isUserIsAdmin() && !isFrontpageConfigured()) { + 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"); + } } - return "home"; } + + public long getUserCount() { return userCount; @@ -121,27 +141,28 @@ public void setBlogCount(long blogCount) { this.blogCount = blogCount; } - public Collection getWeblogs() { - return weblogs; + public boolean isBootstrap() { + return bootstrap; } - public void setWeblogs(Collection weblogs) { - this.weblogs = weblogs; + public void setBootstrap(boolean bootstrap) { + this.bootstrap = bootstrap; } - - public String getFrontpageBlog() { - return frontpageBlog; + + public boolean isFrontpageConfigured() { + return frontpageConfigured; } - public void setFrontpageBlog(String frontpageBlog) { - this.frontpageBlog = frontpageBlog; + public void setFrontpageConfigured(boolean frontpageConfigured) { + this.frontpageConfigured = frontpageConfigured; } - public Boolean getAggregated() { - return aggregated; + public Collection getWeblogs() { + return weblogs; } - public void setAggregated(Boolean aggregated) { - this.aggregated = aggregated; + public void setWeblogs(Collection weblogs) { + this.weblogs = weblogs; } + } diff --git a/app/src/main/resources/ApplicationResources.properties b/app/src/main/resources/ApplicationResources.properties index 66072c23f0..1fab65e322 100644 --- a/app/src/main/resources/ApplicationResources.properties +++ b/app/src/main/resources/ApplicationResources.properties @@ -547,12 +547,15 @@ index.setFrontpageHelp=\ You must specify a weblog to serve as the front page weblog, you can do this \ via the Server Admin->Configuration page or the form that will appear \ below once you have created at least one weblog. +index.setFrontpageAdminRequired=A global administrator must designate the frontpage weblog. frontpageConfig.frontpageBlogName=Name of weblog to serve as frontpage blog 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 +

- - + - @@ -91,9 +92,10 @@

- + - + + +

+

- + diff --git a/app/src/test/java/org/apache/roller/weblogger/business/PropertiesTest.java b/app/src/test/java/org/apache/roller/weblogger/business/PropertiesTest.java index d35a9fc0c2..facccc4c50 100644 --- a/app/src/test/java/org/apache/roller/weblogger/business/PropertiesTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/business/PropertiesTest.java @@ -27,6 +27,10 @@ import org.junit.jupiter.api.Test; import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import static org.junit.jupiter.api.Assertions.*; @@ -91,5 +95,52 @@ public void testProperiesCRUD() throws Exception { assertEquals("foofoo", props.get("site.name").getValue()); assertEquals("blahblah", props.get("site.description").getValue()); } + + @Test + public void compareAndSetAllowsOnlyOneConcurrentWinner() throws Exception { + PropertiesManager mgr = WebloggerFactory.getWeblogger().getPropertiesManager(); + RuntimeConfigProperty prop = mgr.getProperty("site.frontpage.weblog.handle"); + String original = prop.getValue(); + prop.setValue(""); + mgr.saveProperty(prop); + TestUtils.endSession(true); + + CountDownLatch ready = new CountDownLatch(2); + CountDownLatch start = new CountDownLatch(1); + ExecutorService executor = Executors.newFixedThreadPool(2); + try { + Future first = executor.submit(() -> compareAndSetAfterSignal( + "site.frontpage.weblog.handle", "first", ready, start)); + Future second = executor.submit(() -> compareAndSetAfterSignal( + "site.frontpage.weblog.handle", "second", ready, start)); + ready.await(); + start.countDown(); + + assertNotEquals(first.get(), second.get(), "exactly one update must win"); + + RuntimeConfigProperty saved = WebloggerFactory.getWeblogger() + .getPropertiesManager().getProperty("site.frontpage.weblog.handle"); + assertTrue("first".equals(saved.getValue()) || "second".equals(saved.getValue())); + saved.setValue(original); + WebloggerFactory.getWeblogger().getPropertiesManager().saveProperty(saved); + TestUtils.endSession(true); + } finally { + executor.shutdownNow(); + } + } + + private boolean compareAndSetAfterSignal(String name, String value, + CountDownLatch ready, CountDownLatch start) throws Exception { + ready.countDown(); + start.await(); + try { + boolean updated = WebloggerFactory.getWeblogger().getPropertiesManager() + .compareAndSetProperty(name, "", value); + WebloggerFactory.getWeblogger().flush(); + return updated; + } finally { + WebloggerFactory.getWeblogger().release(); + } + } } diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/struts2/core/FrontpageSetupAccessTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/core/FrontpageSetupAccessTest.java new file mode 100644 index 0000000000..c82c64f727 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/struts2/core/FrontpageSetupAccessTest.java @@ -0,0 +1,91 @@ +/* + * 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.List; + +import org.apache.roller.weblogger.business.FrontpageSettings; +import org.apache.roller.weblogger.pojos.GlobalPermission; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; + +/** + * Checks who is allowed to change the site frontpage setting. + * + *

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 validates requests before attempting a write. + */ +public class FrontpageSetupAccessTest { + + /** + * 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 publicSetupPageStillNeedsNoUser() { + Setup setup = new Setup(); + assertFalse(setup.isUserRequired(), + "the bootstrap page must stay reachable on a site with no users"); + } + + /** The separate write action always requires an authenticated user. */ + @Test + public void frontpageSetupRequiresAUser() { + assertEquals(true, new FrontpageSetup().isUserRequired()); + } + + /** The write action must reject requests that are not HTTP POST. */ + @Test + public void frontpageSetupSaveEnforcesPost() { + FrontpageSetup action = new FrontpageSetup() { + @Override + protected boolean isPostRequest() { + return false; + } + }; + assertEquals(FrontpageSetup.DENIED, action.save()); + } + + /** Blank and malformed handles are rejected before a database lookup. */ + @Test + public void invalidHandlesNeverResolve() throws Exception { + assertNull(FrontpageSettings.resolveWeblog(null)); + assertNull(FrontpageSettings.resolveWeblog("")); + assertNull(FrontpageSettings.resolveWeblog(" ")); + assertNull(FrontpageSettings.resolveWeblog("not/a/handle")); + } +} diff --git a/it-selenium/src/test/java/org/apache/roller/selenium/InitialLoginTestIT.java b/it-selenium/src/test/java/org/apache/roller/selenium/InitialLoginTestIT.java index c766c15b7e..95b396678e 100644 --- a/it-selenium/src/test/java/org/apache/roller/selenium/InitialLoginTestIT.java +++ b/it-selenium/src/test/java/org/apache/roller/selenium/InitialLoginTestIT.java @@ -86,7 +86,7 @@ public void testInitialLogin() throws Exception { driver.get(baseUrl); sp = new SetupPage(driver); driver.navigate().refresh(); - BlogHomePage bhp = sp.chooseFrontPageBlog(); + BlogHomePage bhp = sp.chooseFrontPageBlog("bobsblog"); // create and read first blog entry String blogEntryTitle = "My First Blog Entry"; diff --git a/it-selenium/src/test/java/org/apache/roller/selenium/core/SetupPage.java b/it-selenium/src/test/java/org/apache/roller/selenium/core/SetupPage.java index 8359dff863..7a8ae4d42b 100644 --- a/it-selenium/src/test/java/org/apache/roller/selenium/core/SetupPage.java +++ b/it-selenium/src/test/java/org/apache/roller/selenium/core/SetupPage.java @@ -19,7 +19,9 @@ import org.apache.roller.selenium.AbstractRollerPage; import org.apache.roller.selenium.view.BlogHomePage; +import org.openqa.selenium.By; import org.openqa.selenium.WebDriver; +import org.openqa.selenium.support.ui.Select; /** * represents core/Setup.jsp @@ -40,9 +42,12 @@ public RegisterPage createNewUser() { return new RegisterPage(driver); } - public BlogHomePage chooseFrontPageBlog() { - verifyPageTitle("setup_0", "Front Page: Welcome to Roller!"); - clickById("setup_0"); + public BlogHomePage chooseFrontPageBlog(String handle) { + verifyPageTitle("Front Page: Welcome to Roller!"); + Select chooser = new Select(driver.findElement(By.name("frontpageBlog"))); + chooser.selectByValue(handle); + driver.findElement(By.cssSelector( + "form[action*='frontpageSetup'] input[type='submit']")).click(); return new BlogHomePage(driver); } -} \ No newline at end of file +}