Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for sessions cookies to be 'root only' #53

Closed
JoJoBond opened this issue Nov 14, 2016 · 3 comments
Closed

Allow for sessions cookies to be 'root only' #53

JoJoBond opened this issue Nov 14, 2016 · 3 comments

Comments

@JoJoBond
Copy link

JoJoBond commented Nov 14, 2016

The session module will create session cookies whenever it finds no matching cookie for the requested path.
These cookies will e create for the path that has been requested.
e.g. If the request is for http://www.sample.com/foo/bar the cookie path will be something like '/foo/bar'.

This behaviour can lead to a single use having multiple vaild session cookies at once.

If a user has no valid cookie for a site and requests the path '/foo/bar', a cookie for that path will be created.
If the user then requests the path '/bar/foo', another cookie will be created, since the path of the previous one does not match.

Maintaining sessions is quite a task with mutliple sessions per path per user.

To solve issues like this, cookies can be created for the root path of a server exclusively.
This way a user will get a single cookie that is valid for the whole domain.

I'd therefore suggest to add an UseRootPathOnly property to the session module:
In ISessionWebModule.cs after line 56 insert:

bool UseRootPathOnly { get; set; }

In LocalSessionModule.cs after line 192 insert:

public bool UseRootPathOnly { get; set; }

In LocalSessionModule.cs replace line 37 with:

var sessionCookie = (UseRootPathOnly ? new Cookie(SessionCookieName, sessionId, "/") : new Cookie(SessionCookieName, sessionId));

or

Cookie sessionCookie;
if(UseRootPathOnly)
  sessionCookie = new Cookie(SessionCookieName, sessionId, "/");
else
  sessionCookie = new Cookie(SessionCookieName, sessionId);
@mariodivece
Copy link
Member

Very good suggestion @JoJoBond However there are 2 things I have changed:
UseRootPathOnly should not be part of the interface as this only applies to a specific session module.
I changed your suggestion of using UseRootPathOnly further by instead implementing a property called CookiePath. I have made "/" the default

Documentation below:

        /// <summary>
        /// Gets or sets the cookie path.
        /// Inf left empty, a cookie will be created for each path. The default value is "/"
        /// If a route is specified, then session cookies will be created only for the given path.
        /// Examples of this are:
        ///     "/"
        ///     "/app1/"
        /// </summary>
        /// <value>
        /// The cookie path.
        /// </value>

mariodivece added a commit that referenced this issue Nov 14, 2016
@mariodivece
Copy link
Member

Please check it out and let us know your comments. If this works let's just close the issue.

@JoJoBond
Copy link
Author

Looks great. And seems to work as expected.

Small typo in the comment at 'Inf '.

Issue can be closed.

mariodivece added a commit that referenced this issue Nov 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants