gate icon indicating copy to clipboard operation
gate copied to clipboard

allow customize cookie settings

Open shogo82148 opened this issue 10 years ago • 6 comments

cookieのdomain以外の属性もconfigで設定できるようにする修正です。

path属性が指定されていないことが原因でredirect loopになってしまうのを防ぐことが目的です。 今はpath属性を付けないので、以下のようにリダイレクトループが発生してしまいます。

  1. 「/foo/bar」にアクセスする際、認証がexpireしていたらcookieに保存された認証情報を削除する
    • このときcookieのpath属性が指定されていないので、ブラウザは「path=/foo」が指定されたと解釈する
  2. 「/login」へリダイレクトしてログイン処理を行う
  3. 認証完了後「/oauth2callback」へ戻ってきて、cookieに認証情報を追加する
    • ブラウザはcookieのpath属性に「path=/」が指定されたと解釈する
  4. 「/foo/bar」へリダイレクトするがすぐに「/login」へリダイレクトしてしまいリダイレクトループに陥る
    • 「/oauth2callback」と「/foo/bar」でcookieのパス属性が異なるので、別物として扱われる
    • 「path=/」はログイン済みと認識されるが、「/foo」以下は未ログインとして認識されてしまう

shogo82148 avatar Jan 31 '15 13:01 shogo82148

@shogo82148 AuthSessionConf.CookieDomain と、 AuthSessionConf.Cookie.Domain とふたつDomainがあるのはいかんともしがたい?

typester avatar Feb 28 '15 07:02 typester

二つにした理由は、クッキーの設定項目を martini-contrib/sessions の sessions.Optionsnet/httphttp.Cookie とフィールド名前を合わせた方がわかりやすいのでは、と思ったからです。

対応案として、パッと思いつくのは以下の二通りですが、どちらの対応が良いでしょうか?

  1. 既存の設定項目(AuthSessionConf.CookieDomain)にあわせて、 AuthSessionConf.CookiePath, AuthSessionConf.CookieMaxAge, etc. のような名前に変更する
  2. 今の僕の実装をそのまま採用して、AuthSessionConf.Cookie.Domain を非推奨とドキュメントに書いておく
    • コード中には互換性のためしばらく残しておくが、指定されたら警告を出すようにしておく

ちょっと話は変わるのですがPathについて、fujiwaraさんから「Path=/をデフォルトにした方がよいのでは?」というような意見ももらっています。 Pathを未指定にするケースというのは考えにくいので、それでも良いかなと思っているんですが、 他の方のご意見も頂きたいです。

shogo82148 avatar Feb 28 '15 12:02 shogo82148

2でいいかなと思いますー。

あと、 / をデフォルトってのも異論なしです

typester avatar Mar 02 '15 01:03 typester

2番、句読点の前後で逆のこと言ってますね・・・typoしてました。 以下が正しいです。

- 今の僕の実装をそのまま採用して、AuthSessionConf.Cookie.Domain を非推奨とドキュメントに書いておく
+ 今の僕の実装をそのまま採用して、AuthSessionConf.CookieDomain を非推奨とドキュメントに書いておく
  • AuthSessionConf.CookieDomain 非推奨、互換性のために残す
  • AuthSessionConf.Cookie.Domain 今後の推奨
  • Path のデフォルト値は「/」

以上の認識で問題ないでしょうか?

shogo82148 avatar Mar 02 '15 01:03 shogo82148

:+1:

空目してちゃんと意図通りの意味で読んでましたw

typester avatar Mar 02 '15 01:03 typester

修正しました。レビューお願いします :bow:

  • Path のデフォルト値を「/」に変更
  • ドキュメントにauth.session.cookieについて記載
  • テスト追加

shogo82148 avatar Mar 02 '15 02:03 shogo82148