Skip to content

Commit 16f6280

Browse files
fix: address PR review comments - enduser.id timing, window global, consent double-dispatch
- Switch EnrichWithHttpRequest -> EnrichWithHttpResponse so enduser.id is set after authentication middleware has run and HttpContext.User is populated - Remove window.ecsAppInsights assignment; window.ecsGetAppInsights() already provides controlled first-party access without exposing authenticatedId to third-party scripts via window enumeration - Fix double ecs:consent-changed dispatch on returning-visitor page load: pass skipNotify:true when updateConsentMode() is called from loadConsentPreferences() so init()'s notifyConsentChanged() fires exactly once regardless
1 parent 595ee38 commit 16f6280

3 files changed

Lines changed: 9 additions & 6 deletions

File tree

EssentialCSharp.Web/Program.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,11 @@ private static void Main(string[] args)
6868
options.Filter = ctx =>
6969
!ctx.Request.Path.StartsWithSegments("/health")
7070
&& !ctx.Request.Path.StartsWithSegments("/alive");
71-
options.EnrichWithHttpRequest = (activity, request) =>
71+
// EnrichWithHttpResponse fires after the authentication middleware has run,
72+
// so HttpContext.User is populated and IsAuthenticated is reliable.
73+
options.EnrichWithHttpResponse = (activity, response) =>
7274
{
73-
var user = request.HttpContext.User;
75+
var user = response.HttpContext.User;
7476
if (user?.Identity?.IsAuthenticated != true)
7577
{
7678
return;

EssentialCSharp.Web/wwwroot/js/appinsights-manager.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,6 @@
149149
}
150150
if (!appInsights) {
151151
appInsights = createAppInsights();
152-
window.ecsAppInsights = appInsights;
153152
} else {
154153
appInsights.config.disableTelemetry = false;
155154
setAuthenticatedContext();

EssentialCSharp.Web/wwwroot/js/consent-manager.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class ConsentManager {
7575
});
7676

7777
this.consentState = { ...this.consentState, ...validatedPreferences };
78-
this.updateConsentMode();
78+
this.updateConsentMode({ skipNotify: true });
7979
} catch (e) {
8080
// Malformed cookie — delete it so the banner is shown again
8181
console.warn('Failed to parse consent preferences', e);
@@ -253,15 +253,17 @@ class ConsentManager {
253253
this.removeConsentBanner();
254254
}
255255

256-
updateConsentMode() {
256+
updateConsentMode({ skipNotify = false } = {}) {
257257
if (window.gtag) {
258258
try {
259259
window.gtag('consent', 'update', this.consentState);
260260
} catch (error) {
261261
console.warn('Failed to update Google Consent Mode:', error);
262262
}
263263
}
264-
this.notifyConsentChanged();
264+
if (!skipNotify) {
265+
this.notifyConsentChanged();
266+
}
265267
}
266268

267269
updateClarityConsent() {

0 commit comments

Comments
 (0)