OF-1392 Add "No one can create a chat room"#3379
Conversation
option to room creation permissions
Add the muc.create.permission.no_one_created translation key.
Update muc-create-permission.jsp to handle openPerms as a three-state option ("anyone", "specific", "none").
Introduce a third radio button for "No one can create a chat room".
Adjust javascript/CSS visibility logic so that the allowed users/groups container is hidden when "No one" is selected.
📝 WalkthroughWalkthroughThis PR converts the MUC room creation permission model from a boolean to a tri-state string representation ( Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
xmppserver/src/main/webapp/muc-create-permission.jsp (1)
75-107:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing fallback when
openPermshas an unexpected value.If
saveis true butopenPermsisnull, empty, or an unrecognized value (e.g., from a tampered request), no branch matches and the save silently completes without any action or error feedback. Consider adding a fallback or error handling:Proposed fix
else if ("specific".equals(openPerms)) { mucService.setRoomCreationRestricted(true); mucService.setAllRegisteredUsersAllowedToCreate(allowAllRegisteredUsers); // Log the event webManager.logEvent("set MUC room creation to restricted for service "+mucname, null); response.sendRedirect("muc-create-permission.jsp?success=true&mucname="+URLEncoder.encode(mucname, StandardCharsets.UTF_8)); return; } + else { + errors.put("openPerms", "openPerms"); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xmppserver/src/main/webapp/muc-create-permission.jsp` around lines 75 - 107, When save is true but openPerms is null/empty or an unexpected value, no branch executes—add a fallback to handle invalid input: validate openPerms at the start of the save block and if it's null/unknown, call webManager.logEvent with a clear message mentioning mucname and the invalid openPerms, set a sensible default on mucService (e.g., restrict creation or leave unchanged) or explicitly return an error by redirecting via response.sendRedirect("muc-create-permission.jsp?error=invalid_permission&mucname="+URLEncoder.encode(mucname, UTF_8)); ensure this uses the existing mucService and response.sendRedirect flow so the user receives feedback instead of a silent no-op.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@xmppserver/src/main/webapp/muc-create-permission.jsp`:
- Around line 75-107: When save is true but openPerms is null/empty or an
unexpected value, no branch executes—add a fallback to handle invalid input:
validate openPerms at the start of the save block and if it's null/unknown, call
webManager.logEvent with a clear message mentioning mucname and the invalid
openPerms, set a sensible default on mucService (e.g., restrict creation or
leave unchanged) or explicitly return an error by redirecting via
response.sendRedirect("muc-create-permission.jsp?error=invalid_permission&mucname="+URLEncoder.encode(mucname,
UTF_8)); ensure this uses the existing mucService and response.sendRedirect flow
so the user receives feedback instead of a silent no-op.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d853bbe6-9424-4684-9c95-426863462be7
📒 Files selected for processing (2)
i18n/src/main/resources/openfire_i18n.propertiesxmppserver/src/main/webapp/muc-create-permission.jsp
There was a problem hiding this comment.
🧹 Nitpick comments (1)
xmppserver/src/main/webapp/muc-create-permission.jsp (1)
106-108: 💤 Low valueGood defensive error handling for invalid
openPermsvalues.This correctly catches cases where
openPermsisnull(e.g., form submitted without selection) or an unexpected value, preventing silent failures where the user clicks save but nothing happens.Minor style nit: The formatting
}else{is inconsistent with the rest of the file (e.g., line 99 useselse if (...) {with spaces). Consider standardizing:♻️ Optional style fix
- }else{ + } else { errors.put("openPerms", "openPerms"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xmppserver/src/main/webapp/muc-create-permission.jsp` around lines 106 - 108, The style is inconsistent: change the brace spacing for the final conditional block so it matches the project's style (use "} else {" instead of "}else{") around the code that performs errors.put("openPerms", "openPerms");—locate the conditional that sets openPerms errors in muc-create-permission.jsp and update the spacing to align with the existing "else if (...) {" style.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@xmppserver/src/main/webapp/muc-create-permission.jsp`:
- Around line 106-108: The style is inconsistent: change the brace spacing for
the final conditional block so it matches the project's style (use "} else {"
instead of "}else{") around the code that performs errors.put("openPerms",
"openPerms");—locate the conditional that sets openPerms errors in
muc-create-permission.jsp and update the spacing to align with the existing
"else if (...) {" style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f8651c28-408a-4509-a026-1838666c6060
📒 Files selected for processing (1)
xmppserver/src/main/webapp/muc-create-permission.jsp
1 participant
Summary
Currently, room creation permissions support only two policies:
To completely prevent room creation, administrators must select the specific users/groups option and add a placeholder account (for example, an admin user) to the allowed list. This workaround is unintuitive and requires unnecessary configuration.
This PR introduces a third policy:
This allows room creation to be completely disabled for regular users without requiring dummy users or groups. System administrators continue to bypass these restrictions and can always create rooms.
Changes
Internationalization
muc.create.permission.no_one_createdtranslation key toopenfire_i18n.properties.UI & Backend Logic
Updated
muc-create-permission.jspto support three room creation policies:anyonespecificnoneAdditional changes:
Added a new radio button option: "No one can create a chat room."
Updated permission handling to support the new
nonestate.Updated page initialization logic to correctly reflect the selected policy when loading the page.
Added dynamic visibility handling for the Allowed Users/Groups section:
anyoneis selected.noneis selected.specificis selected.Updated audit log messages to accurately describe policy changes for all supported modes.
Verification
UI Verification
Functional Verification