Conversation
EcAuth Identity Providerと連携し、EC-CUBE 4.3系管理画面に パスキー（WebAuthn/FIDO2）認証を追加するプラグインの全機能を実装。 - Docker開発環境（EC-CUBE 4.3 + PostgreSQL 15） - プラグイン設定画面（EcAuth接続情報管理） - EcAuth API連携サービス（7エンドポイント対応） - 管理画面ログインUI拡張（パスキーログインボタン） - OAuth2コールバック処理（state検証・トークン交換・セッション確立） - パスキー管理画面（一覧・登録・削除・パスワード本人確認） - WebAuthn JavaScript API ヘルパー - 静的解析設定（PHPStan Level 6, Rector, PHP CS Fixer） - GitHub Actions CI/CD（静的解析・E2Eテスト・リリースパッケージ） - Playwright E2Eテストスペック Closes #1 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughEC-CUBE 4.3用EcAuthLogin43プラグインの実装を完全に追加。WebAuthn/FIDO2ベースのB2Bパスキー認証、EcAuth APIクライアント、管理画面UI、Docker開発環境、CI/CDワークフロー、E2Eテストを統合。 Changes
Sequence DiagramsequenceDiagram
participant User as ユーザー
participant Browser as ブラウザ
participant AdminUI as 管理画面<br/>(EcAuthLogin43)
participant Server as EC-CUBE<br/>サーバー
participant EcAuthAPI as EcAuth API
rect rgba(100, 150, 200, 0.5)
Note over User,EcAuthAPI: パスキー認証フロー
User->>Browser: ログイン画面訪問
Browser->>AdminUI: /admin/login GET
AdminUI->>Browser: ログインフォーム<br/>(パスキーボタン付き)
User->>Browser: パスキーボタン クリック
Browser->>Server: POST /ecauth/passkey/authenticate/options<br/>(CSRF トークン)
Server->>EcAuthAPI: authenticateOptions(rpId)
EcAuthAPI->>Server: sessionId, challenge, ...
Server->>Browser: チャレンジ取得成功
Browser->>User: WebAuthn認証要求<br/>(navigator.credentials.get)
User->>Browser: パスキー認証完了
Browser->>Server: POST /ecauth/passkey/authenticate/verify<br/>(assertion, CSRF)
Server->>EcAuthAPI: authenticateVerify(sessionId, assertion)
EcAuthAPI->>Server: code, state, ...
Server->>Server: state 検証
Browser->>AdminUI: /ecauth/callback?code=XXX&state=YYY
AdminUI->>Server: コールバック処理
Server->>EcAuthAPI: exchangeToken(code)
EcAuthAPI->>Server: access_token, id_token
Server->>Server: ID Token から sub 抽出<br/>Member 検索<br/>セッション確立
Server->>Browser: /admin リダイレクト
Browser->>AdminUI: 管理画面ホーム
end
rect rgba(150, 200, 100, 0.5)
Note over User,EcAuthAPI: パスキー登録フロー (管理者ログイン後)
User->>Browser: パスキー管理画面訪問
Browser->>AdminUI: GET /%admin%/ecauth/passkey/
AdminUI->>Browser: パスキー一覧<br/>(「登録」ボタン付き)
User->>Browser: 「パスキーを登録」クリック
Browser->>Browser: パスワード再入力モーダル表示
User->>Browser: パスワード入力 + 送信
Browser->>Server: POST /%admin%/ecauth/passkey/verify-password<br/>(password, CSRF)
Server->>Server: パスワード検証<br/>b2b_subject 取得
Server->>Browser: b2b_subject 返却
Browser->>Server: POST /%admin%/ecauth/passkey/register/options<br/>(b2b_subject, CSRF)
Server->>EcAuthAPI: registerOptions(rpId, b2bSubject)
EcAuthAPI->>Server: sessionId, challenge, ...
Server->>Browser: オプション返却
Browser->>User: WebAuthn登録要求<br/>(navigator.credentials.create)
User->>Browser: パスキー登録完了
Browser->>Server: POST /%admin%/ecauth/passkey/register/verify<br/>(credential, CSRF)
Server->>EcAuthAPI: registerVerify(sessionId, credential)
EcAuthAPI->>Server: 登録成功
Server->>Browser: 成功メッセージ
Browser->>AdminUI: ページリロード
AdminUI->>Server: パスキー一覧再取得
Server->>Browser: 新規パスキー表示
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @nanasess, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! このプルリクエストは、EC-CUBE 4.3系の管理画面にパスキー認証機能を追加する画期的なプラグインの初期バージョンを導入します。外部のEcAuth Identity Providerと連携することで、管理者はより安全かつ手軽にログインできるようになり、パスキーの登録や管理も直感的に行えます。これにより、管理画面のセキュリティと利便性が大幅に向上します。 Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
EC-CUBE 4.3向けのB2Bパスキー認証プラグインの初期実装、拝見しました。プラグインの骨格からAPI連携、UI拡張、テストまで、非常に広範囲にわたる実装で、全体としてよく構成されていると感じます。特に、セキュリティに関する考慮（CSRF対策、state検証、レートリミットなど）が随所に見られ、堅牢な作りを目指していることが伺えます。
レビューでは、主に以下の点についてコメントさせていただきました。
- APIコントローラにおける入力値検証の強化
- エラーメッセージの一貫性向上
- 依存ライブラリ活用によるメンテナンス性の向上
- PSR-12に準拠したプロパティ命名規則の適用
これらの点を修正いただくことで、プラグインの品質がさらに向上するかと思います。素晴らしいコントリビューションをありがとうございます。
| $data = json_decode($request->getContent(), true); | ||
| $password = $data['password'] ?? ''; |
There was a problem hiding this comment.
リクエストボディが不正なJSONの場合、json_decodeはnullを返します。その場合、$data['password']でエラーが発生してしまいます。PasskeyAuthControllerで行っているように、$dataが配列であるかチェックを追加することをお勧めします。
$data = json_decode($request->getContent(), true);
if (!is_array($data)) {
return $this->json(['error' => 'Invalid request body'], 400);
}
$password = $data['password'] ?? '';| if ($result['status'] === 200 || $result['status'] === 204) { | ||
| $this->addSuccess('ecauth_login43.admin.passkey.delete.success', 'admin'); | ||
| } else { | ||
| $this->addError('ecauth_login43.admin.passkey.register.error', 'admin'); |
There was a problem hiding this comment.
削除処理で登録エラーのメッセージキー (ecauth_login43.admin.passkey.register.error) が使われています。削除失敗時のための新しいメッセージキー (例: ecauth_login43.admin.passkey.delete.error) をResource/locale/messages.ja.yamlに追加し、それを使用するのが適切です。
| private $ecauth_base_url; | ||
|
|
||
| /** | ||
| * @var string|null | ||
| * | ||
| * @ORM\Column(name="client_id", type="string", length=255, nullable=true) | ||
| */ | ||
| private $client_id; | ||
|
|
||
| /** | ||
| * @var string|null | ||
| * | ||
| * @ORM\Column(name="client_secret", type="string", length=255, nullable=true) | ||
| */ | ||
| private $client_secret; | ||
|
|
||
| /** | ||
| * @var string|null | ||
| * | ||
| * @ORM\Column(name="rp_id", type="string", length=255, nullable=true) | ||
| */ | ||
| private $rp_id; |
There was a problem hiding this comment.
PHPのプロパティ名はPSR-12標準に従い、camelCaseで記述することが推奨されます。現在のsnake_case ($ecauth_base_urlなど) からcamelCase ($ecauthBaseUrlなど) に変更し、@ORM\Columnアノテーションのname属性でデータベースのカラム名を明示的に指定することをお勧めします。
例:
/**
* @var string|null
*
* @ORM\Column(name="ecauth_base_url", type="string", length=1024, nullable=true)
*/
private $ecauthBaseUrl;この変更は、このエンティティ内のすべてのプロパティと、関連するセッターメソッドの引数名にも適用すると、コードの一貫性が保たれます。また、Form/Type/Admin/ConfigType.php の add() メソッドの第一引数も camelCase に合わせる必要があります。
| * | ||
| * @ORM\Column(name="ecauth_subject", type="string", length=255, nullable=true) | ||
| */ | ||
| private $ecauth_subject; |
There was a problem hiding this comment.
PHPのプロパティ名はPSR-12標準に従い、camelCase ($ecauthSubject) で記述することが推奨されます。@ORM\Columnアノテーションでnameが指定されているので、プロパティ名を変更してもデータベーススキーマには影響しません。ゲッター・セッター内の参照も合わせて修正してください。
| private function generateUuidV4(): string | ||
| { | ||
| $data = random_bytes(16); | ||
| $data[6] = chr(ord($data[6]) & 0x0f | 0x40); | ||
| $data[8] = chr(ord($data[8]) & 0x3f | 0x80); | ||
|
|
||
| return vsprintf('%s%s-%s-%s-%s-%s%s%s', str_split(bin2hex($data), 4)); | ||
| } |
There was a problem hiding this comment.
UUIDv4の生成を手動で実装していますが、ramsey/uuidのような広く利用され、テストされているライブラリを使用することを強くお勧めします。これにより、コードの信頼性とメンテナンス性が向上します。
composer require ramsey/uuid を実行した後、このメソッドを以下のように簡潔に書き換えることができます。
private function generateUuidV4(): string
{
return \Ramsey\Uuid\Uuid::uuid4()->toString();
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Fix all issues with AI agents
In @.github/workflows/deploy.yml:
- Around line 40-43: "Upload binaries to release" ステップの files 指定で無効な
runner.workspace を使っているためアップロード先が見つかりません。softprops/action-gh-release@v2 の files
入力を修正して runner.workspace を ${{ github.workspace }} に置き換え、tar
を作成したワーキングディレクトリの相対位置（../）を反映したパスにしてください（参照箇所: step name "Upload binaries to
release", uses: softprops/action-gh-release@v2, input "files"）。
- Around line 17-38: Wrap all uses of the environment variable GITHUB_WORKSPACE
in double quotes (e.g., " $GITHUB_WORKSPACE ") across the rm, find, chmod, cd
and tar commands to protect against spaces and globbing; also remove or replace
the invalid runner.workspace reference by computing the parent directory from
github.workspace (or pass the parent path in as an input) and use that parent
when creating the tar archive (the tar cvzf invocation and any ../ references
should be updated to use the computed parent path instead of runner.workspace).
In @.github/workflows/playwright.yml:
- Around line 38-47: The current "Wait for EC-CUBE to be ready" step can
silently succeed if the loop never detects readiness; change the shell loop so
that after trying up to 30 times it explicitly fails the job (exit non‑zero)
when curl never succeeds. Modify the existing loop in the step (the for/if/curl
logic) to exit 1 and print a clear error message like "EC-CUBE did not become
ready" when the loop completes without a successful curl, ensuring the CI job
fails instead of proceeding.
In `@Controller/Admin/PasskeyController.php`:
- Around line 82-86: 削除処理の結果判定で失敗時に登録用の翻訳キーを使っているため不整合が起きます；条件分岐内の addError
呼び出しで現在使われている 'ecauth_login43.admin.passkey.register.error' を削除用のキー
'ecauth_login43.admin.passkey.delete.error' に差し替えてください（参照箇所：変数 $result
のステータス判定とメソッド addSuccess/addError が使われている部分、該当メソッドは PasskeyController
の該当削除ハンドラです）。
- Around line 69-70: isTokenValid() を呼ぶ箇所は他の CSRF チェックと同様に isCsrfTokenValid()
を使って戻り値で明示的に検証し（verifyPassword()
と同じパターン）、検証失敗時は適切に処理してリクエストを拒否するよう修正してください。また、CSRF 検証後の DELETE
失敗時のフラッシュ/エラーメッセージキーを現在の register.error から削除操作用の
delete.error（存在しない場合は新規追加または適切なキーに差し替え）に変更して、削除失敗時に正しいメッセージが出るようにしてください（対象箇所参照:
isTokenValid(), isCsrfTokenValid(), verifyPassword(), および削除処理のエラーハンドリング）。
- Around line 109-114: The code calls $this->getUser() and passes the result to
passkeyAuthService->verifyPassword(Member $Member, ...) which can throw a
TypeError when getUser() returns null; add an authentication guard: check that
$this->getUser() returns a Member instance (e.g. if (!$Member = $this->getUser()
|| !($Member instanceof \Eccube\Entity\Member)) then return $this->json(['error'
=> 'ecauth_login43.admin.passkey.not_authenticated'], 401) or use
$this->denyAccessUnlessGranted('IS_AUTHENTICATED_FULLY') before calling
verifyPassword, ensuring verifyPassword only receives a non-null Member.
In `@Controller/EcAuthCallbackController.php`:
- Around line 51-54: PasskeyAuthService::handleCallback にあるセッション保存のタイポを修正してください
— 現在 Service/PasskeyAuthService.php の該当箇所は $session->set('ecauth_access_token',
accessToken); となっており $ が欠落しています。handleCallback メソッド内で正しい変数名 $accessToken を使用して
$session->set('ecauth_access_token', $accessToken);
のように修正し、同メソッド内で宣言・代入されている変数名と一致することを確認してください。
In `@Controller/PasskeyAuthController.php`:
- Around line 69-72: Validate that $data['response'] is an array before passing
it upstream: after decoding $data and checking isset($data['response']), add a
type check (is_array($data['response'])) and return $this->json(['error' =>
'Invalid request body: response must be an array'], 400) if it fails; update the
logic around the existing $data variable and any usages of 'response' in
PasskeyAuthController to rely on the validated array.
- Around line 44-55: 検証不足のため authenticateOptions の成功レスポンスに session_id
が欠落している場合に後続で「Session expired」になる問題があります。PasskeyAuthController の処理で
$result['status'] === 200 を確認した後に必須キー存在チェックを追加し、$result['data']['session_id']
が空なら即時に json エラー（例：'Missing session_id in authentication options'）を適切な 400
系ステータスで返すようにしてください；該当する参照箇所は getRpId(), apiClient->authenticateOptions(),
セッション保存時のキー 'ecauth_passkey_session_id'。同様の必須キー検査を registerOptions
を扱うコードパスにも追加してください。
- Around line 115-125: The controller currently trusts a client-supplied
b2b_subject from json_decode and passes it to apiClient->registerOptions;
instead, ignore the client value and fetch the subject from the authenticated
server-side Member by calling ensureB2BUser() (or equivalent method that returns
the Member/subject for the current user) and use that returned subject when
calling registerOptions; remove the $data['b2b_subject'] usage and replace it
with the subject from ensureB2BUser(), leaving display_name/device_name handling
unchanged and keeping registerOptions($rpId, $b2bSubject, $displayName,
$deviceName).
In `@Form/Type/Admin/ConfigType.php`:
- Around line 29-33: The client_secret field currently exposes stored secrets
(due to always_empty => false) and forces NotBlank validation; change the Form
field in ConfigType.php to be unmapped (set 'mapped' => false), remove the
NotBlank constraint and let the input render empty by default, then in the
controller action that handles the form submission (e.g., ConfigController's
edit/save method) read $form->get('client_secret')->getData() and only call
$Config->setClientSecret(...) and persist/flush when that value is non-empty,
preserving existing secrets when the field is left blank.
In `@package.json`:
- Around line 8-10: Update the `@playwright/test` devDependency from "^1.40.0" to
the current stable release (e.g., "1.55.0") in package.json by replacing the
version string for "@playwright/test"; after updating, run your package manager
(npm install or yarn install) to refresh lockfile and commit the generated
package-lock.json or yarn.lock to the repo to ensure reproducible builds and
include security/bugfix updates.
In `@Service/EcAuthApiClient.php`:
- Around line 241-246: The current error log in the $this->logger->error call
logs the full $content which may contain tokens/PII; change it to only log
minimal fields or a sanitized version of $content by implementing and using a
sanitizer (e.g. maskSensitiveFields or sanitizeAuthResponse) before logging —
specifically remove or mask keys like access_token, id_token, refresh_token and
any PII, and then call $this->logger->error with the sanitized response along
with $statusCode and $path instead of the raw $content.
In `@Service/PasskeyAuthService.php`:
- Around line 161-165: セッション固定化対策が抜けているため、UsernamePasswordToken
を保存する前にセッションIDを再生成してください：呼び出し箇所は UsernamePasswordToken を作成して
$this->tokenStorage->setToken($token) している部分です。具体的には $session（現在使用している Session
オブジェクト）でセッションIDを再生成（例：$session->migrate(true) または使用している Session
インターフェースの再生成メソッド）してから $this->tokenStorage->setToken($token) と
$session->set('_security_admin', serialize($token)) を実行するように順序を入れ替えてください。
- Around line 205-221: extractSubFromIdToken currently only base64-decodes the
payload and returns 'sub' without signature or claims checks and lacks proper
base64url padding; update extractSubFromIdToken to (1) perform base64url padding
normalization when decoding the middle JWT part, (2) validate the token
signature using the issuer's JWKS (fetch and cache keys, verify with the key
matching the token's kid), and (3) validate standard claims (iss, aud, exp/iat
as appropriate) before returning the 'sub'; retain the existing fallback of
returning null on any validation or parsing failure.
In `@tests/specs/passkey_auth.spec.ts`:
- Around line 15-39: Wrap the test body starting from creating the CDP session
and adding the virtual authenticator in a try-finally so the cleanup always
runs; move the await cdpSession.send('WebAuthn.removeVirtualAuthenticator', {
authenticatorId }) call into the finally block and ensure you still await it
(and optionally check authenticatorId is defined) while keeping the existing
assertions (e.g., passkeyBtn click and expect(passkeyBtn).toBeDisabled()) inside
the try.
In `@tests/specs/passkey_register.spec.ts`:
- Around line 17-49: Tests in passkey_register.spec.ts use the old route
`/ecauth/passkey/`; update every navigation URL that uses
`${ADMIN_URL}/ecauth/passkey/` to `${ADMIN_URL}/ecauth_login43/passkey/` so the
tests match the plugin naming convention used by ConfigController
(ecauth_login43); keep the rest of the test code (locators like
'#ecauth-passkey-add', '#ecauth-password-modal', etc.) unchanged.
🧹 Nitpick comments (10)
rector.php (1)
26-29: TYPE_DECLARATION ルールセットの追加を検討してください。PHPStan Level 6 と組み合わせる場合、
SetList::TYPE_DECLARATIONを追加すると型宣言の一貫性が向上します。♻️ 提案
$rectorConfig->sets([ SetList::CODE_QUALITY, SetList::DEAD_CODE, + SetList::TYPE_DECLARATION, ]);CLAUDE.md (1)
47-83: ディレクトリ構成のコードブロックに言語を指定してください。静的解析ツールが MD040 警告を報告しています。コードブロックに言語を指定することで、一貫性が向上します。
♻️ 提案
-``` +```text ec-cube4-ecauth/ ├── composer.json # type: eccube-plugin, code: EcAuthLogin43EcAuthLoginNav.php (1)
12-12: 戻り値の型宣言を追加することを検討してください。PHPStan Level 6 の要件に合わせて、明示的な戻り値の型宣言を追加するとより堅牢になります。
♻️ 修正案
/** - * `@return` array + * `@return` array<string, array<string, mixed>> */ - public static function getNav() + public static function getNav(): array {composer.json (1)
1-19: autoload 設定の追加を検討PSR-4 オートロード設定が定義されていません。EC-CUBE プラグインでは通常、プラグインインストーラーがオートロードを処理しますが、明示的な設定があると開発時やテスト時に便利です。
♻️ 推奨される autoload 設定
"require-dev": { "friendsofphp/php-cs-fixer": "^3.64", "phpstan/phpstan": "^1.10", "rector/rector": "^1.0" - } + }, + "autoload": { + "psr-4": { + "Plugin\\EcAuthLogin43\\": "" + } + } }Entity/MemberTrait.php (1)
13-18:ecauth_subjectカラムにインデックス追加を検討
ecauth_subjectはfindOneBy(['ecauth_subject' => $b2bSubject])でのルックアップに使用されます（PasskeyAuthService::handleCallback参照）。認証フロー中のパフォーマンス向上のため、インデックスの追加を検討してください。🔧 インデックス追加の例
/** * `@var` string|null * * `@ORM`\Column(name="ecauth_subject", type="string", length=255, nullable=true) + * `@ORM`\Index(name="idx_ecauth_subject") */ private $ecauth_subject;または、エンティティクラスレベルで
@ORM\Tableアノテーションでインデックスを定義することも可能です。EC-CUBE の EntityExtension でトレイトにインデックスを追加する場合は、マイグレーションファイルでの対応が必要になる場合があります。Controller/EcAuthCallbackController.php (1)
28-63: コールバック処理のロジックは適切です
- エラーパラメータのチェック
codeとstateの存在確認PasskeyAuthServiceへの適切な委譲- セキュリティ上、エラーメッセージが汎用的なのは良い設計です
メソッドに戻り値の型ヒントを追加することを検討してください：
♻️ 型ヒント追加の提案
- public function callback(Request $request) + public function callback(Request $request): \Symfony\Component\HttpFoundation\RedirectResponse,
.github/workflows/ci.yml (1)
22-23: Composer 依存関係のキャッシュ追加を推奨各ジョブで
composer installを実行していますが、キャッシュがないため毎回ダウンロードが発生します。CI の実行時間短縮のため、キャッシュの追加を検討してください。♻️ キャッシュ追加の例（各ジョブに適用）
- name: Setup PHP uses: shivammathur/setup-php@v2 with: php-version: '8.3' tools: composer + - name: Get Composer cache directory + id: composer-cache + run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT + + - name: Cache Composer dependencies + uses: actions/cache@v4 + with: + path: ${{ steps.composer-cache.outputs.dir }} + key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} + restore-keys: ${{ runner.os }}-composer- + - name: Install dependencies run: composer install --no-progress --prefer-distAlso applies to: 40-41, 58-59
tests/specs/plugin_config.spec.ts (1)
22-38: client_secret の再表示仕様をテストで明示すると安全です。再訪時に client_secret を空にする仕様であれば、空であることを assert しておくと漏えい系の回帰を検知しやすくなります。
Resource/template/admin/passkey_list.twig (1)
8-137: インライン CSS/JS は資産ファイルへ切り出し推奨です。CSP 強化・キャッシュ最適化の観点で、plugin の assets に移す方が運用しやすくなります。
Resource/template/admin/login_passkey.twig (1)
33-36: 認証中ラベルも翻訳キーに統一したい
'認証中...'がハードコードされているため、他言語ロケールで日本語が混ざります。翻訳キー化して一貫させたいです。💡 例: 翻訳キー化
- passkeyBtn.textContent = '認証中...'; + passkeyBtn.textContent = '{{ "ecauth_login43.admin.login.passkey.loading"|trans }}';
| run: | | ||
| rm -rf $GITHUB_WORKSPACE/.github | ||
| rm -rf $GITHUB_WORKSPACE/tests | ||
| rm -rf $GITHUB_WORKSPACE/node_modules | ||
| rm -f $GITHUB_WORKSPACE/Dockerfile | ||
| rm -f $GITHUB_WORKSPACE/docker-compose.yml | ||
| rm -f $GITHUB_WORKSPACE/docker-compose.override.yml | ||
| rm -f $GITHUB_WORKSPACE/docker-entrypoint.sh | ||
| rm -f $GITHUB_WORKSPACE/package.json | ||
| rm -f $GITHUB_WORKSPACE/package-lock.json | ||
| rm -f $GITHUB_WORKSPACE/yarn.lock | ||
| rm -f $GITHUB_WORKSPACE/playwright.config.ts | ||
| rm -f $GITHUB_WORKSPACE/phpstan.neon.dist | ||
| rm -f $GITHUB_WORKSPACE/rector.php | ||
| rm -f $GITHUB_WORKSPACE/.php-cs-fixer.dist.php | ||
| rm -f $GITHUB_WORKSPACE/.env.tpl | ||
| rm -f $GITHUB_WORKSPACE/CLAUDE.md | ||
| find $GITHUB_WORKSPACE -name "dummy" -delete | ||
| find $GITHUB_WORKSPACE -name ".git*" -and ! -name ".gitkeep" -print0 | xargs -0 rm -rf | ||
| chmod -R o+w $GITHUB_WORKSPACE | ||
| cd $GITHUB_WORKSPACE | ||
| tar cvzf ../${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz ./* |
There was a problem hiding this comment.
シェル変数のクォートと runner.workspace の修正が必要です。
$GITHUB_WORKSPACEは空白やグロブを防ぐためにダブルクォートで囲む必要があります。- Line 43 の
runner.workspaceは存在しないプロパティです。親ディレクトリを参照する場合はgithub.workspaceの親ディレクトリを計算するか、別の方法を使用してください。
🔧 修正案
working-directory: ../
run: |
- rm -rf $GITHUB_WORKSPACE/.github
- rm -rf $GITHUB_WORKSPACE/tests
- rm -rf $GITHUB_WORKSPACE/node_modules
- rm -f $GITHUB_WORKSPACE/Dockerfile
- rm -f $GITHUB_WORKSPACE/docker-compose.yml
- rm -f $GITHUB_WORKSPACE/docker-compose.override.yml
- rm -f $GITHUB_WORKSPACE/docker-entrypoint.sh
- rm -f $GITHUB_WORKSPACE/package.json
- rm -f $GITHUB_WORKSPACE/package-lock.json
- rm -f $GITHUB_WORKSPACE/yarn.lock
- rm -f $GITHUB_WORKSPACE/playwright.config.ts
- rm -f $GITHUB_WORKSPACE/phpstan.neon.dist
- rm -f $GITHUB_WORKSPACE/rector.php
- rm -f $GITHUB_WORKSPACE/.php-cs-fixer.dist.php
- rm -f $GITHUB_WORKSPACE/.env.tpl
- rm -f $GITHUB_WORKSPACE/CLAUDE.md
- find $GITHUB_WORKSPACE -name "dummy" -delete
- find $GITHUB_WORKSPACE -name ".git*" -and ! -name ".gitkeep" -print0 | xargs -0 rm -rf
- chmod -R o+w $GITHUB_WORKSPACE
- cd $GITHUB_WORKSPACE
+ rm -rf "$GITHUB_WORKSPACE/.github"
+ rm -rf "$GITHUB_WORKSPACE/tests"
+ rm -rf "$GITHUB_WORKSPACE/node_modules"
+ rm -f "$GITHUB_WORKSPACE/Dockerfile"
+ rm -f "$GITHUB_WORKSPACE/docker-compose.yml"
+ rm -f "$GITHUB_WORKSPACE/docker-compose.override.yml"
+ rm -f "$GITHUB_WORKSPACE/docker-entrypoint.sh"
+ rm -f "$GITHUB_WORKSPACE/package.json"
+ rm -f "$GITHUB_WORKSPACE/package-lock.json"
+ rm -f "$GITHUB_WORKSPACE/yarn.lock"
+ rm -f "$GITHUB_WORKSPACE/playwright.config.ts"
+ rm -f "$GITHUB_WORKSPACE/phpstan.neon.dist"
+ rm -f "$GITHUB_WORKSPACE/rector.php"
+ rm -f "$GITHUB_WORKSPACE/.php-cs-fixer.dist.php"
+ rm -f "$GITHUB_WORKSPACE/.env.tpl"
+ rm -f "$GITHUB_WORKSPACE/CLAUDE.md"
+ find "$GITHUB_WORKSPACE" -name "dummy" -delete
+ find "$GITHUB_WORKSPACE" -name ".git*" -and ! -name ".gitkeep" -print0 | xargs -0 rm -rf
+ chmod -R o+w "$GITHUB_WORKSPACE"
+ cd "$GITHUB_WORKSPACE"
tar cvzf ../${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz ./*📝 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.
| run: | | |
| rm -rf $GITHUB_WORKSPACE/.github | |
| rm -rf $GITHUB_WORKSPACE/tests | |
| rm -rf $GITHUB_WORKSPACE/node_modules | |
| rm -f $GITHUB_WORKSPACE/Dockerfile | |
| rm -f $GITHUB_WORKSPACE/docker-compose.yml | |
| rm -f $GITHUB_WORKSPACE/docker-compose.override.yml | |
| rm -f $GITHUB_WORKSPACE/docker-entrypoint.sh | |
| rm -f $GITHUB_WORKSPACE/package.json | |
| rm -f $GITHUB_WORKSPACE/package-lock.json | |
| rm -f $GITHUB_WORKSPACE/yarn.lock | |
| rm -f $GITHUB_WORKSPACE/playwright.config.ts | |
| rm -f $GITHUB_WORKSPACE/phpstan.neon.dist | |
| rm -f $GITHUB_WORKSPACE/rector.php | |
| rm -f $GITHUB_WORKSPACE/.php-cs-fixer.dist.php | |
| rm -f $GITHUB_WORKSPACE/.env.tpl | |
| rm -f $GITHUB_WORKSPACE/CLAUDE.md | |
| find $GITHUB_WORKSPACE -name "dummy" -delete | |
| find $GITHUB_WORKSPACE -name ".git*" -and ! -name ".gitkeep" -print0 | xargs -0 rm -rf | |
| chmod -R o+w $GITHUB_WORKSPACE | |
| cd $GITHUB_WORKSPACE | |
| tar cvzf ../${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz ./* | |
| run: | | |
| rm -rf "$GITHUB_WORKSPACE/.github" | |
| rm -rf "$GITHUB_WORKSPACE/tests" | |
| rm -rf "$GITHUB_WORKSPACE/node_modules" | |
| rm -f "$GITHUB_WORKSPACE/Dockerfile" | |
| rm -f "$GITHUB_WORKSPACE/docker-compose.yml" | |
| rm -f "$GITHUB_WORKSPACE/docker-compose.override.yml" | |
| rm -f "$GITHUB_WORKSPACE/docker-entrypoint.sh" | |
| rm -f "$GITHUB_WORKSPACE/package.json" | |
| rm -f "$GITHUB_WORKSPACE/package-lock.json" | |
| rm -f "$GITHUB_WORKSPACE/yarn.lock" | |
| rm -f "$GITHUB_WORKSPACE/playwright.config.ts" | |
| rm -f "$GITHUB_WORKSPACE/phpstan.neon.dist" | |
| rm -f "$GITHUB_WORKSPACE/rector.php" | |
| rm -f "$GITHUB_WORKSPACE/.php-cs-fixer.dist.php" | |
| rm -f "$GITHUB_WORKSPACE/.env.tpl" | |
| rm -f "$GITHUB_WORKSPACE/CLAUDE.md" | |
| find "$GITHUB_WORKSPACE" -name "dummy" -delete | |
| find "$GITHUB_WORKSPACE" -name ".git*" -and ! -name ".gitkeep" -print0 | xargs -0 rm -rf | |
| chmod -R o+w "$GITHUB_WORKSPACE" | |
| cd "$GITHUB_WORKSPACE" | |
| tar cvzf ../${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz ./* |
🧰 Tools
🪛 actionlint (1.7.10)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:10:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:11:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:12:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:13:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:14:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:15:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:16:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:17:6: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:18:6: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:19:14: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:20:4: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:3:8: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:4:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:5:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:6:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:7:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:8:7: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 17-17: shellcheck reported issue in this script: SC2086:info:9:7: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
In @.github/workflows/deploy.yml around lines 17 - 38, Wrap all uses of the
environment variable GITHUB_WORKSPACE in double quotes (e.g., "
$GITHUB_WORKSPACE ") across the rm, find, chmod, cd and tar commands to protect
against spaces and globbing; also remove or replace the invalid runner.workspace
reference by computing the parent directory from github.workspace (or pass the
parent path in as an input) and use that parent when creating the tar archive
(the tar cvzf invocation and any ../ references should be updated to use the
computed parent path instead of runner.workspace).
| - name: Upload binaries to release | ||
| uses: softprops/action-gh-release@v2 | ||
| with: | ||
| files: ${{ runner.workspace }}/${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz |
There was a problem hiding this comment.
runner.workspace は無効なプロパティです。
runner コンテキストには workspace プロパティが存在しません。tar ファイルは working-directory: ../ で作成されているため、アップロードパスを修正してください。
🔧 修正案
- name: Upload binaries to release
uses: softprops/action-gh-release@v2
with:
- files: ${{ runner.workspace }}/${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz
+ files: ${{ github.workspace }}/../${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz📝 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.
| - name: Upload binaries to release | |
| uses: softprops/action-gh-release@v2 | |
| with: | |
| files: ${{ runner.workspace }}/${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz | |
| - name: Upload binaries to release | |
| uses: softprops/action-gh-release@v2 | |
| with: | |
| files: ${{ github.workspace }}/../${{ github.event.repository.name }}-${{ github.event.release.tag_name }}.tar.gz |
🧰 Tools
🪛 actionlint (1.7.10)
[error] 43-43: property "workspace" is not defined in object type {arch: string; debug: string; environment: string; name: string; os: string; temp: string; tool_cache: string}
(expression)
🤖 Prompt for AI Agents
In @.github/workflows/deploy.yml around lines 40 - 43, "Upload binaries to
release" ステップの files 指定で無効な runner.workspace
を使っているためアップロード先が見つかりません。softprops/action-gh-release@v2 の files 入力を修正して
runner.workspace を ${{ github.workspace }} に置き換え、tar
を作成したワーキングディレクトリの相対位置（../）を反映したパスにしてください（参照箇所: step name "Upload binaries to
release", uses: softprops/action-gh-release@v2, input "files"）。
| - name: Wait for EC-CUBE to be ready | ||
| run: | | ||
| for i in $(seq 1 30); do | ||
| if curl -sk https://localhost:4430/admin/login > /dev/null 2>&1; then | ||
| echo "EC-CUBE is ready" | ||
| break | ||
| fi | ||
| echo "Waiting for EC-CUBE... ($i/30)" | ||
| sleep 5 | ||
| done |
There was a problem hiding this comment.
EC-CUBE 未起動時にジョブが成功扱いになる可能性があります。
準備完了しない場合は明示的に失敗させる方が原因切り分けが容易です。
修正案
- name: Wait for EC-CUBE to be ready
run: |
- for i in $(seq 1 30); do
+ ready=false
+ for i in $(seq 1 30); do
if curl -sk https://localhost:4430/admin/login > /dev/null 2>&1; then
echo "EC-CUBE is ready"
+ ready=true
break
fi
echo "Waiting for EC-CUBE... ($i/30)"
sleep 5
done
+ if [ "$ready" != "true" ]; then
+ echo "EC-CUBE did not become ready in time" >&2
+ exit 1
+ fi📝 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.
| - name: Wait for EC-CUBE to be ready | |
| run: | | |
| for i in $(seq 1 30); do | |
| if curl -sk https://localhost:4430/admin/login > /dev/null 2>&1; then | |
| echo "EC-CUBE is ready" | |
| break | |
| fi | |
| echo "Waiting for EC-CUBE... ($i/30)" | |
| sleep 5 | |
| done | |
| - name: Wait for EC-CUBE to be ready | |
| run: | | |
| ready=false | |
| for i in $(seq 1 30); do | |
| if curl -sk https://localhost:4430/admin/login > /dev/null 2>&1; then | |
| echo "EC-CUBE is ready" | |
| ready=true | |
| break | |
| fi | |
| echo "Waiting for EC-CUBE... ($i/30)" | |
| sleep 5 | |
| done | |
| if [ "$ready" != "true" ]; then | |
| echo "EC-CUBE did not become ready in time" >&2 | |
| exit 1 | |
| fi |
🤖 Prompt for AI Agents
In @.github/workflows/playwright.yml around lines 38 - 47, The current "Wait for
EC-CUBE to be ready" step can silently succeed if the loop never detects
readiness; change the shell loop so that after trying up to 30 times it
explicitly fails the job (exit non‑zero) when curl never succeeds. Modify the
existing loop in the step (the for/if/curl logic) to exit 1 and print a clear
error message like "EC-CUBE did not become ready" when the loop completes
without a successful curl, ensuring the CI job fails instead of proceeding.
| $this->isTokenValid(); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the PasskeyController file and examine the delete method
fd -e php -p "PasskeyController" | head -5Repository: EcAuth/ec-cube4-ecauth
Length of output: 104
🏁 Script executed:
# Search for isTokenValid implementation
rg -n "function isTokenValid|public.*isTokenValid" -g '*.php' | head -20Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Check how isTokenValid is used throughout the codebase
rg -n "isTokenValid\(\)" -g '*.php' -B 2 -A 2 | head -40Repository: EcAuth/ec-cube4-ecauth
Length of output: 423
🏁 Script executed:
# Read the PasskeyController.php file to understand the full context
cat -n Controller/Admin/PasskeyController.php | head -100Repository: EcAuth/ec-cube4-ecauth
Length of output: 3893
🏁 Script executed:
# Find parent class and isTokenValid implementation
rg -n "class PasskeyController" -A 2 -g '*.php'Repository: EcAuth/ec-cube4-ecauth
Length of output: 252
🏁 Script executed:
# Search for isTokenValid in all PHP files to find the implementation
rg -n "function isTokenValid|public.*isTokenValid|private.*isTokenValid" -g '*.php' -B 1 -A 5Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Search for isTokenValid usage in other controllers to understand the pattern
rg -n "isTokenValid\(" -g '*.php' -B 3 -A 3Repository: EcAuth/ec-cube4-ecauth
Length of output: 576
🏁 Script executed:
# Look for AbstractController or isTokenValid in the Eccube framework (vendor directory if present)
find . -name "AbstractController.php" | head -5Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Check if there's a base class or trait that defines isTokenValid
rg -n "isTokenValid" -g '*.php' --type-listRepository: EcAuth/ec-cube4-ecauth
Length of output: 455
🏁 Script executed:
# Check if vendor directory exists and find AbstractController
find . -path "./vendor/*/eccube/*AbstractController.php" 2>/dev/null | head -3Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Search for AbstractController in the entire repo
find . -name "AbstractController.php" -type f 2>/dev/nullRepository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Look for any documentation or comments about isTokenValid
rg -n "isTokenValid" -g '*.php' -B 5 -A 10 | head -60Repository: EcAuth/ec-cube4-ecauth
Length of output: 1304
🏁 Script executed:
# Check composer.json to understand dependencies
cat composer.json 2>/dev/null | head -40Repository: EcAuth/ec-cube4-ecauth
Length of output: 486
🏁 Script executed:
# Look for any exception handling or try-catch patterns around isTokenValid calls
rg -n "try|catch|throw|BadCredentials|TokenException" -g '*.php' -B 2 -A 2 | head -40Repository: EcAuth/ec-cube4-ecauth
Length of output: 1493
🏁 Script executed:
# Check for other admin controller patterns in the plugin
find . -path "*/Controller/Admin/*.php" -type f | xargs wc -l 2>/dev/nullRepository: EcAuth/ec-cube4-ecauth
Length of output: 167
🏁 Script executed:
# Read more of PasskeyController to see complete context
cat -n Controller/Admin/PasskeyController.php | sed -n '60,120p'Repository: EcAuth/ec-cube4-ecauth
Length of output: 2652
🏁 Script executed:
# Read the ConfigController to see how it uses isTokenValid
cat -n Controller/Admin/ConfigController.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 1841
🏁 Script executed:
# Search for message keys to understand the naming pattern
rg -n "ecauth_login43\.admin\.passkey\." -g '*.php'Repository: EcAuth/ec-cube4-ecauth
Length of output: 711
🏁 Script executed:
# Look for EC-CUBE documentation or AbstractController patterns in comments
rg -n "isCsrfTokenValid|isTokenValid" -g '*.php' -B 5 -A 5Repository: EcAuth/ec-cube4-ecauth
Length of output: 5767
🌐 Web query:
isTokenValid AbstractController EC-CUBE 4.3 CSRF return type
💡 Result:
In EC-CUBE 4.3, Eccube\Controller\AbstractController::isTokenValid() (CSRF check) has the return type:
bool(returnstruewhen valid)- If the token is invalid, it throws
Symfony\Component\HttpKernel\Exception\AccessDeniedHttpExceptioninstead of returningfalse. [1]
Sources:
[1] EC-CUBE 4 API doc (AbstractController.php isTokenValid() : bool, throws AccessDeniedHttpException)
🏁 Script executed:
# Verify message keys - check if there are specific delete error messages
rg -n "delete\.error|delete\.fail" -g '*.php'Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Check translation files for available message keys
find . -name "*.yaml" -o -name "*.yml" | xargs grep -l "passkey" 2>/dev/null | head -5Repository: EcAuth/ec-cube4-ecauth
Length of output: 132
🏁 Script executed:
# Check the translation file for passkey message keys
cat Resource/locale/messages.ja.yaml | grep -A 2 -B 2 "passkey"Repository: EcAuth/ec-cube4-ecauth
Length of output: 1535
DELETE エンドポイントの CSRF トークン検証を明示的に扱い、エラーメッセージキーを修正してください
isTokenValid() は EC-CUBE フレームワークのメソッドで、無効時は例外を投げるため現在のコードは動作していますが、同ファイル内の他の CSRF チェック（98 行目の verifyPassword() など）では isCsrfTokenValid() を使用して戻り値を明示的に確認しているため、一貫性に欠けます。
また、69 行目での CSRF 検証後にエラー処理がないため、DELETE 失敗時のエラーメッセージ（85 行目）が誤っています。現在は register.error （"パスキーの登録に失敗しました"）が表示されていますが、削除操作のエラーメッセージを使用すべきです。
修正例
- $this->isTokenValid();
+ if (!$this->isTokenValid()) {
+ $this->addError('admin.common.invalid_token', 'admin');
+ return $this->redirectToRoute('ecauth_login43_admin_passkey');
+ }- $this->addError('ecauth_login43.admin.passkey.register.error', 'admin');
+ $this->addError('ecauth_login43.admin.passkey.delete.error', 'admin');※ delete.error メッセージキーが存在しない場合は、新たに追加するか、別の適切なキーを使用してください。
🤖 Prompt for AI Agents
In `@Controller/Admin/PasskeyController.php` around lines 69 - 70, isTokenValid()
を呼ぶ箇所は他の CSRF チェックと同様に isCsrfTokenValid() を使って戻り値で明示的に検証し（verifyPassword()
と同じパターン）、検証失敗時は適切に処理してリクエストを拒否するよう修正してください。また、CSRF 検証後の DELETE
失敗時のフラッシュ/エラーメッセージキーを現在の register.error から削除操作用の
delete.error（存在しない場合は新規追加または適切なキーに差し替え）に変更して、削除失敗時に正しいメッセージが出るようにしてください（対象箇所参照:
isTokenValid(), isCsrfTokenValid(), verifyPassword(), および削除処理のエラーハンドリング）。
| if ($result['status'] === 200 || $result['status'] === 204) { | ||
| $this->addSuccess('ecauth_login43.admin.passkey.delete.success', 'admin'); | ||
| } else { | ||
| $this->addError('ecauth_login43.admin.passkey.register.error', 'admin'); | ||
| } |
There was a problem hiding this comment.
削除失敗時のメッセージキーが登録用です
削除失敗時に register.error を使うと翻訳が不一致になるので、削除用キーに揃えてください。
📝 修正例
- $this->addError('ecauth_login43.admin.passkey.register.error', 'admin');
+ $this->addError('ecauth_login43.admin.passkey.delete.error', 'admin');🤖 Prompt for AI Agents
In `@Controller/Admin/PasskeyController.php` around lines 82 - 86,
削除処理の結果判定で失敗時に登録用の翻訳キーを使っているため不整合が起きます；条件分岐内の addError 呼び出しで現在使われている
'ecauth_login43.admin.passkey.register.error' を削除用のキー
'ecauth_login43.admin.passkey.delete.error' に差し替えてください（参照箇所：変数 $result
のステータス判定とメソッド addSuccess/addError が使われている部分、該当メソッドは PasskeyController
の該当削除ハンドラです）。
| if ($statusCode >= 400) { | ||
| $this->logger->error('EcAuth API error', [ | ||
| 'status' => $statusCode, | ||
| 'path' => $path, | ||
| 'response' => $content, | ||
| ]); |
There was a problem hiding this comment.
エラーログにレスポンス全量を残すとトークン/PII漏えいの恐れがあります。
最低限のフィールドに絞るか、access_token/id_token 等はマスクしてください。
🛠️ マスキング例
- if ($statusCode >= 400) {
- $this->logger->error('EcAuth API error', [
- 'status' => $statusCode,
- 'path' => $path,
- 'response' => $content,
- ]);
- }
+ if ($statusCode >= 400) {
+ $redacted = $content;
+ foreach (['access_token', 'id_token', 'refresh_token', 'client_secret'] as $key) {
+ if (isset($redacted[$key])) {
+ $redacted[$key] = '[REDACTED]';
+ }
+ }
+ $this->logger->error('EcAuth API error', [
+ 'status' => $statusCode,
+ 'path' => $path,
+ 'response' => $redacted,
+ ]);
+ }📝 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.
| if ($statusCode >= 400) { | |
| $this->logger->error('EcAuth API error', [ | |
| 'status' => $statusCode, | |
| 'path' => $path, | |
| 'response' => $content, | |
| ]); | |
| if ($statusCode >= 400) { | |
| $redacted = $content; | |
| foreach (['access_token', 'id_token', 'refresh_token', 'client_secret'] as $key) { | |
| if (isset($redacted[$key])) { | |
| $redacted[$key] = '[REDACTED]'; | |
| } | |
| } | |
| $this->logger->error('EcAuth API error', [ | |
| 'status' => $statusCode, | |
| 'path' => $path, | |
| 'response' => $redacted, | |
| ]); | |
| } |
🤖 Prompt for AI Agents
In `@Service/EcAuthApiClient.php` around lines 241 - 246, The current error log in
the $this->logger->error call logs the full $content which may contain
tokens/PII; change it to only log minimal fields or a sanitized version of
$content by implementing and using a sanitizer (e.g. maskSensitiveFields or
sanitizeAuthResponse) before logging — specifically remove or mask keys like
access_token, id_token, refresh_token and any PII, and then call
$this->logger->error with the sanitized response along with $statusCode and
$path instead of the raw $content.
| // 管理者セッション確立 | ||
| $token = new UsernamePasswordToken($Member, 'admin', $Member->getRoles()); | ||
| $this->tokenStorage->setToken($token); | ||
| $session->set('_security_admin', serialize($token)); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the PasskeyAuthService.php file
find . -name "PasskeyAuthService.php" -type fRepository: EcAuth/ec-cube4-ecauth
Length of output: 98
🏁 Script executed:
# Search for session-related code in the service files
rg -l "tokenStorage|setToken" --type phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 96
🌐 Web query:
Symfony session fixation attack prevention migrate()
💡 Result:
In Symfony, the primary defense against session fixation is to regenerate the session ID immediately after a user becomes authenticated.
Use the Security firewall strategy (recommended)
Symfony’s SecurityBundle can automatically regenerate the session ID after login via session_fixation_strategy (default is MIGRATE). With MIGRATE, Symfony changes the session ID but keeps existing session attributes; with INVALIDATE, it regenerates the session and clears attributes. [1]
# config/packages/security.yaml
security:
firewalls:
main:
# default is MIGRATE
session_fixation_strategy: migrate # or: invalidate(Values correspond to SessionAuthenticationStrategy::MIGRATE / ::INVALIDATE.) [1]
What $session->migrate() does (and when to call it)
At the session API level, calling migrate() regenerates the session ID, which breaks fixation attacks that rely on a known/fixed ID. Typically you do this right after privilege elevation (login), especially if you’re doing custom authentication flows. [1]
Practical mapping: Security’s
session_fixation_strategy: migrateis the “keep attributes but change session ID” behavior. [1]
Important: make sure you’re on a patched Symfony version
Symfony disclosed a case where the session was not always migrated after successful login in certain flows; it was fixed in Symfony 5.4.31 and 6.3.8 (affecting symfony/security-http in specific version ranges). If you’re on affected versions, update. [2]
Sources: Symfony Security config reference (session fixation strategy) [1]; Symfony security advisory CVE-2023-46733 (fix versions and description) [2].
🏁 Script executed:
# Read the PasskeyAuthService.php file to see the context around lines 161-165
cat -n ./Service/PasskeyAuthService.php | head -200 | tail -60Repository: EcAuth/ec-cube4-ecauth
Length of output: 2107
🏁 Script executed:
# Check the full context - get more lines around the issue
cat -n ./Service/PasskeyAuthService.php | sed -n '140,180p'Repository: EcAuth/ec-cube4-ecauth
Length of output: 1516
🏁 Script executed:
# Search for session_fixation_strategy in the codebase
rg "session_fixation_strategy" --type yaml --type ymlRepository: EcAuth/ec-cube4-ecauth
Length of output: 93
🏁 Script executed:
# Check for any other session migration calls in the codebase
rg "session.*migrate|migrate.*session" --type php -A 2 -B 2Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Check the Symfony version being used
rg "symfony/.*:" --type json | head -20Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Check where this PasskeyAuthService method is being called
rg "PasskeyAuthService|passkey|authenticateWithToken" --type php -B 3 -A 3 | head -100Repository: EcAuth/ec-cube4-ecauth
Length of output: 5467
🏁 Script executed:
# Look for the method definition to understand what parameters are being passed
cat -n ./Service/PasskeyAuthService.php | sed -n '1,170p' | grep -A 30 "public function"Repository: EcAuth/ec-cube4-ecauth
Length of output: 3291
🏁 Script executed:
# Search for security configuration files
find . -name "security.yaml" -o -name "security.yml" -o -name "*security*" -type f | grep -E "\.(yaml|yml)$"Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Check composer.json or similar for Symfony version
find . -name "composer.json" -o -name "composer.lock" | head -5Repository: EcAuth/ec-cube4-ecauth
Length of output: 81
🏁 Script executed:
# Look for the method signature of the function containing lines 161-165
cat -n ./Service/PasskeyAuthService.php | sed -n '100,165p'Repository: EcAuth/ec-cube4-ecauth
Length of output: 2714
🏁 Script executed:
# Check the EcAuthCallbackController to see how this method is called
cat -n ./Controller/EcAuthCallbackController.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 2505
🏁 Script executed:
# Look for Symfony version in composer.json
cat ./composer.json | grep -A 20 -B 5 "symfony"Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Search for session_fixation or migrate calls in the whole codebase
rg "migrate|session_fixation" --type php --type yamlRepository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Check if there's a security.yaml configuration
find . -type f \( -name "*.yaml" -o -name "*.yml" \) | xargs grep -l "session_fixation\|security" 2>/dev/null | head -5Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
# Check if composer.json has Symfony dependency info
cat ./composer.jsonRepository: EcAuth/ec-cube4-ecauth
Length of output: 486
🏁 Script executed:
# Look for any admin firewall or security configuration in the codebase
find . -type f \( -name "*.php" -o -name "*.yaml" -o -name "*.yml" \) | xargs grep -l "admin.*firewall\|firewall.*admin" 2>/dev/nullRepository: EcAuth/ec-cube4-ecauth
Length of output: 50
🏁 Script executed:
# Check the full PasskeyAuthService file to understand the entire authentication flow
wc -l ./Service/PasskeyAuthService.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 102
🏁 Script executed:
# Read the entire PasskeyAuthService.php file
cat -n ./Service/PasskeyAuthService.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 8103
🏁 Script executed:
# Check if there are any other authentication handlers in the codebase
rg "handleCallback|authenticat" --type php -B 2 -A 5Repository: EcAuth/ec-cube4-ecauth
Length of output: 6491
🏁 Script executed:
# Look for any EC-CUBE configuration or parent class that might handle session fixation
rg "extends|implements" ./Service/PasskeyAuthService.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 48
ログイン確立時にセッションID再生成がなく固定化リスクがあります。
この実装はカスタム認証フロー（Symfony firewall を経由しない）のため、セキュリティバンドルの session_fixation_strategy が自動適用されません。トークン保存前にセッションIDを再生成してセッション固定化攻撃から保護してください。
🛠️ 修正案
// 管理者セッション確立
+ $session->migrate(true);
$token = new UsernamePasswordToken($Member, 'admin', $Member->getRoles());
$this->tokenStorage->setToken($token);
$session->set('_security_admin', serialize($token));🤖 Prompt for AI Agents
In `@Service/PasskeyAuthService.php` around lines 161 - 165,
セッション固定化対策が抜けているため、UsernamePasswordToken を保存する前にセッションIDを再生成してください：呼び出し箇所は
UsernamePasswordToken を作成して $this->tokenStorage->setToken($token) している部分です。具体的には
$session（現在使用している Session オブジェクト）でセッションIDを再生成（例：$session->migrate(true)
または使用している Session インターフェースの再生成メソッド）してから $this->tokenStorage->setToken($token) と
$session->set('_security_admin', serialize($token)) を実行するように順序を入れ替えてください。
| /** | ||
| * ID Token (JWT) から sub クレームを取得する。 | ||
| * 署名検証は EcAuth 側で実施済みのため、ペイロードのデコードのみ行う。 | ||
| */ | ||
| private function extractSubFromIdToken(string $idToken): ?string | ||
| { | ||
| $parts = explode('.', $idToken); | ||
| if (count($parts) !== 3) { | ||
| return null; | ||
| } | ||
|
|
||
| $payload = json_decode(base64_decode(strtr($parts[1], '-_', '+/')), true); | ||
| if (!is_array($payload) || !isset($payload['sub'])) { | ||
| return null; | ||
| } | ||
|
|
||
| return $payload['sub']; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.php" | grep -i passkey | head -20Repository: EcAuth/ec-cube4-ecauth
Length of output: 178
🏁 Script executed:
find . -type f -name "PasskeyAuthService.php"Repository: EcAuth/ec-cube4-ecauth
Length of output: 98
🏁 Script executed:
git ls-files | grep -i passkeyRepository: EcAuth/ec-cube4-ecauth
Length of output: 327
🏁 Script executed:
wc -l ./Service/PasskeyAuthService.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 102
🏁 Script executed:
cat -n ./Service/PasskeyAuthService.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 8103
🏁 Script executed:
find . -type f -name "EcAuthApiClient.php"Repository: EcAuth/ec-cube4-ecauth
Length of output: 95
🏁 Script executed:
git ls-files | grep -i apiclientRepository: EcAuth/ec-cube4-ecauth
Length of output: 93
🏁 Script executed:
wc -l ./Service/EcAuthApiClient.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 99
🏁 Script executed:
cat -n ./Service/EcAuthApiClient.phpRepository: EcAuth/ec-cube4-ecauth
Length of output: 8807
🏁 Script executed:
grep -r "jwt\|JWT\|json_decode.*base64" --include="*.php" | head -20Repository: EcAuth/ec-cube4-ecauth
Length of output: 250
🏁 Script executed:
cat composer.json | grep -i jwtRepository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
cat composer.jsonRepository: EcAuth/ec-cube4-ecauth
Length of output: 486
🌐 Web query:
PHP base64_decode base64url padding requirement JWT
💡 Result:
JWT uses Base64URL without padding: per JWS/JWT, you take normal Base64, swap +→-, /→_, and omit trailing = padding. When decoding, you typically add the padding back based on length, then run a standard Base64 decoder. [1]
In PHP, base64_decode() is a standard Base64 decoder (not Base64URL). It will happily decode many “almost valid” inputs unless you enable strict mode; with $strict=true, it returns false if the input contains characters outside the Base64 alphabet. [2]
Practical rule for JWT parts (header/payload/signature) in PHP
- Convert Base64URL → Base64:
-_/→+/ - Add padding so length is a multiple of 4:
len % 4 == 0: add nothinglen % 4 == 2: add==len % 4 == 3: add=len % 4 == 1: malformed Base64URL [1]
function b64url_decode(string $s): string|false {
$s = strtr($s, '-_', '+/');
$pad = strlen($s) % 4;
if ($pad === 1) return false; // malformed
if ($pad) $s .= str_repeat('=', 4 - $pad); // restore padding
return base64_decode($s, true); // strict
}Notes:
- JWT compact serialization is defined using unpadded base64url; some decoders may still accept padded input, but it’s non-canonical and can cause interop/signature-input mismatches across implementations. [1]
Sources: RFC 7515 Appendix C (base64url without padding; how to restore padding) [1]; PHP manual for base64_decode() and strict mode behavior [2].
[1] https://rfcinfo.com/rfc-7515/appendix-c/
[2] https://www.php.net/manual/en/function.base64-decode.php
IDトークンの署名検証とクレーム検証が不足しており、なりすましリスクがあります。
extractSubFromIdToken() はペイロードのデコードのみで、署名検証・iss/aud/exp などのクレーム検証が行われていません。EcAuth側での検証に頼るだけでは、トークン改ざんや有効期限切れトークンの再利用などを防げません。JWKS による署名検証とクレーム検証を追加してください。
併せて、base64url デコード時のパディング補完が不足しており、ペイロードサイズによってはデコード失敗が発生します。
🛠️ base64url パディング補完の修正例（署名検証は別途必須）
- $payload = json_decode(base64_decode(strtr($parts[1], '-_', '+/')), true);
+ $payloadB64 = $parts[1];
+ $payloadB64 .= str_repeat('=', (4 - strlen($payloadB64) % 4) % 4);
+ $payload = json_decode(base64_decode(strtr($payloadB64, '-_', '+/')), true);🤖 Prompt for AI Agents
In `@Service/PasskeyAuthService.php` around lines 205 - 221, extractSubFromIdToken
currently only base64-decodes the payload and returns 'sub' without signature or
claims checks and lacks proper base64url padding; update extractSubFromIdToken
to (1) perform base64url padding normalization when decoding the middle JWT
part, (2) validate the token signature using the issuer's JWKS (fetch and cache
keys, verify with the key matching the token's kid), and (3) validate standard
claims (iss, aud, exp/iat as appropriate) before returning the 'sub'; retain the
existing fallback of returning null on any validation or parsing failure.
| test('パスキーボタンクリックで認証フローが開始される', async ({ page, context }) => { | ||
| // Virtual Authenticator を設定 | ||
| const cdpSession = await context.newCDPSession(page); | ||
| await cdpSession.send('WebAuthn.enable'); | ||
| const { authenticatorId } = await cdpSession.send('WebAuthn.addVirtualAuthenticator', { | ||
| options: { | ||
| protocol: 'ctap2', | ||
| transport: 'internal', | ||
| hasResidentKey: true, | ||
| hasUserVerification: true, | ||
| isUserVerified: true, | ||
| }, | ||
| }); | ||
|
|
||
| await page.goto(`${ADMIN_URL}/login`); | ||
|
|
||
| const passkeyBtn = page.locator('#ecauth-passkey-login'); | ||
| await passkeyBtn.click(); | ||
|
|
||
| // ボタンが disabled になることを確認（認証中状態） | ||
| await expect(passkeyBtn).toBeDisabled(); | ||
|
|
||
| // クリーンアップ | ||
| await cdpSession.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the test around lines 15-39
find . -name "passkey_auth.spec.ts" -type fRepository: EcAuth/ec-cube4-ecauth
Length of output: 100
🏁 Script executed:
# Look for the test file and read the relevant section
if [ -f "tests/specs/passkey_auth.spec.ts" ]; then
echo "=== File content (lines 1-50) ==="
head -50 tests/specs/passkey_auth.spec.ts | cat -n
fiRepository: EcAuth/ec-cube4-ecauth
Length of output: 1601
🏁 Script executed:
# Search for similar Virtual Authenticator patterns in the test suite
rg "WebAuthn\.(addVirtualAuthenticator|removeVirtualAuthenticator)" --type ts -B 5 -A 5Repository: EcAuth/ec-cube4-ecauth
Length of output: 1326
Virtual Authenticator のクリーンアップを finally で保証してください。
認証フロー実行中（行29-35）に例外が発生すると、行38のクリーンアップが実行されず、次のテストが不安定になります。テストロジック全体を try-finally で囲んで、例外発生時も確実にクリーンアップが走るようにしてください。
修正案
test('パスキーボタンクリックで認証フローが開始される', async ({ page, context }) => {
// Virtual Authenticator を設定
const cdpSession = await context.newCDPSession(page);
await cdpSession.send('WebAuthn.enable');
const { authenticatorId } = await cdpSession.send('WebAuthn.addVirtualAuthenticator', {
options: {
protocol: 'ctap2',
transport: 'internal',
hasResidentKey: true,
hasUserVerification: true,
isUserVerified: true,
},
});
- await page.goto(`${ADMIN_URL}/login`);
-
- const passkeyBtn = page.locator('#ecauth-passkey-login');
- await passkeyBtn.click();
-
- // ボタンが disabled になることを確認（認証中状態）
- await expect(passkeyBtn).toBeDisabled();
-
- // クリーンアップ
- await cdpSession.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId });
+ try {
+ await page.goto(`${ADMIN_URL}/login`);
+
+ const passkeyBtn = page.locator('#ecauth-passkey-login');
+ await passkeyBtn.click();
+
+ // ボタンが disabled になることを確認（認証中状態）
+ await expect(passkeyBtn).toBeDisabled();
+ } finally {
+ // クリーンアップ
+ await cdpSession.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId });
+ }
});🤖 Prompt for AI Agents
In `@tests/specs/passkey_auth.spec.ts` around lines 15 - 39, Wrap the test body
starting from creating the CDP session and adding the virtual authenticator in a
try-finally so the cleanup always runs; move the await
cdpSession.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId }) call
into the finally block and ensure you still await it (and optionally check
authenticatorId is defined) while keeping the existing assertions (e.g.,
passkeyBtn click and expect(passkeyBtn).toBeDisabled()) inside the try.
| test('パスキー管理画面にアクセスできる', async ({ page }) => { | ||
| await page.goto(`${ADMIN_URL}/ecauth/passkey/`); | ||
| await expect(page.locator('text=登録済みパスキー')).toBeVisible(); | ||
| }); | ||
|
|
||
| test('パスキー追加ボタンが表示される', async ({ page }) => { | ||
| await page.goto(`${ADMIN_URL}/ecauth/passkey/`); | ||
| const addBtn = page.locator('#ecauth-passkey-add'); | ||
| await expect(addBtn).toBeVisible(); | ||
| }); | ||
|
|
||
| test('パスキー追加クリックでパスワード確認モーダルが表示される', async ({ page }) => { | ||
| await page.goto(`${ADMIN_URL}/ecauth/passkey/`); | ||
| const addBtn = page.locator('#ecauth-passkey-add'); | ||
| await addBtn.click(); | ||
|
|
||
| const modal = page.locator('#ecauth-password-modal'); | ||
| await expect(modal).toBeVisible(); | ||
|
|
||
| const passwordInput = page.locator('#ecauth-password-input'); | ||
| await expect(passwordInput).toBeVisible(); | ||
| }); | ||
|
|
||
| test('キャンセルボタンでモーダルが閉じる', async ({ page }) => { | ||
| await page.goto(`${ADMIN_URL}/ecauth/passkey/`); | ||
| await page.locator('#ecauth-passkey-add').click(); | ||
|
|
||
| const modal = page.locator('#ecauth-password-modal'); | ||
| await expect(modal).toBeVisible(); | ||
|
|
||
| await page.locator('#ecauth-password-cancel').click(); | ||
| await expect(modal).not.toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for passkey route definitions in PHP and YAML files
rg -n "passkey" --type=php --type=yaml --type=yml -A 3 -B 3Repository: EcAuth/ec-cube4-ecauth
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Search for ecauth_login43 references to understand naming convention
rg -n "ecauth_login43" --type=php --type=yaml --type=yml -A 2 -B 2Repository: EcAuth/ec-cube4-ecauth
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Search for route definitions with ADMIN_URL pattern to understand path structure
rg -n "ecauth.*passkey|admin.*passkey" --type=php --type=yaml --type=yml -iRepository: EcAuth/ec-cube4-ecauth
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Look at the test file to see what ADMIN_URL is set to
cat -n tests/specs/passkey_register.spec.ts | head -20Repository: EcAuth/ec-cube4-ecauth
Length of output: 919
🏁 Script executed:
#!/bin/bash
# Search for passkey route definitions in PHP files
rg -n "passkey" --type=php -A 3 -B 3Repository: EcAuth/ec-cube4-ecauth
Length of output: 18439
🏁 Script executed:
#!/bin/bash
# Search for ecauth_login43 references
rg -n "ecauth_login43" --type=php -A 2 -B 2Repository: EcAuth/ec-cube4-ecauth
Length of output: 6882
🏁 Script executed:
#!/bin/bash
# Look for Route definitions with patterns like ecauth_login
rg -n "Route|ecauth.*login|ecauth.*passkey" --type=php -i | head -50Repository: EcAuth/ec-cube4-ecauth
Length of output: 4446
🏁 Script executed:
#!/bin/bash
# Find all PHP files that might contain route definitions
find . -name "*.php" -type f | grep -E "(route|controller|config)" | head -20Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
パスキー管理画面のパスがプラグイン命名規則と不一致です。
パスキー管理画面のルート定義が他の管理画面と異なります。ConfigController では /%eccube_admin_route%/ecauth_login43/config を使用していますが、PasskeyController では /%eccube_admin_route%/ecauth/passkey/ となっており、バージョン番号 login43 が含まれていません。ルート名は両方とも ecauth_login43_admin_* の命名規則に従っているため、URL パスも統一して /%eccube_admin_route%/ecauth_login43/passkey/ に修正してください。
🤖 Prompt for AI Agents
In `@tests/specs/passkey_register.spec.ts` around lines 17 - 49, Tests in
passkey_register.spec.ts use the old route `/ecauth/passkey/`; update every
navigation URL that uses `${ADMIN_URL}/ecauth/passkey/` to
`${ADMIN_URL}/ecauth_login43/passkey/` so the tests match the plugin naming
convention used by ConfigController (ecauth_login43); keep the rest of the test
code (locators like '#ecauth-passkey-add', '#ecauth-password-modal', etc.)
unchanged.
- composer.json: パッケージ名をEC-CUBEプラグイン命名規約に準拠 (ecauth/ec-cube4-ecauth → ec-cube/ecauthlogin43) - docker-entrypoint.sh: ベースイメージの初期化処理を再実装 - doctrine:database:create --if-not-exists でDB既存時のエラー回避 - eccube:composer:require --from オプションでPCRE JITエラー回避 - プラグインインストール済み判定による冪等性確保 - EcAuthApiClient: Symfony HttpClientからGuzzleHttpに変更 (EC-CUBE 4.3にSymfony\Contracts\HttpClient未搭載のため) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docker-entrypoint.sh`:
- Around line 35-41: パイプで bin/console eccube:plugin:list の失敗が grep
に上書きされているため、bin/console の終了コードを正しく扱うよう修正してください — 具体的にはスクリプト先頭で set -o pipefail
を有効にするか、あるいは bin/console eccube:plugin:list の実行結果を一旦変数/ファイルに保存してその終了ステータスを確認してから
echo "$output" | grep -q EcAuthLogin43 のように grep を実行するように変更し、関連する識別子は
bin/console eccube:plugin:list と grep -q EcAuthLogin43（および docker-entrypoint.sh
の該当 if ブロック）です。
- Around line 1-14: The script mixes relative paths and ${APACHE_DOCUMENT_ROOT}
causing failures; update docker-entrypoint.sh to validate that
APACHE_DOCUMENT_ROOT is set and points to a directory, then cd into
${APACHE_DOCUMENT_ROOT} (or a defined fallback) before running relative
operations (e.g., the composer dumpautoload, chown -R vendor, and any references
to bin/console or .env.dist); if APACHE_DOCUMENT_ROOT is missing or invalid,
print an error and exit with non-zero status so the container fails fast.
| #!/bin/bash | ||
| set -e | ||
|
|
||
| # ベースイメージのエントリポイント（composer install, DB初期化）を実行 | ||
| # ただし Apache 起動は行わない（後でプラグインインストール後に起動） | ||
| if [ ! -d /var/www/html/vendor/bin ]; then | ||
| composer install \ | ||
| --no-scripts \ | ||
| --no-autoloader \ | ||
| --no-plugins \ | ||
| -d ${APACHE_DOCUMENT_ROOT} | ||
| composer dumpautoload -o --apcu | ||
| chown -R www-data: vendor | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for WORKDIR and APACHE_DOCUMENT_ROOT in Docker-related files
rg -n 'WORKDIR|APACHE_DOCUMENT_ROOT' -g 'Dockerfile*' -g 'docker-compose*.yml' -g 'docker-compose*.yaml' -g 'compose*.yml' -g 'compose*.yaml'Repository: EcAuth/ec-cube4-ecauth
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Find all Docker-related files
echo "=== Docker-related files ==="
fd -e Dockerfile -e yml -e yaml | grep -E '(Docker|docker|compose)' | head -20
echo -e "\n=== Listing root level Docker files ==="
ls -la | grep -i docker
echo -e "\n=== Checking if docker-entrypoint.sh exists and its location ==="
find . -name "docker-entrypoint.sh" -type fRepository: EcAuth/ec-cube4-ecauth
Length of output: 554
🏁 Script executed:
#!/bin/bash
echo "=== Dockerfile ==="
cat -n Dockerfile
echo -e "\n=== docker-compose.yml ==="
cat -n docker-compose.yml
echo -e "\n=== docker-entrypoint.sh ==="
cat -n docker-entrypoint.shRepository: EcAuth/ec-cube4-ecauth
Length of output: 3572
作業ディレクトリ前提で相対パスが壊れる可能性
スクリプトが相対パス（bin/console、vendor、.env.dist）と絶対パス（${APACHE_DOCUMENT_ROOT}）を混在させており、起動時の作業ディレクトリが確定していません。docker-compose.yml には APACHE_DOCUMENT_ROOT が定義されていないため、ベースイメージに依存しています。composer dumpautoload（12行目）と chown（13行目）が相対パスで実行される場合、作業ディレクトリが異なるとエラーや誤った処理が発生します。
スクリプト冒頭で環境変数を明示的に検証し、作業ディレクトリを固定してください。
修正案（ディレクトリ固定＋必須化）
#!/bin/bash
set -e
+: "${APACHE_DOCUMENT_ROOT:?APACHE_DOCUMENT_ROOT is required}"
+cd "${APACHE_DOCUMENT_ROOT}"
# ベースイメージのエントリポイント（composer install, DB初期化）を実行
# ただし Apache 起動は行わない（後でプラグインインストール後に起動）
if [ ! -d /var/www/html/vendor/bin ]; then
composer install \
--no-scripts \
--no-autoloader \
--no-plugins \
-d ${APACHE_DOCUMENT_ROOT}
- composer dumpautoload -o --apcu
- chown -R www-data: vendor
+ composer dumpautoload -o --apcu -d "${APACHE_DOCUMENT_ROOT}"
+ chown -R www-data: "${APACHE_DOCUMENT_ROOT}/vendor"
fi📝 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.
| #!/bin/bash | |
| set -e | |
| # ベースイメージのエントリポイント（composer install, DB初期化）を実行 | |
| # ただし Apache 起動は行わない（後でプラグインインストール後に起動） | |
| if [ ! -d /var/www/html/vendor/bin ]; then | |
| composer install \ | |
| --no-scripts \ | |
| --no-autoloader \ | |
| --no-plugins \ | |
| -d ${APACHE_DOCUMENT_ROOT} | |
| composer dumpautoload -o --apcu | |
| chown -R www-data: vendor | |
| fi | |
| #!/bin/bash | |
| set -e | |
| : "${APACHE_DOCUMENT_ROOT:?APACHE_DOCUMENT_ROOT is required}" | |
| cd "${APACHE_DOCUMENT_ROOT}" | |
| # ベースイメージのエントリポイント（composer install, DB初期化）を実行 | |
| # ただし Apache 起動は行わない（後でプラグインインストール後に起動） | |
| if [ ! -d /var/www/html/vendor/bin ]; then | |
| composer install \ | |
| --no-scripts \ | |
| --no-autoloader \ | |
| --no-plugins \ | |
| -d ${APACHE_DOCUMENT_ROOT} | |
| composer dumpautoload -o --apcu -d "${APACHE_DOCUMENT_ROOT}" | |
| chown -R www-data: "${APACHE_DOCUMENT_ROOT}/vendor" | |
| fi |
🤖 Prompt for AI Agents
In `@docker-entrypoint.sh` around lines 1 - 14, The script mixes relative paths
and ${APACHE_DOCUMENT_ROOT} causing failures; update docker-entrypoint.sh to
validate that APACHE_DOCUMENT_ROOT is set and points to a directory, then cd
into ${APACHE_DOCUMENT_ROOT} (or a defined fallback) before running relative
operations (e.g., the composer dumpautoload, chown -R vendor, and any references
to bin/console or .env.dist); if APACHE_DOCUMENT_ROOT is missing or invalid,
print an error and exit with non-zero status so the container fails fast.
| if ! bin/console eccube:plugin:list 2>/dev/null | grep -q EcAuthLogin43; then | ||
| echo "Installing EcAuthLogin43 plugin..." | ||
| bin/console eccube:composer:require ec-cube/ecauthlogin43 --from=/plugin | ||
| bin/console eccube:plugin:enable --code=EcAuthLogin43 | ||
| bin/console cache:clear --no-warmup | ||
| bin/console cache:warmup --no-optional-warmers | ||
| echo "EcAuthLogin43 plugin installed and enabled." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "docker-entrypoint.sh" -type fRepository: EcAuth/ec-cube4-ecauth
Length of output: 88
🏁 Script executed:
cat -n ./docker-entrypoint.sh | head -50Repository: EcAuth/ec-cube4-ecauth
Length of output: 2057
パイプラインが bin/console の失敗を隠している
Line 35 のパイプラインは bin/console eccube:plugin:list が失敗した場合、その終了コードが grep の終了コードで上書きされます。bin/console が失敗して出力が空になると、grep -q は 1 を返し、それが ! で反転されて 0 (真) となり、エラーが隠されたまま処理が続行されます。これは意図しない動作です。
修正案
# プラグインがまだインストールされていなければインストール
-if ! bin/console eccube:plugin:list 2>/dev/null | grep -q EcAuthLogin43; then
+plugin_list="$(bin/console eccube:plugin:list 2>/dev/null)" || {
+ echo "eccube:plugin:list failed" >&2
+ exit 1
+}
+if ! echo "$plugin_list" | grep -q EcAuthLogin43; then
echo "Installing EcAuthLogin43 plugin..."📝 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.
| if ! bin/console eccube:plugin:list 2>/dev/null | grep -q EcAuthLogin43; then | |
| echo "Installing EcAuthLogin43 plugin..." | |
| bin/console eccube:composer:require ec-cube/ecauthlogin43 --from=/plugin | |
| bin/console eccube:plugin:enable --code=EcAuthLogin43 | |
| bin/console cache:clear --no-warmup | |
| bin/console cache:warmup --no-optional-warmers | |
| echo "EcAuthLogin43 plugin installed and enabled." | |
| # プラグインがまだインストールされていなければインストール | |
| plugin_list="$(bin/console eccube:plugin:list 2>/dev/null)" || { | |
| echo "eccube:plugin:list failed" >&2 | |
| exit 1 | |
| } | |
| if ! echo "$plugin_list" | grep -q EcAuthLogin43; then | |
| echo "Installing EcAuthLogin43 plugin..." | |
| bin/console eccube:composer:require ec-cube/ecauthlogin43 --from=/plugin | |
| bin/console eccube:plugin:enable --code=EcAuthLogin43 | |
| bin/console cache:clear --no-warmup | |
| bin/console cache:warmup --no-optional-warmers | |
| echo "EcAuthLogin43 plugin installed and enabled." |
🤖 Prompt for AI Agents
In `@docker-entrypoint.sh` around lines 35 - 41, パイプで bin/console
eccube:plugin:list の失敗が grep に上書きされているため、bin/console の終了コードを正しく扱うよう修正してください —
具体的にはスクリプト先頭で set -o pipefail を有効にするか、あるいは bin/console eccube:plugin:list
の実行結果を一旦変数/ファイルに保存してその終了ステータスを確認してから echo "$output" | grep -q EcAuthLogin43 のように
grep を実行するように変更し、関連する識別子は bin/console eccube:plugin:list と grep -q
EcAuthLogin43（および docker-entrypoint.sh の該当 if ブロック）です。
- EcAuth API レスポンスの options をフラット化して返すように修正
(session_id + options 構造から options のみを返す)
- WebAuthn レスポンスに clientExtensionResults と transports を追加
(Fido2.NetLib 4.0.0 の必須フィールド)
- PHP の json_decode が空オブジェクト {} を空配列 [] に変換する問題を修正
(clientExtensionResults を stdClass に変換)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- login_frame.twig は plugin_snippets を描画しないため addSnippet() から setSource() に変更 - CSRF トークンを ecauth_passkey ID で生成するように修正 - アセットパスを EcAuthLogin43/assets/js/webauthn.js に修正 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
OAuth2 仕様に従い /token エンドポイントは application/x-www-form-urlencoded を期待するが、JSON で送信していたため全フィールドが空として解釈されていた。 postForm() メソッドを追加し、Guzzle の form_params で送信するように修正。 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- HTTPS ポートを 4430 から 8081 に変更 (EcAuth の許可 origin に対応) - docker-entrypoint.sh でプラグインアセットのシンボリックリンクを自動作成 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docker-compose.yml`:
- Around line 17-18: The docker-compose.yml currently hardcodes DATABASE_URL
(including plaintext credentials) and DATABASE_SERVER_VERSION; update the
compose file to read these values from environment variables (e.g., use env_file
or ${DATABASE_URL} and ${DATABASE_SERVER_VERSION} placeholders) and remove
embedded credentials, and also update the other occurrences mentioned (lines
29-33) to the same pattern so all database connection settings come from .env
rather than being in-repo plaintext.
In `@Service/EcAuthApiClient.php`:
- Around line 216-255: postForm() creates a Guzzle Client and calls
$client->request(...) without any timeout settings, so add request timeouts
(e.g. 'timeout' and 'connect_timeout') to the options passed to $client->request
(alongside 'form_params', 'headers', 'http_errors') and also update the other
request() method to include the same timeout options; ensure both methods use
sensible numeric values and keep 'http_errors' => false behavior intact so
requests won't hang on network/server stalls.
- Around line 173-192: getClientId(), getClientSecret(), and getBaseUrl()
silently return empty strings when $this->configRepository->get() is null, which
can lead to invalid API calls; change these methods (getClientId,
getClientSecret, getBaseUrl) to detect missing configuration from
configRepository->get() and throw a clear, specific exception (e.g.,
InvalidArgumentException or a custom ConfigMissingException) with a message
indicating which value is missing (client id, client secret, or base URL) so
callers can fail fast and logs show the root cause.
🧹 Nitpick comments (5)
EcAuthLoginEvent.php (1)
10-28: LGTM! 実装は適切で、コメントも明確です。
login_frame.twigがplugin_snippetsを描画しないという理由でsetSource()を使用する判断は妥当です。テンプレート注入のアプローチは EC-CUBE プラグイン開発のベストプラクティスに沿っています。
（任意）戻り値型の追加を検討してください。
PHP 7.1+ の戻り値型宣言を追加すると、型安全性と可読性が向上します。PHPStan Level 6 の要件に対応する場合は特に有用です。
♻️ 戻り値型の追加案
/** - * `@return` array + * `@return` array<string, string> */ - public static function getSubscribedEvents() + public static function getSubscribedEvents(): array { return [ '@admin/login.twig' => 'onAdminLoginTwig', ]; } - public function onAdminLoginTwig(TemplateEvent $event) + public function onAdminLoginTwig(TemplateEvent $event): void {Resource/assets/js/webauthn.js (1)
48-121: fetch リクエストにタイムアウトが設定されていません
navigator.credentials.getにはタイムアウト（line 70）が設定されていますが、fetchリクエストにはタイムアウトがありません。ネットワーク障害時にリクエストが無限にハングする可能性があります。🔧 AbortController を使用したタイムアウト例
// fetch にタイムアウトを追加する例 function fetchWithTimeout(url, options, timeout = 30000) { var controller = new AbortController(); var timeoutId = setTimeout(function() { controller.abort(); }, timeout); options.signal = controller.signal; return fetch(url, options).finally(function() { clearTimeout(timeoutId); }); }Resource/template/admin/passkey_list.twig (2)
51-52: CSRF トークンの取得時に null チェックが必要です
csrfTokenがnullの場合、getAttribute('content')で例外が発生する可能性があります。現在の実装でも動作しますが、明示的な null チェックを追加するとより安全です。🔧 修正案
var csrfToken = document.querySelector('meta[name="csrf-token"]'); - var token = csrfToken ? csrfToken.getAttribute('content') : ''; + if (!csrfToken) { + console.error('CSRF token meta tag not found'); + return; + } + var token = csrfToken.getAttribute('content') || '';
206-206: CSRF メタタグの配置位置を<head>セクションへ移動することを検討現在
{% block main %}の末尾に配置されていますが、通常メタタグは<head>内（{% block meta_tags %}など）に配置します。機能的には問題ありませんが、HTML の慣習に合わせることをお勧めします。Service/EcAuthApiClient.php (1)
21-27: Guzzle Client をコンストラクタでインジェクションすることを検討現在、各リクエストで
new Client()を生成しています（lines 219, 263）。これは非効率であり、テストも困難になります。🔧 修正案
+use GuzzleHttp\ClientInterface; + class EcAuthApiClient { /** * `@var` ConfigRepository */ private $configRepository; /** * `@var` LoggerInterface */ private $logger; + /** + * `@var` ClientInterface + */ + private $httpClient; + public function __construct( ConfigRepository $configRepository, - LoggerInterface $logger + LoggerInterface $logger, + ClientInterface $httpClient = null ) { $this->configRepository = $configRepository; $this->logger = $logger; + $this->httpClient = $httpClient ?? new Client(); }そして
request()とpostForm()で$this->httpClientを使用します。
| DATABASE_URL: "postgresql://eccube:password@postgres:5432/eccube_db" | ||
| DATABASE_SERVER_VERSION: 15 |
There was a problem hiding this comment.
認証情報の直書きを避けて環境変数/.env に移行してください。
リポジトリ内に平文パスワードが残るため、.env 参照へ切り替える方が安全です。
🔧 置き換え例（.env 参照）
environment:
APP_ENV: dev
- DATABASE_URL: "postgresql://eccube:password@postgres:5432/eccube_db"
+ DATABASE_URL: "postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@postgres:5432/${POSTGRES_DB}"
DATABASE_SERVER_VERSION: 15 environment:
TZ: Asia/Tokyo
- POSTGRES_DB: eccube_db
- POSTGRES_USER: eccube
- POSTGRES_PASSWORD: password
+ POSTGRES_DB: ${POSTGRES_DB:-eccube_db}
+ POSTGRES_USER: ${POSTGRES_USER:-eccube}
+ POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?set in .env}Also applies to: 29-33
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 17-18: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
In `@docker-compose.yml` around lines 17 - 18, The docker-compose.yml currently
hardcodes DATABASE_URL (including plaintext credentials) and
DATABASE_SERVER_VERSION; update the compose file to read these values from
environment variables (e.g., use env_file or ${DATABASE_URL} and
${DATABASE_SERVER_VERSION} placeholders) and remove embedded credentials, and
also update the other occurrences mentioned (lines 29-33) to the same pattern so
all database connection settings come from .env rather than being in-repo
plaintext.
| private function getClientId(): string | ||
| { | ||
| $Config = $this->configRepository->get(); | ||
|
|
||
| return $Config ? $Config->getClientId() ?? '' : ''; | ||
| } | ||
|
|
||
| private function getClientSecret(): string | ||
| { | ||
| $Config = $this->configRepository->get(); | ||
|
|
||
| return $Config ? $Config->getClientSecret() ?? '' : ''; | ||
| } | ||
|
|
||
| private function getBaseUrl(): string | ||
| { | ||
| $Config = $this->configRepository->get(); | ||
|
|
||
| return $Config ? rtrim($Config->getEcauthBaseUrl() ?? '', '/') : ''; | ||
| } |
There was a problem hiding this comment.
設定が存在しない場合のエラーハンドリングを強化すべき
getClientId()、getClientSecret()、getBaseUrl() は設定が存在しない場合に空文字列を返します。これにより、API 呼び出しが無効な URL やクレデンシャルで実行され、デバッグが困難なエラーが発生する可能性があります。
🛡️ 修正案: 例外をスローする
private function getClientId(): string
{
$Config = $this->configRepository->get();
+ if (!$Config || !$Config->getClientId()) {
+ throw new \RuntimeException('EcAuth client_id is not configured');
+ }
- return $Config ? $Config->getClientId() ?? '' : '';
+ return $Config->getClientId();
}🤖 Prompt for AI Agents
In `@Service/EcAuthApiClient.php` around lines 173 - 192, getClientId(),
getClientSecret(), and getBaseUrl() silently return empty strings when
$this->configRepository->get() is null, which can lead to invalid API calls;
change these methods (getClientId, getClientSecret, getBaseUrl) to detect
missing configuration from configRepository->get() and throw a clear, specific
exception (e.g., InvalidArgumentException or a custom ConfigMissingException)
with a message indicating which value is missing (client id, client secret, or
base URL) so callers can fail fast and logs show the root cause.
| private function postForm(string $path, array $params): array | ||
| { | ||
| $url = $this->getBaseUrl().$path; | ||
| $client = new Client(); | ||
|
|
||
| try { | ||
| $response = $client->request('POST', $url, [ | ||
| 'form_params' => $params, | ||
| 'headers' => [ | ||
| 'Accept' => 'application/json', | ||
| ], | ||
| 'http_errors' => false, | ||
| ]); | ||
| $statusCode = $response->getStatusCode(); | ||
| $content = json_decode($response->getBody()->getContents(), true) ?? []; | ||
|
|
||
| if ($statusCode >= 400) { | ||
| $this->logger->error('EcAuth API error', [ | ||
| 'status' => $statusCode, | ||
| 'path' => $path, | ||
| 'response' => $content, | ||
| ]); | ||
| } | ||
|
|
||
| return [ | ||
| 'status' => $statusCode, | ||
| 'data' => $content, | ||
| ]; | ||
| } catch (\Exception $e) { | ||
| $this->logger->error('EcAuth API request failed', [ | ||
| 'path' => $path, | ||
| 'error' => $e->getMessage(), | ||
| ]); | ||
|
|
||
| return [ | ||
| 'status' => 500, | ||
| 'data' => ['error' => $e->getMessage()], | ||
| ]; | ||
| } | ||
| } |
There was a problem hiding this comment.
HTTP リクエストにタイムアウトを設定してください
postForm() と request() の両方でタイムアウトが設定されていません。ネットワーク障害やサーバー無応答時にリクエストが無限にハングする可能性があります。
🔧 修正案
$response = $client->request('POST', $url, [
'form_params' => $params,
'headers' => [
'Accept' => 'application/json',
],
'http_errors' => false,
+ 'timeout' => 30,
+ 'connect_timeout' => 10,
]);同様に request() メソッドにも追加してください。
🤖 Prompt for AI Agents
In `@Service/EcAuthApiClient.php` around lines 216 - 255, postForm() creates a
Guzzle Client and calls $client->request(...) without any timeout settings, so
add request timeouts (e.g. 'timeout' and 'connect_timeout') to the options
passed to $client->request (alongside 'form_params', 'headers', 'http_errors')
and also update the other request() method to include the same timeout options;
ensure both methods use sensible numeric values and keep 'http_errors' => false
behavior intact so requests won't hang on network/server stalls.
EC-CUBE 本体の services.yaml が Tests (大文字T) のみを exclude 対象としているため、 tests (小文字t) がサービスコンテナのスキャン対象となり RecursiveDirectoryIterator が シンボリックリンク経由でディレクトリを開けずエラーとなっていた。 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
external_id 必須に変更 |
1 participant
Summary
実装内容
セキュリティ対策
hash_equals()+ 使い捨て削除eccube.rate_limiterで認証API保護Test plan
docker compose up -d --buildで EC-CUBE 4.3 + プラグイン起動確認Closes #1
🤖 Generated with Claude Code
Summary by CodeRabbit
リリースノート
新機能
ドキュメント
テスト