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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use a GDPR-compliant configuration for Ahoy #5504

Merged
merged 5 commits into from May 13, 2024
Merged

Use a GDPR-compliant configuration for Ahoy #5504

merged 5 commits into from May 13, 2024

Conversation

javierm
Copy link
Member

@javierm javierm commented Apr 24, 2024 •
edited

References

Objectives

  • Mask the IPs collected with Ahoy
  • Store less cookies 馃槈
  • Update Ahoy initializer to be compatible with Ahoy 5.x
  • Remove unused visit_id column in the debates table

@javierm javierm added the security Pull requests that address a security vulnerability label Apr 24, 2024
@javierm javierm self-assigned this Apr 24, 2024
@javierm javierm added this to Doing in Consul Democracy Apr 24, 2024
@javierm javierm force-pushed the admin_stats_without_ahoy branch 3 times, most recently from 7e2a818 to e456094 Compare April 25, 2024 03:22
@javierm javierm force-pushed the admin_stats_without_ahoy branch 2 times, most recently from ea81e25 to aa63a8d Compare April 25, 2024 03:56
@javierm javierm force-pushed the remove_ahoy_cookies branch 2 times, most recently from 56a30d8 to 6c02347 Compare April 25, 2024 21:52
@javierm javierm changed the title Remove Ahoy cookies Use a GDPR-compliant configuration for Ahoy Apr 25, 2024
@javierm javierm moved this from Doing to Reviewing in Consul Democracy Apr 25, 2024
@javierm javierm marked this pull request as ready for review April 25, 2024 22:04
@taitus taitus self-assigned this May 8, 2024
@javierm javierm force-pushed the admin_stats_without_ahoy branch 2 times, most recently from f4613b1 to 8dd5c78 Compare May 8, 2024 19:21
@javierm javierm force-pushed the admin_stats_without_ahoy branch 3 times, most recently from bab98f9 to 3fcac3a Compare May 9, 2024 12:28
Base automatically changed from admin_stats_without_ahoy to master May 9, 2024 12:54
@javierm
Remove visit_id from the debates table
5df7b70 
This was added in commit 02f19aa, before we started tracking events.
I don't think we ever used it; in any case, we now use the `Ahoy::Chart`
class to deal with the stats Ahoy used to generate.
@javierm
Remove no longer necessary setting for Ahoy
2740a73 
Geocoding is disabled by default since Ahoy 4.0 [1].

[1] https://github.com/ankane/ahoy/tree/v4.2.1?tab=readme-ov-file#upgrading
@javierm
Update ensure_uuid method in Ahoy initializer
e9e4300 
It's possible to use the `Ahoy::Tracker::UUID_NAMESPACE` since Ahoy
2.1.0 [1].

[1] ankane/ahoy@44f7956bad
@javierm
Use a GDPR-compliant configuration for Ahoy
96ae69f 
As mentioned in Ahoy's README [1]:

> Ahoy provides a number of options to help with GDPR compliance.
> Update config/initializers/ahoy.rb with:
>
> class Ahoy::Store < Ahoy::DatabaseStore
>   def authenticate(data)
>     # disables automatic linking of visits and users
>   end
> end
>
> Ahoy.mask_ips = true
> Ahoy.cookies = :none

As also mentioned in the README:

> If Ahoy was installed before v5, add an index before making this
> change.
> (...)
> For Active Record, create a migration with:
> add_index :ahoy_visits, [:visitor_token, :started_at]

However, the `visitor_token` doesn't exist in our table, since we
generated the `visits` table when Ahoy used the `visitor_id` column. So
we're using this column for the index.

Note we also need to change the `visit` method, since otherwise we get
an exception [2]. As mentioned on the issue reporting the exception:

> you'll need to copy the latest version of that method and adapt it to
> your model. I believe you'll want to replace:
>
> where(visit_token: ahoy.visit_token) with
> where(id: ensure_uuid(ahoy.visit_token))
>
> where(visitor_token: ahoy.visitor_token) with
> where(visitor_id: ensure_uuid(ahoy.visitor_token))

So we're copying the latest version of that method and changing it
accordingly.

[1] https://github.com/ankane/ahoy/blob/v5.0.2/README.md
[2] Issue 549 in https://github.com/ankane/ahoy
Consul Democracy automation moved this from Reviewing to Testing May 13, 2024
@javierm
Add a task to mask existing IPs collected with Ahoy
144d1d8 
According to the README [1]:

> To mask previously collected IPs, use:
> Ahoy::Visit.find_each do |visit|
>   visit.update_column :ip, Ahoy.mask_ip(visit.ip)
> end

We're adapting the code with our version, since we use the `Visit` model
instead of the `Ahoy::Visit` model.

[1] https://github.com/ankane/ahoy/blob/v5.0.2/README.md#ip-masking
@javierm javierm merged commit 6074b91 into master May 13, 2024
13 checks passed
Consul Democracy automation moved this from Testing to Release 2.2.0 May 13, 2024
@javierm javierm deleted the remove_ahoy_cookies branch May 13, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions left review comments

@taitus taitus taitus approved these changes

Assignees

@taitus taitus

@javierm javierm

Labels
security Pull requests that address a security vulnerability
Projects
Consul Democracy
  
Release 2.2.0
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@javierm @taitus