Upd. Firewall. Emergency bypass logic updated to token-via-email model. - #739
Open
alexandergull wants to merge 2 commits into
Open
alexandergull wants to merge 2 commits into
alexandergull wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There is a confirmed race-condition risk in token cleanup (concurrent generation calls can invalidate each other), and there are a couple of integration/API consistency issues that should be corrected before merge.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR introduces an emergency firewall bypass flow that generates a one-time token via remote call, emails an activation link to the site admin, and then grants a time-limited bypass to the browser that opens the link (via an HttpOnly cookie secret whose hash is stored server-side).
Changes:
- Added
FirewallBypassimplementation (token generation, email delivery, cookie grant, and security-log auditing) and integrated it into firewall skip logic. - Added a new remote call endpoint (
send_fw_bypass_email) with a cooldown entry inState. - Added PHPUnit coverage for the bypass flow and added a security-log label for the new
fw_bypassevent.
File summaries
| File | Description |
|---|---|
| tests/lib/CleantalkSP/SpbctWP/Firewall/FirewallBypassTest.php | Adds tests covering token generation, email link parsing, single-use semantics, cookie validation, and logging. |
| lib/CleantalkSP/SpbctWP/State.php | Adds cooldown configuration for the new remote call. |
| lib/CleantalkSP/SpbctWP/RemoteCalls.php | Adds the send_fw_bypass_email remote call action that triggers bypass-link generation. |
| lib/CleantalkSP/SpbctWP/Firewall/FirewallBypass.php | Implements the new emergency bypass flow (token, email, cookie grant, hash verification, logging). |
| inc/spbc-tools.php | Adds human-readable formatting for the new fw_bypass security-log event. |
| inc/spbc-firewall.php | Integrates the bypass grant/check into firewall skip logic and keeps legacy bypass for transition. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+96
to
+103
| } catch (\Exception $e) { | ||
| // Drop the token only if it has been created by this very call. | ||
| if ( $ready_token_saved ) { | ||
| self::removeReadyToken(); | ||
| } | ||
| self::$last_error = $e->getMessage(); | ||
| return false; | ||
| } |
Comment on lines
+704
to
+714
| * | ||
| * @return void Dies with 'OK' or with 'FAIL {"error":"..."}'. | ||
| * @psalm-suppress PossiblyUnusedMethod | ||
| */ | ||
| public static function action__send_fw_bypass_email() // phpcs:ignore PSR1.Methods.CamelCapsMethodName.NotCamelCaps | ||
| { | ||
| $result = FirewallBypass::processGenerationRemoteCall(); | ||
| if ( ! $result ) { | ||
| $error_message = FirewallBypass::$last_error ?? 'Unknown error'; | ||
| die('FAIL ' . json_encode(['error' => $error_message])); | ||
| } |
Comment on lines
+278
to
281
| Firewall\FirewallBypass::maybeSetUserToken(); | ||
| if (Firewall\FirewallBypass::bypassByUserToken()) { | ||
| return true; | ||
| } |
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
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.
https://app.doboard.com/1/task/55234