fix: Add content-negotiation when fetching dataset from url

* Solution works but tests don't

* refactor(FetchUtil): use arrayifyStream

* refactor(FetchUtil): split fetchDataset into 2 separate functions

* style(FetchUtil): onelining instead of declaring new local var

* test: trying to mock rdfDereferencer

* refactor: promise can't have async function as arg

* test(FetchUtil): pass Quad array to mockDereference instead

* test: all tests should pass now and coverage is back to 100%

* style: comment typo

* chore: make package.json and package-lock.json compatible with main

* chore: fix package.json double entries

* chore: updated package.json to be alfabetical again

* refactor(AgentGroupAccessChecker): Remove converter from contructor and config

* refactor(TokenOwnerShipValidator): Remove converter from constructor and config

* refactor(FetchUtil): Return BadRequestHttpError instead of generic Error

* test(FetchUtil): return Response object instead of mocking fetch

* style: typos and newlines

config/identity/ownership/token.json

@@ -5,7 +5,6 @@
       "comment": "Determines WebID ownership by requesting a specific value to be added to the WebID document",
       "@id": "urn:solid-server:auth:password:OwnershipValidator",
       "@type": "TokenOwnershipValidator",
-      "converter": { "@id": "urn:solid-server:default:RepresentationConverter" },
       "storage": { "@id": "urn:solid-server:default:ExpiringTokenStorage" }
     },
 

config/ldp/authorization/readers/access-checkers/agent-group.json

@@ -5,7 +5,6 @@
       "comment": "Checks if the agent belongs to a group that has access.",
       "@id": "urn:solid-server:default:AgentGroupAccessChecker",
       "@type": "AgentGroupAccessChecker",
-      "converter": { "@id": "urn:solid-server:default:RepresentationConverter" },
       "cache": {
         "@id": "urn:solid-server:default:ExpiringAclCache",
         "@type": "WrappedExpiringStorage",

package-lock.json

@@ -51,6 +51,7 @@
         "oidc-provider": "^6.31.1",
         "pump": "^3.0.0",
         "punycode": "^2.1.1",
+        "rdf-dereference": "^1.9.0",
         "rdf-parse": "^1.9.1",
         "rdf-serialize": "^1.2.0",
         "rdf-terms": "^1.7.1",

package.json

@@ -117,6 +117,7 @@
     "oidc-provider": "^6.31.1",
     "pump": "^3.0.0",
     "punycode": "^2.1.1",
+    "rdf-dereference": "^1.9.0",
     "rdf-parse": "^1.9.1",
     "rdf-serialize": "^1.2.0",
     "rdf-terms": "^1.7.1",

src/authorization/access/AgentGroupAccessChecker.ts

@@ -1,6 +1,5 @@
 import type { Store, Term } from 'n3';
 import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
-import type { RepresentationConverter } from '../../storage/conversion/RepresentationConverter';
 import type { ExpiringStorage } from '../../storage/keyvalue/ExpiringStorage';
 import { fetchDataset } from '../../util/FetchUtil';
 import { promiseSome } from '../../util/PromiseUtil';
@@ -19,14 +18,11 @@ import { AccessChecker } from './AccessChecker';
  * `expiration` parameter is how long entries in the cache should be stored in seconds, defaults to 3600.
  */
 export class AgentGroupAccessChecker extends AccessChecker {
-  private readonly converter: RepresentationConverter;
   private readonly cache: ExpiringStorage<string, Promise<Store>>;
   private readonly expiration: number;
 
-  public constructor(converter: RepresentationConverter, cache: ExpiringStorage<string, Promise<Store>>,
+  public constructor(cache: ExpiringStorage<string, Promise<Store>>, expiration = 3600) {
-    expiration = 3600) {
     super();
-    this.converter = converter;
     this.cache = cache;
     this.expiration = expiration * 1000;
   }
@@ -65,7 +61,7 @@ export class AgentGroupAccessChecker extends AccessChecker {
     let result = await this.cache.get(url);
     if (!result) {
       const prom = (async(): Promise<Store> => {
-        const representation = await fetchDataset(url, this.converter);
+        const representation = await fetchDataset(url);
         return readableToQuads(representation.data);
       })();
       await this.cache.set(url, prom, this.expiration);

src/identity/ownership/TokenOwnershipValidator.ts

@@ -2,7 +2,6 @@ import type { Quad } from 'n3';
 import { DataFactory } from 'n3';
 import { v4 } from 'uuid';
 import { getLoggerFor } from '../../logging/LogUtil';
-import type { RepresentationConverter } from '../../storage/conversion/RepresentationConverter';
 import type { ExpiringStorage } from '../../storage/keyvalue/ExpiringStorage';
 import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError';
 import { fetchDataset } from '../../util/FetchUtil';
@@ -17,13 +16,11 @@ const { literal, namedNode, quad } = DataFactory;
 export class TokenOwnershipValidator extends OwnershipValidator {
   protected readonly logger = getLoggerFor(this);
 
-  private readonly converter: RepresentationConverter;
   private readonly storage: ExpiringStorage<string, string>;
   private readonly expiration: number;
 
-  public constructor(converter: RepresentationConverter, storage: ExpiringStorage<string, string>, expiration = 30) {
+  public constructor(storage: ExpiringStorage<string, string>, expiration = 30) {
     super();
-    this.converter = converter;
     this.storage = storage;
     // Convert minutes to milliseconds
     this.expiration = expiration * 60 * 1000;
@@ -66,7 +63,7 @@ export class TokenOwnershipValidator extends OwnershipValidator {
    * Fetches data from the WebID to determine if the token is present.
    */
   private async hasToken(webId: string, token: string): Promise<boolean> {
-    const representation = await fetchDataset(webId, this.converter);
+    const representation = await fetchDataset(webId);
     const expectedQuad = quad(namedNode(webId), SOLID.terms.oidcIssuerRegistrationToken, literal(token));
     for await (const data of representation.data) {
       const triple = data as Quad;

src/identity/storage/WebIdAdapterFactory.ts

@@ -5,7 +5,7 @@ import type { Adapter, AdapterPayload } from 'oidc-provider';
 import { getLoggerFor } from '../../logging/LogUtil';
 import type { RepresentationConverter } from '../../storage/conversion/RepresentationConverter';
 import { createErrorMessage } from '../../util/errors/ErrorUtil';
-import { fetchDataset } from '../../util/FetchUtil';
+import { responseToDataset } from '../../util/FetchUtil';
 import { OIDC } from '../../util/Vocabularies';
 import type { AdapterFactory } from './AdapterFactory';
 
@@ -91,7 +91,7 @@ export class WebIdAdapter implements Adapter {
    * @param response - Response object from the request.
    */
   private async parseRdfWebId(data: string, id: string, response: Response): Promise<AdapterPayload> {
-    const representation = await fetchDataset(response, this.converter, data);
+    const representation = await responseToDataset(response, this.converter, data);
 
     // Find the valid redirect URIs
     const redirectUris: string[] = [];

src/util/FetchUtil.ts

@@ -1,5 +1,8 @@
+import type { Readable } from 'stream';
+import type { Quad } from '@rdfjs/types';
+import arrayifyStream from 'arrayify-stream';
 import type { Response } from 'cross-fetch';
-import { fetch } from 'cross-fetch';
+import rdfDereferencer from 'rdf-dereference';
 import { BasicRepresentation } from '../http/representation/BasicRepresentation';
 import type { Representation } from '../http/representation/Representation';
 import { getLoggerFor } from '../logging/LogUtil';
@@ -12,24 +15,32 @@ const logger = getLoggerFor('FetchUtil');
 
 /**
  * Fetches an RDF dataset from the given URL.
- * Input can also be a Response if the request was already made.
+ *
+ * Response will be a Representation with content-type internal/quads.
+ */
+export async function fetchDataset(url: string): Promise<Representation> {
+  // Try content negotiation to parse quads from the URL
+  return (async(): Promise<Representation> => {
+    try {
+      const quadStream = (await rdfDereferencer.dereference(url)).quads as Readable;
+      const quadArray = await arrayifyStream(quadStream) as Quad[];
+      return new BasicRepresentation(quadArray, { path: url }, INTERNAL_QUADS, false);
+    } catch {
+      throw new BadRequestHttpError(`Could not parse resource at URL (${url})!`);
+    }
+  })();
+}
+
+/**
+ * Converts a given Response (from a request that was already made) to  an RDF dataset.
  * In case the given Response object was already parsed its body can be passed along as a string.
  *
  * The converter will be used to convert the response body to RDF.
  *
  * Response will be a Representation with content-type internal/quads.
  */
-export async function fetchDataset(url: string, converter: RepresentationConverter): Promise<Representation>;
+export async function responseToDataset(response: Response, converter: RepresentationConverter, body?: string):
-export async function fetchDataset(response: Response, converter: RepresentationConverter, body?: string):
-Promise<Representation>;
-export async function fetchDataset(input: string | Response, converter: RepresentationConverter, body?: string):
 Promise<Representation> {
-  let response: Response;
-  if (typeof input === 'string') {
-    response = await fetch(input);
-  } else {
-    response = input;
-  }
   if (!body) {
     body = await response.text();
   }

test/unit/authorization/access/AgentGroupAccessChecker.test.ts

@@ -3,7 +3,6 @@ import type { AccessCheckerArgs } from '../../../../src/authorization/access/Acc
 import { AgentGroupAccessChecker } from '../../../../src/authorization/access/AgentGroupAccessChecker';
 import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation';
 import type { Representation } from '../../../../src/http/representation/Representation';
-import type { RepresentationConverter } from '../../../../src/storage/conversion/RepresentationConverter';
 import type { ExpiringStorage } from '../../../../src/storage/keyvalue/ExpiringStorage';
 import { INTERNAL_QUADS } from '../../../../src/util/ContentTypes';
 import * as fetchUtil from '../../../../src/util/FetchUtil';
@@ -18,7 +17,6 @@ describe('An AgentGroupAccessChecker', (): void => {
   acl.addQuad(namedNode('noMatch'), ACL.terms.agentGroup, namedNode('badGroup'));
   let fetchMock: jest.SpyInstance;
   let representation: Representation;
-  const converter: RepresentationConverter = {} as any;
   let cache: ExpiringStorage<string, Promise<Store>>;
   let checker: AgentGroupAccessChecker;
 
@@ -31,7 +29,7 @@ describe('An AgentGroupAccessChecker', (): void => {
 
     cache = new Map() as any;
 
-    checker = new AgentGroupAccessChecker(converter, cache);
+    checker = new AgentGroupAccessChecker(cache);
   });
 
   it('can handle all requests.', async(): Promise<void> => {

test/unit/identity/ownership/TokenOwnershipValidator.test.ts

@@ -1,15 +1,17 @@
-import { fetch } from 'cross-fetch';
+import { Readable } from 'stream';
 import { DataFactory } from 'n3';
 import type { Quad } from 'n3';
+import rdfDereferencer from 'rdf-dereference';
 import { v4 } from 'uuid';
 import { TokenOwnershipValidator } from '../../../../src/identity/ownership/TokenOwnershipValidator';
-import { RdfToQuadConverter } from '../../../../src/storage/conversion/RdfToQuadConverter';
 import type { ExpiringStorage } from '../../../../src/storage/keyvalue/ExpiringStorage';
 import { SOLID } from '../../../../src/util/Vocabularies';
 const { literal, namedNode, quad } = DataFactory;
 
-jest.mock('cross-fetch');
 jest.mock('uuid');
+jest.mock('rdf-dereference', (): any => ({
+  dereference: jest.fn(),
+}));
 
 function quadToString(qq: Quad): string {
   const subPred = `<${qq.subject.value}> <${qq.predicate.value}>`;
@@ -20,21 +22,19 @@ function quadToString(qq: Quad): string {
 }
 
 describe('A TokenOwnershipValidator', (): void => {
-  const fetchMock: jest.Mock = fetch as any;
+  const rdfDereferenceMock: jest.Mocked<typeof rdfDereferencer> = rdfDereferencer as any;
   const webId = 'http://alice.test.com/#me';
   const token = 'randomlyGeneratedToken';
   const tokenTriple = quad(namedNode(webId), SOLID.terms.oidcIssuerRegistrationToken, literal(token));
   const tokenString = `${quadToString(tokenTriple)}.`;
-  const converter = new RdfToQuadConverter();
   let storage: ExpiringStorage<string, string>;
   let validator: TokenOwnershipValidator;
 
-  function mockFetch(body: string): void {
+  function mockDereference(qq?: Quad): any {
-    fetchMock.mockImplementation((url: string): any => ({
+    rdfDereferenceMock.dereference.mockImplementation((uri: string): any => ({
-      text: (): any => body,
+      uri,
-      url,
+      quads: Readable.from(qq ? [ qq ] : []),
-      status: 200,
+      exists: true,
-      headers: { get: (): any => 'text/turtle' },
     }));
   }
 
@@ -50,32 +50,32 @@ describe('A TokenOwnershipValidator', (): void => {
       delete: jest.fn().mockImplementation((key: string): any => map.delete(key)),
     } as any;
 
-    mockFetch('');
+    mockDereference();
 
-    validator = new TokenOwnershipValidator(converter, storage);
+    validator = new TokenOwnershipValidator(storage);
   });
 
   it('errors if no token is stored in the storage.', async(): Promise<void> => {
     // Even if the token is in the WebId, it will error since it's not in the storage
-    mockFetch(tokenString);
+    mockDereference(tokenTriple);
     await expect(validator.handle({ webId })).rejects.toThrow(expect.objectContaining({
       message: expect.stringContaining(tokenString),
       details: { quad: tokenString },
     }));
-    expect(fetch).toHaveBeenCalledTimes(0);
+    expect(rdfDereferenceMock.dereference).toHaveBeenCalledTimes(0);
   });
 
   it('errors if the expected triple is missing.', async(): Promise<void> => {
     // First call will add the token to the storage
     await expect(validator.handle({ webId })).rejects.toThrow(tokenString);
-    expect(fetch).toHaveBeenCalledTimes(0);
+    expect(rdfDereferenceMock.dereference).toHaveBeenCalledTimes(0);
     // Second call will fetch the WebId
     await expect(validator.handle({ webId })).rejects.toThrow(tokenString);
-    expect(fetch).toHaveBeenCalledTimes(1);
+    expect(rdfDereferenceMock.dereference).toHaveBeenCalledTimes(1);
   });
 
   it('resolves if the WebId contains the verification triple.', async(): Promise<void> => {
-    mockFetch(tokenString);
+    mockDereference(tokenTriple);
     // First call will add the token to the storage
     await expect(validator.handle({ webId })).rejects.toThrow(tokenString);
     // Second call will succeed since it has the verification triple
@@ -84,7 +84,7 @@ describe('A TokenOwnershipValidator', (): void => {
 
   it('fails if the WebId contains the wrong verification triple.', async(): Promise<void> => {
     const wrongQuad = quad(namedNode(webId), SOLID.terms.oidcIssuerRegistrationToken, literal('wrongToken'));
-    mockFetch(`${quadToString(wrongQuad)} .`);
+    mockDereference(wrongQuad);
     // First call will add the token to the storage
     await expect(validator.handle({ webId })).rejects.toThrow(tokenString);
     // Second call will fail since it has the wrong verification triple

test/unit/util/FetchUtil.test.ts

@@ -1,55 +1,81 @@
+import { Readable } from 'stream';
+import type { Quad } from '@rdfjs/types';
 import arrayifyStream from 'arrayify-stream';
-import { fetch } from 'cross-fetch';
+import type { Response } from 'cross-fetch';
 import { DataFactory } from 'n3';
+import rdfDereferencer from 'rdf-dereference';
 import { RdfToQuadConverter } from '../../../src/storage/conversion/RdfToQuadConverter';
-import { fetchDataset } from '../../../src/util/FetchUtil';
+import { fetchDataset, responseToDataset } from '../../../src/util/FetchUtil';
 const { namedNode, quad } = DataFactory;
 
-jest.mock('cross-fetch');
+jest.mock('rdf-dereference', (): any => ({
+  dereference: jest.fn<string, any>(),
+}));
 
 describe('FetchUtil', (): void => {
-  describe('#fetchDataset', (): void => {
+  const url = 'http://test.com/foo';
-    const fetchMock: jest.Mock = fetch as any;
-    const url = 'http://test.com/foo';
-    const converter = new RdfToQuadConverter();
 
-    function mockFetch(body: string, status = 200): void {
+  function mockResponse(body: string, contentType: string | null, status = 200): Response {
-      fetchMock.mockImplementation((input: string): any => ({
+    return ({
-        text: (): any => body,
+      text: (): any => body,
-        url: input,
+      url,
-        status,
+      status,
-        headers: { get: (): any => 'text/turtle' },
+      headers: { get: (): any => contentType },
-      }));
+    }) as any;
+  }
+
+  describe('#fetchDataset', (): void => {
+    const rdfDereferenceMock: jest.Mocked<typeof rdfDereferencer> = rdfDereferencer as any;
+
+    function mockDereference(quads?: Quad[]): any {
+      rdfDereferenceMock.dereference.mockImplementation((uri: string): any => {
+        if (!quads) {
+          throw new Error('Throws error because url does not exist');
+        }
+        return {
+          uri,
+          quads: Readable.from(quads),
+          exists: true,
+        };
+      });
     }
 
-    it('errors if the status code is not 200.', async(): Promise<void> => {
+    it('errors if the URL does not exist.', async(): Promise<void> => {
-      mockFetch('Invalid URL!', 404);
+      mockDereference();
-      await expect(fetchDataset(url, converter)).rejects.toThrow(`Unable to access data at ${url}`);
+      await expect(fetchDataset(url)).rejects.toThrow(`Could not parse resource at URL (${url})!`);
-      expect(fetchMock).toHaveBeenCalledWith(url);
+      expect(rdfDereferenceMock.dereference).toHaveBeenCalledWith(url);
-    });
-
-    it('errors if there is no content-type.', async(): Promise<void> => {
-      fetchMock.mockResolvedValueOnce({ url, text: (): any => '', status: 200, headers: { get: jest.fn() }});
-      await expect(fetchDataset(url, converter)).rejects.toThrow(`Unable to access data at ${url}`);
-      expect(fetchMock).toHaveBeenCalledWith(url);
     });
 
     it('returns a Representation with quads.', async(): Promise<void> => {
-      mockFetch('<http://test.com/s> <http://test.com/p> <http://test.com/o>.');
+      const quads = [ quad(namedNode('http://test.com/s'), namedNode('http://test.com/p'), namedNode('http://test.com/o')) ];
-      const representation = await fetchDataset(url, converter);
+      mockDereference(quads);
-      await expect(arrayifyStream(representation.data)).resolves.toEqual([
+      const representation = await fetchDataset(url);
-        quad(namedNode('http://test.com/s'), namedNode('http://test.com/p'), namedNode('http://test.com/o')),
-      ]);
-    });
-
-    it('accepts Response objects as input.', async(): Promise<void> => {
-      mockFetch('<http://test.com/s> <http://test.com/p> <http://test.com/o>.');
-      const response = await fetch(url);
-      const body = await response.text();
-      const representation = await fetchDataset(response, converter, body);
       await expect(arrayifyStream(representation.data)).resolves.toEqual([
         quad(namedNode('http://test.com/s'), namedNode('http://test.com/p'), namedNode('http://test.com/o')),
       ]);
     });
   });
+
+  describe('#responseToDataset', (): void => {
+    const converter = new RdfToQuadConverter();
+
+    it('accepts Response objects as input.', async(): Promise<void> => {
+      const response = mockResponse('<http://test.com/s> <http://test.com/p> <http://test.com/o>.', 'text/turtle');
+      const body = await response.text();
+      const representation = await responseToDataset(response, converter, body);
+      await expect(arrayifyStream(representation.data)).resolves.toEqual([
+        quad(namedNode('http://test.com/s'), namedNode('http://test.com/p'), namedNode('http://test.com/o')),
+      ]);
+    });
+
+    it('errors if the status code is not 200.', async(): Promise<void> => {
+      const response = mockResponse('Incorrect status!', null, 400);
+      await expect(responseToDataset(response, converter)).rejects.toThrow(`Unable to access data at ${url}`);
+    });
+
+    it('errors if there is no content-type.', async(): Promise<void> => {
+      const response = mockResponse('No content-type!', null);
+      await expect(responseToDataset(response, converter)).rejects.toThrow(`Unable to access data at ${url}`);
+    });
+  });
 });