feat: Let InteractionCompleter return redirect URL

src/identity/IdentityProviderHttpHandler.ts

@@ -1,6 +1,7 @@
 import urljoin from 'url-join';
 import type { ErrorHandler } from '../ldp/http/ErrorHandler';
 import type { RequestParser } from '../ldp/http/RequestParser';
+import { RedirectResponseDescription } from '../ldp/http/response/RedirectResponseDescription';
 import type { ResponseWriter } from '../ldp/http/ResponseWriter';
 import type { Operation } from '../ldp/operations/Operation';
 import type { RepresentationPreferences } from '../ldp/representation/RepresentationPreferences';
@@ -15,11 +16,9 @@ import { assertError, createErrorMessage } from '../util/errors/ErrorUtil';
 import { InternalServerError } from '../util/errors/InternalServerError';
 import { trimTrailingSlashes } from '../util/PathUtil';
 import type { ProviderFactory } from './configuration/ProviderFactory';
-import type {
-  Interaction,
+import type { Interaction,
   InteractionHandler,
-  InteractionHandlerResult,
-} from './interaction/email-password/handler/InteractionHandler';
+  InteractionHandlerResult } from './interaction/email-password/handler/InteractionHandler';
 import { IdpInteractionError } from './interaction/util/IdpInteractionError';
 import type { InteractionCompleter } from './interaction/util/InteractionCompleter';
 
@@ -159,7 +158,8 @@ export class IdentityProviderHttpHandler extends HttpHandler {
         );
       }
       // We need the original request object for the OIDC library
-      return await this.interactionCompleter.handleSafe({ ...result.details, request, response });
+      const location = await this.interactionCompleter.handleSafe({ ...result.details, request });
+      return await this.responseWriter.handleSafe({ response, result: new RedirectResponseDescription(location) });
     }
     if (result.type === 'response' && templateFile) {
       return await this.handleTemplateResponse(response, templateFile, result.details, oidcInteraction);

src/identity/interaction/util/InteractionCompleter.ts

@@ -1,19 +1,26 @@
+import { ServerResponse } from 'http';
 import type { InteractionResults } from 'oidc-provider';
-import type { HttpHandlerInput } from '../../../server/HttpHandler';
+import type { HttpRequest } from '../../../server/HttpRequest';
 import { AsyncHandler } from '../../../util/handlers/AsyncHandler';
 import type { ProviderFactory } from '../../configuration/ProviderFactory';
 
+/**
+ * Parameters required to specify how the interaction should be completed.
+ */
 export interface InteractionCompleterParams {
   webId: string;
   shouldRemember?: boolean;
 }
 
-export type InteractionCompleterInput = HttpHandlerInput & InteractionCompleterParams;
+export interface InteractionCompleterInput extends InteractionCompleterParams {
+  request: HttpRequest;
+}
 
 /**
  * Completes an IDP interaction, logging the user in.
+ * Returns the URL the request should be redirected to.
  */
-export class InteractionCompleter extends AsyncHandler<InteractionCompleterInput> {
+export class InteractionCompleter extends AsyncHandler<InteractionCompleterInput, string> {
   private readonly providerFactory: ProviderFactory;
 
   public constructor(providerFactory: ProviderFactory) {
@@ -21,7 +28,7 @@ export class InteractionCompleter extends AsyncHandler<InteractionCompleterInput
     this.providerFactory = providerFactory;
   }
 
-  public async handle(input: InteractionCompleterInput): Promise<void> {
+  public async handle(input: InteractionCompleterInput): Promise<string> {
     const provider = await this.providerFactory.getProvider();
     const result: InteractionResults = {
       login: {
@@ -34,6 +41,9 @@ export class InteractionCompleter extends AsyncHandler<InteractionCompleterInput
       },
     };
 
-    return provider.interactionFinished(input.request, input.response, result);
+    // 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);
   }
 }

src/ldp/http/response/CreatedResponseDescription.ts

@@ -5,7 +5,7 @@ import type { ResourceIdentifier } from '../../representation/ResourceIdentifier
 import { ResponseDescription } from './ResponseDescription';
 
 /**
- * Corresponds to a 201 response, containing the relevant link metadata.
+ * Corresponds to a 201 response, containing the relevant location metadata.
  */
 export class CreatedResponseDescription extends ResponseDescription {
   public constructor(location: ResourceIdentifier) {

src/ldp/http/response/RedirectResponseDescription.ts

@@ -0,0 +1,14 @@
+import { DataFactory } from 'n3';
+import { SOLID_HTTP } from '../../../util/Vocabularies';
+import { RepresentationMetadata } from '../../representation/RepresentationMetadata';
+import { ResponseDescription } from './ResponseDescription';
+
+/**
+ * Corresponds to a 301/302 response, containing the relevant location metadata.
+ */
+export class RedirectResponseDescription extends ResponseDescription {
+  public constructor(location: string, permanently = false) {
+    const metadata = new RepresentationMetadata({ [SOLID_HTTP.location]: DataFactory.namedNode(location) });
+    super(permanently ? 301 : 302, metadata);
+  }
+}

test/unit/identity/IdentityProviderHttpHandler.test.ts

@@ -15,6 +15,7 @@ import type { HttpResponse } from '../../../src/server/HttpResponse';
 import type { TemplateHandler } from '../../../src/server/util/TemplateHandler';
 import { BadRequestHttpError } from '../../../src/util/errors/BadRequestHttpError';
 import { InternalServerError } from '../../../src/util/errors/InternalServerError';
+import { SOLID_HTTP } from '../../../src/util/Vocabularies';
 
 describe('An IdentityProviderHttpHandler', (): void => {
   const baseUrl = 'http://test.com/';
@@ -64,7 +65,7 @@ describe('An IdentityProviderHttpHandler', (): void => {
 
     templateHandler = { handleSafe: jest.fn() } as any;
 
-    interactionCompleter = { handleSafe: jest.fn() } as any;
+    interactionCompleter = { handleSafe: jest.fn().mockResolvedValue('http://test.com/idp/auth') } as any;
 
     errorHandler = { handleSafe: jest.fn() } as any;
 
@@ -147,7 +148,7 @@ describe('An IdentityProviderHttpHandler', (): void => {
     expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ response, result: { statusCode: 400 }});
   });
 
-  it('calls the interactionCompleter for InteractionCompleteResults.', async(): Promise<void> => {
+  it('calls the interactionCompleter for InteractionCompleteResults and redirects.', async(): Promise<void> => {
     request.url = '/idp/routeComplete';
     request.method = 'POST';
     const oidcInteraction = { session: { accountId: 'account' }} as any;
@@ -157,7 +158,13 @@ describe('An IdentityProviderHttpHandler', (): void => {
     expect(routes.complete.handler.handleSafe).toHaveBeenCalledTimes(1);
     expect(routes.complete.handler.handleSafe).toHaveBeenLastCalledWith({ operation, oidcInteraction });
     expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(1);
-    expect(interactionCompleter.handleSafe).toHaveBeenLastCalledWith({ request, response, webId: 'webId' });
+    expect(interactionCompleter.handleSafe).toHaveBeenLastCalledWith({ request, webId: 'webId' });
+    const location = await interactionCompleter.handleSafe.mock.results[0].value;
+    expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1);
+    const args = responseWriter.handleSafe.mock.calls[0][0];
+    expect(args.response).toBe(response);
+    expect(args.result.statusCode).toBe(302);
+    expect(args.result.metadata?.get(SOLID_HTTP.terms.location)?.value).toBe(location);
   });
 
   it('matches paths based on prompt for requests to the root IDP.', async(): Promise<void> => {

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

@@ -1,22 +1,20 @@
+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';
-import type { HttpResponse } from '../../../../../src/server/HttpResponse';
 
-// Use fixed dates
 jest.useFakeTimers();
 
 describe('An InteractionCompleter', (): void => {
   const request: HttpRequest = {} as any;
-  const response: HttpResponse = {} as any;
   const webId = 'http://alice.test.com/#me';
-  let provider: Provider;
+  let provider: jest.Mocked<Provider>;
   let completer: InteractionCompleter;
 
   beforeEach(async(): Promise<void> => {
     provider = {
-      interactionFinished: jest.fn(),
+      interactionResult: jest.fn(),
     } as any;
 
     const factory: ProviderFactory = {
@@ -27,10 +25,10 @@ describe('An InteractionCompleter', (): void => {
   });
 
   it('sends the correct data to the provider.', async(): Promise<void> => {
-    await expect(completer.handle({ request, response, webId, shouldRemember: true }))
+    await expect(completer.handle({ request, webId, shouldRemember: true }))
       .resolves.toBeUndefined();
-    expect(provider.interactionFinished).toHaveBeenCalledTimes(1);
-    expect(provider.interactionFinished).toHaveBeenLastCalledWith(request, response, {
+    expect(provider.interactionResult).toHaveBeenCalledTimes(1);
+    expect(provider.interactionResult).toHaveBeenLastCalledWith(request, expect.any(ServerResponse), {
       login: {
         account: webId,
         remember: true,
@@ -43,10 +41,10 @@ describe('An InteractionCompleter', (): void => {
   });
 
   it('rejects offline access if shouldRemember is false.', async(): Promise<void> => {
-    await expect(completer.handle({ request, response, webId, shouldRemember: false }))
+    await expect(completer.handle({ request, webId, shouldRemember: false }))
       .resolves.toBeUndefined();
-    expect(provider.interactionFinished).toHaveBeenCalledTimes(1);
-    expect(provider.interactionFinished).toHaveBeenLastCalledWith(request, response, {
+    expect(provider.interactionResult).toHaveBeenCalledTimes(1);
+    expect(provider.interactionResult).toHaveBeenLastCalledWith(request, expect.any(ServerResponse), {
       login: {
         account: webId,
         remember: false,

test/unit/ldp/http/response/RedirectResponseDescription.test.ts

@@ -0,0 +1,18 @@
+import { RedirectResponseDescription } from '../../../../../src/ldp/http/response/RedirectResponseDescription';
+import { SOLID_HTTP } from '../../../../../src/util/Vocabularies';
+
+describe('A RedirectResponseDescription', (): void => {
+  const location = 'http://test.com/foo';
+
+  it('has status code 302 and a location.', async(): Promise<void> => {
+    const description = new RedirectResponseDescription(location);
+    expect(description.metadata?.get(SOLID_HTTP.terms.location)?.value).toBe(location);
+    expect(description.statusCode).toBe(302);
+  });
+
+  it('has status code 301 if the change is permanent.', async(): Promise<void> => {
+    const description = new RedirectResponseDescription(location, true);
+    expect(description.metadata?.get(SOLID_HTTP.terms.location)?.value).toBe(location);
+    expect(description.statusCode).toBe(301);
+  });
+});