Bug(バグ) #2926
完了「モバイルCookie使用設定(is_use_mobile_cookie)」の値をfalseにすると、かんたんログインに必要な Cookie の値が更新・削除されなくなる
100%
説明
Overview (現象)¶
「モバイルCookie使用設定(is_use_mobile_cookie)」の値を false にすると、かんたんログインに必要な Cookie の値が更新・削除されない。
これにより、「モバイルCookie使用設定(is_use_mobile_cookie)」の値がデフォルトの値である true を運用途中に false に変更すると、
これまで Cookie を用いてかんたんログインを行っていたユーザーが、ユーザーの端末側の Cookie を消失していまうとそれ以降かんたんログインを行えなくなってしまう。
この問題は、管理画面の opAuthMobileUIDPlugin の設定で「携帯電話個体識別番号のみによって認証をおこなう」に変更することによって回避することが可能ではある。
再現バージョン¶
- OpenPNE3.6.2
(3.4系は Cookie を用いたかんたんログインの機能は無し)
再現手順¶
1. OpenPNE 新規設置時の設定値
・「モバイルCookie使用設定(is_use_mobile_cookie)」の値がデフォルトの値の true
・認証に使用する ID の設定が「Cookie が利用できる端末の場合は Cookie 内のユニークな ID を用い、それ以外の端末では携帯電話個体識別番号によって認証をおこなう」
1-1. 任意のSNSユーザにて、Cookieが使用できる端末でかんたんログインを設定する(「Cookieを利用したかんたんログイン」の実施)
2. SNS管理者が何らかの理由で OpenPNE.yml にある「モバイルCookie使用設定(is_use_mobile_cookie)」の値を false に変更する。
(false の場合、かんたんログインに必要な Cookie の値は更新・削除されなくなる)
3. 手順1-1のユーザーの端末にて Cookie を削除する等の何らかの操作で端末側の Cookie が 消失する。
4. 手順1-1のユーザーの端末にてかんたんログインを試みるが、ユーザーの端末側に Cookie が無いため認証が行えずログインエラーになる。
再設定しようとしても、「モバイルCookie使用設定(is_use_mobile_cookie)」値が false なので更新が行なわれない。
Causes (原因)¶
バグが発生した原因を記入
Way to fix (修正内容)¶
修正内容を記入
Yuya Watanabe さんが12年以上前に更新
メモ¶
モバイルCookie使用設定を true にした状態でログインしたメンバが存在したとき,モバイルCookie使用設定を false に変更すると,そのメンバがかんたんログインができなくなる.
モバイルCookie使用設定自体は, #2134 「au の一部の端末でセッションが保持できない (ログインできない)」 の回避策である #2342 「セッションキーをURLパラメータで引き回すことを強制する設定の追加」 によって行われた.この変更の意図は「ログイン出来ない人がいるので,セキュアでない状態を許容できるならばログインができるようにする方法を提供する」というものがあって,一部メンバのログイン制限を意図した実装ではない.
今回,Cookie UID が SNS 側に残ることでログイン出来ない状態は「何らかの形で得られた個体識別番号を取得した人間がその個体識別番号を利用してログインしようとするが,Cookie を用いることができる端末の場合はCookie個体識別番号も登録されているはずであり,携帯電話個体識別番号でログインに成功したとしてもCookie個体識別番号が存在した時にログインさせない」という状態と一致している.
これはSNS管理者が「ログイン出来ない人がいるので,個体識別番号を用いた認証が現在よりもセキュアでない状態を許容できるならばログインができるようにする方法を提供する」ということを許容したうえでモバイルCookie使用設定を変更したのならば,意図した動作とはいえない.
現在の実装については,Cookie個体識別番号を使える端末はCookie個体識別番号を用いて認証を行い,失敗している状態でなぜ携帯電話個体識別番号を用いて認証を行なっているかということがまだ理解できていない.
また,モバイルCookie使用設定がfalse担っているにもかかわらず Cookie個体識別番号がバリデーションまで渡されているという点が正しい実装であるかという点について疑問である.
情報メモ¶
認証に用いられる情報- 携帯電話個体識別番号
- Cookie個体識別番号
- Cookie が使える端末かどうか
- SNS が Cookie を使うと設定しているかどうか
- 認証プラグインで Cookie個体識別番号を使うか,携帯電話個体識別番号を使うか.(優先順位をつけることはできるが両方を同時に用いることはできない via 実装)
- 個体識別番号を SNS の設定変更で登録しているかどうか
修正方針メモ¶
検討中.
認証プラグインを変更する必要がある場合は別途 opAuthMobileUIDPlugin のプロジェクトにチケットを作成する予定.
Yuya Watanabe さんが12年以上前に更新
修正案¶
今まで mobile_cookie_uid が存在するかどうかで偽装した認証を弾く実装だったが,この部分をモバイルCookie使用設定の値を反映して mobile_cookie_uid が SNS に登録されている状態だったとしてもログイン可能な状態になるように変更した.
opAuthMobileUIDPlugin 内のdiff
diff --git a/lib/form/opAuthLoginFormMobileUID.class.php b/lib/form/opAuthLoginFormMobileUID.class.php index 59d49fa..747f12e 100644 --- a/lib/form/opAuthLoginFormMobileUID.class.php +++ b/lib/form/opAuthLoginFormMobileUID.class.php @@ -91,7 +91,7 @@ class opAuthLoginFormMobileUID extends opAuthLoginForm } } - if (isset($values['member']) && $values['member']->getConfig('mobile_cookie_uid') && self::MUST_USE_MOBILE_UID != $uidType) + if (isset($values['member']) && $values['member']->getConfig('mobile_cookie_uid') && self::MUST_USE_MOBILE_UID != $uidType && sfConfig::get('op_is_use_mobile_cookie', true)) { // The specified member already use mobile_cookie_uid, but this request doesn't contain the cookie. // This request must not be allowed.
また
また,モバイルCookie使用設定がfalse担っているにもかかわらず Cookie個体識別番号がバリデーションまで渡されているという点が正しい実装であるかという点について疑問である.
という部分については「モバイルCookie使用設定にかかわらず携帯電話に存在する mobile_cookie_uid を取得してきてログインに成功してしまう」という問題に帰着される.この問題は本チケットの主題とは直接は関係ないためこのチケットでの修正は行わない.下記に「モバイルCookie使用設定にかかわらず携帯電話に存在する mobile_cookie_uid を取得してきてログインに成功してしまう」の問題の修正案を示しておく.できれば, opWebRequest などという広い範囲ではなく opWebMobileRequest などのような携帯電話のみのリクエストクラスを用意して,その中で端末の種類や is_use_mobile_cookie の設定などによって getCookie() を使用したとしても何も返さない実装などを行うほうが好ましいが,バグ修正の範疇を超えるため対象チケットでは採用しないほうが望ましい.
diff --git a/lib/request/opWebRequest.class.php b/lib/request/opWebRequest.class.php index aabb1bd..2b8b111 100644 --- a/lib/request/opWebRequest.class.php +++ b/lib/request/opWebRequest.class.php @@ -218,6 +218,10 @@ class opWebRequest extends sfWebRequest public function getMobileUidCookie() { + if (!$this->isCookie()) + { + return null; + } return $this->getCookie(self::MOBILE_UID_COOKIE_NAME); }
プロジェクト移動について¶
本チケットの修正内容は認証処理の opAuthMobileUIDPlugin 単体での修正となり,プロジェクトを opAuthMobileUIDPlugin に移動させる.
Yuya Watanabe さんが12年以上前に更新
- プロジェクト を OpenPNE 3 から opAuthMobileUIDPlugin に変更
- 対象バージョン を削除 (
OpenPNE 3.6.4)
Yuya Watanabe さんが12年以上前に更新
- トラッカー を Backport(バックポート) から Bug(バグ) に変更
- 対象バージョン を v1.3.3 にセット
- 3.6 で発生するか を Unknown (未調査) にセット
- [QA]バグ通知済 を いいえ にセット
Yuya Watanabe さんが12年以上前に更新
- ステータス を Accepted(着手) から Pending Review(レビュー待ち) に変更
- 進捗率 を 0 から 50 に変更
下記コミット群で対応しました.
リファクタリング
https://github.com/ebihara/opAuthMobileUIDPlugin/commit/912d887ab3405eee39131d8e90ebff0c40ef71fc
https://github.com/ebihara/opAuthMobileUIDPlugin/commit/b3a9a272e770903d10f82b4d707fd8053de4279e
実際の修正
https://github.com/ebihara/opAuthMobileUIDPlugin/commit/38ef3ecea8e79aff4561ebc89f5cf6800f021b11
Yuya Watanabe さんが12年以上前に更新
デフォルト値が設定されていなかったので追加しました.
https://github.com/ebihara/opAuthMobileUIDPlugin/commit/4c128224240e910fe82d8412f419a828f5549f9b
Rimpei Ogawa さんが12年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Rejected(差し戻し) に変更
https://github.com/ebihara/opAuthMobileUIDPlugin/pull/1
に修正案をプルリクエストしました。
修正前後で、「uid_type: MUST_USE_MOBILE_UID」かつ「is_use_mobile_cookie: true」のパターンの挙動が変わっています(unset のところ)。
unset が必要なのは Cookie UID が利用可能な場合で、is_use_mobile_cookie だけでなく uid_type も条件に含めなければならないと思います。
Yuya Watanabe さんが12年以上前に更新
uid_type: MUST_USE_MOBILE_UID と uid_type: MUST_USE_COOKIE_UID の確認を誤っていたため,条件が抜けてしまったようです.
note-10 のコードを適用します.
Yuya Watanabe さんが12年以上前に更新
- ステータス を Rejected(差し戻し) から Pending Review(レビュー待ち) に変更
Rimpei Ogawa さんが12年以上前に更新
- ステータス を Pending Review(レビュー待ち) から Pending Testing(テスト待ち) に変更
- 進捗率 を 50 から 70 に変更
適用確認しました。レビューOKです。
Yuma Sakata さんが12年以上前に更新
- ステータス を Pending Testing(テスト待ち) から Rejected(差し戻し) に変更
- 進捗率 を 70 から 50 に変更
テスト実施したところ、期待結果と異なる点がありました。
ご確認お願いします。
大項目 | SNS が Cookie を使用する設定 |
中項目 | 認証プラグインで cookie と uid 使用 |
小項目 | mobile_cookie_uid がサーバーに存在しない |
その他 | 個体識別番号が正しい |
備考 | 一度もログインしてない環境で、かんたんログインできました |
修正方針 | かんたんログインができないように修正お願いします |
Yuya Watanabe さんが12年以上前に更新
note-10 はmobile_cookie_uid が使えるか使えないかによって SNS 側は判定して MobileUID を使うかどうかを決定しているため,
「mobile_cookie_uid が DB に存在しない状態」 は「mobile_cookie_uid が使えない状態」と等価となり,MobileUID を用いて認証を行うことを許容しています.そのためかんたんログインができるべきとして期待結果と一致すると考えてよいとします.
上記理由によりコード変更おこなっていないため Rejected から Pending Testing にステータスを変更します.
Yuya Watanabe さんが12年以上前に更新
- ステータス を Rejected(差し戻し) から Pending Testing(テスト待ち) に変更
- 進捗率 を 50 から 70 に変更
Yuma Sakata さんが12年以上前に更新
- ステータス を Pending Testing(テスト待ち) から Fixed(完了) に変更
- 進捗率 を 70 から 100 に変更
テストOKです。