プロジェクト

全般

プロフィール

Bug(バグ) #2585

自動ログイン状態のメンバをログイン停止にしたとき最初の一回だけログインに成功してしまう

Yuya Watanabe12年以上前に追加. 8年以上前に更新.

ステータス:
Fixed(完了)
優先度:
Urgent(急いで)
担当者:
対象バージョン:
開始日:
2011-11-07
期日:
進捗率:

100%

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

説明

概要

自動ログイン状態のメンバをログイン停止にしたとき最初の一回だけログインに成功してしまう.

確認環境

OnenPNE 3.7.0-dev (master)
OpenPNE 3.6.0 (stable-3.6.x)

再現手順

  • 自動ログインで直接ホームを表示したあとログイン停止になった状態
    • 期待される結果:何らかのページを開くときにログイン画面に遷移する
    1. ブラウザAでログイン画面を表示する
    2. ブラウザAでSNSに自動ログインを有効にしてログイン
    3. ブラウザA終了
    4. ブラウザB:管理画面でログイン停止を行う
    5. ブラウザAを立ち上げてSNSにアクセス
    6. ブラウザAでSNSホーム画面の表示を行う
      • ログイン画面が表示される必要がある

原因

#1985 での実装に不備があったため.

再現手順の際では,下記コード部334行目でopAnonymousMemberのオブジェクトが返ってくるため337行目の条件がfalseになり,次の342行目で自動ログインのメンバを取得するが,このメンバがログイン停止状態かどうかを判定する処理が抜けている.

lib/user/opSecurityUser.class.php

331   public function initializeUserStatus()
332   {
333     opActivateBehavior::disable();
334     $member = $this->getMember();
335     opActivateBehavior::enable();
336 
337     if ($member->getIsLoginRejected())
338     {
339       $this->logout();
340       $isSNSMember = false;
341     }
342     elseif ($memberId = $this->getRememberedMemberId())
343     {
344       $this->setMemberId($memberId);
345       $isSNSMember = true;
346     }
347     elseif ($member instanceof opAnonymousMember)
348     {
349       $this->logout();
350       $isSNSMember = false;
351     }
352     else
353     {
354       $isSNSMember = (bool)$member->getIsActive();
355     }
356 
357     $this->setIsSNSMember($isSNSMember);
358     if ($isSNSMember)
359     {
360       $member->updateLastLoginTime();
361     }
362   }

実装案

diff --git a/lib/user/opSecurityUser.class.php b/lib/user/opSecurityUser.class.php
index be67cc6..25f8392 100644
--- a/lib/user/opSecurityUser.class.php
+++ b/lib/user/opSecurityUser.class.php
@@ -334,19 +334,13 @@ class opSecurityUser extends opAdaptableUser
     $member = $this->getMember();
     opActivateBehavior::enable();

-    if ($member->getIsLoginRejected())
-    {
-      $this->logout();
-      $isSNSMember = false;
-    }
-    elseif ($memberId = $this->getRememberedMemberId())
+    if ($memberId = $this->getRememberedMemberId())
     {
       $this->setMemberId($memberId);
       $isSNSMember = true;
     }
     elseif ($member instanceof opAnonymousMember)
     {
-      $this->logout();
       $isSNSMember = false;
     }
     else
@@ -354,11 +348,20 @@ class opSecurityUser extends opAdaptableUser
       $isSNSMember = (bool)$member->getIsActive();
     }

+    if ($this->getMember()->getIsLoginRejected())
+    {
+      $isSNSMember = false;
+    }
+
     $this->setIsSNSMember($isSNSMember);
     if ($isSNSMember)
     {
       $member->updateLastLoginTime();
     }
+    else
+    {
+      $this->logout();
+    }
   }

   public function isMember()

参考

http://redmine.openpne.jp/issues/1985#note-22


関連するチケット

関連している OpenPNE 3 - Bug(バグ) #1985: Cookie for automatic login is deleted when automatic login is done (自動ログイン時に自動ログイン用のCookieが削除される) Fixed(完了) 2011-03-29
関連している OpenPNE 3 - Backport(バックポート) #2586: 自動ログイン状態のメンバをログイン停止にしたとき最初の一回だけログインに成功してしまう Fixed(完了) 2011-11-07
関連している OpenPNE 3 - Backport(バックポート) #3118: 自動ログイン状態のメンバをログイン停止にしたとき最初の一回だけログインに成功してしまう Fixed(完了) 2011-11-07

関係しているリビジョン

リビジョン 91bbf9ca (差分)
Yuya Watanabe12年以上前に追加

(fixes #2585) fixed not to login rejected member using automatic login cookie

リビジョン 4dbc09c5 (差分)
Yuya Watanabe12年以上前に追加

(fixes #2585) fixed not to login rejected member using automatic login cookie (from patch)

履歴

#1 Yuya Watanabe12年以上前に更新

  • 説明 を更新 (diff)

#2 wa ta12年以上前に更新

  • ステータスAccepted(着手) から Pending Review(レビュー待ち) に変更
  • 進捗率0 から 50 に変更

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

#3 Minoru Takai12年以上前に更新

  • ステータスPending Review(レビュー待ち) から Rejected(差し戻し) に変更

https://github.com/openpne/OpenPNE3/blob/e7d09771f6d0f8aa1a0707c7b5e90a945670d9e0/lib/user/opSecurityUser.class.php#L322

  • opSecurityUser::initializeUserStatus() は、もともと次のような実装にしていました( #1944 修正後の時点)
      public function initializeUserStatus()
      {
        // ユーザが SNS のページにアクセスすると、このメソッドが呼ばれる
    
        opActivateBehavior::disable();
        $member = $this->getMember();
        opActivateBehavior::enable();
        // opActivateBehavior を無効にして getMember() をすると、
        // Member テーブルにレコードのあるユーザの場合に Member インスタンスを返す
    
        // (1) 登録済みメンバー
        // (2) 仮登録中メンバー(招待状発行済でそのトークンを持つURLからのアクセス)
        // のいずれかであれば Member インスタンスが返り、そうでない場合は
        // (3) 未ログインユーザ
        // として opAnonymousMember インスタンスが返る
    
        // $member instanceof opAnonymousMember が真なら (3) だと判断し、
        // そうでなくとも (1) がログイン停止中である場合は、ログアウトさせる
        // ※ (2) がログイン停止中であることはあり得ない(仮登録中なので)
        if ($member instanceof opAnonymousMember || $member->getIsLoginRejected())
        {
          $this->logout();
          $isSNSMember = false;
        }
    
        // (1) か (2) の場合に到達
        else
        {
          // (1) なら true が返り、 (2) なら false が返る
          $isSNSMember = (bool)$member->getIsActive();
        }
    
        $this->setIsSNSMember($isSNSMember);
    
        if ($isSNSMember)
        {
          // (1) の場合には、最終ログイン日時を更新する
          $member->updateLastLoginTime();
        }
      }
    

note-2 の修正後のソースでも動作上は問題ないようですが、若干冗長になっているかもしれません。また、修正前は仮登録中のメンバーに対しては logout() が呼ばれていませんでしたが、この修正で呼ばれるようになっています(どちらでも動作上は問題ないのだろうか)。

修正案

diff --git a/lib/user/opSecurityUser.class.php b/lib/user/opSecurityUser.class.php
index 25f8392..fb0d353 100644
--- a/lib/user/opSecurityUser.class.php
+++ b/lib/user/opSecurityUser.class.php
@@ -327,41 +327,42 @@ class opSecurityUser extends opAdaptableUser

  /**
   * Initializes all credentials associated and status with this user.
+  * The user is one of the following:
+  *  - (U1) a user as a logged in member
+  *  - (U2) a user as a registering as a new member
+  *  - (U3) a user as a logged out member
   */
   public function initializeUserStatus()
   {
-    opActivateBehavior::disable();
-    $member = $this->getMember();
-    opActivateBehavior::enable();
-
+    // Automatic Login
     if ($memberId = $this->getRememberedMemberId())
     {
       $this->setMemberId($memberId);
-      $isSNSMember = true;
     }
-    elseif ($member instanceof opAnonymousMember)
+
+    // get a instance of Member or opAnonymousMember as the user
+    opActivateBehavior::disable();
+    $member = $this->getMember();
+    opActivateBehavior::enable();
+
+    // if user is (U3), or (U1) but rejected login
+    if ($member instanceof opAnonymousMember || $member->getIsLoginRejected())
     {
+      $this->logout();
       $isSNSMember = false;
     }
     else
     {
+      // this value is true only if user is (U1)
       $isSNSMember = (bool)$member->getIsActive();
     }

-    if ($this->getMember()->getIsLoginRejected())
-    {
-      $isSNSMember = false;
-    }
-
-    $this->setIsSNSMember($isSNSMember);
     if ($isSNSMember)
     {
       $member->updateLastLoginTime();
     }
-    else
-    {
-      $this->logout();
-    }
+
+    $this->setIsSNSMember($isSNSMember);
   }

#4 Yuya Watanabe12年以上前に更新

  • ステータスRejected(差し戻し) から Accepted(着手) に変更

note-3の修正案では冗長な部分を排していると思われ,また,修正案を適用して基本的な動作確認を行ったところ問題なかったためnote-3の修正案を適用します.

#5 Yuya Watanabe12年以上前に更新

  • ステータスAccepted(着手) から Pending Fixing(修正待ち) に変更

時間がとれないため,一旦ステータスを「修正待ち」にします.

#6 Kousuke Ebihara12年以上前に更新

  • 優先度Normal(通常) から High(高め) に変更

#7 Kousuke Ebihara12年以上前に更新

  • 優先度High(高め) から Urgent(急いで) に変更

#8 Yuya Watanabe12年以上前に更新

  • ステータスPending Fixing(修正待ち) から Accepted(着手) に変更

#9 Yuya Watanabe12年以上前に更新

  • ステータスAccepted(着手) から Pending Review(レビュー待ち) に変更
  • 進捗率0 から 50 に変更

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

#10 Kousuke Ebihara12年以上前に更新

  • ステータスPending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
  • 進捗率50 から 70 に変更

#11 Youichi Kimura約12年前に更新

  • ステータスPending Testing(テスト待ち) から Fixed(完了) に変更
  • 進捗率70 から 100 に変更
  • 3.6 で発生するかUnknown (未調査) にセット
  • 3.4 で発生するかUnknown (未調査) にセット

テスト完了しました。

#12 kaoru n8年以上前に更新

  • 3.8 で発生するかUnknown (未調査) にセット

他の形式にエクスポート: Atom PDF