change: Make credential extractors specialized.

config/presets/ldp/credentials-extractor.json

@@ -3,10 +3,18 @@
   "@graph": [
     {
       "@id": "urn:solid-server:default:CredentialsExtractor",
+      "@type": "FirstCompositeHandler",
+      "FirstCompositeHandler:_handlers": [
+        {
           "@type": "DPoPWebIdExtractor",
           "DPoPWebIdExtractor:_targetExtractor": {
             "@id": "urn:solid-server:default:TargetExtractor"
           }
+        },
+        {
+          "@type": "EmptyCredentialsExtractor"
+        }
+      ]
     }
   ]
 }

index.ts

@@ -2,6 +2,7 @@
 export * from './src/authentication/Credentials';
 export * from './src/authentication/CredentialsExtractor';
 export * from './src/authentication/DPoPWebIdExtractor';
+export * from './src/authentication/EmptyCredentialsExtractor';
 export * from './src/authentication/UnsecureWebIdExtractor';
 
 // Authorization

src/authentication/DPoPWebIdExtractor.ts

@@ -2,6 +2,8 @@ import { verify } from 'ts-dpop';
 import type { TargetExtractor } from '../ldp/http/TargetExtractor';
 import { getLoggerFor } from '../logging/LogUtil';
 import type { HttpRequest } from '../server/HttpRequest';
+import { BadRequestHttpError } from '../util/errors/BadRequestHttpError';
+import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
 import type { Credentials } from './Credentials';
 import { CredentialsExtractor } from './CredentialsExtractor';
 
@@ -17,20 +19,27 @@ export class DPoPWebIdExtractor extends CredentialsExtractor {
     this.targetExtractor = targetExtractor;
   }
 
+  public async canHandle({ headers }: HttpRequest): Promise<void> {
+    const { authorization } = headers;
+    if (!authorization || !authorization.startsWith('DPoP ')) {
+      throw new NotImplementedHttpError('No DPoP Authorization header specified.');
+    }
+  }
+
   public async handle(request: HttpRequest): Promise<Credentials> {
-    let webID: string | undefined;
-    const authorizationHeader = request.headers.authorization;
-    const dpopHeader = request.headers.dpop as string;
-    if (authorizationHeader && dpopHeader) {
-      const method = request.method as any;
+    const { headers: { authorization, dpop }, method } = request;
+    if (!dpop) {
+      throw new BadRequestHttpError('No DPoP token specified.');
+    }
     const resource = await this.targetExtractor.handleSafe(request);
     try {
-        webID = await verify(authorizationHeader, dpopHeader, method, resource.path);
-        this.logger.info(`Extracted WebID via DPoP token: ${webID}`);
-      } catch (error: unknown) {
-        this.logger.warn(`Error verifying WebID via DPoP token: ${(error as Error).message}`);
-      }
-    }
+      const webID = await verify(authorization as string, dpop as string, method as any, resource.path);
+      this.logger.info(`Verified WebID via DPoP token: ${webID}`);
       return { webID };
+    } catch (error: unknown) {
+      const message = `Error verifying WebID via DPoP token: ${(error as Error).message}`;
+      this.logger.warn(message);
+      throw new BadRequestHttpError(message);
+    }
   }
 }

src/authentication/EmptyCredentialsExtractor.ts

@@ -0,0 +1,20 @@
+import type { HttpRequest } from '../server/HttpRequest';
+import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
+import type { Credentials } from './Credentials';
+import { CredentialsExtractor } from './CredentialsExtractor';
+
+/**
+ * Extracts the empty credentials, indicating an unauthenticated agent.
+ */
+export class EmptyCredentialsExtractor extends CredentialsExtractor {
+  public async canHandle({ headers }: HttpRequest): Promise<void> {
+    const { authorization } = headers;
+    if (authorization) {
+      throw new NotImplementedHttpError('Unexpected Authorization scheme.');
+    }
+  }
+
+  public async handle(): Promise<Credentials> {
+    return {};
+  }
+}

src/authentication/UnsecureWebIdExtractor.ts

@@ -1,4 +1,6 @@
+import { getLoggerFor } from '../logging/LogUtil';
 import type { HttpRequest } from '../server/HttpRequest';
+import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
 import type { Credentials } from './Credentials';
 import { CredentialsExtractor } from './CredentialsExtractor';
 
@@ -6,7 +8,18 @@ import { CredentialsExtractor } from './CredentialsExtractor';
  * Credentials extractor which simply interprets the contents of the Authorization header as a webID.
  */
 export class UnsecureWebIdExtractor extends CredentialsExtractor {
-  public async handle(input: HttpRequest): Promise<Credentials> {
-    return { webID: input.headers.authorization };
+  protected readonly logger = getLoggerFor(this);
+
+  public async canHandle({ headers }: HttpRequest): Promise<void> {
+    const { authorization } = headers;
+    if (!authorization || !authorization.startsWith('WebID ')) {
+      throw new NotImplementedHttpError('No WebID Authorization header specified.');
+    }
+  }
+
+  public async handle({ headers }: HttpRequest): Promise<Credentials> {
+    const webID = /^WebID\s+(.*)/u.exec(headers.authorization as string)![1];
+    this.logger.info(`Agent unsecurely claims to be ${webID}`);
+    return { webID };
   }
 }

test/configs/AuthenticatedDataAccessorBasedConfig.ts

@@ -5,10 +5,10 @@ import type {
 } from '../../index';
 import {
   AuthenticatedLdpHandler,
+  EmptyCredentialsExtractor,
   FirstCompositeHandler,
   MethodPermissionsExtractor,
   RdfToQuadConverter,
-  UnsecureWebIdExtractor,
   QuadToRdfConverter,
 } from '../../index';
 import type { ServerConfig } from './ServerConfig';
@@ -43,7 +43,7 @@ export class AuthenticatedDataAccessorBasedConfig implements ServerConfig {
   public getHttpHandler(): HttpHandler {
     const requestParser = getBasicRequestParser();
 
-    const credentialsExtractor = new UnsecureWebIdExtractor();
+    const credentialsExtractor = new EmptyCredentialsExtractor();
     const permissionsExtractor = new FirstCompositeHandler([
       new MethodPermissionsExtractor(),
     ]);

test/configs/BasicConfig.ts

@@ -3,8 +3,8 @@ import type { HttpHandler,
 import {
   AllowEverythingAuthorizer,
   AuthenticatedLdpHandler,
+  EmptyCredentialsExtractor,
   MethodPermissionsExtractor,
-  UnsecureWebIdExtractor,
 } from '../../index';
 import type { ServerConfig } from './ServerConfig';
 import { getOperationHandler, getInMemoryResourceStore, getBasicRequestParser, getResponseWriter } from './Util';
@@ -26,7 +26,7 @@ export class BasicConfig implements ServerConfig {
   public getHttpHandler(): HttpHandler {
     const requestParser = getBasicRequestParser();
 
-    const credentialsExtractor = new UnsecureWebIdExtractor();
+    const credentialsExtractor = new EmptyCredentialsExtractor();
     const permissionsExtractor = new MethodPermissionsExtractor();
     const authorizer = new AllowEverythingAuthorizer();
 

test/configs/BasicHandlersConfig.ts

@@ -3,6 +3,7 @@ import type { HttpHandler,
 import {
   AllowEverythingAuthorizer,
   AuthenticatedLdpHandler,
+  EmptyCredentialsExtractor,
   FirstCompositeHandler,
   MethodPermissionsExtractor,
   QuadToRdfConverter,
@@ -10,7 +11,6 @@ import {
   RdfToQuadConverter,
   SparqlUpdateBodyParser,
   SparqlPatchPermissionsExtractor,
-  UnsecureWebIdExtractor,
 } from '../../index';
 
 import type { ServerConfig } from './ServerConfig';
@@ -47,7 +47,7 @@ export class BasicHandlersConfig implements ServerConfig {
       new RawBodyParser(),
     ]);
 
-    const credentialsExtractor = new UnsecureWebIdExtractor();
+    const credentialsExtractor = new EmptyCredentialsExtractor();
     const permissionsExtractor = new FirstCompositeHandler([
       new MethodPermissionsExtractor(),
       new SparqlPatchPermissionsExtractor(),

test/configs/BasicHandlersWithAclConfig.ts

@@ -2,10 +2,10 @@ import type { HttpHandler,
   ResourceStore } from '../../index';
 import {
   AuthenticatedLdpHandler,
+  EmptyCredentialsExtractor,
   FirstCompositeHandler,
   MethodPermissionsExtractor,
   RdfToQuadConverter,
-  UnsecureWebIdExtractor,
   QuadToRdfConverter,
 } from '../../index';
 import type { ServerConfig } from './ServerConfig';
@@ -39,7 +39,7 @@ export class BasicHandlersWithAclConfig implements ServerConfig {
   public getHttpHandler(): HttpHandler {
     const requestParser = getBasicRequestParser();
 
-    const credentialsExtractor = new UnsecureWebIdExtractor();
+    const credentialsExtractor = new EmptyCredentialsExtractor();
     const permissionsExtractor = new FirstCompositeHandler([
       new MethodPermissionsExtractor(),
     ]);

test/configs/DataAccessorBasedConfig.ts

@@ -6,12 +6,12 @@ import type {
 import {
   AllowEverythingAuthorizer,
   AuthenticatedLdpHandler,
+  EmptyCredentialsExtractor,
   FirstCompositeHandler,
   MethodPermissionsExtractor,
   QuadToRdfConverter,
   RawBodyParser,
   RdfToQuadConverter,
-  UnsecureWebIdExtractor,
 } from '../../index';
 import type { ServerConfig } from './ServerConfig';
 import {
@@ -43,7 +43,7 @@ export class DataAccessorBasedConfig implements ServerConfig {
     // This is for the sake of test coverage, as it could also be just getBasicRequestParser()
     const requestParser = getBasicRequestParser([ new RawBodyParser() ]);
 
-    const credentialsExtractor = new UnsecureWebIdExtractor();
+    const credentialsExtractor = new EmptyCredentialsExtractor();
     const permissionsExtractor = new FirstCompositeHandler([
       new MethodPermissionsExtractor(),
     ]);

test/unit/authentication/DPoPWebIdExtractor.test.ts

@@ -1,17 +1,12 @@
 import { verify } from 'ts-dpop';
 import { DPoPWebIdExtractor } from '../../../src/authentication/DPoPWebIdExtractor';
-import { TargetExtractor } from '../../../src/ldp/http/TargetExtractor';
-import type { ResourceIdentifier } from '../../../src/ldp/representation/ResourceIdentifier';
 import type { HttpRequest } from '../../../src/server/HttpRequest';
-
-class DummyTargetExtractor extends TargetExtractor {
-  public async handle(): Promise<ResourceIdentifier> {
-    return { path: 'http://example.org/foo/bar' };
-  }
-}
+import { BadRequestHttpError } from '../../../src/util/errors/BadRequestHttpError';
+import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError';
+import { StaticAsyncHandler } from '../../util/StaticAsyncHandler';
 
 describe('A DPoPWebIdExtractor', (): void => {
-  const targetExtractor = new DummyTargetExtractor();
+  const targetExtractor = new StaticAsyncHandler(true, { path: 'http://example.org/foo/bar' });
   const webIdExtractor = new DPoPWebIdExtractor(targetExtractor);
 
   beforeEach((): void => {
@@ -27,8 +22,26 @@ describe('A DPoPWebIdExtractor', (): void => {
       },
     } as any as HttpRequest;
 
-    it('returns empty credentials.', async(): Promise<void> => {
-      await expect(webIdExtractor.handle(request)).resolves.toEqual({});
+    it('throws an error.', async(): Promise<void> => {
+      const result = webIdExtractor.handleSafe(request);
+      await expect(result).rejects.toThrow(NotImplementedHttpError);
+      await expect(result).rejects.toThrow('No DPoP Authorization header specified.');
+    });
+  });
+
+  describe('on a request with an Authorization header that does not start with DPoP', (): void => {
+    const request = {
+      method: 'GET',
+      headers: {
+        authorization: 'Other token-1234',
+        dpop: 'token-5678',
+      },
+    } as any as HttpRequest;
+
+    it('throws an error.', async(): Promise<void> => {
+      const result = webIdExtractor.handleSafe(request);
+      await expect(result).rejects.toThrow(NotImplementedHttpError);
+      await expect(result).rejects.toThrow('No DPoP Authorization header specified.');
     });
   });
 
@@ -40,8 +53,10 @@ describe('A DPoPWebIdExtractor', (): void => {
       },
     } as any as HttpRequest;
 
-    it('returns empty credentials.', async(): Promise<void> => {
-      await expect(webIdExtractor.handle(request)).resolves.toEqual({});
+    it('throws an error.', async(): Promise<void> => {
+      const result = webIdExtractor.handleSafe(request);
+      await expect(result).rejects.toThrow(BadRequestHttpError);
+      await expect(result).rejects.toThrow('No DPoP token specified.');
     });
   });
 
@@ -55,20 +70,20 @@ describe('A DPoPWebIdExtractor', (): void => {
     } as any as HttpRequest;
 
     it('calls the target extractor with the correct parameters.', async(): Promise<void> => {
-      await webIdExtractor.handle(request);
+      await webIdExtractor.handleSafe(request);
       expect(targetExtractor.handle).toHaveBeenCalledTimes(1);
       expect(targetExtractor.handle).toHaveBeenCalledWith(request);
     });
 
     it('calls the DPoP verifier with the correct parameters.', async(): Promise<void> => {
-      await webIdExtractor.handle(request);
+      await webIdExtractor.handleSafe(request);
       expect(verify).toHaveBeenCalledTimes(1);
       expect(verify).toHaveBeenCalledWith('DPoP token-1234', 'token-5678', 'GET', 'http://example.org/foo/bar');
     });
 
     it('returns the extracted WebID.', async(): Promise<void> => {
-      await expect(webIdExtractor.handle(request)).resolves
-        .toEqual({ webID: 'http://alice.example/card#me' });
+      const result = webIdExtractor.handleSafe(request);
+      await expect(result).resolves.toEqual({ webID: 'http://alice.example/card#me' });
     });
   });
 
@@ -87,8 +102,10 @@ describe('A DPoPWebIdExtractor', (): void => {
       });
     });
 
-    it('returns empty credentials.', async(): Promise<void> => {
-      await expect(webIdExtractor.handle(request)).resolves.toEqual({});
+    it('throws an error.', async(): Promise<void> => {
+      const result = webIdExtractor.handleSafe(request);
+      await expect(result).rejects.toThrow(BadRequestHttpError);
+      await expect(result).rejects.toThrow('Error verifying WebID via DPoP token: invalid');
     });
   });
 });

test/unit/authentication/EmptyCredentialsExtractor.test.ts

@@ -0,0 +1,20 @@
+import { EmptyCredentialsExtractor } from '../../../src/authentication/EmptyCredentialsExtractor';
+import type { HttpRequest } from '../../../src/server/HttpRequest';
+import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError';
+
+describe('An EmptyCredentialsExtractor', (): void => {
+  const extractor = new EmptyCredentialsExtractor();
+
+  it('throws an error if an Authorization header is specified.', async(): Promise<void> => {
+    const headers = { authorization: 'Other http://alice.example/card#me' };
+    const result = extractor.handleSafe({ headers } as HttpRequest);
+    await expect(result).rejects.toThrow(NotImplementedHttpError);
+    await expect(result).rejects.toThrow('Unexpected Authorization scheme.');
+  });
+
+  it('returns the empty credentials.', async(): Promise<void> => {
+    const headers = {};
+    const result = extractor.handleSafe({ headers } as HttpRequest);
+    await expect(result).resolves.toEqual({});
+  });
+});

test/unit/authentication/UnsecureWebIdExtractor.test.ts

@@ -0,0 +1,27 @@
+import { UnsecureWebIdExtractor } from '../../../src/authentication/UnsecureWebIdExtractor';
+import type { HttpRequest } from '../../../src/server/HttpRequest';
+import { NotImplementedHttpError } from '../../../src/util/errors/NotImplementedHttpError';
+
+describe('An UnsecureWebIdExtractor', (): void => {
+  const extractor = new UnsecureWebIdExtractor();
+
+  it('throws an error if no Authorization header is specified.', async(): Promise<void> => {
+    const headers = {};
+    const result = extractor.handleSafe({ headers } as HttpRequest);
+    await expect(result).rejects.toThrow(NotImplementedHttpError);
+    await expect(result).rejects.toThrow('No WebID Authorization header specified.');
+  });
+
+  it('throws an error if a non-WebID Authorization header is specified.', async(): Promise<void> => {
+    const headers = { authorization: 'Other http://alice.example/card#me' };
+    const result = extractor.handleSafe({ headers } as HttpRequest);
+    await expect(result).rejects.toThrow(NotImplementedHttpError);
+    await expect(result).rejects.toThrow('No WebID Authorization header specified.');
+  });
+
+  it('returns the authorization header as WebID if there is one.', async(): Promise<void> => {
+    const headers = { authorization: 'WebID http://alice.example/card#me' };
+    const result = extractor.handleSafe({ headers } as HttpRequest);
+    await expect(result).resolves.toEqual({ webID: 'http://alice.example/card#me' });
+  });
+});

test/unit/authentication/UnsecureWebIdExtractor.ts

@@ -1,19 +0,0 @@
-import { UnsecureWebIdExtractor } from '../../../src/authentication/UnsecureWebIdExtractor';
-import type { HttpRequest } from '../../../src/server/HttpRequest';
-
-describe('An UnsecureWebIdExtractor', (): void => {
-  const extractor = new UnsecureWebIdExtractor();
-
-  it('can handle all input.', async(): Promise<void> => {
-    await expect(extractor.canHandle({} as HttpRequest)).resolves.toBeUndefined();
-  });
-
-  it('returns undefined if there is no input.', async(): Promise<void> => {
-    await expect(extractor.handle({ headers: {}} as HttpRequest)).resolves.toEqual({});
-  });
-
-  it('returns the authorization header as webID if there is one.', async(): Promise<void> => {
-    await expect(extractor.handle({ headers: { authorization: 'test' }} as HttpRequest))
-      .resolves.toEqual({ webID: 'test' });
-  });
-});