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

Fix cookie config #1873

Merged
merged 2 commits into from Jan 24, 2015
Merged

Fix cookie config #1873

merged 2 commits into from Jan 24, 2015

Conversation

tao-s
Copy link
Contributor

@tao-s tao-s commented Jan 24, 2015

NativeSessionStorage need prefix “cookie_” in paramater.
@aembler
Copy link
Member

aembler commented Jan 24, 2015

Thanks!

aembler added a commit that referenced this pull request Jan 24, 2015
@aembler aembler merged commit f3f3278 into concretecms:develop Jan 24, 2015
@aembler
Copy link
Member

aembler commented Jan 28, 2015

@tao-s I had to revert these two commits. Something about them was causing problems. Could you explain what this pull request was solving? I don't think the cookie paths are right.

For example, when I had your pull request, this would cause me problems.

  1. Login to a concrete5 site.
  2. Go into the Dashboard, into a System & Settings page like Pretty URLs.
  3. Open an incognito window, and paste the Pretty URLs URL into the new window.
  4. You will have to login. Sign in. Everything will look fine. You'll have a dashboard session in the new window.
  5. Go into the Dashboard, everything is fine. Go to Dashboard > System & Settings > Pretty URLs, and you'll have to login. Look at Chrome developer console and you'll see that you have two sessions.

@tao-s
Copy link
Contributor Author

tao-s commented Jan 29, 2015

@aembler
If you install 2 concrete5 in other directory case:
http://www.example.com/a/
http://www.example.com/b/

  1. Set path to cookie config. ex: "cookie_path" => DIR_REL.'/'
  2. Access each site, login, dashboard...
  3. You can see cookie path "/a/" or "/b/", not 2 cookie.

see:
http://c57demo.xross-cube.net/a/
http://c57demo.xross-cube.net/b/

The original code made 2 cookie when this case. ( "/" and "/a/" or "/b/" ).
Because NativeSessionStorage::setOption() is not allow the original code "path".
"path" was be ignore.

Case original code:

  1. Log In to /a/.
  2. Access to /b/.
  3. Display error "session invalidated".

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

Successfully merging this pull request may close these issues.

None yet

2 participants