fix: Output required OAuth error fields

This commit is contained in:
Joachim Van Herwegen
2023-03-06 16:01:07 +01:00
parent 7eb938044d
commit 63fd062f16
8 changed files with 124 additions and 11 deletions

View File

@@ -20,7 +20,9 @@ import { BasicRepresentation } from '../../http/representation/BasicRepresentati
import { getLoggerFor } from '../../logging/LogUtil';
import type { KeyValueStorage } from '../../storage/keyvalue/KeyValueStorage';
import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError';
import type { HttpError } from '../../util/errors/HttpError';
import { InternalServerError } from '../../util/errors/InternalServerError';
import { OAuthHttpError } from '../../util/errors/OAuthHttpError';
import { RedirectHttpError } from '../../util/errors/RedirectHttpError';
import { guardStream } from '../../util/GuardedStream';
import { joinUrl } from '../../util/PathUtil';
@@ -379,7 +381,8 @@ export class IdentityProviderFactory implements ProviderFactory {
// Doesn't really matter which type it is since all relevant fields are optional
const oidcError = error as errors.OIDCProviderError;
let detailedError = error.message;
// Create a more detailed error message for logging and to show is `showStackTrace` is enabled.
let detailedError = oidcError.message;
if (oidcError.error_description) {
detailedError += ` - ${oidcError.error_description}`;
}
@@ -389,13 +392,21 @@ export class IdentityProviderFactory implements ProviderFactory {
this.logger.warn(`OIDC request failed: ${detailedError}`);
// OIDC library hides extra details in these fields
// Convert to our own error object.
// This ensures serializing the error object will generate the correct output later on.
// We specifically copy the fields instead of passing the object to contain the `oidc-provider` dependency
// to the current file.
let resultingError: HttpError = new OAuthHttpError(out, oidcError.name, oidcError.statusCode, oidcError.message);
// Keep the original stack to make debugging easier
resultingError.stack = oidcError.stack;
if (this.showStackTrace) {
error.message = detailedError;
// Expose more information if `showStackTrace` is enabled
resultingError.message = detailedError;
// Also change the error message in the stack trace
if (error.stack) {
error.stack = error.stack.replace(/.*/u, `${error.name}: ${error.message}`);
if (resultingError.stack) {
resultingError.stack = resultingError.stack.replace(/.*/u, `${oidcError.name}: ${oidcError.message}`);
}
}
@@ -411,11 +422,11 @@ export class IdentityProviderFactory implements ProviderFactory {
},
},
);
unknownClientError.stack = error.stack;
error = unknownClientError;
unknownClientError.stack = oidcError.stack;
resultingError = unknownClientError;
}
const result = await this.errorHandler.handleSafe({ error, request: guardStream(ctx.req) });
const result = await this.errorHandler.handleSafe({ error: resultingError, request: guardStream(ctx.req) });
await this.responseWriter.handleSafe({ response: ctx.res, result });
};
}

View File

@@ -406,6 +406,7 @@ export * from './util/errors/MethodNotAllowedHttpError';
export * from './util/errors/MovedPermanentlyHttpError';
export * from './util/errors/NotFoundHttpError';
export * from './util/errors/NotImplementedHttpError';
export * from './util/errors/OAuthHttpError';
export * from './util/errors/PreconditionFailedHttpError';
export * from './util/errors/RedirectHttpError';
export * from './util/errors/SystemError';

View File

@@ -2,6 +2,7 @@ import { BasicRepresentation } from '../../http/representation/BasicRepresentati
import type { Representation } from '../../http/representation/Representation';
import { APPLICATION_JSON, INTERNAL_ERROR } from '../../util/ContentTypes';
import { HttpError } from '../../util/errors/HttpError';
import { OAuthHttpError } from '../../util/errors/OAuthHttpError';
import { getSingleItem } from '../../util/StreamUtil';
import { BaseTypedRepresentationConverter } from './BaseTypedRepresentationConverter';
import type { RepresentationConverterArgs } from './RepresentationConverter';
@@ -22,6 +23,11 @@ export class ErrorToJsonConverter extends BaseTypedRepresentationConverter {
message: error.message,
};
// OAuth errors responses require additional fields
if (OAuthHttpError.isInstance(error)) {
Object.assign(result, error.mandatoryFields);
}
if (HttpError.isInstance(error)) {
result.statusCode = error.statusCode;
result.errorCode = error.errorCode;

View File

@@ -0,0 +1,36 @@
import type { HttpErrorOptions } from './HttpError';
import { HttpError } from './HttpError';
/**
* These are the fields that can occur in an OAuth error response as described in RFC 6749, §4.1.2.1.
* https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1
*
* This interface is identical to the ErrorOut interface of the `oidc-provider` library,
* but having our own version reduces the part of the codebase that is dependent on that library.
*/
export interface OAuthErrorFields {
error: string;
// eslint-disable-next-line @typescript-eslint/naming-convention
error_description?: string | undefined;
scope?: string | undefined;
state?: string | undefined;
}
/**
* Represents on OAuth error that is being thrown.
* OAuth error responses have additional fields that need to be present in the JSON response,
* as described in RFC 6749, §4.1.2.1.
*/
export class OAuthHttpError extends HttpError {
public readonly mandatoryFields: OAuthErrorFields;
public constructor(mandatoryFields: OAuthErrorFields, name?: string, statusCode?: number, message?: string,
options?: HttpErrorOptions) {
super(statusCode ?? 500, name ?? 'OAuthHttpError', message, options);
this.mandatoryFields = mandatoryFields;
}
public static isInstance(error: unknown): error is OAuthHttpError {
return HttpError.isInstance(error) && Boolean((error as OAuthHttpError).mandatoryFields);
}
}