[18.0][FIX] dbs value has to be kept in case of special proxy config

[cvinh]

this commit has been merged for v14 and v16 but still pending for v17

[StefanRijnhart]

Point of order: please include the module name in the commit and PR title, as in [FIX] dbfilter_from_header: keep dbs value in case of special proxy config.

On the 17.0 PR (#3261), someone mentions that this causes a regression wrt their combined use of both dbfilter and dbfilter_from_header. We need to discuss the pros and cons of each approach. To help with this, @cvinh can you add a test case to illustrate what issue this change solves for you?

[hbrunn]

see see #3694 (review)

[ivilata]

I'm copying @hbrunn's comment here since I retracted PR #3694 (which was a duplicate of this one, sorry!):

reexamining this, it's actually quite a radical change that in any case makes

Please keep in mind that the standard odoo dbfilter configuration is still applied before looking at the regular expression in the header.

from the readme false. So please update that too. And stress that it's super relevant to set the header everywhere.

Without this patch, you can set dbfilter = ^db1|db2|db3$ and be sure this Odoo instance will only ever be able to access those three dbs. With the patch, whenever one has access to the unproxied instance or one that doesn't set the header, you can pass whatever other database that's accessible to the database user, which might be an issue for some deployments.

So actually, I'm not sure any more I agree with this patch.

[ivilata]

Thanks @hbrunn for the comments, I see that there's more to the original 14.0 & 16.0s PR than it seems and a discussion is in point (as @StefanRijnhart suggests), even if they were already merged.

The PR allows something like this:

• dbfilter = %d
• For (www.)example.com: X-Odoo-dbfilter: ^admin$
• For *.example.com: X-Odoo-dbfilter is not set/configured

This simple configuration allows one to have admin.example.com serve the admin Odoo instance, other1.example.com serve another other1 one, other2.example.com another other2 one… while the admin instance also serves the public (www.)example.com website, without forcing a renaming of the admin db to example. Without the PR, one would need to set dbfilter = .* (or ^admin|other1|other2|…) then X-Odoo-dbfilter for each and every Odoo instance/db, which may be a lot more configuration (at least in NginX since the presence of whatever proxy_set_header in a proxied location dumps that directive in upper blocks like server or global HTTP).

I see @hbrunn's point that this may expose the database listing for sites where X-Odoo-dbfilter is not set if the user adds a properly crafted X-Odoo-dbfilter header in their requests. To counter that case, the proxy may be configured to remove the header from all incoming requests (in NginX, one may set more_clear_input_headers "x-odoo-dbfilter"; of the "Headers More" module in fthe http block).

My suggestion here would be to add another PR, to version 16.0 as a start, that fixes the readme sentence quoted above to (i) indicate that the header may override the dbfilter setting and (ii) warn about the issue with user-provided header and hint on how to fix it at the proxy level. Then, when that PR is merged, oca-port both #2828 and that PR to newer Odoo versions.

If the approach seems ok, I can create the PR myself. Thanks!

[hbrunn]

no, as original author of this module I say it should work exactly as the readme says and does in all versions except 14 and 16. Security by default is a good thing.

[ivilata]

Fair enough. I guess that this module may not be a good match for having an existing instance (coexisting with others on the same domain) serve a public website in the end. 🙂

Thanks @hbrunn for taking the time to discuss this!