diff --git a/config/ldp/authorization/authorizers/access-checkers/agentGroup.json b/config/ldp/authorization/authorizers/access-checkers/agentGroup.json index fdc402bb48..6f28a234b4 100644 --- a/config/ldp/authorization/authorizers/access-checkers/agentGroup.json +++ b/config/ldp/authorization/authorizers/access-checkers/agentGroup.json @@ -4,7 +4,17 @@ { "@id": "urn:solid-server:default:AgentGroupAccessChecker", "@type": "AgentGroupAccessChecker", - "converter": { "@id": "urn:solid-server:default:RepresentationConverter" } + "converter": { "@id": "urn:solid-server:default:RepresentationConverter" }, + "cache": { + "@id": "urn:solid-server:default:ExpiringAclCache", + "@type": "WrappedExpiringStorage", + "source": { "@type": "MemoryMapStorage" } + } + }, + { + "comment": "Makes sure the expiring storage cleanup timer is stopped when the application needs to stop.", + "@id": "urn:solid-server:default:Finalizer", + "ParallelFinalizer:_finalizers": [ { "@id": "urn:solid-server:default:ExpiringAclCache" } ] } ] } diff --git a/src/authorization/access-checkers/AgentGroupAccessChecker.ts b/src/authorization/access-checkers/AgentGroupAccessChecker.ts index 66074ebe5f..d1a506da0f 100644 --- a/src/authorization/access-checkers/AgentGroupAccessChecker.ts +++ b/src/authorization/access-checkers/AgentGroupAccessChecker.ts @@ -1,6 +1,8 @@ -import type { Term } from 'n3'; +import type { Store, Term } from 'n3'; + import type { ResourceIdentifier } from '../../ldp/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'; import { readableToQuads } from '../../util/StreamUtil'; @@ -10,13 +12,24 @@ import { AccessChecker } from './AccessChecker'; /** * Checks if the given WebID belongs to a group that has access. + * + * Fetched results will be stored in an ExpiringStorage. + * + * Requires a storage that can store JS objects. + * `expiration` parameter is how long entries in the cache should be stored in minutes, defaults to 60. */ export class AgentGroupAccessChecker extends AccessChecker { private readonly converter: RepresentationConverter; + private readonly cache: ExpiringStorage>; + private readonly expiration: number; - public constructor(converter: RepresentationConverter) { + public constructor(converter: RepresentationConverter, cache: ExpiringStorage>, + expiration = 60) { super(); this.converter = converter; + this.cache = cache; + // Convert minutes to milliseconds + this.expiration = expiration * 60 * 1000; } public async handle({ acl, rule, credentials }: AccessCheckerArgs): Promise { @@ -41,9 +54,24 @@ export class AgentGroupAccessChecker extends AccessChecker { const groupDocument: ResourceIdentifier = { path: /^[^#]*/u.exec(group.value)![0] }; // Fetch the required vCard group file - const dataset = await fetchDataset(groupDocument.path, this.converter); - - const quads = await readableToQuads(dataset.data); + const quads = await this.fetchCachedQuads(groupDocument.path); return quads.countQuads(group, VCARD.terms.hasMember, webId, null) !== 0; } + + /** + * Fetches quads from the given URL. + * Will cache the values for later re-use. + */ + private async fetchCachedQuads(url: string): Promise { + let result = await this.cache.get(url); + if (!result) { + const prom = (async(): Promise => { + const representation = await fetchDataset(url, this.converter); + return readableToQuads(representation.data); + })(); + await this.cache.set(url, prom, this.expiration); + result = await prom; + } + return result; + } } diff --git a/src/identity/ownership/TokenOwnershipValidator.ts b/src/identity/ownership/TokenOwnershipValidator.ts index 6ee1aca3c6..9447a93e5d 100644 --- a/src/identity/ownership/TokenOwnershipValidator.ts +++ b/src/identity/ownership/TokenOwnershipValidator.ts @@ -36,7 +36,7 @@ export class TokenOwnershipValidator extends OwnershipValidator { // No reason to fetch the WebId if we don't have a token yet if (!token) { token = this.generateToken(); - await this.storage.set(key, token, new Date(Date.now() + this.expiration)); + await this.storage.set(key, token, this.expiration); this.throwError(webId, token); } diff --git a/src/identity/storage/ExpiringAdapterFactory.ts b/src/identity/storage/ExpiringAdapterFactory.ts index 24d7028b60..d540ab7c5b 100644 --- a/src/identity/storage/ExpiringAdapterFactory.ts +++ b/src/identity/storage/ExpiringAdapterFactory.ts @@ -43,13 +43,13 @@ export class ExpiringAdapter implements Adapter { public async upsert(id: string, payload: AdapterPayload, expiresIn?: number): Promise { // Despite what the typings say, `expiresIn` can be undefined - const expires = expiresIn ? new Date(Date.now() + (expiresIn * 1000)) : undefined; + const expiration = expiresIn ? expiresIn * 1000 : undefined; const key = this.keyFor(id); this.logger.debug(`Storing payload data for ${id}`); const storagePromises: Promise[] = [ - this.storage.set(key, payload, expires), + this.storage.set(key, payload, expiration), ]; if (payload.grantId) { storagePromises.push( @@ -57,15 +57,15 @@ export class ExpiringAdapter implements Adapter { const grantKey = this.grantKeyFor(payload.grantId!); const grants = (await this.storage.get(grantKey) || []) as string[]; grants.push(key); - await this.storage.set(grantKey, grants, expires); + await this.storage.set(grantKey, grants, expiration); })(), ); } if (payload.userCode) { - storagePromises.push(this.storage.set(this.userCodeKeyFor(payload.userCode), id, expires)); + storagePromises.push(this.storage.set(this.userCodeKeyFor(payload.userCode), id, expiration)); } if (payload.uid) { - storagePromises.push(this.storage.set(this.uidKeyFor(payload.uid), id, expires)); + storagePromises.push(this.storage.set(this.uidKeyFor(payload.uid), id, expiration)); } await Promise.all(storagePromises); } diff --git a/src/storage/keyvalue/ExpiringStorage.ts b/src/storage/keyvalue/ExpiringStorage.ts index ac53e8faf9..ab83d8d42e 100644 --- a/src/storage/keyvalue/ExpiringStorage.ts +++ b/src/storage/keyvalue/ExpiringStorage.ts @@ -1,10 +1,23 @@ import type { KeyValueStorage } from './KeyValueStorage'; +/* eslint-disable @typescript-eslint/method-signature-style */ /** * A KeyValueStorage in which the values can expire. * Entries with no expiration date never expire. */ export interface ExpiringStorage extends KeyValueStorage { + /** + * Sets the value for the given key. + * Should error if the data is already expired. + * + * @param key - Key to set/update. + * @param value - Value to store. + * @param expiration - How long this data should stay valid in milliseconds. + * + * @returns The storage. + */ + set(key: TKey, value: TValue, expiration?: number): Promise; + /** * Sets the value for the given key. * Should error if the data is already expired. @@ -15,5 +28,5 @@ export interface ExpiringStorage extends KeyValueStorage Promise; + set(key: TKey, value: TValue, expires?: Date): Promise; } diff --git a/src/storage/keyvalue/WrappedExpiringStorage.ts b/src/storage/keyvalue/WrappedExpiringStorage.ts index 5fffdf48a3..6ac9cc3c8d 100644 --- a/src/storage/keyvalue/WrappedExpiringStorage.ts +++ b/src/storage/keyvalue/WrappedExpiringStorage.ts @@ -34,7 +34,10 @@ export class WrappedExpiringStorage implements ExpiringStorage { + public async set(key: TKey, value: TValue, expiration?: number): Promise; + public async set(key: TKey, value: TValue, expires?: Date): Promise; + public async set(key: TKey, value: TValue, expireValue?: number | Date): Promise { + const expires = typeof expireValue === 'number' ? new Date(Date.now() + expireValue) : expireValue; if (this.isExpired(expires)) { throw new InternalServerError('Value is already expired'); } diff --git a/test/unit/authorization/access-checkers/AgentGroupAccessChecker.test.ts b/test/unit/authorization/access-checkers/AgentGroupAccessChecker.test.ts index 826e02d60d..363a96fc0d 100644 --- a/test/unit/authorization/access-checkers/AgentGroupAccessChecker.test.ts +++ b/test/unit/authorization/access-checkers/AgentGroupAccessChecker.test.ts @@ -4,6 +4,7 @@ import { AgentGroupAccessChecker } from '../../../../src/authorization/access-ch import { BasicRepresentation } from '../../../../src/ldp/representation/BasicRepresentation'; import type { Representation } from '../../../../src/ldp/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'; import { ACL, VCARD } from '../../../../src/util/Vocabularies'; @@ -18,6 +19,7 @@ describe('An AgentGroupAccessChecker', (): void => { let fetchMock: jest.SpyInstance; let representation: Representation; const converter: RepresentationConverter = {} as any; + let cache: ExpiringStorage>; let checker: AgentGroupAccessChecker; beforeEach(async(): Promise => { @@ -25,8 +27,11 @@ describe('An AgentGroupAccessChecker', (): void => { representation = new BasicRepresentation(groupQuads, INTERNAL_QUADS, false); fetchMock = jest.spyOn(fetchUtil, 'fetchDataset'); fetchMock.mockResolvedValue(representation); + fetchMock.mockClear(); - checker = new AgentGroupAccessChecker(converter); + cache = new Map() as any; + + checker = new AgentGroupAccessChecker(converter, cache); }); it('can handle all requests.', async(): Promise => { @@ -47,4 +52,11 @@ describe('An AgentGroupAccessChecker', (): void => { const input: AccessCheckerArgs = { acl, rule: namedNode('groupMatch'), credentials: {}}; await expect(checker.handle(input)).resolves.toBe(false); }); + + it('caches fetched results.', async(): Promise => { + const input: AccessCheckerArgs = { acl, rule: namedNode('groupMatch'), credentials: { webId }}; + await expect(checker.handle(input)).resolves.toBe(true); + await expect(checker.handle(input)).resolves.toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/test/unit/identity/storage/ExpiringAdapterFactory.test.ts b/test/unit/identity/storage/ExpiringAdapterFactory.test.ts index 6f2a50414a..58d1010e4d 100644 --- a/test/unit/identity/storage/ExpiringAdapterFactory.test.ts +++ b/test/unit/identity/storage/ExpiringAdapterFactory.test.ts @@ -15,8 +15,7 @@ describe('An ExpiringAdapterFactory', (): void => { let storage: ExpiringStorage; let adapter: ExpiringAdapter; let factory: ExpiringAdapterFactory; - const expiresIn = 333; - const expireDate = new Date(Date.now() + (expiresIn * 1000)); + const expiresIn = 333 * 1000; beforeEach(async(): Promise => { payload = { data: 'data!' }; @@ -35,7 +34,7 @@ describe('An ExpiringAdapterFactory', (): void => { it('can find payload by id.', async(): Promise => { await expect(adapter.upsert(id, payload, 333)).resolves.toBeUndefined(); expect(storage.set).toHaveBeenCalledTimes(1); - expect(storage.set).toHaveBeenCalledWith(expect.anything(), payload, expireDate); + expect(storage.set).toHaveBeenCalledWith(expect.anything(), payload, expiresIn); await expect(adapter.find(id)).resolves.toBe(payload); }); @@ -50,8 +49,8 @@ describe('An ExpiringAdapterFactory', (): void => { payload.userCode = userCode; await expect(adapter.upsert(id, payload, 333)).resolves.toBeUndefined(); expect(storage.set).toHaveBeenCalledTimes(2); - expect(storage.set).toHaveBeenCalledWith(expect.anything(), payload, expireDate); - expect(storage.set).toHaveBeenCalledWith(expect.anything(), id, expireDate); + expect(storage.set).toHaveBeenCalledWith(expect.anything(), payload, expiresIn); + expect(storage.set).toHaveBeenCalledWith(expect.anything(), id, expiresIn); await expect(adapter.findByUserCode(userCode)).resolves.toBe(payload); }); @@ -60,8 +59,8 @@ describe('An ExpiringAdapterFactory', (): void => { payload.uid = uid; await expect(adapter.upsert(id, payload, 333)).resolves.toBeUndefined(); expect(storage.set).toHaveBeenCalledTimes(2); - expect(storage.set).toHaveBeenCalledWith(expect.anything(), payload, expireDate); - expect(storage.set).toHaveBeenCalledWith(expect.anything(), id, expireDate); + expect(storage.set).toHaveBeenCalledWith(expect.anything(), payload, expiresIn); + expect(storage.set).toHaveBeenCalledWith(expect.anything(), id, expiresIn); await expect(adapter.findByUid(uid)).resolves.toBe(payload); }); @@ -69,8 +68,8 @@ describe('An ExpiringAdapterFactory', (): void => { payload.grantId = grantId; await expect(adapter.upsert(id, payload, 333)).resolves.toBeUndefined(); expect(storage.set).toHaveBeenCalledTimes(2); - expect(storage.set).toHaveBeenCalledWith(expect.anything(), payload, expireDate); - expect(storage.set).toHaveBeenCalledWith(expect.anything(), [ expect.anything() ], expireDate); + expect(storage.set).toHaveBeenCalledWith(expect.anything(), payload, expiresIn); + expect(storage.set).toHaveBeenCalledWith(expect.anything(), [ expect.anything() ], expiresIn); await expect(adapter.find(id)).resolves.toBe(payload); await expect(adapter.revokeByGrantId(grantId)).resolves.toBeUndefined(); expect(storage.delete).toHaveBeenCalledTimes(2); diff --git a/test/unit/storage/keyvalue/WrappedExpiringStorage.test.ts b/test/unit/storage/keyvalue/WrappedExpiringStorage.test.ts index 729473d49d..b531edb775 100644 --- a/test/unit/storage/keyvalue/WrappedExpiringStorage.test.ts +++ b/test/unit/storage/keyvalue/WrappedExpiringStorage.test.ts @@ -17,7 +17,7 @@ describe('A WrappedExpiringStorage', (): void => { tomorrow.setDate(tomorrow.getDate() + 1); const yesterday = new Date(); yesterday.setDate(yesterday.getDate() - 1); - let source: KeyValueStorage; + let source: jest.Mocked>; let storage: WrappedExpiringStorage; beforeEach(async(): Promise => { @@ -42,12 +42,12 @@ describe('A WrappedExpiringStorage', (): void => { }); it('returns data if it has not expired.', async(): Promise => { - (source.get as jest.Mock).mockResolvedValueOnce(createExpires('data!', tomorrow)); + source.get.mockResolvedValueOnce(createExpires('data!', tomorrow)); await expect(storage.get('key')).resolves.toEqual('data!'); }); it('deletes expired data when trying to get it.', async(): Promise => { - (source.get as jest.Mock).mockResolvedValueOnce(createExpires('data!', yesterday)); + source.get.mockResolvedValueOnce(createExpires('data!', yesterday)); await expect(storage.get('key')).resolves.toBeUndefined(); expect(source.delete).toHaveBeenCalledTimes(1); expect(source.delete).toHaveBeenLastCalledWith('key'); @@ -60,12 +60,12 @@ describe('A WrappedExpiringStorage', (): void => { }); it('true on `has` checks if there is non-expired data.', async(): Promise => { - (source.get as jest.Mock).mockResolvedValueOnce(createExpires('data!', tomorrow)); + source.get.mockResolvedValueOnce(createExpires('data!', tomorrow)); await expect(storage.has('key')).resolves.toBe(true); }); it('deletes expired data when checking if it exists.', async(): Promise => { - (source.get as jest.Mock).mockResolvedValueOnce(createExpires('data!', yesterday)); + source.get.mockResolvedValueOnce(createExpires('data!', yesterday)); await expect(storage.has('key')).resolves.toBe(false); expect(source.delete).toHaveBeenCalledTimes(1); expect(source.delete).toHaveBeenLastCalledWith('key'); @@ -77,6 +77,12 @@ describe('A WrappedExpiringStorage', (): void => { expect(source.set).toHaveBeenLastCalledWith('key', createExpires('data!', tomorrow)); }); + it('can store data with an expiration duration.', async(): Promise => { + await storage.set('key', 'data!', tomorrow.getTime() - Date.now()); + expect(source.set).toHaveBeenCalledTimes(1); + expect(source.set).toHaveBeenLastCalledWith('key', createExpires('data!', tomorrow)); + }); + it('can store data without expiry date.', async(): Promise => { await storage.set('key', 'data!'); expect(source.set).toHaveBeenCalledTimes(1); @@ -99,7 +105,7 @@ describe('A WrappedExpiringStorage', (): void => { [ 'key2', createExpires('data2', yesterday) ], [ 'key3', createExpires('data3') ], ]; - (source.entries as jest.Mock).mockImplementationOnce(function* (): any { + source.entries.mockImplementationOnce(function* (): any { yield* data; }); const it = storage.entries(); @@ -123,7 +129,7 @@ describe('A WrappedExpiringStorage', (): void => { [ 'key2', createExpires('data2', yesterday) ], [ 'key3', createExpires('data3') ], ]; - (source.entries as jest.Mock).mockImplementationOnce(function* (): any { + source.entries.mockImplementationOnce(function* (): any { yield* data; }); @@ -150,7 +156,7 @@ describe('A WrappedExpiringStorage', (): void => { [ 'key2', createExpires('data2', yesterday) ], [ 'key3', createExpires('data3') ], ]; - (source.entries as jest.Mock).mockImplementationOnce(function* (): any { + source.entries.mockImplementationOnce(function* (): any { yield* data; });