Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ログイン履歴機能のマイグレーションを追加 #5010

Merged
merged 5 commits into from
Aug 30, 2021

Conversation

chihiro-adachi
Copy link
Contributor

@chihiro-adachi chihiro-adachi commented Apr 12, 2021

概要(Overview・Refs Issue)

#4978 ログイン履歴機能のマイグレーションを追加

方針(Policy)

管理画面セキュリティ対策プラグインから移行できるよう、プラグインが保持しているデータのマイグレーションを行う

以下の手順で、導入済みのプラグインから本体機能へマイグレーションすることができる。

// featureブランチをマージ
git merge origin/4.1-feature

// スキーマ更新、マイグレーション実行
bin/console doctrine:schema:update --dump-sql -f
bin/console doctrine:migration:migrate

// プラグインの無効化
bin/console eccube:plugin:disable --code=AdminSecurity4

実装に関する補足(Appendix)

以下のテーブル・ファイルへ移行する

プラグイン 本体 備考
plg_admin_record_config .env ECCUBE_ADMIN_DENY_HOSTSに出力
plg_admin_record dtb_login_history  

テスト(Test)

  • マイグレーションを実行し、データ移行されることを確認(PostgreSQL)

相談(Discussion)

マイナーバージョン互換性保持のための制限事項チェックリスト

  • 既存機能の仕様変更
  • フックポイントの呼び出しタイミングの変更
  • フックポイントのパラメータの削除・データ型の変更
  • twigファイルに渡しているパラメータの削除・データ型の変更
  • Serviceクラスの公開関数の、引数の削除・データ型の変更
  • 入出力ファイル(CSVなど)のフォーマット変更

レビュワー確認項目

  • 動作確認
  • コードレビュー
  • E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
  • 互換性が保持されているか
  • セキュリティ上の問題がないか

@chihiro-adachi chihiro-adachi changed the title プラグインからのマイグレーションを追加 ログイン履歴機能のマイグレーションを追加 Apr 12, 2021
@okazy
Copy link
Contributor

okazy commented Apr 15, 2021

up(プラグイン -> 本体機能)のみ実装
down(本体機能 -> プラグイン)は「schema:update -> migration」の流れなので工夫が必要
手順を工夫すれば可能なので、念の為 down も実装する。

@chihiro-adachi
Copy link
Contributor Author

@okazy
down実装しました。
.envのECCUBE_ADMIN_DENY_HOSTS に関しては残っていても問題は発生しないと思われるため対応していません。

@matsuoshi
Copy link
Contributor

@chihiro-adachi もとのプラグイン側で拒否IPアドレスを複数、改行区切りで入力していた場合、マイグレーション実行後の .env ファイルは以下のようになりました

ECCUBE_ADMIN_DENY_HOSTS='["1.1.1.1\r\n2.2.2.2"]'

結果、管理画面アクセス時にエラーが起きるようです

Invalid JSON in env var "ECCUBE_ADMIN_DENY_HOSTS": Syntax error

なお、 .env の内容が以下のようであればエラーは起きませんでした

ECCUBE_ADMIN_DENY_HOSTS='["1.1.1.1\\r\\n2.2.2.2"]'
# または
ECCUBE_ADMIN_DENY_HOSTS='["1.1.1.1","2.2.2.2"]'

@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2021

Codecov Report

Merging #5010 (b057295) into 4.1 (49ec600) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              4.1    #5010      +/-   ##
==========================================
- Coverage   68.16%   68.15%   -0.01%     
==========================================
  Files         457      457              
  Lines       24980    24980              
==========================================
- Hits        17027    17026       -1     
- Misses       7953     7954       +1     
Flag Coverage Δ
tests 68.15% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/Eccube/Entity/TaxRule.php 85.18% <0.00%> (-1.24%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49ec600...b057295. Read the comment docs.

@chihiro-adachi
Copy link
Contributor Author

@matsuoshi
ありがとうございます。修正しました。

@matsuoshi
Copy link
Contributor

@chihiro-adachi ありがとうございます! 動作確認しまして、問題ないと思います。

@okazy okazy added the enhancement 機能追加 label Apr 26, 2021
@okazy okazy added this to the 4.1 milestone Apr 26, 2021
@okazy okazy changed the base branch from 4.1-feature to 4.1 August 30, 2021 02:33
@okazy okazy changed the base branch from 4.1 to 4.1-feature August 30, 2021 02:34
@okazy
Copy link
Contributor

okazy commented Aug 30, 2021

機能自体の検証手順

git clone https://github.com/EC-CUBE/ec-cube.git ec-cube_migration
cd ec-cube_migration
php ../composer.phar install
vi .env
bin/console eccube:install --no-interaction
bin/console s:run --env=dev

管理画面から認証キーの設定
ログイン履歴プラグインのインストール、有効化

ログイン履歴追加

移行前のデータ

image

plg_admin_record

1,,testlogin,0,127.0.0.1,2021-08-30 02:47:30 +00:00,2021-08-30 02:47:30 +00:00
2,1,admin,0,127.0.0.1,2021-08-30 02:47:42 +00:00,2021-08-30 02:47:42 +00:00
3,1,admin,1,127.0.0.1,2021-08-30 02:47:45 +00:00,2021-08-30 02:47:45 +00:00

プラグイン一覧からプラグインの無効化
EC-CUBE をアップデート

git checkout composer.json composer.lock
hub checkout https://github.com/EC-CUBE/ec-cube/pull/5010
php ../composer.phar install --no-plugins --no-scripts
bin/console cache:clear --no-warmup
bin/console eccube:composer:require-already-installed
bin/console doctrine:schema:update --force --dump-sql
bin/console doctrine:migrations:migrate
bin/console s:run --env=dev

image

dtb_login_history

1,0,,testlogin,127.0.0.1,2021-08-30 02:47:30 +00:00,2021-08-30 02:47:30 +00:00,loginhistory
2,0,1,admin,127.0.0.1,2021-08-30 02:47:42 +00:00,2021-08-30 02:47:42 +00:00,loginhistory
3,1,1,admin,127.0.0.1,2021-08-30 02:47:45 +00:00,2021-08-30 02:47:45 +00:00,loginhistory

4.0.6-p1 -> 4.1 の検証手順

git clone https://github.com/EC-CUBE/ec-cube.git ec-cube_migration2
cd ec-cube_migration2
git checkout refs/tags/4.0.6-p1 -b 4.0.6-p1
php ../composer.phar install
vi .env
bin/console eccube:install --no-interaction
bin/console s:run --env=dev

管理画面から認証キーの設定
ログイン履歴プラグインのインストール、有効化

ログイン履歴追加

移行前のデータ

image

image

plg_admin_record

1,1,admin,1,127.0.0.1,2021-08-30 03:10:32 +00:00,2021-08-30 03:10:32 +00:00
2,1,admin,0,127.0.0.1,2021-08-30 03:10:41 +00:00,2021-08-30 03:10:41 +00:00
3,,hogehoge,0,127.0.0.1,2021-08-30 03:10:48 +00:00,2021-08-30 03:10:48 +00:00

プラグイン一覧からプラグインの無効化

EC-CUBE をアップデート

git checkout composer.json composer.lock
hub checkout https://github.com/EC-CUBE/ec-cube/pull/5010
git merge origin/4.1
composer install --no-plugins --no-scripts
bin/console cache:clear --no-warmup
vi .env
# ECCUBE_PACKAGE_API_URL=https://package-api-c2.ec-cube.net
## エラー
bin/console eccube:composer:require-already-installed

bin/console doctrine:schema:update --force --dump-sql
bin/console doctrine:migrations:migrate
bin/console s:run --env=dev

image

dtb_login_history

1,1,1,admin,127.0.0.1,2021-08-30 03:10:32 +00:00,2021-08-30 03:10:32 +00:00,loginhistory
2,0,1,admin,127.0.0.1,2021-08-30 03:10:41 +00:00,2021-08-30 03:10:41 +00:00,loginhistory
3,0,,hogehoge,127.0.0.1,2021-08-30 03:10:48 +00:00,2021-08-30 03:10:48 +00:00,loginhistory
4,1,1,admin,127.0.0.1,2021-08-30 03:24:40 +00:00,2021-08-30 03:24:40 +00:00,loginhistory

4.0.6-p1 -> 4.1 の検証手順(拒否リスト、down)

git clone https://github.com/EC-CUBE/ec-cube.git ec-cube_migration2
cd ec-cube_migration2
git checkout refs/tags/4.0.6-p1 -b 4.0.6-p1
php ../composer.phar install
vi .env
bin/console eccube:install --no-interaction
bin/console s:run --env=dev

管理画面から認証キーの設定
ログイン履歴プラグインのインストール、有効化

ログイン履歴追加

移行前のデータ

image

プラグイン一覧からプラグインの無効化

EC-CUBE をアップデート

git checkout composer.json composer.lock
hub checkout https://github.com/EC-CUBE/ec-cube/pull/5010
php ../composer.phar install --no-plugins --no-scripts
bin/console cache:clear --no-warmup

bin/console doctrine:schema:update --force --dump-sql
bin/console doctrine:migrations:migrate
bin/console s:run --env=dev

image

ECCUBE_ADMIN_DENY_HOSTS='["127.0.0.0","192.0.0.0"]'

image

bin/console doctrine:migrations:execute 20210412073123 --down

image

public function down(Schema $schema): void
{
if ($schema->hasTable('dtb_login_history')) {
$this->addSql('DROP TABLE dtb_login_history');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chihiro-adachi
DROP TABLE してしまうと 500 エラーが発生してしまいます。
migration の役割ではなく、 schema:update の役割かと思います。
実行したかったのは下記ではないでしょうか?

Suggested change
$this->addSql('DROP TABLE dtb_login_history');
$this->addSql('DELETE FROM dtb_login_history');

image

@chihiro-adachi chihiro-adachi changed the base branch from 4.1-feature to 4.1 August 30, 2021 06:10
@chihiro-adachi
Copy link
Contributor Author

@okazy
ありがとうございます。修正しました。

※少々古くなっていたので、rebaseして4.1ブランチへPR先を付け替えています。

@okazy
Copy link
Contributor

okazy commented Aug 30, 2021

IP拒否リストのマージもテストしました。

マージ前プラグイン

image

マージ前本体

image

マージ後

image

Copy link
Contributor

@okazy okazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

動作、ソースコード共に問題ありませんでした!

@okazy okazy merged commit d6c45a4 into EC-CUBE:4.1 Aug 30, 2021
@okazy
Copy link
Contributor

okazy commented Aug 30, 2021

ありがとうございます!マージしました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants