I understand and fully support checking for forbidden tokens to prevent abuse and violations of Google Analytics' TOS. However false positives do occur. It would be nice to provide developers a way to prevent these checks. To give an example of common problems, we wanted to track the names of categories but were met with the following error:

The Custom variable value #1 is using the following forbidden tokens with personal identifying information: [node:field-article-type:name].
The Custom variable value #2 is using the following forbidden tokens with personal identifying information: [node:field-category-primary:name].
The Custom variable value #3 is using the following forbidden tokens with personal identifying information: [node:field-topic:name].

Comments

rickvug’s picture

Status: Active » Needs review
StatusFileSize
new1.38 KB

Attached is a patch that adds a variable to turn off checking. This variable is intended for developers who know what they are doing so no admin UI is provided to turn off checking.

Thoughts?

hass’s picture

Status: Needs review » Needs work

Hm, why not a parameter to the function. This would make it really conditionally and not global.

tobyontour’s picture

It's a laudable aim to have the tokens checked, and they all look like good ones to check, but :name rules out taxonomy:name which is a problem. Replacing it with "user:name" should be better.

tobyontour’s picture

Here's a patch that changes the token regex from ':name]' to 'user:name]'.

nicksanta’s picture

StatusFileSize
new472 bytes

Heres a patch which provides a drupal_alter hook to change the list of forbidden tokens. I think that's a better way to go as it allows developers to either expand on, or reduce that list.

hass’s picture

Sound like a good idea, but the default list should also be more fool prove.

If I see any module comming up that allows users to abuse the system (remove the list items), I will immediatly remove the line without any notice. Be warned.

hass’s picture

Per comment in http://drupal.org/node/1012190#comment-5787510 I will not let the alter mode in as this contributor has no understanding what personally identifying data is and we can expect intentional abuse.

hass’s picture

Status: Needs work » Postponed
cafuego’s picture

It would be trivial to expose that same information to GA via ones own differently named tokens, why would you even bother blacklisting a tiny subset?

Maybe a better solution is to display a warning about data privacy and a link to the GA terms of use on the modules settings page and leave it up to the user to decide what to do.

hass’s picture

The information is already there. Have you missed it, too?

I know that many do not unstand what personal identifying information is! We need to protect their users and stop these people at least to make them turn brain on and rethinking that they are doing illegal things. I know this is not 100% safe and you can crack it, but it requires a developer plus intent to break laws and TOS. We may block at least 98% of the installations from doing a big fault and breaking TOS and laws. This is good.

I will work on hardening the regex with every release, until we have the token feature implemented reliable. I'm sorry for everyone who is not able to use a token that may be blocked by the rules and who to not contain personal identifying information, but this is the price we need to pay for data protection. Data protection is more important than tracking data.

nicksanta’s picture

Status: Postponed » Needs review
StatusFileSize
new565 bytes

Honestly, I am completely fine with you continuing to implement this feature, as long as you provide a straight-forward way for developers to alter it's behaviour. That is what this issue is about.

You have already acknowledged that google's TOS could be easily circumvented with developer effort. So please commit this patch to make it convenient for those of us who just want to get our honest jobs done.

Rerolled patch attached.

hass’s picture

Status: Needs review » Postponed

@nicksanta: See #7 again. You are one of the people who seems not to understand what personal identifying information is.

nicksanta’s picture

You continue to miss the point, hass. This patch has every reason to be committed. I don't see what you are trying to achieve by blocking this patch.

Have you even read it?

And in regard to your opinion of my knowledge of "personably identifiable information", I still 100% believe that, and I will quote myself from the issue you linked to:

Drupal user accounts aren't necessarily people, nor do they necessarily contain personal information.

Drupal is a flexible framework with millions of websites and developers worldwide. Making an assertion about how EVERYONE uses user accounts is naive.

hass’s picture

As said a user account can and must be expected to be personal data. A user account is normally assigned to individuals to know who have created or edited content on a site. Other use cases are really edge cases and I remember that you still use it to identify a user, if the username contains fantasy names or not makes no real difference.

nicksanta’s picture

StatusFileSize
new15.19 KB
nicksanta’s picture

@hass: Log into any linux server, and type the command

cat /etc/passwd

Tell me how many of those accounts are real people.

jasonmce’s picture

Hass: I'm with you on wanting to protect user information from ignorant sysadmins (probably the 98% you mention above) haphazardly flipping switches, but I think you are only making it tougher on honest developers.

+1 for patch in #11 from @nicksanta

hass’s picture

Issue tags: +privacy
halefx’s picture

Just add a checkbox to each custom variable that says "Override TOS Warning" or something.

The Drupal accounts on one of my projects are shared by groups of 10-20 people. The names are just assigned numbers that don't identify any individuals, but we want to track the groups throughout the site. It would be just as easy to disable this check entirely as it was to remove the ":name" filter.

krlucas’s picture

@halefx group membership is personally identifiable information. Google Analytics forbids tracking IP addresses, for instance, even though IP addresses often only identify a (sometimes very large) group of users. Still, it's forbidden.

nicksanta’s picture

The point of this issue is that the module maintainers can not (and should not) make assumptions about every single drupal site using this module.

krlucas’s picture

Yeah, I know how that is. But I've read through the issue. And unfortunately, so far, no one has described a use case that didn't try to track personal information at least as granular as IP address. And whatever you think about @hass's abundance of caution, he's the maintainer so if he remains unpersuaded, that's that.

I believe #9 suggests a workaround that a developer should be able to implement. You can always fork this module and periodically rebase if you want to volunteer to maintain your own hook APIs. If you are correct and your implementation to allow personal information to be tracked has wide value in the community, I'm sure people will gravitate towards it in due time.

mikhailian’s picture

Well,

Here's my use case. I work on a news site where the only registered users are internal team members.

Today, I was asked to improve our editorial stats, so I added user names as a custom variable and got this "forbidden tokens with personal identifying information". It is surely not applicable to my case.

I am now rather annoyed that I have to keep yet another fork of yet another Drupal module just because someone believes they have the right to decide how I should comply to my country's laws.

nicksanta’s picture

Yep, absolutely spot on @mikhailian.

mikhailian’s picture

I was adding taxonomy term's name into the custom variables, and got the same message. With all due respect to the author of this otherwise great Drupal module… that check is just plain stupid.

andybill’s picture

another use case: a members only site where the members have explicitly consented to being tracked? Why should that site not be able to do exactly what their members are expecting them to do?

hass’s picture

This case is not about tracking. It's about personal data and breaking Google TOS.

krlucas’s picture

Whatever your users may have agreed to, you agree to the GA terms of sevice which prevents you from tracking user's personal data. GA defines personal information rather broadly to include the user's IP address, even though a single IP address could be shared by many individuals at an institution, company or in a household.

You'll have to find another service/method to do that kind of individual tracking for you and for your users if you don't want to violate your agreement with Google. Plenty of such services exist. Some I'm sure have Drupal modules, and all are as easy as GA is to add to your site via the theme layer.

nicksanta’s picture

The point of this issue is to provide a way for developers to bypass or disable the forbidden token checking.

You aren't saving anyone's personal data being tracked by rejecting this patch. You are simply making life difficult for developers with legitimate use cases.

hass’s picture

GA defines personal information rather broadly to include the user's IP address, even though a single IP address could be shared by many individuals at an institution, company or in a household.

An IP is seen as personal identifying information. It becomes more and more personal if IPv6 becomes standard. German link http://www.datenschutzbeauftragter-info.de/fachbeitraege/ip-adressen-per...

andybill’s picture

well, I found this post from a reference in a discussion about the Piwik module, so it obviously is not just about GA terms of service but a more wide-ranging area. (Btw a group of IP addresses shared by a thousand PCs behind an office NAT cannot conceivably be considered personally identifiable unless the person writing the definition is stupid, but that's a different area and not one for drupal.org)

Happy to move away from GA if their stance remains this way, will go back to looking at Piwik - provided the same silliness with filtering is removed from that module, as it is clearly not necessary in every use case and therefore it should be up to the site admin (and his / her legal team) to determine what is appropriate?

Oh, and as an aside:
"An IP address is a number assigned to your computer by your Internet Service Provider (ISP), so you can access the internet. It is generally considered to be non-personally identifiable information, because in most cases an IP address is dynamic (changing each time you connect to the internet), rather than static (unique to a particular user's computer)." (from UK's HMRC website, my emphasis).

hass’s picture

It's not your legal department that decide if a customer is trackable by ip or not. I convinced your opinion in past, but this is not reality any longer. That's also the reason for anonymize ip to remove the last octet.

With IPv6 the Deutsche Telekom (and others) already said you will always get the same ip if you disconnect and reconnect within a short timeframe. There are good chances that a dynamic ipv6 becomes like a static ip. The ipv6 privacy extensions are not solving this. You will always find some articles that say an ip is not personal information, but most people changed their mind in last years and now EU also says it is personal identifyable data. This includes UK.

andybill’s picture

I'm not trying - and am not interested - in tracking IP, I want to track the UID of members who have consented to it being tracked.
GA's T&Cs, a separate issue, obviously, but why is the blocking code from this module copied into Piwik when GA's T&Cs do not apply and it IS entirely a company's legal team that has to decide which legal framework applies - not the people writing the code or putting the site together. The customer decides - we can advise, and should - but it is their decision.

The point about tracking IPs still stands - if I've got a thousand PCs behind a handful of NATted IPv4 addresses (don't care about IPv6 at the moment), please introduce me to the person who drafted the rules that say they are each personally identifiable to GA.
On second thoughts, please don't.

krlucas’s picture

@hass I'm agreeing with you! :-)

@andybill Well tracking user id's ("UID") is clearly banned by the Google TOS and regardless of what your users have agreed to with you, you explicitly agreed (you had to have clicked the check box) to not track personally identifiable information.

I'm not familiar with the Piwik service and don't know why the GA module's code would affect it. But generally speaking, I think it's great you're exploring other services and modules because Google Analytics clearly does not allow your use case.

andybill’s picture

@krkucas - as I said, looked at GA T&Cs and clearly not appropriate as I implied.
The Piwik module code is impacted by this issue purely because the issue queue on Piwik frequently references GA module code - for example, https://drupal.org/node/1460616 - and the two should serve different user cases, one for a third party service, one for self-hosted analytics, very different things so they should not be treated as the same?

mikhailian’s picture

@hass, andybill,

I just carefully read the Google ToS to make sure I understand everything correctly. It still looks like my use case does not fit the interpretation of hass. Neither me nor my fellow colleagues are customers. We work for a legal entity and we publicly represent it, be it in articles or in the information we wilfully submit to Google Analytics.

P.S. And yes, I patch this module to track Drupal users and I acknowledge this publicly because I believe my use case is 100% legitimate.

seaneffel’s picture

A small injection here.

There are other cases of tokens turning up forbidden when there is actually no personal information transmitted. Taxonomy Term ID tokens turn up as forbidden for some reason.

https://drupal.org/node/2037595

eelkeblok’s picture

Issue summary: View changes

Just for the sake of completeness, I provided a patch over in https://www.drupal.org/node/2037595 that changes the validation error into a warning message. Strictly speaking, that patch probably belonged here, because the other issue started out about tweaking the regex triggering the validation error (https://www.drupal.org/node/2037595#comment-9129257). (If anyone thinks I should take that patch over to this issue, please let me know. It may be taking things too far to the other extreme, though, because the warning is only issued when the form is submitted, since it is triggered in the validation hook).

I too am all for protecting irresponsible sysadmins from themselves, but as it is, there are just too many legitimate use cases (taxonomy terms being one notable example) that cause false positives. I would argue that as long as the triggering mechanism isn't absolutely perfect (and quite honestly, I don't think it can ever be, it is just too fuzzy), it should not be a hard error. Also, as I said in the other issue, who's responsibility is it, in the end, that a site administrator adds user identifiable information to GA? The module's (or the module maintainer's)? Or the user's? I think it is fantastic that code is being written that is trying to protect naive sysadmins from themselves. However, if it is preventing people who know what they are doing from easily accomplishing their task, I think it's taken too far. If that also means that people who *think* they know what they are doing get themselves into trouble, despite automated warnings, isn't that *their* problem?

hass’s picture

It would be great if you stop moaning and just provide a fix for the regex and/or add a white-list regex. Than you do not need to whine about false positives.

A detected taxonomy term is a bug, not a feature.

eelkeblok’s picture

https://www.drupal.org/dcoc

All I'm saying is that as long as the detection isn't perfect, there should be some way of circumventing it for those that want to. I don't appear to be the only one.

hass’s picture

The process is to fix bugs in existing code and not implement workarounds for bugs. It looks like not many care about the issue or it would have been fixed.

robloach’s picture


/**
 * @file googleanalytics_allowforbiddentokens.module
 *
 * Allows all tokens to be used in the Google Analytics module.
 *
 * Requires: https://www.drupal.org/node/1307452#comment-6526582
 */

/**
 * Implementation of hook_googleanalytics_forbidden_tokens().
 *
 * @see https://www.drupal.org/node/1307452#comment-6526582
 */
function googleanalytics_allowforbiddentokens_googleanalytics_forbidden_tokens_alter(&$token_blacklist, &$token_string) {
  $token_blacklist = array(':forbidden]');
}

Use at your own risk. You've read the comments above in regards to security, privacy, and legalities behind it.

osopolar’s picture

In my opinion the google terms of services are just about the personal data of the visitor, which should not be tracked, which means personal data related to the user/user object. It should be fine for example to track the nodes author name, see example for custom dimensions in google analytics developer guide:

Custom dimensions and metrics are a powerful way to send custom data to Google Analytics. Web developers can use custom dimensions and metrics to segment and measure differences between logged in and logged out users, authors of pages, levels in games, or any other business data you have on a page.

osopolar’s picture

Status: Postponed » Closed (duplicate)
gellweiler’s picture


/**
 * @file googleanalytics_alltokens.module
 *
 * Allows all tokens to be used in the Google Analytics module.
 */

/**
 * Implemets hook_form_alter().
 */
function googleanalytics_alltokens_form_alter(&$form, &$form_state, $form_id)
{
  if ($form_id == 'googleanalytics_admin_settings_form') {
    foreach (array('googleanalytics_custom_dimension', 'googleanalytics_custom_metric') as $field) {
      foreach ($form[$field]['indexes'] as $i => $item) {
        $form[$field]['indexes'][$i]['value']['#element_validate'] =
          array_diff($item['value']['#element_validate'], array('googleanalytics_token_element_validate'));
      }
    }
  }
}