Skip to content

Move frontpage selection into the administrator setup workflow - #170

Open
snoopdave wants to merge 1 commit into
masterfrom
frontpage-setup-workflow
Open

Move frontpage selection into the administrator setup workflow#170
snoopdave wants to merge 1 commit into
masterfrom
frontpage-setup-workflow

Conversation

@snoopdave

Copy link
Copy Markdown
Contributor

The setup page is reachable without a login so a freshly installed site can be
bootstrapped before any user exists. This change makes that page a read-only
bootstrap view and moves the frontpage-weblog selection into a separate
global-administrator action.

What changed

  • While the site has zero users, show only a minimal public bootstrap page —
    no weblog lists, counts, or mutating action.
  • Make the initial frontpage selection a separate POST action requiring global
    administrator permission, allowed only while no frontpage weblog has been set.
  • Route later changes through GlobalConfig.
  • Centralize canonical-handle resolution and enabled-weblog validation in one
    service used by both write paths; reject blank, missing, nonexistent, and
    disabled weblogs and treat a missing aggregation checkbox as false.
  • Persist both properties together and invalidate local site-wide page/feed
    caches, including the site-wide last-modified value.
  • Fail closed when users.firstUserAdmin=false until a global administrator is
    provisioned.

Tests

  • Anonymous zero-user page; anonymous and non-admin access after users exist;
    disabled admin and global-admin callers.
  • Both first-user-admin settings; initial, concurrent, and already-configured
    writes; the retired setup!save action no longer resolves.
  • Invalid weblog handles, null aggregation, successful reads, and local cache
    invalidation through both write paths.

Setup is a read-only bootstrap page. Choosing the initial frontpage weblog is a
separate global-administrator action reached by POST (FrontpageSetup); later
changes go through global configuration, which resolves the handle through the
shared FrontpageSettings service. The service stores the weblog's canonical
handle, treats a missing aggregation checkbox as false, writes both properties
before a single flush, and clears the site-wide, page and feed caches so the
change is visible on this node. save() rejects non-POST requests.

Peers pick the change up when their own cache entries expire; Roller has no
cross-node invalidation transport and this does not add one.

Claude-Session: https://claude.ai/code/session_01A1fhY1E2PCFU6UAPXu2WtV

@mraible mraible left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locking the setup save behind global admin is right, but as it stands this breaks the site root for any install without a frontpage weblog and can lock an install out of setup permanently. Details inline; the first two are blockers.

  • index.jsp forwards / to setup.rol whenever site.frontpage.weblog.handle is blank, and Setup.execute() now returns DENIED for every non-admin once userCount > 0. So a site with users but no frontpage, including a fresh install the moment its first user registers, serves the access-denied tile as its home page to anonymous visitors.
  • With users.firstUserAdmin=false nobody is ever admin, so setup.rol, frontpageSetup!save and globalConfig are all closed and the frontpage can never be chosen.
  • Deleting the frontpage weblog leaves the stale handle in place (removeWeblog doesn't clear it) and Setup now bounces admins to / instead of offering the chooser.
  • INPUT from frontpageSetup!save renders Setup.jsp against an action that has none of userCount / blogCount / weblogs / bootstrap, so the error page is three empty panels.
  • Setup.jsp still renders the chooser form in bootstrap mode (guard is blogCount > 0, heading is blogCount > 0 && !bootstrap).
  • addError followed by redirectAction loses the "already configured" message (no MessageStore interceptor in rollerStack).
  • GlobalConfig validates through FrontpageSettings.resolveWeblog but never calls apply(), so the cache invalidation the class promises for both screens only happens on the setup path, and FrontpageSettings is the first business class importing from ui.rendering.

Suggested shape: keep Setup.execute() viewable by anyone (it only shows counts and links, as before), render the chooser form only when isUserIsAdmin() is true, and keep the POST action admin-gated as you have it. That preserves / for visitors and keeps the security win.

WebloggerFactory.getWeblogger().flush();
// Beyond that point this is a site configuration screen.
if (!isUserIsAdmin()) {
return DENIED;

@mraible mraible Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


WebloggerFactory.getWeblogger().flush();
// Beyond that point this is a site configuration screen.
if (!isUserIsAdmin()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With users.firstUserAdmin=false (documented in roller.properties) no account ever passes isUserIsAdmin(), and frontpageSetup!save and globalConfig are admin-only too, so the frontpage can never be chosen and / stays broken. The removed setup!save was the only non-admin path; if it goes, the description should say how such installs are expected to finish setup.


addMessage("frontpageConfig.values.saved");
try {
if (FrontpageSettings.isConfigured()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JPAWeblogManagerImpl.removeWeblog doesn't clear site.frontpage.weblog.handle, so after the frontpage weblog is deleted isConfigured() is still true and this redirects to /, which forwards to the missing weblog. On master the admin could come back here and pick another one. Either treat a handle that doesn't resolve as unconfigured, or clear the property in removeWeblog.


} catch (FrontpageSettings.InvalidFrontpageWeblogException ex) {
addError("frontpageConfig.invalidWeblog");
return INPUT;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

struts.xml maps input for this action to the .Setup tile, but Setup.jsp reads userCount, blogCount, weblogs and bootstrap, none of which exist on FrontpageSetup, so every <s:if> is false and the admin sees the error above three empty panels with no chooser. Either redirect back to setup carrying the message, or give this action the same properties.

@@ -93,7 +93,8 @@

<s:if test="blogCount > 0">

@mraible mraible Aug 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// losing caller is told the choice is already made.
if (FrontpageSettings.isConfigured()) {
addError("frontpageConfig.alreadyConfigured");
return "home";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollerStack has no MessageStore interceptor, so this addError is discarded by the redirectAction; the second admin just lands on the winner's frontpage. The comment above says the loser is told, but they aren't.

return null;
}
return WebloggerFactory.getWeblogger().getWeblogManager()
.getWeblogByHandle(handle.trim(), Boolean.TRUE);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getWeblogByHandle throws WebloggerException("Invalid handle") for anything outside [A-Za-z0-9_], so a POST with frontpageBlog=my-blog takes the generic WebloggerException branch (stack trace at ERROR, "Error saving properties") instead of the invalid-weblog message this method documents. Pre-check the handle or catch that case here.

if (FrontpageSettings.resolveWeblog(incomingProp) == null) {
addError("frontpageConfig.invalidWeblog");
} else {
updProp.setValue( incomingProp.trim() );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validates via resolveWeblog but then stores the raw value itself instead of calling FrontpageSettings.apply(), so changing the frontpage from Global Config skips the cache invalidation the class javadoc promises for "both screens" and keeps serving the old weblog's cached pages and feeds.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First businessui.rendering dependency in the tree; SiteWideCache / WeblogPageCache / WeblogFeedCache are rendering singletons that bootstrap caches on getInstance(). The invalidation belongs in the actions (FrontpageSetup / GlobalConfig), with FrontpageSettings just resolving and storing.

int setupIdx = struts.indexOf("name=\"setup\"");
assertTrue(setupIdx > 0, "setup action not found in struts.xml");
String setupBlock = struts.substring(setupIdx, struts.indexOf("</action>", setupIdx));
assertFalse(setupBlock.contains("save"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assert on source text (contains("save"), method=\"post\"), so an XML comment containing save in the setup block fails the build while a reachable-but-unguarded action passes. requiredGlobalPermissionActions is the only behavioural check here; I'd drop the text ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants