Repository: solid/community-server (ts)

Issue:
Support X-Forwarded-* header in OriginalUrlExtractor
# Description
I added the following ACL file to my deployment to avoid that people put stuff in my server:

```
# Root ACL resource for the agent account
@prefix acl: <http://www.w3.org/ns/auth/acl#>.
@prefix foaf: <http://xmlns.com/foaf/0.1/>.

# The homepage is readable by the public
<#public>
    a acl:Authorization;
    acl:agentClass foaf:Agent;
    acl:accessTo <./>;
    acl:mode acl:Read.

# The owner has full access to every resource in their pod.
# Other agents have no access rights,
# unless specifically authorized in other .acl resources.
<#owner>
    a acl:Authorization;
    acl:agent <http://dylanvanassche.inrupt.net>;
    # Optional owner email, to be used for account recovery:
    acl:agent <mailto:me@dylanvanassche.be>;
    # Set the access to the root storage folder itself
    acl:accessTo <./>;
    # All resources will inherit this authorization, by default
    acl:default <./>;
    # The owner has all of the access modes allowed
    acl:mode
        acl:Read, acl:Write, acl:Control.
```

When I try to use Penny to view my data as a logged-in user, I get:

![image](https://user-images.githubusercontent.com/4999159/109665064-17bfd900-7b6e-11eb-80bb-a2532c89ab96.png)

And the following in my server logs:

```
2021-03-02T14:27:29.395Z [DPoPWebIdExtractor] warn: Error verifying WebID via DPoP-bound access token: Incorrect issuer https://inrupt.net/ for WebID https://dylanvanassche.inrupt.net/profile/card#me
```

# Steps to reproduce
1. Setup the SOLID Community Server
2. Push the ACL file with a HTTP PUT command
3. Open Penny, authenticate with your external SOLID WebID
4. Try to view your data by entering the server URL in the URL bar of Penny.

# Environment
- Community Server main branch 02/03/2021 (`e3cf2f9469fdeb7abf2916209be3443e8bee3d87`) at https://data.dylanvanassche.be
- Inrupt WebID provider: https://dylanvanassche.inrupt.net
- Armbian 21.02.2 Focal
- Odroid HC2 (armv7)
- Penny (https://penny.vincenttunru.com)

Pull request description:
Fallback to X-Forwarded-* headers
This uses the first value from `X-Forwarded-Host` and `X-Forwarded-Proto` if they're present and the standard `Forwarded` header is not.

Please let me know if I can implement this in a different way. I tried to follow existing patterns and limit the changes required.

Resolves #645

Patch:
diff --git a/src/ldp/UnsecureWebSocketsProtocol.ts b/src/ldp/UnsecureWebSocketsProtocol.ts
index 40741a270..5bfe5040b 100644
--- a/src/ldp/UnsecureWebSocketsProtocol.ts
+++ b/src/ldp/UnsecureWebSocketsProtocol.ts
@@ -44,7 +44,7 @@ class WebSocketListener extends EventEmitter {
     }
 
     // Store the HTTP host and protocol
-    const forwarded = parseForwarded(headers.forwarded);
+    const forwarded = parseForwarded(headers);
     this.host = forwarded.host ?? headers.host ?? 'localhost';
     this.protocol = forwarded.proto === 'https' || (socket as any).secure ? 'https:' : 'http:';
   }
diff --git a/src/ldp/http/OriginalUrlExtractor.ts b/src/ldp/http/OriginalUrlExtractor.ts
index ea36dde8c..facf4865a 100644
--- a/src/ldp/http/OriginalUrlExtractor.ts
+++ b/src/ldp/http/OriginalUrlExtractor.ts
@@ -21,17 +21,17 @@ export class OriginalUrlExtractor extends TargetExtractor {
       throw new Error('Missing URL');
     }
 
-    // Extract host and protocol (possibly overridden by the Forwarded header)
+    // Extract host and protocol (possibly overridden by the Forwarded/X-Forwarded-* header)
     let { host } = headers;
     let protocol = (connection as TLSSocket)?.encrypted ? 'https' : 'http';
-    if (headers.forwarded) {
-      const forwarded = parseForwarded(headers.forwarded);
-      if (forwarded.host) {
-        ({ host } = forwarded);
-      }
-      if (forwarded.proto) {
-        ({ proto: protocol } = forwarded);
-      }
+
+    // Check Forwarded/X-Forwarded-* headers
+    const forwarded = parseForwarded(headers);
+    if (forwarded.host) {
+      ({ host } = forwarded);
+    }
+    if (forwarded.proto) {
+      ({ proto: protocol } = forwarded);
     }
 
     // Perform a sanity check on the host
diff --git a/src/util/HeaderUtil.ts b/src/util/HeaderUtil.ts
index 7c9d47276..311c31baf 100644
--- a/src/util/HeaderUtil.ts
+++ b/src/util/HeaderUtil.ts
@@ -1,3 +1,4 @@
+import type { IncomingHttpHeaders } from 'http';
 import { getLoggerFor } from '../logging/LogUtil';
 import type { HttpResponse } from '../server/HttpResponse';
 import { BadRequestHttpError } from './errors/BadRequestHttpError';
@@ -429,21 +430,29 @@ export interface Forwarded {
 }
 
 /**
- * Parses a Forwarded header value.
+ * Parses a Forwarded header value and will fall back to X-Forwarded-* headers.
  *
- * @param value - The Forwarded header value.
+ * @param headers - The incoming HTTP headers.
  *
  * @returns The parsed Forwarded header.
  */
-export function parseForwarded(value = ''): Forwarded {
+export function parseForwarded(headers: IncomingHttpHeaders): Forwarded {
   const forwarded: Record<string, string> = {};
-  if (value) {
-    for (const pair of value.replace(/\s*,.*$/u, '').split(';')) {
+  if (headers.forwarded) {
+    for (const pair of headers.forwarded.replace(/\s*,.*/u, '').split(';')) {
       const components = /^(by|for|host|proto)=(.+)$/u.exec(pair);
       if (components) {
         forwarded[components[1]] = components[2];
       }
     }
+  } else {
+    const suffixes = [ 'host', 'proto' ];
+    for (const suffix of suffixes) {
+      const value = headers[`x-forwarded-${suffix}`] as string;
+      if (value) {
+        forwarded[suffix] = value.trim().replace(/\s*,.*/u, '');
+      }
+    }
   }
   return forwarded;
 }


Test patch:
diff --git a/test/unit/ldp/UnsecureWebSocketsProtocol.test.ts b/test/unit/ldp/UnsecureWebSocketsProtocol.test.ts
index 3fa182daa..8f241203a 100644
--- a/test/unit/ldp/UnsecureWebSocketsProtocol.test.ts
+++ b/test/unit/ldp/UnsecureWebSocketsProtocol.test.ts
@@ -182,4 +182,20 @@ describe('An UnsecureWebSocketsProtocol', (): void => {
     expect(webSocket.messages).toHaveLength(2);
     expect(webSocket.messages.pop()).toBe('ack https://other.example/protocol/foo');
   });
+
+  it('respects the X-Forwarded-* headers if Forwarded header is not present.', async(): Promise<void> => {
+    const webSocket = new DummySocket();
+    const upgradeRequest = {
+      headers: {
+        'x-forwarded-host': 'other.example',
+        'x-forwarded-proto': 'https',
+        'sec-websocket-protocol': 'solid-0.1',
+      },
+      socket: {},
+    } as any as HttpRequest;
+    await protocol.handle({ webSocket, upgradeRequest } as any);
+    webSocket.emit('message', 'sub https://other.example/protocol/foo');
+    expect(webSocket.messages).toHaveLength(2);
+    expect(webSocket.messages.pop()).toBe('ack https://other.example/protocol/foo');
+  });
 });
diff --git a/test/unit/ldp/http/OriginalUrlExtractor.test.ts b/test/unit/ldp/http/OriginalUrlExtractor.test.ts
index 83cdc664d..c1c6061ba 100644
--- a/test/unit/ldp/http/OriginalUrlExtractor.test.ts
+++ b/test/unit/ldp/http/OriginalUrlExtractor.test.ts
@@ -81,4 +81,53 @@ describe('A OriginalUrlExtractor', (): void => {
     await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
       .resolves.toEqual({ path: 'https://pod.example/foo/bar' });
   });
+
+  it('should fallback to x-fowarded-* headers.', async(): Promise<void> => {
+    const headers = {
+      host: 'test.com',
+      'x-forwarded-host': 'pod.example',
+      'x-forwarded-proto': 'https',
+    };
+    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+      .resolves.toEqual({ path: 'https://pod.example/foo/bar' });
+  });
+
+  it('should just take x-forwarded-host if provided.', async(): Promise<void> => {
+    const headers = {
+      host: 'test.com',
+      'x-forwarded-host': 'pod.example',
+    };
+    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+      .resolves.toEqual({ path: 'http://pod.example/foo/bar' });
+  });
+
+  it('should just take x-forwarded-protocol if provided.', async(): Promise<void> => {
+    const headers = {
+      host: 'test.com',
+      'x-forwarded-proto': 'https',
+    };
+    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+      .resolves.toEqual({ path: 'https://test.com/foo/bar' });
+  });
+
+  it('should prefer forwarded header to x-forwarded-* headers.', async(): Promise<void> => {
+    const headers = {
+      host: 'test.com',
+      forwarded: 'proto=http;host=pod.example',
+      'x-forwarded-proto': 'https',
+      'x-forwarded-host': 'anotherpod.example',
+    };
+    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+      .resolves.toEqual({ path: 'http://pod.example/foo/bar' });
+  });
+
+  it('should just take the first x-forwarded-* value.', async(): Promise<void> => {
+    const headers = {
+      host: 'test.com',
+      'x-forwarded-host': 'pod.example, another.domain',
+      'x-forwarded-proto': 'http,https',
+    };
+    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+      .resolves.toEqual({ path: 'http://pod.example/foo/bar' });
+  });
 });
diff --git a/test/unit/util/HeaderUtil.test.ts b/test/unit/util/HeaderUtil.test.ts
index f6884f5fc..3610d395e 100644
--- a/test/unit/util/HeaderUtil.test.ts
+++ b/test/unit/util/HeaderUtil.test.ts
@@ -188,16 +188,18 @@ describe('HeaderUtil', (): void => {
   });
 
   describe('#parseForwarded', (): void => {
-    it('parses an undefined value.', (): void => {
-      expect(parseForwarded()).toEqual({});
+    it('handles an empty set of headers.', (): void => {
+      expect(parseForwarded({})).toEqual({});
     });
 
-    it('parses an empty string.', (): void => {
-      expect(parseForwarded('')).toEqual({});
+    it('handles empty string values.', (): void => {
+      const headers = { forwarded: '', 'x-forwarded-host': '', 'x-forwarded-proto': '' };
+      expect(parseForwarded(headers)).toEqual({});
     });
 
     it('parses a Forwarded header value.', (): void => {
-      expect(parseForwarded('for=192.0.2.60;proto=http;by=203.0.113.43;host=example.org')).toEqual({
+      const headers = { forwarded: 'for=192.0.2.60;proto=http;by=203.0.113.43;host=example.org' };
+      expect(parseForwarded(headers)).toEqual({
         by: '203.0.113.43',
         for: '192.0.2.60',
         host: 'example.org',
@@ -206,15 +208,48 @@ describe('HeaderUtil', (): void => {
     });
 
     it('skips empty fields.', (): void => {
-      expect(parseForwarded('for=192.0.2.60;proto=;by=;host=')).toEqual({
+      const headers = { forwarded: 'for=192.0.2.60;proto=;by=;host=' };
+      expect(parseForwarded(headers)).toEqual({
         for: '192.0.2.60',
       });
     });
 
     it('takes only the first value into account.', (): void => {
-      expect(parseForwarded('host=pod.example, for=192.0.2.43, host=other')).toEqual({
+      const headers = { forwarded: 'host=pod.example, for=192.0.2.43, host=other' };
+      expect(parseForwarded(headers)).toEqual({
         host: 'pod.example',
       });
     });
+
+    it('should fall back to X-Forwarded-Host and X-Forwarded-Proto without Forward header.', (): void => {
+      const headers = { 'x-forwarded-host': 'pod.example', 'x-forwarded-proto': 'https' };
+      expect(parseForwarded(headers)).toEqual({
+        host: 'pod.example',
+        proto: 'https',
+      });
+    });
+
+    it('should prefer Forwarded to X-Forwarded-Host and X-Forwarded-Proto with Forward header.', (): void => {
+      const headers = {
+        forwarded: 'proto=http;host=pod.example',
+        'x-forwarded-host': 'anotherpod.example',
+        'x-forwarded-proto': 'https',
+      };
+      expect(parseForwarded(headers)).toEqual({
+        host: 'pod.example',
+        proto: 'http',
+      });
+    });
+
+    it('should properly handle multiple values with varying spaces for X-Forwarded-*.', (): void => {
+      const headers = {
+        'x-forwarded-host': ' pod.example ,192.0.2.60, 192.0.2.43',
+        'x-forwarded-proto': ' https ,http',
+      };
+      expect(parseForwarded(headers)).toEqual({
+        host: 'pod.example',
+        proto: 'https',
+      });
+    });
   });
 });