fix: ログイン API の失敗応答を揃え、回数制限をログイン API にかける - #1928
Conversation
- 認証情報が誤っている場合の応答を1種類にまとめる - 不正なリクエスト本文は 400 を返す - REST アプリでも共通の limiter を初期化する Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
直前のコミットは、UI 側と共有の limiter を /api アプリにも初期化したため、 既定の制限(WEKO_API_LIMIT_RATE_DEFAULT)が /api 配下の全経路にかかって いた。検索・統計・ファイル・IIIF の画像などは、同じ IP を共有する環境で 通常の閲覧でも制限に達しうる。 - 既定の制限を持たないログイン専用の Limiter を /api アプリにだけ初期化し、 ログイン API にだけ付ける。制限値は WEKO_API_LIMIT_RATE_DEFAULT を使う - 共有の limiter は従来どおり /api アプリには初期化しない - Flask-Limiter はビュー関数の名前で制限を照合するため、MethodView の メソッドではなく decorators 属性で as_view() の関数に付ける (メソッドに付けた従来の指定は名前が合わず効いていなかった) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
Reviewer's GuideThe PR hardens the login API by unifying authentication failure responses, converting malformed requests from server errors to 400 responses, and enforcing a dedicated configurable per-IP rate limit only on login while preserving unrestricted access to other REST routes. Sequence diagram for hardened login API flowsequenceDiagram
actor Client
participant LoginAPI
participant UserStore
participant PasswordHash
Client->>LoginAPI: POST login
LoginAPI->>LoginAPI: request.get_json(silent=True)
alt malformed body or missing/empty fields
LoginAPI-->>Client: 400 InvalidLoginRequestError
else valid credentials format
LoginAPI->>UserStore: User.query.filter_by(email=email).first()
alt user missing or password unset
LoginAPI->>PasswordHash: hash_password(password)
LoginAPI-->>Client: 403 InvalidCredentialsError
else password mismatch
LoginAPI->>PasswordHash: verify_password(password, user.password)
LoginAPI-->>Client: 403 InvalidCredentialsError
else valid password
LoginAPI->>PasswordHash: verify_password(password, user.password)
LoginAPI-->>Client: successful login
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
🔍 Claude レビュー統合指摘はありません。 次にすること: 外部データにあるのは要約とレビュー停止の通知だけで、裁定すべき指摘スレッドはありませんでした。差分と weko_accounts/utils.py・rest.py・ext.py の関連箇所を読み、認可の後退、破壊的操作、入力検証の不足、呼び出し側への影響のどれも見つかりませんでした。 モデル sonnet ／ 2 回実行して和集合 ／ コスト $0.2102。同じ入力でも結果が揺れるため複数回まわし、一部のパスでしか挙がらなかったものには回数を添えています 他レビューを踏まえた自動レビューです。誤りが含まれることがあります。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. This changes the authentication boundary and the login rate-limit threshold: if the limit is ineffective or too permissive, attackers can make unrestricted password-guessing attempts, and any accounts compromised before a revert remain compromised. Reverting stops the new behavior but cannot undo successful unauthorized logins.
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 0)サマリ
変化はありません。 台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
ソース由来の経路検知ソース由来の経路検知
判定: ✅ 全検知が台帳に対応 (0件)
参考: 静的検知と結びつかなかった台帳行
|
2 participants
概要 (Summary)
/apiアプリにだけ初期化し、ログイン API にだけ付ける(制限値はWEKO_API_LIMIT_RATE_DEFAULT)。UI と共有のlimiterは従来どおり/apiアプリには初期化しないdecorators属性で付ける(メソッドに付けた従来の指定は名前が合わず効いていなかった)/api配下の他の経路は従来どおりWEKO_ACCOUNTS_REAL_IPに従う。既定(None)ではクライアントが送るX-Real-IPを優先するため、nginx のreal_ipで送信元を求めている環境ではWEKO_ACCOUNTS_REAL_IP = 'remote_addr'を推奨関連Issue / チケット (Related Issues)
変更タイプ (Type of Change)
🤖 0. CI 自動チェック (API Inventory Drift)
PR ごとに WEKO3 コンテナを起動し、
url_mapのダンプ・台帳との突き合わせ・変更行の到達可否測定を自動実行する。結果は PR コメントと Actions の artifact
(
api-inventory-summary) に出る。このリポジトリは public のため、台帳もベースラインも同梱していない。
実データはプライベートリポジトリ
RCOSDP/weko-secretにあり、CI は Secret 経由で取得する。以降この文書では、そこを単にプライベートリポジトリと呼ぶ。
台帳はブランチごとに内容が違うため、CI は weko 側と同名のブランチを
プライベートリポジトリから探して使う（head → base → 既定ブランチ の順）。
採用されたブランチ名は PR コメントの冒頭に出るので、件数を読む前にそこを見ること。
対応ブランチが無い場合は既定ブランチと比較され、コメント冒頭に警告が出る。
その件数は当てにならないので、PASS でも「確認済み」と読まないこと。
詳細:
tools/api-inventory/ci/README.md§3aSecret (
API_INVENTORY_REPO/API_INVENTORY_SSH_KEY) が未設定のリポジトリ、および fork からの PR では、このジョブは何もせずスキップされる。
API を追加・変更した場合（必須）
この PR が
fix/issue62569→develop_v2.0.4なら、プライベート側もfix/issue62569→develop_v2.0.4。同名にしておけば台帳 PR が未マージでもCI がそれを見るので、2つの PR のマージ順を気にしなくてよい。
api_snapshot.jsonを更新し、対応する PR を出したweko3_api_list_full.tsvに行を追加・更新し、build_checklist.pyで 24 列版を再生成した（未収載だと reconcile が FAIL する）（
git statusに*.tsv/api_snapshot.jsonが出ていないこと）FAIL したときの対処（要約）
まず PR コメント冒頭の台帳ブランチを見る。警告が出ていれば、件数を追う前に
プライベートリポジトリ側の対応ブランチを用意すること（比較相手が違うので件数に意味がない）。
ジョブが落ちる条件は 3 つある。PR コメントのどのセクションに件数が出ているかで切り分ける。
drift.md)reconcile.md)*_PERMISSION_FACTORY/ CSRF 保護 等が危険側の値に変わったcan_delete/can_exportがFalse→Truedata_opを更新data_opが作成/更新/削除の経路に、未認証で到達したdata_opの記載誤りなら台帳を直すurl_mapに無いreconcile B のうち、実機に存在しないことが正当な行（プラグイン未登録・config で無効等）は
プライベートリポジトリの
reconcile_allow.jsonに理由付きで登録する。理由なしの登録は不可。登録済みの行は B'（既知・許容）として集計され、E'（endpoint が実機に無い）と併せてゲート対象外になる。
W1〜W6 は WARN でゲートは通るが、レビューでは見ること
（ModelView の追加 / 実装本体の変化 / HTTP メソッド・URL の変化 / 監視対象 config の変化 /
依存パッケージの版の変化）。特に W6（依存の版）は、ベースラインを CI と異なる環境で作ると
毎回出続けて形骸化するため、ベースラインは
install.shで作った環境から生成する。🔒 1. セキュリティ & API アクセス制御チェック (必須)
認証・認可 (Authentication & Authorization)
@login_required,@pass_record,need(...), Invenio Access Action/api/*ではPermission.require(http_exception=403)を使うこと。@login_requiredは API アプリにsecurity.loginが無いため 401 ではなく 500 になる--allow-writes付きでGET / HEAD 以外も叩く）
起動した経路のみ。ワークフロー系など未解決プレースホルダの行は skip される
Noneで無効化していない*_PERMISSION_FACTORY等を監視機能クローズ・非公開化の場合 (Feature Disable)
404 Not Foundまたは403 Forbiddenが返ることを確認した🧪 2. テストコード観点チェック (pytest / Invenio Test Suite)
権限・異常系テスト (Negative & Authorization Tests)
401 Unauthorizedまたは403 Forbidden/404 Not Foundが返ることを検証するテストがある403になるテストがある404/403を返すテストがある境界値・入力バリデーションテスト (Boundary & Validation)
400 Bad Request/ バリデーションエラーが返るテストがあるデータ整合性・トランザクションテスト (Integrity & Rollback)
🛡️ 3. データ保護 & 破壊的変更防止チェック (Data Safety)
⚙️ 4. マイグレーション & システム影響チェック (Invenio / WEKO3 Stack)
データベース (DB / Alembic)
invenio alembic upgrade（適用）およびdowngrade（ロールバック）スクリプトを作成・検証した検索インデックス (Elasticsearch / OpenSearch)
設定 & 非同期処理 (Config / Celery / Cache)
invenio.cfg/ 環境変数のデフォルト値を設定した📚 5. ドキュメント・仕様書更新チェック (weko-document)
tools/api-inventory/、台帳・調査記録はプライベートリポジトリ（public リポジトリには置かない）。
§0 のチェック項目で対応済みなら、ここは確認のみ。
weko3_api_auth_findings.md）もプライベートリポジトリに置く。台帳は二重管理しない📋 6. 動作検証エビデンス (Verification Evidence)
テスト実行結果
CI の成果物 (artifact:
api-inventory-summary)drift.mdreconcile.md明細（該当した経路名・実測結果）は公開できないため artifact に含めていない。
プライベートリポジトリ側で同じコマンドを
--summary-onlyなしで実行して確認する。手動で確認したこと
/api全体にかけると、検索・統計・IIIF などゲストの通常の閲覧でも制限に達しうることを確認し、ログイン API だけに絞った🤖 Generated with Claude Code