Consider allowing cookie creation to be configured, and to be deferred until user accepts

apps4uco commented 2 years ago

Session cookies are complicated that's why I wanted to use this crate :)

Some best practices:

Taken from the Microsoft page (Fair Use .. hope MS agrees)

Change the default session ID name. In ASP.NET, the default name is ASP.NET_SessionId. This immediately gives away that the application is ASP.NET and that that cookie contains the session ID value Make the cookie name configurable
- Make sure the length of the session ID is long enough to prevent brute force attacks. Recommended length is 128 bits checked
- Make sure to create the session ID in a completely random way. This ensures that attackers can’t guess the session ID by using predictability analysis checked
- Ensure that the session ID does not contain any additional sensitive data. The data should be a random string of characters with no meaning checked
- HTTPS should be used for all session based applications handling sensitive data that depends on the user - but it could be logged server side
- Session cookies should be created with the Secure and HttpOnly attributes note secure wont work if not using https
- Prevent concurrent sessions where possible optional and may be complex to implement use ip address / UserAgent?
- Destroy sessions upon timeout, logoff, browser close or log-in from a separate location optionally store ip address and destroy session if it changes
Do not store any critical information in cookies. For example, do not store a user’s password in a cookie. As a rule, do not keep anything in a cookie that can compromise your application. Instead, keep a reference in the cookie to a location on the server where the data is checked
- Set expiration dates on cookies to the shortest practical time. Avoid using permanent cookies allow to be configured / or left as a session cookie (by not setting expiration)
- Consider encrypting information in cookies not useful in this case

Current code in src/manager.rs

hardcodes the session expiry duration:

sess.expires = Utc::now() + Duration::hours(6);

sets the cookie to expire in 20 years:

cookie.make_permanent();

Also in Europe legally you should not set a cookie until the user agrees.

https://www.cookielaw.org/your-cookie-law-rights/

So the cookie should only be created when the session is written to for the first time. Granted this could be handled by configuring layers and routers but at least an example would be good. However, seeing as most websites will get used in Europe as well it would be good to have this as default policy, i.e. when the user agrees to cookies then the session and cookie are created,.

Maybe it would be a good idea to allow all of the following to be configured by a configuration struct:

let cookie = Cookie::build("configuredname", "value")
    .domain("configured domain")
    .same_site(SameSite::Strict)
    .max_age(Duration::minutes(30)) //optional if not set expire on browser close
    .path("/")
    .secure(true)
    .http_only(true)
    .finish();

I could work on a PR if you are interested.

apps4uco commented 2 years ago

I just saw you were already planning some of the proposed enhancements

 //TODO: Make lifespan Adjustable to be Permenant, Per Session OR Based on a Set Duration from Config.

genusistimelord commented 2 years ago

yes if you would like to help out feel free to. But also know this library should not do everything that the end user should be handling in Axum. Though the Cookie stuff was a thing i wanted to add in later to help the end user set it up for HTTPS since currently it is not. Also creating a cookie for a Session is a must so we could check if one exists or not and make it so the end user cant get a session until the web back-end developer calls a function allowing it.

Also the Cookie only has a UUID so there is no actual data other than this that I am adding in this library to the cookie.

The Cookie name is configurable via https://github.com/AscendingCreations/AxumSessions/blob/main/src/config.rs#L57 I agree HTTPS is needed for sensitive data. I leave this choice up to the end user though so Adding in more configurations for the cookie can help with making their choice.

Anyways Feel free to help out.

genusistimelord commented 2 years ago

The

Prevent concurrent sessions where possible optional and may be complex to implement use ip address / UserAgent? This is not really much of a issue unless 2 people are using it at the same time which only way to prevent would be Secure Cookies. Which can be implemented pretty easily. Other than that as long as the Cookie itself is Secure and cant be copied between Sessions we can prevent a decent majority of Session stealing.

Storing a IP address to later destroy if it is different can make it more Secure but can also make it more aggravating to a end user. Especially if they where using a session to store Specific data and just went from a Coffee shop to there home for example. This could cause their site form or other information to be redone from scratch due to a login. So if this is Added it needs to be a Optional feature to the end user with a note saying that Data loss on IP switch can occur. Id prefer implementing Secure Cookies first here.

genusistimelord commented 2 years ago

@apps4uco hey Andy got any ideas how I should implement what is needed for the European Union laws? I have everything else in now other than that.

AscendingCreations / AxumSession