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.

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

Already on GitHub? Sign in to your account

chore: Masking user data in mixpanel session recordings #38267

Merged
merged 7 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/client/cypress/support/Pages/DataSources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export class DataSources {
option +
"']";
_queryTableResponse =
"//div[@data-guided-tour-id='query-table-response']//div[@class='tbody']//div[@class ='td']";
"//div[@data-guided-tour-id='query-table-response']//div[@class='tbody']//div[@class ='td mp-mask']";
_queryResponseHeader = (header: string) =>
"//div[@data-guided-tour-id='query-table-response']//div[@class='table']//div[@role ='columnheader']//span[text()='" +
header +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,11 @@ function Table(props: TableProps) {
{/* eslint-disable-next-line @typescript-eslint/no-explicit-any */}
{row.cells.map((cell: any, cellIndex: number) => {
return (
<div {...cell.getCellProps()} className="td" key={cellIndex}>
<div
{...cell.getCellProps()}
className="td mp-mask"
key={cellIndex}
>
<CellWrapper>{cell.render("Cell")}</CellWrapper>
</div>
);
Expand Down Expand Up @@ -344,7 +348,7 @@ function Table(props: TableProps) {
{headerGroups.map((headerGroup: any, index: number) => (
<div
{...headerGroup.getHeaderGroupProps()}
className="tr"
className="tr mp-mask"
key={index}
>
{headerGroup.headers.map(
Expand Down
8 changes: 8 additions & 0 deletions app/client/src/ce/entities/FeatureFlag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ export const FEATURE_FLAG = {
release_gs_all_sheets_options_enabled:
"release_gs_all_sheets_options_enabled",
ab_premium_datasources_view_enabled: "ab_premium_datasources_view_enabled",
kill_session_recordings_enabled: "kill_session_recordings_enabled",
config_mask_session_recordings_enabled:
"config_mask_session_recordings_enabled",
config_user_session_recordings_enabled:
"config_user_session_recordings_enabled",
} as const;

export type FeatureFlag = keyof typeof FEATURE_FLAG;
Expand Down Expand Up @@ -91,6 +96,9 @@ export const DEFAULT_FEATURE_FLAG_VALUE: FeatureFlags = {
release_table_html_column_type_enabled: false,
release_gs_all_sheets_options_enabled: false,
ab_premium_datasources_view_enabled: false,
kill_session_recordings_enabled: false,
config_user_session_recordings_enabled: true,
config_mask_session_recordings_enabled: true,
};

export const AB_TESTING_EVENT_KEYS = {
Expand Down
36 changes: 35 additions & 1 deletion app/client/src/ce/sagas/userSagas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ import type {
} from "reducers/uiReducers/usersReducer";
import { selectFeatureFlags } from "ee/selectors/featureFlagsSelectors";
import { getFromServerWhenNoPrefetchedResult } from "sagas/helper";
import type { SessionRecordingConfig } from "utils/Analytics/mixpanel";

export function* getCurrentUserSaga(action?: {
payload?: { userProfile?: ApiResponse };
Expand Down Expand Up @@ -107,9 +108,42 @@ export function* getCurrentUserSaga(action?: {
}
}

function* getSessionRecordingConfig() {
const featureFlags: FeatureFlags = yield select(selectFeatureFlags);

// This is a tenant level flag to kill session recordings
// If this is true, we do not do any session recordings
if (featureFlags.kill_session_recordings_enabled) {
return {
enabled: false,
mask: false,
};
}

// This is a user level flag to control session recordings for a user
// If this is false, we do not do any session recordings
if (!featureFlags.config_user_session_recordings_enabled) {
return {
enabled: false,
mask: false,
};
}

// Now we know that both tenant and user level flags are not blocking session recordings
return {
enabled: true,
// Check if we need to mask the session recordings from feature flags
mask: featureFlags.config_mask_session_recordings_enabled,
};
}

function* initTrackers(currentUser: User) {
try {
yield call(AnalyticsUtil.initialize, currentUser);
const sessionRecordingConfig: SessionRecordingConfig = yield call(
getSessionRecordingConfig,
);

yield call(AnalyticsUtil.initialize, currentUser, sessionRecordingConfig);
} catch (e) {
log.error(e);
}
Expand Down
11 changes: 8 additions & 3 deletions app/client/src/ce/utils/AnalyticsUtil.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import type { EventName } from "ee/utils/analyticsUtilTypes";
import type { EventProperties } from "@segment/analytics-next";

import SegmentSingleton from "utils/Analytics/segment";
import MixpanelSingleton from "utils/Analytics/mixpanel";
import MixpanelSingleton, {
type SessionRecordingConfig,
} from "utils/Analytics/mixpanel";
import SentryUtil from "utils/Analytics/sentry";
import SmartlookUtil from "utils/Analytics/smartlook";
import TrackedUser from "ee/utils/Analytics/trackedUser";
Expand All @@ -25,7 +27,10 @@ export enum AnalyticsEventType {
let blockErrorLogs = false;
let segmentAnalytics: SegmentSingleton | null = null;

async function initialize(user: User) {
async function initialize(
user: User,
sessionRecordingConfig: SessionRecordingConfig,
) {
SentryUtil.init();
await SmartlookUtil.init();

Expand All @@ -34,7 +39,7 @@ async function initialize(user: User) {
await segmentAnalytics.init();

// Mixpanel needs to be initialized after Segment
await MixpanelSingleton.getInstance().init();
await MixpanelSingleton.getInstance().init(sessionRecordingConfig);

// Identify the user after all services are initialized
await identifyUser(user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export function PeekOverlayPopUpContent(
>
{(dataType === "object" || dataType === "array") && jsData !== null && (
<JsonWrapper
className="mp-mask"
onClick={objectCollapseAnalytics}
style={{
minHeight: "20px",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ export function LogItem(props: LogItemProps) {
})}
{props.state && (
<JsonWrapper
className="t--debugger-log-state"
className="t--debugger-log-state mp-mask"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Additional sensitive data displays found in ErrorLogs that need masking

The LogCollapseData.tsx component also uses ReactJson to display state data but lacks the mp-mask class. Consider applying consistent masking across all debugger data displays.

  • app/client/src/components/editorComponents/Debugger/ErrorLogs/components/LogCollapseData.tsx: Add mp-mask class to the JsonWrapper component displaying state data
🔗 Analysis chain

LGTM! Comprehensive masking implementation

The mp-mask class is consistently applied across all data display elements, covering both JSON and text content.

Also applies to: 328-328, 337-337


Let me verify if the masking is consistently implemented across other debugger components.


Let me check if there are any other sensitive data displays in the debugger components that might need masking.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify consistent masking across debugger components
rg -A 2 "className.*debugger.*mp-mask"

Length of output: 1124


Script:

#!/bin/bash
# Search for other debugger components that might need masking
rg -l "className.*debugger" | grep -i "debug"

# Check for any existing mp-mask usage in debugger components
rg "mp-mask" $(rg -l "className.*debugger" | grep -i "debug")

Length of output: 2771


Script:

#!/bin/bash
# Search for JSON or data display components in debugger that might need masking
rg -A 2 "(JsonView|ReactJson|data-value|console\.log)" $(rg -l "className.*debugger" | grep -i "debug")

Length of output: 3035

onClick={(e) => e.stopPropagation()}
>
<ReactJson src={props.state} {...reactJsonProps} />
Expand All @@ -325,7 +325,7 @@ export function LogItem(props: LogItemProps) {
if (typeof logDatum === "object") {
return (
<JsonWrapper
className="t--debugger-console-log-data"
className="t--debugger-console-log-data mp-mask"
key={Math.random()}
onClick={(e) => e.stopPropagation()}
>
Expand All @@ -334,7 +334,7 @@ export function LogItem(props: LogItemProps) {
);
} else {
return (
<span className="debugger-label" key={Math.random()}>
<span className="debugger-label mp-mask" key={Math.random()}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hope you realize that this random key is your doing now :)

{`${logDatum} `}
</span>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ function ReadOnlyEditor(props: Props) {
isReadOnly: true,
isRawView: props.isRawView,
border: CodeEditorBorder.NONE,
className: "mp-mask",
};

return <LazyCodeEditor {...editorProps} />;
Expand Down
6 changes: 5 additions & 1 deletion app/client/src/pages/AppViewer/AppPage/AppPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ export function AppPage(props: AppPageProps) {
ref={pageViewWrapperRef}
sidebarWidth={sidebarWidth}
>
<PageView data-testid="t--app-viewer-page" width={width}>
<PageView
className="mp-mask"
data-testid="t--app-viewer-page"
width={width}
>
{widgetsStructure.widgetId &&
renderAppsmithCanvas(widgetsStructure as WidgetProps)}
</PageView>
Expand Down
2 changes: 1 addition & 1 deletion app/client/src/pages/Editor/Canvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const Canvas = (props: CanvasProps) => {
<Wrapper
$enableMainCanvasResizer={!!props.enableMainCanvasResizer}
background={isAnvilLayout ? "" : backgroundForCanvas}
className={`relative t--canvas-artboard ${paddingBottomClass} ${marginHorizontalClass} ${getViewportClassName(
className={`relative t--canvas-artboard mp-mask ${paddingBottomClass} ${marginHorizontalClass} ${getViewportClassName(
canvasWidth,
)}`}
data-testid={"t--canvas-artboard"}
Expand Down
17 changes: 16 additions & 1 deletion app/client/src/utils/Analytics/mixpanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ import { getAppsmithConfigs } from "ee/configs";
import SegmentSingleton from "./segment";
import type { ID } from "@segment/analytics-next";

export interface SessionRecordingConfig {
enabled: boolean;
mask: boolean;
}

class MixpanelSingleton {
private static instance: MixpanelSingleton;
private mixpanel: OverridedMixpanel | null = null;
Expand All @@ -17,13 +22,21 @@ class MixpanelSingleton {
}

// Segment needs to be initialized before Mixpanel
public async init(): Promise<boolean> {
public async init({
enabled,
mask,
}: SessionRecordingConfig): Promise<boolean> {
if (this.mixpanel) {
log.warn("Mixpanel is already initialized.");

return true;
}

// Do not initialize Mixpanel if session recording is disabled
if (!enabled) {
return false;
}

try {
const { default: loadedMixpanel } = await import("mixpanel-browser");
const { mixpanel } = getAppsmithConfigs();
Expand All @@ -32,6 +45,8 @@ class MixpanelSingleton {
this.mixpanel = loadedMixpanel;
this.mixpanel.init(mixpanel.apiKey, {
record_sessions_percent: 100,
record_block_selector: mask ? ".mp-block" : "",
record_mask_text_selector: mask ? ".mp-mask" : "",
});

await this.addSegmentMiddleware();
Expand Down
Loading