django-cookie-consent icon indicating copy to clipboard operation
django-cookie-consent copied to clipboard

conent in query selector and cookie attributes.

Open binoyudayan opened this issue 4 years ago • 6 comments

Hello, First of all, thank you so much for the package, it saved a lot of time for me.

I have found the following problem when using the cookies consent.

  1. .querySelector(".cc-cookie-decline", content) https://github.com/bmihelac/django-cookie-consent/blob/b2c88a7fabf557f39adc22340995c60dbecae54a/cookie_consent/static/cookie_consent/cookiebar.js#L33 Same issue in line 50 as well in the same file. Firefox showing a warning and working without any issue. But Chrome is throwing an error, so not setting the cookies. Working fine when changed to 'window.top' as suggested by Firefox console.

  2. set_cookies without 'lax' and secure attributes. Cookie “messages” will be soon rejected because it has the “sameSite” attribute set to “none” or an invalid value, without the “secure” attribute. To know more about the “sameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite Warning by Firefox. So I have to set directly on response

response.set_cookie(settings.COOKIE_CONSENT_NAME,
                                 dict_to_cookie_str(cookie_dic),
                                 settings.COOKIE_CONSENT_MAX_AGE,
                                 secure=settings.SESSION_COOKIE_SECURE,
                                 samesite='lax')

I think better to pass all option from settings if available from set_cookie_dict_to_response.

I'm not sure about the backward compatibility to make a pull request.

Firefox version : 77.0.1 64 bit
Chrome version: 83.04103.106 64bit
OS: Ubuntu 20.04
Django : 3.0.3

binoyudayan avatar Jun 24 '20 14:06 binoyudayan

hi @binoyudayan and thanks for time to write this report.

querySelector(".cc-cookie-decline", content) https://github.com/bmihelac/django-cookie-consent/blob/b2c88a7fabf557f39adc22340995c60dbecae54a/cookie_consent/static/cookie_consent/cookiebar.js#L33

I do not see warning / error in both chrome and firefox. Can you reproduce this errors in example app? https://github.com/bmihelac/django-cookie-consent#example-app

set_cookies without 'lax' and secure attributes. Cookie “messages” will be soon rejected because it has the “sameSite” attribute set to “none” or an invalid value, without the “secure” attribute. To know more about the “sameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite Warning by Firefox. So I have to set directly on response

I guess it would be a good addition to add COOKIE_SECURE and SAMESITE config options, using default values from SESSION_COOKIE_SAMESITE and SESSION_COOKIE_SECURE. This should be added to CookieConsentConf. Pull request would be awesome

bmihelac avatar Jun 24 '20 14:06 bmihelac

Hi @bmihelac , Thank you for the quick response. I could not reproduce the first issue in the test project. :(

I will make a pull request after going through the guidelines.

Thank you.

binoyudayan avatar Jun 24 '20 15:06 binoyudayan

@binoyudayan Still wondering if you can provide a PR even a few years later? If not, hoping someone could create some of the settings as suggested by bmihelac.

I am seeing the same warning message when I decline all cookies from the cookie bar in firefox. But I am using firefox 81.0.2. The warning does not show up for accepting all cookies from the cookie bar.

Warning points to this line in cookiebar.js: document.cookie = opts.cookie_decline;

some1ataplace avatar Mar 02 '22 04:03 some1ataplace

Actually I have fixed it as suggested by bmihelac. But the pull request was failed due to compatibility with python 2.7. I think i fixed that as well. But I lost track of this at some point(really sorry about that). I'll test again and make pull request. @some1ataplace Thank you for the reminder.

Update

Aalready made the pull requested.

binoyudayan avatar Mar 02 '22 11:03 binoyudayan

@binoyudayan You are awesome thank you so much I would have never figured that out myself!

some1ataplace avatar Mar 02 '22 13:03 some1ataplace

@binoyudayan - sorry to bother you again, is there any chance you could update your PR by rebasing on the latest master branch (or opening a new one)? The project setup has been updated so you get useful feedback from CI and writing/running tests should be a bit easier.

I'd love to get this into the next release of the package (which will be 0.4.0).

sergei-maertens avatar Sep 04 '22 21:09 sergei-maertens

Hi @sergei-maertens, Sorry for the late response. I already see the SECURE and SAMESITE in app conf. Should I still do this?

binoyudayan avatar Dec 09 '22 09:12 binoyudayan

Looking at the code in https://github.com/jazzband/django-cookie-consent/blob/39eec9363b6043a288871ecedc369701828436b5/cookie_consent/util.py#L30 this appears to have been implemented indeed, sorry for the confusion!

sergei-maertens avatar Apr 23 '23 13:04 sergei-maertens