Project

General

Profile

Bug(バグ) #1985

Cookie for automatic login is deleted when automatic login is done (自動ログイン時に自動ログイン用のCookieが削除される)

Added by Shogo Kawahara about 8 years ago. Updated over 3 years ago.

Status:
Fixed(完了)
Priority:
High(高め)
Assignee:
Target version:
Start date:
2011-03-29
Due date:
% Done:

100%

3.6 で発生するか:
Unknown (未調査)
3.8 で発生するか:
Unknown (未調査)

Description

Overview

Cookie for automatic login is deleted when automatic login is done.

(自動ログイン時に自動ログイン用のCookieが削除される。)

Way to repro

1. SNSに自動ログインを有効にしてログイン
2. ブラウザ終了
3. 同ブラウザでSNSにアクセス (自動ログイン・この時点でCookie消失)
4. 同ブラウザ終了
5. 同ブラウザでSNSにアクセスするとログイン画面になる。

Environment

OpenPNE3.6.x 〜

Causes

opBaseSecurityUser::isValidSiteIdentifier()

がセッション無効時に false を返す。そのため、同クラス initialize() により
ログアウト処理が実行され、自動ログイン用Cookieが削除される。

その後、自動ログインの処理が実行されるため、再現方法 3 の時点では
ログインすると考えられる。

Way to fix

opBaseSecurityUser::isValidSiteIdentifier() について セッションが無いときは
true を返すように変更

上記修正後 #1182 で再度発生した問題について

概要

ログイン時に自動ログインにチェックを入れているにも関わらず,次回自動ログインは成功するが,その次の自動ログインに失敗する.
失敗する原因として自動ログイン用のCookieが削除されていることが確認されている.
#1182 の修正によって再度発生するようになった.

再現手順

  1. SNSに自動ログインを有効にしてログイン
  2. ブラウザ終了
  3. 同ブラウザでSNSにアクセス
  4. 同ブラウザ終了
  5. 同ブラウザでSNSにアクセス
    • ログアウト状態となっている

問題までの処理の流れ

  1. メンバが「次回から自動的にログイン」にチェックをいれてログインする
    • 自動ログイン用のCookieと今回限りのログイン用のCookieがサーバから送られてきてブラウザに保存される
  2. ブラウザを終了する
    • 今回限りのログイン用Cooikeが削除されるが自動ログイン用Cookieは残ったままとなる
  3. ブラウザを立ち上げてSNSにアクセスする
    • 最初は未ログイン状態となりopAnonymousMemberのインスタンスが生成される
    • opAnonymousMember生成時にログアウト処理が行われて自動ログイン用Cookieを削除する処理が行われる
    • ブラウザから送られて来た自動ログイン用Cookieを用いてopRememberLoginFilterがログインに成功する
    • ログインを完了してSNSを表示したときには今回限りのログイン用Cookieが存在するが自動ログイン用Cookieが削除されている状態となる
  4. ブラウザを終了する
    • 今回限りのログイン用Cookieが削除されてなにもない状態となる
  5. ブラウザを立ち上げてSNSをにアクセスする
    • opAmonymouseMemberとしてログイン画面にリダイレクトされる

原因

#1182 で修正されたコミット( decc4e2d )以降のソースで下記部分においてopAnonymousMemberのインスタンス生成時にログアウト処理が行われるように修正されたため.

lib/user/opSecurityUser.class.php 330行目

321  /**
322   * Initializes all credentials associated and status with this user.
323   */
324   public function initializeUserStatus()
325   {
326     opActivateBehavior::disable();
327     $member = $this->getMember();
328     opActivateBehavior::enable();
329 
330     if ($member instanceof opAnonymousMember || $member->getIsLoginRejected())
331     {
332       $this->logout();
333       $isSNSMember = false;
334     }
335     else
336     {
337       $isSNSMember = (bool)$member->getIsActive();
338     }
339 
340     $this->setIsSNSMember($isSNSMember);
341     if ($isSNSMember)
342     {
343       $member->updateLastLoginTime();
344     }
345   }

修正方針

opRememberLoginFilterを廃止し,Memberオブジェクトが初期化される際にopAnonymousMemberかどうかでログアウト処理を行うときと同じタイミングで自動ログインの判定を行う方針とする.

参考

調査を行ったログ: http://redmine.openpne.jp/issues/1985#note-13


Related issues

Related to OpenPNE 3 - Backport(バックポート) #2139: [PATCH] Cookie for automatic login is deleted when automatic login is done (自動ログイン時に自動ログイン用のCookieが削除される) Fixed(完了) 2011-03-29 2011-06-24
Related to OpenPNE 3 - Bug(バグ) #1182: 携帯でSNS強制退会直後にアクセスするとCredentials Required画面が表示される Fixed(完了) 2010-06-18
Related to OpenPNE 3 - Backport(バックポート) #2453: 自動ログイン時に自動ログイン用のCookieが削除される Fixed(完了) 2011-10-03
Related to OpenPNE 3 - Bug(バグ) #2585: 自動ログイン状態のメンバをログイン停止にしたとき最初の一回だけログインに成功してしまう Fixed(完了) 2011-11-07

Associated revisions

Revision 708159d1 (diff)
Added by Shogo Kawahara about 8 years ago

fixed opBaseSecurityUser because it delete Cookie for automatic log-in (fixes #1985)

Revision eebcc9da (diff)
Added by Yuya Watanabe about 8 years ago

fixed for coding standard (fixes #1985)

Revision 4cd2f0a9 (diff)
Added by Yuya Watanabe over 7 years ago

(fixes #1985) fixed automatic log-in not to eliminate cookie in correct pattern

History

#1 Updated by Shogo Kawahara about 8 years ago

具体的な修正案を Pull Request しました。
セッションまわりの処理に変更を加えているため、一応チェックの上取り込んでいただければと思います。

https://github.com/kawahara/OpenPNE3/commit/708159d12065270dbbea8e494acd8ba34da67d74

#2 Updated by Shogo Kawahara about 8 years ago

  • Subject changed from Cookie for automatic login is deleted when automatic login is done (自動ログイン時に自動ログイン用のCookieが削除される) to [PATCH] Cookie for automatic login is deleted when automatic login is done (自動ログイン時に自動ログイン用のCookieが削除される)

#3 Updated by Shogo Kawahara about 8 years ago

  • Status changed from New(新規) to Accepted(着手)

#4 Updated by Shingo Yamada about 8 years ago

  • Priority changed from Normal(通常) to High(高め)

#5 Updated by Shogo Kawahara about 8 years ago

  • Assignee deleted (Shogo Kawahara)

#6 Updated by Shogo Kawahara about 8 years ago

取込みを作業を他の人にお願いしたいです

#7 Updated by Shingo Yamada about 8 years ago

  • Assignee set to Yuya Watanabe

#8 Updated by Yuya Watanabe about 8 years ago

下記の動作を確認し、取り込みを行いました。
また、コーディング規約違反部分の修正も行いました。

  1. SNSに自動ログインを有効にしてログイン
  2. ブラウザ終了
  3. 同ブラウザでSNSにアクセス
  4. 同ブラウザ終了
  5. 同ブラウザでSNSにアクセスしてログイン画面にならないことを確認

#9 Updated by wa ta about 8 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

更新履歴 eebcc9dae9f2fb175e3575949a559b119f3949a5 で適用されました。

#10 Updated by Masato Nagasawa about 8 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

レビューOKです。

#11 Updated by Shogo Kawahara almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Pending Review(レビュー待ち)
  • % Done changed from 70 to 50

更新履歴 708159d12065270dbbea8e494acd8ba34da67d74 で適用されました。

#12 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Accepted(着手)

下記手順で確認を行うとログアウト状態となり,本チケットの問題が解決できてないことを確認したためステータスを変更します.

  1. SNSに自動ログインを有効にしてログイン
  2. ブラウザ終了
  3. 同ブラウザでSNSにアクセス
  4. 同ブラウザ終了
  5. 同ブラウザでSNSにアクセス
  6. ログアウト状態となっている

#13 Updated by Yuya Watanabe over 7 years ago

note-12の動作中におけるブラウザ側のCookieを確認しましたが,3の時点で自動ログイン用のCookieが削除された状態であることを確認しました.ここでソースを見てみると,自動ログイン用のCookieが変更される部分はlib/user/opSecurityUser.class.phpのsetRememberLoginCookie()です.該当箇所は以下のようになります.

lib/user/opSecurityUser.class.php

154  /**
155   * set remember login cookie
156   */
157   protected function setRememberLoginCookie($isDeleteCookie = false)
158   {
159     $key = md5(sfContext::getInstance()->getRequest()->getHost());
160     $path = sfContext::getInstance()->getRequest()->getRelativeUrlRoot();
161     if (!$path)
162     {
163       $path = '/';
164     }
165 
166     if ($isDeleteCookie)
167     {
168       if (!sfContext::getInstance()->getRequest()->getCookie($key))
169       {
170         return;
171       }
172 
173       if ($this->getMemberId())
174       {
175         $this->getMember()->setConfig('remember_key', '');
176       }
177 
178       $value = null;
179       $expire = time() - 3600;
180     }
181     else
182     {
183       $rememberKey = opToolkit::getRandom();
184       if (!$this->getMemberId())
185       {
186         throw new LogicException('No login');
187       }
188       $this->getMember()->setConfig('remember_key', $rememberKey);
189 
190       $value = base64_encode(serialize(array($this->getMemberId(), $rememberKey)));
191       $expire = time() + sfConfig::get('op_remember_login_limit', 60*60*24*30);
192     }
193 
194     sfContext::getInstance()->getResponse()->setCookie($key, $value, $expire, $path, '', false, true);
195   }
196 

このソースより,setRememberLoginCookie()の引数にtrueを渡す処理を行うとCookieが削除される動作が行われると思います.
そしてsetRememberLoginCooike()の引数にtrueを渡すのは同ファイル内logout()だけであることを確認しました.
このlogout()がどこのタイミングで呼び出されるかを確認するために以下の実験を行いました.

実験内容

ログ出力部分変更を加えてdev環境によってログを収集し,logout()が呼び出されるタイミングを確認する.

実験手順

  1. SNSに自動ログインを有効にしてログイン
  2. ブラウザ終了
  3. ログ開始
  4. 同ブラウザを立ち上げてSNSにアクセス
  5. ホームを表示

ログ収集のための変更内容

diff --git a/lib/user/opSecurityUser.class.php b/lib/user/opSecurityUser.class.php
index 7d66a46..490b26f 100644
--- a/lib/user/opSecurityUser.class.php
+++ b/lib/user/opSecurityUser.class.php
@@ -289,6 +289,7 @@ class opSecurityUser extends opAdaptableUser
   {
     $authMode = $this->getCurrentAuthMode();

+    $this->dispatcher->notify(new sfEvent($this, 'application.log', array("opSecurityUser logout()")));
     $this->setRememberLoginCookie(true);

     $this->setAuthenticated(false);
@@ -329,6 +330,7 @@ class opSecurityUser extends opAdaptableUser

     if ($member instanceof opAnonymousMember || $member->getIsLoginRejected())
     {
+      $this->dispatcher->notify(new sfEvent($this, 'application.log', array(sprintf("%s || %s", get_class($member), $member->getIsLoginRejected()))));
       $this->logout();
       $isSNSMember = false;
     }

実験結果

結果は以下のとおり.ルーティングやレンダリングに関するログは省略.

Sep 28 21:14:22 symfony [info] {sfPatternRouting} Match route "homepage" (/) for / with parameters array (  'module' => 'member',  'action' => 'home',)
 9月 28 21:14:22 symfony [info] {myUser} opAnonymousMember ||
 9月 28 21:14:22 symfony [info] {myUser} opSecurityUser logout()
 9月 28 21:14:22 symfony [info] {myUser} User is not authenticated
 9月 28 21:14:22 symfony [info] {myUser} User is not authenticated
 9月 28 21:14:22 symfony [info] {sfFilterChain} Executing filter "sfRenderingFilter" 
 9月 28 21:14:22 symfony [info] {sfFilterChain} Executing filter "opCacheControlFilter" 
 9月 28 21:14:22 symfony [info] {sfFilterChain} Executing filter "opCheckEnabledApplicationFilter" 
 9月 28 21:14:22 symfony [info] {sfFilterChain} Executing filter "opAppendXRDSHeaderFilter" 
 9月 28 21:14:22 symfony [info] {sfFilterChain} Executing filter "opRememberLoginFilter" 
 9月 28 21:14:22 symfony [info] {myUser} User is authenticated
 9月 28 21:14:22 symfony [info] {myUser} Add credential(s) "SNSMember" 
 9月 28 21:14:22 symfony [info] {sfFilterChain} Executing filter "sfBasicSecurityFilter" 
 9月 28 21:14:22 symfony [info] {sfFilterChain} Executing filter "opEmojiFilter" 
 9月 28 21:14:22 symfony [info] {sfFilterChain} Executing filter "opExecutionFilter" 
 9月 28 21:14:22 symfony [info] {memberActions} Call "memberActions->executeHome()" 
 9月 28 21:14:22 symfony [info] {memberActions} Change layout to "layoutA" 
 9月 28 21:14:22 symfony [info] {opView} Set customize "cautionAboutFriendPre" (friend/cautionAboutFriendPre)
 9月 28 21:14:22 symfony [info] {opView} Set customize "cautionAboutCommunityMemberPre" (community/cautionAboutCommunityMemberPre)
…
 9月 28 21:14:23 symfony [info] {opWebResponse} Send status "HTTP/1.1 200 OK" 
 9月 28 21:14:23 symfony [info] {opWebResponse} Send header "Content-Type: text/html; charset=utf-8" 
 9月 28 21:14:23 symfony [info] {opWebResponse} Send header "Expires: Fri, 22 Apr 1988 17:00:00 GMT" 
 9月 28 21:14:23 symfony [info] {opWebResponse} Send header "Lastmodified: Wed, 28 Sep 2011 12:14:23 GMT" 
 9月 28 21:14:23 symfony [info] {opWebResponse} Send header "Cache-Control: no-store, no-cache, private, max-age=0, must-revalidate, post-check=0, pre-check=0" 
 9月 28 21:14:23 symfony [info] {opWebResponse} Send header "Pragma: no-cache" 
 9月 28 21:14:23 symfony [info] {opWebResponse} Send cookie "2dc1b1d9f6936933a8af7f0f643c3ab9": "" 
 9月 28 21:14:24 symfony [info] {opWebResponse} Send content (875197 o)
…

上記よりわかること

ブラウザを立ち上げ直した後最初にSNSにアクセスした場合,opAnonymousMemberでアクセスした状態となり,logout()が実行されて自動ログイン用のセッションが初期化される.この時,opSecurityUser.class.phpの175行目でセッションを初期化しようとするがopAnonymousMemberのためDBのテーブルの方は初期化されない.しかし194行目の処理によってCookieを削除してしまう.その後opRememberLoginFilterによってブラウザから送られてきたCookieを用いてログインが成功する.次にホームが表示されたときには先程の処理によってCookieが削除されてしまうため次のログインが行われない状態となっている.
ブラウザ内のCookieが削除される.

#14 Updated by Rimpei Ogawa over 7 years ago

(補足)
#1182 の修正で、opSecurityUser の初期化時に有効なセッションがない場合(ただし、この時点では自動ログイン用のCookieは評価されていない)、logout() を呼び出す修正がなされており、この修正以降で本チケットで報告されているのと同じ現象が再発しているようです。

#15 Updated by Yuya Watanabe over 7 years ago

修正方針

ログアウトを分類すると2種類あると考えられる.それは自動ログインの情報も初期化するログアウトと自動ログインの情報を保持したままするログアウトである.前者の例としてはメンバ自身がログアウトのリンクをクリックしたり,退会したり,自動ログインの有効期限が切れている場合がある.後者の例としては本チケットの問題となっているブラウザ終了がある.

本チケットではopRememberLoginFilterを廃止し,Memberオブジェクトが初期化される際にopAnonymousMemberかどうかでログアウト処理を行うときと同じタイミングで自動ログインの判定を行う方針とする.こうすることで自動ログインの有効期限切れも含めたログイン管理をうまく処理することができると考えられる.

#16 Updated by Yuya Watanabe over 7 years ago

  • Description updated (diff)

#17 Updated by Yuya Watanabe over 7 years ago

  • Subject changed from [PATCH] Cookie for automatic login is deleted when automatic login is done (自動ログイン時に自動ログイン用のCookieが削除される) to Cookie for automatic login is deleted when automatic login is done (自動ログイン時に自動ログイン用のCookieが削除される)

#18 Updated by Yuya Watanabe over 7 years ago

実装案

diff --git a/apps/pc_frontend/config/filters.yml b/apps/pc_frontend/config/filters.yml
index b202269..c44ddce 100644
--- a/apps/pc_frontend/config/filters.yml
+++ b/apps/pc_frontend/config/filters.yml
@@ -9,9 +9,6 @@ enable_app:
 xrds_header:
   class: opAppendXRDSHeaderFilter

-remember_login:
-  class: opRememberLoginFilter
-
 security:  ~

 emoji:
diff --git a/lib/filter/opRememberLoginFilter.class.php b/lib/filter/opRememberLoginFilter.class.php
deleted file mode 100644
index b41e372..0000000
--- a/lib/filter/opRememberLoginFilter.class.php
+++ /dev/null
@@ -1,36 +0,0 @@
-<?php
-
-/**
- * This file is part of the OpenPNE package.
- * (c) OpenPNE Project (http://www.openpne.jp/)
- *
- * For the full copyright and license information, please view the LICENSE
- * file and the NOTICE file that were distributed with this source code.
- */
-
-/**
- * opRememberLoginFilter
- *
- * @package    OpenPNE
- * @subpackage filter
- * @author     Shogo Kawahara <kawahara@tejimaya.net>
- */
-class opRememberLoginFilter extends sfFilter
-{
-  /**
-   * Executes this filter.
-   *
-   * @param sfFilterChain $filterChain A sfFilterChain instance
-   */
-  public function execute($filterChain)
-  {
-    if ($this->isFirstCall() && !$this->context->getUser()->isAuthenticated())
-    {
-      if ($memberId = $this->context->getUser()->getRememberedMemberId())
-      {
-        $this->context->getUser()->login($memberId);
-      }
-    }
-    $filterChain->execute();
-  }
-}
diff --git a/lib/user/opSecurityUser.class.php b/lib/user/opSecurityUser.class.php
index 2dd1714..c39089c 100644
--- a/lib/user/opSecurityUser.class.php
+++ b/lib/user/opSecurityUser.class.php
@@ -327,7 +327,17 @@ class opSecurityUser extends opAdaptableUser
     $member = $this->getMember();
     opActivateBehavior::enable();

-    if ($member instanceof opAnonymousMember || $member->getIsLoginRejected())
+    if ($member->getIsLoginRejected())
+    {
+      $this->logout();
+      $isSNSMember = false;
+    }
+    elseif ($memberId = $this->getRememberedMemberId())
+    {
+      $this->setMemberId($memberId);
+      $isSNSMember = true;
+    }
+    elseif ($member instanceof opAnonymousMember)
     {
       $this->logout();
       $isSNSMember = false;

動作確認

確認内容

影響がありそうな点について正しい動作が行えているかについて確認を行った.網羅的に行なっていないのでここに記述されているだけのテストでは不十分であることに注意.

確認環境

OpenPNE3.7.0-dev (master)
上記実装案を適用した状態+ #2469 の修正を施した状態
SNSログイン用ブラウザA: Chrome 14.0.835.202
管理画面用ブラウザB: Firefox 7.0.1

確認内容と確認結果

  • 普通の状態
    • 期待される結果:正しい入力及び自動ログインに成功する
    1. ブラウザAでログイン画面を表示する
      • ログイン画面の表示に成功
    2. ブラウザAでSNSに自動ログインを有効にしてログイン
      • ホームの表示に成功
    3. ブラウザA終了
    4. ブラウザAを立ち上げてSNSにアクセス
      • ホームの表示に成功
    5. ブラウザA終了
    6. ブラウザAを立ち上げてSNSにアクセス
      • ホームの表示に成功
  • ログイン停止状態
    • 期待される結果:正しい入力をしてもログイン出来ない状態
    1. ブラウザB:管理画面でメンバをログイン停止状態にする
    2. ブラウザAでログイン画面を表示する
      • ログイン画面の表示に成功
    3. ブラウザAでSNSに自動ログインを有効にしてログイン
      • 「ログインに失敗しました。」という表示がされてログインに失敗する
  • 自動ログインにチェックを入れてログイン画面からログインしたあとログイン停止になった状態
    • 期待される結果:何らかのページを開くときにログイン画面に遷移する
    1. ブラウザAでログイン画面を表示する
      • ログイン画面の表示に成功
    2. ブラウザAでSNSに自動ログインを有効にしてログイン
      • ホーム画面の表示に成功
    3. ブラウザB:管理画面でログイン停止を行う
    4. ブラウザAでSNSでホーム画面の表示を行う
      • ログイン画面が表示される
      • 自動ログイン用のCookieは削除されセッションCookieも新規のものが発行されている
  • 自動ログインで直接ホームを表示したあとログイン停止になった状態
    • 期待される結果:何らかのページを開くときにログイン画面に遷移する
    1. ブラウザAでログイン画面を表示する
      • ログイン画面の表示に成功
    2. ブラウザAでSNSに自動ログインを有効にしてログイン
      • ホーム画面の表示に成功
    3. ブラウザA終了
    4. ブラウザB:管理画面でログイン停止を行う
    5. ブラウザAを立ち上げてSNSにアクセス
    6. ブラウザAでSNSホーム画面の表示を行う
      • ログイン画面が表示される
      • 自動ログイン用のCookieは削除されセッションCookieも新規のものが発行されている

#19 Updated by wa ta over 7 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

更新履歴 4cd2f0a94a80358ae36b3b9b0a93fa91f35ac725 で適用されました。

#20 Updated by Mutsumi Imamura over 7 years ago

  • 360対象 set to 3.6.0

#21 Updated by Kousuke Ebihara over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

#22 Updated by Fumie Toyooka over 7 years ago

  • Status changed from Pending Testing(テスト待ち) to Rejected(差し戻し)
  • % Done changed from 70 to 50

テスターテストで以下の動作の確認ができませんでした。ご確認ください。
(note-18 の確認内容と確認結果の3つめの動作に相当)

[テスト手順]
  1. ブラウザでSNSに自動ログインを有効にして member_id = 2 でログイン
  2. ブラウザを終了させる
  3. 別のブラウザで管理画面より member_id = 2 のログイン停止を行う
  4. (2.) のブラウザを立ち上げて SNS (トップのURL) にアクセスする

[期待結果]
4. の時点でログイン画面に遷移する

[テスト結果]
しかし 4. の時点でマイホームが表示されてしまった。
しかも 4. で表示されたのは 1. 時点のキャッシュではなかった。
再現性は100%では無く、たまにログイン画面が表示されたので、要調査。

その後の動作結果は次の通り:
5-a. (4.) の後、ページをリロード(ブラウザでリロード)したらログイン画面へ遷移した
5-b. (4.) の後、ナビゲーションから別のページへ遷移したらログイン画面へ遷移した
5-b2. ログイン不要のページにアクセスした場合は、ログアウト状態でそのページが表示された

#23 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)

#24 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

note-22の問題は #2585 で対応することにしました.本チケットでは対象の問題を除いた部分に関してを取り扱うことにします.

#25 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70
  • 3.6 で発生するか changed from Yes to Unknown (未調査)
  • 3.4 で発生するか set to Unknown (未調査)

#26 Updated by Shouta Kashiwagi about 7 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

テストOKです。

#27 Updated by kaoru n over 3 years ago

  • 3.8 で発生するか set to Unknown (未調査)

Also available in: Atom PDF