feat: Move redirect support from IDP handler to specific handlers

config/identity/handler/default.json

@@ -43,11 +43,6 @@
       "args_idpPath": "/idp",
       "args_providerFactory": { "@id": "urn:solid-server:default:IdentityProviderFactory" },
       "args_converter": { "@id": "urn:solid-server:default:RepresentationConverter" },
-      "args_interactionCompleter": {
-        "comment": "Responsible for finishing OIDC interactions.",
-        "@type": "InteractionCompleter",
-        "providerFactory": { "@id": "urn:solid-server:default:IdentityProviderFactory" }
-      },
       "args_errorHandler": { "@id": "urn:solid-server:default:ErrorHandler" }
     }
   ]

config/identity/handler/interaction/routes/login.json

@@ -17,7 +17,8 @@
       },
       "handler": {
         "@type": "LoginHandler",
-        "accountStore": { "@id": "urn:solid-server:auth:password:AccountStore" }
+        "accountStore": { "@id": "urn:solid-server:auth:password:AccountStore" },
+        "interactionCompleter": { "@type": "BaseInteractionCompleter" }
       }
     }
   ]

config/identity/handler/interaction/routes/session.json

@@ -11,7 +11,10 @@
         "BasicInteractionRoute:_viewTemplates_key": "text/html",
         "BasicInteractionRoute:_viewTemplates_value": "@css:templates/identity/email-password/confirm.html.ejs"
       },
-      "handler": { "@type": "SessionHttpHandler" }
+      "handler": {
+        "@type": "SessionHttpHandler",
+        "interactionCompleter": { "@type": "BaseInteractionCompleter" }
+      }
     }
   ]
 }

src/identity/IdentityProviderHttpHandler.ts

@@ -1,26 +1,20 @@
 import type { Operation } from '../http/Operation';
 import type { ErrorHandler } from '../http/output/error/ErrorHandler';
-import { RedirectResponseDescription } from '../http/output/response/RedirectResponseDescription';
 import { ResponseDescription } from '../http/output/response/ResponseDescription';
 import { BasicRepresentation } from '../http/representation/BasicRepresentation';
 import { getLoggerFor } from '../logging/LogUtil';
-import type { HttpRequest } from '../server/HttpRequest';
 import type { OperationHttpHandlerInput } from '../server/OperationHttpHandler';
 import { OperationHttpHandler } from '../server/OperationHttpHandler';
 import type { RepresentationConverter } from '../storage/conversion/RepresentationConverter';
 import { APPLICATION_JSON } from '../util/ContentTypes';
-import { BadRequestHttpError } from '../util/errors/BadRequestHttpError';
-import { FoundHttpError } from '../util/errors/FoundHttpError';
 import { NotFoundHttpError } from '../util/errors/NotFoundHttpError';
 import { joinUrl, trimTrailingSlashes } from '../util/PathUtil';
 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 { Interaction } from './interaction/InteractionHandler';
 import type { InteractionRoute, TemplatedInteractionResult } from './interaction/routing/InteractionRoute';
-import type { InteractionCompleter } from './interaction/util/InteractionCompleter';
 
-// Registration is not standardized within Solid yet, so we use a custom versioned API for now
 const API_VERSION = '0.2';
 
 export interface IdentityProviderHttpHandlerArgs {
@@ -44,10 +38,6 @@ export interface IdentityProviderHttpHandlerArgs {
    * Used for content negotiation.
    */
   converter: RepresentationConverter;
-  /**
-   * Used for POST requests that need to be handled by the OIDC library.
-   */
-  interactionCompleter: InteractionCompleter;
   /**
    * Used for converting output errors.
    */
@@ -73,7 +63,6 @@ export class IdentityProviderHttpHandler extends OperationHttpHandler {
   private readonly providerFactory: ProviderFactory;
   private readonly interactionRoutes: InteractionRoute[];
   private readonly converter: RepresentationConverter;
-  private readonly interactionCompleter: InteractionCompleter;
   private readonly errorHandler: ErrorHandler;
 
   private readonly controls: Record<string, string>;
@@ -85,7 +74,6 @@ export class IdentityProviderHttpHandler extends OperationHttpHandler {
     this.providerFactory = args.providerFactory;
     this.interactionRoutes = args.interactionRoutes;
     this.converter = args.converter;
-    this.interactionCompleter = args.interactionCompleter;
     this.errorHandler = args.errorHandler;
 
     this.controls = Object.assign(
@@ -131,7 +119,7 @@ export class IdentityProviderHttpHandler extends OperationHttpHandler {
     // Reset the body so it can be reused when needed for output
     operation.body = clone;
 
-    return this.handleInteractionResult(operation, request, result, oidcInteraction);
+    return this.handleInteractionResult(operation, result, oidcInteraction);
   }
 
   /**
@@ -155,21 +143,11 @@ export class IdentityProviderHttpHandler extends OperationHttpHandler {
    * Creates a ResponseDescription based on the InteractionHandlerResult.
    * This will either be a redirect if type is "complete" or a data stream if the type is "response".
    */
-  private async handleInteractionResult(operation: Operation, request: HttpRequest,
-    result: TemplatedInteractionResult, oidcInteraction?: Interaction): Promise<ResponseDescription> {
+  private async handleInteractionResult(operation: Operation, result: TemplatedInteractionResult,
+    oidcInteraction?: Interaction): Promise<ResponseDescription> {
     let responseDescription: ResponseDescription | undefined;
 
-    if (result.type === 'complete') {
-      if (!oidcInteraction) {
-        throw new BadRequestHttpError(
-          'This action can only be performed as part of an OIDC authentication flow.',
-          { errorCode: 'E0002' },
-        );
-      }
-      // Create a redirect URL with the OIDC library
-      const location = await this.interactionCompleter.handleSafe({ ...result.details, request });
-      responseDescription = new RedirectResponseDescription(new FoundHttpError(location));
-    } else if (result.type === 'error') {
+    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 });

src/identity/interaction/CompletingInteractionHandler.ts

@@ -0,0 +1,47 @@
+import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError';
+import { FoundHttpError } from '../../util/errors/FoundHttpError';
+import type { InteractionHandlerInput } from './InteractionHandler';
+import { InteractionHandler } from './InteractionHandler';
+import type { InteractionCompleterInput, InteractionCompleter } from './util/InteractionCompleter';
+
+/**
+ * Abstract class for {@link InteractionHandler}s that need to call an {@link InteractionCompleter}.
+ * This is required by handlers that handle IDP behaviour
+ * and need to complete an OIDC interaction by redirecting back to the client,
+ * such as when logging in.
+ *
+ * Calls the InteractionCompleter with the results returned by the helper function
+ * and throw a corresponding {@link FoundHttpError}.
+ */
+export abstract class CompletingInteractionHandler extends InteractionHandler {
+  protected readonly interactionCompleter: InteractionCompleter;
+
+  protected constructor(interactionCompleter: InteractionCompleter) {
+    super();
+    this.interactionCompleter = interactionCompleter;
+  }
+
+  public async canHandle(input: InteractionHandlerInput): Promise<void> {
+    await super.canHandle(input);
+    if (!input.oidcInteraction) {
+      throw new BadRequestHttpError(
+        'This action can only be performed as part of an OIDC authentication flow.',
+        { errorCode: 'E0002' },
+      );
+    }
+  }
+
+  public async handle(input: InteractionHandlerInput): Promise<never> {
+    // Interaction is defined due to canHandle call
+    const parameters = await this.getCompletionParameters(input as Required<InteractionHandlerInput>);
+    const location = await this.interactionCompleter.handleSafe(parameters);
+    throw new FoundHttpError(location);
+  }
+
+  /**
+   * Generates the parameters necessary to call an InteractionCompleter.
+   * @param input - The original input parameters to the `handle` function.
+   */
+  protected abstract getCompletionParameters(input: Required<InteractionHandlerInput>):
+  Promise<InteractionCompleterInput>;
+}

src/identity/interaction/InteractionHandler.ts

@@ -1,12 +1,11 @@
 import type { KoaContextWithOIDC } from 'oidc-provider';
-import type { Operation } from '../../../../http/Operation';
-import { APPLICATION_JSON } from '../../../../util/ContentTypes';
-import { NotImplementedHttpError } from '../../../../util/errors/NotImplementedHttpError';
-import { AsyncHandler } from '../../../../util/handlers/AsyncHandler';
-import type { InteractionCompleterParams } from '../../util/InteractionCompleter';
+import type { Operation } from '../../http/Operation';
+import { APPLICATION_JSON } from '../../util/ContentTypes';
+import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError';
+import { AsyncHandler } from '../../util/handlers/AsyncHandler';
 
 // OIDC library does not directly export the Interaction type
-export type Interaction = KoaContextWithOIDC['oidc']['entities']['Interaction'];
+export type Interaction = NonNullable<KoaContextWithOIDC['oidc']['entities']['Interaction']>;
 
 export interface InteractionHandlerInput {
   /**
@@ -20,18 +19,13 @@ export interface InteractionHandlerInput {
   oidcInteraction?: Interaction;
 }
 
-export type InteractionHandlerResult = InteractionResponseResult | InteractionCompleteResult | InteractionErrorResult;
+export type InteractionHandlerResult = InteractionResponseResult | InteractionErrorResult;
 
 export interface InteractionResponseResult<T = NodeJS.Dict<any>> {
   type: 'response';
   details?: T;
 }
 
-export interface InteractionCompleteResult {
-  type: 'complete';
-  details: InteractionCompleterParams;
-}
-
 export interface InteractionErrorResult {
   type: 'error';
   error: Error;

src/identity/interaction/SessionHttpHandler.ts

@@ -1,21 +1,25 @@
 import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError';
 import { readJsonStream } from '../../util/StreamUtil';
-import { InteractionHandler } from './email-password/handler/InteractionHandler';
-import type { InteractionCompleteResult, InteractionHandlerInput } from './email-password/handler/InteractionHandler';
+import { CompletingInteractionHandler } from './CompletingInteractionHandler';
+import type { InteractionHandlerInput } from './InteractionHandler';
+import type { InteractionCompleter, InteractionCompleterInput } from './util/InteractionCompleter';
 
 /**
  * Simple InteractionHttpHandler that sends the session accountId to the InteractionCompleter as webId.
+ * This is relevant when a client already logged in this session and tries logging in again.
  */
-export class SessionHttpHandler extends InteractionHandler {
-  public async handle({ operation, oidcInteraction }: InteractionHandlerInput): Promise<InteractionCompleteResult> {
-    if (!oidcInteraction?.session) {
+export class SessionHttpHandler extends CompletingInteractionHandler {
+  public constructor(interactionCompleter: InteractionCompleter) {
+    super(interactionCompleter);
+  }
+
+  protected async getCompletionParameters({ operation, oidcInteraction }: Required<InteractionHandlerInput>):
+  Promise<InteractionCompleterInput> {
+    if (!oidcInteraction.session) {
       throw new NotImplementedHttpError('Only interactions with a valid session are supported.');
     }
 
     const { remember } = await readJsonStream(operation.body.data);
-    return {
-      type: 'complete',
-      details: { webId: oidcInteraction.session.accountId, shouldRemember: Boolean(remember) },
-    };
+    return { oidcInteraction, webId: oidcInteraction.session.accountId, shouldRemember: Boolean(remember) };
   }
 }

src/identity/interaction/email-password/handler/ForgotPasswordHandler.ts

@@ -3,10 +3,10 @@ import { getLoggerFor } from '../../../../logging/LogUtil';
 import { ensureTrailingSlash, joinUrl } from '../../../../util/PathUtil';
 import { readJsonStream } from '../../../../util/StreamUtil';
 import type { TemplateEngine } from '../../../../util/templates/TemplateEngine';
+import { InteractionHandler } from '../../InteractionHandler';
+import type { InteractionResponseResult, InteractionHandlerInput } from '../../InteractionHandler';
 import type { EmailSender } from '../../util/EmailSender';
 import type { AccountStore } from '../storage/AccountStore';
-import { InteractionHandler } from './InteractionHandler';
-import type { InteractionResponseResult, InteractionHandlerInput } from './InteractionHandler';
 
 export interface ForgotPasswordHandlerArgs {
   accountStore: AccountStore;

src/identity/interaction/email-password/handler/LoginHandler.ts

@@ -3,24 +3,28 @@ import type { Operation } from '../../../../http/Operation';
 import { getLoggerFor } from '../../../../logging/LogUtil';
 import { BadRequestHttpError } from '../../../../util/errors/BadRequestHttpError';
 import { readJsonStream } from '../../../../util/StreamUtil';
+import { CompletingInteractionHandler } from '../../CompletingInteractionHandler';
+import type { InteractionHandlerInput } from '../../InteractionHandler';
+import type { InteractionCompleterInput, InteractionCompleter } from '../../util/InteractionCompleter';
+
 import type { AccountStore } from '../storage/AccountStore';
-import { InteractionHandler } from './InteractionHandler';
-import type { InteractionCompleteResult, InteractionHandlerInput } from './InteractionHandler';
 
 /**
  * Handles the submission of the Login Form and logs the user in.
+ * Will throw a RedirectHttpError on success.
  */
-export class LoginHandler extends InteractionHandler {
+export class LoginHandler extends CompletingInteractionHandler {
   protected readonly logger = getLoggerFor(this);
 
   private readonly accountStore: AccountStore;
 
-  public constructor(accountStore: AccountStore) {
-    super();
+  public constructor(accountStore: AccountStore, interactionCompleter: InteractionCompleter) {
+    super(interactionCompleter);
     this.accountStore = accountStore;
   }
 
-  public async handle({ operation }: InteractionHandlerInput): Promise<InteractionCompleteResult> {
+  protected async getCompletionParameters({ operation, oidcInteraction }: Required<InteractionHandlerInput>):
+  Promise<InteractionCompleterInput> {
     const { email, password, remember } = await this.parseInput(operation);
     // Try to log in, will error if email/password combination is invalid
     const webId = await this.accountStore.authenticate(email, password);
@@ -30,10 +34,8 @@ export class LoginHandler extends InteractionHandler {
       throw new BadRequestHttpError('This server is not an identity provider for this account.');
     }
     this.logger.debug(`Logging in user ${email}`);
-    return {
-      type: 'complete',
-      details: { webId, shouldRemember: remember },
-    };
+
+    return { oidcInteraction, webId, shouldRemember: remember };
   }
 
   /**

src/identity/interaction/email-password/handler/RegistrationHandler.ts

@@ -1,8 +1,8 @@
 import { getLoggerFor } from '../../../../logging/LogUtil';
 import { readJsonStream } from '../../../../util/StreamUtil';
+import type { InteractionResponseResult, InteractionHandlerInput } from '../../InteractionHandler';
+import { InteractionHandler } from '../../InteractionHandler';
 import type { RegistrationManager, RegistrationResponse } from '../util/RegistrationManager';
-import type { InteractionResponseResult, InteractionHandlerInput } from './InteractionHandler';
-import { InteractionHandler } from './InteractionHandler';
 
 /**
  * Supports registration based on the `RegistrationManager` behaviour.

src/identity/interaction/email-password/handler/ResetPasswordHandler.ts

@@ -1,10 +1,10 @@
 import assert from 'assert';
 import { getLoggerFor } from '../../../../logging/LogUtil';
 import { readJsonStream } from '../../../../util/StreamUtil';
+import type { InteractionResponseResult, InteractionHandlerInput } from '../../InteractionHandler';
+import { InteractionHandler } from '../../InteractionHandler';
 import { assertPassword } from '../EmailPasswordUtil';
 import type { AccountStore } from '../storage/AccountStore';
-import type { InteractionResponseResult, InteractionHandlerInput } from './InteractionHandler';
-import { InteractionHandler } from './InteractionHandler';
 
 /**
  * Handles the submission of the ResetPassword form:

src/identity/interaction/routing/BasicInteractionRoute.ts

@@ -2,11 +2,12 @@ import type { Operation } from '../../../http/Operation';
 import { BadRequestHttpError } from '../../../util/errors/BadRequestHttpError';
 import { createErrorMessage, isError } from '../../../util/errors/ErrorUtil';
 import { InternalServerError } from '../../../util/errors/InternalServerError';
+import { RedirectHttpError } from '../../../util/errors/RedirectHttpError';
 import { trimTrailingSlashes } from '../../../util/PathUtil';
 import type {
   InteractionHandler,
   Interaction,
-} from '../email-password/handler/InteractionHandler';
+} from '../InteractionHandler';
 import type { InteractionRoute, TemplatedInteractionResult } from './InteractionRoute';
 
 /**
@@ -84,6 +85,11 @@ export class BasicInteractionRoute implements InteractionRoute {
           const result = await this.handler.handleSafe({ operation, oidcInteraction });
           return { ...result, templateFiles: this.responseTemplates };
         } catch (err: unknown) {
+          // Redirect errors need to be propagated and not rendered on the response pages.
+          // Otherwise, the user would be redirected to a new page only containing that error.
+          if (RedirectHttpError.isInstance(err)) {
+            throw err;
+          }
           const error = isError(err) ? err : new InternalServerError(createErrorMessage(err));
           // Potentially render the error in the view
           return { type: 'error', error, templateFiles: this.viewTemplates };

src/identity/interaction/routing/InteractionRoute.ts

@@ -1,5 +1,5 @@
 import type { Operation } from '../../../http/Operation';
-import type { Interaction, InteractionHandlerResult } from '../email-password/handler/InteractionHandler';
+import type { Interaction, InteractionHandlerResult } from '../InteractionHandler';
 
 export type TemplatedInteractionResult<T extends InteractionHandlerResult = InteractionHandlerResult> = T & {
   templateFiles: Record<string, string>;

src/identity/interaction/util/BaseInteractionCompleter.ts

@@ -0,0 +1,37 @@
+import type { InteractionResults } from 'oidc-provider';
+import type { InteractionCompleterInput } from './InteractionCompleter';
+import { InteractionCompleter } from './InteractionCompleter';
+
+/**
+ * Creates a simple InteractionResults object based on the input parameters and injects it in the Interaction.
+ */
+export class BaseInteractionCompleter extends InteractionCompleter {
+  public async handle(input: InteractionCompleterInput): Promise<string> {
+    const now = Math.floor(Date.now() / 1000);
+    const result: InteractionResults = {
+      login: {
+        account: input.webId,
+        // Indicates if a persistent cookie should be used instead of a session cookie.
+        remember: input.shouldRemember,
+        ts: now,
+      },
+      consent: {
+        // When OIDC clients want a refresh token, they need to request the 'offline_access' scope.
+        // This indicates that this scope is not granted to the client in case they do not want to be remembered.
+        rejectedScopes: input.shouldRemember ? [] : [ 'offline_access' ],
+      },
+    };
+
+    // Generates the URL a client needs to be redirected to
+    // after a successful interaction completion (such as logging in).
+    // Identical behaviour to calling `provider.interactionResult`.
+    // We use the code below instead of calling that function
+    // since that function also uses Request/Response objects to generate the Interaction object,
+    // which we already have here.
+    const { oidcInteraction } = input;
+    oidcInteraction.result = { ...oidcInteraction.lastSubmission, ...result };
+    await oidcInteraction.save(oidcInteraction.exp - now);
+
+    return oidcInteraction.returnTo;
+  }
+}

src/identity/interaction/util/InteractionCompleter.ts

@@ -1,49 +1,16 @@
-import { ServerResponse } from 'http';
-import type { InteractionResults } from 'oidc-provider';
-import type { HttpRequest } from '../../../server/HttpRequest';
 import { AsyncHandler } from '../../../util/handlers/AsyncHandler';
-import type { ProviderFactory } from '../../configuration/ProviderFactory';
+import type { Interaction } from '../InteractionHandler';
 
 /**
  * Parameters required to specify how the interaction should be completed.
  */
-export interface InteractionCompleterParams {
+export interface InteractionCompleterInput {
+  oidcInteraction: Interaction;
   webId: string;
   shouldRemember?: boolean;
 }
 
-export interface InteractionCompleterInput extends InteractionCompleterParams {
-  request: HttpRequest;
-}
-
 /**
- * Completes an IDP interaction, logging the user in.
- * Returns the URL the request should be redirected to.
+ * Class responsible for completing the interaction based on the parameters provided.
  */
-export class InteractionCompleter extends AsyncHandler<InteractionCompleterInput, string> {
-  private readonly providerFactory: ProviderFactory;
-
-  public constructor(providerFactory: ProviderFactory) {
-    super();
-    this.providerFactory = providerFactory;
-  }
-
-  public async handle(input: InteractionCompleterInput): Promise<string> {
-    const provider = await this.providerFactory.getProvider();
-    const result: InteractionResults = {
-      login: {
-        account: input.webId,
-        remember: input.shouldRemember,
-        ts: Math.floor(Date.now() / 1000),
-      },
-      consent: {
-        rejectedScopes: input.shouldRemember ? [] : [ 'offline_access' ],
-      },
-    };
-
-    // Response object is not actually needed here so we can just mock it like this
-    // to bypass the OIDC library checks.
-    // See https://github.com/panva/node-oidc-provider/discussions/1078
-    return provider.interactionResult(input.request, Object.create(ServerResponse.prototype), result);
-  }
-}
+export abstract class InteractionCompleter extends AsyncHandler<InteractionCompleterInput, string> {}

src/index.ts

@@ -126,7 +126,6 @@ export * from './identity/configuration/IdentityProviderFactory';
 export * from './identity/configuration/ProviderFactory';
 
 // Identity/Interaction/Email-Password/Handler
-export * from './identity/interaction/email-password/handler/InteractionHandler';
 export * from './identity/interaction/email-password/handler/ForgotPasswordHandler';
 export * from './identity/interaction/email-password/handler/LoginHandler';
 export * from './identity/interaction/email-password/handler/RegistrationHandler';
@@ -148,10 +147,13 @@ export * from './identity/interaction/routing/InteractionRoute';
 
 // Identity/Interaction/Util
 export * from './identity/interaction/util/BaseEmailSender';
+export * from './identity/interaction/util/BaseInteractionCompleter';
 export * from './identity/interaction/util/EmailSender';
 export * from './identity/interaction/util/InteractionCompleter';
 
 // Identity/Interaction
+export * from './identity/interaction/CompletingInteractionHandler';
+export * from './identity/interaction/InteractionHandler';
 export * from './identity/interaction/SessionHttpHandler';
 
 // Identity/Ownership

test/unit/identity/IdentityProviderHttpHandler.test.ts

@@ -9,7 +9,6 @@ import type { ProviderFactory } from '../../../src/identity/configuration/Provid
 import type { IdentityProviderHttpHandlerArgs } from '../../../src/identity/IdentityProviderHttpHandler';
 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 { HttpRequest } from '../../../src/server/HttpRequest';
 import type { HttpResponse } from '../../../src/server/HttpResponse';
 import { getBestPreference } from '../../../src/storage/conversion/ConversionUtil';
@@ -20,7 +19,7 @@ import type {
 import { NotFoundHttpError } from '../../../src/util/errors/NotFoundHttpError';
 import { joinUrl } from '../../../src/util/PathUtil';
 import { guardedStreamFrom, readableToString } from '../../../src/util/StreamUtil';
-import { CONTENT_TYPE, SOLID_HTTP, SOLID_META } from '../../../src/util/Vocabularies';
+import { CONTENT_TYPE, SOLID_META } from '../../../src/util/Vocabularies';
 
 describe('An IdentityProviderHttpHandler', (): void => {
   const apiVersion = '0.2';
@@ -32,7 +31,6 @@ describe('An IdentityProviderHttpHandler', (): void => {
   let providerFactory: jest.Mocked<ProviderFactory>;
   let routes: Record<'response' | 'complete' | 'error', jest.Mocked<InteractionRoute>>;
   let controls: Record<string, string>;
-  let interactionCompleter: jest.Mocked<InteractionCompleter>;
   let converter: jest.Mocked<RepresentationConverter>;
   let errorHandler: jest.Mocked<ErrorHandler>;
   let provider: jest.Mocked<Provider>;
@@ -94,8 +92,6 @@ describe('An IdentityProviderHttpHandler', (): void => {
       }),
     } as any;
 
-    interactionCompleter = { handleSafe: jest.fn().mockResolvedValue('http://test.com/idp/auth') } as any;
-
     errorHandler = { handleSafe: jest.fn(({ error }: ErrorHandlerArgs): ResponseDescription => ({
       statusCode: 400,
       data: guardedStreamFrom(`{ "name": "${error.name}", "message": "${error.message}" }`),
@@ -107,7 +103,6 @@ describe('An IdentityProviderHttpHandler', (): void => {
       providerFactory,
       interactionRoutes: Object.values(routes),
       converter,
-      interactionCompleter,
       errorHandler,
     };
     handler = new IdentityProviderHttpHandler(args);
@@ -184,38 +179,4 @@ describe('An IdentityProviderHttpHandler', (): void => {
     expect(result).toBeDefined();
     expect(JSON.parse(await readableToString(result.data!))).toEqual({ apiVersion, authenticating: true, controls });
   });
-
-  it('errors for InteractionCompleteResults if no oidcInteraction is defined.', async(): Promise<void> => {
-    operation.target.path = joinUrl(baseUrl, '/idp/routeComplete');
-    operation.method = 'POST';
-
-    const error = expect.objectContaining({
-      statusCode: 400,
-      message: 'This action can only be performed as part of an OIDC authentication flow.',
-      errorCode: 'E0002',
-    });
-    await expect(handler.handle({ request, response, operation })).rejects.toThrow(error);
-    expect(routes.complete.handleOperation).toHaveBeenCalledTimes(1);
-    expect(routes.complete.handleOperation).toHaveBeenLastCalledWith(operation, undefined);
-    expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(0);
-  });
-
-  it('calls the interactionCompleter for InteractionCompleteResults and redirects.', async(): Promise<void> => {
-    operation.target.path = joinUrl(baseUrl, '/idp/routeComplete');
-    operation.method = 'POST';
-    operation.body = new BasicRepresentation('value', 'text/plain');
-    const oidcInteraction = { session: { accountId: 'account' }, prompt: {}} as any;
-    provider.interactionDetails.mockResolvedValueOnce(oidcInteraction);
-    const result = (await handler.handle({ request, response, operation }))!;
-    expect(result).toBeDefined();
-    expect(routes.complete.handleOperation).toHaveBeenCalledTimes(1);
-    expect(routes.complete.handleOperation).toHaveBeenLastCalledWith(operation, oidcInteraction);
-    expect(operation.body?.metadata.contentType).toBe('application/json');
-
-    expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(1);
-    expect(interactionCompleter.handleSafe).toHaveBeenLastCalledWith({ request, webId: 'webId' });
-    const location = await interactionCompleter.handleSafe.mock.results[0].value;
-    expect(result.statusCode).toBe(302);
-    expect(result.metadata?.get(SOLID_HTTP.terms.location)?.value).toBe(location);
-  });
 });

test/unit/identity/interaction/CompletingInteractionHandler.test.ts

@@ -0,0 +1,65 @@
+import type { Operation } from '../../../../src/http/Operation';
+import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation';
+import { CompletingInteractionHandler } from '../../../../src/identity/interaction/CompletingInteractionHandler';
+import type { Interaction, InteractionHandlerInput } from '../../../../src/identity/interaction/InteractionHandler';
+import type {
+  InteractionCompleter,
+  InteractionCompleterInput,
+} from '../../../../src/identity/interaction/util/InteractionCompleter';
+import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
+
+const webId = 'http://alice.test.com/card#me';
+class DummyCompletingInteractionHandler extends CompletingInteractionHandler {
+  public constructor(interactionCompleter: InteractionCompleter) {
+    super(interactionCompleter);
+  }
+
+  public async getCompletionParameters(input: Required<InteractionHandlerInput>): Promise<InteractionCompleterInput> {
+    return { webId, oidcInteraction: input.oidcInteraction };
+  }
+}
+
+describe('A CompletingInteractionHandler', (): void => {
+  const oidcInteraction: Interaction = {} as any;
+  const location = 'http://test.com/redirect';
+  let operation: Operation;
+  let interactionCompleter: jest.Mocked<InteractionCompleter>;
+  let handler: DummyCompletingInteractionHandler;
+
+  beforeEach(async(): Promise<void> => {
+    const representation = new BasicRepresentation('', 'application/json');
+    operation = { body: representation } as any;
+
+    interactionCompleter = {
+      handleSafe: jest.fn().mockResolvedValue(location),
+    } as any;
+
+    handler = new DummyCompletingInteractionHandler(interactionCompleter);
+  });
+
+  it('calls the parent JSON canHandle check.', async(): Promise<void> => {
+    operation.body.metadata.contentType = 'application/x-www-form-urlencoded';
+    await expect(handler.canHandle({ operation } as any)).rejects.toThrow(NotImplementedHttpError);
+  });
+
+  it('errors if no OidcInteraction is defined.', async(): Promise<void> => {
+    const error = expect.objectContaining({
+      statusCode: 400,
+      message: 'This action can only be performed as part of an OIDC authentication flow.',
+      errorCode: 'E0002',
+    });
+    await expect(handler.canHandle({ operation })).rejects.toThrow(error);
+
+    await expect(handler.canHandle({ operation, oidcInteraction })).resolves.toBeUndefined();
+  });
+
+  it('throws a redirect error with the completer location.', async(): Promise<void> => {
+    const error = expect.objectContaining({
+      statusCode: 302,
+      location,
+    });
+    await expect(handler.handle({ operation, oidcInteraction })).rejects.toThrow(error);
+    expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(1);
+    expect(interactionCompleter.handleSafe).toHaveBeenLastCalledWith({ oidcInteraction, webId });
+  });
+});

test/unit/identity/interaction/InteractionHandler.test.ts

@@ -1,11 +1,11 @@
-import { BasicRepresentation } from '../../../../../../src/http/representation/BasicRepresentation';
+import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation';
 import type {
   InteractionResponseResult,
-} from '../../../../../../src/identity/interaction/email-password/handler/InteractionHandler';
+} from '../../../../src/identity/interaction/InteractionHandler';
 import {
   InteractionHandler,
-} from '../../../../../../src/identity/interaction/email-password/handler/InteractionHandler';
-import { NotImplementedHttpError } from '../../../../../../src/util/errors/NotImplementedHttpError';
+} from '../../../../src/identity/interaction/InteractionHandler';
+import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
 
 class SimpleInteractionHandler extends InteractionHandler {
   public async handle(): Promise<InteractionResponseResult> {

test/unit/identity/interaction/SessionHttpHandler.test.ts

@@ -1,31 +1,47 @@
-import type { Interaction } from '../../../../src/identity/interaction/email-password/handler/InteractionHandler';
+import type { InteractionHandlerInput, Interaction } from '../../../../src/identity/interaction/InteractionHandler';
 import { SessionHttpHandler } from '../../../../src/identity/interaction/SessionHttpHandler';
+import type {
+  InteractionCompleter,
+  InteractionCompleterInput,
+} from '../../../../src/identity/interaction/util/InteractionCompleter';
 import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
 import { createPostJsonOperation } from './email-password/handler/Util';
 
+class PublicSessionHttpHandler extends SessionHttpHandler {
+  public constructor(interactionCompleter: InteractionCompleter) {
+    super(interactionCompleter);
+  }
+
+  public async getCompletionParameters(input: Required<InteractionHandlerInput>): Promise<InteractionCompleterInput> {
+    return super.getCompletionParameters(input);
+  }
+}
+
 describe('A SessionHttpHandler', (): void => {
   const webId = 'http://test.com/id#me';
   let oidcInteraction: Interaction;
-  let handler: SessionHttpHandler;
+  let interactionCompleter: jest.Mocked<InteractionCompleter>;
+  let handler: PublicSessionHttpHandler;
 
   beforeEach(async(): Promise<void> => {
     oidcInteraction = { session: { accountId: webId }} as any;
 
-    handler = new SessionHttpHandler();
+    interactionCompleter = {
+      handleSafe: jest.fn().mockResolvedValue('http://test.com/redirect'),
+    } as any;
+
+    handler = new PublicSessionHttpHandler(interactionCompleter);
   });
 
-  it('requires a defined oidcInteraction with a session.', async(): Promise<void> => {
-    oidcInteraction!.session = undefined;
-    await expect(handler.handle({ operation: {} as any, oidcInteraction })).rejects.toThrow(NotImplementedHttpError);
-
-    await expect(handler.handle({ operation: {} as any })).rejects.toThrow(NotImplementedHttpError);
+  it('requires an oidcInteraction with a defined session.', async(): Promise<void> => {
+    oidcInteraction.session = undefined;
+    await expect(handler.getCompletionParameters({ operation: {} as any, oidcInteraction }))
+      .rejects.toThrow(NotImplementedHttpError);
   });
 
-  it('returns an InteractionCompleteResult when done.', async(): Promise<void> => {
+  it('returns the correct completion parameters.', async(): Promise<void> => {
     const operation = createPostJsonOperation({ remember: true });
-    await expect(handler.handle({ operation, oidcInteraction })).resolves.toEqual({
-      details: { webId, shouldRemember: true },
-      type: 'complete',
-    });
+    await expect(handler.getCompletionParameters({ operation, oidcInteraction }))
+      .resolves.toEqual({ oidcInteraction, webId, shouldRemember: true });
   });
 });

test/unit/identity/interaction/email-password/handler/LoginHandler.test.ts

@@ -1,60 +1,81 @@
-import type {
-  InteractionHandlerInput,
-} from '../../../../../../src/identity/interaction/email-password/handler/InteractionHandler';
 import { LoginHandler } from '../../../../../../src/identity/interaction/email-password/handler/LoginHandler';
 import type { AccountStore } from '../../../../../../src/identity/interaction/email-password/storage/AccountStore';
+import type {
+  Interaction,
+  InteractionHandlerInput,
+} from '../../../../../../src/identity/interaction/InteractionHandler';
+import type {
+  InteractionCompleterInput,
+  InteractionCompleter,
+} from '../../../../../../src/identity/interaction/util/InteractionCompleter';
+
 import { createPostJsonOperation } from './Util';
 
+class PublicLoginHandler extends LoginHandler {
+  public constructor(accountStore: AccountStore, interactionCompleter: InteractionCompleter) {
+    super(accountStore, interactionCompleter);
+  }
+
+  public async getCompletionParameters(input: Required<InteractionHandlerInput>): Promise<InteractionCompleterInput> {
+    return super.getCompletionParameters(input);
+  }
+}
+
 describe('A LoginHandler', (): void => {
   const webId = 'http://alice.test.com/card#me';
   const email = 'alice@test.email';
-  let input: InteractionHandlerInput;
+  const oidcInteraction: Interaction = {} as any;
+  let input: Required<InteractionHandlerInput>;
   let accountStore: jest.Mocked<AccountStore>;
-  let handler: LoginHandler;
+  let interactionCompleter: jest.Mocked<InteractionCompleter>;
+  let handler: PublicLoginHandler;
 
   beforeEach(async(): Promise<void> => {
-    input = {} as any;
+    input = { oidcInteraction } as any;
 
     accountStore = {
       authenticate: jest.fn().mockResolvedValue(webId),
       getSettings: jest.fn().mockResolvedValue({ useIdp: true }),
     } as any;
 
-    handler = new LoginHandler(accountStore);
+    interactionCompleter = {
+      handleSafe: jest.fn().mockResolvedValue('http://test.com/redirect'),
+    } as any;
+
+    handler = new PublicLoginHandler(accountStore, interactionCompleter);
   });
 
   it('errors on invalid emails.', async(): Promise<void> => {
     input.operation = createPostJsonOperation({});
-    await expect(handler.handle(input)).rejects.toThrow('Email required');
+    await expect(handler.getCompletionParameters(input)).rejects.toThrow('Email required');
     input.operation = createPostJsonOperation({ email: [ 'a', 'b' ]});
-    await expect(handler.handle(input)).rejects.toThrow('Email required');
+    await expect(handler.getCompletionParameters(input)).rejects.toThrow('Email required');
   });
 
   it('errors on invalid passwords.', async(): Promise<void> => {
     input.operation = createPostJsonOperation({ email });
-    await expect(handler.handle(input)).rejects.toThrow('Password required');
+    await expect(handler.getCompletionParameters(input)).rejects.toThrow('Password required');
     input.operation = createPostJsonOperation({ email, password: [ 'a', 'b' ]});
-    await expect(handler.handle(input)).rejects.toThrow('Password required');
+    await expect(handler.getCompletionParameters(input)).rejects.toThrow('Password required');
   });
 
   it('throws an error if there is a problem.', async(): Promise<void> => {
     input.operation = createPostJsonOperation({ email, password: 'password!' });
     accountStore.authenticate.mockRejectedValueOnce(new Error('auth failed!'));
-    await expect(handler.handle(input)).rejects.toThrow('auth failed!');
+    await expect(handler.getCompletionParameters(input)).rejects.toThrow('auth failed!');
   });
 
   it('throws an error if the account does not have the correct settings.', async(): Promise<void> => {
     input.operation = createPostJsonOperation({ email, password: 'password!' });
     accountStore.getSettings.mockResolvedValueOnce({ useIdp: false });
-    await expect(handler.handle(input)).rejects.toThrow('This server is not an identity provider for this account.');
+    await expect(handler.getCompletionParameters(input))
+      .rejects.toThrow('This server is not an identity provider for this account.');
   });
 
-  it('returns an InteractionCompleteResult when done.', async(): Promise<void> => {
+  it('returns the correct completion parameters.', async(): Promise<void> => {
     input.operation = createPostJsonOperation({ email, password: 'password!' });
-    await expect(handler.handle(input)).resolves.toEqual({
-      type: 'complete',
-      details: { webId, shouldRemember: false },
-    });
+    await expect(handler.getCompletionParameters(input))
+      .resolves.toEqual({ oidcInteraction, webId, shouldRemember: false });
     expect(accountStore.authenticate).toHaveBeenCalledTimes(1);
     expect(accountStore.authenticate).toHaveBeenLastCalledWith(email, 'password!');
   });

test/unit/identity/interaction/routing/BasicInteractionRoute.test.ts

@@ -1,8 +1,9 @@
 import type {
   InteractionHandler,
-} from '../../../../../src/identity/interaction/email-password/handler/InteractionHandler';
+} from '../../../../../src/identity/interaction/InteractionHandler';
 import { BasicInteractionRoute } from '../../../../../src/identity/interaction/routing/BasicInteractionRoute';
 import { BadRequestHttpError } from '../../../../../src/util/errors/BadRequestHttpError';
+import { FoundHttpError } from '../../../../../src/util/errors/FoundHttpError';
 import { InternalServerError } from '../../../../../src/util/errors/InternalServerError';
 
 describe('A BasicInteractionRoute', (): void => {
@@ -57,6 +58,12 @@ describe('A BasicInteractionRoute', (): void => {
       .resolves.toEqual({ type: 'error', error, templateFiles: viewTemplates });
   });
 
+  it('re-throws redirect errors.', async(): Promise<void> => {
+    const error = new FoundHttpError('http://test.com/redirect');
+    handler.handleSafe.mockRejectedValueOnce(error);
+    await expect(route.handleOperation({ method: 'POST' } as any)).rejects.toThrow(error);
+  });
+
   it('creates an internal error in case of non-native errors.', async(): Promise<void> => {
     handler.handleSafe.mockRejectedValueOnce('notAnError');
     await expect(route.handleOperation({ method: 'POST' } as any)).resolves.toEqual({

test/unit/identity/interaction/util/BaseInteractionCompleter.test.ts

@@ -0,0 +1,56 @@
+import type { Interaction } from '../../../../../src/identity/interaction/InteractionHandler';
+import { BaseInteractionCompleter } from '../../../../../src/identity/interaction/util/BaseInteractionCompleter';
+
+jest.useFakeTimers();
+
+describe('A BaseInteractionCompleter', (): void => {
+  const now = Math.floor(Date.now() / 1000);
+  const webId = 'http://alice.test.com/#me';
+  let oidcInteraction: jest.Mocked<Interaction>;
+  let completer: BaseInteractionCompleter;
+
+  beforeEach(async(): Promise<void> => {
+    oidcInteraction = {
+      lastSubmission: {},
+      exp: now + 500,
+      returnTo: 'http://test.com/redirect',
+      save: jest.fn(),
+    } as any;
+
+    completer = new BaseInteractionCompleter();
+  });
+
+  it('stores the correct data in the interaction.', async(): Promise<void> => {
+    await expect(completer.handle({ oidcInteraction, webId, shouldRemember: true }))
+      .resolves.toBe(oidcInteraction.returnTo);
+    expect(oidcInteraction.result).toEqual({
+      login: {
+        account: webId,
+        remember: true,
+        ts: now,
+      },
+      consent: {
+        rejectedScopes: [],
+      },
+    });
+    expect(oidcInteraction.save).toHaveBeenCalledTimes(1);
+    expect(oidcInteraction.save).toHaveBeenLastCalledWith(500);
+  });
+
+  it('rejects offline access if shouldRemember is false.', async(): Promise<void> => {
+    await expect(completer.handle({ oidcInteraction, webId, shouldRemember: false }))
+      .resolves.toBe(oidcInteraction.returnTo);
+    expect(oidcInteraction.result).toEqual({
+      login: {
+        account: webId,
+        remember: false,
+        ts: now,
+      },
+      consent: {
+        rejectedScopes: [ 'offline_access' ],
+      },
+    });
+    expect(oidcInteraction.save).toHaveBeenCalledTimes(1);
+    expect(oidcInteraction.save).toHaveBeenLastCalledWith(500);
+  });
+});

test/unit/identity/interaction/util/InteractionCompleter.test.ts

@@ -1,58 +0,0 @@
-import { ServerResponse } from 'http';
-import type { Provider } from 'oidc-provider';
-import type { ProviderFactory } from '../../../../../src/identity/configuration/ProviderFactory';
-import { InteractionCompleter } from '../../../../../src/identity/interaction/util/InteractionCompleter';
-import type { HttpRequest } from '../../../../../src/server/HttpRequest';
-
-jest.useFakeTimers();
-
-describe('An InteractionCompleter', (): void => {
-  const request: HttpRequest = {} as any;
-  const webId = 'http://alice.test.com/#me';
-  let provider: jest.Mocked<Provider>;
-  let completer: InteractionCompleter;
-
-  beforeEach(async(): Promise<void> => {
-    provider = {
-      interactionResult: jest.fn(),
-    } as any;
-
-    const factory: ProviderFactory = {
-      getProvider: jest.fn().mockResolvedValue(provider),
-    };
-
-    completer = new InteractionCompleter(factory);
-  });
-
-  it('sends the correct data to the provider.', async(): Promise<void> => {
-    await expect(completer.handle({ request, webId, shouldRemember: true }))
-      .resolves.toBeUndefined();
-    expect(provider.interactionResult).toHaveBeenCalledTimes(1);
-    expect(provider.interactionResult).toHaveBeenLastCalledWith(request, expect.any(ServerResponse), {
-      login: {
-        account: webId,
-        remember: true,
-        ts: Math.floor(Date.now() / 1000),
-      },
-      consent: {
-        rejectedScopes: [],
-      },
-    });
-  });
-
-  it('rejects offline access if shouldRemember is false.', async(): Promise<void> => {
-    await expect(completer.handle({ request, webId, shouldRemember: false }))
-      .resolves.toBeUndefined();
-    expect(provider.interactionResult).toHaveBeenCalledTimes(1);
-    expect(provider.interactionResult).toHaveBeenLastCalledWith(request, expect.any(ServerResponse), {
-      login: {
-        account: webId,
-        remember: false,
-        ts: Math.floor(Date.now() / 1000),
-      },
-      consent: {
-        rejectedScopes: [ 'offline_access' ],
-      },
-    });
-  });
-});