Bug(バグ) #3416
自動ログインCookieを有効にしてログインしているブラウザで新規登録を実行すると member/registerInput で404エラーとなる
0%
Description
Overview (現象)¶
- 自動ログインを有効にして sns@example.com にログインする
- 友人を招待する (/invite) 画面から招待メールを送信する
- 届いたメールに書かれている招待 URL (/member/register) を開く
- 「プロフィール入力ページへ」(/member/registerInput) ボタンをクリックする
- 404 エラーが表示される
Causes (原因)¶
#1100 では新規登録時にログイン中のセッションがあった場合に削除する処理が追加されているが、自動ログインが有効な場合が考慮されていない。そのため member/register までは正常に表示されるが、次の画面に遷移したところで自動ログイン Cookie によりログイン状態となってしまう。
Way to fix (修正内容)¶
member/register アクション内に自動ログイン Cookie を失効させる処理を追加する
Related issues
Associated revisions
revoke automatic login cookie before registration (fixes #3416)
use empty string to clear member cache (fixes #3416)
revoke automatic login cookie on registration pages using register_token (fixes #3416)
History
#1 Updated by Youichi Kimura about 11 years ago
- Description updated (diff)
#2 Updated by Youichi Kimura about 11 years ago
- Status changed from Accepted(着手) to Pending Review(レビュー待ち)
- % Done changed from 0 to 50
更新履歴 2cac338f0edd68b57d534413e54d4b4380237a83 で適用されました。
#3 Updated by Mutsumi Imamura almost 11 years ago
- 3.6 で発生するか changed from Unknown (未調査) to Yes (はい)
- 3.8 で発生するか changed from Unknown (未調査) to Yes (はい)
#4 Updated by Yuya Watanabe almost 11 years ago
- File auto_login.png added
- Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)
フィードバック内容¶
$this->serializedMember の上書きに関して, 84 行目のように if の条件式で false になればよいですが,既存の実装ではキャッシュが空であることを空文字列で表しているようであり文字列のみを扱えば良い状態ですがここで扱われるべきデータに null が追加されるのが気になるため空文字列で初期化してください.
lib/user/opSecurityUser.class.php 84: if ($this->serializedMember)
lib/user/opSecurityUser.class.php 23: $serializedMember = ''; 42: $this->serializedMember = ''; 99: $this->serializedMember = serialize($member);
レビューメモ¶
opSecurityUser#setMemberId() が呼ばれる段階
- sfContext の初期化時
- executeRegister() および executeRegisterInput() 実行時
opSecurityUser#getMember() で opSecurityUser にメンバ情報がメモリにキャッシュされる. sfContext 初期化時には opSecurityUser がインスタンス化される段階で setMemberId() された上で getMember() を呼んでいるためこの段階でキャッシュされる. setMemberId() でこのキャッシュを削除しない状態だと getMember() は sfContext 初期化時にキャッシュされたメンバが取得されるため(また,このメンバは is_remember_me の Cookie があることから is_active=1 であるはずであり opAuthAdapterMailAddress の) isRegisterBegin() が false となり 404 に遷移する.
期待される内容としては
- opSecurityUser の getMember() が現在のメンバIDに対応するメンバ情報を取得する
- セッションの member_id が現在のメンバIDを表している
という前提だと問題としては opSecurityUser から取得されるメンバ ID がキャッシュの ID とセッションにある ID で一致しないということであることになる.
対応方法としては下記のものが考えられる.- getMember() を行う際にメンバ情報をキャッシュしない
- sfContext の初期化時に正しく setMemberId() を行うようにする
- executeRegister() および executeRegisterInput() 実行時に呼ばれる setMemberId() でキャッシュを削除する
この内 getMember() でキャッシュしないようにすることは getMember() の呼ばれる頻度から正しい対応とは言えない.また setMemberId() でキャッシュである serialiedMember を上書きすることが 3 番目の対応となり, executeRegister() で logout() に変更することが executeRegisterInput() 時における 2 番目の対応ということになるはず.setMemberId() を行うかは URL やパラメータなどによって決定されるため sfContext の初期化時点ではセットされる member_id が正しく決定できないと思うため修正内容としては妥当ではあるがリクエスト時に一瞬でも以前のメンバでログイン状態となるところは気になる.(ユーザが明示的な操作を行わない限りはメンバのステートは次のリクエストの sfContext 初期化時までを含むという考え方を持ってないと気になる)
参考¶
sfContext 初期化時における setMemberId までの流れ
getRememberedMemberId() の中で Cookie を見て対応する member_id を DB から取得してくる. 得られた member_id を帰って来た initializeUserStatus() でセッションに保存する.
#5 Updated by Yuya Watanabe almost 11 years ago
- File autologin.png View added
#6 Updated by Yuya Watanabe almost 11 years ago
- File deleted (
auto_login.png)
#7 Updated by Youichi Kimura almost 11 years ago
- Status changed from Rejected(差し戻し) to Pending Review(レビュー待ち)
更新履歴 eb5fe39f5619e7bcac973ca61bd32a94bfa28090 で適用されました。
#8 Updated by Yuya Watanabe almost 11 years ago
- Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
- % Done changed from 50 to 70
#9 Updated by Mutsumi Imamura almost 11 years ago
- Status changed from Pending Testing(テスト待ち) to Rejected(差し戻し)
- % Done changed from 70 to 50
招待からの登録の場合と異なり、新規登録(ログイン画面の新規登録をクリック)で登録フローを行うと招待メールに記載されているURLをクリックすると /member/registerInput にリダイレクトされます。
(招待された場合は「プロフィールを入力する」の/member/register/ の画面に遷移します。)
この場合、今回追加したログアウト処理をスキップしてしまうため、登録完了させると自動ログインでログイン済みのユーザーになってしまいます。
招待の場合とフローを合わせる、選択肢もありますが、ユーザーが履歴などから直接 /member/registerInput にアクセスする可能性もありますので、
/member/registerInput にアクセス時もログアウト処理を追加するようにしてください。
#10 Updated by Youichi Kimura almost 11 years ago
- Status changed from Rejected(差し戻し) to Pending Review(レビュー待ち)
更新履歴 8fbdf778cd6133103bbf08a261928893a8c2eb0d で適用されました。
#11 Updated by Youichi Kimura almost 11 years ago
/member/register 以外の /member/registerInput, /opAuthMailAddress/registerEnd 等に直接遷移した場合であってもセッションを失効させるために、各アクションで使用する opSecurityUser::setRegisterToken() 内で logout() を呼び出すよう修正しました。
#12 Updated by Yuya Watanabe almost 11 years ago
図に含まれてないものとして, member/login 時に registerBegin であれば member/registerInput に遷移する可能性がありますが基本的には registerInput でやれば問題無さそうですね.
#13 Updated by Yuya Watanabe almost 11 years ago
- Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
- % Done changed from 50 to 70
#15 Updated by isao sano over 7 years ago
- Status changed from Pending Testing(テスト待ち) to Won't fix(対応せず)
- % Done changed from 70 to 0
OpenPNE 3.8.10 にて対応済みであったため、対応せずとします。