Bug(バグ) #3073
http のWeb全体に公開が可能な画面に https のログイン画面を経由してアクセスしようとするとログインに失敗する
0%
Description
現象¶
diary/show, member/profile のような is_secure:true ではないページに https のログイン画面を経由してアクセスしようとするとログインに失敗する。
再現手順¶
1. OpenPNE.yml にて use_ssl:true にし、それ以外のSSL設定を初期値のままにする
2. SNSにアクセスし、全体に公開の日記「日記A」を作成する
3. ログアウトして「日記A」のURLにアクセス
4. ログイン画面にリダイレクトされる(ログイン画面の next_uri に日記Aのアクションが入っている)
5. 正しいメールアドレスとパスワードを入力してログインを実行
6. 「ログインに失敗しました」画面が表示される
現象確認バージョン¶
- OpenPNE 3.6.3
- opDiaryPlugin 1.4.0.1
Related issues
Associated revisions
(refs #3073) fixed
make redirect to login action when tha page is following condition
1. login action is required ssl setting.
2. access doesn't use ssl then the member is not login.
3. the accessed page is that 'is_secure' is 'false' or 'both'.
(refs #3073) add feature to set next_uri value when the GET parameter was given
Merge branch 't3073' (fixes #3073)
(refs #3073) fixed not to occur redirect loop when access to login page
(fixes #3073) fixed to make login url using ssl_base_url setting for not to redirect twice
(fixes #3073) fixed to redirect considering include path
(refs #3073) extract redirect logic to private method
History
#1 Updated by Kiwa Sakai over 12 years ago
- Project changed from opDiaryPlugin to OpenPNE 3
coreの問題だったのでチケットを移動します
#2 Updated by Kiwa Sakai over 12 years ago
- Subject changed from httpの日記ページに https のログイン画面を経由してアクセスしようとするとログインに失敗する to http のWeb全体に公開が可能な画面に https のログイン画面を経由してアクセスしようとするとログインに失敗する
- Description updated (diff)
#3 Updated by Yuma Sakata about 12 years ago
- Target version set to OpenPNE 3.8.x
#4 Updated by Yuya Watanabe about 12 years ago
- Target version changed from OpenPNE 3.8.x to OpenPNE 3.9.0-old
#5 Updated by Yuya Watanabe about 12 years ago
- Status changed from New(新規) to Accepted(着手)
- Assignee set to Yuya Watanabe
#6 Updated by Yuya Watanabe over 11 years ago
- 3.8 で発生するか set to Unknown (未調査)
修正方針¶
ログインアクションが SSL を用いる必要があり,ログインアクションに到達したときに SSL を用いていない場合(ログインアクションに forward した場合が該当)にログインアクションへリダイレクトする処理を追加する.
このとき、リダイレクトする前の URL をログイン後の遷移先に指定するために URL のパラメータに next_uri を指定できるようにする.
#7 Updated by Yuya Watanabe over 11 years ago
- 3.6 で発生するか changed from Unknown (未調査) to Yes (はい)
- 3.8 で発生するか changed from Unknown (未調査) to Yes (はい)
#8 Updated by Yuya Watanabe over 11 years ago
- Status changed from Accepted(着手) to Pending Review(レビュー待ち)
- % Done changed from 0 to 50
更新履歴 aacea12b69f661f784731521f4b2e56998ec8c1d で適用されました。
#9 Updated by Rimpei Ogawa over 11 years ago
- Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)
opMemberAction の $loginUrl = $this->generateUrl('login', array(), true);
は、現在のリクエストが https でない場合には、 http://〜 のURLを生成してしまいます。
リダイレクト後に http:// へリクエストするとさらにリダイレクトとなるため、リダイレクトループとなっているようです。
単純に absolute=true で URL を生成することができないので、opExecutionFilter::handleSsl() のように sfConfig::get('op_ssl_base_url') を利用するコードにする必要がありそうです。
#10 Updated by Mutsumi Imamura over 11 years ago
note-9 について、再現手順を補足します。
1. OpenPNE.yml にて use_ssl:false であることを確認(初期設定)する。
2. http://hogehoge.com/member/login にアクセスする
3. 下記のエラーメッセージが表示される(google chromeの場合)
このウェブページにはリダイレクト ループが含まれています http://hogehoge.com/member/login?next_uri=member%2Flogin%3Fnext_uri%3Dmember%252Flogin%253Fnext_uri%253Dmember%25252Flogin%25253Fnext_uri%25253Dmember%2525252Flogin%2525253Fnext_uri%2525253Dmember%252525252Flogin%252525253Fnext_uri%252525253Dmember%25252525252Flogin%25252525253Fnext_uri%25252525253Dmember%2525252525252Flogin%2525252525253Fnext_uri%2525252525253Dmember%252525252525252Flogin%252525252525253Fnext_uri%252525252525253Dmember%25252525252525252Flogin%25252525252525253Fnext_uri%25252525252525253Dmember%2525252525252525252Flogin%2525252525252525253Fnext_uri%2525252525252525253Dmember%252525252525252525252Flogin%252525252525252525253Fnext_uri%252525252525252525253Dmember%25252525252525252525252Flogin%25252525252525252525253Fnext_uri%25252525252525252525253Dmember%2525252525252525252525252Flogin%2525252525252525252525253Fnext_uri%2525252525252525252525253Dmember%252525252525252525252525252Flogin%252525252525252525252525253Fnext_uri%252525252525252525252525253Dmember%25252525252525252525252525252Flogin%25252525252525252525252525253Fnext_uri%25252525252525252525252525253Dmember%2525252525252525252525252525252Flogin%2525252525252525252525252525253Fnext_uri%2525252525252525252525252525253Dmember%252525252525252525252525252525252Flogin%252525252525252525252525252525253Fnext_uri%252525252525252525252525252525253Dmember%25252525252525252525252525252525252Flogin%25252525252525252525252525252525253Fnext_uri%25252525252525252525252525252525253Dmember%2525252525252525252525252525252525252Flogin%2525252525252525252525252525252525253Fnext_uri%2525252525252525252525252525252525253Dmember%252525252525252525252525252525252525252Flogin のウェブページは リダイレクトの回数が多すぎます。このサイトの Cookie を削除するか、サードパーティの Cookie を許可すると問題が解決することがあります。 引き続き問題が解決しない場合は、ご使用のコンピュータではなく、サーバー側の設定上の問題である 可能性があります。
#11 Updated by Yuya Watanabe over 11 years ago
- Status changed from Rejected(差し戻し) to Accepted(着手)
- % Done changed from 50 to 0
#12 Updated by Yuya Watanabe over 11 years ago
note-10 の原因¶
opExecuteFilter#handleSsl() のように,requiredAction のリストに入っているかどうかのチェックを行う際に use_ssl の値を見ていなかったため, ssl へのリダイレクトが不必要な場合でもリダイレクトを発生させていたため.
修正内容¶
opExecuteFilter#handleSsl() の様に use_ssl の値をチェックした上で note-6 の動作を行うようにする.
その他¶
note-9 と note-10 の内容はおそらく別問題のため, note-9 の懸念が払拭されていないようなのでステータスは変更していない.
#13 Updated by Rimpei Ogawa over 11 years ago
note-9 についてですが、リダイレクトループするというのは嘘でした。
http://example.com/hoge/fuga → http://example.com/member/login?next_uri=hoge%2Ffuga → https://example.com/member/login?next_uri=hoge%2Ffuga
のようにリダイレクトし、動作はしているようです。ただ、直接 https の URL へリダイレクトすれば、リダイレクト回数が1回分減らせるので、やはり note-9 に書いた通り op_ssl_base_url を利用したコードに修正したほうがよいと思います。
#14 Updated by Yuya Watanabe over 11 years ago
- Status changed from Accepted(着手) to Pending Review(レビュー待ち)
- % Done changed from 0 to 50
更新履歴 a037339495d50ba8c6de6b4aefc77018cab418d5 で適用されました。
#15 Updated by Yuya Watanabe over 11 years ago
- Status changed from Pending Review(レビュー待ち) to Accepted(着手)
- % Done changed from 50 to 0
パス付き (例: http://example.com/sns のような形)の場合にうまくいかないようなので再度修正します.
#16 Updated by Yuya Watanabe over 11 years ago
- Status changed from Accepted(着手) to Pending Review(レビュー待ち)
- % Done changed from 0 to 50
更新履歴 9994d6dbb4f0695af85c6af25cfea059690a42af で適用されました。
#17 Updated by Yuya Watanabe over 11 years ago
リダイレクト処理が大きくなりすぎたため,プライペートメソッドに切り出しました.
#18 Updated by Rimpei Ogawa over 11 years ago
- Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)
GETリクエストから取得した next_uri から URL が生成できなかった場合、ログイン直後に 500 エラーとなってしまいます。ただし、next_uri の値は OpenPNE へアクセスする URL の形式(以下で「外部URL形式」と言っているもの)として正しい形式の場合のみエラーとなります。
例えば、 /member/login?next_uri=hogefuga のような URL へアクセスすると再現できます。
現在、サーバが混み合っているか、メンテナンス中です。 ご迷惑をおかけいたしますが、しばらく時間を空けて再度アクセスしてください。
dev 環境の場合、
500 | Internal Server Error | sfConfigurationException The route "hogefuga" does not exist.
500 エラーの画面はユーザーが正しい画面へ戻るためのリンクがないため、この挙動では不便です。以下のいずれかに修正すべきだと思います。
- next_uri が正しくない場合、next_uri が空の場合と同じ挙動にする
- next_uri が正しくない場合、404 のエラー画面を表示する
opValidatorNextUri を見ると、findRoute() が失敗する(このチェックは妥当ではない→後述)不正な next_uri に対しては empty_value を返すよう実装されていることから、前者の「next_uri が空の場合と同じ挙動にする」を採用するのが自然だと思います。
この問題は opValidatorNextUri が findRoute() で 外部URL形式 からルートを探している一方で、sfAction::redirect() から呼ばれる sfWebController::genUrl() では 内部URL形式 で next_uri の値を扱っていることにより発生しています。
例えば、next_uri=hogefuga は opValidatorNextUri 内の findRoute() 時点で以下のルーティングルールとマッチするために、バリデーションをパスしています。
array(3) { ["name"]=> string(13) "default_index" ["pattern"]=> string(8) "/:module" ["parameters"]=> array(3) { ["module"]=> string(8) "hogefuga" ["action"]=> string(5) "index" ["sf_culture"]=> string(5) "ja_JP" } }
一方、sfWebController::genUrl() は hogefuga を module_name ではなく route_name と扱ってしまうため、例外が発生します。
現行の実装では next_uri へ渡す値は symfony の内部URL形式を意図しているため、 opValidatorNextUri も内部URL形式として正しいかどうかを判別するように修正すべきです。もしくは next_uri の値を外部URL形式へ変更する修正方法でもよいと思います。
なお、このコメントで指摘した問題はこのチケットの変更で GET パラメーターを受けつけることにより発生しやすくなったとはいえ、根本的には別問題であるので別チケット対応でもよいかもしれません。
#19 Updated by Yuya Watanabe over 11 years ago
- Status changed from Rejected(差し戻し) to Accepted(着手)
- % Done changed from 50 to 0
#20 Updated by Yuya Watanabe over 11 years ago
- Status changed from Accepted(着手) to Pending Review(レビュー待ち)
- % Done changed from 0 to 50
なお、このコメントで指摘した問題はこのチケットの変更で GET パラメーターを受けつけることにより発生しやすくなったとはいえ、根本的には別問題であるので別チケット対応でもよいかもしれません。
note-18 の最後のとおり、問題としては別だと思うので別チケットで扱うことが妥当だと思います.
とりあえず本チケット自体は note-18 の問題については取り扱わないという方針でいきたいと思います.
#21 Updated by Rimpei Ogawa over 11 years ago
- Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
- % Done changed from 50 to 70
note-18 の問題を取り扱わないということであれば、レビューOKです。
#22 Updated by Rimpei Ogawa over 11 years ago
note-18 の問題について、 #3295 で別チケットを作成しました。
#24 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.4 にて対応済みであったため、対応せずとします。