allow customize cookie settings

[shogo82148]

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」以下は未ログインとして認識されてしまう

[typester]

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

[shogo82148]

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

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

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

---

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

[typester]

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

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

[shogo82148]

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

- 今の僕の実装をそのまま採用して、AuthSessionConf.Cookie.Domain を非推奨とドキュメントに書いておく
+ 今の僕の実装をそのまま採用して、AuthSessionConf.CookieDomain を非推奨とドキュメントに書いておく

• AuthSessionConf.CookieDomain 非推奨、互換性のために残す
• AuthSessionConf.Cookie.Domain 今後の推奨
• Path のデフォルト値は「/」

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

[typester]

👍

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

[shogo82148]

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

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