fix: Return WAC-Allow header in 304 responses

This commit is contained in:
Joachim Van Herwegen 2023-07-27 13:31:54 +02:00
parent baa64987c6
commit 43e8ef99b0
2 changed files with 55 additions and 1 deletions

View File

@ -8,6 +8,7 @@ import { AccessMode } from '../authorization/permissions/Permissions';
import type { ResponseDescription } from '../http/output/response/ResponseDescription'; import type { ResponseDescription } from '../http/output/response/ResponseDescription';
import type { RepresentationMetadata } from '../http/representation/RepresentationMetadata'; import type { RepresentationMetadata } from '../http/representation/RepresentationMetadata';
import { getLoggerFor } from '../logging/LogUtil'; import { getLoggerFor } from '../logging/LogUtil';
import { NotModifiedHttpError } from '../util/errors/NotModifiedHttpError';
import { ACL, AUTH } from '../util/Vocabularies'; import { ACL, AUTH } from '../util/Vocabularies';
import type { OperationHttpHandlerInput } from './OperationHttpHandler'; import type { OperationHttpHandlerInput } from './OperationHttpHandler';
import { OperationHttpHandler } from './OperationHttpHandler'; import { OperationHttpHandler } from './OperationHttpHandler';
@ -47,7 +48,18 @@ export class WacAllowHttpHandler extends OperationHttpHandler {
public async handle(input: OperationHttpHandlerInput): Promise<ResponseDescription> { public async handle(input: OperationHttpHandlerInput): Promise<ResponseDescription> {
const { request, operation } = input; const { request, operation } = input;
const response = await this.operationHandler.handleSafe(input); let response: ResponseDescription | NotModifiedHttpError;
try {
response = await this.operationHandler.handleSafe(input);
} catch (error: unknown) {
// WAC-Allow headers need to be added to 304 responses
// as the value can differ even if the representation is the same.
if (NotModifiedHttpError.isInstance(error)) {
response = error;
} else {
throw error;
}
}
const { metadata } = response; const { metadata } = response;
// WAC-Allow is only needed for HEAD/GET requests // WAC-Allow is only needed for HEAD/GET requests
@ -81,6 +93,10 @@ export class WacAllowHttpHandler extends OperationHttpHandler {
this.addWacAllowMetadata(metadata, everyone, user); this.addWacAllowMetadata(metadata, everyone, user);
} }
if (NotModifiedHttpError.isInstance(response)) {
throw response;
}
return response; return response;
} }

View File

@ -11,6 +11,7 @@ import type { HttpRequest } from '../../../src/server/HttpRequest';
import type { HttpResponse } from '../../../src/server/HttpResponse'; import type { HttpResponse } from '../../../src/server/HttpResponse';
import type { OperationHttpHandler } from '../../../src/server/OperationHttpHandler'; import type { OperationHttpHandler } from '../../../src/server/OperationHttpHandler';
import { WacAllowHttpHandler } from '../../../src/server/WacAllowHttpHandler'; import { WacAllowHttpHandler } from '../../../src/server/WacAllowHttpHandler';
import { NotModifiedHttpError } from '../../../src/util/errors/NotModifiedHttpError';
import { IdentifierMap, IdentifierSetMultiMap } from '../../../src/util/map/IdentifierMap'; import { IdentifierMap, IdentifierSetMultiMap } from '../../../src/util/map/IdentifierMap';
import { ACL, AUTH } from '../../../src/util/Vocabularies'; import { ACL, AUTH } from '../../../src/util/Vocabularies';
@ -77,6 +78,43 @@ describe('A WacAllowHttpHandler', (): void => {
}); });
}); });
it('adds permission metadata for 304 responses.', async(): Promise<void> => {
permissionReader.handleSafe.mockResolvedValueOnce(new IdentifierMap(
[[ target, { read: true, write: true, append: false }]],
));
source.handleSafe.mockRejectedValueOnce(new NotModifiedHttpError());
let error: unknown;
try {
await handler.handle({ operation, request, response });
} catch (err: unknown) {
error = err;
}
expect(NotModifiedHttpError.isInstance(error)).toBe(true);
expect((error as NotModifiedHttpError).metadata.getAll(AUTH.terms.userMode))
.toEqualRdfTermArray([ ACL.terms.Read, ACL.terms.Write ]);
expect((error as NotModifiedHttpError).metadata.getAll(AUTH.terms.publicMode))
.toEqualRdfTermArray([ ACL.terms.Read, ACL.terms.Write ]);
expect(source.handleSafe).toHaveBeenCalledTimes(1);
expect(source.handleSafe).toHaveBeenLastCalledWith({ operation, request, response });
expect(credentialsExtractor.handleSafe).toHaveBeenCalledTimes(1);
expect(credentialsExtractor.handleSafe).toHaveBeenLastCalledWith(request);
expect(modesExtractor.handleSafe).toHaveBeenCalledTimes(1);
expect(modesExtractor.handleSafe).toHaveBeenLastCalledWith(operation);
expect(permissionReader.handleSafe).toHaveBeenCalledTimes(1);
expect(permissionReader.handleSafe).toHaveBeenLastCalledWith({
credentials: await credentialsExtractor.handleSafe.mock.results[0].value,
requestedModes: await modesExtractor.handleSafe.mock.results[0].value,
});
});
it('rethrows errors.', async(): Promise<void> => {
const error = new Error('bad data');
source.handleSafe.mockRejectedValueOnce(error);
await expect(handler.handle({ operation, request, response })).rejects.toThrow(error);
});
it('determines public permissions separately in case of an authenticated request.', async(): Promise<void> => { it('determines public permissions separately in case of an authenticated request.', async(): Promise<void> => {
credentialsExtractor.handleSafe.mockResolvedValue({ agent: { webId: 'http://example.com/#me' }}); credentialsExtractor.handleSafe.mockResolvedValue({ agent: { webId: 'http://example.com/#me' }});
permissionReader.handleSafe.mockResolvedValueOnce(new IdentifierMap( permissionReader.handleSafe.mockResolvedValueOnce(new IdentifierMap(