-
Notifications
You must be signed in to change notification settings - Fork 1
Secure “Login via Cashtab” Flow (Server-Verified Login Tokens) #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughFrontend now captures PayButton transaction data and defers validation to server-side endpoints; backend adds login tracking table and AJAX validation endpoints for login/unlock flows; plugin ensures DB tables exist on load; payment unlock flow now validates tokens server-side before finalizing and fetching content. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant PayButton
participant WP_Server
participant DB
User->>Browser: Click "Login" / "Pay"
Browser->>PayButton: Open (opReturn:'login' or unlock)
PayButton-->>Browser: onSuccess(address, tx)
Note over Browser: store loginAddr/unlockAddr and loginTx/unlockTx (hash,timestamp)
PayButton-->>Browser: onClose
rect rgb(220,235,255)
Browser->>WP_Server: POST validate_login_tx / validate_unlock_tx (wallet_address, tx_hash[, post_id])
WP_Server->>DB: find unused matching row
DB-->>WP_Server: row found / not found
alt row found
WP_Server->>DB: generate token, set used=1
DB-->>WP_Server: token stored
WP_Server-->>Browser: { success, token }
else not found
WP_Server-->>Browser: { error }
end
end
rect rgb(220,255,230)
Browser->>WP_Server: handleLogin / mark_payment_successful (address, txHash, token, post_id)
WP_Server->>DB: verify token + row, save address or unlock content access
DB-->>WP_Server: success
WP_Server-->>Browser: success
end
Browser->>User: Show logged-in / unlocked content (fetchUnlocked)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas needing extra attention:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
paybutton.php (1)
48-52: Guardcreate_tables()so dbDelta doesn’t run on every requestCalling
PayButton_Activator::create_tables()on everyplugins_loadedrun guarantees schema upgrades but also runsdbDelta()on every page load, which can be non‑trivial on busy sites. Consider gating this behind a stored schema/version option (e.g.,paybutton_db_version) and only callingcreate_tables()when the stored version is older than the code’s version, or when an explicit upgrade routine runs. That keeps the upgrade safety while avoiding per‑request DDL checks.assets/js/paybutton-paywall-cashtab-login.js (1)
51-106: HardentryValidateLoginagainst network/HTTP failuresThe retry logic for
validate_login_txcovers application‑level failures (resp.success === false), but there’s no error callback for network or HTTP‑level errors, so a 500/timeout would silently drop the flow without retry or user feedback.Consider switching this block to
jQuery.ajaxand treatingerrorthe same as the “validation failed” branch:- function tryValidateLogin(attempt) { - jQuery.post( - PaywallAjax.ajaxUrl, - { - action: 'validate_login_tx', - security: PaywallAjax.nonce, - wallet_address: addrCopy, - tx_hash: hashCopy - }, - function (resp) { - if (resp && resp.success && resp.data && resp.data.login_token) { - // Pass the random token from the server - handleLogin(addrCopy, hashCopy, resp.data.login_token); - } else { - if (attempt === 1) { - // Retry once again after 3 seconds - setTimeout(() => tryValidateLogin(2), 3000); - } else { - alert('⚠️ Login failed: Invalid or expired transaction.'); - } - } - } - ); - } + function tryValidateLogin(attempt) { + jQuery.ajax({ + url: PaywallAjax.ajaxUrl, + method: 'POST', + data: { + action: 'validate_login_tx', + security: PaywallAjax.nonce, + wallet_address: addrCopy, + tx_hash: hashCopy + }, + success: function (resp) { + if (resp && resp.success && resp.data && resp.data.login_token) { + handleLogin(addrCopy, hashCopy, resp.data.login_token); + return; + } + if (attempt === 1) { + setTimeout(() => tryValidateLogin(2), 3000); + } else { + alert('⚠️ Login failed: Invalid or expired transaction.'); + } + }, + error: function () { + if (attempt === 1) { + setTimeout(() => tryValidateLogin(2), 3000); + } else { + alert('⚠️ Login failed: Network or server error.'); + } + } + }); + }includes/class-paybutton-ajax.php (2)
83-99: Clean up login branch logging and unreachable codeThe new login-specific branch in
payment_trigger()looks sound: it short‑circuits unlock logic, enforces presence ofuser_address/tx_hash/tx_timestamp, and records intopaybutton_loginswith an idempotency check, which fits the new flow.Two small nits:
- There’s a stray
global $wpdb;immediately afterwp_send_json_success(['message' => 'Login tx recorded']);followed by areturn;— thatglobalis unreachable and can be removed.- The multiple
error_log()calls (payment_trigger hit,signature ok,rawMessage=...,login-branch..., insert error/ok) are useful while rolling out but may be noisy in production logs.You could trim or gate them behind a debug flag, e.g. only log when
WP_DEBUGor apaybutton_debugoption is enabled.Example for the unreachable
global:- wp_send_json_success(['message' => 'Login tx recorded']); - global $wpdb; - return; + wp_send_json_success(['message' => 'Login tx recorded']); + return;Also applies to: 120-171
228-268: Token semantics: currently one-time per tx row, but reusable across browser sessions
ajax_validate_login_tx()correctly enforces “unused row only” (used = 0), generates a high‑entropy token, and flipsusedto1while attachinglogin_token.save_address()then requires a row with matchingwallet_address,tx_hash,login_token, andused = 1before it will set the cookie.This achieves the main goal (you can’t fake a login from the browser without a PayButton‑recorded tx), but note that the token is not consumed at
save_address: once a row is inused = 1with alogin_token, any future POST with the same triplet will still succeed. That makes the token “one‑time per db row” (at validation) but effectively reusable for re‑logins.If you truly want a strictly single‑use token that cannot be replayed later (even by the same browser), you could consume it after a successful
save_address():if (!$row) { wp_send_json_error(['message' => 'No validated login found for this token']); } + // Optionally consume the token so it cannot be reused once the address + // has been applied to the current browser/session. + /* + $wpdb->update( + $login_table, + array( + 'used' => 2, // 0 = new, 1 = validated, 2 = consumed + 'login_token' => '', + ), + array('id' => (int) $row->id), + array('%d','%s'), + array('%d') + ); + */ + // Retrieve the blacklist and check the address $blacklist = get_option( 'paybutton_blacklist', array() );That’s optional — the current behavior already blocks client‑side forgery — but worth considering if you want tokens to be strictly non‑replayable.
Also applies to: 469-521
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
assets/js/paybutton-paywall-cashtab-login.js(4 hunks)includes/class-paybutton-activator.php(3 hunks)includes/class-paybutton-ajax.php(7 hunks)paybutton.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
paybutton.php (1)
includes/class-paybutton-activator.php (2)
PayButton_Activator(12-110)create_tables(38-80)
assets/js/paybutton-paywall-cashtab-login.js (1)
assets/js/paybutton-generator.js (1)
address(21-21)
includes/class-paybutton-ajax.php (1)
includes/class-paybutton-state.php (1)
set_address(97-134)
🔇 Additional comments (2)
assets/js/paybutton-paywall-cashtab-login.js (1)
5-18: LGTM: client now submits address, tx hash, and login tokenThe updated
handleLogin(address, txHash, loginToken)correctly posts all three fields expected by the hardenedpaybutton_save_addressendpoint, keeping the browser from unilaterally declaring a login without a server-issued token.includes/class-paybutton-activator.php (1)
42-80: LGTM: login table schema matches the new validation flowThe new
paybutton_loginsDDL looks consistent with how it’s used inPayButton_AJAX: you index bytx_hash,wallet_address(190),used, andlogin_token, which matches the SELECT/UPDATE patterns, and usingDECIMAL(20,2)fortx_amountaligns with the hard‑coded 5.5 XEC login price. Reusingcreate_tables()for both activation and upgrades keeps the schema centralized.
|
Ignoring CodeRabbit's feedback. Not useful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a secure server-verified login flow for the "Login via Cashtab" feature, replacing the previous client-side authentication that could be easily spoofed. The new implementation requires users to complete a payment transaction, after which the server generates a one-time token that must be validated before login is granted.
Key Changes:
- Server-side validation of login transactions with one-time random tokens
- New database table (
paybutton_logins) to track login transactions and prevent replay attacks - Client-side retry mechanism for login validation with proper token handling
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| paybutton.php | Ensures database tables are created/updated on plugin load |
| includes/class-paybutton-ajax.php | Implements login transaction recording, token generation, and validation endpoints |
| includes/class-paybutton-activator.php | Adds new paybutton_logins table schema for tracking login transactions |
| assets/js/paybutton-paywall-cashtab-login.js | Updates client-side login flow to validate transactions and pass tokens to server |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Ready to merge |
…ecurity Secure “Content Unlocking” Flow (Server-Verified Login Tokens)
Klakurka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should start writing tests for a lot of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
includes/class-paybutton-activator.php (1)
50-50: DECIMAL(20,2) precision may truncate cryptocurrency amounts.Cryptocurrencies like eCash typically require more than 2 decimal places for accurate representation. Both
tx_amountcolumns useDECIMAL(20,2), which will lose fractional precision. Consider usingDECIMAL(20,8)or higher to preserve sub-cent amounts.Also applies to: 72-72
🧹 Nitpick comments (4)
includes/class-paybutton-activator.php (1)
68-76: Consider aligningtx_timestampcolumn types between tables.The
paybutton_loginstable usesINT(11)fortx_timestamp(Unix epoch), whilepaybutton_paywall_unlockedusesDATETIME. While this works because the PHP code handles conversion differently for each table, using a consistent type would simplify maintenance and querying.includes/class-paybutton-ajax.php (2)
240-259: Add explicitreturnafterwp_send_json_errorcalls for clarity.While
wp_send_json_errorinternally callswp_die()and terminates execution, adding explicitreturnstatements improves code clarity and provides defense against future WordPress changes.if (!$address || !$tx_hash || !$login_token) { wp_send_json_error(['message' => 'Missing address, tx_hash, or login_token']); + return; } ... if (!$row) { wp_send_json_error(['message' => 'No validated login found for this token']); + return; }
301-336: Add explicitreturnstatements after error responses.Same as in
save_address(), add explicitreturnafterwp_send_json_errorcalls for defensive coding:if ( $post_id <= 0 || empty( $tx_hash ) || empty( $user_address ) || empty( $unlock_token ) ) { wp_send_json_error( array( 'message' => 'Missing required payment fields.' ), 400 ); + return; } ... if ( ! $row ) { wp_send_json_error( array( 'message' => 'No validated unlock found for this token.' ), 403 ); + return; } ... if ( in_array( $user_address, $blacklist, true ) ) { wp_send_json_error( array( 'message' => 'This wallet address is blocked.' ) ); + return; }assets/js/paywalled-content.js (1)
42-73: Consider adding error handling for content fetch.If
fetch_unlocked_contentfails, the user sees no feedback. Since the unlock was already marked successful, users can refresh the page to see content, but adding an error handler would improve UX.jQuery.ajax({ method: 'POST', url: PaywallAjax.ajaxUrl, data: { action: 'fetch_unlocked_content', post_id: configData.postId, security: PaywallAjax.nonce }, success: function (resp) { // ... existing success handling - } + }, + error: function () { + // Content is unlocked; refresh will show it + location.reload(); + } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
assets/js/paywalled-content.js(1 hunks)includes/class-paybutton-activator.php(2 hunks)includes/class-paybutton-ajax.php(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
includes/class-paybutton-ajax.php (1)
includes/class-paybutton-state.php (3)
set_address(97-134)add_article(187-227)get_address(139-150)
assets/js/paywalled-content.js (1)
assets/js/paybutton-paywall-cashtab-login.js (2)
addrCopy(74-74)hashCopy(75-75)
🔇 Additional comments (1)
assets/js/paywalled-content.js (1)
78-98: Solid implementation of the server-verified unlock flow.The separation of concerns between
onSuccess(capture tx data) andonClose(validate + mark + fetch) correctly implements the server-verified pattern. The use of local copies before async operations prevents race conditions with state clearing.
| jQuery.ajax({ | ||
| method: 'POST', | ||
| url: PaywallAjax.ajaxUrl, | ||
| data: { | ||
| action: 'mark_payment_successful', | ||
| post_id: postIdCopy, | ||
| security: PaywallAjax.nonce, | ||
| tx_hash: hashCopy, | ||
| tx_amount: amtCopy, | ||
| tx_timestamp: tsCopy, | ||
| user_address: addrCopy, | ||
| unlock_token: resp.data.unlock_token | ||
| }, | ||
| success: function () { | ||
| // Finally, fetch and render the unlocked content | ||
| fetchUnlocked(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for mark_payment_successful AJAX call.
The mark_payment_successful call has no error handler. If this request fails (e.g., network issue, server error), the user receives no feedback despite having a valid token. The payment would be validated but the content wouldn't unlock.
jQuery.ajax({
method: 'POST',
url: PaywallAjax.ajaxUrl,
data: {
action: 'mark_payment_successful',
post_id: postIdCopy,
security: PaywallAjax.nonce,
tx_hash: hashCopy,
tx_amount: amtCopy,
tx_timestamp: tsCopy,
user_address: addrCopy,
unlock_token: resp.data.unlock_token
},
success: function () {
fetchUnlocked();
- }
+ },
+ error: function () {
+ alert('⚠️ Failed to complete unlock. Please refresh the page and try again.');
+ }
});🤖 Prompt for AI Agents
In assets/js/paywalled-content.js around lines 121 to 138, the AJAX call to
action 'mark_payment_successful' lacks error handling; add an error (and/or
complete) callback to handle network/server failures by logging the error,
showing a user-facing message (e.g., "Unable to mark payment; retrying/unlock
may be delayed"), and ensuring the unlocked content is still attempted (call
fetchUnlocked() on success and consider calling it or queuing a retry in
error/complete so a valid unlock_token isn't wasted); ensure you include the
jqXHR/textStatus/errorThrown info in logs and avoid silent failures.
| // Idempotency: avoid dupes on replays | ||
| $exists = $wpdb->get_var( $wpdb->prepare( | ||
| "SELECT id FROM {$login_table} WHERE wallet_address = %s AND tx_hash = %s LIMIT 1", | ||
| $user_address, $tx_hash | ||
| ) ); | ||
| //error_log('[paybutton] login-branch addr=' . $user_address . ' tx=' . $tx_hash . ' ts=' . $tx_timestamp); | ||
| if ( ! $exists ) { | ||
| $wpdb->insert( | ||
| $login_table, | ||
| array( | ||
| 'wallet_address' => $user_address, | ||
| 'tx_hash' => $tx_hash, | ||
| 'tx_amount' => (float) $tx_amount, | ||
| 'tx_timestamp' => (int) $tx_timestamp, | ||
| 'used' => 0, | ||
| ), | ||
| array('%s','%s','%f','%d','%d') | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential duplicate rows due to race condition in idempotency check.
The SELECT-then-INSERT pattern has a TOCTOU (time-of-check to time-of-use) race condition. If two webhook requests arrive simultaneously with the same wallet_address and tx_hash, both could pass the $exists check and insert duplicate rows.
Consider adding a unique index on (wallet_address, tx_hash) to the paybutton_logins table and using INSERT IGNORE or ON DUPLICATE KEY UPDATE:
- $exists = $wpdb->get_var( $wpdb->prepare(
- "SELECT id FROM {$login_table} WHERE wallet_address = %s AND tx_hash = %s LIMIT 1",
- $user_address, $tx_hash
- ) );
- //error_log('[paybutton] login-branch addr=' . $user_address . ' tx=' . $tx_hash . ' ts=' . $tx_timestamp);
- if ( ! $exists ) {
- $wpdb->insert(
- $login_table,
- array(
- 'wallet_address' => $user_address,
- 'tx_hash' => $tx_hash,
- 'tx_amount' => (float) $tx_amount,
- 'tx_timestamp' => (int) $tx_timestamp,
- 'used' => 0,
- ),
- array('%s','%s','%f','%d','%d')
- );
- }
+ // Use INSERT IGNORE to safely handle concurrent duplicate webhooks
+ $wpdb->query( $wpdb->prepare(
+ "INSERT IGNORE INTO {$login_table}
+ (wallet_address, tx_hash, tx_amount, tx_timestamp, used)
+ VALUES (%s, %s, %f, %d, %d)",
+ $user_address, $tx_hash, (float) $tx_amount, (int) $tx_timestamp, 0
+ ) );This requires adding a unique index in the activator:
UNIQUE KEY wallet_tx_idx (wallet_address(190), tx_hash)🤖 Prompt for AI Agents
In includes/class-paybutton-ajax.php around lines 147 to 165, the
SELECT-then-INSERT creates a TOCTOU race that can produce duplicate rows; add a
unique constraint on (wallet_address, tx_hash) in the plugin activator (e.g.
create UNIQUE KEY wallet_tx_idx (wallet_address(190), tx_hash)) and change the
insertion logic to use a collision-safe SQL pattern (INSERT IGNORE or INSERT ...
ON DUPLICATE KEY UPDATE) or wrap the insert in a single atomic query so
concurrent requests cannot create duplicates.
| $row = $wpdb->get_row($wpdb->prepare( | ||
| "SELECT id FROM {$table} | ||
| WHERE wallet_address = %s AND tx_hash = %s AND used = 0 | ||
| ORDER BY id DESC LIMIT 1", | ||
| $wallet_address, $tx_hash | ||
| )); | ||
|
|
||
| if (!$row) { | ||
| wp_send_json_error('Login validation failed'); // no match or already used | ||
| } | ||
|
|
||
| // Generate a random, unguessable token like "9fx0..._..." so that malicious actors | ||
| // can't fake login attempts by reusing the same wallet address + tx hash using fake | ||
| // AJAX calls from the browser. | ||
| $raw = random_bytes(18); // 18 bytes → ~24 chars base64url | ||
| $token = rtrim(strtr(base64_encode($raw), '+/', '-_'), '='); | ||
|
|
||
| // Mark as used + attach token | ||
| $wpdb->update( | ||
| $table, | ||
| array( | ||
| 'used' => 1, | ||
| 'login_token' => $token, | ||
| ), | ||
| array('id' => (int)$row->id), | ||
| array('%d','%s'), | ||
| array('%d') | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Race condition allows token overwrite; use atomic conditional UPDATE.
If two concurrent requests validate the same login transaction, both could read used=0, then both issue UPDATE statements. The second UPDATE would overwrite the token generated by the first, potentially allowing an attacker to steal a login session.
Use an atomic UPDATE with a WHERE condition and check affected rows:
- $row = $wpdb->get_row($wpdb->prepare(
- "SELECT id FROM {$table}
- WHERE wallet_address = %s AND tx_hash = %s AND used = 0
- ORDER BY id DESC LIMIT 1",
- $wallet_address, $tx_hash
- ));
-
- if (!$row) {
- wp_send_json_error('Login validation failed'); // no match or already used
- }
-
// Generate a random, unguessable token ...
$raw = random_bytes(18);
$token = rtrim(strtr(base64_encode($raw), '+/', '-_'), '=');
- // Mark as used + attach token
- $wpdb->update(
- $table,
- array(
- 'used' => 1,
- 'login_token' => $token,
- ),
- array('id' => (int)$row->id),
- array('%d','%s'),
- array('%d')
+ // Atomically claim an unused row
+ $affected = $wpdb->query($wpdb->prepare(
+ "UPDATE {$table}
+ SET used = 1, login_token = %s
+ WHERE wallet_address = %s AND tx_hash = %s AND used = 0
+ ORDER BY id DESC LIMIT 1",
+ $token, $wallet_address, $tx_hash
+ ));
+
+ if ($affected === 0) {
+ wp_send_json_error('Login validation failed');
+ return;
- );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $row = $wpdb->get_row($wpdb->prepare( | |
| "SELECT id FROM {$table} | |
| WHERE wallet_address = %s AND tx_hash = %s AND used = 0 | |
| ORDER BY id DESC LIMIT 1", | |
| $wallet_address, $tx_hash | |
| )); | |
| if (!$row) { | |
| wp_send_json_error('Login validation failed'); // no match or already used | |
| } | |
| // Generate a random, unguessable token like "9fx0..._..." so that malicious actors | |
| // can't fake login attempts by reusing the same wallet address + tx hash using fake | |
| // AJAX calls from the browser. | |
| $raw = random_bytes(18); // 18 bytes → ~24 chars base64url | |
| $token = rtrim(strtr(base64_encode($raw), '+/', '-_'), '='); | |
| // Mark as used + attach token | |
| $wpdb->update( | |
| $table, | |
| array( | |
| 'used' => 1, | |
| 'login_token' => $token, | |
| ), | |
| array('id' => (int)$row->id), | |
| array('%d','%s'), | |
| array('%d') | |
| ); | |
| // Generate a random, unguessable token like "9fx0..._..." so that malicious actors | |
| // can't fake login attempts by reusing the same wallet address + tx hash using fake | |
| // AJAX calls from the browser. | |
| $raw = random_bytes(18); // 18 bytes → ~24 chars base64url | |
| $token = rtrim(strtr(base64_encode($raw), '+/', '-_'), '='); | |
| // Atomically claim an unused row | |
| $affected = $wpdb->query($wpdb->prepare( | |
| "UPDATE {$table} | |
| SET used = 1, login_token = %s | |
| WHERE wallet_address = %s AND tx_hash = %s AND used = 0 | |
| ORDER BY id DESC LIMIT 1", | |
| $token, $wallet_address, $tx_hash | |
| )); | |
| if ($affected === 0) { | |
| wp_send_json_error('Login validation failed'); | |
| return; | |
| } |
| $row = $wpdb->get_row($wpdb->prepare( | ||
| "SELECT id FROM {$table} | ||
| WHERE pb_paywall_user_wallet_address = %s | ||
| AND tx_hash = %s | ||
| AND post_id = %d | ||
| AND used = 0 | ||
| ORDER BY id DESC | ||
| LIMIT 1", | ||
| $wallet_address, | ||
| $tx_hash, | ||
| $post_id | ||
| )); | ||
|
|
||
| if (!$row) { | ||
| wp_send_json_error('Unlock validation failed'); // no match or already used | ||
| } | ||
|
|
||
| // Generate a random, unguessable token | ||
| $raw = random_bytes(18); // ~24 chars base64url | ||
| $token = rtrim(strtr(base64_encode($raw), '+/', '-_'), '='); | ||
|
|
||
| // Mark row as used + attach unlock token | ||
| $wpdb->update( | ||
| $table, | ||
| array( | ||
| 'used' => 1, | ||
| 'unlock_token' => $token, | ||
| ), | ||
| array( 'id' => (int) $row->id ), | ||
| array( '%d', '%s' ), | ||
| array( '%d' ) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same race condition as ajax_validate_login_tx; apply atomic UPDATE.
This function has the same vulnerability where concurrent requests could both claim the same unlock row. Apply the same atomic UPDATE pattern:
- $row = $wpdb->get_row($wpdb->prepare(
- "SELECT id FROM {$table}
- WHERE pb_paywall_user_wallet_address = %s
- AND tx_hash = %s
- AND post_id = %d
- AND used = 0
- ORDER BY id DESC
- LIMIT 1",
- $wallet_address,
- $tx_hash,
- $post_id
- ));
-
- if (!$row) {
- wp_send_json_error('Unlock validation failed');
- }
-
// Generate a random, unguessable token
$raw = random_bytes(18);
$token = rtrim(strtr(base64_encode($raw), '+/', '-_'), '=');
- // Mark row as used + attach unlock token
- $wpdb->update(
- $table,
- array(
- 'used' => 1,
- 'unlock_token' => $token,
- ),
- array( 'id' => (int) $row->id ),
- array( '%d', '%s' ),
- array( '%d' )
+ // Atomically claim an unused row
+ $affected = $wpdb->query($wpdb->prepare(
+ "UPDATE {$table}
+ SET used = 1, unlock_token = %s
+ WHERE pb_paywall_user_wallet_address = %s
+ AND tx_hash = %s
+ AND post_id = %d
+ AND used = 0
+ ORDER BY id DESC LIMIT 1",
+ $token, $wallet_address, $tx_hash, $post_id
+ ));
+
+ if ($affected === 0) {
+ wp_send_json_error('Unlock validation failed');
+ return;
- );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $row = $wpdb->get_row($wpdb->prepare( | |
| "SELECT id FROM {$table} | |
| WHERE pb_paywall_user_wallet_address = %s | |
| AND tx_hash = %s | |
| AND post_id = %d | |
| AND used = 0 | |
| ORDER BY id DESC | |
| LIMIT 1", | |
| $wallet_address, | |
| $tx_hash, | |
| $post_id | |
| )); | |
| if (!$row) { | |
| wp_send_json_error('Unlock validation failed'); // no match or already used | |
| } | |
| // Generate a random, unguessable token | |
| $raw = random_bytes(18); // ~24 chars base64url | |
| $token = rtrim(strtr(base64_encode($raw), '+/', '-_'), '='); | |
| // Mark row as used + attach unlock token | |
| $wpdb->update( | |
| $table, | |
| array( | |
| 'used' => 1, | |
| 'unlock_token' => $token, | |
| ), | |
| array( 'id' => (int) $row->id ), | |
| array( '%d', '%s' ), | |
| array( '%d' ) | |
| ); | |
| // Generate a random, unguessable token | |
| $raw = random_bytes(18); | |
| $token = rtrim(strtr(base64_encode($raw), '+/', '-_'), '='); | |
| // Atomically claim an unused row | |
| $affected = $wpdb->query($wpdb->prepare( | |
| "UPDATE {$table} | |
| SET used = 1, unlock_token = %s | |
| WHERE pb_paywall_user_wallet_address = %s | |
| AND tx_hash = %s | |
| AND post_id = %d | |
| AND used = 0 | |
| ORDER BY id DESC LIMIT 1", | |
| $token, $wallet_address, $tx_hash, $post_id | |
| )); | |
| if ($affected === 0) { | |
| wp_send_json_error('Unlock validation failed'); | |
| return; | |
| } |
This PR fixes #82 partially (login via Cashtab part) by replacing the old Cashtab login method, which relied on the browser to declare a user “logged in,” with a secure server-verified process. Previously, a bad actor could fake a login from the browser console (from literally any wallet address) without making any payment. This new approach prevents this behavior and implements a secure server-verified login flow.
When a user makes a login payment, the server records it and generates a one-time random token. The browser must then return that token, tx hash, and wallet address to the server to complete the login, making it impossible to fake or replay a login using only client-side tricks. This ensures the login system is now reliable, secure, and resistant to tampering. Though we will have to enforce users now to have a registered PayButton account and make the Public Key feature in the Paywall Settings mandatory (#89). A similar approach will be implemented in the next PR for content unlocking.
Test Plan
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.