[DO NOT MERGE] Add Google Analytics 4 #1341
Conversation
|
Something went wrong with PR preview build please check |
|
Preview is available here: |
|
Preview is available here: |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Preview is available here: |
|
Preview is available here: |
|
Preview is available here: |
|
Preview is available here: |
There was a problem hiding this comment.
Can whether the GA code is enabled be configurable, so that when building to static and deploying inside quickstart it can be disabled? Or will the production check cover that?
There was a problem hiding this comment.
That's a good question, @leighmcculloch . @fnando, Should we add a flag to disable tracking in Quickstart? 🙏
There was a problem hiding this comment.
I think it makes sense making this configurable for the quickstart.
|
@quietbits Signed transaction XDR (containing full transaction details and signature) is being leaked in requests to Google such as below (in query parameters Because, as we browse through the lab application and use different features such as signing, simulate, view in XDR viewer etc., most of the data is passed through and added to URL query parameters, which is recorded by the Google Analytics plugin. For e.g. - the above request contains the following (in which contains signed XDR: I suppose this is not ideal, because leaking signed XDR to third parties can lead to abuse of functionality such as simulate, and front running transactions (as in other issue earlier). |
|
Even unsigned xdr would be best not to capture. Unsigned txs can contain signed auth entries within, or communicate an intent to take some action that might be non public information. |
|
@leighmcculloch Seems like you can't rely on excluding just certain URL paths for the integration, as the application currently appends parameters from the UI to URL progressively. So doesn't matter which URL path you are on, it can still contain XDR info. Unless there is a way to strip certain URL params from being included in tracking, the application will have to be changed to support this integration. |
|
Thanks for the feedback, @kanwalpreetd and @leighmcculloch! 🙌 We can't control what is submitted to Google Analytics 4 from the client. I think it grabs the whole URL, and then you decide what to keep on the dashboard side. We might need to ask our SEO masters to confirm this (@alexisdowney1 @Fastish 🙏 ). What if we moved away from storing everything in the URL? We mainly want this to allow sharing, but we could provide a button to create a shareable URL (we already do that for requests in API Explorer, for example). With this approach, we save the app's state in the browser's session storage (or local storage if we need to clear the store data manually). That way, we:
What are your thoughts? @janewang @sagpatil @jeesunikim 🙏 |
quietbits
changed the title
|
I know sharing URLs across different users is a key feature based on user interviews. If you create a URL and share it to me without URL params, I don't have the data stored in session local storage. How would I be able to load the URL? |
|
Also the CLI links back to Lab for signing passing data using the URL. That's an existing feature. In recent conversations, it appears that Lab needs a backend. Once a product grows to a point where it runs into the same root issue of not having a backend, a rational decision is to add the backend so the product can move forward. @sagpatil We have encountered this issue of not having a backend multiple times, a higher frequency as of late. What are your thoughts? |
|
The current limitations can't be fixed solely through GTM / GA4 but the only other additional thing I can do is |
@janewang , link sharing will still be available. We would just create the link on demand (users would need to click a button to get a link). When Lab loads, we would detect params in the URL and parse them, so they will be added to the session storage and removed from the URL. Generated shared URL example for "View XDR":
Lab URL after initial load (params stored in session storage):
We can think of ways to improve the UX if the generic approach is acceptable. I think it's just a matter of time before we run out of space to store state in the URL (most browsers allow max 2000 characters or so). |
Thanks, @alexisdowney1 ! Could we exclude all search params? Or keep only the ones we definitely need for tracking (for example, network). Like, we would exclude all params by default, but add only the ones we want to track. |
@quietbits +1. The approach you mentioned above (whitelisting params that can be excluded in tracking) is better for security. If we follow the blacklisting approach (excluding a list of params for tracking), that has the potential to expose sensitive params to be tracked in cases where we miss certain params, when a parameter name is changed, or a new sensitive param is added. |
|
Preview is available here: |
|
Preview is available here: |
|
Preview is available here: |
|
Still see the following request with leaked XDR (the XDR data is being leaked in both URL query parameters as well POST body to Google): |
|
Hi there, here Daniel from Hypersolid. I would recommend to make the solution to be hardcoded; So implement a <script async src="https://www.googletagmanager.com/gtag/js?id=G-XXXXXXX"></script> <script> window.dataLayer = window.dataLayer || []; function gtag(){ dataLayer.push(arguments); } gtag('js', new Date()); gtag('config', 'G-XXXXXXX', { {{variables}} }); </script> where we push the stripped URL as a page_location event with the full URL; we can then override the page-location send via GTM with this value and make sure the xdr is not send to GA |
|
@quietbits After reviewing the options discussed, I propose the following solution: - Option 1 Let's implement a custom page path redaction before sending data to GA4: // In src/metrics/GoogleAnalytics.tsx
import {
GoogleAnalytics as NextGoogleAnalytics,
GoogleTagManager,
} from "@next/third-parties/google";
import { usePathname, useSearchParams } from "next/navigation";
const GA_MEASUREMENT_ID = "GTM-KCNDDL3";
// Function to strip sensitive parameters from the URL
const stripSensitiveParams = (pathname, searchParams) => {
// Create a copy of search params to modify
const cleanParams = new URLSearchParams();
// Only copy safe parameters (whitelist approach)
// Example: Only keep "network" parameter
if (searchParams.has("network")) {
cleanParams.set("network", searchParams.get("network"));
}
// Return clean URL path + permitted parameters
return `${pathname}${cleanParams.toString() ? `?${cleanParams.toString()}` : ''}`;
};
export const GoogleAnalytics = () => {
const pathname = usePathname();
const searchParams = useSearchParams();
const isGoogleTrackingEnabled =
process.env.NEXT_PUBLIC_DISABLE_GOOGLE_ANALYTICS !== "true" &&
process.env.NODE_ENV === "production";
if (!isGoogleTrackingEnabled) {
return null;
}
// Create safe URL for analytics
const safePagePath = stripSensitiveParams(pathname, searchParams);
return (
<>
<GoogleTagManager gtmId={GA_MEASUREMENT_ID} />
<NextGoogleAnalytics
gaId={GA_MEASUREMENT_ID}
pageViewOptions={{
page_path: safePagePath,
}}
/>
</>
);
}; Option 2 Middleware We could implement a Next.js middleware solution to clean URLs for analytics purposes: Option 3 Comprehensive Session Storage Approach Move application state to session/local storage instead of keeping it in URLs Let me know what you think! |
|
Preview is available here: |
|
Preview is available here: |
Added Google Tag Manager and Google Analytics 4.