[7 / 7] - Admin settings + Cross-site request forgery / Nonces#204
Open
TheSCREWEDSoftware wants to merge 7 commits into
Open
[7 / 7] - Admin settings + Cross-site request forgery / Nonces#204TheSCREWEDSoftware wants to merge 7 commits into
TheSCREWEDSoftware wants to merge 7 commits into
Conversation
…nection logging + GeoIP
📝 WalkthroughWalkthroughThis PR adds a user Security page with website and in-game 2FA management (TOTP/email verification, WP2FA integration), a password-change handler with SOAP sync and reuse history, CSRF nonce protection across all admin forms, a connection logger recording login IPs with GeoIP enrichment, a dark-mode admin toggle, server-module management panels in Realm Settings, admin 2FA removal tools in the Tools page, and UI overhauls for character ordering, mail return, item restoration, and unstuck menus backed by a new ChangesAzerothCore WP Plugin: Security, 2FA, Connection Logging, and UI Overhaul
Sequence Diagram(s)sequenceDiagram
participant User as User Browser
participant SecurityPage as SecurityPage.php / JS
participant PasswordHandler as PasswordHandler (admin_init)
participant ServerInfoApi as ServerInfoApi REST
participant WP2FA as WP2FA Plugin
participant GameDB as Game Account DB
participant ConnectionLogger as ConnectionLogger (wp_login)
User->>SecurityPage: Load /security page
SecurityPage->>ServerInfoApi: GET 2fa-status
ServerInfoApi-->>SecurityPage: {website_enabled, ingame_enabled, removal_count}
User->>PasswordHandler: POST change-password form (nonce)
PasswordHandler->>GameDB: SOAP sync new password
PasswordHandler-->>User: redirect with transient message
User->>SecurityPage: POST verify-website-2fa (6-digit TOTP)
SecurityPage->>ServerInfoApi: POST verify-website-2fa
ServerInfoApi->>WP2FA: validate TOTP token
ServerInfoApi-->>SecurityPage: {success} + set unlock transient
SecurityPage-->>User: reveal WP2FA management panel
User->>SecurityPage: POST remove in-game 2FA
SecurityPage->>ServerInfoApi: POST remove-ingame-2fa
ServerInfoApi->>GameDB: UPDATE account SET totp_secret=NULL
ServerInfoApi-->>SecurityPage: {success}
SecurityPage-->>User: reload page
User->>ConnectionLogger: Login (wp_login hook)
ConnectionLogger->>GameDB: query last_ip
ConnectionLogger-->>ConnectionLogger: INSERT acore_login_history
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/acore-wp-plugin/src/Components/Tools/ToolsApi.php (1)
9-10: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReturn REST errors for invalid item-list requests.
getRestorableItemsByCharacter()now throws for invalid or not-owned characters; without catching that here, a tampered REST request can surface as a 500 instead of a controlled 400/403 response.Proposed fix
public static function ItemRestoreList($request) { - return ACoreServices::I()->getRestorableItemsByCharacter($request['cguid']); + try { + return ACoreServices::I()->getRestorableItemsByCharacter($request['cguid']); + } catch (\InvalidArgumentException $e) { + return new \WP_Error('invalid_character', 'Character not found.', ['status' => 403]); + } }🤖 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 `@src/acore-wp-plugin/src/Components/Tools/ToolsApi.php` around lines 9 - 10, The ItemRestoreList method currently does not handle exceptions thrown by getRestorableItemsByCharacter(), which means invalid or not-owned character requests will result in unhandled exceptions and 500 errors. Wrap the call to getRestorableItemsByCharacter() in a try-catch block to catch any exceptions thrown by that method, and return appropriate REST error responses (such as 400 for invalid requests or 403 for unauthorized/not-owned characters) instead of allowing the exception to propagate and cause a 500 error.src/acore-wp-plugin/src/Hooks/User/User.php (1)
105-111: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftApply the password-history policy to native password-change paths too.
These hooks now update
acore_password_changed_at, but they still sync passwords without enforcingacore_password_is_reused()or recording the old hash. That lets profile edits/admin resets/password resets bypass theallow_old_passwordspolicy enforced by the Security page handler.Also applies to: 126-136
🤖 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 `@src/acore-wp-plugin/src/Hooks/User/User.php` around lines 105 - 111, The password update logic in the User class is bypassing the password history policy enforcement. Before calling setAccountPassword() in the password change handler, you need to check whether the new password is being reused by calling acore_password_is_reused() with the user login and new password. If the function returns true and the allow_old_passwords policy is disabled, prevent the password change and display an appropriate error message to the user. Additionally, before updating the password with setAccountPassword(), capture and record the current password hash by storing it in user metadata (similar to how the old hash should be preserved for the password history policy). This ensures that profile edits, admin password resets, and password reset paths all enforce the same password reuse restrictions as the Security page handler.
🧹 Nitpick comments (2)
src/acore-wp-plugin/src/Utils/AcoreCharColors.php (1)
84-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
$fDarkassignment.Line 88 computes a dark faction color, but the returned style never uses it, so this just creates static-analysis noise and makes the styling contract look broader than it is.
Proposed cleanup
public static function rowStyle(int $classId, int $raceId): string { $cls = self::CLASS_COLORS[$classId] ?? ['light' => self::FALLBACK_LIGHT, 'dark' => self::FALLBACK_DARK]; $faction = self::RACE_FACTION[$raceId] ?? 'unknown'; $fLight = $faction === 'alliance' ? '`#3FACF4`' : '`#FF653D`'; - $fDark = $faction === 'alliance' ? '`#3FACF4`' : '`#FF653D`'; return sprintf(🤖 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 `@src/acore-wp-plugin/src/Utils/AcoreCharColors.php` around lines 84 - 93, In the rowStyle method, the variable $fDark is assigned on line 88 but is never used in the sprintf return statement that follows it. Remove the unused $fDark assignment to clean up unnecessary code and reduce static analysis noise, as only $fLight is utilized in the returned style string.Source: Linters/SAST tools
src/acore-wp-plugin/src/Components/UserPanel/UserController.php (1)
217-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the stale
getModel()return contract.Line 220 now returns the controller itself, but the PHPDoc still advertises
UserModel, which is the contract PHPStan is checking. Update the docblock or return a real model.Proposed fix
/** - * `@return` UserModel + * `@return` self */ public function getModel() { return $this;🤖 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 `@src/acore-wp-plugin/src/Components/UserPanel/UserController.php` around lines 217 - 221, The getModel() method in the UserController class has a PHPDoc that declares `@return` UserModel, but the method actually returns $this (the controller instance), causing a type mismatch that PHPStan will flag. Either update the `@return` docblock to accurately reflect that it returns UserController (self), or modify the implementation to return an actual UserModel instance instead of the controller itself.Source: Linters/SAST tools
🤖 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.
Inline comments:
In `@src/acore-wp-plugin/src/Components/AdminPanel/Pages/PVPRewards.php`:
- Line 17: The nonce field acore_pvp_rewards_nonce added via wp_nonce_field() on
line 17 is being included in the GET request URL when the form is submitted for
Preview (lines 247-251), which leaks the CSRF token via referrers and browser
history. Fix this by splitting the form into two separate forms: one for Preview
that submits as GET without the nonce field, and one for Send that submits as
POST with the wp_nonce_field() call. Alternatively, conditionally exclude the
nonce field when the preview action is triggered, ensuring the nonce is only
sent with POST requests and not exposed in preview URLs.
In `@src/acore-wp-plugin/src/Components/AdminPanel/Pages/Tools.php`:
- Around line 180-186: The hidden input field for acore_geoip_lookup is only
created when Security Logging is disabled (inside the conditional if
acore_security_logging != '1'). When Security Logging is enabled and then later
disabled via JavaScript, the select element gets disabled and won't submit any
value, allowing the old GeoIP setting to persist. To fix this, ensure that a
fallback hidden input with acore_geoip_lookup value of 0 is ALWAYS submitted
regardless of the Security Logging state. Remove the conditional wrapper around
the hidden input so it is always present in the form, guaranteeing that a
default value of 0 will be submitted even when the select element is disabled.
- Around line 255-262: When all rows are deleted from the
acore-name-unlock-thresholds table, the form submission does not include the
acore_name_unlock_thresholds key, causing SettingsController::loadTools() to
skip updating it and leaving stale data in the database. Add a hidden marker
field (acore_name_unlock_thresholds_present) to the table in Tools.php that
indicates the thresholds section was submitted, then in
SettingsController::loadTools() check for this marker before the option update
loop and explicitly clear the acore_name_unlock_thresholds option by calling
storeConf with an empty array if the marker exists but the thresholds key is
absent in $_POST. Apply the same pattern to the other affected tables noted in
the comment (lines 588-590).
In `@src/acore-wp-plugin/src/Components/MailReturnMenu/MailReturnView.php`:
- Around line 58-59: The src attributes in the race-icon and class-icon img tags
are not escaped with esc_url(), creating a security inconsistency with other
character views. Wrap both src attribute values (the concatenated URL strings
for ACORE_URL_PLG . 'web/assets/race/...' and ACORE_URL_PLG .
'web/assets/class/...') with the esc_url() function to properly escape the URLs
before they are output in the HTML.
In `@src/acore-wp-plugin/src/Components/ServerInfo/ServerInfoApi.php`:
- Around line 350-353: The issue is that checking if affected rows equals zero
after the UPDATE statement in the executeStatement call is unreliable for
determining account existence, because updating totp_secret to NULL returns 0
affected rows when the account exists but the field is already NULL. Instead of
relying on the affected rows count, first verify the account exists using a
separate SELECT query (checking if the account with the given username exists),
and only if the account is confirmed to exist, then proceed with the UPDATE
statement. This way the account existence check is independent from whether the
field was actually changed, preventing false "No game account" errors from
double-clicks or race conditions.
- Around line 200-208: The code calls ServerInfoApi::serverInfo() and
immediately uses stripos() to scan the result against error patterns, but if
serverInfo() throws an exception or returns a non-scalar value (like an
Exception object), stripos() will fail fatally instead of gracefully returning a
503 response. Wrap the ServerInfoApi::serverInfo() call in a try-catch block to
handle any exceptions, and normalize the result to ensure it is a scalar string
value before passing it to the stripos() calls in the foreach loop that scans
for error patterns.
- Around line 447-456: The fresh TOTP code validation path in the in-game 2FA
removal endpoint lacks attempt rate limiting, allowing unlimited brute-force
attacks on the token validation. Apply the same 2FA attempt limiter mechanism
that is used in the verify-website-2fa endpoint to this code path. Specifically,
before and after validating the token with the acore_wp2fa_code_is_valid()
function, implement attempt tracking that increments on failed validation
(including invalid token format checks) and blocks further attempts after a
threshold is reached, returning an appropriate error when the limit is exceeded.
- Around line 124-136: The functions acore_website_totp_enabled() and
acore_website_2fa_enabled() do not verify that the WP2FA plugin is actually
active before reading and processing its metadata. This causes stale
wp_2fa_enabled_methods and wp_2fa_totp_key metadata from a disabled plugin to
incorrectly report 2FA as enabled. Both functions should check if the WP2FA
plugin is active at the start and return false immediately if it is not
available, ensuring that password changes are not blocked when the plugin is
inactive.
In `@src/acore-wp-plugin/src/Components/Tools/ToolsApi.php`:
- Around line 14-22: The function validates that the item ID is a valid integer
and that the character name belongs to the current user, but it does not verify
that the recovery item ID actually belongs to that character or account before
executing the restore command. Add an authorization check after the
currentAccountOwnsCharacterName validation and before the executeCommand call to
verify that the recovery item specified by the item variable is owned by or
associated with the character identified by cname, returning a WP_Error with
appropriate status code if the authorization fails.
In `@src/acore-wp-plugin/src/Components/UnstuckMenu/UnstuckView.php`:
- Line 62: The hearthstone image URL on line 62 in the img src attribute is
echoing the asset path directly without escaping, while adjacent race and class
icon URLs use esc_url() for security. Wrap the ACORE_URL_PLG constant and the
hearthstone.jpg path with the esc_url() function to match the security pattern
used for the other icon URLs in the same file.
In `@src/acore-wp-plugin/src/Components/UserPanel/Pages/ItemRestorationPage.php`:
- Around line 8-16: The translation domain parameter in the _e() function calls
throughout the ItemRestorationPage.php file is using Opts::I()->page_alias,
which is incorrect. The Opts class exposes acore_page_alias as the proper
translation domain constant. Replace all instances of Opts::I()->page_alias with
acore_page_alias in all _e() function calls across the file (at lines 8, 20, 42,
54, and 66 as mentioned in the review comment) to ensure the translations
resolve with the correct domain.
- Around line 114-123: The selectCharacter function initiates fetch requests
that can race when users click characters quickly, allowing stale responses from
older requests to overwrite newer selections. Implement request sequencing by
storing a request identifier (such as a timestamp or counter) at the start of
selectCharacter, and before processing the response in the then handlers (where
the grid is updated around lines 120-202), verify that the current response
matches the latest request identifier. Only proceed with updating the grid and
content if the response is from the most recent request; otherwise, ignore it as
stale data. This ensures that out-of-order responses from slower network
requests do not corrupt the current character's item list.
In `@src/acore-wp-plugin/src/Components/UserPanel/Pages/SecurityPage.php`:
- Around line 220-267: The security issue is that the $wp2faHtml containing
sensitive 2FA settings and backup codes is being rendered in the page source
with display:none CSS in both the websiteTotpEnabled and websiteEmailEnabled
branches, making it accessible by viewing source code before verification.
Remove the hidden acore-2fa-panel div containing $wp2faHtml from both the elseif
($websiteTotpEnabled) and elseif ($websiteEmailEnabled) conditional blocks
entirely. Instead, after successful TOTP or email verification (when the unlock
transient is set server-side), either reload the page so $twofaUnlocked becomes
true and the panel renders, or fetch the panel content via AJAX and inject it
into the DOM only after successful verification. This ensures sensitive content
is never included in the initial page source until the user has been properly
authenticated.
In `@src/acore-wp-plugin/src/Components/UserPanel/UserController.php`:
- Around line 54-65: The IP detection logic in the RAF abuse check section of
UserController.php uses spoofable headers (HTTP_CLIENT_IP and
HTTP_X_FORWARDED_FOR) as the primary source, allowing users to bypass the
same-IP RAF block. Replace this insecure IP detection with the trusted-proxy
resolver that is already used elsewhere in the codebase for connection logging,
and fall back to REMOTE_ADDR as the trusted source. Specifically, in the block
where check_ip is enabled and $activeUserIp is being determined, use the
existing trusted-proxy resolver instead of checking the client-supplied headers
first.
In `@src/acore-wp-plugin/src/Hooks/User/ConnectionLogger.php`:
- Around line 15-24: The ConnectionLogger class creates a table that stores user
IP history indefinitely without any data retention mechanism. Add a
configuration option to define a retention period (in days) for login records,
then implement a scheduled cleanup method (likely using WordPress cron hooks)
that runs periodically to delete login_at records older than the retention
period. This retention logic should be applied to the table creation in the
ConnectionLogger class constructor and referenced at the other locations
mentioned (lines 44-54, 94-104) to ensure consistent data minimization practices
across all table operations.
In `@src/acore-wp-plugin/src/Hooks/User/User.php`:
- Around line 760-764: The code that displays the "Website 2FA removed" message
in the printf statement does not handle self-removal entries properly. When a
2FA removal is marked as self-removal (with by => self), the lastWebRemoval
array contains no staff key, which causes an undefined-index warning and
displays incorrect attribution. Add a conditional check before the printf
statement to determine if the removal was self-initiated or by staff, and
display appropriate text accordingly. For self-removals, display a message
indicating the user removed it themselves, and for staff removals, use the
existing message that includes the staff name. This ensures the warning displays
correct information without undefined-index errors regardless of removal type.
In `@src/acore-wp-plugin/web/assets/mail-return/mail-return.js`:
- Around line 28-33: The issue is that the return-success cleanup code only
shows the child message but doesn't hide the content wrapper or show the empty
wrapper when the final mail is returned. Find the return-success cleanup handler
(the code that executes after a mail is successfully returned) and mirror the
empty-state logic from the load path: after handling the successful return,
check if there are no more mails remaining, and if so, hide the content wrapper
and show the emptyWrap element, similar to how it's done in the conditional
check at the start of the load path.
- Around line 60-63: The factionClr function receives raceId values that may be
strings from the REST API, but the allianceRaces array contains numbers, causing
indexOf to always fail and return incorrect faction colors. Convert the raceId
parameter to a number using parseInt, Number conversion, or the unary plus
operator before performing the indexOf comparison against the allianceRaces
array to ensure proper type matching.
---
Outside diff comments:
In `@src/acore-wp-plugin/src/Components/Tools/ToolsApi.php`:
- Around line 9-10: The ItemRestoreList method currently does not handle
exceptions thrown by getRestorableItemsByCharacter(), which means invalid or
not-owned character requests will result in unhandled exceptions and 500 errors.
Wrap the call to getRestorableItemsByCharacter() in a try-catch block to catch
any exceptions thrown by that method, and return appropriate REST error
responses (such as 400 for invalid requests or 403 for unauthorized/not-owned
characters) instead of allowing the exception to propagate and cause a 500
error.
In `@src/acore-wp-plugin/src/Hooks/User/User.php`:
- Around line 105-111: The password update logic in the User class is bypassing
the password history policy enforcement. Before calling setAccountPassword() in
the password change handler, you need to check whether the new password is being
reused by calling acore_password_is_reused() with the user login and new
password. If the function returns true and the allow_old_passwords policy is
disabled, prevent the password change and display an appropriate error message
to the user. Additionally, before updating the password with
setAccountPassword(), capture and record the current password hash by storing it
in user metadata (similar to how the old hash should be preserved for the
password history policy). This ensures that profile edits, admin password
resets, and password reset paths all enforce the same password reuse
restrictions as the Security page handler.
---
Nitpick comments:
In `@src/acore-wp-plugin/src/Components/UserPanel/UserController.php`:
- Around line 217-221: The getModel() method in the UserController class has a
PHPDoc that declares `@return` UserModel, but the method actually returns $this
(the controller instance), causing a type mismatch that PHPStan will flag.
Either update the `@return` docblock to accurately reflect that it returns
UserController (self), or modify the implementation to return an actual
UserModel instance instead of the controller itself.
In `@src/acore-wp-plugin/src/Utils/AcoreCharColors.php`:
- Around line 84-93: In the rowStyle method, the variable $fDark is assigned on
line 88 but is never used in the sprintf return statement that follows it.
Remove the unused $fDark assignment to clean up unnecessary code and reduce
static analysis noise, as only $fLight is utilized in the returned style string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 044ea6ab-4d9c-4513-9abe-e55fc6d5e4b2
📒 Files selected for processing (35)
src/acore-wp-plugin/src/Components/AdminPanel/Pages/ElunaSettings.phpsrc/acore-wp-plugin/src/Components/AdminPanel/Pages/PVPRewards.phpsrc/acore-wp-plugin/src/Components/AdminPanel/Pages/RealmSettings.phpsrc/acore-wp-plugin/src/Components/AdminPanel/Pages/Tools.phpsrc/acore-wp-plugin/src/Components/AdminPanel/SettingsController.phpsrc/acore-wp-plugin/src/Components/CharactersMenu/CharactersController.phpsrc/acore-wp-plugin/src/Components/CharactersMenu/CharactersView.phpsrc/acore-wp-plugin/src/Components/MailReturnMenu/MailReturnController.phpsrc/acore-wp-plugin/src/Components/MailReturnMenu/MailReturnView.phpsrc/acore-wp-plugin/src/Components/ResurrectionScrollMenu/ResurrectionScrollMenu.phpsrc/acore-wp-plugin/src/Components/ResurrectionScrollMenu/ResurrectionScrollView.phpsrc/acore-wp-plugin/src/Components/ServerInfo/ServerInfoApi.phpsrc/acore-wp-plugin/src/Components/Tools/ToolsApi.phpsrc/acore-wp-plugin/src/Components/UnstuckMenu/UnstuckView.phpsrc/acore-wp-plugin/src/Components/UserPanel/Pages/ItemRestorationPage.phpsrc/acore-wp-plugin/src/Components/UserPanel/Pages/RafProgressPage.phpsrc/acore-wp-plugin/src/Components/UserPanel/Pages/SecurityPage.phpsrc/acore-wp-plugin/src/Components/UserPanel/UserController.phpsrc/acore-wp-plugin/src/Components/UserPanel/UserMenu.phpsrc/acore-wp-plugin/src/Components/UserPanel/UserView.phpsrc/acore-wp-plugin/src/Hooks/User/ConnectionLogger.phpsrc/acore-wp-plugin/src/Hooks/User/Include.phpsrc/acore-wp-plugin/src/Hooks/User/PasswordHandler.phpsrc/acore-wp-plugin/src/Hooks/User/User.phpsrc/acore-wp-plugin/src/Hooks/Various/DarkMode.phpsrc/acore-wp-plugin/src/Hooks/Various/tgmplugin_activator.phpsrc/acore-wp-plugin/src/Manager/ACoreServices.phpsrc/acore-wp-plugin/src/Manager/Opts.phpsrc/acore-wp-plugin/src/Manager/Soap/SmartstoneService.phpsrc/acore-wp-plugin/src/Utils/AcoreCharColors.phpsrc/acore-wp-plugin/src/boot.phpsrc/acore-wp-plugin/web/assets/css/dark-mode.csssrc/acore-wp-plugin/web/assets/css/main.csssrc/acore-wp-plugin/web/assets/css/theme.csssrc/acore-wp-plugin/web/assets/mail-return/mail-return.js
TheSCREWEDSoftware
added a commit
to TheSCREWEDSoftware/acore-cms
that referenced
this pull request
Jun 24, 2026
(Resume generated by Claude) **PR azerothcore#199 — Characters Menu** - Fixed typo: "succesfully" → "successfully" in save confirmation - Fixed broken sentence in the Order Character Screen description - Extended `user-select: none` to cover all `.acore-char-row` elements, not just `button` **PR azerothcore#200 — Mail Return** - *(issues covered by later PRs — no unique fixes)* **PR azerothcore#201 — Item Restoration** - Added `alt=` attributes to race/class icons in `CharactersView.php` - `ToolsApi`: `ItemRestoreList` now catches exceptions and returns a 403 instead of a 500 - `ToolsApi` + `ACoreServices`: **Security fix** — `ItemRestore` now verifies the recovery item belongs to the character, not just that the user owns the character name (IDOR vulnerability) **PR azerothcore#202 — Unstuck / Scroll / RAF** - `ItemRestorationPage`: removed brittle `data.includes('mail')` success check — any HTTP success now triggers the success path **PR azerothcore#203 — Security Tab** - `CharactersController`: `resetCharacterOrder()` now guards against invalid/null account ID before running UPDATE - `Tools.php`: added `esc_attr()` on banned-names table input (XSS fix) - `Tools.php`: added `is_array()` guard on thresholds `foreach` - `Tools.php` + `SettingsController`: added sentinel input so deleting all thresholds actually clears them in the DB - `Tools.php`: GeoIP hidden fallback `value="0"` is now always rendered, not conditionally **PR azerothcore#204 — Admin Settings / CSRF** - `AcoreCharColors`: removed unused `$fDark`, unknown race IDs now get a neutral color instead of Horde red - `MailReturnView`: added `esc_url()` on race/class icon `src` attributes - `DarkMode`: added `dmToggleInFlight` flag to prevent overlapping AJAX requests on rapid dark mode clicks
Contributor
Author
|
The reviews are valid for this, latest PR #205 PR #204 — Admin Settings / CSRF
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split from the main PR: #197
Split by Claude
Realm Settings revamp: SOAP status badge (online/offline on save), live module list from the server, and module-requirement mapping that hides a page when its module is missing. Tools page gets the Web Integration group (security logging, allow-old-passwords, 2FA admin controls, IP-history lookup), Name-Unlock delete buttons moved inside the box + a reset-to-default, and CSRF nonces added to all settings forms (Realm, Tools, Eluna, PvP).
Summary by CodeRabbit
New Features
Bug Fixes