a2b4dd6d66
Two findings from KeiSeiKit2.0 pr-review (~/Projects/KeiSeiKit2.0/skills/pr-review)
applied to commit range b346250..HEAD.
1. BLOCKER — SecretString silently leaked plaintext via Serialize.
File: _primitives/_rust/kei-runtime-core/src/secrets.rs
Was: derive(Serialize) + serde(transparent) -> serde_json::to_string(&secret)
emitted the raw plaintext in any parent struct with #[derive(Serialize)].
Debug was redacted but Serialize was not. Defeated the type's purpose.
Now: manual Serialize impl always emits literal "<redacted>". Deserialize
derive kept (callers need to read secrets from config/env).
Test serialize_emits_redacted_literal asserts JSON output is "\"<redacted>\"".
2. WARNING — PKCE code_verifier dropped before token exchange.
build_auth_url generated code_challenge = SHA256(verifier) but verify() never
threaded the verifier to the token endpoint. Token exchange submitted no
code_verifier, defeating the PKCE protection.
Files:
- _primitives/_rust/kei-runtime-core/src/traits/auth.rs:
AuthChallenge::OAuthCode now carries code_verifier: Option<String>.
Caller stores verifier alongside state in their session-store, exactly as
they already store state for CSRF check.
- _primitives/_rust/kei-auth-google/src/provider.rs:
verify() destructures code_verifier and passes to client.exchange_code(...).
- _primitives/_rust/kei-auth-apple/src/provider.rs:
same change.
Tests added (wiremock body assertions):
- google_smoke / apple_smoke: assert exchange request body contains
code_verifier=<value> when challenge carried Some(verifier).
- existing tests updated to construct OAuthCode { ..., code_verifier: None }.
Test split (Constructor Pattern 200 LOC):
- apple_smoke.rs grew over 200 LOC after PKCE test addition. Split into
apple_smoke.rs (provider tests) + apple_client_smoke.rs (client tests).
- same for google_smoke.rs / google_client_smoke.rs.
Test results: 31 passed; 0 failed across kei-auth, kei-auth-apple, kei-auth-google,
kei-runtime-core unit + integration tests. cargo check --workspace clean.
Breaking change: any caller that constructs AuthChallenge::OAuthCode outside this
workspace must add code_verifier field (None for legacy no-PKCE; Some for PKCE).
Compile-time surfaced gap, not runtime regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
86 lines
2.8 KiB
Rust
86 lines
2.8 KiB
Rust
// SPDX-License-Identifier: Apache-2.0
|
|
// Copyright 2026 <author org>
|
|
//!
|
|
//! Wiremock smoke tests for `GoogleAuthClient` HTTP layer. No live HTTP.
|
|
|
|
use kei_auth_google::GoogleAuthClient;
|
|
use serde_json::json;
|
|
use wiremock::matchers::{body_string_contains, header, method, path};
|
|
use wiremock::{Mock, MockServer, ResponseTemplate};
|
|
|
|
fn client_for(server: &MockServer) -> GoogleAuthClient {
|
|
GoogleAuthClient::with_urls(
|
|
format!("{}/token", server.uri()),
|
|
format!("{}/userinfo", server.uri()),
|
|
"client-id-xyz",
|
|
"client-secret-xyz",
|
|
"https://example.com/cb",
|
|
)
|
|
.unwrap()
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn token_endpoint_200_returns_access_token() {
|
|
let server = MockServer::start().await;
|
|
Mock::given(method("POST"))
|
|
.and(path("/token"))
|
|
.and(body_string_contains("grant_type=authorization_code"))
|
|
.and(body_string_contains("code=abc123"))
|
|
.and(body_string_contains("client_id=client-id-xyz"))
|
|
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
|
"access_token": "ya29.a0AfH-test",
|
|
"expires_in": 3600,
|
|
"id_token": "eyJ.fake.jwt"
|
|
})))
|
|
.expect(1)
|
|
.mount(&server)
|
|
.await;
|
|
|
|
let client = client_for(&server);
|
|
let token = client.exchange_code("abc123", None).await.unwrap();
|
|
assert_eq!(token.access_token, "ya29.a0AfH-test");
|
|
assert_eq!(token.expires_in, 3600);
|
|
assert_eq!(token.id_token.as_deref(), Some("eyJ.fake.jwt"));
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn userinfo_200_returns_email_and_sub() {
|
|
let server = MockServer::start().await;
|
|
Mock::given(method("GET"))
|
|
.and(path("/userinfo"))
|
|
.and(header("authorization", "Bearer ya29.a0AfH-test"))
|
|
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
|
"sub": "1234567890",
|
|
"email": "alice@example.com",
|
|
"name": "Alice"
|
|
})))
|
|
.expect(1)
|
|
.mount(&server)
|
|
.await;
|
|
|
|
let client = client_for(&server);
|
|
let info = client.userinfo("ya29.a0AfH-test").await.unwrap();
|
|
assert_eq!(info.sub, "1234567890");
|
|
assert_eq!(info.email, "alice@example.com");
|
|
assert_eq!(info.name, "Alice");
|
|
}
|
|
|
|
#[tokio::test]
|
|
async fn exchange_code_400_returns_api_error() {
|
|
let server = MockServer::start().await;
|
|
Mock::given(method("POST"))
|
|
.and(path("/token"))
|
|
.respond_with(ResponseTemplate::new(400).set_body_json(json!({
|
|
"error": "invalid_grant",
|
|
"error_description": "Bad code"
|
|
})))
|
|
.expect(1)
|
|
.mount(&server)
|
|
.await;
|
|
|
|
let client = client_for(&server);
|
|
let err = client.exchange_code("bad-code", None).await.unwrap_err();
|
|
let msg = format!("{err}");
|
|
assert!(msg.contains("api"), "expected api variant, got {msg}");
|
|
assert!(msg.contains("400"), "expected status 400 in message, got {msg}");
|
|
}
|