From e604c0c2e427f7cf426cda6e3a52c2d72b997057 Mon Sep 17 00:00:00 2001 From: Joachim Van Herwegen Date: Wed, 16 Feb 2022 10:57:47 +0100 Subject: [PATCH] feat: Return client information from consent handler --- RELEASE_NOTES.md | 1 + src/identity/interaction/ConsentHandler.ts | 33 ++++++++++++++++++- .../identity/email-password/consent.html.ejs | 28 ++++++++++++++-- test/integration/Identity.test.ts | 19 ++++++++--- .../interaction/ConsentHandler.test.ts | 33 ++++++++++++++++++- 5 files changed, 105 insertions(+), 9 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index e6a208d7f..18c71194d 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -12,6 +12,7 @@ which enables passing custom variables to configurations and setting new default values. - The AppRunner functions have changed to require Components.js variables. This is important for anyone who starts the server from code. +- When logging in, a consent screen will now provide information about the client. ### Configuration changes You might need to make changes to your v2 configuration if you use a custom config. diff --git a/src/identity/interaction/ConsentHandler.ts b/src/identity/interaction/ConsentHandler.ts index a7cf11144..79e1df6c6 100644 --- a/src/identity/interaction/ConsentHandler.ts +++ b/src/identity/interaction/ConsentHandler.ts @@ -1,4 +1,12 @@ -import type { InteractionResults, KoaContextWithOIDC, UnknownObject } from 'oidc-provider'; +import type { + AllClientMetadata, + InteractionResults, + KoaContextWithOIDC, + UnknownObject, +} from 'oidc-provider'; +import { BasicRepresentation } from '../../http/representation/BasicRepresentation'; +import type { Representation } from '../../http/representation/Representation'; +import { APPLICATION_JSON } from '../../util/ContentTypes'; import { BadRequestHttpError } from '../../util/errors/BadRequestHttpError'; import { FoundHttpError } from '../../util/errors/FoundHttpError'; import { NotImplementedHttpError } from '../../util/errors/NotImplementedHttpError'; @@ -11,6 +19,8 @@ type Grant = NonNullable; /** * Handles the OIDC consent prompts where the user confirms they want to log in for the given client. + * + * Returns all the relevant Client metadata on GET requests. */ export class ConsentHandler extends BaseInteractionHandler { private readonly providerFactory: ProviderFactory; @@ -30,6 +40,27 @@ export class ConsentHandler extends BaseInteractionHandler { } } + protected async handleGet(input: Required): Promise { + const { operation, oidcInteraction } = input; + const provider = await this.providerFactory.getProvider(); + const client = await provider.Client.find(oidcInteraction.params.client_id as string); + const metadata: AllClientMetadata = client?.metadata() ?? {}; + + // Only extract specific fields to prevent leaking information + // Based on https://www.w3.org/ns/solid/oidc-context.jsonld + const keys = [ 'client_id', 'client_uri', 'logo_uri', 'policy_uri', + 'client_name', 'contacts', 'grant_types', 'scope' ]; + + const jsonLd = Object.fromEntries( + keys.filter((key): boolean => key in metadata) + .map((key): [ string, unknown ] => [ key, metadata[key] ]), + ); + jsonLd['@context'] = 'https://www.w3.org/ns/solid/oidc-context.jsonld'; + const json = { client: jsonLd }; + + return new BasicRepresentation(JSON.stringify(json), operation.target, APPLICATION_JSON); + } + protected async handlePost({ operation, oidcInteraction }: InteractionHandlerInput): Promise { const { remember } = await readJsonStream(operation.body.data); diff --git a/templates/identity/email-password/consent.html.ejs b/templates/identity/email-password/consent.html.ejs index cd3edcbce..750f3999c 100644 --- a/templates/identity/email-password/consent.html.ejs +++ b/templates/identity/email-password/consent.html.ejs @@ -1,19 +1,41 @@

Authorize

-

You are authorizing an application to access your Pod.

+

The following client wants to do authorized requests in your name:

+
    +

  1. - +
-

+

diff --git a/test/integration/Identity.test.ts b/test/integration/Identity.test.ts index 5e3ddacf0..6f258d48e 100644 --- a/test/integration/Identity.test.ts +++ b/test/integration/Identity.test.ts @@ -193,6 +193,12 @@ describe('A Solid server with IDP', (): void => { default_max_age: 3600, require_auth_time: true, }; + // This client will always reject requests since there is no valid redirect + const badClientJson = { + ...clientJson, + client_id: badClientId, + redirect_uris: [], + }; /* eslint-enable @typescript-eslint/naming-convention */ let state: IdentityTestState; @@ -205,13 +211,10 @@ describe('A Solid server with IDP', (): void => { body: JSON.stringify(clientJson), }); - // This client will always reject requests since there is no valid redirect - clientJson.client_id = badClientId; - clientJson.redirect_uris = []; await fetch(badClientId, { method: 'PUT', headers: { 'content-type': 'application/ld+json' }, - body: JSON.stringify(clientJson), + body: JSON.stringify(badClientJson), }); }); @@ -224,6 +227,14 @@ describe('A Solid server with IDP', (): void => { const res = await state.fetchIdp(url); expect(res.status).toBe(200); url = await state.login(url, email, password); + + // Verify the client information the server discovered + const consentRes = await state.fetchIdp(url, 'GET'); + expect(consentRes.status).toBe(200); + const { client } = await consentRes.json(); + expect(client.client_id).toBe(clientJson.client_id); + expect(client.client_name).toBe(clientJson.client_name); + await state.consent(url); expect(state.session.info?.webId).toBe(webId); }); diff --git a/test/unit/identity/interaction/ConsentHandler.test.ts b/test/unit/identity/interaction/ConsentHandler.test.ts index 28cf92890..fbb8159e5 100644 --- a/test/unit/identity/interaction/ConsentHandler.test.ts +++ b/test/unit/identity/interaction/ConsentHandler.test.ts @@ -4,6 +4,7 @@ import { ConsentHandler } from '../../../../src/identity/interaction/ConsentHand import type { Interaction } from '../../../../src/identity/interaction/InteractionHandler'; import { FoundHttpError } from '../../../../src/util/errors/FoundHttpError'; import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError'; +import { readJsonStream } from '../../../../src/util/StreamUtil'; import { createPostJsonOperation } from './email-password/handler/Util'; const newGrantId = 'newGrantId'; @@ -45,6 +46,10 @@ class DummyGrant { describe('A ConsentHandler', (): void => { const accountId = 'http://example.com/id#me'; const clientId = 'clientId'; + const clientMetadata = { + // eslint-disable-next-line @typescript-eslint/naming-convention + client_id: 'clientId', + }; let grantFn: jest.Mock & { find: jest.Mock }; let knownGrant: DummyGrant; let oidcInteraction: Interaction; @@ -66,8 +71,12 @@ describe('A ConsentHandler', (): void => { grantFn = jest.fn((props): DummyGrant => new DummyGrant(props)) as any; grantFn.find = jest.fn((grantId: string): any => grantId ? knownGrant : undefined); provider = { - // eslint-disable-next-line @typescript-eslint/naming-convention + /* eslint-disable @typescript-eslint/naming-convention */ Grant: grantFn, + Client: { + find: (id: string): any => (id ? { metadata: jest.fn().mockReturnValue(clientMetadata) } : undefined), + }, + /* eslint-enable @typescript-eslint/naming-convention */ } as any; providerFactory = { @@ -89,6 +98,28 @@ describe('A ConsentHandler', (): void => { .resolves.toBeUndefined(); }); + it('returns the client metadata on a GET request.', async(): Promise => { + const operation = { method: 'GET', target: { path: 'http://example.com/foo' }} as any; + const representation = await handler.handle({ operation, oidcInteraction }); + await expect(readJsonStream(representation.data)).resolves.toEqual({ + client: { + ...clientMetadata, + '@context': 'https://www.w3.org/ns/solid/oidc-context.jsonld', + }, + }); + }); + + it('returns an empty object if no client was found.', async(): Promise => { + delete oidcInteraction.params.client_id; + const operation = { method: 'GET', target: { path: 'http://example.com/foo' }} as any; + const representation = await handler.handle({ operation, oidcInteraction }); + await expect(readJsonStream(representation.data)).resolves.toEqual({ + client: { + '@context': 'https://www.w3.org/ns/solid/oidc-context.jsonld', + }, + }); + }); + it('requires an oidcInteraction with a defined session.', async(): Promise => { oidcInteraction.session = undefined; await expect(handler.handle({ operation: createPostJsonOperation({}), oidcInteraction }))