erlyaws / yaws

Yaws webserver
https://erlyaws.github.io
BSD 3-Clause "New" or "Revised" License
1.28k stars 267 forks source link

[feature] making session_id customisable for ysession #326

Closed leoliu closed 6 years ago

leoliu commented 6 years ago

I have been wanting this feature for a while. Basically allow people to configure ysession_id_generator={M,F,A} and have yaws_session_server use M:F(A) to generate the session ID.

Currently how session id is generated by yaws_session_server is hardcoded. It has two issues (for me).

  1. Include node() in the ID
  2. Cannot customise randomness

thoughts?

vinoski commented 6 years ago

I'll look into it.

vinoski commented 6 years ago

Currently, the yaws_session_server:new_session/4 function takes a Cookie parameter, hypothetically allowing for the provision of session cookies generated outside of yaws_session_server. But the yaws_api wrapper functions around new_session do not expose that variant, and even if it were exposed, there's nothing compelling other parts of Yaws to call that variant or to accept session cookies passed to them.

The most practical way to allow for custom session cookies is to add a new yaws.conf global configuration variable. Normally in Yaws, such a variable is set to a module name, not an MFA, as there's no way to specify the latter in yaws.conf, plus specifying a module allows ample flexibility for the user. Yaws will expect a cookie generation module to export a new_cookie/0 function. This way, the custom session cookie generator is configured for yaws_session_server regardless of which API functions for new sessions are invoked.

I'll push a branch with this functionality later today.

leoliu commented 6 years ago

Thanks Steve for the detailed investigation. I didn't notice yaws_session_server:new_session/4 was there. So there is a way to inject session id after all. BTW the use of the word cookie in yaws_session_server is a bit unfortunate i.e. it has nearly nothing to do with cookies. Look forward to your branch.

[update] I just experimented with yaws_session_server:new_session/4 and it works well. I think it would be good to have it in yaws_api. Also yaws_api:set_cookie/3 crashed because I used a binary session id.

vinoski commented 6 years ago

See the custom-session-cookies branch, commit 05aadd7. It's not merged yet; feedback welcomed.

leoliu commented 6 years ago

yaws:setup_gconf/2 needs change as well.

vinoski commented 6 years ago

Thanks, force-pushed an update with this fix in 9d79c78. Some other ysession-related fields were missing from setup_gconf/2 as well.

leoliu commented 6 years ago

I have no further complaints. Thanks.

vinoski commented 6 years ago

Closing, addressed in f0f7426.