feat(agent-commerce): エージェントコマース用 OAuth2(client_credentials / scope / AccessTokenHandler / クライアント登録導線)を追加 (#188) - #191
Conversation
…nHandler を追加 (EC-CUBE#188) エージェントコマース (ACP/UCP) の machine-to-machine インバウンド認証を有効化する。 - client_credentials grant を有効化 (services.yaml・ClientType の grants 選択肢) - scope レジストリに acp:/ucp: の 6 scope を登録 (<protocol>:<capability> 規約) - AgentCommerceAccessTokenHandler: Symfony 標準 AccessTokenHandlerInterface 実装。 league ResourceServer で Bearer JWT を検証 (公開鍵署名/期限/失効) し、付与 scope を UserBadge attributes へ載せる。本体 AgentCommerceOAuth2Authenticator が依存する口を提供。 - ハンドラのユニットテスト (scope 付与 / 不正トークンの BadCredentials 正規化) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
…erce # Conflicts: # Resource/config/services.yaml
a6c3042 to
f1a5322
Compare
client_credentials と acp:/ucp: scope を汎用の OAuth クライアント登録フォームに 足しただけでは、 成立しない組み合わせ (例: acp:checkout × authorization_code や、 会員同意が前提の ucp:identity × client_credentials) を作れてしまう。 また 1 つの クライアントに ACP と UCP を混在させると、 受注に記録される agent_id (= クライアント識別子) から事業者を特定できず、 失効・監査を事業者単位で行えない。 そこで注意文言ではなく登録導線自体を protocol ごとに分ける。 - 「ACP 新規追加」「UCP 新規追加」ボタンと専用画面を追加。 grant は client_credentials 固定、 scope は当該 protocol のものだけを提示し、 リダイレクト URI は入力させない - ucp:identity は Customer subject の authorization_code が前提のため この導線から除外 (eccube-api4#189 landing 後に会員同意を伴う別導線で追加) - 汎用フォーム (ClientType) は GraphQL 用に戻し、 acp:/ucp: と client_credentials の選択肢を削除 - クライアントに名称を持たせ (Client::name)、 一覧へ名称列を追加。 どの事業者向けのクライアントかを一覧で追える - 契約テスト: grant 固定 / protocol 跨ぎの scope 拒否 / ucp:identity 不可 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
f1a5322 to
a2c388d
Compare
ttokoro20240902
left a comment
There was a problem hiding this comment.
コードベースと静的確認(league/oauth2-server-bundle 1.x・symfony/form・EC-CUBE 本体 4.4 の実装との照合)によるレビューです。動作確認は行っていません。CI は全ジョブ green を確認済みです。
疎結合設計(本体は Symfony 標準 AccessTokenHandlerInterface にのみ依存)と、protocol ごとに登録導線を分けて成立しない grant×scope の組み合わせを作れなくする方針は妥当だと思います。
指摘は 13 件、各行のインラインコメントに置きました。内訳は 本PRでの修正をお願いしたいもの 4 件 / 別Issue・別PRが妥当と考えるもの 4 件 / コメント・注意書きの追記で足りるもの 5 件 です。リリースブロッカーに相当するものはありません。
最優先は AgentCommerceClientType の identifier に予約識別子 mcp_pat を弾く制約がない件で、登録されると管理画面の一覧から消えて削除不能になり、かつ MCP PAT が ACP クライアント配下で発行されます。
PR 説明について 1 点
「公開 DCR は grant をハードクランプするため、client_credentials を全体で有効化しても匿名登録クライアントが machine トークンを取得することはない」の部分ですが、根拠としては不正確です。実際に効いている主たる防御は league 側にあり、DCR のクランプはそれとは独立した二重防御です。またこの記述だと「既存レコードは安全」と読めてしまいますが、そうとは限りません(services.yaml のインラインコメント参照)。実装の妥当性は変わらないので、説明の書き換えだけお願いできればと思います。
| # Whether to enable the client credentials grant | ||
| enable_client_credentials_grant: false | ||
| # エージェントコマース (ACP/UCP) の machine-to-machine インバウンド認証で使用する (#188)。 | ||
| enable_client_credentials_grant: true |
There was a problem hiding this comment.
【中】client_credentials の全体有効化が grants 空の既存クライアントに波及する(別Issue提案)
ClientRepository::isGrantSupported() は $client->getGrants() が空なら全 grant を許可します。そのため oauth2_client.grants が空のレコードは、これまで authorization_code(=同意経由)でしか使えなかったのに、同意なしで client_credentials トークンを取得できるようになります。scope は read / write のまま通ります。
ただし実際の攻撃面は限定的で、リリースブロッカーではないと判断しています。
- public client は対象外:
ClientCredentialsGrantが!isConfidential()をinvalid_clientで弾きます。DCR 登録クライアントと PAT クライアントはいずれもsecret = nullなので該当しません。 - 本プラグインが作ったクライアントも対象外:
ClientTypeのgrantsは初版からNotBlank必須で、api プラグイン経由では grants 空のレコードは生成されません。
残るのは手動投入 / 他プラグイン由来 / データ移行由来の、シークレットを持つ grants 空クライアントのみです。一律 backfill は破壊的なので避け、grants 空の confidential クライアントを検出するコマンド(または管理画面での警告表示)を別Issueに切り出すのが良さそうです。
There was a problem hiding this comment.
ClientRepository::isGrantSupported() が grants 空で全 grant を許可すること(vendor/league/oauth2-server-bundle/src/Repository/ClientRepository.php:117-119)、および攻撃面が限定的である根拠(public client は ClientCredentialsGrant.php:39-43 で invalid_client、ClientType の grants は初版 936a67f から NotBlank 必須)は、いずれもこちらでも確認しました。ご分析のとおりです。
影響も限定的なので本 PR のスコープ外とします
There was a problem hiding this comment.
本 PR のスコープ外というご判断に同意します。
その上で、grants 空の confidential クライアントを検出する手段(コマンドまたは管理画面での警告)を別 Issue に切り出すかどうかだけ、意思決定をお願いできますか。不要という判断であればこのまま resolve します。
| # attributes['scopes'] に載せて返す。本体は handler が無ければ 503 を返す疎結合設計。 | ||
| Plugin\Api44\Security\AgentCommerceAccessTokenHandler: | ||
| autowire: true | ||
| Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterface: '@Plugin\Api44\Security\AgentCommerceAccessTokenHandler' |
There was a problem hiding this comment.
【低】グローバル alias である旨の注意書きが欲しい
本体が @?Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterface を参照する契約なので、この ID で alias を張ること自体は妥当です。
ただしこの alias はアプリ全体に効くため、将来 access_token firewall や他プラグインが同インターフェースを autowire / alias すると、無言で ACP ハンドラに解決される(あるいは alias が上書きされて ACP 側が壊れる)可能性があります。上のコメントブロックに一言残しておくと事故防止になります。
There was a problem hiding this comment.
ご指摘のとおりです。efc050d で注意書きを追加しました。
補足すると、この衝突はすでに現実に起きています。EC-CUBE 本体の app/config/eccube/services_test.yaml が同じ ID(Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterface)にテスト用スタブを登録しているため、本プラグインを導入した環境では本体側テストの解決先が入れ替わります。この実例もコメントに含めました。
There was a problem hiding this comment.
efc050d の注意書きを確認しました。app/config/eccube/services_test.yaml:147-152 が同じ ID に InMemoryAccessTokenHandler を alias している点も確認しています。
この衝突はコメントに残す以上の意味があるかもしれません。api44 を導入した環境で本体のテストを流すと、alias の解決順によっては core の ACP テストがスタブではなく実ハンドラを掴み、本体側テストが落ちる可能性があります。本体テストは通常プラグイン無しで走るため実害は限定的ですが、プラグイン同梱構成の CI では踏むはずです。本体側に確認用の Issue を立てておくのが良いと思いますが、いかがでしょうか。
PR EC-CUBE#191 のレビュー指摘のうち、実害が限定的で対応コストの低い 5 件に対応する。 - クライアントシークレットに最小長 (32 文字) を追加。client_credentials では シークレットが唯一の認証情報のため。既定値は sha512 hex (128 文字) なので、 管理者が手で書き換えた場合にだけ効く。汎用の ClientType も token エンドポイントの 認証材料はシークレットのみなので同条件に揃える。 - scopes の Assert\All + Assert\Choice のコメントを実機構に合わせて訂正。 choices 外の値を実際に弾いているのは ChoiceType 自身 (PRE_SUBMIT で submitted data から除去し POST_SUBMIT で FormError を積む) で、本制約は多層防御として残す。 - 発行完了画面のコピーボタンに document.execCommand フォールバックを追加。 navigator.clipboard は secure context 以外では undefined になり TypeError で 無言のコピー失敗になる。一覧画面 (index.twig) の既存実装に揃える。 - AgentCommerceAccessTokenHandler の docblock に、role_prefix による scope → role 変換を経由しない旨を明記。access_control や is_granted() で ROLE_OAUTH2_<SCOPE> を 期待した人が確実に踏むため。 - AccessTokenHandlerInterface の alias がアプリ全体に効く旨を services.yaml に注記。 実際に EC-CUBE 本体の services_test.yaml が同じ ID にスタブを登録しており、 本プラグイン導入下では解決先が入れ替わる。 検証: PHPUnit 128 tests / 343 assertions / 0 failures、PHPStan level 6 No errors、 php-cs-fixer 0 件。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
概要
AI エージェント (ChatGPT / Gemini 等) → EC-CUBE のインバウンド machine-to-machine 認証を成立させるため、OAuth2 の client_credentials グラントを有効化し、エージェントコマース (ACP/UCP) 用の scope を登録します。あわせて、EC-CUBE 本体の
AgentCommerceOAuth2Authenticatorが依存する Symfony 標準AccessTokenHandlerInterfaceの具象と、ACP/UCP 用クライアントの登録導線を提供します。Closes #188
変更内容
認証基盤
Resource/config/services.yaml)。<protocol>:<capability>規約で 6 scope をscopes.availableに追加 (acp:checkout/acp:catalog/ucp:checkout/ucp:cart/ucp:catalog/ucp:identity)。defaultはreadのまま (明示要求時のみ付与)。Plugin\Api44\Security\AgentCommerceAccessTokenHandler(新規): league のResourceServerで Bearer トークン (JWT) を検証 (公開鍵署名・有効期限・失効) し、付与 scope を Symfony のUserBadgeattributes (scopes) に載せて返す。Symfony\Component\Security\Http\AccessToken\AccessTokenHandlerInterfaceを alias で束ね、本体のAgentCommerceOAuth2Authenticatorが@?optional 依存で解決する (api4 未導入時は本体が 503 を返す疎結合)。管理画面: ACP/UCP クライアントの登録導線を分離
汎用の OAuth クライアント登録フォームに scope と grant を並べるだけでは、成立しない組み合わせを作れてしまいます (例:
acp:checkout× authorization_code、会員同意が前提のucp:identity× client_credentials)。また 1 クライアントに ACP と UCP を混在させると、受注に記録されるOrder.agent_id(= クライアント識別子) から事業者を特定できず、失効・監査を事業者単位で行えません。そこで登録導線自体を protocol ごとに分離しました。AgentCommerceClientController/AgentCommerceClientType)。grant は client_credentials 固定、scope は当該 protocol のものだけを提示、リダイレクト URI は入力させない。ucp:identityはこの導線から除外。Customer を subject とする authorization_code が前提で client_credentials では成立しないため (Customer(会員) に紐づく OAuth2 authorization_code フロー (ID 連携) のサポート #189)。Customer(会員) に紐づく OAuth2 authorization_code フロー (ID 連携) のサポート #189 に必要な追加実装をコメント済み。ClientType) は GraphQL 用に戻し、acp:/ucp:と client_credentials の選択肢を持たせない。Client::name)、一覧に名称列を追加。汎用フォームにも名称欄を追加 (従来は空文字固定)。管理画面: クライアントシークレットの扱い
league は保存時にハッシュ化せず、初回のトークン取得成功時に bcrypt へ日和見アップグレードします (
ClientRepository::validateClient)。そのため一覧に表示される値は、一度使われた後は事業者へ渡せないハッシュになります。Cache-Control: no-store, private)。登録画面にもその旨を明記。-表示 (理由をツールチップで提示)。public client (DCR 登録等) も同様に-表示に統一。<colgroup>で明示 (見出しの折り返し解消)。テスト
AgentCommerceAccessTokenHandlerTest: scope 付与 / 不正トークンのBadCredentialsException正規化。AgentCommerceClientControllerTest: protocol 別の scope 提示 / grant が client_credentials に固定される / protocol を跨いだ scope はフォームバイパスでも拒否 /ucp:identityは付与不可 / 完了画面で平文を 1 度だけ提示しno-storeが付く / 一覧にシークレットが出ない。OAuthControllerTest: 名称の永続化を追加。設計メモ
AccessTokenHandlerInterfaceのみに依存する疎結合設計。本 PR がその口を提供する。scopes.availableは MCP read 4 件と agentic 6 件が共存する。MCP discovery のscopes_supportedはMcpTokenService::AVAILABLE_SCOPESを唯一のソースにしているため、agentic scope の追加による汚染はない。POST /register) は grant をauthorization_code+refresh_token、scope を MCP read にハードクランプするため、client_credentials を全体で有効化しても匿名登録クライアントが machine トークンを取得することはない。ucp:identity) は Customer(会員) に紐づく OAuth2 authorization_code フロー (ID 連携) のサポート #189 の範囲。検証
ローカルで EC-CUBE 4.4 + 本 PR の api44 + sample-payment-plugin を共存させ、HTTPS (symfony CLI) 上で確認しました。
POST /token(client_credentials) → 実 JWT を取得 →e2e/agent/acp-checkout.phpPASS (31 assertions) /e2e/agent/ucp-checkout.phpPASS (23 assertions)。3DS 中断・再開、決済拒否を含む complete まで通過し、受注のagent_protocol/agent_idが期待どおり記録されることを確認。ucp:checkoutを要求するとinvalid_scope。tools/list(11 ツール) /tools/callまで到達。refresh_token のローテーションと旧トークン失効も確認。エージェントコマース側の E2E もこの状態で PASS。No errors/ PHPUnit 127 tests, 342 assertions, 0 failures (deprecation 3 件はいずれも既存コード由来)。🤖 Generated with Claude Code