feat(recaptcha): add reCAPTCHA verification library#1605
Conversation
WalkthroughA new workspace crate libs/recaptcha-verifier was added. It provides a RecaptchaClient that posts verification requests to Google's API, VerifyRequest/VerifyResponse types modeling v2/v3 payloads, an Error enum for failure cases, and metadata helpers to extract v2/v3 tokens from gRPC metadata. The client validates success, hostname, score threshold, and expected action. The crate includes unit tests for types and metadata and wiremock-based tests for client behavior. Cargo manifest and feature flags for TLS and HTTP dependencies were added. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@libs/recaptcha-verifier/Cargo.toml`:
- Line 8: The reqwest dependency in Cargo.toml is configured with
default-features = false and only "json", which disables TLS and will break
HTTPS calls to Google's reCAPTCHA endpoint; update the reqwest entry to enable a
TLS backend (e.g., add the "rustls-tls" feature or "native-tls") so the HTTP
client has TLS support — modify the reqwest line in Cargo.toml to include either
"rustls-tls" or "native-tls" alongside "json".
In `@libs/recaptcha-verifier/src/client.rs`:
- Around line 100-109: The mock_client function creates a MockServer locally and
returns only RecaptchaClient, which lets the server be dropped and makes tests
flaky; change mock_client to return the MockServer together with the
RecaptchaClient (e.g. -> (MockServer, RecaptchaClient)), construct the server as
before, mount the mock, build the RecaptchaClient with
with_verify_url(server.uri()), and return (server, client) so the server stays
alive for the test; update test call sites (e.g. verify_v2_success) to
destructure the tuple like let (_server, client) = mock_client(...).await.
🧹 Nitpick comments (2)
libs/recaptcha-verifier/src/metadata.rs (1)
15-23: Consider distinguishing malformed tokens from missing tokens.Non-ASCII metadata values are silently treated as missing due to
to_str().ok()on line 18. If a token is present but contains invalid characters, the error will misleadingly report "token not found" rather than "invalid token format". This may complicate debugging.Optional: Add a distinct error variant for malformed tokens
If desired, you could add an
InvalidTokenFormaterror variant to distinguish between truly missing headers and headers with non-ASCII content:+ #[error("reCAPTCHA token in header '{header}' contains invalid characters")] + InvalidTokenFormat { header: String },Then in the extraction logic:
pub fn extract_token(metadata: &MetadataMap, header_name: &str) -> Result<String, Error> { match metadata.get(header_name) { Some(v) => v .to_str() .map(|s| s.to_string()) .map_err(|_| Error::InvalidTokenFormat { header: header_name.to_string(), }), None => Err(Error::MissingToken { header: header_name.to_string(), }), } }libs/recaptcha-verifier/src/client.rs (1)
77-87: Action validation is skipped when response lacks action field.Similar to hostname, if a v3 response lacks the
actionfield butexpected_actionis set, validation passes silently. This could allow requests with missing actions to bypass action verification.Consider failing when expected action is set but response has none
// Check action if expected if let Some(expected_action) = request.expected_action { - if let Some(ref actual_action) = verify_response.action { - if actual_action != expected_action { - return Err(Error::ActionMismatch { - expected: expected_action.to_string(), - actual: actual_action.clone(), - }); - } + match &verify_response.action { + Some(actual_action) if actual_action != expected_action => { + return Err(Error::ActionMismatch { + expected: expected_action.to_string(), + actual: actual_action.clone(), + }); + } + None => { + return Err(Error::ActionMismatch { + expected: expected_action.to_string(), + actual: String::new(), + }); + } + _ => {} } }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@libs/recaptcha-verifier/src/client.rs`:
- Around line 57-87: The current checks for request.expected_hostname vs
verify_response.hostname and request.expected_action vs verify_response.action
skip verification when the response omits those fields; change them to treat a
missing actual value as a mismatch: for request.expected_hostname (in client.rs)
if Some(expected_hostname) is set, explicitly match verify_response.hostname and
return Error::HostnameMismatch when the hostname is None or not equal to
expected; do the same for request.expected_action (inside the v3 score branch)
returning Error::ActionMismatch when verify_response.action is None or different
from expected, using the same Error variants and copying expected/actual strings
as done elsewhere.
- Around line 43-50: The current call using
self.http_client.post(&self.verify_url)...send().await? then
response.json().await? should be hardened: add a per-request timeout (e.g. using
RequestBuilder.timeout(Duration::from_secs(5))) on the POST and after send()
check response.status().is_success() before attempting to parse JSON into
VerifyResponse; if not success, read response.text().await (or response.bytes())
and return a descriptive error including the HTTP status and body. Update the
code around self.http_client.post, .send(), and the usage of response.json() /
VerifyResponse accordingly.
♻️ Duplicate comments (1)
libs/recaptcha-verifier/src/client.rs (1)
102-111: MockServer lifetime can end before tests use the client.
2 participants
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.