fix: 管理系 API の担当範囲を確認し、SWORD の登録可能ロールの設定を効かせる - #1927
Conversation
フォームで指定されたリポジトリが操作者の担当範囲に含まれることを デコレータで確認する。System/Repository Administrator は従来どおり。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
検索設定画面と同じく System/Repository Administrator のみ利用できるよう login_required と roles_required を付ける。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WEKO_SWORDSERVER_DEPOSIT_ROLE_ENABLE をモジュール定数として import せず、 リクエストごとに current_app.config から読むデコレータ check_deposit_role を 追加し、POST /sword/service-document・PUT/DELETE /sword/deposit に適用する。 ロールの判定自体は従来どおり weko_accounts の roles_required に委ねる。 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 Guide管理系 API の認可漏れを補い、サイトライセンスメール送信のリポジトリ担当範囲と検索設定 API の管理者ロールを検証するよう変更しています。加えて、SWORD の登録可能ロール判定をモジュール定数からリクエスト時の current_app.config 読み込みへ移し、設定上書きが POST/PUT/DELETE に反映されるようにしています。 Sequence diagram for scoped site license mail authorizationsequenceDiagram
participant Admin as AdminUser
participant API as SiteLicenseMailAPI
participant Auth as RoleAndScopeCheck
participant Community as Community
participant Mail as SiteLicenseMailer
Admin->>API: POST manual_send_site_license_mail
API->>Auth: roles_required
Auth->>Auth: _form_repository_scope_required
Auth->>Community: get_repositories_by_user(current_user)
alt Repository in scope or super role
Auth-->>API: Allow
API->>Mail: manual_send_site_license_mail
Mail-->>Admin: Success
else Repository outside scope
Auth-->>Admin: 403 Forbidden
end
Sequence diagram for runtime SWORD deposit role configurationsequenceDiagram
participant Client as SWORDClient
participant API as SWORDAPI
participant Decorator as check_deposit_role
participant Config as current_app.config
participant Roles as roles_required
Client->>API: POST, PUT, or DELETE deposit request
API->>Decorator: check_deposit_role
Decorator->>Config: Read WEKO_SWORDSERVER_DEPOSIT_ROLE_ENABLE
Config-->>Decorator: Allowed roles
Decorator->>Roles: roles_required(allowed roles)
alt Role authorized
Roles-->>API: Allow request
API-->>Client: Process deposit operation
else Role unauthorized
Roles-->>Client: 403 Forbidden
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 |
API インベントリ差分(件数のみ)
ベースラインとの差分(生成されませんでした) 台帳との突き合わせ(生成されませんでした) ソース由来の経路検知(生成されませんでした) |
🔍 Claude レビュー統合Claude の追加指摘 2 件 — 🔴 高 0 / 🟠 中 0 / 🟡 低 2
1. 🟡 [低] 担当リポジトリの判定に削除済みコミュニティが含まれる(Claude の追加指摘)
`Community.get_repositories_by_user 修正案 削除済みを除外したいなら、 根拠確認: modules/invenio-communities/invenio_communities/models.py:396-405 を確認。weko_admin/views.py:587-629 の view 本体と tasks.py:224 の内部呼び出し(repo_id をキーワード指定するため対象外)も確認した。 2. 🟡 [低] 担当リポジトリ判定に削除済みコミュニティが含まれる(Claude の追加指摘)
`Community.get_repositories_by_user 修正案 削除済みを除外する必要があるなら、 根拠確認: modules/invenio-communities/invenio_communities/models.py:396-405 と weko_admin/views.py:546-567 を確認。他の管理画面のリポジトリ選択も同じ関数を使っているため、影響は小さい。 次にすること: 外部レビューには裁定すべき実質的な指摘がなく、bot の自動コメントだけだった。差分は、サイトライセンスメール送信のスコープ制限、 モデル sonnet / 2 回実行して和集合 / コスト $0.2776。同じ入力でも結果が揺れるため複数回まわし、一部のパスでしか挙がらなかったものには回数を添えています 他レビューを踏まえた自動レビューです。誤りが含まれることがあります。 |
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 authorization for SWORD deposits, updates, and object deletion, as well as administrative license-mail actions. If the configured roles are wrong or the checks are bypassed, unauthorized users could persist or delete records and send messages; reverting prevents future requests but cannot undo those effects.
|
|
||
| @blueprint_api.route('/search/init_display_index/<string:selected_index>', | ||
| methods=['GET']) | ||
| @login_required |
| return jsonify(result) | ||
|
|
||
|
|
||
| def _is_repository_in_user_scope(repo_id): |
| @roles_required([WEKO_ADMIN_PERMISSION_ROLE_SYSTEM, | ||
| WEKO_ADMIN_PERMISSION_ROLE_REPO, | ||
| WEKO_ADMIN_PERMISSION_ROLE_COMMUNITY]) | ||
| @_form_repository_scope_required |
| return decorated | ||
| return wrapper | ||
|
|
||
| def check_deposit_role(): |
| @require_oauth_scopes(write_scope.id, actions_scope.id) | ||
| @require_oauth_scopes(item_create_scope.id) | ||
| @roles_required(WEKO_SWORDSERVER_DEPOSIT_ROLE_ENABLE) | ||
| @check_deposit_role() |
| @require_oauth_scopes(write_scope.id, actions_scope.id) | ||
| @require_oauth_scopes(item_update_scope.id) | ||
| @roles_required(WEKO_SWORDSERVER_DEPOSIT_ROLE_ENABLE) | ||
| @check_deposit_role() |
| @require_oauth_scopes(write_scope.id, actions_scope.id) | ||
| @require_oauth_scopes(item_delete_scope.id) | ||
| @roles_required(WEKO_SWORDSERVER_DEPOSIT_ROLE_ENABLE) | ||
| @check_deposit_role() |
概要 (Summary)
login_requiredとroles_required(System / Repository Administrator)を付ける(検索設定の管理画面に入れるロールと同じ)WEKO_SWORDSERVER_DEPOSIT_ROLE_ENABLEを、リクエストごとにcurrent_app.configから読むデコレータcheck_deposit_roleに置き換える(POST service-document / PUT / DELETE)関連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なしで実行して確認する。手動で確認したこと
search_management.js)だけであることを確認した🤖 Generated with Claude Code