feat: Extend OIDC error descriptions

RELEASE_NOTES.md

@@ -36,6 +36,10 @@ The following changes are relevant for v4 custom configs that replaced certain f
 - RegexPathRouting has changed from a map datastructure to an array datastructure, allowing for fallthrough regex parsing. The change is reflected in the following default configs:
    - `/storage/backend/regex.json`
    - `/sparql-file-storage.json`
+- The `IdentityProviderFactory` inputs have been extended.
+  - `/identity/handler/provider-factory/identity.json`
+- LDP components have slightly changed so the preference parser is in a separate config file.
+  - `/config/ldp/handler/*`
 
 ### Interface changes
 These changes are relevant if you wrote custom modules for the server that depend on existing interfaces.
@@ -43,6 +47,8 @@ These changes are relevant if you wrote custom modules for the server that depen
 - `RedirectAllHttpHandler` was removed and fully replaced by `RedirectingHttpHandler`.
 - `SingleThreadedResourceLocker` has been renamed to `MemoryResourceLocker`.
 - Both `TemplateEngine` implementations now take a `baseUrl` parameter as input.
+- The `IdentityProviderFactory` and `ConvertingErrorHandler` now additionally take a `PreferenceParser` as input.
+- Error handlers now take the incoming HttpRequest as input instead of just the preferences.
 
 A new interface `SingleThreaded` has been added. This empty interface can be implemented to mark a component as not-threadsafe. When the CSS starts in multithreaded mode, it will error and halt if any SingleThreaded components are instantiated.
 

config/identity/handler/provider-factory/identity.json

@@ -18,6 +18,7 @@
         "relativePath": "/idp/keys/",
         "source": { "@id": "urn:solid-server:default:KeyValueStorage" }
       },
+      "args_showStackTrace": { "@id": "urn:solid-server:default:variable:showStackTrace" },
       "args_errorHandler": { "@id": "urn:solid-server:default:ErrorHandler" },
       "args_responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" },
       "config": {

src/identity/configuration/IdentityProviderFactory.ts

@@ -10,7 +10,8 @@ import type { Account,
   ErrorOut,
   KoaContextWithOIDC,
   ResourceServer,
-  UnknownObject } from 'oidc-provider';
+  UnknownObject,
+  errors } from 'oidc-provider';
 import { Provider } from 'oidc-provider';
 import type { Operation } from '../../http/Operation';
 import type { ErrorHandler } from '../../http/output/error/ErrorHandler';
@@ -51,6 +52,10 @@ export interface IdentityProviderFactoryArgs {
    * Storage used to store cookie and JWT keys so they can be re-used in case of multithreading.
    */
   storage: KeyValueStorage<string, unknown>;
+  /**
+   * Extra information will be added to the error output if this is true.
+   */
+  showStackTrace: boolean;
   /**
    * Used to convert errors thrown by the OIDC library.
    */
@@ -73,14 +78,15 @@ const COOKIES_KEY = 'cookie-secret';
  */
 export class IdentityProviderFactory implements ProviderFactory {
   private readonly config: Configuration;
-  private readonly adapterFactory!: AdapterFactory;
+  private readonly adapterFactory: AdapterFactory;
-  private readonly baseUrl!: string;
+  private readonly baseUrl: string;
-  private readonly oidcPath!: string;
+  private readonly oidcPath: string;
-  private readonly interactionHandler!: InteractionHandler;
+  private readonly interactionHandler: InteractionHandler;
-  private readonly credentialStorage!: KeyValueStorage<string, ClientCredentials>;
+  private readonly credentialStorage: KeyValueStorage<string, ClientCredentials>;
-  private readonly storage!: KeyValueStorage<string, unknown>;
+  private readonly storage: KeyValueStorage<string, unknown>;
-  private readonly errorHandler!: ErrorHandler;
+  private readonly showStackTrace: boolean;
-  private readonly responseWriter!: ResponseWriter;
+  private readonly errorHandler: ErrorHandler;
+  private readonly responseWriter: ResponseWriter;
 
   private readonly jwtAlg = 'ES256';
   private provider?: Provider;
@@ -91,7 +97,16 @@ export class IdentityProviderFactory implements ProviderFactory {
    */
   public constructor(config: Configuration, args: IdentityProviderFactoryArgs) {
     this.config = config;
-    Object.assign(this, args);
+
+    this.adapterFactory = args.adapterFactory;
+    this.baseUrl = args.baseUrl;
+    this.oidcPath = args.oidcPath;
+    this.interactionHandler = args.interactionHandler;
+    this.credentialStorage = args.credentialStorage;
+    this.storage = args.storage;
+    this.showStackTrace = args.showStackTrace;
+    this.errorHandler = args.errorHandler;
+    this.responseWriter = args.responseWriter;
   }
 
   public async getProvider(): Promise<Provider> {
@@ -317,13 +332,27 @@ export class IdentityProviderFactory implements ProviderFactory {
    * Pipes library errors to the provided ErrorHandler and ResponseWriter.
    */
   private configureErrors(config: Configuration): void {
-    config.renderError = async(ctx: KoaContextWithOIDC, out: ErrorOut, error: Error): Promise<void> => {
+    config.renderError = async(ctx: KoaContextWithOIDC, out: ErrorOut, error: errors.OIDCProviderError | Error):
+    Promise<void> => {
       // This allows us to stream directly to the response object, see https://github.com/koajs/koa/issues/944
       ctx.respond = false;
 
-      // OIDC library hides extra details in this field
+      // Doesn't really matter which type it is since all relevant fields are optional
-      if (out.error_description) {
+      const oidcError = error as errors.OIDCProviderError;
-        error.message += ` - ${out.error_description}`;
+
+      // OIDC library hides extra details in these fields
+      if (this.showStackTrace) {
+        if (oidcError.error_description) {
+          error.message += ` - ${oidcError.error_description}`;
+        }
+        if (oidcError.error_detail) {
+          oidcError.message += ` - ${oidcError.error_detail}`;
+        }
+
+        // Also change the error message in the stack trace
+        if (error.stack) {
+          error.stack = error.stack.replace(/.*/u, `${error.name}: ${error.message}`);
+        }
       }
 
       const result = await this.errorHandler.handleSafe({ error, request: guardStream(ctx.req) });

test/unit/identity/configuration/IdentityProviderFactory.test.ts

@@ -1,5 +1,5 @@
 import { Readable } from 'stream';
-import type { Configuration, KoaContextWithOIDC } from 'oidc-provider';
+import type { errors, Configuration, KoaContextWithOIDC } from 'oidc-provider';
 import type { ErrorHandler } from '../../../../src/http/output/error/ErrorHandler';
 import type { ResponseWriter } from '../../../../src/http/output/ResponseWriter';
 import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation';
@@ -92,6 +92,7 @@ describe('An IdentityProviderFactory', (): void => {
       interactionHandler,
       storage,
       credentialStorage,
+      showStackTrace: true,
       errorHandler,
       responseWriter,
     });
@@ -175,6 +176,7 @@ describe('An IdentityProviderFactory', (): void => {
       interactionHandler,
       storage,
       credentialStorage,
+      showStackTrace: true,
       errorHandler,
       responseWriter,
     });
@@ -198,6 +200,7 @@ describe('An IdentityProviderFactory', (): void => {
       interactionHandler,
       storage,
       credentialStorage,
+      showStackTrace: true,
       errorHandler,
       responseWriter,
     });
@@ -214,16 +217,17 @@ describe('An IdentityProviderFactory', (): void => {
     const provider = await factory.getProvider() as any;
     const { config } = provider as { config: Configuration };
 
-    const error = new Error('bad data');
+    const error = new Error('bad data') as errors.OIDCProviderError;
-    const out = { error_description: 'more info' };
+    error.error_description = 'more info';
+    error.error_detail = 'more details';
 
-    await expect((config.renderError as any)(ctx, out, error)).resolves.toBeUndefined();
+    await expect((config.renderError as any)(ctx, {}, error)).resolves.toBeUndefined();
     expect(errorHandler.handleSafe).toHaveBeenCalledTimes(1);
     expect(errorHandler.handleSafe)
       .toHaveBeenLastCalledWith({ error, request: ctx.req });
     expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1);
     expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ response: ctx.res, result: { statusCode: 500 }});
-    expect(error.message).toBe('bad data - more info');
+    expect(error.message).toBe('bad data - more info - more details');
-    expect(error.stack).toContain('Error: bad data - more info');
+    expect(error.stack).toContain('Error: bad data - more info - more details');
   });
 });