Setting localstorage without consent on login page

Created on 20 September 2024, 7 months ago

Problem/Motivation

If Gin is used as admin theme and a visitor opens the login page, the key `Drupal.gin.darkmode` is written to the local storage of the browser.

Writing data to the browser needs consent (GDPR, in Germany additional TDDDG).

In most cases Gin is only used by the data controller or employees. In this case opening the login page without login write in local storage for any user. To avoid a consent only for this page please remove this use of localstorage.

Steps to reproduce

Install Gin as admin theme. Logout or use private browser tab and open the login page (/user).

Proposed resolution

Write to localstorage after login.

Remaining tasks

Write code.

User interface changes

Write to localstorage after login.

API changes

No.

Data model changes

No.

🐛 Bug report
Status

Active

Version

3.0

Component

User interface

Created by

🇩🇪Germany jan kellermann

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @jan kellermann
  • 🇨🇭Switzerland saschaeggi Zurich

    EU law requires you to use cookie banners if your website contains cookies that are not required for it to work (like analytics or tracking related cookies). As we don't store any tracking or analytical information I don't see why this would be affected by GDPR to be honest.

    Also I had a quick look at https://cookie-script.com/blog/local-storage-and-session-storage

    Note, that if you use local storage or session storage to track visitors, but do not show them in your cookie declaration report, you might be violating GDPR and could get a fine for the violation of privacy laws.

    The GDPR and ePrivacy Directive (the Cookie law) states, that if you use cookies or other tracking technologies to collect and manage data for purposes other than essential ones, such as sharing them with others or marketing, you need to obtain user consent. Users should be fully informed about what data you collect and store, for what reasons, how long, and other related information about their data. Users should have the opportunity to consent to this or to decline.

    I think that's the important thing, we don't track users nor do we share any data with 3rd parties – it's a integral (& crucial part as it would remove some functionality) part of the module. So I don't see how GDPR would apply here at all tbh.

    You can include a mention in your privacy policy that your application is storing a localStorage variable for your systems darkmode behavior if you want to be 100% sure.

  • 🇩🇪Germany jan kellermann

    The EU Directive 2002/58/EC (Directive on privacy and electronic communications) says:

    > Member States shall ensure that the storing of information, or the gaining of access to information already stored, in the terminal equipment of a subscriber or user is only allowed on condition that the subscriber or user concerned has given his or her consent, having been provided with clear and comprehensive information, in accordance with Directive 95/46/EC, inter alia, about the purposes of the processing. This shall not prevent any technical storage or access for the sole purpose of carrying out the transmission of a communication over an electronic communications network, or as strictly necessary in order for the provider of an information society service explicitly requested by the subscriber or user to provide the service.

    See https://eur-lex.europa.eu/legal-content/EN/TXT/?uri=CELEX%3A02002L0058-2...

    The implementation in Germany is the TDDDG, which enforces consent before changes are made to the browser (cookie, session storage, local storage).

    See §25: https://www.gesetze-im-internet.de/ttdsg/__25.html

  • Pipeline finished with Success
    7 months ago
    Total: 171s
    #288235
  • Status changed to Needs review 7 months ago
  • Status changed to Postponed: needs info 7 months ago
  • 🇨🇭Switzerland saschaeggi Zurich

    I'll move this to postponed for now as I still believe it doesn't apply here.

    Also as we can't really be sure where and how Gin is used – this proposal is to change things in Gin while the (potential) problem surfaces in Gin Login.

    It will also potentially completely break usage for those who use Gin in the frontend as well (yes that's also a thing) so we need to be super cautious here.

    I can also see that Drupal's Olivero also sets another localStorage by itself without permission Drupal.olivero.stickyHeaderState and it doesn't seem to be a problem so far.

    So we likely need an expert review here.

  • 🇩🇪Germany jan kellermann

    Because another theme is violating european law it is okay, if gin is doing this, too? I hoped we dont need to argue on this level.

    Gin is labeled as admin-theme and should be used for this. If it is a problem that people use gin as frontend theme and need the darlmode, then we need a change record that darkmode in frontend can only used after login. I can also add a config item
    for this, too. Then site admins can decicde to allow this localstorage item and add this to their consent tool.

    In the current situation gin in combination with gin_login violates european privacy law and the issue should not be postponed. Please set to need work and i can add an config item.

  • 🇨🇭Switzerland saschaeggi Zurich

    Because another theme is violating european law it is okay, if gin is doing this, too? I hoped we dont need to argue on this level.

    Again, I still believe we don't violating any law here, as it's a technical requirement and not data we gather. We just want to be sure if we're violating anything first before we act rashly and break a working feature for potentially hundreds or thousands of users.

    In the current situation gin in combination with gin_login violates european privacy law and the issue should not be postponed. Please set to need work and i can add an config item.

    As I mentioned before as a workaround – until we figured this out if this really applies here – would be that you can add it to your privacy policy that Gin sets Drupal.gin.darkmode which sets the browser default value of the user preferences a dark scheme or not.

    PS: I would like to remind you that in the Drupal community we respect each other, no need to be offensive here.

  • 🇦🇹Austria Grienauer Vienna

    The GDPR is about personal data.

    If you have a "darkmode" cookie that's set to "true" or "false", then there's nothing to worry about at all, and if you use a session token to only store that information server-side you should theoretically be safe too. but this should be asked a gdpr lawyer/expert.

    But it is save, to use a "technically necessary cookie" to store a dark/bright preference as this stores also no personal data at all.
    If we need a clear yes, no. I can ping a GDPR Lawyer I can ask this question. but I am 99.9% sure, this is save to store!

  • Pipeline finished with Success
    7 months ago
    Total: 228s
    #288737
  • Pipeline finished with Success
    7 months ago
    Total: 348s
    #288763
  • 🇩🇪Germany jan kellermann

    I added new commits:

    - The use of localStorage is replaced by the global variable window.ginDarkmode.
    - Added a config item for setting in localStorage never, always or on admin paths to avoid flickering.
    - Added tests.

    Please have a look.

    @saschaeggi: I apologize if my comment was hurtful. I don't feel taken seriously by the argument “But others do it too” hence my reaction. I hope we can now get back to the matter in hand. There is now an MR that adds the settings for the localStorage and otherwise retains the old logic, but uses the localStorage only optionally to prevent flickering.

    @saschaeggi @grienauer: As you can see in Art. 95 GDPR it relates to Directive 2002/58/EC. You're right that the directive is not the GDPR, but it is part of European data protection law - that's what I meant by the umbrella term “GDPR.” Therefore, the directive or the national implementation (in Germany the TDDDG) is valid law to which we must adhere. @grienauer it would be great if you could ask for another opinion. Our experience with data protection authorities is that the term “technically necessary” includes things like session cookies for logins, but not “convenience features” like dark mode.

    Thank you in advance.

  • 🇦🇹Austria legalwebio

    hi all!

    I'm a dpo from legalweb.io, responsible for the integrations of cms systems to our gdpr cloud solution. @grienauer asked me, if I could clearify this issue/question about the dark mode setting.

    There is no problem in case of gdpr setting such an information in local storage, or as a cookie.
    Such an "information" is or can be called "technically necessary".
    You don't use this kind of date for further processing of private data, or forward this information to a 3rd party.
    As explanation or example: if you would forward the statistcs about dark mode/normal mode usage to a 3rd party and if it would be possible to identify a single user to get his specific setting, a consent would be needed.

    The usage of this information is just for deciding which css should be loaded - the normal or the dark one, so no need to get a consent of the visitor in this case.

    kind regards,
    Matthias

  • 🇩🇪Germany jan kellermann

    Thank you, @legalwebio. We asked our external privacy experts and report then.

    I still have doubts that the procedure complies with §25 (2) TDDDG:

    Die Einwilligung nach Absatz 1 ist nicht erforderlich,
    wenn die Speicherung von Informationen in der Endeinrichtung des Endnutzers oder der Zugriff auf bereits in der Endeinrichtung des Endnutzers gespeicherte Informationen unbedingt erforderlich ist, damit der Anbieter eines digitalen Dienstes einen vom Nutzer ausdrücklich gewünschten digitalen Dienst zur Verfügung stellen kann.

    https://dsgvo-gesetz.de/tdddg/25-tdddg/

    In the TDDDG, it does not matter whether personal data is stored or not - it is about the access to the user's browser.

    Best regards

  • 🇩🇪Germany jan kellermann

    I discussed the issue directly with the data protection authority today. The authority's assessment is that consent is required if it is not absolutely technically necessary for the provision of the expressly requested digital service (e.g. session cookie after login). Consent can also be given indirectly (for dark mode, for example, when a button is pressed). In this case, writing to the localStorage is therefore not compliant.

    I will soon receive an opinion from an external data protection officer.

  • 🇩🇪Germany jan kellermann

    Our external data protection officer wrote:

    Data is to be written/read on a third-party computer, whereby I am not of the opinion that the dark mode status is “absolutely necessary” if it is only a matter of optical optimization compared to another solution option.

    The discussion partners have probably not understood that a personal reference is not important here (see paragraph 24 on page 9 of the data protection conference paper you linked to).

    I am also not convinced by the argument that you don't need a cookie banner because there are no cookies. After all, the law does not differentiate between cookies and local storage. Both are local storage.

    I share your view and your underlying argumentation in the discussion.

    @saschaeggi Please consider to change statue of this issue to "Needs review".

  • 🇩🇪Germany jan kellermann

    For people who want to make their site privacy compliant before the update, I am uploading the patch individually.

    The patch only blocks the setting of the localstorage on non-admin paths, e.g. on the login page when using gin_login.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    While I, personally, don't agree with the conclusion from the unknown and unnamed external data protection officer, I suggest trying to find a way where we don't require local storage in the first place. This is because I don't follow the logic why the situation should be different on admin and non-admin pages. And since the selected dark-mode is the only local storage item that we currently use, it's not worth continuing this discussion. I'm therefore also changing the title of the issue, as the reason for changing the methodology is not privacy, it's a change in direction in general.

  • 🇨🇭Switzerland saschaeggi Zurich

    Note that we need to set this to localStorage as otherwise there will be a flickering at every page load. So it's an essential piece where it doesn't work as indented without. We therefore can't move this to drupal settings.

    The question still remains if this violates any privacy law when we set exactly a binary value the browser offer us out of the box (user prefers dark scheme).

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Ah, my bad. Only in local storage is the value available early enough so that the pages can be rendered properly right away. Thanks for clarifying this.

    The question still remains if this violates any privacy law when we set exactly a binary value the browser offer us out of the box (user prefers dark scheme).

    This is certainly not a privacy issue. If anything, it's a question of storing data on the user's browser. There is regulation around that limits when something like that is allowed, but not for privacy reasons because that data is not used for tracking, and it is no PII.

    Let's see what @legalwebio has to say about that.

  • Pipeline finished with Failed
    7 months ago
    Total: 263s
    #295776
  • 🇩🇪Germany jan kellermann

    > I suggest trying to find a way where we don't require local storage in the first place.

    I added a new MR with uses a globar JS var instead of localstorage.

  • Pipeline finished with Success
    7 months ago
    Total: 209s
    #300788
  • Pipeline finished with Success
    7 months ago
    Total: 198s
    #300790
  • 🇩🇪Germany jan kellermann

    I have formulated the title in a solution-neutral way.

  • 🇨🇭Switzerland saschaeggi Zurich

    @jurgenhaas

    This is certainly not a privacy issue. If anything, it's a question of storing data on the user's browser. There is regulation around that limits when something like that is allowed, but not for privacy reasons because that data is not used for tracking, and it is no PII.

    Let's see what @legalwebio has to say about that.

    Do we know more about this?

    @jan kellermann

    I looked at MR !509 and left one question regarding potential issues with CSP inline script rules.

  • Pipeline finished with Success
    6 months ago
    #318737
  • Pipeline finished with Success
    6 months ago
    #318738
  • 🇩🇪Germany jan kellermann

    jan kellermann changed the visibility of the branch 3475773-setting-localstorage-without to hidden.

  • Pipeline finished with Success
    6 months ago
    #318745
  • 🇩🇪Germany jan kellermann

    Here is an actual example why storing data in user‘s browser needs consent:
    https://gdprhub.eu/index.php?title=AEPD_(Spain)_-_PS/00524/2023&mtc=today

  • 🇦🇹Austria legalwebio

    The article https://gdprhub.eu/index.php?title=AEPD_(Spain)_-_PS/00524/2023&mtc=today is about 3rd party cookies. It's correct, that 3rd party cookies are not allowed without a users consent.
    Your dark mode cookie will be a first party cookie, furthermore a binary value, not for tracking purposes.
    So you can't compare these.

    We are not aware of any process that has dealt with anything like this first party cookie with a binary value for technical purpose.
    What would probably cause less discussion (also from the thread): the default (=cookie not present) is light mode; if the user explicitly selects dark mode, the cookie is set.

    What is correctly presented in the thread is that setting a cookie and writing a value to local storage are of course the same thing on a more abstract level, namely storing data on the end device.

  • 🇩🇪Germany jan kellermann

    You didn't understand the judgment. It is not about the type of cookie, but about the fact that data was stored on the user's device without consent. See also Article 22.2 LSSI.

    The ePrivacy Directive says, that you need users’ consent before you use any cookies except strictly necessary cookies. This EU directive is not a direct law, but is implemented by national laws, in Germany the TDDDG and in Spain the LSSI.

    The new EU e-Privacy Directive is currently being drafted; this will replace the directive and will then become direct law in the same way as the GDPR. According to the current status, it contains an analogous passage, see Article 8 "Protection of information stored in and related to end-users’ terminal equipment".

    It is not about which data is processed, but about the protection of the visitor's end device - so it does not matter whether it is a tracking, 1st or 3rd party cookie.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    I have to admit that I'm a layman when it comes to legal topics. But following the above discussion seems to be boiling down to a simple question:

    Is storing a user preference for, e.g. dark or light mode, technically necessary or not?

    I can follow the argument that it is technically necessary because there is no other way to follow the user's preference.

  • 🇩🇪Germany jan kellermann

    You are right, the question is if the storage item is technically necessary.

    > I can follow the argument that it is technically necessary because there is no other way to follow the user's preference.

    The user has not made any preference at this point. Only the website operator's default setting is saved in the visitor's browser. That is the problem, that it is NOT a setting made by the user. Hence my approach of working without storage until the user makes a selection.

  • 🇨🇦Canada ambient.impact Toronto

    My original implementation of dark mode toggling was primarily CSS using @media (prefers-color-scheme: dark) {}, which not only would have avoided having to store anything by default, but also removed the additional problems with having to check storage with JavaScript, i.e. no chance of brief flashes of light mode unless you used an inline script which is a problem with CSP, etc. See #3161904-27: Auto Darkmode option for my concerns at the time.

  • 🇩🇪Germany jan kellermann

    Yes, I learned, too. Now we are using a json variable instead of inline-JS:

    
        $page['#attached']['html_head'][] = [
    +      [
    +        '#tag' => 'script',
    +        '#attributes' => [
    +          'type' => 'application/json',
    +          'id' => 'gin-darkmode',
    +        ],
    +        '#value' => new FormattableMarkup('{ "ginDarkmode": "@value" }', ['@value' => $settings->get('enable_darkmode')]),
    +      ],
    +      'gin_darkmode',
    +    ];
    
    

    And my CSP does not report anymore.

  • 🇨🇭Switzerland saschaeggi Zurich

    @jan

    And my CSP does not report anymore.

    The new solution looks promising (clever idea)! I'll look into it this week. I'll report back once I tested it (potentially needs some slight asjustments to Gin Toolbar as well). Thank you for working on this

    @ambient.impact

    My original implementation of dark mode toggling was primarily CSS using @media (prefers-color-scheme: dark) {}, which not only would have avoided having to store anything by default

    Both solutions had it's pro and cons (and still have). I still think the solution we went with is suited pretty well for the specific use case we have.

  • Assigned to jurgenhaas
  • Status changed to Needs review 5 months ago
  • 🇨🇭Switzerland saschaeggi Zurich

    @jan kellermann I finally found the time to test this. So far I couldn't find any issues.

    Can you have a final look at the MR – I renamed the ID and created the needed counterpart change for gin_toolbar here: 📌 Change inital darkmode setting Active

    Once that RTBC is in we can move ahead with this. Thank you again!

  • 🇩🇪Germany jan kellermann

    I installed from scratch (together with #3489495) and all works fine!

    Thank you very much.

  • 🇨🇭Switzerland saschaeggi Zurich

    Thanks everyone for their patience and inputs here. While I still would argue that this setting is essential, @jan put in quite some effort to find an alternative way which does not alter the way we handle things, thanks!

  • 🇨🇭Switzerland saschaeggi Zurich
  • 🇩🇪Germany jurgenhaas Gottmadingen

    I need to re-open this, as this gets all our pipelines to fail because of the following JS error in the browser's console:

    Uncaught TypeError: document.getElementById(...) is null
        <anonymous> https://mobr.dev.jurgenhaas.de/themes/contrib/gin/dist/js/init.js?snlxsd:5
        <anonymous> https://mobr.dev.jurgenhaas.de/themes/contrib/gin/dist/js/init.js?snlxsd:27
    

    That's caused by this line:

    localStorage.removeItem("GinSidebarOpen")), window.ginDarkmode = JSON.parse(document.getElementById("gin-setting-darkmode").textContent).ginDarkmode, 
    

    Looks like document.getElementById("gin-setting-darkmode") is NULL.

    Haven't found a quick fix for that. Could anyone look into this quickly, please?

  • 🇨🇭Switzerland saschaeggi Zurich

    @Jürgen 3.x or 4.x branch?

  • 🇩🇪Germany jurgenhaas Gottmadingen

    3.x branch

  • 🇩🇪Germany jurgenhaas Gottmadingen

    And only for authenticated users.

  • 🇨🇭Switzerland saschaeggi Zurich

    Maybe it's cache related?

  • 🇩🇪Germany jurgenhaas Gottmadingen

    I've tried that, it doesn't seem to be cache related.

  • 🇨🇭Switzerland saschaeggi Zurich

    You'll most likely need to update gin_toolbar to the latest dev as well\

    Potentially we want to add a fallback to the old value in storage if it's present or setting it to null

  • 🇩🇪Germany jan kellermann

    You need to update gin_toolbar, too.

    This patch adds only the element if the Gin Theme is active. But gin_toolbar loads this init-script, too - and for this the patch for gin_toolbar is needed: https://git.drupalcode.org/project/gin_toolbar/-/merge_requests/51/diffs

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Confirmed, thanks.

    Another approach could be to make that javascript code safer by checking for the existence first.

  • 🇨🇭Switzerland saschaeggi Zurich

    Another approach could be to make that javascript code safer by checking for the existence first.

    Yes, that's what I was after. We can add that in an additional MR here and merge it.

  • 🇩🇪Germany jan kellermann

    Created MR. Please check.

  • Pipeline finished with Success
    5 months ago
    Total: 210s
    #351979
  • Pipeline finished with Success
    5 months ago
    Total: 217s
    #351981
  • 🇨🇭Switzerland saschaeggi Zurich

    Left a small code suggestion otherwise LGTM 🤝

  • Pipeline finished with Success
    5 months ago
    Total: 226s
    #352609
  • 🇩🇪Germany jan kellermann

    I changed the code. Please review.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    MR!538 looks good to me. Thanks.

  • 🇨🇭Switzerland saschaeggi Zurich

    Commited to 3.x and 4.x, thanks!

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024