From d68854a474e28b3b4a732dcf1f7d3df36e81405b Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Wed, 25 Aug 2021 14:00:24 +0200 Subject: [PATCH] feat: Simplify IDP routing --- .../handler/interaction/routes/login.json | 2 +- src/identity/IdentityProviderHttpHandler.ts | 48 +++++-------------- .../IdentityProviderHttpHandler.test.ts | 44 ++--------------- 3 files changed, 15 insertions(+), 79 deletions(-) diff --git a/config/identity/handler/interaction/routes/login.json b/config/identity/handler/interaction/routes/login.json index d91f3adfc..ae0f92921 100644 --- a/config/identity/handler/interaction/routes/login.json +++ b/config/identity/handler/interaction/routes/login.json @@ -6,7 +6,7 @@ "@id": "urn:solid-server:auth:password:LoginRoute", "@type": "InteractionRoute", "route": "^/login/?$", - "prompt": "default", + "prompt": "login", "viewTemplates": { "InteractionRoute:_viewTemplates_key": "text/html", "InteractionRoute:_viewTemplates_value": "@css:templates/identity/email-password/login.html.ejs" diff --git a/src/identity/IdentityProviderHttpHandler.ts b/src/identity/IdentityProviderHttpHandler.ts index a6bf32392..b986b5968 100644 --- a/src/identity/IdentityProviderHttpHandler.ts +++ b/src/identity/IdentityProviderHttpHandler.ts @@ -15,7 +15,6 @@ import type { RepresentationConverter } from '../storage/conversion/Representati import { APPLICATION_JSON } from '../util/ContentTypes'; import { BadRequestHttpError } from '../util/errors/BadRequestHttpError'; import { createErrorMessage } from '../util/errors/ErrorUtil'; -import { InternalServerError } from '../util/errors/InternalServerError'; import { joinUrl, trimTrailingSlashes } from '../util/PathUtil'; import { addTemplateMetadata } from '../util/ResourceUtil'; import type { ProviderFactory } from './configuration/ProviderFactory'; @@ -46,7 +45,6 @@ export class InteractionRoute { * Keys are content-types, values paths to a template. * @param handler - Handler to call on POST requests. * @param prompt - In case of requests to the IDP entry point, the session prompt will be compared to this. - * One entry should have a value of "default" here in case there are no prompt matches. * @param responseTemplates - Templates to render as a response to POST requests when required. * Keys are content-types, values paths to a template. */ @@ -143,6 +141,7 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { // If our own interaction handler does not support the input, it is either invalid or a request for the OIDC library const route = await this.findRoute(operation, oidcInteraction); + if (!route) { const provider = await this.providerFactory.getProvider(); this.logger.debug(`Sending request to oidc-provider: ${request.url}`); @@ -176,13 +175,19 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { return; } const pathName = operation.target.path.slice(this.baseUrl.length); - let route = this.getRouteMatch(pathName); // In case the request targets the IDP entry point the prompt determines where to go - if (!route && oidcInteraction && trimTrailingSlashes(pathName).length === 0) { - route = this.getPromptMatch(oidcInteraction.prompt.name); + const checkPrompt = oidcInteraction && trimTrailingSlashes(pathName).length === 0; + + for (const route of this.interactionRoutes) { + if (checkPrompt) { + if (route.prompt === oidcInteraction!.prompt.name) { + return route; + } + } else if (route.route.test(pathName)) { + return route; + } } - return route; } /** @@ -267,35 +272,4 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { return new OkResponseDescription(converted.metadata, converted.data); } - - /** - * Find a route by matching the URL. - */ - private getRouteMatch(url: string): InteractionRoute | undefined { - for (const route of this.interactionRoutes) { - if (route.route.test(url)) { - return route; - } - } - } - - /** - * Find a route by matching the prompt. - */ - private getPromptMatch(prompt: string): InteractionRoute { - let def: InteractionRoute | undefined; - for (const route of this.interactionRoutes) { - if (route.prompt === prompt) { - return route; - } - if (route.prompt === 'default') { - def = route; - } - } - if (!def) { - throw new InternalServerError('No handler for the default session prompt has been configured.'); - } - - return def; - } } diff --git a/test/unit/identity/IdentityProviderHttpHandler.test.ts b/test/unit/identity/IdentityProviderHttpHandler.test.ts index ac7c2e101..452534ca4 100644 --- a/test/unit/identity/IdentityProviderHttpHandler.test.ts +++ b/test/unit/identity/IdentityProviderHttpHandler.test.ts @@ -25,7 +25,6 @@ import type { RepresentationConverterArgs, } from '../../../src/storage/conversion/RepresentationConverter'; import { BadRequestHttpError } from '../../../src/util/errors/BadRequestHttpError'; -import { InternalServerError } from '../../../src/util/errors/InternalServerError'; import { joinUrl } from '../../../src/util/PathUtil'; import { readableToString } from '../../../src/util/StreamUtil'; import { CONTENT_TYPE, SOLID_HTTP, SOLID_META } from '../../../src/util/Vocabularies'; @@ -78,7 +77,7 @@ describe('An IdentityProviderHttpHandler', (): void => { response: new InteractionRoute('/routeResponse', { 'text/html': '/view1' }, handlers[0], - 'default', + 'login', { 'text/html': '/response1' }), complete: new InteractionRoute('/routeComplete', { 'text/html': '/view2' }, @@ -157,7 +156,7 @@ describe('An IdentityProviderHttpHandler', (): void => { it('indicates to the templates if the request is part of an auth flow.', async(): Promise => { request.url = '/idp/routeResponse'; request.method = 'POST'; - const oidcInteraction = { session: { accountId: 'account' }} as any; + const oidcInteraction = { session: { accountId: 'account' }, prompt: {}} as any; provider.interactionDetails.mockResolvedValueOnce(oidcInteraction); (routes.response.handler as jest.Mocked).handleSafe.mockResolvedValueOnce({ type: 'response' }); await expect(handler.handle({ request, response })).resolves.toBeUndefined(); @@ -191,7 +190,7 @@ describe('An IdentityProviderHttpHandler', (): void => { it('calls the interactionCompleter for InteractionCompleteResults and redirects.', async(): Promise => { request.url = '/idp/routeComplete'; request.method = 'POST'; - const oidcInteraction = { session: { accountId: 'account' }} as any; + const oidcInteraction = { session: { accountId: 'account' }, prompt: {}} as any; provider.interactionDetails.mockResolvedValueOnce(oidcInteraction); await expect(handler.handle({ request, response })).resolves.toBeUndefined(); const operation: Operation = await requestParser.handleSafe.mock.results[0].value; @@ -222,19 +221,6 @@ describe('An IdentityProviderHttpHandler', (): void => { expect(operation.body?.metadata.contentType).toBe('application/json'); }); - it('uses the default route for requests to the root IDP without (matching) prompt.', async(): Promise => { - request.url = '/idp'; - request.method = 'POST'; - const oidcInteraction = { prompt: { name: 'notSupported' }}; - provider.interactionDetails.mockResolvedValueOnce(oidcInteraction as any); - await expect(handler.handle({ request, response })).resolves.toBeUndefined(); - const operation: Operation = await requestParser.handleSafe.mock.results[0].value; - expect(routes.response.handler.handleSafe).toHaveBeenCalledTimes(1); - expect(routes.response.handler.handleSafe).toHaveBeenLastCalledWith({ operation, oidcInteraction }); - expect(operation.body?.metadata.contentType).toBe('application/json'); - expect(routes.complete.handler.handleSafe).toHaveBeenCalledTimes(0); - }); - it('displays a viewTemplate again in case of POST errors.', async(): Promise => { request.url = '/idp/routeResponse'; request.method = 'POST'; @@ -289,28 +275,4 @@ describe('An IdentityProviderHttpHandler', (): void => { expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ response, result: { statusCode: 500 }}); }); - - it('errors if no route is configured for the default prompt.', async(): Promise => { - const args: IdentityProviderHttpHandlerArgs = { - baseUrl, - idpPath, - requestParser, - providerFactory, - interactionRoutes: [], - converter, - interactionCompleter, - errorHandler, - responseWriter, - }; - handler = new IdentityProviderHttpHandler(args); - request.url = '/idp'; - provider.interactionDetails.mockResolvedValueOnce({ prompt: { name: 'other' }} as any); - const error = new InternalServerError('No handler for the default session prompt has been configured.'); - errorHandler.handleSafe.mockResolvedValueOnce({ statusCode: 500 }); - await expect(handler.handle({ request, response })).resolves.toBeUndefined(); - expect(errorHandler.handleSafe).toHaveBeenCalledTimes(1); - expect(errorHandler.handleSafe).toHaveBeenLastCalledWith({ error, preferences: { type: { 'text/html': 1 }}}); - expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); - expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ response, result: { statusCode: 500 }}); - }); });