From a9b4c7e3399778056e88d64df4216c59dd91d035 Mon Sep 17 00:00:00 2001 From: phamhieu Date: Thu, 20 Apr 2023 16:31:51 +0700 Subject: [PATCH 1/3] fix: page_view should use browser url --- studio/components/ui/PageTelemetry.tsx | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/studio/components/ui/PageTelemetry.tsx b/studio/components/ui/PageTelemetry.tsx index bc819a6d77a..144fc136350 100644 --- a/studio/components/ui/PageTelemetry.tsx +++ b/studio/components/ui/PageTelemetry.tsx @@ -10,9 +10,8 @@ const PageTelemetry: FC = ({ children }) => { const { ui } = useStore() useEffect(() => { - function handleRouteChange() { - // We want to send dynamic route path - handlePageTelemetry(router.route) + function handleRouteChange(url: string) { + handlePageTelemetry(url) } // Listen for page changes after a navigation or when the query changes @@ -24,14 +23,17 @@ const PageTelemetry: FC = ({ children }) => { useEffect(() => { // Send page telemetry on first page load - // We want to send dynamic route path - handlePageTelemetry(router.route) - }, []) + // Waiting for router ready before sending page_view + // if not the path will be dynamic route instead of the browser url + if (router.isReady) { + handlePageTelemetry(router.asPath) + } + }, [router.isReady]) /** * send page_view event * - * @param route: dynamic route path. Don't use the browser url + * @param route: the browser url * */ const handlePageTelemetry = async (route?: string) => { if (IS_PLATFORM) { From ec68b10672420bef583d6404de8efefe03c577cf Mon Sep 17 00:00:00 2001 From: phamhieu Date: Thu, 20 Apr 2023 17:13:44 +0700 Subject: [PATCH 2/3] fix: sanitize route before sending page_view event --- studio/components/ui/PageTelemetry.tsx | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/studio/components/ui/PageTelemetry.tsx b/studio/components/ui/PageTelemetry.tsx index 144fc136350..61ac1214ece 100644 --- a/studio/components/ui/PageTelemetry.tsx +++ b/studio/components/ui/PageTelemetry.tsx @@ -5,6 +5,18 @@ import { observer } from 'mobx-react-lite' import { useRouter } from 'next/router' import { FC, useEffect } from 'react' +function sanitizePageViewRoute(_route?: string) { + const hashSplits = _route?.split('#') + if (hashSplits && hashSplits?.length > 1) { + const urlParams = new URLSearchParams(hashSplits[1]) + if (urlParams?.get('access_token')) urlParams.set('access_token', 'xxxxx') + if (urlParams?.get('refresh_token')) urlParams.set('refresh_token', 'xxxxx') + if (urlParams?.get('token')) urlParams.set('token', 'xxxxx') + return urlParams?.toString() ?? _route + } + return _route +} + const PageTelemetry: FC = ({ children }) => { const router = useRouter() const { ui } = useStore() @@ -35,8 +47,11 @@ const PageTelemetry: FC = ({ children }) => { * * @param route: the browser url * */ - const handlePageTelemetry = async (route?: string) => { + const handlePageTelemetry = async (_route?: string) => { if (IS_PLATFORM) { + // filter out sensitive query params + const route = sanitizePageViewRoute(_route) + /** * Get referrer from browser */ From b630c81bb3410a01014dd3ea193248bd1936f711 Mon Sep 17 00:00:00 2001 From: phamhieu Date: Thu, 20 Apr 2023 18:35:41 +0700 Subject: [PATCH 3/3] fix: sanitizePageViewRoute to remove instead of replace --- studio/components/ui/PageTelemetry.tsx | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/studio/components/ui/PageTelemetry.tsx b/studio/components/ui/PageTelemetry.tsx index 61ac1214ece..412336e28d0 100644 --- a/studio/components/ui/PageTelemetry.tsx +++ b/studio/components/ui/PageTelemetry.tsx @@ -6,15 +6,21 @@ import { useRouter } from 'next/router' import { FC, useEffect } from 'react' function sanitizePageViewRoute(_route?: string) { - const hashSplits = _route?.split('#') - if (hashSplits && hashSplits?.length > 1) { - const urlParams = new URLSearchParams(hashSplits[1]) - if (urlParams?.get('access_token')) urlParams.set('access_token', 'xxxxx') - if (urlParams?.get('refresh_token')) urlParams.set('refresh_token', 'xxxxx') - if (urlParams?.get('token')) urlParams.set('token', 'xxxxx') - return urlParams?.toString() ?? _route + // remove all fragments + const noFragments = _route?.split('#')[0] + // remove sensitive params + const paramsSplits = noFragments?.split('?') + const hasParams = paramsSplits && paramsSplits?.length > 1 + + if (hasParams) { + const urlParams = new URLSearchParams(paramsSplits[1]) + const sensitiveKeys = [...urlParams.keys()].filter((x) => x.includes('token')) + const sensitiveParams = ['code', ...sensitiveKeys] + sensitiveParams.forEach((name) => urlParams.delete(name)) + return `${paramsSplits[0]}?${urlParams?.toString()}` } - return _route + + return noFragments } const PageTelemetry: FC = ({ children }) => {