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

[ALL] Add support for Same-Site and secure Cookies #51065

Closed
yucer opened this issue May 12, 2020 · 8 comments
Closed

[ALL] Add support for Same-Site and secure Cookies #51065

yucer opened this issue May 12, 2020 · 8 comments
Labels
13.0 Framework General frontend/backend framework issues

Comments

@yucer
Copy link

yucer commented May 12, 2020

Impacted versions: all ?

Same-Site cookies were announced by Firefox and Google in the last years.

The browsers are actively warning that they will soon not accept unsecured cookies without the samesite parameter.

Werkzeug has already merged the changes relative to samesite and secure cookies.

The browsers can stop suddenly the support of the current cookies being used by Odoo, and the browsers versions is not a dependency that can be controlled by the release management. Such situation would be more stressful for the support team that adding the support previously.

Steps to reproduce: No action needed to reproduce it.

Current behavior: Warning appears without further action. They can be errors.

Expected behavior: No Warning. Secure cookies or Same-Site cookies should be used.

This call to "set_cookie":

odoo/odoo/http.py

Line 1401 in f96a17c

'session_id', httprequest.session.sid, max_age=90 * 24 * 60 * 60, httponly=True)
method should have one extra keyword parameter samesite. Valid values for Werkzeug are: 'Strict', 'Lax' or None. Also the secure parameter could be passed to prevent this issue.

Supposedly all the calls for other cookies should do something similar, but I guess none is more important that the session_id. There is also one explicit set_cookie for the session_id in the implementation of the web tours.

The pinpointed version used in Odoo 12.0 is Werkzeug 0.11.15 and it still has no support for the parameter. So upgrading the Werzeug for 12.0 might be mandatory once the samesite cookies are enforced by the browsers.

References:

@Yenthe666 Yenthe666 added 13.0 Website Framework General frontend/backend framework issues and removed Website labels May 12, 2020
@Yenthe666
Copy link
Collaborator

Thanks for the report @yucer! The SaaS team should be working on this actually :) Not sure who's responsible but it is known at Odoo anyways.

@mathben
Copy link

mathben commented Aug 11, 2020

I confirm this problem with Odoo 12.0

@amon-ra
Copy link

amon-ra commented Aug 13, 2020

I confirm this in Odoo 13.0.
Odoo 13.0 has werkzeug 0.14 and this has secure definition:

set_cookie(key, value='', max_age=None, expires=None, path='/', domain=None, secure=False, httponly=False, samesite=None)

@ghost
Copy link

ghost commented Sep 8, 2020

Due to this update of the chrome, PayUMoney Payment Acquire is also not working properly.
According to the There support team "In Chrome v.84 SameSite cookie attribute is released which if not handled by the server may lead to an issue causing loss of session data or session ID gets NULL. Merchants might experience a sudden surge of incomplete orders at their end."

I recently found that after placing an order with successful payment, Orders are not get confirmed.

@jsilvestar
Copy link

facing the same issue after setting to samesite=None, secure=False.

What could be the solution to this?

@Rai0090
Copy link

Rai0090 commented Sep 11, 2020

facing the same issue after setting to samesite=None, secure=False.

What could be the solution to this?

If you set sametime="None"(note: quotes) you HAVE to set secure=True otherwise the cookie is rejected. This is also not supported on werkzeug 0.14.1 which is what Odoo 13 used afaik.

You can either wait for Odoo to support Werkzeug 1.0 or alternatively set-up your server configuration to overwrite cookies

@mart-e
Copy link
Contributor

mart-e commented Oct 6, 2020

Hi!

secure=True by default is unfortunatly not an option for Odoo we provide a software that will be deployed in numerous different ways where it's not always an option to get HTTPS (local, offline, intranet,...).
Forcing a specific SameSite is also tricky as you don't really know how the user plans to use its odoo server (iframe anyone?).

The way to go is to actually set the right cookies on your web server (Nginx, Apache,...) based on your requirements.
If you have HTTPS deployed, you can force the Secure flag.
A bit of NGINX config hacking but nothing impossible.

We may set SameSite=Lax by default in master in the future as it's a good new default but don't plan to change it before. And we won't force to set the Secure flag (maybe in a future of HTTPS everywhere by default but not now).

@wahibimoh
Copy link

Hi!

secure=True by default is unfortunatly not an option for Odoo we provide a software that will be deployed in numerous different ways where it's not always an option to get HTTPS (local, offline, intranet,...). Forcing a specific SameSite is also tricky as you don't really know how the user plans to use its odoo server (iframe anyone?).

The way to go is to actually set the right cookies on your web server (Nginx, Apache,...) based on your requirements. If you have HTTPS deployed, you can force the Secure flag. A bit of NGINX config hacking but nothing impossible.

We may set SameSite=Lax by default in master in the future as it's a good new default but don't plan to change it before. And we won't force to set the Secure flag (maybe in a future of HTTPS everywhere by default but not now).

I propose that we do the following in odoo/http.py:

Change 'set_cookie' From:

    @functools.wraps(werkzeug.Response.set_cookie)
    def set_cookie(self, key, value='', max_age=None, expires=None, path='/', domain=None, secure=False, httponly=False, samesite=None, cookie_type='required'):
        if request.db and not request.env['ir.http']._is_allowed_cookie(cookie_type):
            expires = 0
            max_age = 0
        werkzeug.Response.set_cookie(self, key, value=value, max_age=max_age, expires=expires, path=path, domain=domain, secure=secure, httponly=httponly, samesite=samesite)

To:

    @functools.wraps(werkzeug.Response.set_cookie)
    def set_cookie(self, key, value='', max_age=None, expires=None, path='/', domain=None, secure=False, httponly=False, samesite=None, cookie_type='required'):
        if request.db and not request.env['ir.http']._is_allowed_cookie(cookie_type):
            expires = 0
            max_age = 0
	samesite_override = self.env['ir.config_parameter'].sudo().get_param('website.cookie_samesite_override')
        if samesite_override:	
		samesite = samesite_override
	secure_override = self.env['ir.config_parameter'].sudo().get_param('website.cookie_secure_override')
        if secure_override:	
		secure = secure_override	
        werkzeug.Response.set_cookie(self, key, value=value, max_age=max_age, expires=expires, path=path, domain=domain, secure=secure, httponly=httponly, samesite=samesite)

This way, the default behavior will be as is today, with giving an option to Saas user an option to change it in system parameters if they need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
13.0 Framework General frontend/backend framework issues
Projects
None yet
Development

No branches or pull requests

8 participants