Pfad zu Cookie hinzufügen

Entwickler
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.

3 „Gefällt mir“
2

One addition. In this guide Use a subfolder (path prefix) to serve Discourse with 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?

4 „Gefällt mir“
3

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

2 „Gefällt mir“
4

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

4 „Gefällt mir“
5

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

2 „Gefällt mir“
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.

3 „Gefällt mir“
7

@neil

I have 2 discourse setups.

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

setup 2 running from Salesforce Einstein 1 Platform for Application Development - Salesforce.com US

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
4 „Gefällt mir“
8

Yeah we have a bug here:

https://github.com/discourse/discourse/blob/master/lib/auth/default_current_user_provider.rb#L157-L170

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?

3 „Gefällt mir“
9

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

10

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

1 „Gefällt mir“
11

Ok I am following the steps here to upgrade:

and will let you know soon

12

@neil damm man … it works !!

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

Very good … :penguin:

4 „Gefällt mir“
13

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

image.png1222×661 121 KB

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:

14

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

15

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

This error is a transient error …

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?

2 „Gefällt mir“
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

3 „Gefällt mir“
18

Related ?
https://meta.discourse.org/t/video-demo-of-oauth2-failing-intermitently/64835

19

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

20

Try deleting cookies, does the problem go away?