fix: ファイルの取得・プレビュー・IIIF でファイル単位の権限判定を行う - #1925
Conversation
ファイルのダウンロード可否と所有者・管理者判定で、Community Administrator を check_created_id と同じく has_comadmin_permission により自コミュニティ 配下のアイテムに限定する。System / Repository Administrator は従来どおり。 判定を is_superuser_or_record_comadmin にまとめ、 check_file_download_permission と is_owners_or_superusers から使う。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ファイル単位の権限を要求するデコレータ file_permission_required を追加し、 preview に付ける。判定は file_ui と同じく file_permission_factory を使う。 権限がない場合、未ログインならログイン画面へ誘導し、ログイン済みなら 403 を返す。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ゲストトークンを持つセッションに許可するファイル操作を、トークンに対応する ゲストアクティビティのアイテム（およびそのルートバージョン）に紐づくバケットに 限定する。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IIIF の画像・画像情報の取得時に、対象ファイルが属するレコードの閲覧権限と ファイルのダウンロード権限を weko-records-ui の既存判定で確認する。 レコードのファイルでないオブジェクトは Invenio-Files-REST の権限で判定する。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
マニフェストのエンドポイントに weko-records-ui のレコード閲覧権限を permission factory として設定し、マニフェストに含める画像も ファイルの権限があるものに限定する。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
権限判定はファイル実体の JSON にある accessrole を見る。アイテム登録時は メタデータがそこへ書き込まれるが、テストのフィクスチャは書き込んでいない ため、判定が常に許可になりテストが成立していなかった。登録時と同じく メタデータを書き込んでから判定させる。 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
画像を開く処理に権限判定を入れたため、リクエストの外で動くサムネイル作成 タスクが、利用者の情報が無く失敗するようになっていた。内部処理なので対象の オブジェクトを直接解決し、画像を開く処理にはそれを使わせる。利用者の リクエストで通る経路の判定は変えない。 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 |
|
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 |
Reviewer's GuideThe PR closes file-content authorization gaps by adding file-specific checks to preview and IIIF delivery, scoping Community Administrator privileges to their communities, filtering unauthorized IIIF manifest images, and binding guest-token operations to activity-owned buckets; accompanying tests cover the new permission branches and HTTP behavior. Sequence diagram for file-authorized IIIF deliverysequenceDiagram
participant Client
participant IIIF as IIIF API
participant RecordPermission as page_permission_factory
participant FilePermission as check_file_download_permission
participant ObjectStore as ObjectVersion
Client->>IIIF: Request manifest
IIIF->>RecordPermission: permission_factory(record).can()
alt Record view denied
IIIF-->>Client: 401 or 403
else Record view allowed
IIIF->>FilePermission: iiif_object_permission_factory(obj, record).can()
FilePermission->>ObjectStore: ObjectVersion.get(bucket, key, version_id)
alt File permission denied
IIIF-->>Client: Manifest excludes image
else File permission allowed
IIIF-->>Client: Manifest with authorized images
end
end
Client->>IIIF: Request image or info.json
IIIF->>ObjectStore: ObjectVersion.get(bucket, key, version_id)
IIIF->>FilePermission: iiif_object_permission_factory(obj).can()
alt Object missing or permission denied
IIIF-->>Client: 404
else Permission allowed
IIIF-->>Client: Image or info.json
end
Flow diagram for guest-token bucket authorizationflowchart TD
A[Guest file operation] --> B[get_guest_activity_bucket_ids]
B --> C[Resolve GuestActivity from guest_token]
C --> D[Resolve WorkActivity item]
D --> E[Find item and root-version buckets]
E --> F{Requested bucket in activity buckets?}
F -->|Yes| G[Allow file operation]
F -->|No| H[Deny file operation]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🔍 Claude レビュー統合Claude の追加指摘 3 件 — 🔴 高 0 ／ 🟠 中 1 ／ 🟡 低 2
1. 🟠 [中] item_id が未設定のゲストアクティビティでは、ゲストのファイル操作が全て拒否される可能性がある（Claude の追加指摘）
`get_guest_activity_bucket_ids 修正案 ゲストのアップロードは 根拠確認: permissions.py:160-198、views.py:376-400 を確認した。weko_workflow/views.py:786 と 1016 は、 2. 🟡 [低] create_thumbnail で対象が見つからないと不明瞭な AttributeError になる（Claude の追加指摘）
ObjectVersion.get が None を返すと g.obj = None になります。image_opener は hasattr(g, 'obj') だけを見て obj.file を呼ぶため、AttributeError で落ちます。また g が前のタスクの obj を残したままだと、別の画像を誤って処理するおそれがあります。 修正案 obj が None ならタスクを早期 return し、g.obj は使用後に削除する(または image_opener に obj を明示的に渡す)。 根拠確認: tasks.py の差分と handlers.py:45-56 の image_opener を確認しました。 3. 🟡 [低] サムネイル作成タスクで対象が見つからないと g.obj が None になり、後段で AttributeError になる（Claude の追加指摘）
`ObjectVersion.get 修正案 obj が None のときは g.obj を設定せずに return するか、明示的にログを出して終了する。 根拠確認: tasks.py の差分と handlers.py:45-56 を確認した。 次にすること: 既存レビューは自動化ボットの状態通知だけで、裁定すべき指摘はありませんでした。認可まわりの変更(ゲストトークンのバケット限定、IIIF の権限判定、comadmin のスコープ限定)は、実コードで矛盾を確認できませんでした。create_thumbnail で対象が None の場合の扱いだけ補ってください。 モデル sonnet ／ 2 回実行して和集合 ／ コスト $0.4212。同じ入力でも結果が揺れるため複数回まわし、一部のパスでしか挙がらなかったものには回数を添えています 他レビューを踏まえた自動レビューです。誤りが含まれることがあります。 |
![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 authorization decisions for guest file access, previews, and IIIF retrieval, so a flawed bucket or record/file permission check could expose protected files broadly from the moment it ships. Reverting would stop future access but would not undo files or images already disclosed.
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 1)サマリ
[WARN] W2 実装本体が変化(data_op / 情報露出を再確認) — 1件
台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
ソース由来の経路検知ソース由来の経路検知
判定: ✅ 全検知が台帳に対応 (0件)
参考: 静的検知と結びつかなかった台帳行
|
| return False | ||
|
|
||
|
|
||
| def get_guest_activity_bucket_ids(token): |
There was a problem hiding this comment.
fix no.460, 25, 474, 475, 476
| if need.method == 'action' and \ | ||
| need.value in guest_access_file_actions: | ||
| return True | ||
| if bucket_ids is None: |
There was a problem hiding this comment.
fix no.460, 25, 474, 475, 476
| "recid": { | ||
| "pid_type": "recid", | ||
| "route": "/records/<pid_value>", | ||
| "permission_factory_imp": |
| # skip Invenio-Files-REST permission factory | ||
| g.obj = ObjectVersion.get(bucket, key, version_id=version_id) | ||
| #g.obj = ObjectResource.get_object(bucket, key, version_id) | ||
| obj = ObjectVersion.get(bucket, key, version_id=version_id) |
| obj | ||
| for obj in ObjectVersion.get_by_bucket(bucket).all() | ||
| if can_preview(PreviewFile(None, None, obj)) | ||
| and iiif_object_permission_factory(obj, record=self.record).can() |
| @@ -0,0 +1,65 @@ | |||
| # -*- coding: utf-8 -*- | |||
| @shared_task(ignore_result=True) | ||
| def create_thumbnail(uuid, thumbnail_width): | ||
| """Create the thumbnail for an image.""" | ||
| # 利用者のリクエストではない内部処理なので、利用者の権限は確かめずに |
| abort(500) | ||
|
|
||
| # TODO Check permissions | ||
| if permission_factory and not permission_factory(record).can(): |
| return type('FileDownLoadPermissionChecker', (), {'can': can})() | ||
|
|
||
|
|
||
| def file_permission_required(f): |
| from .permissions import file_permission_required | ||
|
|
||
|
|
||
| @file_permission_required |
| if role.name in super_users: | ||
| is_ok = True | ||
| break | ||
| is_ok = is_superuser_or_record_comadmin(record) |
| for role in list(current_user.roles or []): | ||
| if role.name in supers: | ||
| return is_can | ||
| if is_superuser_or_record_comadmin(record): |
| for role in list(current_user.roles or []): | ||
| if role.name in supers: | ||
| return True | ||
| return is_superuser_or_record_comadmin(record) |
| return is_superuser_or_record_comadmin(record) | ||
|
|
||
|
|
||
| def is_superuser_or_record_comadmin(record) -> bool: |
2 participants
概要 (Summary)
check_file_download_permission/is_owners_or_superusers)で、Community Administrator を自コミュニティ配下のアイテムに限る(has_comadmin_permission)。System / Repository Administrator は従来どおりpreview)に、ファイル単位の権限を要求するデコレータfile_permission_requiredを付けるprotect_api)で、対象ファイルが属するレコードの閲覧権限とファイルの権限を判定する。マニフェストにpage_permission_factoryを適用し、列挙する画像も権限のあるものに絞る。サムネイル作成タスク(内部処理)は利用者の権限を確かめずに対象を解決する関連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なしで実行して確認する。手動で確認したこと
has_comadmin_permissionはインデックスの無いレコードに False を返す)🤖 Generated with Claude Code