From cc1c3d9223aede72232bb716be20030406179297 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Fri, 27 Aug 2021 11:23:27 +0200 Subject: [PATCH] feat: Support JSON errors The IDP behaviour has been changed to move all error related knowledge to the IdentityProviderHttpHandler instead of managing it in the Interactionhandlers. --- .../converters/errors.json | 4 + .../representation-conversion/default.json | 1 + src/identity/IdentityProviderHttpHandler.ts | 47 +++++++-- .../email-password/EmailPasswordUtil.ts | 24 ----- .../handler/ForgotPasswordHandler.ts | 15 +-- .../handler/InteractionHandler.ts | 7 +- .../email-password/handler/LoginHandler.ts | 35 +++---- .../handler/RegistrationHandler.ts | 25 ++--- .../handler/ResetPasswordHandler.ts | 28 +++--- .../routing/BasicInteractionRoute.ts | 17 ++-- .../interaction/routing/InteractionRoute.ts | 2 +- .../interaction/util/IdpInteractionError.ts | 19 ---- src/index.ts | 2 +- .../conversion/ErrorToJsonConverter.ts | 48 +++++++++ .../conversion/ErrorToQuadConverter.ts | 9 +- .../conversion/ErrorToTemplateConverter.ts | 10 +- src/util/StreamUtil.ts | 15 +++ .../identity/email-password/confirm.html.ejs | 4 +- .../email-password/forgot-password.html.ejs | 4 +- .../identity/email-password/login.html.ejs | 8 +- .../identity/email-password/register.html.ejs | 14 +-- .../email-password/reset-password.html.ejs | 4 +- test/integration/DynamicPods.test.ts | 3 +- test/integration/Identity.test.ts | 6 +- test/integration/Subdomains.test.ts | 3 +- .../IdentityProviderHttpHandler.test.ts | 70 ++++++++++++- .../email-password/EmailPasswordUtil.test.ts | 48 --------- .../handler/LoginHandler.test.ts | 7 +- .../handler/RegistrationHandler.test.ts | 11 +-- .../handler/ResetPasswordHandler.test.ts | 7 -- .../routing/BasicInteractionRoute.test.ts | 15 ++- .../conversion/ErrorToJsonConverter.test.ts | 99 +++++++++++++++++++ .../conversion/ErrorToQuadConverter.test.ts | 8 -- .../ErrorToTemplateConverter.test.ts | 8 -- test/unit/util/StreamUtil.test.ts | 14 ++- 35 files changed, 368 insertions(+), 273 deletions(-) delete mode 100644 src/identity/interaction/util/IdpInteractionError.ts create mode 100644 src/storage/conversion/ErrorToJsonConverter.ts create mode 100644 test/unit/storage/conversion/ErrorToJsonConverter.test.ts diff --git a/config/util/representation-conversion/converters/errors.json b/config/util/representation-conversion/converters/errors.json index 2467acdad..8a5b09d8d 100644 --- a/config/util/representation-conversion/converters/errors.json +++ b/config/util/representation-conversion/converters/errors.json @@ -1,6 +1,10 @@ { "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^1.0.0/components/context.jsonld", "@graph": [ + { + "@id": "urn:solid-server:default:ErrorToJsonConverter", + "@type": "ErrorToJsonConverter" + }, { "@id": "urn:solid-server:default:ErrorToQuadConverter", "@type": "ErrorToQuadConverter" diff --git a/config/util/representation-conversion/default.json b/config/util/representation-conversion/default.json index 7eec43e87..5e5f8b27c 100644 --- a/config/util/representation-conversion/default.json +++ b/config/util/representation-conversion/default.json @@ -31,6 +31,7 @@ { "@id": "urn:solid-server:default:RdfToQuadConverter" }, { "@id": "urn:solid-server:default:QuadToRdfConverter" }, { "@id": "urn:solid-server:default:ContainerToTemplateConverter" }, + { "@id": "urn:solid-server:default:ErrorToJsonConverter" }, { "@id": "urn:solid-server:default:ErrorToQuadConverter" }, { "@id": "urn:solid-server:default:ErrorToTemplateConverter" }, { "@id": "urn:solid-server:default:MarkdownToHtmlConverter" }, diff --git a/src/identity/IdentityProviderHttpHandler.ts b/src/identity/IdentityProviderHttpHandler.ts index 52094f612..c41859d0b 100644 --- a/src/identity/IdentityProviderHttpHandler.ts +++ b/src/identity/IdentityProviderHttpHandler.ts @@ -1,11 +1,11 @@ import type { ErrorHandler } from '../ldp/http/ErrorHandler'; import type { RequestParser } from '../ldp/http/RequestParser'; -import { OkResponseDescription } from '../ldp/http/response/OkResponseDescription'; import { RedirectResponseDescription } from '../ldp/http/response/RedirectResponseDescription'; -import type { ResponseDescription } from '../ldp/http/response/ResponseDescription'; +import { ResponseDescription } from '../ldp/http/response/ResponseDescription'; import type { ResponseWriter } from '../ldp/http/ResponseWriter'; import type { Operation } from '../ldp/operations/Operation'; import { BasicRepresentation } from '../ldp/representation/BasicRepresentation'; +import type { Representation } from '../ldp/representation/Representation'; import { getLoggerFor } from '../logging/LogUtil'; import { BaseHttpHandler } from '../server/BaseHttpHandler'; import type { BaseHttpHandlerArgs } from '../server/BaseHttpHandler'; @@ -15,7 +15,8 @@ import type { RepresentationConverter } from '../storage/conversion/Representati import { APPLICATION_JSON } from '../util/ContentTypes'; import { BadRequestHttpError } from '../util/errors/BadRequestHttpError'; import { joinUrl, trimTrailingSlashes } from '../util/PathUtil'; -import { addTemplateMetadata } from '../util/ResourceUtil'; +import { addTemplateMetadata, cloneRepresentation } from '../util/ResourceUtil'; +import { readJsonStream } from '../util/StreamUtil'; import type { ProviderFactory } from './configuration/ProviderFactory'; import type { Interaction } from './interaction/email-password/handler/InteractionHandler'; import type { InteractionRoute, TemplatedInteractionResult } from './interaction/routing/InteractionRoute'; @@ -123,6 +124,9 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { return; } + // Cloning input data so it can be sent back in case of errors + let clone: Representation | undefined; + // IDP handlers expect JSON data if (operation.body) { const args = { @@ -131,9 +135,14 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { identifier: operation.target, }; operation.body = await this.converter.handleSafe(args); + clone = await cloneRepresentation(operation.body); } const result = await route.handleOperation(operation, oidcInteraction); + + // Reset the body so it can be reused when needed for output + operation.body = clone; + return this.handleInteractionResult(operation, request, result, oidcInteraction); } @@ -172,9 +181,27 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { // Create a redirect URL with the OIDC library const location = await this.interactionCompleter.handleSafe({ ...result.details, request }); responseDescription = new RedirectResponseDescription(location); + } else if (result.type === 'error') { + // We want to show the errors on the original page in case of html interactions, so we can't just throw them here + const preferences = { type: { [APPLICATION_JSON]: 1 }}; + const response = await this.errorHandler.handleSafe({ error: result.error, preferences }); + const details = await readJsonStream(response.data!); + + // Add the input data to the JSON response; + if (operation.body) { + details.prefilled = await readJsonStream(operation.body.data); + + // Don't send passwords back + delete details.prefilled.password; + delete details.prefilled.confirmPassword; + } + + responseDescription = + await this.handleResponseResult(details, operation, result.templateFiles, oidcInteraction, response.statusCode); } else { // Convert the response object to a data stream - responseDescription = await this.handleResponseResult(result, operation, oidcInteraction); + responseDescription = + await this.handleResponseResult(result.details ?? {}, operation, result.templateFiles, oidcInteraction); } return responseDescription; @@ -184,19 +211,19 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { * Converts an InteractionResponseResult to a ResponseDescription by first converting to a Representation * and applying necessary conversions. */ - private async handleResponseResult(result: TemplatedInteractionResult, operation: Operation, - oidcInteraction?: Interaction): Promise { - // Convert the object to a valid JSON representation + private async handleResponseResult(details: Record, operation: Operation, + templateFiles: Record, oidcInteraction?: Interaction, statusCode = 200): + Promise { const json = { + ...details, apiVersion: API_VERSION, - ...result.details, authenticating: Boolean(oidcInteraction), controls: this.controls, }; const representation = new BasicRepresentation(JSON.stringify(json), operation.target, APPLICATION_JSON); // Template metadata is required for conversion - for (const [ type, templateFile ] of Object.entries(result.templateFiles)) { + for (const [ type, templateFile ] of Object.entries(templateFiles)) { addTemplateMetadata(representation.metadata, templateFile, type); } @@ -204,7 +231,7 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { const args = { representation, preferences: operation.preferences, identifier: operation.target }; const converted = await this.converter.handleSafe(args); - return new OkResponseDescription(converted.metadata, converted.data); + return new ResponseDescription(statusCode, converted.metadata, converted.data); } /** diff --git a/src/identity/interaction/email-password/EmailPasswordUtil.ts b/src/identity/interaction/email-password/EmailPasswordUtil.ts index 7ace30de0..8c6954af4 100644 --- a/src/identity/interaction/email-password/EmailPasswordUtil.ts +++ b/src/identity/interaction/email-password/EmailPasswordUtil.ts @@ -1,28 +1,4 @@ import assert from 'assert'; -import { createErrorMessage } from '../../../util/errors/ErrorUtil'; -import { HttpError } from '../../../util/errors/HttpError'; -import { IdpInteractionError } from '../util/IdpInteractionError'; - -/** - * Throws an IdpInteractionError with contents depending on the type of input error. - * Default status code is 500 and default error message is 'Unknown Error'. - * @param error - Error to create an IdPInteractionError from. - * @param prefilled - Prefilled data for IdpInteractionError. - */ -export function throwIdpInteractionError(error: unknown, prefilled: Record = {}): never { - if (IdpInteractionError.isInstance(error)) { - if (Object.keys(prefilled).length > 0) { - const { statusCode, message } = error; - throw new IdpInteractionError(statusCode, message, { ...error.prefilled, ...prefilled }, { cause: error }); - } else { - throw error; - } - } else if (HttpError.isInstance(error)) { - throw new IdpInteractionError(error.statusCode, error.message, prefilled, { cause: error }); - } else { - throw new IdpInteractionError(500, createErrorMessage(error), prefilled, { cause: error }); - } -} /** * Asserts that `password` is a string that matches `confirmPassword`. diff --git a/src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts b/src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts index e8e6e7168..835562c3e 100644 --- a/src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts +++ b/src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts @@ -4,7 +4,6 @@ import { ensureTrailingSlash, joinUrl } from '../../../../util/PathUtil'; import { readJsonStream } from '../../../../util/StreamUtil'; import type { TemplateEngine } from '../../../../util/templates/TemplateEngine'; import type { EmailSender } from '../../util/EmailSender'; -import { throwIdpInteractionError } from '../EmailPasswordUtil'; import type { AccountStore } from '../storage/AccountStore'; import { InteractionHandler } from './InteractionHandler'; import type { InteractionResponseResult, InteractionHandlerInput } from './InteractionHandler'; @@ -39,16 +38,12 @@ export class ForgotPasswordHandler extends InteractionHandler { } public async handle({ operation }: InteractionHandlerInput): Promise> { - try { - // Validate incoming data - const { email } = await readJsonStream(operation.body!.data); - assert(typeof email === 'string' && email.length > 0, 'Email required'); + // Validate incoming data + const { email } = await readJsonStream(operation.body!.data); + assert(typeof email === 'string' && email.length > 0, 'Email required'); - await this.resetPassword(email); - return { type: 'response', details: { email }}; - } catch (err: unknown) { - throwIdpInteractionError(err, {}); - } + await this.resetPassword(email); + return { type: 'response', details: { email }}; } /** diff --git a/src/identity/interaction/email-password/handler/InteractionHandler.ts b/src/identity/interaction/email-password/handler/InteractionHandler.ts index 528fd3c80..c0d9a9305 100644 --- a/src/identity/interaction/email-password/handler/InteractionHandler.ts +++ b/src/identity/interaction/email-password/handler/InteractionHandler.ts @@ -20,7 +20,7 @@ export interface InteractionHandlerInput { oidcInteraction?: Interaction; } -export type InteractionHandlerResult = InteractionResponseResult | InteractionCompleteResult; +export type InteractionHandlerResult = InteractionResponseResult | InteractionCompleteResult | InteractionErrorResult; export interface InteractionResponseResult> { type: 'response'; @@ -32,6 +32,11 @@ export interface InteractionCompleteResult { details: InteractionCompleterParams; } +export interface InteractionErrorResult { + type: 'error'; + error: Error; +} + /** * Handler used for IDP interactions. * Only supports JSON data. diff --git a/src/identity/interaction/email-password/handler/LoginHandler.ts b/src/identity/interaction/email-password/handler/LoginHandler.ts index f852260ac..c8996de58 100644 --- a/src/identity/interaction/email-password/handler/LoginHandler.ts +++ b/src/identity/interaction/email-password/handler/LoginHandler.ts @@ -2,7 +2,6 @@ import assert from 'assert'; import type { Operation } from '../../../../ldp/operations/Operation'; import { getLoggerFor } from '../../../../logging/LogUtil'; import { readJsonStream } from '../../../../util/StreamUtil'; -import { throwIdpInteractionError } from '../EmailPasswordUtil'; import type { AccountStore } from '../storage/AccountStore'; import { InteractionHandler } from './InteractionHandler'; import type { InteractionCompleteResult, InteractionHandlerInput } from './InteractionHandler'; @@ -22,34 +21,26 @@ export class LoginHandler extends InteractionHandler { public async handle({ operation }: InteractionHandlerInput): Promise { const { email, password, remember } = await this.parseInput(operation); - try { - // Try to log in, will error if email/password combination is invalid - const webId = await this.accountStore.authenticate(email, password); - this.logger.debug(`Logging in user ${email}`); - return { - type: 'complete', - details: { webId, shouldRemember: remember }, - }; - } catch (err: unknown) { - throwIdpInteractionError(err, { email }); - } + // Try to log in, will error if email/password combination is invalid + const webId = await this.accountStore.authenticate(email, password); + this.logger.debug(`Logging in user ${email}`); + return { + type: 'complete', + details: { webId, shouldRemember: remember }, + }; } /** * Parses and validates the input form data. - * Will throw an {@link IdpInteractionError} in case something is wrong. + * Will throw an error in case something is wrong. * All relevant data that was correct up to that point will be prefilled. */ private async parseInput(operation: Operation): Promise<{ email: string; password: string; remember: boolean }> { const prefilled: Record = {}; - try { - const { email, password, remember } = await readJsonStream(operation.body!.data); - assert(typeof email === 'string' && email.length > 0, 'Email required'); - prefilled.email = email; - assert(typeof password === 'string' && password.length > 0, 'Password required'); - return { email, password, remember: Boolean(remember) }; - } catch (err: unknown) { - throwIdpInteractionError(err, prefilled); - } + const { email, password, remember } = await readJsonStream(operation.body!.data); + assert(typeof email === 'string' && email.length > 0, 'Email required'); + prefilled.email = email; + assert(typeof password === 'string' && password.length > 0, 'Password required'); + return { email, password, remember: Boolean(remember) }; } } diff --git a/src/identity/interaction/email-password/handler/RegistrationHandler.ts b/src/identity/interaction/email-password/handler/RegistrationHandler.ts index c0f1c0b54..68a623b32 100644 --- a/src/identity/interaction/email-password/handler/RegistrationHandler.ts +++ b/src/identity/interaction/email-password/handler/RegistrationHandler.ts @@ -8,7 +8,7 @@ import type { PodSettings } from '../../../../pods/settings/PodSettings'; import { joinUrl } from '../../../../util/PathUtil'; import { readJsonStream } from '../../../../util/StreamUtil'; import type { OwnershipValidator } from '../../../ownership/OwnershipValidator'; -import { assertPassword, throwIdpInteractionError } from '../EmailPasswordUtil'; +import { assertPassword } from '../EmailPasswordUtil'; import type { AccountStore } from '../storage/AccountStore'; import type { InteractionResponseResult, InteractionHandlerInput } from './InteractionHandler'; import { InteractionHandler } from './InteractionHandler'; @@ -105,15 +105,8 @@ export class RegistrationHandler extends InteractionHandler { public async handle({ operation }: InteractionHandlerInput): Promise> { const result = await this.parseInput(operation); - - try { - const details = await this.register(result); - return { type: 'response', details }; - } catch (error: unknown) { - // Don't expose the password field - delete result.password; - throwIdpInteractionError(error, result as Record); - } + const details = await this.register(result); + return { type: 'response', details }; } /** @@ -188,15 +181,11 @@ export class RegistrationHandler extends InteractionHandler { private async parseInput(operation: Operation): Promise { const parsed = await readJsonStream(operation.body!.data); const prefilled: Record = {}; - try { - for (const [ key, value ] of Object.entries(parsed)) { - assert(!Array.isArray(value), `Unexpected multiple values for ${key}.`); - prefilled[key] = typeof value === 'string' ? value.trim() : value; - } - return this.validateInput(prefilled); - } catch (err: unknown) { - throwIdpInteractionError(err, prefilled); + for (const [ key, value ] of Object.entries(parsed)) { + assert(!Array.isArray(value), `Unexpected multiple values for ${key}.`); + prefilled[key] = typeof value === 'string' ? value.trim() : value; } + return this.validateInput(prefilled); } /** diff --git a/src/identity/interaction/email-password/handler/ResetPasswordHandler.ts b/src/identity/interaction/email-password/handler/ResetPasswordHandler.ts index 427a30308..27a0917d5 100644 --- a/src/identity/interaction/email-password/handler/ResetPasswordHandler.ts +++ b/src/identity/interaction/email-password/handler/ResetPasswordHandler.ts @@ -1,7 +1,7 @@ import assert from 'assert'; import { getLoggerFor } from '../../../../logging/LogUtil'; import { readJsonStream } from '../../../../util/StreamUtil'; -import { assertPassword, throwIdpInteractionError } from '../EmailPasswordUtil'; +import { assertPassword } from '../EmailPasswordUtil'; import type { AccountStore } from '../storage/AccountStore'; import type { InteractionResponseResult, InteractionHandlerInput } from './InteractionHandler'; import { InteractionHandler } from './InteractionHandler'; @@ -21,22 +21,18 @@ export class ResetPasswordHandler extends InteractionHandler { } public async handle({ operation }: InteractionHandlerInput): Promise { - try { - // Extract record ID from request URL - const recordId = /\/([^/]+)$/u.exec(operation.target.path)?.[1]; - // Validate input data - const { password, confirmPassword } = await readJsonStream(operation.body!.data); - assert( - typeof recordId === 'string' && recordId.length > 0, - 'Invalid request. Open the link from your email again', - ); - assertPassword(password, confirmPassword); + // Extract record ID from request URL + const recordId = /\/([^/]+)$/u.exec(operation.target.path)?.[1]; + // Validate input data + const { password, confirmPassword } = await readJsonStream(operation.body!.data); + assert( + typeof recordId === 'string' && recordId.length > 0, + 'Invalid request. Open the link from your email again', + ); + assertPassword(password, confirmPassword); - await this.resetPassword(recordId, password); - return { type: 'response' }; - } catch (error: unknown) { - throwIdpInteractionError(error); - } + await this.resetPassword(recordId, password); + return { type: 'response' }; } /** diff --git a/src/identity/interaction/routing/BasicInteractionRoute.ts b/src/identity/interaction/routing/BasicInteractionRoute.ts index fa8180bff..53f06e30d 100644 --- a/src/identity/interaction/routing/BasicInteractionRoute.ts +++ b/src/identity/interaction/routing/BasicInteractionRoute.ts @@ -1,13 +1,12 @@ import type { Operation } from '../../../ldp/operations/Operation'; import { BadRequestHttpError } from '../../../util/errors/BadRequestHttpError'; -import { createErrorMessage } from '../../../util/errors/ErrorUtil'; +import { createErrorMessage, isError } from '../../../util/errors/ErrorUtil'; +import { InternalServerError } from '../../../util/errors/InternalServerError'; import { trimTrailingSlashes } from '../../../util/PathUtil'; import type { - InteractionResponseResult, InteractionHandler, Interaction, } from '../email-password/handler/InteractionHandler'; -import { IdpInteractionError } from '../util/IdpInteractionError'; import type { InteractionRoute, TemplatedInteractionResult } from './InteractionRoute'; /** @@ -84,14 +83,10 @@ export class BasicInteractionRoute implements InteractionRoute { try { const result = await this.handler.handleSafe({ operation, oidcInteraction }); return { ...result, templateFiles: this.responseTemplates }; - } catch (error: unknown) { - // Render error in the view - const errorMessage = createErrorMessage(error); - const result: InteractionResponseResult = { type: 'response', details: { errorMessage }}; - if (IdpInteractionError.isInstance(error)) { - result.details!.prefilled = error.prefilled; - } - return { ...result, templateFiles: this.viewTemplates }; + } catch (err: unknown) { + const error = isError(err) ? err : new InternalServerError(createErrorMessage(err)); + // Potentially render the error in the view + return { type: 'error', error, templateFiles: this.viewTemplates }; } default: throw new BadRequestHttpError(`Unsupported request: ${operation.method} ${operation.target.path}`); diff --git a/src/identity/interaction/routing/InteractionRoute.ts b/src/identity/interaction/routing/InteractionRoute.ts index f550a493d..5f415744e 100644 --- a/src/identity/interaction/routing/InteractionRoute.ts +++ b/src/identity/interaction/routing/InteractionRoute.ts @@ -1,7 +1,7 @@ import type { Operation } from '../../../ldp/operations/Operation'; import type { Interaction, InteractionHandlerResult } from '../email-password/handler/InteractionHandler'; -export type TemplatedInteractionResult = InteractionHandlerResult & { +export type TemplatedInteractionResult = T & { templateFiles: Record; }; diff --git a/src/identity/interaction/util/IdpInteractionError.ts b/src/identity/interaction/util/IdpInteractionError.ts deleted file mode 100644 index b87c364d5..000000000 --- a/src/identity/interaction/util/IdpInteractionError.ts +++ /dev/null @@ -1,19 +0,0 @@ -import type { HttpErrorOptions } from '../../../util/errors/HttpError'; -import { HttpError } from '../../../util/errors/HttpError'; - -/** - * An error made for IDP Interactions. It allows a function to set the prefilled - * information that would be included in a response UI render. - */ -export class IdpInteractionError extends HttpError { - public readonly prefilled: Record; - - public constructor(status: number, message: string, prefilled: Record, options?: HttpErrorOptions) { - super(status, 'IdpInteractionError', message, options); - this.prefilled = prefilled; - } - - public static isInstance(error: unknown): error is IdpInteractionError { - return HttpError.isInstance(error) && typeof (error as any).prefilled === 'object'; - } -} diff --git a/src/index.ts b/src/index.ts index 521ed3afe..1162c4d50 100644 --- a/src/index.ts +++ b/src/index.ts @@ -48,7 +48,6 @@ export * from './identity/interaction/routing/InteractionRoute'; // Identity/Interaction/Util export * from './identity/interaction/util/BaseEmailSender'; export * from './identity/interaction/util/EmailSender'; -export * from './identity/interaction/util/IdpInteractionError'; export * from './identity/interaction/util/InteractionCompleter'; // Identity/Interaction @@ -232,6 +231,7 @@ export * from './storage/conversion/ContainerToTemplateConverter'; export * from './storage/conversion/ContentTypeReplacer'; export * from './storage/conversion/ConversionUtil'; export * from './storage/conversion/DynamicJsonToTemplateConverter'; +export * from './storage/conversion/ErrorToJsonConverter'; export * from './storage/conversion/ErrorToQuadConverter'; export * from './storage/conversion/ErrorToTemplateConverter'; export * from './storage/conversion/FormToJsonConverter'; diff --git a/src/storage/conversion/ErrorToJsonConverter.ts b/src/storage/conversion/ErrorToJsonConverter.ts new file mode 100644 index 000000000..c40b3aa83 --- /dev/null +++ b/src/storage/conversion/ErrorToJsonConverter.ts @@ -0,0 +1,48 @@ +import { BasicRepresentation } from '../../ldp/representation/BasicRepresentation'; +import type { Representation } from '../../ldp/representation/Representation'; +import { APPLICATION_JSON, INTERNAL_ERROR } from '../../util/ContentTypes'; +import { HttpError } from '../../util/errors/HttpError'; +import { getSingleItem } from '../../util/StreamUtil'; +import type { RepresentationConverterArgs } from './RepresentationConverter'; +import { TypedRepresentationConverter } from './TypedRepresentationConverter'; + +/** + * Converts an Error object to JSON by copying its fields. + */ +export class ErrorToJsonConverter extends TypedRepresentationConverter { + public constructor() { + super(INTERNAL_ERROR, APPLICATION_JSON); + } + + public async handle({ representation }: RepresentationConverterArgs): Promise { + const error = await getSingleItem(representation.data) as Error; + + const result: Record = { + name: error.name, + message: error.message, + }; + + if (HttpError.isInstance(error)) { + result.statusCode = error.statusCode; + result.errorCode = error.errorCode; + if (error.details) { + try { + // The details might not be serializable + JSON.stringify(error.details); + result.details = error.details; + } catch { + // Do not store the details + } + } + } else { + result.statusCode = 500; + } + + if (error.stack) { + result.stack = error.stack; + } + + // Update the content-type to JSON + return new BasicRepresentation(JSON.stringify(result), representation.metadata, APPLICATION_JSON); + } +} diff --git a/src/storage/conversion/ErrorToQuadConverter.ts b/src/storage/conversion/ErrorToQuadConverter.ts index 9591c236b..8b146db47 100644 --- a/src/storage/conversion/ErrorToQuadConverter.ts +++ b/src/storage/conversion/ErrorToQuadConverter.ts @@ -1,9 +1,8 @@ -import arrayifyStream from 'arrayify-stream'; import { BasicRepresentation } from '../../ldp/representation/BasicRepresentation'; import type { Representation } from '../../ldp/representation/Representation'; import { RepresentationMetadata } from '../../ldp/representation/RepresentationMetadata'; import { INTERNAL_ERROR, INTERNAL_QUADS } from '../../util/ContentTypes'; -import { InternalServerError } from '../../util/errors/InternalServerError'; +import { getSingleItem } from '../../util/StreamUtil'; import { DC, SOLID_ERROR } from '../../util/Vocabularies'; import type { RepresentationConverterArgs } from './RepresentationConverter'; import { TypedRepresentationConverter } from './TypedRepresentationConverter'; @@ -17,11 +16,7 @@ export class ErrorToQuadConverter extends TypedRepresentationConverter { } public async handle({ identifier, representation }: RepresentationConverterArgs): Promise { - const errors = await arrayifyStream(representation.data); - if (errors.length !== 1) { - throw new InternalServerError('Only single errors are supported.'); - } - const error = errors[0] as Error; + const error = await getSingleItem(representation.data) as Error; // A metadata object makes it easier to add triples due to the utility functions const data = new RepresentationMetadata(identifier); diff --git a/src/storage/conversion/ErrorToTemplateConverter.ts b/src/storage/conversion/ErrorToTemplateConverter.ts index 61b5932db..e9ee5e19d 100644 --- a/src/storage/conversion/ErrorToTemplateConverter.ts +++ b/src/storage/conversion/ErrorToTemplateConverter.ts @@ -1,11 +1,10 @@ import assert from 'assert'; -import arrayifyStream from 'arrayify-stream'; import { BasicRepresentation } from '../../ldp/representation/BasicRepresentation'; import type { Representation } from '../../ldp/representation/Representation'; import { INTERNAL_ERROR } from '../../util/ContentTypes'; import { HttpError } from '../../util/errors/HttpError'; -import { InternalServerError } from '../../util/errors/InternalServerError'; import { modulePathPlaceholder } from '../../util/PathUtil'; +import { getSingleItem } from '../../util/StreamUtil'; import type { TemplateEngine } from '../../util/templates/TemplateEngine'; import type { RepresentationConverterArgs } from './RepresentationConverter'; import { TypedRepresentationConverter } from './TypedRepresentationConverter'; @@ -57,12 +56,7 @@ export class ErrorToTemplateConverter extends TypedRepresentationConverter { } public async handle({ representation }: RepresentationConverterArgs): Promise { - // Obtain the error from the representation stream - const errors = await arrayifyStream(representation.data); - if (errors.length !== 1) { - throw new InternalServerError('Only single errors are supported.'); - } - const error = errors[0] as Error; + const error = await getSingleItem(representation.data) as Error; // Render the error description using an error-specific template let description: string | undefined; diff --git a/src/util/StreamUtil.ts b/src/util/StreamUtil.ts index 4f3d083d1..f76c8e498 100644 --- a/src/util/StreamUtil.ts +++ b/src/util/StreamUtil.ts @@ -7,6 +7,7 @@ import { Store } from 'n3'; import pump from 'pump'; import { getLoggerFor } from '../logging/LogUtil'; import { isHttpRequest } from '../server/HttpRequest'; +import { InternalServerError } from './errors/InternalServerError'; import type { Guarded } from './GuardedStream'; import { guardStream } from './GuardedStream'; @@ -48,6 +49,20 @@ export async function readJsonStream(stream: Readable): Promise return JSON.parse(body); } +/** + * Converts the stream to a single object. + * This assumes the stream is in object mode and only contains a single element, + * otherwise an error will be thrown. + * @param stream - Object stream with single entry. + */ +export async function getSingleItem(stream: Readable): Promise { + const items = await arrayifyStream(stream); + if (items.length !== 1) { + throw new InternalServerError('Expected a stream with a single object.'); + } + return items[0]; +} + // These error messages usually indicate expected behaviour so should not give a warning. // We compare against the error message instead of the code // since the second one is from an external library that does not assign an error code. diff --git a/templates/identity/email-password/confirm.html.ejs b/templates/identity/email-password/confirm.html.ejs index 46c9ba92b..01797c1c7 100644 --- a/templates/identity/email-password/confirm.html.ejs +++ b/templates/identity/email-password/confirm.html.ejs @@ -1,8 +1,8 @@

Authorize

You are authorizing an application to access your Pod.

- <% if (locals.errorMessage) { %> -

<%= errorMessage %>

+ <% if (locals.message) { %> +

<%= message %>

<% } %>
diff --git a/templates/identity/email-password/forgot-password.html.ejs b/templates/identity/email-password/forgot-password.html.ejs index dea8a8d5d..5ed4412cd 100644 --- a/templates/identity/email-password/forgot-password.html.ejs +++ b/templates/identity/email-password/forgot-password.html.ejs @@ -1,7 +1,7 @@

Forgot password

- <% if (locals.errorMessage) { %> -

<%= errorMessage %>

+ <% if (locals.message) { %> +

<%= message %>

<% } %>
diff --git a/templates/identity/email-password/login.html.ejs b/templates/identity/email-password/login.html.ejs index b6352431d..1d28d70a4 100644 --- a/templates/identity/email-password/login.html.ejs +++ b/templates/identity/email-password/login.html.ejs @@ -1,9 +1,9 @@

Log in

- <% const safePrefilled = locals.prefilled || {}; %> + <% prefilled = locals.prefilled || {}; %> - <% if (locals.errorMessage) { %> -

<%= errorMessage %>

+ <% if (locals.message) { %> +

<%= message %>

<% } %>
@@ -11,7 +11,7 @@
  1. - +
  2. diff --git a/templates/identity/email-password/register.html.ejs b/templates/identity/email-password/register.html.ejs index 3c1b5d4c5..7ce12accb 100644 --- a/templates/identity/email-password/register.html.ejs +++ b/templates/identity/email-password/register.html.ejs @@ -1,10 +1,10 @@

    Sign up

    <% const isBlankForm = !('prefilled' in locals); %> - <% const safePrefilled = locals.prefilled || {}; %> + <% prefilled = locals.prefilled || {}; %> - <% if (locals.errorMessage) { %> -

    Error: <%= errorMessage %>

    + <% if (locals.message) { %> +

    Error: <%= message %>

    <% } %>
    @@ -20,7 +20,7 @@

  3. @@ -36,7 +36,7 @@

    1. - +
      3. @@ -83,7 +83,7 @@
      1. - +
        diff --git a/templates/identity/email-password/reset-password.html.ejs b/templates/identity/email-password/reset-password.html.ejs index 5cd43397b..a5c605eb8 100644 --- a/templates/identity/email-password/reset-password.html.ejs +++ b/templates/identity/email-password/reset-password.html.ejs @@ -1,7 +1,7 @@

        Reset password

        - <% if (locals.errorMessage) { %> -

        <%= errorMessage %>

        + <% if (locals.message) { %> +

        <%= message %>

        <% } %>
        diff --git a/test/integration/DynamicPods.test.ts b/test/integration/DynamicPods.test.ts index eb6447a38..4f3ff971e 100644 --- a/test/integration/DynamicPods.test.ts +++ b/test/integration/DynamicPods.test.ts @@ -114,8 +114,7 @@ describe.each(configs)('A dynamic pod server with template config %s', (template headers: { 'content-type': 'application/json' }, body: JSON.stringify(settings), }); - // 200 due to there only being a HTML solution right now that only returns 200 - expect(res.status).toBe(200); + expect(res.status).toBe(409); await expect(res.text()).resolves.toContain(`There already is a pod at ${podUrl}`); }); }); diff --git a/test/integration/Identity.test.ts b/test/integration/Identity.test.ts index 799730627..f81d41861 100644 --- a/test/integration/Identity.test.ts +++ b/test/integration/Identity.test.ts @@ -96,7 +96,7 @@ describe('A Solid server with IDP', (): void => { it('sends the form once to receive the registration triple.', async(): Promise => { const res = await postForm(`${baseUrl}idp/register`, formBody); - expect(res.status).toBe(200); + expect(res.status).toBe(400); registrationTriple = extractRegistrationTriple(await res.text(), webId); }); @@ -231,7 +231,7 @@ describe('A Solid server with IDP', (): void => { await state.parseLoginPage(url); const formData = stringify({ email, password }); const res = await state.fetchIdp(url, 'POST', formData, APPLICATION_X_WWW_FORM_URLENCODED); - expect(res.status).toBe(200); + expect(res.status).toBe(500); expect(await res.text()).toContain('Incorrect password'); }); @@ -253,7 +253,7 @@ describe('A Solid server with IDP', (): void => { it('sends the form once to receive the registration triple.', async(): Promise => { const res = await postForm(`${baseUrl}idp/register`, formBody); - expect(res.status).toBe(200); + expect(res.status).toBe(400); registrationTriple = extractRegistrationTriple(await res.text(), webId); }); diff --git a/test/integration/Subdomains.test.ts b/test/integration/Subdomains.test.ts index 98598b489..e59e4449d 100644 --- a/test/integration/Subdomains.test.ts +++ b/test/integration/Subdomains.test.ts @@ -147,8 +147,7 @@ describe.each(stores)('A subdomain server with %s', (name, { storeConfig, teardo headers: { 'content-type': 'application/json' }, body: JSON.stringify(settings), }); - // 200 due to there only being a HTML solution right now that only returns 200 - expect(res.status).toBe(200); + expect(res.status).toBe(409); await expect(res.text()).resolves.toContain(`There already is a resource at ${podUrl}`); }); }); diff --git a/test/unit/identity/IdentityProviderHttpHandler.test.ts b/test/unit/identity/IdentityProviderHttpHandler.test.ts index 871cb235d..e5b2020c6 100644 --- a/test/unit/identity/IdentityProviderHttpHandler.test.ts +++ b/test/unit/identity/IdentityProviderHttpHandler.test.ts @@ -4,8 +4,9 @@ import type { IdentityProviderHttpHandlerArgs } from '../../../src/identity/Iden import { IdentityProviderHttpHandler } from '../../../src/identity/IdentityProviderHttpHandler'; import type { InteractionRoute } from '../../../src/identity/interaction/routing/InteractionRoute'; import type { InteractionCompleter } from '../../../src/identity/interaction/util/InteractionCompleter'; -import type { ErrorHandler } from '../../../src/ldp/http/ErrorHandler'; +import type { ErrorHandler, ErrorHandlerArgs } from '../../../src/ldp/http/ErrorHandler'; import type { RequestParser } from '../../../src/ldp/http/RequestParser'; +import type { ResponseDescription } from '../../../src/ldp/http/response/ResponseDescription'; import type { ResponseWriter } from '../../../src/ldp/http/ResponseWriter'; import type { Operation } from '../../../src/ldp/operations/Operation'; import { BasicRepresentation } from '../../../src/ldp/representation/BasicRepresentation'; @@ -19,7 +20,7 @@ import type { RepresentationConverterArgs, } from '../../../src/storage/conversion/RepresentationConverter'; import { joinUrl } from '../../../src/util/PathUtil'; -import { readableToString } from '../../../src/util/StreamUtil'; +import { guardedStreamFrom, readableToString } from '../../../src/util/StreamUtil'; import { CONTENT_TYPE, SOLID_HTTP, SOLID_META } from '../../../src/util/Vocabularies'; describe('An IdentityProviderHttpHandler', (): void => { @@ -30,7 +31,7 @@ describe('An IdentityProviderHttpHandler', (): void => { const response: HttpResponse = {} as any; let requestParser: jest.Mocked; let providerFactory: jest.Mocked; - let routes: { response: jest.Mocked; complete: jest.Mocked }; + let routes: Record<'response' | 'complete' | 'error', jest.Mocked>; let controls: Record; let interactionCompleter: jest.Mocked; let converter: jest.Mocked; @@ -48,7 +49,7 @@ describe('An IdentityProviderHttpHandler', (): void => { method: req.method!, body: req.method === 'GET' ? undefined : - new BasicRepresentation('', req.headers['content-type'] ?? 'text/plain'), + new BasicRepresentation('{}', req.headers['content-type'] ?? 'text/plain'), preferences: { type: { 'text/html': 1 }}, })), } as any; @@ -81,6 +82,15 @@ describe('An IdentityProviderHttpHandler', (): void => { templateFiles: {}, }), }, + error: { + getControls: jest.fn().mockReturnValue({}), + supportsPath: jest.fn((path: string): boolean => /^\/routeError$/u.test(path)), + handleOperation: jest.fn().mockResolvedValue({ + type: 'error', + error: new Error('test error'), + templateFiles: { 'text/html': '/response' }, + }), + }, }; controls = { response: 'http://test.com/idp/routeResponse' }; @@ -95,7 +105,10 @@ describe('An IdentityProviderHttpHandler', (): void => { interactionCompleter = { handleSafe: jest.fn().mockResolvedValue('http://test.com/idp/auth') } as any; - errorHandler = { handleSafe: jest.fn() } as any; + errorHandler = { handleSafe: jest.fn(({ error }: ErrorHandlerArgs): ResponseDescription => ({ + statusCode: 400, + data: guardedStreamFrom(`{ "name": "${error.name}", "message": "${error.message}" }`), + })) } as any; responseWriter = { handleSafe: jest.fn() } as any; @@ -139,6 +152,53 @@ describe('An IdentityProviderHttpHandler', (): void => { expect(result.metadata?.get(SOLID_META.template)?.value).toBe('/response'); }); + it('creates Representations for InteractionErrorResults.', async(): Promise => { + requestParser.handleSafe.mockResolvedValueOnce({ + target: { path: joinUrl(baseUrl, '/idp/routeError') }, + method: 'POST', + preferences: { type: { 'text/html': 1 }}, + }); + + await expect(handler.handle({ request, response })).resolves.toBeUndefined(); + const operation: Operation = await requestParser.handleSafe.mock.results[0].value; + expect(routes.error.handleOperation).toHaveBeenCalledTimes(1); + expect(routes.error.handleOperation).toHaveBeenLastCalledWith(operation, undefined); + + expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); + const { response: mockResponse, result } = responseWriter.handleSafe.mock.calls[0][0]; + expect(mockResponse).toBe(response); + expect(JSON.parse(await readableToString(result.data!))) + .toEqual({ apiVersion, name: 'Error', message: 'test error', authenticating: false, controls }); + expect(result.statusCode).toBe(400); + expect(result.metadata?.contentType).toBe('text/html'); + expect(result.metadata?.get(SOLID_META.template)?.value).toBe('/response'); + }); + + it('adds a prefilled field in case error requests had a body.', async(): Promise => { + requestParser.handleSafe.mockResolvedValueOnce({ + target: { path: joinUrl(baseUrl, '/idp/routeError') }, + method: 'POST', + body: new BasicRepresentation('{ "key": "val" }', 'application/json'), + preferences: { type: { 'text/html': 1 }}, + }); + + await expect(handler.handle({ request, response })).resolves.toBeUndefined(); + const operation: Operation = await requestParser.handleSafe.mock.results[0].value; + expect(routes.error.handleOperation).toHaveBeenCalledTimes(1); + expect(routes.error.handleOperation).toHaveBeenLastCalledWith(operation, undefined); + expect(operation.body?.metadata.contentType).toBe('application/json'); + + expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); + const { response: mockResponse, result } = responseWriter.handleSafe.mock.calls[0][0]; + expect(mockResponse).toBe(response); + expect(JSON.parse(await readableToString(result.data!))).toEqual( + { apiVersion, name: 'Error', message: 'test error', authenticating: false, controls, prefilled: { key: 'val' }}, + ); + expect(result.statusCode).toBe(400); + expect(result.metadata?.contentType).toBe('text/html'); + expect(result.metadata?.get(SOLID_META.template)?.value).toBe('/response'); + }); + it('indicates to the templates if the request is part of an auth flow.', async(): Promise => { request.url = '/idp/routeResponse'; request.method = 'POST'; diff --git a/test/unit/identity/interaction/email-password/EmailPasswordUtil.test.ts b/test/unit/identity/interaction/email-password/EmailPasswordUtil.test.ts index 3f74896ba..dbcc7c454 100644 --- a/test/unit/identity/interaction/email-password/EmailPasswordUtil.test.ts +++ b/test/unit/identity/interaction/email-password/EmailPasswordUtil.test.ts @@ -1,56 +1,8 @@ import { assertPassword, - throwIdpInteractionError, } from '../../../../../src/identity/interaction/email-password/EmailPasswordUtil'; -import { IdpInteractionError } from '../../../../../src/identity/interaction/util/IdpInteractionError'; -import { NotFoundHttpError } from '../../../../../src/util/errors/NotFoundHttpError'; describe('EmailPasswordUtil', (): void => { - describe('#throwIdpInteractionError', (): void => { - const prefilled = { test: 'data' }; - - it('copies the values of other IdpInteractionErrors.', async(): Promise => { - const error = new IdpInteractionError(404, 'Not found!', { test2: 'data2' }); - expect((): never => throwIdpInteractionError(error, prefilled)).toThrow(expect.objectContaining({ - statusCode: error.statusCode, - message: error.message, - prefilled: { ...error.prefilled, ...prefilled }, - })); - }); - - it('re-throws IdpInteractionErrors if there are no new prefilled values.', async(): Promise => { - const error = new IdpInteractionError(404, 'Not found!', { test2: 'data2' }); - expect((): never => throwIdpInteractionError(error)).toThrow(error); - }); - - it('copies status code and message for HttpErrors.', async(): Promise => { - const error = new NotFoundHttpError('Not found!'); - expect((): never => throwIdpInteractionError(error, prefilled)).toThrow(expect.objectContaining({ - statusCode: error.statusCode, - message: error.message, - prefilled, - })); - }); - - it('copies message for native Errors.', async(): Promise => { - const error = new Error('Error!'); - expect((): never => throwIdpInteractionError(error, prefilled)).toThrow(expect.objectContaining({ - statusCode: 500, - message: error.message, - prefilled, - })); - }); - - it('defaults all values in case a non-native Error object gets thrown.', async(): Promise => { - const error = 'Error!'; - expect((): never => throwIdpInteractionError(error, prefilled)).toThrow(expect.objectContaining({ - statusCode: 500, - message: 'Unknown error: Error!', - prefilled, - })); - }); - }); - describe('#assertPassword', (): void => { it('validates the password against the confirmPassword.', async(): Promise => { expect((): void => assertPassword(undefined, undefined)).toThrow('Please enter a password.'); diff --git a/test/unit/identity/interaction/email-password/handler/LoginHandler.test.ts b/test/unit/identity/interaction/email-password/handler/LoginHandler.test.ts index 24942dfbd..fc7209122 100644 --- a/test/unit/identity/interaction/email-password/handler/LoginHandler.test.ts +++ b/test/unit/identity/interaction/email-password/handler/LoginHandler.test.ts @@ -26,30 +26,25 @@ describe('A LoginHandler', (): void => { input.operation = createPostJsonOperation({}); let prom = handler.handle(input); await expect(prom).rejects.toThrow('Email required'); - await expect(prom).rejects.toThrow(expect.objectContaining({ prefilled: {}})); input.operation = createPostJsonOperation({ email: [ 'a', 'b' ]}); prom = handler.handle(input); await expect(prom).rejects.toThrow('Email required'); - await expect(prom).rejects.toThrow(expect.objectContaining({ prefilled: { }})); }); it('errors on invalid passwords.', async(): Promise => { input.operation = createPostJsonOperation({ email }); let prom = handler.handle(input); await expect(prom).rejects.toThrow('Password required'); - await expect(prom).rejects.toThrow(expect.objectContaining({ prefilled: { email }})); input.operation = createPostJsonOperation({ email, password: [ 'a', 'b' ]}); prom = handler.handle(input); await expect(prom).rejects.toThrow('Password required'); - await expect(prom).rejects.toThrow(expect.objectContaining({ prefilled: { email }})); }); - it('throws an IdpInteractionError if there is a problem.', async(): Promise => { + it('throws an error if there is a problem.', async(): Promise => { input.operation = createPostJsonOperation({ email, password: 'password!' }); (storageAdapter.authenticate as jest.Mock).mockRejectedValueOnce(new Error('auth failed!')); const prom = handler.handle(input); await expect(prom).rejects.toThrow('auth failed!'); - await expect(prom).rejects.toThrow(expect.objectContaining({ prefilled: { email }})); }); it('returns an InteractionCompleteResult when done.', async(): Promise => { diff --git a/test/unit/identity/interaction/email-password/handler/RegistrationHandler.test.ts b/test/unit/identity/interaction/email-password/handler/RegistrationHandler.test.ts index 6301aac17..8e2ad111b 100644 --- a/test/unit/identity/interaction/email-password/handler/RegistrationHandler.test.ts +++ b/test/unit/identity/interaction/email-password/handler/RegistrationHandler.test.ts @@ -2,7 +2,6 @@ import { RegistrationHandler, } from '../../../../../../src/identity/interaction/email-password/handler/RegistrationHandler'; import type { AccountStore } from '../../../../../../src/identity/interaction/email-password/storage/AccountStore'; -import { IdpInteractionError } from '../../../../../../src/identity/interaction/util/IdpInteractionError'; import type { OwnershipValidator } from '../../../../../../src/identity/ownership/OwnershipValidator'; import type { Operation } from '../../../../../../src/ldp/operations/Operation'; import type { ResourceIdentifier } from '../../../../../../src/ldp/representation/ResourceIdentifier'; @@ -278,20 +277,12 @@ describe('A RegistrationHandler', (): void => { expect(accountStore.deleteAccount).toHaveBeenCalledTimes(0); }); - it('throws an IdpInteractionError with all data prefilled if something goes wrong.', async(): Promise => { + it('throws an error if something goes wrong.', async(): Promise => { const params = { email, webId, podName, createPod }; operation = createPostJsonOperation(params); (podManager.createPod as jest.Mock).mockRejectedValueOnce(new Error('pod error')); const prom = handler.handle({ operation }); await expect(prom).rejects.toThrow('pod error'); - await expect(prom).rejects.toThrow(IdpInteractionError); - // Using the cleaned input for prefilled - await expect(prom).rejects.toThrow(expect.objectContaining({ prefilled: { - ...params, - createWebId: false, - register: false, - createPod: true, - }})); }); }); }); diff --git a/test/unit/identity/interaction/email-password/handler/ResetPasswordHandler.test.ts b/test/unit/identity/interaction/email-password/handler/ResetPasswordHandler.test.ts index 82193ad52..543d9990b 100644 --- a/test/unit/identity/interaction/email-password/handler/ResetPasswordHandler.test.ts +++ b/test/unit/identity/interaction/email-password/handler/ResetPasswordHandler.test.ts @@ -54,11 +54,4 @@ describe('A ResetPasswordHandler', (): void => { expect(accountStore.changePassword).toHaveBeenCalledTimes(1); expect(accountStore.changePassword).toHaveBeenLastCalledWith(email, 'password!'); }); - - it('has a default error for non-native errors.', async(): Promise => { - const errorMessage = 'Unknown error: not native'; - operation = createPostJsonOperation({ password: 'password!', confirmPassword: 'password!' }, url); - (accountStore.getForgotPasswordRecord as jest.Mock).mockRejectedValueOnce('not native'); - await expect(handler.handle({ operation })).rejects.toThrow(errorMessage); - }); }); diff --git a/test/unit/identity/interaction/routing/BasicInteractionRoute.test.ts b/test/unit/identity/interaction/routing/BasicInteractionRoute.test.ts index 8c89a3ad9..ef9d0fb1a 100644 --- a/test/unit/identity/interaction/routing/BasicInteractionRoute.test.ts +++ b/test/unit/identity/interaction/routing/BasicInteractionRoute.test.ts @@ -2,8 +2,8 @@ import type { InteractionHandler, } from '../../../../../src/identity/interaction/email-password/handler/InteractionHandler'; import { BasicInteractionRoute } from '../../../../../src/identity/interaction/routing/BasicInteractionRoute'; -import { IdpInteractionError } from '../../../../../src/identity/interaction/util/IdpInteractionError'; import { BadRequestHttpError } from '../../../../../src/util/errors/BadRequestHttpError'; +import { InternalServerError } from '../../../../../src/util/errors/InternalServerError'; describe('A BasicInteractionRoute', (): void => { const path = '^/route$'; @@ -50,19 +50,18 @@ describe('A BasicInteractionRoute', (): void => { expect(handler.handleSafe).toHaveBeenLastCalledWith({ operation: { method: 'POST' }}); }); - it('creates a response result in case the InteractionHandler errors.', async(): Promise => { + it('creates an error result in case the InteractionHandler errors.', async(): Promise => { const error = new Error('bad data'); handler.handleSafe.mockRejectedValueOnce(error); await expect(route.handleOperation({ method: 'POST' } as any)) - .resolves.toEqual({ type: 'response', details: { errorMessage: 'bad data' }, templateFiles: viewTemplates }); + .resolves.toEqual({ type: 'error', error, templateFiles: viewTemplates }); }); - it('adds prefilled data in case the error is an IdpInteractionError.', async(): Promise => { - const error = new IdpInteractionError(400, 'bad data', { name: 'Alice' }); - handler.handleSafe.mockRejectedValueOnce(error); + it('creates an internal error in case of non-native errors.', async(): Promise => { + handler.handleSafe.mockRejectedValueOnce('notAnError'); await expect(route.handleOperation({ method: 'POST' } as any)).resolves.toEqual({ - type: 'response', - details: { errorMessage: 'bad data', prefilled: { name: 'Alice' }}, + type: 'error', + error: new InternalServerError('Unknown error: notAnError'), templateFiles: viewTemplates, }); }); diff --git a/test/unit/storage/conversion/ErrorToJsonConverter.test.ts b/test/unit/storage/conversion/ErrorToJsonConverter.test.ts new file mode 100644 index 000000000..f0c97c44e --- /dev/null +++ b/test/unit/storage/conversion/ErrorToJsonConverter.test.ts @@ -0,0 +1,99 @@ +import { BasicRepresentation } from '../../../../src/ldp/representation/BasicRepresentation'; +import { ErrorToJsonConverter } from '../../../../src/storage/conversion/ErrorToJsonConverter'; +import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError'; +import { readJsonStream } from '../../../../src/util/StreamUtil'; + +describe('An ErrorToJsonConverter', (): void => { + const identifier = { path: 'http://test.com/error' }; + const converter = new ErrorToJsonConverter(); + const preferences = {}; + + it('supports going from errors to json.', async(): Promise => { + await expect(converter.getInputTypes()).resolves.toEqual({ 'internal/error': 1 }); + await expect(converter.getOutputTypes()).resolves.toEqual({ 'application/json': 1 }); + }); + + it('adds all HttpError fields.', async(): Promise => { + const error = new BadRequestHttpError('error text'); + const representation = new BasicRepresentation([ error ], 'internal/error', false); + const prom = converter.handle({ identifier, representation, preferences }); + await expect(prom).resolves.toBeDefined(); + const result = await prom; + expect(result.binary).toBe(true); + expect(result.metadata.contentType).toBe('application/json'); + await expect(readJsonStream(result.data)).resolves.toEqual({ + name: 'BadRequestHttpError', + message: 'error text', + statusCode: 400, + errorCode: 'H400', + stack: error.stack, + }); + }); + + it('copies the HttpError details.', async(): Promise => { + const error = new BadRequestHttpError('error text', { details: { important: 'detail' }}); + const representation = new BasicRepresentation([ error ], 'internal/error', false); + const prom = converter.handle({ identifier, representation, preferences }); + await expect(prom).resolves.toBeDefined(); + const result = await prom; + expect(result.binary).toBe(true); + expect(result.metadata.contentType).toBe('application/json'); + await expect(readJsonStream(result.data)).resolves.toEqual({ + name: 'BadRequestHttpError', + message: 'error text', + statusCode: 400, + errorCode: 'H400', + details: { important: 'detail' }, + stack: error.stack, + }); + }); + + it('does not copy the details if they are not serializable.', async(): Promise => { + const error = new BadRequestHttpError('error text', { details: { object: BigInt(1) }}); + const representation = new BasicRepresentation([ error ], 'internal/error', false); + const prom = converter.handle({ identifier, representation, preferences }); + await expect(prom).resolves.toBeDefined(); + const result = await prom; + expect(result.binary).toBe(true); + expect(result.metadata.contentType).toBe('application/json'); + await expect(readJsonStream(result.data)).resolves.toEqual({ + name: 'BadRequestHttpError', + message: 'error text', + statusCode: 400, + errorCode: 'H400', + stack: error.stack, + }); + }); + + it('defaults to status code 500 for non-HTTP errors.', async(): Promise => { + const error = new Error('error text'); + const representation = new BasicRepresentation([ error ], 'internal/error', false); + const prom = converter.handle({ identifier, representation, preferences }); + await expect(prom).resolves.toBeDefined(); + const result = await prom; + expect(result.binary).toBe(true); + expect(result.metadata.contentType).toBe('application/json'); + await expect(readJsonStream(result.data)).resolves.toEqual({ + name: 'Error', + message: 'error text', + statusCode: 500, + stack: error.stack, + }); + }); + + it('only adds stack if it is defined.', async(): Promise => { + const error = new Error('error text'); + delete error.stack; + const representation = new BasicRepresentation([ error ], 'internal/error', false); + const prom = converter.handle({ identifier, representation, preferences }); + await expect(prom).resolves.toBeDefined(); + const result = await prom; + expect(result.binary).toBe(true); + expect(result.metadata.contentType).toBe('application/json'); + await expect(readJsonStream(result.data)).resolves.toEqual({ + name: 'Error', + message: 'error text', + statusCode: 500, + }); + }); +}); diff --git a/test/unit/storage/conversion/ErrorToQuadConverter.test.ts b/test/unit/storage/conversion/ErrorToQuadConverter.test.ts index ac4d372a0..82c17423f 100644 --- a/test/unit/storage/conversion/ErrorToQuadConverter.test.ts +++ b/test/unit/storage/conversion/ErrorToQuadConverter.test.ts @@ -4,7 +4,6 @@ import { DataFactory } from 'n3'; import { BasicRepresentation } from '../../../../src/ldp/representation/BasicRepresentation'; import { ErrorToQuadConverter } from '../../../../src/storage/conversion/ErrorToQuadConverter'; import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError'; -import { InternalServerError } from '../../../../src/util/errors/InternalServerError'; import { DC, SOLID_ERROR } from '../../../../src/util/Vocabularies'; const { literal, namedNode, quad } = DataFactory; @@ -18,13 +17,6 @@ describe('An ErrorToQuadConverter', (): void => { await expect(converter.getOutputTypes()).resolves.toEqual({ 'internal/quads': 1 }); }); - it('does not support multiple errors.', async(): Promise => { - const representation = new BasicRepresentation([ new Error('a'), new Error('b') ], 'internal/error', false); - const prom = converter.handle({ identifier, representation, preferences }); - await expect(prom).rejects.toThrow('Only single errors are supported.'); - await expect(prom).rejects.toThrow(InternalServerError); - }); - it('adds triples for all error fields.', async(): Promise => { const error = new BadRequestHttpError('error text'); const representation = new BasicRepresentation([ error ], 'internal/error', false); diff --git a/test/unit/storage/conversion/ErrorToTemplateConverter.test.ts b/test/unit/storage/conversion/ErrorToTemplateConverter.test.ts index 85907046c..0cbaf589c 100644 --- a/test/unit/storage/conversion/ErrorToTemplateConverter.test.ts +++ b/test/unit/storage/conversion/ErrorToTemplateConverter.test.ts @@ -1,7 +1,6 @@ import { BasicRepresentation } from '../../../../src/ldp/representation/BasicRepresentation'; import { ErrorToTemplateConverter } from '../../../../src/storage/conversion/ErrorToTemplateConverter'; import { BadRequestHttpError } from '../../../../src/util/errors/BadRequestHttpError'; -import { InternalServerError } from '../../../../src/util/errors/InternalServerError'; import { readableToString } from '../../../../src/util/StreamUtil'; import type { TemplateEngine } from '../../../../src/util/templates/TemplateEngine'; @@ -29,13 +28,6 @@ describe('An ErrorToTemplateConverter', (): void => { await expect(converter.getOutputTypes()).resolves.toEqual({ 'text/html': 1 }); }); - it('does not support multiple errors.', async(): Promise => { - const representation = new BasicRepresentation([ new Error('a'), new Error('b') ], 'internal/error', false); - const prom = converter.handle({ identifier, representation, preferences }); - await expect(prom).rejects.toThrow('Only single errors are supported.'); - await expect(prom).rejects.toThrow(InternalServerError); - }); - it('works with non-HTTP errors.', async(): Promise => { const error = new Error('error text'); const representation = new BasicRepresentation([ error ], 'internal/error', false); diff --git a/test/unit/util/StreamUtil.test.ts b/test/unit/util/StreamUtil.test.ts index e71da4f00..dd8df1ca5 100644 --- a/test/unit/util/StreamUtil.test.ts +++ b/test/unit/util/StreamUtil.test.ts @@ -6,7 +6,7 @@ import { getLoggerFor } from '../../../src/logging/LogUtil'; import { isHttpRequest } from '../../../src/server/HttpRequest'; import { guardedStreamFrom, pipeSafely, transformSafely, - readableToString, readableToQuads, readJsonStream, + readableToString, readableToQuads, readJsonStream, getSingleItem, } from '../../../src/util/StreamUtil'; jest.mock('../../../src/logging/LogUtil', (): any => { @@ -56,6 +56,18 @@ describe('StreamUtil', (): void => { }); }); + describe('#getSingleItem', (): void => { + it('extracts a single item from the stream.', async(): Promise => { + const stream = Readable.from([ 5 ]); + await expect(getSingleItem(stream)).resolves.toBe(5); + }); + + it('errors if there are multiple items.', async(): Promise => { + const stream = Readable.from([ 5, 5 ]); + await expect(getSingleItem(stream)).rejects.toThrow('Expected a stream with a single object.'); + }); + }); + describe('#pipeSafely', (): void => { beforeEach(async(): Promise => { jest.clearAllMocks();