mraible commented on code in PR #170:
URL: https://github.com/apache/roller/pull/170#discussion_r3891439308


##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/Setup.java:
##########
@@ -64,46 +70,49 @@ 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());
             
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);
+        // 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;

Review Comment:
   `index.jsp` (lines 29-31) forwards `/` here whenever the frontpage handle is 
blank, so returning `DENIED` for non-admins makes the home page an 
access-denied tile for every visitor of a site that has users but no frontpage 
yet (`checkPermission` with a `null` user throws and `isUserIsAdmin()` swallows 
it to false). The page only showed counts and links before; I'd keep 
`execute()` open and gate the form in the JSP on `isUserIsAdmin()`, leaving the 
POST admin-only.



##########
app/src/main/webapp/WEB-INF/jsps/core/Setup.jsp:
##########
@@ -93,7 +93,8 @@
 
         <s:if test="blogCount > 0">

Review Comment:
   The heading was changed to `blogCount > 0 && !bootstrap` but this guard 
wasn't, so in bootstrap mode (weblogs exist, zero users, `weblogs` is `null`) 
anonymous visitors still get the form with an empty select and a Save that 
POSTs to an admin-only action.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to