From 7b42c721428d8639511fbff302fc2f695821cb2f Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Fri, 6 Aug 2021 15:26:25 +0200 Subject: [PATCH] feat: Let InteractionCompleter return redirect URL --- src/identity/IdentityProviderHttpHandler.ts | 10 +++++----- .../interaction/util/InteractionCompleter.ts | 20 ++++++++++++++----- .../response/CreatedResponseDescription.ts | 2 +- .../response/RedirectResponseDescription.ts | 14 +++++++++++++ .../IdentityProviderHttpHandler.test.ts | 13 +++++++++--- .../util/InteractionCompleter.test.ts | 20 +++++++++---------- .../RedirectResponseDescription.test.ts | 18 +++++++++++++++++ 7 files changed, 72 insertions(+), 25 deletions(-) create mode 100644 src/ldp/http/response/RedirectResponseDescription.ts create mode 100644 test/unit/ldp/http/response/RedirectResponseDescription.test.ts diff --git a/src/identity/IdentityProviderHttpHandler.ts b/src/identity/IdentityProviderHttpHandler.ts index 76caaff14..580ecce1d 100644 --- a/src/identity/IdentityProviderHttpHandler.ts +++ b/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); diff --git a/src/identity/interaction/util/InteractionCompleter.ts b/src/identity/interaction/util/InteractionCompleter.ts index 121d68590..37938d8d6 100644 --- a/src/identity/interaction/util/InteractionCompleter.ts +++ b/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 { +export class InteractionCompleter extends AsyncHandler { private readonly providerFactory: ProviderFactory; public constructor(providerFactory: ProviderFactory) { @@ -21,7 +28,7 @@ export class InteractionCompleter extends AsyncHandler { + public async handle(input: InteractionCompleterInput): Promise { const provider = await this.providerFactory.getProvider(); const result: InteractionResults = { login: { @@ -34,6 +41,9 @@ export class InteractionCompleter extends AsyncHandler { 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 => { + it('calls the interactionCompleter for InteractionCompleteResults and redirects.', async(): Promise => { 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 => { diff --git a/test/unit/identity/interaction/util/InteractionCompleter.test.ts b/test/unit/identity/interaction/util/InteractionCompleter.test.ts index 910ec0b11..520f5746b 100644 --- a/test/unit/identity/interaction/util/InteractionCompleter.test.ts +++ b/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; let completer: InteractionCompleter; beforeEach(async(): Promise => { 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 => { - 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 => { - 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, diff --git a/test/unit/ldp/http/response/RedirectResponseDescription.test.ts b/test/unit/ldp/http/response/RedirectResponseDescription.test.ts new file mode 100644 index 000000000..caddb874a --- /dev/null +++ b/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 => { + 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 => { + const description = new RedirectResponseDescription(location, true); + expect(description.metadata?.get(SOLID_HTTP.terms.location)?.value).toBe(location); + expect(description.statusCode).toBe(301); + }); +});