From 8708b67b03f02086b2e93494b3d7a7f73b0e199d Mon Sep 17 00:00:00 2001 From: Stacey Levine Date: Thu, 9 Jul 2026 10:43:00 -0400 Subject: [PATCH 1/2] fix: fully honor block-anonymous-tracking flag on client and server The configure_block_event_tracking_for_anonymous_users flag left two gaps that let anonymous-user events through when it was on: - Client (shouldTrackUser): an active license short-circuited the flag, so licensed EE/cloud instances always tracked anonymous users. Removed the licenseActive bypass so the flag is honored on all instances. - Server (AnalyticsServiceCEImpl.sendEvent): the flag was only checked in sendObjectEvent. Direct sendEvent callers passing an anonymous userId bypassed it. Added a gate on the public sendEvent that blocks anonymous userIds before the x-anonymous-user-id is resolved, keeping the existing sendObjectEvent check for the session-user (signup) path. sendObjectEvent now routes through the extracted private sendEventInternal to avoid a redundant (Redis-backed) flag check on the hot anonymous path. usage-pulse billing/seat metering is intentionally out of scope. Adds unit tests: client shouldTrackUser matrix (incl. the license-bypass regression guard) and a server sendEvent anonymous-block guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/client/src/ce/sagas/userSagas.test.ts | 47 +++++++++++++++++++ app/client/src/ce/sagas/userSagas.tsx | 11 ++--- .../services/ce/AnalyticsServiceCEImpl.java | 28 +++++++++-- .../ce/AnalyticsServiceCEImplTest.java | 32 +++++++++++++ 4 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 app/client/src/ce/sagas/userSagas.test.ts diff --git a/app/client/src/ce/sagas/userSagas.test.ts b/app/client/src/ce/sagas/userSagas.test.ts new file mode 100644 index 000000000000..1bf8037f2e8e --- /dev/null +++ b/app/client/src/ce/sagas/userSagas.test.ts @@ -0,0 +1,47 @@ +import type { User } from "constants/userConstants"; +import { shouldTrackUser } from "ee/sagas/userSagas"; + +const makeUser = (overrides: Partial): User => + ({ + isAnonymous: false, + username: "user@example.com", + ...overrides, + }) as User; + +describe("shouldTrackUser", () => { + it("tracks a non-anonymous user regardless of the block flag", () => { + const user = makeUser({ isAnonymous: false, username: "user@example.com" }); + + expect(shouldTrackUser(user, false)).toBe(true); + expect(shouldTrackUser(user, true)).toBe(true); + }); + + it("tracks an anonymous user when telemetry is on and the flag is off", () => { + const user = makeUser({ isAnonymous: true, enableTelemetry: true }); + + expect(shouldTrackUser(user, false)).toBe(true); + }); + + it("does not track an anonymous user when the block flag is on (even with telemetry on)", () => { + // Regression guard: previously an active license bypassed the flag here. + const user = makeUser({ isAnonymous: true, enableTelemetry: true }); + + expect(shouldTrackUser(user, true)).toBe(false); + }); + + it("does not track an anonymous user when telemetry is off", () => { + const user = makeUser({ isAnonymous: true, enableTelemetry: false }); + + expect(shouldTrackUser(user, false)).toBe(false); + }); + + it("treats a user named anonymousUser as anonymous", () => { + const user = makeUser({ + isAnonymous: false, + username: "anonymousUser", + enableTelemetry: true, + }); + + expect(shouldTrackUser(user, true)).toBe(false); + }); +}); diff --git a/app/client/src/ce/sagas/userSagas.tsx b/app/client/src/ce/sagas/userSagas.tsx index 619f0d309898..5298b8b29eb8 100644 --- a/app/client/src/ce/sagas/userSagas.tsx +++ b/app/client/src/ce/sagas/userSagas.tsx @@ -89,7 +89,6 @@ import { segmentInitUncertain, } from "actions/analyticsActions"; import { getSegmentState } from "selectors/analyticsSelectors"; -import { getOrganizationConfig } from "ee/selectors/organizationSelectors"; export function* getCurrentUserSaga(action?: { payload?: { userProfile?: ApiResponse }; @@ -152,9 +151,8 @@ function* getSessionRecordingConfig() { }; } -function shouldTrackUser( +export function shouldTrackUser( currentUser: User, - licenseActive: boolean, featureFlag: boolean, ): boolean { try { @@ -167,7 +165,10 @@ function shouldTrackUser( const telemetryOn = currentUser?.enableTelemetry ?? false; - return isAnonymous && (licenseActive || (telemetryOn && !featureFlag)); + // When the block-anonymous-tracking flag is on, never track anonymous + // users — including on licensed instances. Otherwise, track only if + // telemetry is enabled. + return telemetryOn && !featureFlag; } catch (error) { return true; } @@ -186,11 +187,9 @@ function* initTrackers(currentUser: User): SagaIterator { ); const featureFlags: FeatureFlags = yield select(selectFeatureFlags); - const organizationConfig = yield select(getOrganizationConfig); const shouldTrack = shouldTrackUser( currentUser, - organizationConfig.license.active, featureFlags.configure_block_event_tracking_for_anonymous_users, ); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.java index e2cce557d020..87c0f7e69bf2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.java @@ -206,6 +206,26 @@ public Mono sendEvent(String event, String userId, Map properti return Mono.empty(); } + // If the event is for an anonymous user, respect the + // configure_block_event_tracking_for_anonymous_users feature flag. sendObjectEvent applies the same + // check upstream on the session user; gating here additionally covers direct sendEvent callers that + // pass an anonymous userId. + if (FieldName.ANONYMOUS_USER.equals(userId)) { + return featureFlagService + .check(FeatureFlagEnum.configure_block_event_tracking_for_anonymous_users) + .flatMap(isBlocked -> { + if (Boolean.TRUE.equals(isBlocked)) { + log.debug("Analytics event {} is not sent for anonymous user", event); + return Mono.empty(); + } + return sendEventInternal(event, userId, properties, hashUserId); + }); + } + + return sendEventInternal(event, userId, properties, hashUserId); + } + + private Mono sendEventInternal(String event, String userId, Map properties, boolean hashUserId) { // Can't update the properties directly as it's throwing ImmutableCollection error // java.lang.UnsupportedOperationException: null // at java.base/java.util.ImmutableCollections.uoe(ImmutableCollections.java) @@ -332,8 +352,8 @@ public Mono sendObjectEvent(AnalyticsEvents event, T object, Map { - if (isDisabled) { + .flatMap(isBlocked -> { + if (isBlocked) { log.debug("Analytics event {} is not sent for anonymous user", eventTag); return Mono.empty(); } else { @@ -388,7 +408,9 @@ public Mono sendObjectEvent(AnalyticsEvents event, T object, Map Date: Thu, 9 Jul 2026 11:04:14 -0400 Subject: [PATCH 2/2] review: fail closed on flag-check errors; align style; reuse ANONYMOUS_USERNAME Code-review follow-ups on the anonymous-tracking gate: - sendEvent's anonymous gate now handles featureFlagService.check() errors with onErrorResume, failing closed (event dropped, chain completes) instead of propagating the error into previously fire-and-forget callers like GlobalExceptionHandler. Adds a regression test for the error path. - Align gate style with repo convention: raw if (isBlocked) and plain Mono.empty() (Reactor forbids null onNext, so the defensive forms bought nothing). - Client: use the existing ANONYMOUS_USERNAME constant from userConstants instead of the hardcoded "anonymousUser" literal in shouldTrackUser and its test. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/client/src/ce/sagas/userSagas.test.ts | 3 ++- app/client/src/ce/sagas/userSagas.tsx | 3 ++- .../services/ce/AnalyticsServiceCEImpl.java | 13 +++++++++++-- .../ce/AnalyticsServiceCEImplTest.java | 18 ++++++++++++++++++ 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/app/client/src/ce/sagas/userSagas.test.ts b/app/client/src/ce/sagas/userSagas.test.ts index 1bf8037f2e8e..3d1af2ba36ad 100644 --- a/app/client/src/ce/sagas/userSagas.test.ts +++ b/app/client/src/ce/sagas/userSagas.test.ts @@ -1,4 +1,5 @@ import type { User } from "constants/userConstants"; +import { ANONYMOUS_USERNAME } from "constants/userConstants"; import { shouldTrackUser } from "ee/sagas/userSagas"; const makeUser = (overrides: Partial): User => @@ -38,7 +39,7 @@ describe("shouldTrackUser", () => { it("treats a user named anonymousUser as anonymous", () => { const user = makeUser({ isAnonymous: false, - username: "anonymousUser", + username: ANONYMOUS_USERNAME, enableTelemetry: true, }); diff --git a/app/client/src/ce/sagas/userSagas.tsx b/app/client/src/ce/sagas/userSagas.tsx index 5298b8b29eb8..b5f04700406d 100644 --- a/app/client/src/ce/sagas/userSagas.tsx +++ b/app/client/src/ce/sagas/userSagas.tsx @@ -50,6 +50,7 @@ import { import AnalyticsUtil from "ee/utils/AnalyticsUtil"; import { INVITE_USERS_TO_WORKSPACE_FORM } from "ee/constants/forms"; import type { User } from "constants/userConstants"; +import { ANONYMOUS_USERNAME } from "constants/userConstants"; import { flushErrorsAndRedirect, safeCrashAppRequest, @@ -157,7 +158,7 @@ export function shouldTrackUser( ): boolean { try { const isAnonymous = - currentUser?.isAnonymous || currentUser?.username === "anonymousUser"; + currentUser?.isAnonymous || currentUser?.username === ANONYMOUS_USERNAME; if (!isAnonymous) { return true; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.java index 87c0f7e69bf2..1039286c9a6f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/AnalyticsServiceCEImpl.java @@ -213,10 +213,19 @@ public Mono sendEvent(String event, String userId, Map properti if (FieldName.ANONYMOUS_USER.equals(userId)) { return featureFlagService .check(FeatureFlagEnum.configure_block_event_tracking_for_anonymous_users) + // Fail closed: if the flag state can't be resolved, drop the anonymous event rather than + // erroring the caller's chain (analytics is fire-and-forget for direct callers). + .onErrorResume(error -> { + log.warn( + "Could not resolve the block-anonymous-tracking flag; dropping anonymous event {}", + event, + error); + return Mono.just(Boolean.TRUE); + }) .flatMap(isBlocked -> { - if (Boolean.TRUE.equals(isBlocked)) { + if (isBlocked) { log.debug("Analytics event {} is not sent for anonymous user", event); - return Mono.empty(); + return Mono.empty(); } return sendEventInternal(event, userId, properties, hashUserId); }); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/AnalyticsServiceCEImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/AnalyticsServiceCEImplTest.java index 3ccf19b2db05..0f9919deeeb3 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/AnalyticsServiceCEImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/AnalyticsServiceCEImplTest.java @@ -57,4 +57,22 @@ void sendEvent_anonymousUserWithBlockFlagOn_doesNotEnqueueEvent() { verify(analytics, never()).enqueue(any()); } + + // If the flag state can't be resolved, the gate must fail closed: complete without error (so + // fire-and-forget callers' chains don't break) and drop the anonymous event. + @Test + void sendEvent_anonymousUserWhenFlagCheckErrors_completesWithoutEnqueueing() { + Analytics analytics = mock(Analytics.class); + FeatureFlagService featureFlagService = mock(FeatureFlagService.class); + when(featureFlagService.check(FeatureFlagEnum.configure_block_event_tracking_for_anonymous_users)) + .thenReturn(Mono.error(new RuntimeException("flag service unavailable"))); + + AnalyticsServiceCEImpl analyticsService = + new AnalyticsServiceCEImpl(analytics, null, null, null, null, null, null, null, featureFlagService); + + StepVerifier.create(analyticsService.sendEvent("execute_ACTION_TRIGGERED", "anonymousUser", Map.of("id", "x"))) + .verifyComplete(); + + verify(analytics, never()).enqueue(any()); + } }