From 32a182dde8b3c83ab4d949e11134ca1f345f9689 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Wed, 25 Aug 2021 14:49:57 +0200 Subject: [PATCH] feat: Add controls to IDP response JSON Controls are now used in templates to prevent IDP URL hardcoding --- .../interaction/routes/forgot-password.json | 4 +++ .../handler/interaction/routes/login.json | 4 +++ .../registration/route/registration.json | 4 +++ src/identity/IdentityProviderHttpHandler.ts | 33 +++++++++++++++++-- .../identity/email-password/confirm.html.ejs | 2 +- .../forgot-password-response.html.ejs | 4 +-- .../email-password/forgot-password.html.ejs | 4 +-- .../identity/email-password/login.html.ejs | 6 ++-- .../email-password/register-response.html.ejs | 2 +- .../identity/email-password/register.html.ejs | 2 +- test/integration/Identity.test.ts | 28 +++++++--------- test/integration/IdentityTestState.ts | 20 ++--------- .../IdentityProviderHttpHandler.test.ts | 25 ++++++++------ 13 files changed, 80 insertions(+), 58 deletions(-) diff --git a/config/identity/handler/interaction/routes/forgot-password.json b/config/identity/handler/interaction/routes/forgot-password.json index 3d72f53de..281397b4d 100644 --- a/config/identity/handler/interaction/routes/forgot-password.json +++ b/config/identity/handler/interaction/routes/forgot-password.json @@ -14,6 +14,10 @@ "InteractionRoute:_responseTemplates_key": "text/html", "InteractionRoute:_responseTemplates_value": "@css:templates/identity/email-password/forgot-password-response.html.ejs" }, + "controls": { + "InteractionRoute:_controls_key": "forgotPassword", + "InteractionRoute:_controls_value": "/forgotpassword" + }, "handler": { "@type": "ForgotPasswordHandler", "args_accountStore": { "@id": "urn:solid-server:auth:password:AccountStore" }, diff --git a/config/identity/handler/interaction/routes/login.json b/config/identity/handler/interaction/routes/login.json index ae0f92921..74a80f1fb 100644 --- a/config/identity/handler/interaction/routes/login.json +++ b/config/identity/handler/interaction/routes/login.json @@ -11,6 +11,10 @@ "InteractionRoute:_viewTemplates_key": "text/html", "InteractionRoute:_viewTemplates_value": "@css:templates/identity/email-password/login.html.ejs" }, + "controls": { + "InteractionRoute:_controls_key": "login", + "InteractionRoute:_controls_value": "/login" + }, "handler": { "@type": "LoginHandler", "accountStore": { "@id": "urn:solid-server:auth:password:AccountStore" } diff --git a/config/identity/registration/route/registration.json b/config/identity/registration/route/registration.json index 0d38f8807..e9446cff0 100644 --- a/config/identity/registration/route/registration.json +++ b/config/identity/registration/route/registration.json @@ -14,6 +14,10 @@ "InteractionRoute:_responseTemplates_key": "text/html", "InteractionRoute:_responseTemplates_value": "@css:templates/identity/email-password/register-response.html.ejs" }, + "controls": { + "InteractionRoute:_controls_key": "register", + "InteractionRoute:_controls_value": "/register" + }, "handler": { "@type": "RegistrationHandler", "args_baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, diff --git a/src/identity/IdentityProviderHttpHandler.ts b/src/identity/IdentityProviderHttpHandler.ts index b986b5968..d131eabc7 100644 --- a/src/identity/IdentityProviderHttpHandler.ts +++ b/src/identity/IdentityProviderHttpHandler.ts @@ -27,7 +27,8 @@ import type { import { IdpInteractionError } from './interaction/util/IdpInteractionError'; import type { InteractionCompleter } from './interaction/util/InteractionCompleter'; -const API_VERSION = '0.1'; +// Registration is not standardized within Solid yet, so we use a custom versioned API for now +const API_VERSION = '0.2'; /** * All the information that is required to handle a request to a custom IDP path. @@ -38,6 +39,7 @@ export class InteractionRoute { public readonly viewTemplates: Record; public readonly prompt?: string; public readonly responseTemplates: Record; + public readonly controls: Record; /** * @param route - Regex to match this route. @@ -47,17 +49,21 @@ export class InteractionRoute { * @param prompt - In case of requests to the IDP entry point, the session prompt will be compared to this. * @param responseTemplates - Templates to render as a response to POST requests when required. * Keys are content-types, values paths to a template. + * @param controls - Controls to add to the response JSON. + * The keys will be copied and the values will be converted to full URLs. */ public constructor(route: string, viewTemplates: Record, handler: InteractionHandler, prompt?: string, - responseTemplates: Record = {}) { + responseTemplates: Record = {}, + controls: Record = {}) { this.route = new RegExp(route, 'u'); this.viewTemplates = viewTemplates; this.handler = handler; this.prompt = prompt; this.responseTemplates = responseTemplates; + this.controls = controls; } } @@ -113,6 +119,8 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { private readonly converter: RepresentationConverter; private readonly interactionCompleter: InteractionCompleter; + private readonly controls: Record; + public constructor(args: IdentityProviderHttpHandlerArgs) { // It is important that the RequestParser does not read out the Request body stream. // Otherwise we can't pass it anymore to the OIDC library when needed. @@ -123,6 +131,11 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { this.interactionRoutes = args.interactionRoutes; this.converter = args.converter; this.interactionCompleter = args.interactionCompleter; + + this.controls = Object.assign( + {}, + ...this.interactionRoutes.map((route): Record => this.getRouteControls(route)), + ); } /** @@ -258,7 +271,12 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { private async handleResponseResult(result: InteractionResponseResult, templateFiles: Record, operation: Operation, oidcInteraction?: Interaction): Promise { // Convert the object to a valid JSON representation - const json = { ...result.details, authenticating: Boolean(oidcInteraction), apiVersion: API_VERSION }; + const json = { + apiVersion: API_VERSION, + ...result.details, + authenticating: Boolean(oidcInteraction), + controls: this.controls, + }; const representation = new BasicRepresentation(JSON.stringify(json), operation.target, APPLICATION_JSON); // Template metadata is required for conversion @@ -272,4 +290,13 @@ export class IdentityProviderHttpHandler extends BaseHttpHandler { return new OkResponseDescription(converted.metadata, converted.data); } + + /** + * Converts the controls object of a route to one with full URLs. + */ + private getRouteControls(route: InteractionRoute): Record { + return Object.fromEntries( + Object.entries(route.controls).map(([ name, path ]): [ string, string ] => [ name, joinUrl(this.baseUrl, path) ]), + ); + } } diff --git a/templates/identity/email-password/confirm.html.ejs b/templates/identity/email-password/confirm.html.ejs index adb56e929..2daa01aa8 100644 --- a/templates/identity/email-password/confirm.html.ejs +++ b/templates/identity/email-password/confirm.html.ejs @@ -1,6 +1,6 @@

Authorize

You are authorizing an application to access your Pod.

-
+ <% if (errorMessage) { %>

<%= errorMessage %>

<% } %> diff --git a/templates/identity/email-password/forgot-password-response.html.ejs b/templates/identity/email-password/forgot-password-response.html.ejs index 73d99f644..dddc23bf6 100644 --- a/templates/identity/email-password/forgot-password-response.html.ejs +++ b/templates/identity/email-password/forgot-password-response.html.ejs @@ -1,11 +1,11 @@

Email sent

- +

If your account exists, an email has been sent with a link to reset your password.

If you do not receive your email in a couple of minutes, check your spam folder or click the link below to send another email.

-

Back to Log In

+

Back to Log In

diff --git a/templates/identity/email-password/forgot-password.html.ejs b/templates/identity/email-password/forgot-password.html.ejs index 91a5c357f..871e0c385 100644 --- a/templates/identity/email-password/forgot-password.html.ejs +++ b/templates/identity/email-password/forgot-password.html.ejs @@ -1,5 +1,5 @@

Forgot password

- + <% if (errorMessage) { %>

<%= errorMessage %>

<% } %> @@ -15,5 +15,5 @@

-

Log in

+

Log in

diff --git a/templates/identity/email-password/login.html.ejs b/templates/identity/email-password/login.html.ejs index a1b8174a5..8ee0271f1 100644 --- a/templates/identity/email-password/login.html.ejs +++ b/templates/identity/email-password/login.html.ejs @@ -1,5 +1,5 @@

Log in

-
+ <% if (errorMessage) { %>

<%= errorMessage %>

<% } %> @@ -24,7 +24,7 @@

diff --git a/templates/identity/email-password/register-response.html.ejs b/templates/identity/email-password/register-response.html.ejs index b9ce94f1b..b6f667dd5 100644 --- a/templates/identity/email-password/register-response.html.ejs +++ b/templates/identity/email-password/register-response.html.ejs @@ -27,7 +27,7 @@

Via your email address <%= email %>, <% if (authenticating) { %> - you can now log in + you can now log in <% } else { %> this server lets you log in to Solid apps <% } %> diff --git a/templates/identity/email-password/register.html.ejs b/templates/identity/email-password/register.html.ejs index 16149d1cd..76c63c0ef 100644 --- a/templates/identity/email-password/register.html.ejs +++ b/templates/identity/email-password/register.html.ejs @@ -1,5 +1,5 @@

Sign up

-
+ <% const isBlankForm = !('email' in prefilled); %> <% if (errorMessage) { %> diff --git a/test/integration/Identity.test.ts b/test/integration/Identity.test.ts index 140822e17..799730627 100644 --- a/test/integration/Identity.test.ts +++ b/test/integration/Identity.test.ts @@ -145,9 +145,8 @@ describe('A Solid server with IDP', (): void => { it('initializes the session and logs in.', async(): Promise => { const url = await state.startSession(); - const { login } = await state.parseLoginPage(url); - expect(typeof login).toBe('string'); - await state.login(login, email, password); + await state.parseLoginPage(url); + await state.login(url, email, password); expect(state.session.info?.webId).toBe(webId); }); @@ -168,10 +167,10 @@ describe('A Solid server with IDP', (): void => { it('can log in again.', async(): Promise => { const url = await state.startSession(); - const form = await state.extractFormUrl(url); - expect(form.url.endsWith('/confirm')).toBe(true); + let res = await state.fetchIdp(url); + expect(res.status).toBe(200); - const res = await state.fetchIdp(form.url, 'POST', '', APPLICATION_X_WWW_FORM_URLENCODED); + res = await state.fetchIdp(url, 'POST', '', APPLICATION_X_WWW_FORM_URLENCODED); const nextUrl = res.headers.get('location'); expect(typeof nextUrl).toBe('string'); @@ -226,16 +225,12 @@ describe('A Solid server with IDP', (): void => { state = new IdentityTestState(baseUrl, redirectUrl, oidcIssuer); }); - it('initializes the session.', async(): Promise => { - const url = await state.startSession(); - const { login } = await state.parseLoginPage(url); - expect(typeof login).toBe('string'); - nextUrl = login; - }); - it('can not log in with the old password anymore.', async(): Promise => { + const url = await state.startSession(); + nextUrl = url; + await state.parseLoginPage(url); const formData = stringify({ email, password }); - const res = await state.fetchIdp(nextUrl, 'POST', formData, APPLICATION_X_WWW_FORM_URLENCODED); + const res = await state.fetchIdp(url, 'POST', formData, APPLICATION_X_WWW_FORM_URLENCODED); expect(res.status).toBe(200); expect(await res.text()).toContain('Incorrect password'); }); @@ -307,9 +302,8 @@ describe('A Solid server with IDP', (): void => { it('initializes the session and logs in.', async(): Promise => { state = new IdentityTestState(baseUrl, redirectUrl, oidcIssuer); const url = await state.startSession(); - const { login } = await state.parseLoginPage(url); - expect(typeof login).toBe('string'); - await state.login(login, newMail, password); + await state.parseLoginPage(url); + await state.login(url, newMail, password); expect(state.session.info?.webId).toBe(newWebId); }); diff --git a/test/integration/IdentityTestState.ts b/test/integration/IdentityTestState.ts index 167c849a1..28597d006 100644 --- a/test/integration/IdentityTestState.ts +++ b/test/integration/IdentityTestState.ts @@ -94,15 +94,14 @@ export class IdentityTestState { return nextUrl; } - public async parseLoginPage(url: string): Promise<{ register: string; login: string; forgotPassword: string }> { + public async parseLoginPage(url: string): Promise<{ register: string; forgotPassword: string }> { const res = await this.fetchIdp(url); expect(res.status).toBe(200); const text = await res.text(); const register = this.extractUrl(text, 'a:contains("Sign up")', 'href'); - const login = this.extractUrl(text, 'form', 'action'); const forgotPassword = this.extractUrl(text, 'a:contains("Forgot password")', 'href'); - return { register, login, forgotPassword }; + return { register, forgotPassword }; } /** @@ -118,21 +117,6 @@ export class IdentityTestState { return this.handleLoginRedirect(nextUrl); } - /** - * Calls the given URL and extracts the action URL from a form contained within the resulting body. - * Also returns the resulting body in case further parsing is needed. - */ - public async extractFormUrl(url: string): Promise<{ url: string; body: string }> { - const res = await this.fetchIdp(url); - expect(res.status).toBe(200); - const text = await res.text(); - const formUrl = this.extractUrl(text, 'form', 'action'); - return { - url: new URL(formUrl, this.baseUrl).href, - body: text, - }; - } - /** * Handles the redirect that happens after logging in. */ diff --git a/test/unit/identity/IdentityProviderHttpHandler.test.ts b/test/unit/identity/IdentityProviderHttpHandler.test.ts index 452534ca4..2c9e37e9d 100644 --- a/test/unit/identity/IdentityProviderHttpHandler.test.ts +++ b/test/unit/identity/IdentityProviderHttpHandler.test.ts @@ -30,7 +30,7 @@ import { readableToString } from '../../../src/util/StreamUtil'; import { CONTENT_TYPE, SOLID_HTTP, SOLID_META } from '../../../src/util/Vocabularies'; describe('An IdentityProviderHttpHandler', (): void => { - const apiVersion = '0.1'; + const apiVersion = '0.2'; const baseUrl = 'http://test.com/'; const idpPath = '/idp'; let request: HttpRequest; @@ -38,6 +38,7 @@ describe('An IdentityProviderHttpHandler', (): void => { let requestParser: jest.Mocked; let providerFactory: jest.Mocked; let routes: { response: InteractionRoute; complete: InteractionRoute }; + let controls: Record; let interactionCompleter: jest.Mocked; let converter: jest.Mocked; let errorHandler: jest.Mocked; @@ -74,16 +75,18 @@ describe('An IdentityProviderHttpHandler', (): void => { ]; routes = { - response: new InteractionRoute('/routeResponse', + response: new InteractionRoute('^/routeResponse$', { 'text/html': '/view1' }, handlers[0], 'login', - { 'text/html': '/response1' }), - complete: new InteractionRoute('/routeComplete', + { 'text/html': '/response1' }, + { response: '/routeResponse' }), + complete: new InteractionRoute('^/routeComplete$', { 'text/html': '/view2' }, handlers[1], 'other'), }; + controls = { response: 'http://test.com/idp/routeResponse' }; converter = { handleSafe: jest.fn((input: RepresentationConverterArgs): Representation => { @@ -129,7 +132,7 @@ describe('An IdentityProviderHttpHandler', (): void => { const { response: mockResponse, result } = responseWriter.handleSafe.mock.calls[0][0]; expect(mockResponse).toBe(response); expect(JSON.parse(await readableToString(result.data!))) - .toEqual({ apiVersion, errorMessage: '', prefilled: {}, authenticating: false }); + .toEqual({ apiVersion, errorMessage: '', prefilled: {}, authenticating: false, controls }); expect(result.statusCode).toBe(200); expect(result.metadata?.contentType).toBe('text/html'); expect(result.metadata?.get(SOLID_META.template)?.value).toBe(routes.response.viewTemplates['text/html']); @@ -147,7 +150,8 @@ describe('An IdentityProviderHttpHandler', (): void => { expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); const { response: mockResponse, result } = responseWriter.handleSafe.mock.calls[0][0]; expect(mockResponse).toBe(response); - expect(JSON.parse(await readableToString(result.data!))).toEqual({ apiVersion, key: 'val', authenticating: false }); + expect(JSON.parse(await readableToString(result.data!))) + .toEqual({ apiVersion, key: 'val', authenticating: false, controls }); expect(result.statusCode).toBe(200); expect(result.metadata?.contentType).toBe('text/html'); expect(result.metadata?.get(SOLID_META.template)?.value).toBe(routes.response.responseTemplates['text/html']); @@ -163,7 +167,7 @@ describe('An IdentityProviderHttpHandler', (): void => { expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); const { result } = responseWriter.handleSafe.mock.calls[0][0]; - expect(JSON.parse(await readableToString(result.data!))).toEqual({ apiVersion, authenticating: true }); + expect(JSON.parse(await readableToString(result.data!))).toEqual({ apiVersion, authenticating: true, controls }); }); it('errors for InteractionCompleteResults if no oidcInteraction is defined.', async(): Promise => { @@ -231,8 +235,9 @@ describe('An IdentityProviderHttpHandler', (): void => { expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); const { response: mockResponse, result } = responseWriter.handleSafe.mock.calls[0][0]; expect(mockResponse).toBe(response); - expect(JSON.parse(await readableToString(result.data!))) - .toEqual({ apiVersion, errorMessage: 'handle error', prefilled: { name: 'name' }, authenticating: false }); + expect(JSON.parse(await readableToString(result.data!))).toEqual( + { apiVersion, errorMessage: 'handle error', prefilled: { name: 'name' }, authenticating: false, controls }, + ); expect(result.statusCode).toBe(200); expect(result.metadata?.contentType).toBe('text/html'); expect(result.metadata?.get(SOLID_META.template)?.value).toBe(routes.response.viewTemplates['text/html']); @@ -248,7 +253,7 @@ describe('An IdentityProviderHttpHandler', (): void => { const { response: mockResponse, result } = responseWriter.handleSafe.mock.calls[0][0]; expect(mockResponse).toBe(response); expect(JSON.parse(await readableToString(result.data!))) - .toEqual({ apiVersion, errorMessage: 'handle error', prefilled: {}, authenticating: false }); + .toEqual({ apiVersion, errorMessage: 'handle error', prefilled: {}, authenticating: false, controls }); }); it('calls the errorHandler if there is a problem resolving the request.', async(): Promise => {