Project

General

Profile

Bug(バグ) #2585

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

Added by Yuya Watanabe almost 8 years ago. Updated almost 4 years ago.

Status:
Fixed(完了)
Priority:
Urgent(急いで)
Assignee:
Target version:
Start date:
2011-11-07
Due date:
% Done:

100%

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

Description

概要

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

確認環境

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


Related issues

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

Associated revisions

Revision 91bbf9ca (diff)
Added by Yuya Watanabe almost 8 years ago

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

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

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

History

#1 Updated by Yuya Watanabe almost 8 years ago

  • Description updated (diff)

#2 Updated by wa ta almost 8 years ago

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

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

#3 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Review(レビュー待ち) to 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 Updated by Yuya Watanabe almost 8 years ago

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

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

#5 Updated by Yuya Watanabe almost 8 years ago

  • Status changed from Accepted(着手) to Pending Fixing(修正待ち)

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

#6 Updated by Kousuke Ebihara almost 8 years ago

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

#7 Updated by Kousuke Ebihara almost 8 years ago

  • Priority changed from High(高め) to Urgent(急いで)

#8 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Pending Fixing(修正待ち) to Accepted(着手)

#9 Updated by Yuya Watanabe over 7 years ago

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

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

#10 Updated by Kousuke Ebihara over 7 years ago

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

#11 Updated by Youichi Kimura over 7 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100
  • 3.6 で発生するか set to Unknown (未調査)
  • 3.4 で発生するか set to Unknown (未調査)

テスト完了しました。

#12 Updated by kaoru n almost 4 years ago

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

Also available in: Atom PDF