feat(backend): メール認証を行えるように#13
Open
yupix wants to merge 15 commits into
Open
Conversation
…ment and user email verification status
Walkthroughユーザー登録のメール検証を導入：エンティティ拡張、Redisトークン管理、ApalisジョブでSMTP送信、ハンドラ/サーバ/OpenAPI更新、Brunoテストを追加。 Changesメール認証フロー実装
Sequence Diagram(s)sequenceDiagram
participant Client
participant BackendServer
participant Redis
participant Postgres
participant ApalisWorker
participant SMTP
Client->>BackendServer: POST /v1/auth/register (email,password)
BackendServer->>Postgres: Create user (transaction)
BackendServer->>BackendServer: generate_email_verification_token()
BackendServer->>ApalisWorker: enqueue VerificationEmailJob(user_id,email,token)
ApalisWorker->>Postgres: persist job (PostgresStorage)
ApalisWorker->>Redis: store_token(token -> user_id) (Lua atomic)
ApalisWorker->>SMTP: send_verification_email(email, verify_url)
Client->>BackendServer: POST /v1/auth/verify-email (token)
BackendServer->>Redis: consume_token(token) -> user_id
BackendServer->>Postgres: update users.email_verified = true
BackendServer-->>Client: 200 OK (verification result)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
yupix
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/entities/users.rs`:
- Around line 20-24: The project is missing the database migration that matches
the users entity (so the #[sea_orm(unique)] on the email field and the
email_verified: bool column are not created); add a new migration file (e.g.,
m20240101_create_users.rs) under apps/backend/migration/src/ that creates the
users table with a unique email column and a boolean email_verified column (plus
any other columns present in the users entity), and register that migration in
the migrations() function in apps/backend/migration/src/lib.rs so it returns a
vec! containing the new migration module; ensure the migration’s up/down
functions reflect the schema (unique constraint on email and creation/removal of
email_verified) to prevent schema mismatch errors when handlers/auth.rs uses
Set(false) and reads email_verified.
In `@apps/backend/src/handlers/auth.rs`:
- Around line 121-146: The current flow persists the user via
users::Entity::insert then performs email_verification::store_token and
send_verification_email, causing inconsistent state if Redis/SMTP fail; change
this by either (preferred) writing an outbox entry (e.g., create an Outbox
entity record in the same DB transaction alongside users::Entity::insert) and
dispatching email/redis work asynchronously from the outbox worker, or (minimal)
implement compensating cleanup: if email_verification::store_token or
send_verification_email returns an error, delete the created user (undo
users::Entity::insert) and return a proper error mapping instead of leaving a
half-created user; update error handling around users::Entity::insert,
email_verification::store_token, and send_verification_email accordingly so
state remains consistent.
- Around line 292-297: The build_verify_url function embeds the raw token into
the query string, which can break links if the token contains reserved
characters; update build_verify_url to percent-encode the token before
interpolation (e.g., use a URL-encoding helper such as url::form_urlencoded or
Url::parse + append_pair) and build the final URL from the trimmed
settings.email_verification_app_url plus the encoded token so the query is
always safe and correct.
In `@apps/backend/src/settings.rs`:
- Around line 17-27: The field email_verification_app_url is currently given a
default via serde(default = "default_email_verification_app_url") which will
silently fall back to "http://localhost:3000"; remove the default so the config
deserialization fails when that key is missing (make the struct field required),
delete or stop using default_email_verification_app_url()/default_allow_origin()
for this field, and ensure any callers that relied on the default now require
the explicit setting at startup so the process fails fast if
email_verification_app_url is not provided.
In `@apps/backend/src/utils/db.rs`:
- Around line 6-12: Replace the brittle string-based check in
is_postgres_unique_violation by using DbErr::sql_err() and matching the
structured SQL error; specifically, call err.sql_err(), if Some, match against
sea_orm::error::SqlErr::UniqueConstraintViolation(_) and return true for that
branch and false otherwise, falling back to false when sql_err() is None,
instead of using err.to_string() and substring checks.
In `@apps/backend/src/utils/email_verification.rs`:
- Around line 29-60: The current sequence using redis::cmd to GET old_token, DEL
old token, SET token_key and SET user_key is non-atomic and can leave multiple
valid tokens during concurrent requests; replace these multi-command ops with a
single Redis Lua script (use redis::Script or EVAL via the redis client) that:
reads the existing value of user_key, if present deletes the corresponding
KEY_TOKEN..old_token, then sets KEY_TOKEN..token to user_id with EX
TOKEN_TTL_SECS and sets user_key to token with EX TOKEN_TTL_SECS, and returns
success/failure; invoke this script in place of the code manipulating old_token,
token_key, KEY_TOKEN, TOKEN_TTL_SECS and propagate errors via the same
anyhow::anyhow! handling so error messages still include context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 22356514-2dca-4680-a0f1-7a304649adb4
📒 Files selected for processing (15)
apps/backend/.env.exampleapps/backend/src/entities/README.mdapps/backend/src/entities/scopes.rsapps/backend/src/entities/users.rsapps/backend/src/handlers/auth.rsapps/backend/src/handlers/labels.rsapps/backend/src/handlers/personal_tokens.rsapps/backend/src/openapi/mod.rsapps/backend/src/openapi/responses.rsapps/backend/src/routes/auth.rsapps/backend/src/settings.rsapps/backend/src/utils/auth.rsapps/backend/src/utils/db.rsapps/backend/src/utils/email_verification.rsapps/backend/src/utils/mod.rs
…ng in email verification URL
…tion and update dependencies
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/utils/verification_email_outbox.rs`:
- Around line 61-70: run_worker currently sleeps before the first work cycle
causing up to a 30s startup delay; change the loop so the initial iteration runs
process_pending immediately (or set idle=false for the first loop) and only then
awaits POLL_INTERVAL_IDLE/POLL_INTERVAL_ACTIVE. Specifically, in run_worker
adjust the idle handling so process_pending() (or the function that handles
pending emails) is invoked before the first tokio::time::sleep, keeping the
existing idle flag logic and using POLL_INTERVAL_IDLE / POLL_INTERVAL_ACTIVE
unchanged for subsequent sleeps to preserve backoff behavior.
- Around line 52-58: process_pending currently selects rows with
status='Pending' without locking, so wake_worker and run_worker can pick the
same outbox row and cause duplicate processing; change process_pending to
atomically claim work by either using UPDATE ... WHERE status='Pending'
RETURNING (to flip status to 'Processing' and return claimed rows) or use SELECT
... FOR UPDATE SKIP LOCKED to fetch rows while preventing others from seeing
them, and ensure process_one only runs on claimed rows; also remove or move the
initial POLL_INTERVAL_IDLE sleep in run_worker so the worker polls immediately
on startup (or call process_pending once at startup) to avoid delayed backlog
processing.
In `@apps/frontend/app/utils/openapi.json`:
- Around line 244-250: OpenAPI responses for register, verify_email, and
resend_verification_email currently declare "text/plain" but the handlers return
Axum's Json<String> (application/json); update the response content media type
to "application/json" (keeping the schema as type: string) in the generated
openapi.json for those endpoints, or adjust the utoipa annotations for the
corresponding handlers (register, verify_email, resend_verification_email) so
the generated response content_type is application/json instead of text/plain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 44537a31-a7d4-482a-97b5-a606b66b4e48
⛔ Files ignored due to path filters (17)
apps/backend/Cargo.lockis excluded by!**/*.lockapps/frontend/app/utils/openapi/.openapi-generator/FILESis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/apis/DefaultApi.tsis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/docs/CrateEntitiesUsersModel.mdis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/docs/DefaultApi.mdis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/docs/Login401Response.mdis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/docs/ResendVerificationRequest.mdis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/docs/ServerError.mdis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/docs/VerifyEmailRequest.mdis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/models/CrateEntitiesUsersModel.tsis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/models/CreatePersonalTokenResponse.tsis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/models/Login401Response.tsis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/models/PersonalTokenResponse.tsis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/models/ResendVerificationRequest.tsis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/models/ServerError.tsis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/models/VerifyEmailRequest.tsis excluded by!apps/frontend/app/utils/openapi/**apps/frontend/app/utils/openapi/models/index.tsis excluded by!apps/frontend/app/utils/openapi/**
📒 Files selected for processing (11)
apps/backend/.env.exampleapps/backend/Cargo.tomlapps/backend/src/entities/mod.rsapps/backend/src/entities/verification_email_outbox.rsapps/backend/src/handlers/auth.rsapps/backend/src/server.rsapps/backend/src/settings.rsapps/backend/src/utils/mod.rsapps/backend/src/utils/verification_email_delivery.rsapps/backend/src/utils/verification_email_outbox.rsapps/frontend/app/utils/openapi.json
…us and transaction handling
…ve idle state handling
… ServerError types
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/backend/src/handlers/auth.rs (2)
237-252:⚠️ Potential issue | 🟠 Major | ⚡ Quick winユーザー列挙攻撃が可能な状態になっています
レスポンスが
404 UserNotFound、409 EmailAlreadyVerified、200 成功と分かれているため、攻撃者がメールアドレスの登録有無を特定できます。レートリミットもメールアドレス単位のため、異なるメールを連続して試行でき、列挙を抑止できません。推奨対策: 存在しないユーザーや既に確認済みのユーザーに対しても、成功時と同一のレスポンス（200）を返すことでタイミングや応答コードによる情報漏洩を防ぎます。
🛡️ 修正案
pub async fn resend_verification_email( State(state): State<AppState>, Valid(Json(payload)): Valid<Json<ResendVerificationRequest>>, ) -> Result<Json<String>, AuthError> { let email = payload.email.trim().to_string(); if !email_verification::try_acquire_resend_slot(&state.redis_client, &email) .await .map_err(|e| AuthError::Internal(anyhow::anyhow!("redis resend cooldown: {e}")))? { return Err(AuthError::TooManyRequests); } + // 存在しないユーザー・既確認ユーザーでも同一レスポンスを返し、列挙を防止 + let generic_response = Json(format!( + "確認メールを再送しました（同一メールアドレスへの再送は{}秒に1回までです）。", + email_verification::RESEND_COOLDOWN_SECS + )); + let user = users::Entity::find() .filter(users::Column::Email.eq(email.clone())) .one(&state.db) - .await? - .ok_or(AuthError::UserNotFound)?; + .await?; - if user.email_verified { - return Err(AuthError::EmailAlreadyVerified); + let Some(user) = user else { + return Ok(generic_response); + }; + + if user.email_verified { + return Ok(generic_response); } let token = generate_email_verification_token(); verification_email_outbox::enqueue(&state.db, user.id, email.clone(), token) .await .map_err(|e| AuthError::Internal(anyhow::anyhow!("enqueue verification email: {e}")))?; verification_email_outbox::wake_worker(state); - Ok(Json(format!( - "確認メールを再送しました（同一メールアドレスへの再送は{}秒に1回までです）。", - email_verification::RESEND_COOLDOWN_SECS - ))) + Ok(generic_response) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/handlers/auth.rs` around lines 237 - 252, The handler leaks user existence via differing errors; change the flow so both a non-existent user and an already-verified user produce the same successful response (HTTP 200) and body as the genuine resend path: still call email_verification::try_acquire_resend_slot(...) and perform users::Entity::find(...).one(&state.db) and inspect user.email_verified to decide whether to enqueue/send email, but do not return AuthError::UserNotFound or AuthError::EmailAlreadyVerified — instead return the same success result for those cases while logging internally if needed; keep returning AuthError::TooManyRequests and internal errors unchanged so rate-limits and failure modes remain enforced.
235-235:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winメールアドレスの大文字小文字を正規化していません
trim()のみで大文字小文字の正規化がされていないため、ユーザーが登録時と異なるケースで入力すると照合に失敗します。多くのサービスでは小文字に正規化して保存・検索します。🔧 修正案
- let email = payload.email.trim().to_string(); + let email = payload.email.trim().to_lowercase();
registerハンドラの🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/handlers/auth.rs` at line 235, 現在の `let email = payload.email.trim().to_string();` は大文字小文字を正規化しておらず、照合に失敗する可能性があります。payload.email を trim() のあとで小文字化（to_lowercase）して保存/比較するように変更し、ログインや比較に使っているすべての箇所と同様に `register` ハンドラでも同じ正規化を適用してください（参照するシンボル: payload.email, 変数 email, register ハンドラ）。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/backend/src/handlers/auth.rs`:
- Around line 237-252: The handler leaks user existence via differing errors;
change the flow so both a non-existent user and an already-verified user produce
the same successful response (HTTP 200) and body as the genuine resend path:
still call email_verification::try_acquire_resend_slot(...) and perform
users::Entity::find(...).one(&state.db) and inspect user.email_verified to
decide whether to enqueue/send email, but do not return AuthError::UserNotFound
or AuthError::EmailAlreadyVerified — instead return the same success result for
those cases while logging internally if needed; keep returning
AuthError::TooManyRequests and internal errors unchanged so rate-limits and
failure modes remain enforced.
- Line 235: 現在の `let email = payload.email.trim().to_string();`
は大文字小文字を正規化しておらず、照合に失敗する可能性があります。payload.email を trim()
のあとで小文字化（to_lowercase）して保存/比較するように変更し、ログインや比較に使っているすべての箇所と同様に `register`
ハンドラでも同じ正規化を適用してください（参照するシンボル: payload.email, 変数 email, register ハンドラ）。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 45a85525-5046-4a88-8d21-2e0ed9be8203
📒 Files selected for processing (10)
apps/backend/src/entities/verification_email_outbox.rsapps/backend/src/error.rsapps/backend/src/handlers/auth.rsapps/backend/src/handlers/labels.rsapps/backend/src/lib.rsapps/backend/src/openapi/mod.rsapps/backend/src/openapi/responses.rsapps/backend/src/utils/auth.rsapps/backend/src/utils/db.rsapps/backend/src/utils/verification_email_outbox.rs
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/bruno/01-register-verify/01-register.bru`:
- Around line 25-30: The vars:pre-request block currently hardcodes baseUrl,
username, email, and password; remove those hardcoded entries and instead read
them from the shared environment file by referencing the environment variables
defined in environments/local.bru (baseUrl, email, password, username,
verificationToken) so the script uses the centralized environment values; update
any references in the 01-register.bru flow that assumed the inline vars to use
the environment variables instead.
In `@apps/backend/bruno/01-register-verify/02-verify-email.bru`:
- Around line 23-26: The pre-request block currently hardcodes verificationToken
(vars:pre-request { verificationToken: ... }), which overrides
environment/runtime values and causes expiry/mismatch failures; remove the
verificationToken entry from the pre-request vars so the flow uses the value
from environments/local.bru or the runtime-updated token, leaving baseUrl (and
other required entries) intact and ensuring verificationToken is set only in
environments/local.bru or updated at runtime.
In `@apps/backend/bruno/01-register-verify/03-login-unverified.bru`:
- Around line 24-27: Add a concrete test that asserts the unverified-login
behavior described in the docs: create or extend the tests block for
03-login-unverified.bru to perform the login request for an unverified email and
assert the HTTP status is 403 and the returned error code equals
"email-not-verified"; locate the test near the existing docs block in this file,
invoke the same login endpoint used by the scenario, and include assertions for
both the status and response error.code to ensure future regressions are caught.
In `@apps/backend/bruno/02-login/01-login.bru`:
- Around line 24-27: The vars:pre-request block currently hardcodes baseUrl and
email which must be moved to environment config; remove the hardcoded entries
from the vars:pre-request block in 01-login.bru and instead reference the
environment variables defined in environments/local.bru (ensure
environments/local.bru contains baseUrl, email, password, username,
verificationToken per guidelines), so the login scenario reads those variables
at runtime rather than using the fixed values.
In `@apps/backend/bruno/02-login/02-me.bru`:
- Around line 13-15: 現在の固定ヘッダ "Cookies: session=..." を削除し、正しいヘッダ名 "Cookie"
を使わないでください；代わりに Bruno の Cookie
管理を有効にして直前ログインで保存されたセッションCookieを再利用するように変更してください（固定の "session"
値を取り除き、リクエスト実行は同一コレクション内で順次実行して Cookie ストアが引き継がれるようにしてください）。
In `@apps/backend/bruno/environments/local.bru`:
- Around line 3-6: Replace the hardcoded/reused values in environments/local.bru
with explicit placeholders so re-runs don't collide: update the email, password,
username, and verificationToken entries to clearly indicate they must be filled
per-run (e.g., use "<your-unique-email@example.com>", "<your-password>",
"<your-username>", "<verification-token-from-email-or-apalis>"); ensure the keys
email, password, username, and verificationToken remain present but set to these
explicit placeholder strings so testers are forced to supply an unregistered
email and a valid verificationToken before running.
In `@apps/backend/Cargo.toml`:
- Around line 51-54: The apalis crate versions are inconsistent (apalis = rc.9
vs apalis-postgres/apalis-board = rc.8); update the Cargo.toml so the
apalis-family crates use the same RC (e.g., set apalis-postgres and apalis-board
to "1.0.0-rc.9" or pin apalis to "1.0.0-rc.8"), then run cargo update to
regenerate Cargo.lock and verify no duplicate resolutions; additionally check
apps/backend/migration/Cargo.lock to ensure it doesn't introduce a different
apalis RC and reconcile it if needed.
In `@apps/backend/src/entities/personal_tokens.rs`:
- Line 4: このファイルの不要なインポートを削除してください — 現在の行で宣言されている `use
crate::entities::scopes::Scope;` は `scopes`
フィールドを削除したため未使用になっています。該当インポートを取り除き、ビルド/コンパイル時の未使用警告を解消してください（参照箇所: `Scope`
インポート、`personal_tokens.rs` 内の `scopes` 関連コードが既に削除されていることを前提）。
In `@apps/backend/src/jobs/verification_email.rs`:
- Around line 65-73: The process function race allows an old retry job to
overwrite a newer token because process always calls
email_verification::store_token first; add a generation/issued_at field to
VerificationEmailJob (e.g., job.generation or job.issued_at) and change
email_verification::store_token to perform a conditional/atomic update that only
writes the token if the provided generation is strictly newer than the stored
one (use Redis compare-and-set/transaction or server-side check) so old jobs
cannot re-enable older tokens; update process to pass the new generation field
when calling store_token and ensure
verification_email_delivery::send_verification_email still receives the token
only after a successful conditional store.
In `@apps/backend/src/server.rs`:
- Around line 36-48: Extract the duplicated EnvFilter construction by first
computing the filter string once (e.g., let filter_str =
std::env::var("RUST_LOG").unwrap_or_else(|_| "info,sqlx=warn".into())), then
build a tracing_subscriber::EnvFilter from that value and reuse it in both
places (the TracingSubscriber::new(&broadcaster).layer().with_filter(...) that
produces board_tracing and the registry().with(...) call), using cloning if
necessary; update references to use the single env_filter variable instead of
creating EnvFilter twice.
In `@apps/backend/src/settings.rs`:
- Around line 25-27: The field verification_email_worker_concurrency currently
accepts 0 which can stop worker execution; add validation to ensure its value is
>= 1 during settings load/validation (e.g., in Settings::validate or
Settings::try_from after deserialization) and return a clear error (or panic on
invalid config) if it's < 1; reference the verification_email_worker_concurrency
field and the default_verification_email_worker_concurrency helper when
implementing the check so startup will fail fast when configuration is invalid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 90ed11e3-06b7-4dce-853f-be61e2f57c56
⛔ Files ignored due to path filters (1)
apps/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.vscode/extensions.jsonapps/backend/.env.exampleapps/backend/Cargo.tomlapps/backend/bruno/01-register-verify/01-register.bruapps/backend/bruno/01-register-verify/02-verify-email.bruapps/backend/bruno/01-register-verify/03-login-unverified.bruapps/backend/bruno/01-register-verify/04-resend-verification.bruapps/backend/bruno/01-register-verify/folder.bruapps/backend/bruno/02-login/01-login.bruapps/backend/bruno/02-login/02-me.bruapps/backend/bruno/02-login/03-logout.bruapps/backend/bruno/02-login/folder.bruapps/backend/bruno/03-apalis/01-list-tasks.bruapps/backend/bruno/03-apalis/folder.bruapps/backend/bruno/README.mdapps/backend/bruno/bruno.jsonapps/backend/bruno/collection.bruapps/backend/bruno/environments/local.bruapps/backend/src/entities/personal_tokens.rsapps/backend/src/handlers/auth.rsapps/backend/src/jobs/mod.rsapps/backend/src/jobs/verification_email.rsapps/backend/src/lib.rsapps/backend/src/main.rsapps/backend/src/server.rsapps/backend/src/settings.rsapps/backend/src/utils/mod.rs
💤 Files with no reviewable changes (1)
- apps/backend/src/utils/mod.rs
Comment on lines
+23
to
+26
| vars:pre-request { | ||
| verificationToken: trs1GDUsdryLKl-ckR2sVGMHWJIf5PaKwGaw2BOiTIY | ||
| baseUrl: http://localhost:3400 | ||
| } |
There was a problem hiding this comment.
検証トークンの固定値が手順を破壊しています。
verificationToken を pre-request で固定すると、Env 設定（または最新トークン取得）が上書きされ、期限切れ/不一致で検証が失敗します。token は environments/local.bru または実行時更新値のみを使うようにしてください。
As per coding guidelines "Set required environment variables in environments/local.bru: baseUrl, email, password, username, verificationToken".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/bruno/01-register-verify/02-verify-email.bru` around lines 23 -
26, The pre-request block currently hardcodes verificationToken
(vars:pre-request { verificationToken: ... }), which overrides
environment/runtime values and causes expiry/mismatch failures; remove the
verificationToken entry from the pre-request vars so the flow uses the value
from environments/local.bru or the runtime-updated token, leaving baseUrl (and
other required entries) intact and ensuring verificationToken is set only in
environments/local.bru or updated at runtime.
Comment on lines
+24
to
+27
| docs { | ||
| メール確認前に実行すると 403 email-not-verified が返る想定。 | ||
| 確認後は 4. ログイン を使う。 | ||
| } |
There was a problem hiding this comment.
未認証ログインの期待結果をテストで固定してください。
docs に期待値はありますが、tests が無いため回帰を検出できません。403 とエラーコード（email-not-verified）の検証を追加してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/bruno/01-register-verify/03-login-unverified.bru` around lines
24 - 27, Add a concrete test that asserts the unverified-login behavior
described in the docs: create or extend the tests block for
03-login-unverified.bru to perform the login request for an unverified email and
assert the HTTP status is 403 and the returned error code equals
"email-not-verified"; locate the test near the existing docs block in this file,
invoke the same login endpoint used by the scenario, and include assertions for
both the status and response error.code to ensure future regressions are caught.
Comment on lines
+3
to
+6
| email: test-user@example.com | ||
| password: password123 | ||
| username: testuser | ||
| verificationToken: |
There was a problem hiding this comment.
local.bru の初期値を再実行しやすいプレースホルダにしてください。
email の固定値は再実行時に登録済み衝突を起こしやすく、verificationToken の空値は確認リクエスト失敗の原因になります。初期値は「毎回埋める前提」の明示的なプレースホルダにしておくのが安全です。
🔧 修正例
vars {
baseUrl: http://localhost:3400
- email: test-user@example.com
+ email: your-unregistered-email@example.com
password: password123
username: testuser
- verificationToken:
+ verificationToken: paste-token-from-email-or-apalis-board
}As per coding guidelines "Set required environment variables in environments/local.bru: baseUrl, email (unregistered), password, username, verificationToken (from email or Apalis Board)". Based on learnings, same-email retests can conflict unless changed or DB is reset.
📝 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.
Suggested change
| email: test-user@example.com | |
| password: password123 | |
| username: testuser | |
| verificationToken: | |
| vars { | |
| baseUrl: http://localhost:3400 | |
| email: your-unregistered-email@example.com | |
| password: password123 | |
| username: testuser | |
| verificationToken: paste-token-from-email-or-apalis-board | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/bruno/environments/local.bru` around lines 3 - 6, Replace the
hardcoded/reused values in environments/local.bru with explicit placeholders so
re-runs don't collide: update the email, password, username, and
verificationToken entries to clearly indicate they must be filled per-run (e.g.,
use "<your-unique-email@example.com>", "<your-password>", "<your-username>",
"<verification-token-from-email-or-apalis>"); ensure the keys email, password,
username, and verificationToken remain present but set to these explicit
placeholder strings so testers are forced to supply an unregistered email and a
valid verificationToken before running.
Comment on lines
+51
to
+54
| apalis = { version = "1.0.0-rc.9", features = ["limit", "retry", "tracing"] } | ||
| apalis-postgres = { version = "1.0.0-rc.8", default-features = false, features = ["migrate", "tokio-comp", "time"] } | ||
| sqlx = { version = "0.8", features = ["runtime-tokio-rustls", "postgres", "time"] } | ||
| apalis-board = { version = "1.0.0-rc.8", features = ["axum", "ui", "events"] } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
LOCK_FILE="$(fd -HI '^Cargo.lock$' | head -n1 || true)"
if [ -z "${LOCK_FILE}" ]; then
echo "Cargo.lock が見つかりません。"
exit 0
fi
python - <<'PY'
import pathlib, tomllib
lock = pathlib.Path("Cargo.lock")
data = tomllib.loads(lock.read_text())
targets = {"apalis", "apalis-postgres", "apalis-board"}
rows = [(p["name"], p["version"]) for p in data.get("package", []) if p.get("name") in targets]
if not rows:
print("対象クレートが Cargo.lock に見つかりませんでした。")
raise SystemExit(0)
print("Resolved versions:")
for name, version in sorted(rows):
print(f" {name}: {version}")
from collections import defaultdict
vers = defaultdict(set)
for n, v in rows:
vers[n].add(v)
multi = {k: sorted(v) for k, v in vers.items() if len(v) > 1}
if multi:
print("\n同一クレートで複数バージョンが解決されています:")
for k, v in multi.items():
print(f" {k}: {v}")
PYRepository: TeamBlackCrystal/task
Length of output: 602
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate any Cargo.lock in repo (limit output)
echo "Searching for Cargo.lock files..."
fd -HI 'Cargo.lock$' . || true
# Also try common locations under apps/backend
echo
echo "Searching under apps/backend..."
fd -HI 'Cargo.lock$' apps/backend . || true
# If any are found, check versions from the first one
LOCKS=()
while IFS= read -r f; do LOCKS+=("$f"); done < <(fd -HI 'Cargo.lock$' . || true)
if [ ${`#LOCKS`[@]} -eq 0 ]; then
echo
echo "No Cargo.lock found anywhere in repo."
exit 0
fi
LOCK_FILE="${LOCKS[0]}"
echo
echo "Using lock file: $LOCK_FILE"
python3 - <<PY
import pathlib, tomllib, sys
lock_path = pathlib.Path("$LOCK_FILE")
data = tomllib.loads(lock_path.read_text())
targets = {"apalis", "apalis-postgres", "apalis-board"}
rows=[]
for p in data.get("package", []):
n=p.get("name")
if n in targets:
rows.append((n, p.get("version")))
if not rows:
print("対象クレートが Cargo.lock に見つかりませんでした。")
sys.exit(0)
print("Resolved versions (from Cargo.lock):")
for name, version in sorted(rows):
print(f" {name}: {version}")
from collections import defaultdict
vers=defaultdict(set)
for n,v in rows:
vers[n].add(v)
multi={k:sorted(list(v)) for k,v in vers.items() if len(v)>1}
if multi:
print("\\n同一クレートで複数バージョンが解決されています:")
for k,v in multi.items():
print(f" {k}: {v}")
else:
print("\\napalis 系は単一バージョンで解決されています。")
PYRepository: TeamBlackCrystal/task
Length of output: 390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate any Cargo.lock in repo (limit output)
echo "Searching for Cargo.lock files..."
fd -HI 'Cargo.lock$' . || true
# Also try common locations under apps/backend
echo
echo "Searching under apps/backend..."
fd -HI 'Cargo.lock$' apps/backend . || true
# If any are found, check versions from the first one
LOCKS=()
while IFS= read -r f; do LOCKS+=("$f"); done < <(fd -HI 'Cargo.lock$' . || true)
if [ ${`#LOCKS`[@]} -eq 0 ]; then
echo
echo "No Cargo.lock found anywhere in repo."
exit 0
fi
LOCK_FILE="${LOCKS[0]}"
echo
echo "Using lock file: $LOCK_FILE"
python3 - <<PY
import pathlib, tomllib, sys
lock_path = pathlib.Path("$LOCK_FILE")
data = tomllib.loads(lock_path.read_text())
targets = {"apalis", "apalis-postgres", "apalis-board"}
rows=[]
for p in data.get("package", []):
n=p.get("name")
if n in targets:
rows.append((n, p.get("version")))
if not rows:
print("対象クレートが Cargo.lock に見つかりませんでした。")
sys.exit(0)
print("Resolved versions (from Cargo.lock):")
for name, version in sorted(rows):
print(f" {name}: {version}")
from collections import defaultdict
vers=defaultdict(set)
for n,v in rows:
vers[n].add(v)
multi={k:sorted(list(v)) for k,v in vers.items() if len(v)>1}
if multi:
print("\\n同一クレートで複数バージョンが解決されています:")
for k,v in multi.items():
print(f" {k}: {v}")
else:
print("\\napalis 系は単一バージョンで解決されています。")
PYRepository: TeamBlackCrystal/task
Length of output: 390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Identify lock files explicitly (avoid process substitution issues)
LOCKS=(
"apps/backend/Cargo.lock"
"apps/backend/migration/Cargo.lock"
)
PY_SCRIPT=$(cat <<'PY'
import pathlib, tomllib, sys
targets = {"apalis", "apalis-postgres", "apalis-board"}
def check(lock_path: str):
p = pathlib.Path(lock_path)
data = tomllib.loads(p.read_text())
rows=[]
for pkg in data.get("package", []):
n = pkg.get("name")
if n in targets:
rows.append((n, pkg.get("version")))
if not rows:
print(f"{lock_path}: 対象クレートが Cargo.lock に見つかりませんでした。")
return
print(f"{lock_path}: Resolved versions:")
for name, version in sorted(rows):
print(f" {name}: {version}")
from collections import defaultdict
vers=defaultdict(set)
for n,v in rows:
vers[n].add(v)
multi={k:sorted(list(v)) for k,v in vers.items() if len(v)>1}
if multi:
print(" 同一クレートで複数バージョンが解決されています:")
for k,v in multi.items():
print(f" {k}: {v}")
else:
print(" apalis 系は単一バージョンで解決されています。")
print()
for path in sys.argv[1:]:
check(path)
PY
)
python3 - <<PY
$PY_SCRIPT
PY "${LOCKS[@]}"Repository: TeamBlackCrystal/task
Length of output: 346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, tomllib
targets = {"apalis", "apalis-postgres", "apalis-board"}
lock_paths = ["apps/backend/Cargo.lock", "apps/backend/migration/Cargo.lock"]
def check(lock_path: str):
p = pathlib.Path(lock_path)
if not p.exists():
print(f"{lock_path}: Cargo.lock が存在しません")
return
data = tomllib.loads(p.read_text())
rows=[]
for pkg in data.get("package", []):
n = pkg.get("name")
if n in targets:
rows.append((n, pkg.get("version")))
if not rows:
print(f"{lock_path}: 対象クレートが Cargo.lock に見つかりませんでした。")
return
print(f"{lock_path}: Resolved versions:")
for name, version in sorted(rows):
print(f" {name}: {version}")
from collections import defaultdict
vers=defaultdict(set)
for n,v in rows:
vers[n].add(v)
multi={k:sorted(list(v)) for k,v in vers.items() if len(v)>1}
if multi:
print(" 同一クレートで複数バージョンが解決されています:")
for k,v in multi.items():
print(f" {k}: {v}")
else:
print(" apalis 系は単一バージョンで解決されています。")
print()
for lp in lock_paths:
check(lp)
PYRepository: TeamBlackCrystal/task
Length of output: 1458
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== migration/Cargo.lock line 780-840 =="
nl -ba apps/backend/migration/Cargo.lock | sed -n '780,840p'Repository: TeamBlackCrystal/task
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== migration/Cargo.lock line 780-840 =="
sed -n '780,840p' apps/backend/migration/Cargo.lockRepository: TeamBlackCrystal/task
Length of output: 1717
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, tomllib
lock_path = pathlib.Path("apps/backend/Cargo.lock")
data = tomllib.loads(lock_path.read_text())
targets = {("apalis", "1.0.0-rc.9"),
("apalis-postgres", "1.0.0-rc.8"),
("apalis-board", "1.0.0-rc.8")}
# Build quick lookup
pkgs = {(p.get("name"), p.get("version")): p for p in data.get("package", [])}
def show(name, ver):
p = pkgs.get((name, ver))
if not p:
print(f"Not found: {name} {ver}")
return
deps = p.get("dependencies", [])
print(f"\n{name} {ver} dependencies ({len(deps)}):")
for d in deps:
print(f" {d}")
show("apalis", "1.0.0-rc.9")
show("apalis-postgres", "1.0.0-rc.8")
show("apalis-board", "1.0.0-rc.8")
PYRepository: TeamBlackCrystal/task
Length of output: 1964
Apalis 系の RC を揃えるか、Cargo.lock での解決（重複解決の有無）を確認
apps/backend/Cargo.lock では apalis = 1.0.0-rc.9、apalis-postgres = 1.0.0-rc.8、apalis-board = 1.0.0-rc.8 が解決されており、少なくとも同一クレート名としての apalis の重複解決は確認できませんでした。それでも rc.8 / rc.9 間の API 互換性で問題が出る可能性があるため、可能なら apalis-family の RC を揃えるか、apps/backend/migration/Cargo.lock 側でも同様に apalis が重複解決していないことを確認してください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/Cargo.toml` around lines 51 - 54, The apalis crate versions are
inconsistent (apalis = rc.9 vs apalis-postgres/apalis-board = rc.8); update the
Cargo.toml so the apalis-family crates use the same RC (e.g., set
apalis-postgres and apalis-board to "1.0.0-rc.9" or pin apalis to "1.0.0-rc.8"),
then run cargo update to regenerate Cargo.lock and verify no duplicate
resolutions; additionally check apps/backend/migration/Cargo.lock to ensure it
doesn't introduce a different apalis RC and reconcile it if needed.
Comment on lines
+65
to
+73
| pub async fn process(job: VerificationEmailJob, state: Data<AppState>) -> Result<(), BoxDynError> { | ||
| email_verification::store_token(&state.redis_client, job.user_id, &job.token).await?; | ||
| verification_email_delivery::send_verification_email( | ||
| &state.smtp_client, | ||
| &job.email, | ||
| &state.settings, | ||
| &job.token, | ||
| ) | ||
| .await?; |
There was a problem hiding this comment.
古いリトライジョブが新しい検証トークンを上書きできる競合があります。
process が毎回 store_token を先に実行するため、再送で新トークンが発行済みでも、失敗リトライ中の旧ジョブが後続実行されると旧トークンを再有効化し得ます。結果として最新リンクが無効化される競合が起きます。
ジョブに発行世代（例: issued_at/generation）を持たせ、store_token 側で「現在値より新しい世代のみ更新」を強制してください。
As per coding guidelines apps/backend/**/*.rs: 「非同期処理の安全性を優先して確認してください。」
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/jobs/verification_email.rs` around lines 65 - 73, The
process function race allows an old retry job to overwrite a newer token because
process always calls email_verification::store_token first; add a
generation/issued_at field to VerificationEmailJob (e.g., job.generation or
job.issued_at) and change email_verification::store_token to perform a
conditional/atomic update that only writes the token if the provided generation
is strictly newer than the stored one (use Redis compare-and-set/transaction or
server-side check) so old jobs cannot re-enable older tokens; update process to
pass the new generation field when calling store_token and ensure
verification_email_delivery::send_verification_email still receives the token
only after a successful conditional store.
Comment on lines
+36
to
48
| let board_tracing = TracingSubscriber::new(&broadcaster).layer().with_filter( | ||
| tracing_subscriber::EnvFilter::new( | ||
| std::env::var("RUST_LOG").unwrap_or_else(|_| "info,sqlx=warn".into()), | ||
| ), | ||
| ); | ||
|
|
||
| tracing_subscriber::registry() | ||
| .with(tracing_subscriber::EnvFilter::new( | ||
| std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()), | ||
| std::env::var("RUST_LOG").unwrap_or_else(|_| "info,sqlx=warn".into()), | ||
| )) | ||
| .with(tracing_subscriber::fmt::layer()) | ||
| .with(board_tracing) | ||
| .init(); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
EnvFilter の作成ロジックが重複しています。
同一のフィルタ文字列で EnvFilter を2回生成しています。可読性向上のため変数に抽出できます。
リファクタ案
+ let env_filter = || {
+ tracing_subscriber::EnvFilter::new(
+ std::env::var("RUST_LOG").unwrap_or_else(|_| "info,sqlx=warn".into()),
+ )
+ };
+
let broadcaster = TracingBroadcaster::create();
- let board_tracing = TracingSubscriber::new(&broadcaster).layer().with_filter(
- tracing_subscriber::EnvFilter::new(
- std::env::var("RUST_LOG").unwrap_or_else(|_| "info,sqlx=warn".into()),
- ),
- );
+ let board_tracing = TracingSubscriber::new(&broadcaster)
+ .layer()
+ .with_filter(env_filter());
tracing_subscriber::registry()
- .with(tracing_subscriber::EnvFilter::new(
- std::env::var("RUST_LOG").unwrap_or_else(|_| "info,sqlx=warn".into()),
- ))
+ .with(env_filter())
.with(tracing_subscriber::fmt::layer())
.with(board_tracing)
.init();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/server.rs` around lines 36 - 48, Extract the duplicated
EnvFilter construction by first computing the filter string once (e.g., let
filter_str = std::env::var("RUST_LOG").unwrap_or_else(|_|
"info,sqlx=warn".into())), then build a tracing_subscriber::EnvFilter from that
value and reuse it in both places (the
TracingSubscriber::new(&broadcaster).layer().with_filter(...) that produces
board_tracing and the registry().with(...) call), using cloning if necessary;
update references to use the single env_filter variable instead of creating
EnvFilter twice.
Comment on lines
+25
to
+27
| /// 認証メール Apalis ワーカーの並列度 | ||
| #[serde(default = "default_verification_email_worker_concurrency")] | ||
| pub verification_email_worker_concurrency: usize, |
There was a problem hiding this comment.
verification_email_worker_concurrency に >= 1 の検証を追加してください。
0 が設定されても現在は通過するため、ワーカー実行が止まり、認証メール配信フローが実質停止する設定ミスを起動時に検出できません。
修正案
/// 認証メール Apalis ワーカーの並列度
+ #[validate(range(
+ min = 1,
+ message = "verification_email_worker_concurrency must be >= 1"
+ ))]
#[serde(default = "default_verification_email_worker_concurrency")]
pub verification_email_worker_concurrency: usize,As per coding guidelines apps/backend/**/*.rs: 「非同期処理の安全性を優先して確認してください」。
📝 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.
Suggested change
| /// 認証メール Apalis ワーカーの並列度 | |
| #[serde(default = "default_verification_email_worker_concurrency")] | |
| pub verification_email_worker_concurrency: usize, | |
| /// 認証メール Apalis ワーカーの並列度 | |
| #[validate(range( | |
| min = 1, | |
| message = "verification_email_worker_concurrency must be >= 1" | |
| ))] | |
| #[serde(default = "default_verification_email_worker_concurrency")] | |
| pub verification_email_worker_concurrency: usize, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/settings.rs` around lines 25 - 27, The field
verification_email_worker_concurrency currently accepts 0 which can stop worker
execution; add validation to ensure its value is >= 1 during settings
load/validation (e.g., in Settings::validate or Settings::try_from after
deserialization) and return a clear error (or panic on invalid config) if it's <
1; reference the verification_email_worker_concurrency field and the
default_verification_email_worker_concurrency helper when implementing the check
so startup will fail fast when configuration is invalid.
…w with associated API requests
…enhanced scope management
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/backend/src/handlers/auth.rs (1)
236-275: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winメールアドレスの正規化が不十分です
resend_verification_emailではtrim()のみ適用していますが、registerでは正規化なしで保存しています。PostgreSQL のデフォルトでは大文字小文字を区別するため、User@Example.comで登録後にuser@example.comで再送リクエストするとユーザーが見つからず 404 になります。登録時と再送時の両方でメールアドレスを小文字に正規化することを検討してください。
♻️ 修正案
registerとresend_verification_emailの両方で統一した正規化を適用：pub async fn register( State(state): State<AppState>, Valid(Json(payload)): Valid<Json<RegisterRequest>>, ) -> Result<(StatusCode, Json<String>), AuthError> { let RegisterRequest { username, email, password, } = payload; + let email = email.trim().to_lowercase(); let password_hash = create_password_hash(&password)?;pub async fn resend_verification_email( State(state): State<AppState>, Valid(Json(payload)): Valid<Json<ResendVerificationRequest>>, ) -> Result<Json<String>, AuthError> { - let email = payload.email.trim().to_string(); + let email = payload.email.trim().to_lowercase();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/handlers/auth.rs` around lines 236 - 275, The resend_verification_email handler only trims the email but does not lowercase it, causing mismatches with registrations; update both resend_verification_email and the register path to apply a consistent normalization (trim() then to_lowercase()) to the user's email before saving and before any lookups or rate-limit checks (e.g., when calling email_verification::try_acquire_resend_slot, users::Entity::find filter on users::Column::Email, and when enqueuing VerificationEmailJob) so stored emails and query inputs always use the same canonical lowercase form.
♻️ Duplicate comments (3)
apps/backend/src/settings.rs (1)
25-27: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
verification_email_worker_concurrencyは起動時に>= 1を検証した方が安全です。現状は
0設定でも受理され、後段で暗黙に丸められるため設定ミスを見逃します。設定ロード時に明示的に弾くと運用時の切り分けが容易です。As per coding guidelines
apps/backend/**/*.rs: 「非同期処理の安全性を優先して確認してください」。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/settings.rs` around lines 25 - 27, Add an explicit runtime validation that verification_email_worker_concurrency is >= 1 when settings are loaded: in the Settings type (the struct containing verification_email_worker_concurrency) or its settings-load helper, check the field after deserialization (use the existing default_verification_email_worker_concurrency for fallback) and return an error / panic with a clear message if the value is 0; reference the verification_email_worker_concurrency field and default_verification_email_worker_concurrency function so the check is colocated with settings deserialization/creation (or expose a Settings::validate() called after load) to fail fast on invalid configuration.apps/backend/src/jobs/verification_email.rs (1)
20-24:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift古いリトライジョブが新しい検証トークンを再上書きできる競合リスクがあります。
process()が毎回トークン保存を実行し、ジョブ側に世代情報（issued_at/generation）がないため、再送後に遅延した旧ジョブが走ると最新トークンを巻き戻す可能性があります。トークン保存は「より新しい世代のみ反映」の原子的条件付き更新にしてください。As per coding guidelines
apps/backend/**/*.rs: 「認証・認可、非同期処理の安全性を優先して確認してください」。#!/bin/bash set -euo pipefail echo "== VerificationEmailJob definition ==" rg -n --type rust -C3 '\bstruct\s+VerificationEmailJob\b|\buser_id\b|\bemail\b|\btoken\b' echo echo "== store_token definition and call sites ==" rg -n --type rust -C4 '\bstore_token\s*\(' echo echo "== resend/enqueue paths for verification jobs ==" rg -n --type rust -C4 '\benqueue\s*\(|resend|verification_email'Also applies to: 65-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/jobs/verification_email.rs` around lines 20 - 24, The VerificationEmailJob struct and its process() currently overwrite tokens unconditionally, allowing stale retry jobs to roll back a newer token; update the token persistence (store_token and where process() calls it) to perform an atomic conditional update that only writes when the incoming token is newer (compare an issued_at timestamp or a numeric generation on the user/token row). Concretely: add a generation/issued_at field to VerificationEmailJob and include that value when calling store_token, then change store_token to execute a single atomic DB update (e.g., UPDATE ... SET token=?, generation=? WHERE user_id=? AND (generation IS NULL OR generation < ?) or use a conditional INSERT/UPDATE in a transaction/UPSERT) so old jobs cannot overwrite newer tokens; ensure process(), VerificationEmailJob, and store_token signatures are updated consistently.apps/backend/bruno/environments/local.bru (1)
3-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win再実行時に衝突しやすい初期値です。
verificationToken空値はそのまま実行時に失敗要因になります。埋め忘れ防止のプレースホルダにしてください。🔧 修正例
vars { baseUrl: http://localhost:3400 - email: test-user@example.com + email: your-unregistered-email@example.com password: password123 username: testuser - verificationToken: + verificationToken: paste-token-from-email-or-apalis-board }As per coding guidelines
apps/backend/bruno/**/environments/local.bru: Bruno environment variable🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/bruno/environments/local.bru` around lines 3 - 6, Replace the hard-coded email and empty verificationToken placeholders in the environment block: change the email field from a fixed address to a non-conflicting placeholder or pattern (e.g., a template like your-email+timestamp@example.com) so tests won’t collide on re-runs, and set verificationToken to an explicit placeholder string (e.g., <verification-token> or a generated token) instead of leaving it empty; update the email and verificationToken entries referenced as "email" and "verificationToken" in the local.bru environment to these placeholder values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/bruno/01-register-verify/04-resend-verification.bru`:
- Line 24: vars:pre-request に固定で書かれている baseUrl: http://localhost:3400 を残さないでください
— 代わりにリクエストで参照している変数名 {{baseUrl}} を使するか、該当の baseUrl エントリ自体を削除してください（対象の識別子:
baseUrl, vars:pre-request, {{baseUrl}}）。これによりローカル固定値が環境変数を上書きするリスクを解消します。
---
Outside diff comments:
In `@apps/backend/src/handlers/auth.rs`:
- Around line 236-275: The resend_verification_email handler only trims the
email but does not lowercase it, causing mismatches with registrations; update
both resend_verification_email and the register path to apply a consistent
normalization (trim() then to_lowercase()) to the user's email before saving and
before any lookups or rate-limit checks (e.g., when calling
email_verification::try_acquire_resend_slot, users::Entity::find filter on
users::Column::Email, and when enqueuing VerificationEmailJob) so stored emails
and query inputs always use the same canonical lowercase form.
---
Duplicate comments:
In `@apps/backend/bruno/environments/local.bru`:
- Around line 3-6: Replace the hard-coded email and empty verificationToken
placeholders in the environment block: change the email field from a fixed
address to a non-conflicting placeholder or pattern (e.g., a template like
your-email+timestamp@example.com) so tests won’t collide on re-runs, and set
verificationToken to an explicit placeholder string (e.g., <verification-token>
or a generated token) instead of leaving it empty; update the email and
verificationToken entries referenced as "email" and "verificationToken" in the
local.bru environment to these placeholder values.
In `@apps/backend/src/jobs/verification_email.rs`:
- Around line 20-24: The VerificationEmailJob struct and its process() currently
overwrite tokens unconditionally, allowing stale retry jobs to roll back a newer
token; update the token persistence (store_token and where process() calls it)
to perform an atomic conditional update that only writes when the incoming token
is newer (compare an issued_at timestamp or a numeric generation on the
user/token row). Concretely: add a generation/issued_at field to
VerificationEmailJob and include that value when calling store_token, then
change store_token to execute a single atomic DB update (e.g., UPDATE ... SET
token=?, generation=? WHERE user_id=? AND (generation IS NULL OR generation < ?)
or use a conditional INSERT/UPDATE in a transaction/UPSERT) so old jobs cannot
overwrite newer tokens; ensure process(), VerificationEmailJob, and store_token
signatures are updated consistently.
In `@apps/backend/src/settings.rs`:
- Around line 25-27: Add an explicit runtime validation that
verification_email_worker_concurrency is >= 1 when settings are loaded: in the
Settings type (the struct containing verification_email_worker_concurrency) or
its settings-load helper, check the field after deserialization (use the
existing default_verification_email_worker_concurrency for fallback) and return
an error / panic with a clear message if the value is 0; reference the
verification_email_worker_concurrency field and
default_verification_email_worker_concurrency function so the check is colocated
with settings deserialization/creation (or expose a Settings::validate() called
after load) to fail fast on invalid configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d790fd2d-1583-4603-b205-c3d63a3ff171
⛔ Files ignored due to path filters (1)
apps/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.vscode/extensions.jsonapps/backend/.env.exampleapps/backend/Cargo.tomlapps/backend/bruno/01-register-verify/01-register.bruapps/backend/bruno/01-register-verify/02-verify-email.bruapps/backend/bruno/01-register-verify/03-login-unverified.bruapps/backend/bruno/01-register-verify/04-resend-verification.bruapps/backend/bruno/01-register-verify/folder.bruapps/backend/bruno/02-login/01-login.bruapps/backend/bruno/02-login/02-me.bruapps/backend/bruno/02-login/03-logout.bruapps/backend/bruno/02-login/folder.bruapps/backend/bruno/03-apalis/01-list-tasks.bruapps/backend/bruno/03-apalis/folder.bruapps/backend/bruno/README.mdapps/backend/bruno/bruno.jsonapps/backend/bruno/collection.bruapps/backend/bruno/environments/local.bruapps/backend/src/entities/personal_tokens.rsapps/backend/src/handlers/auth.rsapps/backend/src/jobs/mod.rsapps/backend/src/jobs/verification_email.rsapps/backend/src/lib.rsapps/backend/src/main.rsapps/backend/src/server.rsapps/backend/src/settings.rsapps/backend/src/utils/mod.rs
💤 Files with no reviewable changes (1)
- apps/backend/src/utils/mod.rs
| } | ||
|
|
||
| vars:pre-request { | ||
| baseUrl: http://localhost:3400 |
There was a problem hiding this comment.
baseUrl をリクエスト内で固定しないでください。
vars:pre-request の baseUrl 固定値が環境変数を上書きし、環境切り替え時にこのリクエストだけ localhost を向くリスクがあります。{{baseUrl}} 参照に統一するか、このブロック自体を削除してください。
修正案（例）
vars:pre-request {
- baseUrl: http://localhost:3400
+ baseUrl: "{{baseUrl}}"
}📝 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.
Suggested change
| baseUrl: http://localhost:3400 | |
| vars:pre-request { | |
| baseUrl: "{{baseUrl}}" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/bruno/01-register-verify/04-resend-verification.bru` at line 24,
vars:pre-request に固定で書かれている baseUrl: http://localhost:3400 を残さないでください —
代わりにリクエストで参照している変数名 {{baseUrl}} を使するか、該当の baseUrl エントリ自体を削除してください（対象の識別子:
baseUrl, vars:pre-request, {{baseUrl}}）。これによりローカル固定値が環境変数を上書きするリスクを解消します。
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
1 participant
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.
メール認証フロー（フロント含む）
sequenceDiagram autonumber actor U as ユーザー participant F as フロント(Web) participant B as API(バックエンド) participant A as Apalisワーカー<br/>(同一プロセス) participant Q as PostgreSQL<br/>(Apalis ジョブキュー) participant DB as PostgreSQL<br/>(アプリ DB) participant R as Redis participant S as SMTPサーバ participant M as メール rect rgb(240, 248, 255) Note over U,F: アカウント作成 U->>F: 登録情報を入力 F->>B: POST /v1/auth/register B->>DB: トランザクション<br/>ユーザー作成（未認証） B->>Q: Apalis にジョブ投入<br/>(user_id, email, token) B->>F: 201 Created F->>U: メール確認の案内を表示 par バックグラウンド送信（Apalis） A->>Q: ジョブ取得（NOTIFY + ポーリング） A->>R: 認証トークン保存（TTL 約15分・Lua で原子的） A->>S: 認証メール送信 S->>M: 配信 A->>Q: ジョブ完了（成功） and 送信失敗時 A->>Q: リトライ（最大 8 回・Apalis RetryPolicy） Note over A,Q: 失敗が続くとジョブは failed 相当で打ち切り end end rect rgb(245, 255, 245) Note over U,F: メールのリンクからフロントへ U->>M: メール受信・リンクをクリック M-->>U: ブラウザでフロントを開く<br/>（token は URL エンコード済み） U->>F: /verify-email?token=... でアクセス F->>F: URL から token を取得（デコード） end rect rgb(255, 250, 240) Note over F,B: 認証完了（副作用は API が担当） F->>B: POST /v1/auth/verify-email（JSON: token） B->>R: トークン検証・消費（GETDEL + Lua） alt トークン有効 B->>DB: email_verified = true に更新 B->>F: 200 OK F->>U: 認証完了の表示 else 無効・期限切れ B->>F: 400 F->>U: エラー／再送案内 end end rect rgb(255, 245, 245) Note over U,F: 認証メールの再送（任意） U->>F: 再送を依頼 F->>B: POST /v1/auth/resend-verification-email B->>R: 再送間隔チェック（SET NX・60秒） B->>DB: ユーザー状態を確認 alt 送信可能な未認証ユーザー B->>Q: Apalis にジョブ投入（新トークン） B->>F: 200 OK F->>U: 「送信しました」等 A->>Q: ジョブ取得 A->>R: 新トークン保存（旧トークンは Lua で無効化） A->>S: メール再送 S->>M: 配信 A->>Q: ジョブ完了 else 該当なし／既認証など B->>F: 404 / 429 / 409 F->>U: 案内のみ（詳細は API 設計どおり） end end rect rgb(248, 248, 255) Note over U,F: ログイン（認証後） U->>F: メール・PWでログイン F->>B: POST /v1/auth/login alt 未メール確認 B->>F: 403（message: email-not-verified） F->>U: メール確認の案内 else メール確認済み B->>F: 204 No Content（セッション Cookie） F->>U: ログイン後画面へ end endSummary by CodeRabbit
リリースノート
New Features
Bug Fixes
Documentation
Tests