Simplify code: deduplicate patterns, fix captcha field name bug

Bug fix:
- Fix frontend sending captcha_token instead of turnstile_token in
  login/register requests (would cause 422 errors on auth)

Backend simplifications:
- Deduplicate VALID_PROVIDERS constant (provider.rs is now the single source)
- Extract validate_display_name/validate_models helpers in provider model
- Add From<UserSettings> for SettingsResponse, From<User> for AdminUserResponse
- Consolidate Resend API call pattern into shared send_via_resend()
- Extract do_bulk_import() for sources bulk/CSV import
- Use idiomatic range.contains() for rate limit validation

Frontend simplifications:
- Consolidate file download logic (exportCsv reuses shared fetchFile/triggerDownload)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
master
oabrivard 3 months ago
parent 8dc4900c47
commit 04819aa926

@ -295,6 +295,19 @@ pub struct AdminUserResponse {
pub updated_at: chrono::DateTime<chrono::Utc>,
}
impl From<crate::models::user::User> for AdminUserResponse {
fn from(u: crate::models::user::User) -> Self {
Self {
id: u.id,
email: u.email,
display_name: u.display_name,
role: u.role.to_string(),
created_at: u.created_at,
updated_at: u.updated_at,
}
}
}
/// `GET /api/v1/admin/users`
///
/// Returns all users in the system.
@ -303,17 +316,7 @@ pub async fn list_users(
State(state): State<AppState>,
) -> Result<impl IntoResponse, AppError> {
let users = db::users::list_all(&state.pool).await?;
let response: Vec<AdminUserResponse> = users
.into_iter()
.map(|u| AdminUserResponse {
id: u.id,
email: u.email,
display_name: u.display_name,
role: u.role.to_string(),
created_at: u.created_at,
updated_at: u.updated_at,
})
.collect();
let response: Vec<AdminUserResponse> = users.into_iter().map(AdminUserResponse::from).collect();
Ok(Json(response))
}

@ -22,14 +22,7 @@ pub async fn get_settings(
State(state): State<AppState>,
) -> Result<impl IntoResponse, AppError> {
let settings = db::settings::get_or_create_default(&state.pool, auth_user.id).await?;
Ok(Json(SettingsResponse {
theme: settings.theme,
max_age_days: settings.max_age_days,
categories: settings.categories,
max_items_per_category: settings.max_items_per_category,
search_agent_behavior: settings.search_agent_behavior,
}))
Ok(Json(SettingsResponse::from(settings)))
}
/// `PUT /api/v1/settings`
@ -41,18 +34,9 @@ pub async fn update_settings(
State(state): State<AppState>,
Json(body): Json<UpdateSettingsRequest>,
) -> Result<impl IntoResponse, AppError> {
// Validate the request
body.validate().map_err(AppError::Validation)?;
let settings = db::settings::upsert(&state.pool, auth_user.id, &body).await?;
tracing::info!(user_id = %auth_user.id, "Settings updated");
Ok(Json(SettingsResponse {
theme: settings.theme,
max_age_days: settings.max_age_days,
categories: settings.categories,
max_items_per_category: settings.max_items_per_category,
search_agent_behavior: settings.search_agent_behavior,
}))
Ok(Json(SettingsResponse::from(settings)))
}

@ -98,10 +98,6 @@ pub async fn bulk_import(
return Err(AppError::Validation("No sources provided".into()));
}
// Check how many sources the user already has
let current_count = db::sources::count_for_user(&state.pool, auth_user.id).await?;
// Validate each source and collect the valid ones
let mut valid_sources: Vec<(String, String)> = Vec::new();
let mut errors: Vec<String> = Vec::new();
@ -113,8 +109,30 @@ pub async fn bulk_import(
valid_sources.push((source.title.clone(), source.url.clone()));
}
// Check if adding all valid sources would exceed the limit
let response = do_bulk_import(
&state.pool,
auth_user.id,
&mut valid_sources,
&mut errors,
"Bulk import",
)
.await?;
Ok(Json(response))
}
/// Shared logic for bulk and CSV imports: enforce per-user limit,
/// insert into DB, and build the response summary.
async fn do_bulk_import(
pool: &sqlx::PgPool,
user_id: uuid::Uuid,
valid_sources: &mut Vec<(String, String)>,
errors: &mut Vec<String>,
log_label: &str,
) -> Result<BulkImportResponse, AppError> {
let current_count = db::sources::count_for_user(pool, user_id).await?;
let remaining_capacity = (MAX_SOURCES_PER_USER - current_count).max(0) as usize;
if valid_sources.len() > remaining_capacity {
valid_sources.truncate(remaining_capacity);
errors.push(format!(
@ -123,23 +141,23 @@ pub async fn bulk_import(
));
}
let created = db::sources::bulk_create(&state.pool, auth_user.id, &valid_sources).await?;
let created = db::sources::bulk_create(pool, user_id, valid_sources).await?;
let imported = created.len();
let skipped = valid_sources.len() - imported; // duplicates that were silently skipped
let skipped = valid_sources.len() - imported;
tracing::info!(
user_id = %auth_user.id,
user_id = %user_id,
imported = imported,
skipped = skipped,
errors = errors.len(),
"Bulk import completed"
"{} completed", log_label
);
Ok(Json(BulkImportResponse {
Ok(BulkImportResponse {
imported,
skipped,
errors,
}))
errors: errors.clone(),
})
}
/// `POST /api/v1/sources/import-csv`
@ -188,7 +206,6 @@ pub async fn import_csv(
}
// Validate each row
let current_count = db::sources::count_for_user(&state.pool, auth_user.id).await?;
let mut valid_sources: Vec<(String, String)> = Vec::new();
let mut errors: Vec<String> = Vec::new();
@ -204,33 +221,11 @@ pub async fn import_csv(
valid_sources.push((title.clone(), url.clone()));
}
// Enforce per-user limit
let remaining_capacity = (MAX_SOURCES_PER_USER - current_count).max(0) as usize;
if valid_sources.len() > remaining_capacity {
valid_sources.truncate(remaining_capacity);
errors.push(format!(
"Only {} sources could be imported (limit of {} reached)",
remaining_capacity, MAX_SOURCES_PER_USER
));
}
let created = db::sources::bulk_create(&state.pool, auth_user.id, &valid_sources).await?;
let imported = created.len();
let skipped = valid_sources.len() - imported;
let response =
do_bulk_import(&state.pool, auth_user.id, &mut valid_sources, &mut errors, "CSV import")
.await?;
tracing::info!(
user_id = %auth_user.id,
imported = imported,
skipped = skipped,
errors = errors.len(),
"CSV import completed"
);
Ok(Json(BulkImportResponse {
imported,
skipped,
errors,
}))
Ok(Json(response))
}
/// `GET /api/v1/sources/export-csv`

@ -20,8 +20,7 @@ pub struct UserApiKey {
pub updated_at: DateTime<Utc>,
}
/// Known provider names for validation.
const VALID_PROVIDERS: &[&str] = &["gemini", "openai", "anthropic"];
use crate::models::provider::VALID_PROVIDERS;
/// Request body for `POST /api/v1/user/api-keys`.
///

@ -43,7 +43,9 @@ fn default_true() -> bool {
}
/// Known provider names.
const VALID_PROVIDERS: &[&str] = &["gemini", "openai", "anthropic"];
///
/// Also used by `models::api_key` for validating user API key requests.
pub const VALID_PROVIDERS: &[&str] = &["gemini", "openai", "anthropic"];
impl CreateProviderRequest {
/// Validate the provider creation request.
@ -66,37 +68,8 @@ impl CreateProviderRequest {
));
}
let display = self.display_name.trim();
if display.is_empty() {
return Err("Display name cannot be empty".into());
}
if display.len() > 100 {
return Err("Display name must be at most 100 characters".into());
}
if self.models.is_empty() {
return Err("At least one model must be provided".into());
}
let default_count = self.models.iter().filter(|m| m.is_default).count();
if default_count > 1 {
return Err("At most one model can be marked as default".into());
}
for model in &self.models {
if model.model_id.trim().is_empty() {
return Err("Model ID cannot be empty".into());
}
if model.model_id.len() > 100 {
return Err("Model ID must be at most 100 characters".into());
}
if model.display_name.trim().is_empty() {
return Err("Model display name cannot be empty".into());
}
if model.display_name.len() > 200 {
return Err("Model display name must be at most 200 characters".into());
}
}
validate_display_name(&self.display_name)?;
validate_models(&self.models)?;
Ok(())
}
@ -114,42 +87,53 @@ impl UpdateProviderRequest {
/// Validate the provider update request.
pub fn validate(&self) -> Result<(), String> {
if let Some(ref display) = self.display_name {
if display.trim().is_empty() {
return Err("Display name cannot be empty".into());
}
if display.len() > 100 {
return Err("Display name must be at most 100 characters".into());
}
validate_display_name(display)?;
}
if let Some(ref models) = self.models {
if models.is_empty() {
return Err("At least one model must be provided".into());
}
validate_models(models)?;
}
Ok(())
}
}
let default_count = models.iter().filter(|m| m.is_default).count();
if default_count > 1 {
return Err("At most one model can be marked as default".into());
}
/// Validate a provider display name.
fn validate_display_name(display_name: &str) -> Result<(), String> {
if display_name.trim().is_empty() {
return Err("Display name cannot be empty".into());
}
if display_name.len() > 100 {
return Err("Display name must be at most 100 characters".into());
}
Ok(())
}
for model in models {
if model.model_id.trim().is_empty() {
return Err("Model ID cannot be empty".into());
}
if model.model_id.len() > 100 {
return Err("Model ID must be at most 100 characters".into());
}
if model.display_name.trim().is_empty() {
return Err("Model display name cannot be empty".into());
}
if model.display_name.len() > 200 {
return Err("Model display name must be at most 200 characters".into());
}
}
}
/// Validate a list of provider models.
fn validate_models(models: &[ProviderModel]) -> Result<(), String> {
if models.is_empty() {
return Err("At least one model must be provided".into());
}
Ok(())
let default_count = models.iter().filter(|m| m.is_default).count();
if default_count > 1 {
return Err("At most one model can be marked as default".into());
}
for model in models {
if model.model_id.trim().is_empty() {
return Err("Model ID cannot be empty".into());
}
if model.model_id.len() > 100 {
return Err("Model ID must be at most 100 characters".into());
}
if model.display_name.trim().is_empty() {
return Err("Model display name cannot be empty".into());
}
if model.display_name.len() > 200 {
return Err("Model display name must be at most 200 characters".into());
}
}
Ok(())
}
/// Public response for enabled providers (no admin-only data).

@ -31,10 +31,10 @@ impl UpdateRateLimitRequest {
/// - `max_requests` must be between 1 and 1000.
/// - `time_window_seconds` must be between 1 and 3600.
pub fn validate(&self) -> Result<(), String> {
if self.max_requests < 1 || self.max_requests > 1000 {
if !(1..=1000).contains(&self.max_requests) {
return Err("max_requests must be between 1 and 1000".into());
}
if self.time_window_seconds < 1 || self.time_window_seconds > 3600 {
if !(1..=3600).contains(&self.time_window_seconds) {
return Err("time_window_seconds must be between 1 and 3600".into());
}
Ok(())

@ -26,6 +26,18 @@ pub struct SettingsResponse {
pub search_agent_behavior: String,
}
impl From<UserSettings> for SettingsResponse {
fn from(s: UserSettings) -> Self {
Self {
theme: s.theme,
max_age_days: s.max_age_days,
categories: s.categories,
max_items_per_category: s.max_items_per_category,
search_agent_behavior: s.search_agent_behavior,
}
}
}
/// Request body for `PUT /api/v1/settings`.
#[derive(Debug, Deserialize)]
pub struct UpdateSettingsRequest {

@ -29,13 +29,57 @@ struct ResendEmailRequest<'a> {
/// without network access to the Resend API.
pub const TEST_API_KEY: &str = "re_test_bypass_no_send";
/// Send an email via the Resend API.
///
/// Shared implementation used by both magic link and synthesis emails.
/// When `api_key` equals [`TEST_API_KEY`], the external call is skipped
/// and the function returns success immediately (used in integration tests).
async fn send_via_resend(
client: &reqwest::Client,
api_key: &str,
request_body: &ResendEmailRequest<'_>,
context: &str,
) -> Result<(), AppError> {
if api_key == TEST_API_KEY {
tracing::debug!(to = ?request_body.to, "Email send bypassed (test mode)");
return Ok(());
}
let response = client
.post(RESEND_API_URL)
.header("Authorization", format!("Bearer {}", api_key))
.json(request_body)
.send()
.await
.map_err(|e| {
tracing::error!("Failed to send {} via Resend: {:?}", context, e);
AppError::Internal(anyhow::anyhow!("Failed to send {}", context))
})?;
if !response.status().is_success() {
let status = response.status();
let body = response
.text()
.await
.unwrap_or_else(|_| "unknown".to_string());
tracing::error!(
status = %status,
body = %body,
"Resend API returned error for {}", context
);
return Err(AppError::Internal(anyhow::anyhow!(
"Email service returned status {}", status
)));
}
tracing::info!(to = ?request_body.to, "{} sent successfully", context);
Ok(())
}
/// Send a magic link email to the given address.
///
/// The email contains a link that the user clicks to authenticate.
/// The link points to `GET /api/v1/auth/verify?token={raw_token}`.
///
/// When `api_key` equals [`TEST_API_KEY`], the external call is skipped
/// and the function returns success immediately (used in integration tests).
pub async fn send_magic_link(
client: &reqwest::Client,
api_key: &str,
@ -44,12 +88,6 @@ pub async fn send_magic_link(
app_url: &str,
raw_token: &str,
) -> Result<(), AppError> {
// Bypass for integration tests — no external HTTP call
if api_key == TEST_API_KEY {
tracing::debug!(to = to, "Email send bypassed (test mode)");
return Ok(());
}
let verify_url = format!("{}/api/v1/auth/verify?token={}", app_url, raw_token);
let html = format!(
@ -82,35 +120,7 @@ pub async fn send_magic_link(
text: None,
};
let response = client
.post(RESEND_API_URL)
.header("Authorization", format!("Bearer {}", api_key))
.json(&request_body)
.send()
.await
.map_err(|e| {
tracing::error!("Failed to send email via Resend: {:?}", e);
AppError::Internal(anyhow::anyhow!("Failed to send email"))
})?;
if !response.status().is_success() {
let status = response.status();
let body = response
.text()
.await
.unwrap_or_else(|_| "unknown".to_string());
tracing::error!(
status = %status,
body = %body,
"Resend API returned error"
);
return Err(AppError::Internal(anyhow::anyhow!(
"Email service returned status {}", status
)));
}
tracing::info!(to = to, "Magic link email sent successfully");
Ok(())
send_via_resend(client, api_key, &request_body, "magic link email").await
}
/// Escape special HTML characters to prevent XSS in email templates.
@ -227,12 +237,6 @@ pub async fn send_synthesis_email(
date: &str,
sections: &[NewsSection],
) -> Result<(), AppError> {
// Bypass for integration tests
if api_key == TEST_API_KEY {
tracing::debug!(to = to, "Synthesis email send bypassed (test mode)");
return Ok(());
}
let html = build_synthesis_html(week, date, sections);
let text = build_synthesis_text(week, date, sections);
let subject = format!("Synthese de la Semaine {} - AI Weekly Synth", week);
@ -245,35 +249,7 @@ pub async fn send_synthesis_email(
text: Some(&text),
};
let response = client
.post(RESEND_API_URL)
.header("Authorization", format!("Bearer {}", api_key))
.json(&request_body)
.send()
.await
.map_err(|e| {
tracing::error!("Failed to send synthesis email via Resend: {:?}", e);
AppError::Internal(anyhow::anyhow!("Failed to send synthesis email"))
})?;
if !response.status().is_success() {
let status = response.status();
let body = response
.text()
.await
.unwrap_or_else(|_| "unknown".to_string());
tracing::error!(
status = %status,
body = %body,
"Resend API returned error for synthesis email"
);
return Err(AppError::Internal(anyhow::anyhow!(
"Email service returned status {}", status
)));
}
tracing::info!(to = to, week = week, "Synthesis email sent successfully");
Ok(())
send_via_resend(client, api_key, &request_body, "synthesis email").await
}
#[cfg(test)]

@ -1,4 +1,5 @@
import { api } from './client';
import { fetchFile, triggerDownload } from './syntheses';
import type {
Source,
CreateSourceRequest,
@ -6,8 +7,6 @@ import type {
BulkImportResponse,
} from '~/types';
const API_BASE = '/api/v1';
export const sourcesApi = {
list: (): Promise<Source[]> => api.get<Source[]>('/sources'),
@ -26,29 +25,7 @@ export const sourcesApi = {
},
exportCsv: async (): Promise<void> => {
const response = await fetch(`${API_BASE}/sources/export-csv`, {
method: 'GET',
headers: {
'X-Requested-With': 'XMLHttpRequest',
},
credentials: 'same-origin',
});
if (!response.ok) {
if (response.status === 401) {
window.location.href = '/login';
}
throw new Error(`Export failed: HTTP ${response.status}`);
}
const blob = await response.blob();
const url = URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = 'sources.csv';
document.body.appendChild(a);
a.click();
a.remove();
URL.revokeObjectURL(url);
const response = await fetchFile('/sources/export-csv');
await triggerDownload(response, 'sources.csv');
},
};

@ -7,7 +7,7 @@ const API_BASE = '/api/v1';
* Trigger a file download from a fetch Response.
* Reads the blob, creates a temporary object URL, clicks a hidden anchor, then cleans up.
*/
async function triggerDownload(response: Response, fallbackFilename: string): Promise<void> {
export async function triggerDownload(response: Response, fallbackFilename: string): Promise<void> {
const blob = await response.blob();
// Try to extract filename from Content-Disposition header
@ -34,7 +34,7 @@ async function triggerDownload(response: Response, fallbackFilename: string): Pr
* Perform an authenticated GET request that expects a binary/file response.
* Throws an ApiError-shaped object on failure.
*/
async function fetchFile(path: string): Promise<Response> {
export async function fetchFile(path: string): Promise<Response> {
const response = await fetch(`${API_BASE}${path}`, {
method: 'GET',
headers: {

@ -71,7 +71,7 @@ const Login: Component = () => {
try {
await authApi.login({
email: email(),
captcha_token: turnstileToken()!,
turnstile_token: turnstileToken()!,
});
setSubmitted(true);
setResendCooldown(60);
@ -98,7 +98,7 @@ const Login: Component = () => {
try {
await authApi.login({
email: email(),
captcha_token: turnstileToken()!,
turnstile_token: turnstileToken()!,
});
setResendCooldown(60);
} catch (err) {

@ -73,7 +73,7 @@ const Register: Component = () => {
await authApi.register({
email: email(),
display_name: displayName() || undefined,
captcha_token: turnstileToken()!,
turnstile_token: turnstileToken()!,
});
setSubmitted(true);
setResendCooldown(60);
@ -101,7 +101,7 @@ const Register: Component = () => {
await authApi.register({
email: email(),
display_name: displayName() || undefined,
captcha_token: turnstileToken()!,
turnstile_token: turnstileToken()!,
});
setResendCooldown(60);
} catch (err) {

@ -15,13 +15,13 @@ export interface AuthState {
export interface LoginRequest {
email: string;
captcha_token: string;
turnstile_token: string;
}
export interface RegisterRequest {
email: string;
display_name?: string;
captcha_token: string;
turnstile_token: string;
}
export interface LoginResponse {

Loading…
Cancel
Save