[server] Fix extraction of clientIp

Author: geropl

components/server/src/analytics.ts

@@ -8,6 +8,7 @@ import { Request } from "express";
 import { IAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
 import { SubscriptionService } from "@gitpod/gitpod-payment-endpoint/lib/accounting";
 import * as crypto from "crypto";
+import { clientIp } from "./express-util";
 
 export async function trackLogin(
     user: User,
@@ -18,7 +19,7 @@ export async function trackLogin(
 ) {
     // make new complete identify call for each login
     await fullIdentify(user, request, analytics, subscriptionService);
-    const ip = request.ips[0];
+    const ip = clientIp(request);
     const ua = request.headers["user-agent"];
 
     // track the login
@@ -35,7 +36,7 @@ export async function trackLogin(
 export async function trackSignup(user: User, request: Request, analytics: IAnalyticsWriter) {
     // make new complete identify call for each signup
     await fullIdentify(user, request, analytics);
-    const ip = request.ips[0];
+    const ip = clientIp(request);
     const ua = request.headers["user-agent"];
 
     // track the signup
@@ -79,7 +80,7 @@ async function fullIdentify(
 ) {
     // makes a full identify call for authenticated users
     const coords = request.get("x-glb-client-city-lat-long")?.split(", ");
-    const ip = request.ips[0];
+    const ip = clientIp(request);
     const ua = request.headers["user-agent"];
     var subscriptionIDs: string[] = [];
     const subscriptions = await subscriptionService?.getNotYetCancelledSubscriptions(user, new Date().toISOString());

components/server/src/express-util.ts

@@ -92,7 +92,7 @@ export function destroySession(session: session.Session): Promise<void> {
  * @returns fingerprint which is a hash over (potential) client ip (or just proxy ip) and User Agent
  */
 export function getRequestingClientInfo(req: express.Request) {
-    const ip = req.ips[0] || req.ip; // on PROD this should be a client IP address
+    const ip = clientIp(req);
     const ua = req.get("user-agent");
     const fingerprint = crypto.createHash("sha256").update(`${ip}–${ua}`).digest("hex");
     return { ua, fingerprint };
@@ -170,3 +170,13 @@ export const takeFirst = (h: string | string[] | undefined): string | undefined
     }
     return h;
 };
+
+export function clientIp(req: express.Request): string | undefined {
+    const forwardedFor = takeFirst(req.headers["x-forwarded-for"]);
+    if (!forwardedFor) {
+        return undefined;
+    }
+
+    // We now have a ,-separated string of IPs, where the first one is the (closest to) client IP
+    return forwardedFor.split(",")[0];
+}

components/server/src/websocket/websocket-connection-manager.ts

@@ -35,7 +35,7 @@ import {
     WithResourceAccessGuard,
     RepositoryResourceGuard,
 } from "../auth/resource-access";
-import { takeFirst } from "../express-util";
+import { clientIp, takeFirst } from "../express-util";
 import {
     increaseApiCallCounter,
     increaseApiConnectionClosedCounter,
@@ -47,6 +47,7 @@ import { GitpodServerImpl } from "../workspace/gitpod-server-impl";
 import * as opentracing from "opentracing";
 import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
 import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url";
+import { maskIp } from "../analytics";
 
 export type GitpodServiceFactory = () => GitpodServerImpl;
 
@@ -237,11 +238,15 @@ export class WebsocketConnectionManager implements ConnectionHandler {
         }
 
         const clientHeaderFields: ClientHeaderFields = {
-            ip: takeFirst(expressReq.ips),
+            ip: clientIp(expressReq),
             userAgent: expressReq.headers["user-agent"],
             dnt: takeFirst(expressReq.headers.dnt),
             clientRegion: takeFirst(expressReq.headers["x-glb-client-region"]),
         };
+        // TODO(gpl): remove once we validated the current approach works
+        log.debug("masked wss client IP", {
+            maskedClientIp: maskIp(clientHeaderFields.ip),
+        });
 
         gitpodServer.initialize(
             client,