Fix GDPR cookie consent manager#1072
Open
BenjaminMichaelis wants to merge 3 commits intomainfrom
Open
Conversation
- Show consent banner to all visitors (remove unreliable timezone-based
region detection — VPN gaps, missing timezones, maintenance burden)
- Add CONSENT_VERSION with invalidation: bumping re-prompts all users
- Store _timestamp and _version in consent cookie for GDPR audit trail
- Always send Clarity denied signal on init for new visitors (was never
receiving a consent signal before user interaction)
- Fix personalization_storage bound to advertising, not analytics
- Add wait_for_update: 500 to gtag consent defaults (prevents GA pings
before banner renders)
- Fire gtag('config') unconditionally per Consent Mode v2 Advanced pattern
(GA handles denied state with cookieless modeled pings)
- Remove duplicate gtag('consent', 'default') from initGoogleConsentMode()
- Fix clearTrackingCookies() to delete domain-scoped GA/Clarity cookies
- Remove conflicting Clarity v1 API call from revokeAllConsent()
- Add Secure flag to consent cookie on HTTPS
- Add privacy policy link to consent banner
- Replace fragile setTimeout in openConsentPreferences() with direct call
- Prevent functionality_storage/security_storage from being user-overridden
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the site’s cookie consent implementation to improve GDPR compliance and correct Consent Mode v2 / Clarity signaling behavior, including moving Consent Mode “default” initialization into _Layout.cshtml (before gtag.js loads) and updating the client-side consent manager to show the banner globally and persist auditable consent state.
Changes:
- Remove timezone-based consent gating and show the consent banner to all visitors until valid consent is stored.
- Align Google Consent Mode v2 + GA initialization to the “default before gtag.js” pattern and avoid duplicate config/consent calls.
- Add consent versioning/audit metadata, improve cookie deletion behavior, and fix consent key mappings (e.g.,
personalization_storage).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| EssentialCSharp.Web/wwwroot/js/consent-manager.js | Updates consent persistence/versioning, Clarity updates, banner logic, and cookie clearing behavior. |
| EssentialCSharp.Web/Views/Shared/_Layout.cshtml | Sets Consent Mode default values before loading gtag.js and configures GA in a single place. |
Comment on lines
10
to
13
| this.COOKIE_DURATION = 365; // days | ||
| this.CONSENT_VERSION = '2'; // Bump this to re-prompt all users when consent terms change | ||
| this.GOOGLE_ANALYTICS_ID = options.googleAnalyticsId || 'G-761B4BMK2R'; | ||
| this.consentState = { |
Comment on lines
+265
to
+272
|
|
||
| // Notify layout scripts so they can fire gtag('config') on interactive consent | ||
| document.dispatchEvent(new CustomEvent('consentUpdated', { | ||
| detail: { | ||
| hasAnalyticsConsent: this.hasAnalyticsConsent(), | ||
| hasAdvertisingConsent: this.hasAdvertisingConsent() | ||
| } | ||
| })); |
Comment on lines
464
to
468
| // GA and Clarity cookies are often set on the root domain, so attempt deletion on both the | ||
| // exact hostname and the parent domain (e.g. .essentialcsharp.com) | ||
| const hostname = window.location.hostname; | ||
| const rootDomain = '.' + hostname.split('.').slice(-2).join('.'); | ||
| trackingCookies.forEach(cookieName => { |
Comment on lines
41
to
52
| hasAdvertisingConsent: this.hasAdvertisingConsent() | ||
| } | ||
| }); | ||
| document.dispatchEvent(event); |
Comment on lines
98
to
104
| shouldShowConsentBanner() { | ||
| // Show banner if in EEA/UK/Switzerland and no consent stored | ||
| return this.requiresConsent && !this.getCookie(this.COOKIE_NAME); | ||
| // Allow forcing the banner via URL param (useful for testing) | ||
| const urlParams = new URLSearchParams(window.location.search); | ||
| if (urlParams.get('testConsent') === 'true') return true; | ||
| // Show to all visitors who haven't given valid consent yet | ||
| return !this.getCookie(this.COOKIE_NAME); | ||
| } |
- Remove dead GOOGLE_ANALYTICS_ID option (gtag config is in _Layout.cshtml) - Remove dead consentManagerReady and consentUpdated event dispatches - Delete malformed consent cookie so banner re-shows on parse error - Fix clearTrackingCookies() to handle multi-part TLDs by iterating all parent domain suffixes instead of assuming a 2-label registrable domain
The ad_storage/ad_user_data/ad_personalization signals are Google Consent Mode v2 modeling signals, not for serving ads. The previous label and description were factually wrong for a site that serves no advertisements.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive overhaul of the cookie consent manager to fix GDPR compliance issues, bugs, and a broken Google Analytics integration.
Changes
Strategy change
Bug fixes
updateClarityConsent()was never called for first-time visitors (only on returning visitors and after interaction). Now called unconditionally ininit()so Clarity always receives the current consent state on page load.personalization_storagemis-grouped — was bound toanalyticsCheckedinsaveCustomPreferences(), should beadvertisingChecked.gtag('config')firing twice — removed duplicate call; now fires unconditionally once per Google Consent Mode v2 "Advanced" pattern (GA handles denied state with cookieless modeled pings).gtag('consent', 'default')— removed frominitGoogleConsentMode();_Layout.cshtmlis the single owner (required to fire beforegtag.jsloads).clearTrackingCookies()didn't clear domain-scoped cookies — now deletes on exact hostname, root domain, and no-domain for full GA/Clarity cookie coverage.revokeAllConsent()called conflicting Clarity v1 API — removedclarity('consent', false);consentv2handles it.Improvements
wait_for_update: 500added togtag('consent', 'default')— prevents GA from firing pings before the consent banner has a chance to render.CONSENT_VERSION = '2'— bumping this constant re-prompts all existing users. Stale cookies are invalidated on load._timestamp+_versionstored in consent cookie — GDPR Art. 7(1) audit trail.Secureflag on consent cookie (HTTPS only).functionality_storage/security_storageprotected — can no longer be overridden via cookie tampering.openConsentPreferences()setTimeout removed — direct synchronous call.Testing
Use
?testConsent=trueto force the banner regardless of existing consent cookie.