gate
gate copied to clipboard
allow customize cookie settings
cookieのdomain以外の属性もconfigで設定できるようにする修正です。
path属性が指定されていないことが原因でredirect loopになってしまうのを防ぐことが目的です。 今はpath属性を付けないので、以下のようにリダイレクトループが発生してしまいます。
- 「/foo/bar」にアクセスする際、認証がexpireしていたらcookieに保存された認証情報を削除する
- このときcookieのpath属性が指定されていないので、ブラウザは「path=/foo」が指定されたと解釈する
- 「/login」へリダイレクトしてログイン処理を行う
- 認証完了後「/oauth2callback」へ戻ってきて、cookieに認証情報を追加する
- ブラウザはcookieのpath属性に「path=/」が指定されたと解釈する
- 「/foo/bar」へリダイレクトするがすぐに「/login」へリダイレクトしてしまいリダイレクトループに陥る
- 「/oauth2callback」と「/foo/bar」でcookieのパス属性が異なるので、別物として扱われる
- 「path=/」はログイン済みと認識されるが、「/foo」以下は未ログインとして認識されてしまう
@shogo82148 AuthSessionConf.CookieDomain と、 AuthSessionConf.Cookie.Domain とふたつDomainがあるのはいかんともしがたい?
二つにした理由は、クッキーの設定項目を martini-contrib/sessions の sessions.Options や net/http のhttp.Cookie とフィールド名前を合わせた方がわかりやすいのでは、と思ったからです。
対応案として、パッと思いつくのは以下の二通りですが、どちらの対応が良いでしょうか?
- 既存の設定項目(
AuthSessionConf.CookieDomain)にあわせて、AuthSessionConf.CookiePath,AuthSessionConf.CookieMaxAge, etc. のような名前に変更する - 今の僕の実装をそのまま採用して、
AuthSessionConf.Cookie.Domainを非推奨とドキュメントに書いておく- コード中には互換性のためしばらく残しておくが、指定されたら警告を出すようにしておく
ちょっと話は変わるのですがPathについて、fujiwaraさんから「Path=/をデフォルトにした方がよいのでは?」というような意見ももらっています。 Pathを未指定にするケースというのは考えにくいので、それでも良いかなと思っているんですが、 他の方のご意見も頂きたいです。
2でいいかなと思いますー。
あと、 / をデフォルトってのも異論なしです
2番、句読点の前後で逆のこと言ってますね・・・typoしてました。 以下が正しいです。
- 今の僕の実装をそのまま採用して、AuthSessionConf.Cookie.Domain を非推奨とドキュメントに書いておく
+ 今の僕の実装をそのまま採用して、AuthSessionConf.CookieDomain を非推奨とドキュメントに書いておく
- AuthSessionConf.CookieDomain 非推奨、互換性のために残す
- AuthSessionConf.Cookie.Domain 今後の推奨
- Path のデフォルト値は「/」
以上の認識で問題ないでしょうか?
:+1:
空目してちゃんと意図通りの意味で読んでましたw
修正しました。レビューお願いします :bow:
- Path のデフォルト値を「/」に変更
- ドキュメントにauth.session.cookieについて記載
- テスト追加