OpenPNE 3

現時点の master (bfc73921817b0e82db93 時点) で再現することができませんでした。

修正内容について、

OpenPNE3.6 以降の場合は opSecurityUser::setIsSNSMember(false) がよばれた場合、SNSMember クレデンシャルを削除するのではなく、opSecurityUser::setAuthenticated(false) を呼ぶのが適切ではないかと考える。

クレデンシャルの削除だけよりも、 sfBasicSecurityUser::setAuthenticated(false) を呼ぶ方が適切であることは間違いないですが、もう一歩踏み込んで opSecurityUser::logout() を呼ぶようにするのがより適切な修正方法だと思います。opSecurityUser::logout() には OpenPNE で拡張している Remember Login Cookie の削除や member_id を保存している opSecurityUser namespace のセッションデータの削除の処理が含まれているため、こちらを呼ぶことでより確実に非ログイン状態と同じ状態にすることができます。

ただし、単純に setIsSNSMember() 内で logout() してしまうと仮登録中のメンバーにも影響が出てしまうため、 initializeCredentials() の処理を書き換える必要があります。

opSecurityUser の具体的な修正案をパッチにしたのでこのコメントに添付します。

修正の差分を小さくするためにメソッド名は既存のものをそのまま使っていますが、initializeCredentials() はクレデンシャルの初期化処理以外の役割も担っているため（元からそうでしたが）、メソッド名を変更したほうがよいかもしれません。（そもそも SNSMember クレデンシャルを付与すること自体が OpenPNE 3.6 以降は for BC 扱いになっています）

また、関連チケットのコメント http://redmine.openpne.jp/issues/1944#note-16 で本チケットでの修正内容について触れていますが、

	session	member	member.is_active	member.is_login_rejected	備考
1	×	-	-	-	非ログイン状態
2	○	×	-	-	退会済み、招待からの仮登録中に招待削除された
3	○	○	0	0	仮登録中
4	○	○	1	0	ログイン中
5	○	○	1	1	ログイン停止中

（中略）

• 2, 5 の場合については、 opSecurityUser のインスタンス初期化時にログアウト処理を行うようにすれば 1 と同じ状態にすることが可能である。
  • この対応については、 #1182 にて先に同様の対応がなされているため本チケットでは扱わないこととする。

この表のうち、本チケットで元々対象としている「SNS強制退会直後」の状態は 2 に該当します。

しかし、実際に 982b00d8 で修正されている箇所は 5 の場合についても実行される箇所ですので、本チケットで 5 のログイン停止中の問題も同時に扱ったほうがよいと思います（テスト項目の追加などが必要になります）。

修正内容¶

• http://redmine.openpne.jp/issues/1182#note-16 での指摘でのバッチを適用しました。
• opSecurityUser::initializeCredentials() を抽象的な名称 opSecurityUser::initializeUserStatus() に変更しました。

Member::isOnBlacklist() について¶

このバッチ適用時の確認として、Member::isOnBlacklist() ブラックリストについては旧 opSecurityUser::initializeCredentials() では対処されておらず、ログイン時のみログインさせない処理となっていることに問題がないか判断ができませんでした。

ブラックリストの機能の意味を考える必要があり、あるべき姿によっては修正が必要と考えます。（別チケット扱いでよいと考えます）

1. (携帯、PC) のログイン処理時のみ、ログインさせない機能としての位置づけ「現状の状態」
2. (携帯) のログイン処理時のみ、ログインさせない機能としての位置づけ
3. (携帯、PC) のログインしているユーザに対しても、ログインさせない機能としての位置づけ
4. (携帯) のログインしているユーザに対しても、ログインさせない機能としての位置づけ

opAnonymousMember::getId() の返り値について¶

• http://redmine.openpne.jp/issues/1944#note-16 で扱う範囲になると思われますが、opAnonymousMember::getId() が 0 を返しているので NULL を返す必要があると思います

default/secure のログインページへの forward 処理について¶

• http://redmine.openpne.jp/issues/1182#note-17 について クレデンシャルの指定を AND と OR の組み合わせで [A, B, [SNSMember, C], D] とした場合に SNSMember と C のみのクレデンシャルがない場合、ログインページではなく、default/secure アクションのページが表示されたままとなる。しかしながら、setIsSNSMember(true) のときは SNSMember クレデンシャルが for BC として付与されるため、また false の場合はそもそも認証状態から外れるため、SNSMember クレデンシャルがない状態で default/secure に forward されることは考えなくてもよいと考える

レビューしました。 OK です。

• このチケットに紐付くコミットが master ブランチに取り込まれていることを確認しました。
• 前半のコミット 982b00d8, 96765c9b, ef7d864d
  • 携帯版にも default/secure のページを用意するという修正ですが、修正内容は適切だと思います。
• 後半のコミット decc4e2d, 7cde87c9
  • 強制退会させられたり、ログイン停止中にされたときに、「そのメンバーがログアウトしない間は操作ができていた」という問題を修正できていることを確認しました。条件式も適切だと思います。

note-19 で、ブラックリストの仕様について、ブラックリストに登録されたときには、「そのメンバーがログアウトしない間は操作ができない」という問題が残ったままになっていることが指摘されています。関連する話であるため併せて対応することも考えられますが、「ブラックリストの仕様が明確でないこと」と「ブラックリストの件を併せて修正することが必須ではないこと」を理由に、このチケットでは扱わなくてよいと判断します。これについては別チケットで対応すればよいと思います。

ログイン中に強制退会させられたメンバーが、そのまま操作を続けた場合に Credentials Required 画面が表示されてしまう問題を解消し、 #1944 の修正と並行して、ログイン停止中にされた場合も含めて、続けて操作ができないようにログアウト処理を追加する修正を行うチケットとして扱うことにします。

テストOKです。