Add path to cookie


(Mikko Koski) #1

When Discourse is hosted from a subfolder (e.g. /forum) it seems that the cookie is still set to the root folder /.

To me this sounds like a potential security issue. For example, if a forum is hosted in /forum and a WordPress blog is hosted in /blog, the cookie with Discourse session is sent also to the WordPress site.

This should be pretty easy to fix by changing config/initializer/100-session_store.rb from

Discourse::Application.config.session_store :cookie_store, key: '_forum_session'

to

Discourse::Application.config.session_store(
  :cookie_store,
  key: '_forum_session',
  path: (Rails.application.config.relative_url_root.nil?) ? '/' : Rails.application.config.relative_url_root
)

To my understanding this change shouldn’t invalidate existing sessions, because cookies for root folder / work also for subdirectory /forum. So the migration path should be ok.

Does this make sense? I could make a PR if you think this makes sense.


(Mikko Koski) #2

One addition. In this guide Discourse in a subfolder, multiple servers sharing a domain it says that

Note: This won’t work for serving multiple Discourse instances from different folders on the same domain. You need to use different subdomains so that each site can have different cookies.

So with this change you could remove that note. There’s no need for different subdomain if the cookies are scoped by the path. Or are there any other reasons why one couldn’t host two Discourse instances from different folders?


(Gerhard Schlager) #3

It would be nice if Discourse set the correct cookie path, but it’s definitely not a security issue.


(Jeff Atwood) #4

@neil we should look at doing this if it is not too risky. Feels risky to me for live sites though…


(Neil Lalonde) #5

Yes please submit a PR. :smile: I’ll test it out myself to see how it impacts live sites after a deploy.


(Neil Lalonde) #6

I tested the change and it’s safe. Sessions aren’t invalidated. I committed the change today.

Serving multiple Discourse instances from different paths on the same domain will most likely work now, but note that the instances can’t be served from one multisite setup. Multisite is still keyed off of domain, not path.


(Vikas Kedia) #7

@neil

I have 2 discourse setups.

setup 1 running from www.site.com/d1/

setup 2 running from www.site.com/d2/

Problems noticed

  1. I cannot remain logged into both the discourse setups at the same time
  2. If I am logged into d1 and I refresh d2 I am logged out of d1.

More information about the setup:

  1. Both are running in their own docker containers using their own DB
  2. installed using discourse_docker
  3. version is 1.9 beta2

(Sam Saffron) #8

Yeah we have a bug here:

We need to explicitly add the path there, just like session has the path.

Unfortunately fixing this will log everyone off on all subfolder installs, but still, it is probably worth fixing.

@neil maybe look at adding path there?


(Vikas Kedia) #9

@neil let me know once you commit the fix … I can check it out if it works …


(Neil Lalonde) #10

@vikaskedia I pushed a fix so please try it out.


(Vikas Kedia) #11

Ok I am following the steps here to upgrade:

and will let you know soon


(Vikas Kedia) #12

@neil damm man … it works !!

I am successfully logged into
www.site.com/d1/
and
www.site.com/d2/

Very good … :penguin:


(Vikas Kedia) #13

@neil I just started getting “internal server error 500” can this be related to the above fix …

The error auto corrects itself in a couple of minutes. So it seems to indicate it is the “rate limiting” issue.

But this is just me using the site and its a beefy server …

and i am seeing it for the first time after i upgraded. :grimacing:


(Neil Lalonde) #14

Looks like your subfolder is missing? “/subfolder/faq”


(Vikas Kedia) #15

If the subfolder was missing the error will come all the time.

This error is a transient error …


(Neil Lalonde) #16

It sounds like another problem we saw today. @sam seems like this code is being hit in this case too? The cookie is invalid now?


(Sam Saffron) #17

I tested on local and it tries to reset the cookie, I guess it is failing cause it can no longer reset the cookie from the top level path


(Vikas Kedia) #18

Related ?


(Vikas Kedia) #19

Let me know when you release a fix … I will check it out …


(Sam Saffron) #20

Try deleting cookies, does the problem go away?