fix: Update OIDC provider dependency to v7

The biggest resulting change is that the consent page always appears
after logging in.
Some minor fixes to be closer to the spec are included
together with some minor structural refactors.

test/unit/identity/OidcHttpHandler.test.ts

@@ -13,7 +13,7 @@ describe('An OidcHttpHandler', (): void => {
 
   beforeEach(async(): Promise<void> => {
     provider = {
-      callback: jest.fn(),
+      callback: jest.fn().mockReturnValue(jest.fn()),
     } as any;
 
     providerFactory = {
@@ -26,6 +26,7 @@ describe('An OidcHttpHandler', (): void => {
   it('sends all requests to the OIDC library.', async(): Promise<void> => {
     await expect(handler.handle({ request, response })).resolves.toBeUndefined();
     expect(provider.callback).toHaveBeenCalledTimes(1);
-    expect(provider.callback).toHaveBeenLastCalledWith(request, response);
+    expect(provider.callback.mock.results[0].value).toHaveBeenCalledTimes(1);
+    expect(provider.callback.mock.results[0].value).toHaveBeenLastCalledWith(request, response);
   });
 });

test/unit/identity/configuration/IdentityProviderFactory.test.ts

@@ -16,7 +16,7 @@ jest.mock('oidc-provider', (): any => ({
 
 const routes = {
   authorization: '/foo/oidc/auth',
-  check_session: '/foo/oidc/session/check',
+  backchannel_authentication: '/foo/oidc/backchannel',
   code_verification: '/foo/oidc/device',
   device_authorization: '/foo/oidc/device/auth',
   end_session: '/foo/oidc/session/end',
@@ -100,23 +100,32 @@ describe('An IdentityProviderFactory', (): void => {
     expect(adapterFactory.createStorageAdapter).toHaveBeenLastCalledWith('test!');
 
     expect(config.cookies?.keys).toEqual([ expect.any(String) ]);
-    expect(config.jwks).toEqual({ keys: [ expect.objectContaining({ kty: 'RSA' }) ]});
+    expect(config.jwks).toEqual({ keys: [ expect.objectContaining({ alg: 'ES256' }) ]});
     expect(config.routes).toEqual(routes);
+    expect(config.pkce?.methods).toEqual([ 'S256' ]);
+    expect((config.pkce!.required as any)()).toBe(true);
+    expect(config.clientDefaults?.id_token_signed_response_alg).toBe('ES256');
 
     await expect((config.interactions?.url as any)(ctx, oidcInteraction)).resolves.toBe(redirectUrl);
-    expect((config.audiences as any)(null, null, {}, 'access_token')).toBe('solid');
-    expect((config.audiences as any)(null, null, { clientId: 'clientId' }, 'client_credentials')).toBe('clientId');
 
-    const findResult = await config.findAccount?.({ oidc: { client: { clientId: 'clientId' }}} as any, webId);
+    let findResult = await config.findAccount?.({ oidc: { client: { clientId: 'clientId' }}} as any, webId);
     expect(findResult?.accountId).toBe(webId);
+    await expect((findResult?.claims as any)()).resolves.toEqual({ sub: webId, webid: webId, azp: 'clientId' });
+    findResult = await config.findAccount?.({ oidc: {}} as any, webId);
     await expect((findResult?.claims as any)()).resolves.toEqual({ sub: webId, webid: webId });
 
-    expect((config.extraAccessTokenClaims as any)({}, {})).toEqual({});
-    expect((config.extraAccessTokenClaims as any)({}, { kind: 'AccessToken', accountId: webId, clientId: 'clientId' }))
-      .toEqual({
-        webid: webId,
-        client_id: 'clientId',
-      });
+    expect((config.extraTokenClaims as any)({}, {})).toEqual({});
+    expect((config.extraTokenClaims as any)({}, { kind: 'AccessToken', accountId: webId, clientId: 'clientId' }))
+      .toEqual({ webid: webId });
+
+    expect(config.features?.resourceIndicators?.enabled).toBe(true);
+    expect((config.features?.resourceIndicators?.defaultResource as any)()).toBe('http://example.com/');
+    expect((config.features?.resourceIndicators?.getResourceServerInfo as any)()).toEqual({
+      scope: '',
+      audience: 'solid',
+      accessTokenFormat: 'jwt',
+      jwt: { sign: { alg: 'ES256' }},
+    });
 
     // Test the renderError function
     const response = { } as HttpResponse;

test/unit/identity/interaction/CompletingInteractionHandler.test.ts

@@ -1,76 +0,0 @@
-import type { Operation } from '../../../../src/http/Operation';
-import { BasicRepresentation } from '../../../../src/http/representation/BasicRepresentation';
-import { CompletingInteractionHandler } from '../../../../src/identity/interaction/CompletingInteractionHandler';
-import type {
-  Interaction,
-  InteractionHandlerInput,
-} from '../../../../src/identity/interaction/InteractionHandler';
-import type {
-  InteractionCompleter,
-  InteractionCompleterInput,
-} from '../../../../src/identity/interaction/util/InteractionCompleter';
-import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
-
-const webId = 'http://alice.test.com/card#me';
-class DummyCompletingInteractionHandler extends CompletingInteractionHandler {
-  public constructor(interactionCompleter: InteractionCompleter) {
-    super({}, interactionCompleter);
-  }
-
-  public async getCompletionParameters(input: Required<InteractionHandlerInput>): Promise<InteractionCompleterInput> {
-    return { webId, oidcInteraction: input.oidcInteraction };
-  }
-}
-
-describe('A CompletingInteractionHandler', (): void => {
-  const oidcInteraction: Interaction = {} as any;
-  const location = 'http://test.com/redirect';
-  let operation: Operation;
-  let interactionCompleter: jest.Mocked<InteractionCompleter>;
-  let handler: DummyCompletingInteractionHandler;
-
-  beforeEach(async(): Promise<void> => {
-    const representation = new BasicRepresentation('', 'application/json');
-    operation = {
-      method: 'POST',
-      body: representation,
-    } as any;
-
-    interactionCompleter = {
-      handleSafe: jest.fn().mockResolvedValue(location),
-    } as any;
-
-    handler = new DummyCompletingInteractionHandler(interactionCompleter);
-  });
-
-  it('calls the parent JSON canHandle check.', async(): Promise<void> => {
-    operation.body.metadata.contentType = 'application/x-www-form-urlencoded';
-    await expect(handler.canHandle({ operation, oidcInteraction } as any)).rejects.toThrow(NotImplementedHttpError);
-  });
-
-  it('can handle GET requests without interaction.', async(): Promise<void> => {
-    operation.method = 'GET';
-    await expect(handler.canHandle({ operation } as any)).resolves.toBeUndefined();
-  });
-
-  it('errors if no OidcInteraction is defined on POST requests.', async(): Promise<void> => {
-    const error = expect.objectContaining({
-      statusCode: 400,
-      message: 'This action can only be performed as part of an OIDC authentication flow.',
-      errorCode: 'E0002',
-    });
-    await expect(handler.canHandle({ operation })).rejects.toThrow(error);
-
-    await expect(handler.canHandle({ operation, oidcInteraction })).resolves.toBeUndefined();
-  });
-
-  it('throws a redirect error with the completer location.', async(): Promise<void> => {
-    const error = expect.objectContaining({
-      statusCode: 302,
-      location,
-    });
-    await expect(handler.handle({ operation, oidcInteraction })).rejects.toThrow(error);
-    expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(1);
-    expect(interactionCompleter.handleSafe).toHaveBeenLastCalledWith({ oidcInteraction, webId });
-  });
-});

test/unit/identity/interaction/ConsentHandler.test.ts

@@ -0,0 +1,142 @@
+import type { Provider } from 'oidc-provider';
+import type { ProviderFactory } from '../../../../src/identity/configuration/ProviderFactory';
+import { ConsentHandler } from '../../../../src/identity/interaction/ConsentHandler';
+import type { Interaction } from '../../../../src/identity/interaction/InteractionHandler';
+import { FoundHttpError } from '../../../../src/util/errors/FoundHttpError';
+import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
+import { createPostJsonOperation } from './email-password/handler/Util';
+
+const newGrantId = 'newGrantId';
+class DummyGrant {
+  public accountId: string;
+  public clientId: string;
+
+  public readonly scopes: string[] = [];
+  public claims: string[] = [];
+  public readonly rejectedScopes: string[] = [];
+  public readonly resourceScopes: Record<string, string> = {};
+
+  public constructor(props: { accountId: string; clientId: string }) {
+    this.accountId = props.accountId;
+    this.clientId = props.clientId;
+  }
+
+  public rejectOIDCScope(scope: string): void {
+    this.rejectedScopes.push(scope);
+  }
+
+  public addOIDCScope(scope: string): void {
+    this.scopes.push(scope);
+  }
+
+  public addOIDCClaims(claims: string[]): void {
+    this.claims = claims;
+  }
+
+  public addResourceScope(resource: string, scope: string): void {
+    this.resourceScopes[resource] = scope;
+  }
+
+  public async save(): Promise<string> {
+    return newGrantId;
+  }
+}
+
+describe('A ConsentHandler', (): void => {
+  const accountId = 'http://example.com/id#me';
+  const clientId = 'clientId';
+  let grantFn: jest.Mock<DummyGrant> & { find: jest.Mock<DummyGrant> };
+  let knownGrant: DummyGrant;
+  let oidcInteraction: Interaction;
+  let provider: jest.Mocked<Provider>;
+  let providerFactory: jest.Mocked<ProviderFactory>;
+  let handler: ConsentHandler;
+
+  beforeEach(async(): Promise<void> => {
+    oidcInteraction = {
+      session: { accountId },
+      // eslint-disable-next-line @typescript-eslint/naming-convention
+      params: { client_id: clientId },
+      prompt: { details: {}},
+      save: jest.fn(),
+    } as any;
+
+    knownGrant = new DummyGrant({ accountId, clientId });
+
+    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
+      Grant: grantFn,
+    } as any;
+
+    providerFactory = {
+      getProvider: jest.fn().mockResolvedValue(provider),
+    };
+
+    handler = new ConsentHandler(providerFactory);
+  });
+
+  it('errors if no oidcInteraction is defined on POST requests.', async(): Promise<void> => {
+    const error = expect.objectContaining({
+      statusCode: 400,
+      message: 'This action can only be performed as part of an OIDC authentication flow.',
+      errorCode: 'E0002',
+    });
+    await expect(handler.canHandle({ operation: createPostJsonOperation({}) })).rejects.toThrow(error);
+
+    await expect(handler.canHandle({ operation: createPostJsonOperation({}), oidcInteraction }))
+      .resolves.toBeUndefined();
+  });
+
+  it('requires an oidcInteraction with a defined session.', async(): Promise<void> => {
+    oidcInteraction.session = undefined;
+    await expect(handler.handle({ operation: createPostJsonOperation({}), oidcInteraction }))
+      .rejects.toThrow(NotImplementedHttpError);
+  });
+
+  it('throws a redirect error.', async(): Promise<void> => {
+    const operation = createPostJsonOperation({});
+    await expect(handler.handle({ operation, oidcInteraction })).rejects.toThrow(FoundHttpError);
+  });
+
+  it('stores the requested scopes and claims in the grant.', async(): Promise<void> => {
+    oidcInteraction.prompt.details = {
+      missingOIDCScope: [ 'scope1', 'scope2' ],
+      missingOIDCClaims: [ 'claim1', 'claim2' ],
+      missingResourceScopes: { resource: [ 'scope1', 'scope2' ]},
+    };
+
+    const operation = createPostJsonOperation({ remember: true });
+    await expect(handler.handle({ operation, oidcInteraction })).rejects.toThrow(FoundHttpError);
+    expect(grantFn.mock.results).toHaveLength(1);
+    expect(grantFn.mock.results[0].value.scopes).toEqual([ 'scope1 scope2' ]);
+    expect(grantFn.mock.results[0].value.claims).toEqual([ 'claim1', 'claim2' ]);
+    expect(grantFn.mock.results[0].value.resourceScopes).toEqual({ resource: 'scope1 scope2' });
+    expect(grantFn.mock.results[0].value.rejectedScopes).toEqual([]);
+  });
+
+  it('creates a new Grant when needed.', async(): Promise<void> => {
+    const operation = createPostJsonOperation({});
+    await expect(handler.handle({ operation, oidcInteraction })).rejects.toThrow(FoundHttpError);
+    expect(grantFn).toHaveBeenCalledTimes(1);
+    expect(grantFn).toHaveBeenLastCalledWith({ accountId, clientId });
+    expect(grantFn.find).toHaveBeenCalledTimes(0);
+  });
+
+  it('reuses existing Grant objects.', async(): Promise<void> => {
+    const operation = createPostJsonOperation({});
+    oidcInteraction.grantId = '123456';
+    await expect(handler.handle({ operation, oidcInteraction })).rejects.toThrow(FoundHttpError);
+    expect(grantFn).toHaveBeenCalledTimes(0);
+    expect(grantFn.find).toHaveBeenCalledTimes(1);
+    expect(grantFn.find).toHaveBeenLastCalledWith('123456');
+  });
+
+  it('rejectes offline_access as scope if a user does not want to be remembered.', async(): Promise<void> => {
+    const operation = createPostJsonOperation({});
+    await expect(handler.handle({ operation, oidcInteraction })).rejects.toThrow(FoundHttpError);
+    expect(grantFn.mock.results).toHaveLength(1);
+    expect(grantFn.mock.results[0].value.rejectedScopes).toEqual([ 'offline_access' ]);
+  });
+});

test/unit/identity/interaction/ExistingLoginHandler.test.ts

@@ -1,38 +0,0 @@
-import { ExistingLoginHandler } from '../../../../src/identity/interaction/ExistingLoginHandler';
-import type { Interaction } from '../../../../src/identity/interaction/InteractionHandler';
-import type {
-  InteractionCompleter,
-} from '../../../../src/identity/interaction/util/InteractionCompleter';
-import { FoundHttpError } from '../../../../src/util/errors/FoundHttpError';
-import { NotImplementedHttpError } from '../../../../src/util/errors/NotImplementedHttpError';
-import { createPostJsonOperation } from './email-password/handler/Util';
-
-describe('An ExistingLoginHandler', (): void => {
-  const webId = 'http://test.com/id#me';
-  let oidcInteraction: Interaction;
-  let interactionCompleter: jest.Mocked<InteractionCompleter>;
-  let handler: ExistingLoginHandler;
-
-  beforeEach(async(): Promise<void> => {
-    oidcInteraction = { session: { accountId: webId }} as any;
-
-    interactionCompleter = {
-      handleSafe: jest.fn().mockResolvedValue('http://test.com/redirect'),
-    } as any;
-
-    handler = new ExistingLoginHandler(interactionCompleter);
-  });
-
-  it('requires an oidcInteraction with a defined session.', async(): Promise<void> => {
-    oidcInteraction.session = undefined;
-    await expect(handler.handle({ operation: createPostJsonOperation({}), oidcInteraction }))
-      .rejects.toThrow(NotImplementedHttpError);
-  });
-
-  it('returns the correct completion parameters.', async(): Promise<void> => {
-    const operation = createPostJsonOperation({ remember: true });
-    await expect(handler.handle({ operation, oidcInteraction })).rejects.toThrow(FoundHttpError);
-    expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(1);
-    expect(interactionCompleter.handleSafe).toHaveBeenLastCalledWith({ oidcInteraction, webId, shouldRemember: true });
-  });
-});

test/unit/identity/interaction/email-password/handler/ForgotPasswordHandler.test.ts

@@ -3,8 +3,8 @@ import {
   ForgotPasswordHandler,
 } from '../../../../../../src/identity/interaction/email-password/handler/ForgotPasswordHandler';
 import type { AccountStore } from '../../../../../../src/identity/interaction/email-password/storage/AccountStore';
+import type { EmailSender } from '../../../../../../src/identity/interaction/email-password/util/EmailSender';
 import type { InteractionRoute } from '../../../../../../src/identity/interaction/routing/InteractionRoute';
-import type { EmailSender } from '../../../../../../src/identity/interaction/util/EmailSender';
 import { readJsonStream } from '../../../../../../src/util/StreamUtil';
 import type { TemplateEngine } from '../../../../../../src/util/templates/TemplateEngine';
 import { createPostJsonOperation } from './Util';

test/unit/identity/interaction/email-password/handler/LoginHandler.test.ts

@@ -4,22 +4,23 @@ import type {
   Interaction,
   InteractionHandlerInput,
 } from '../../../../../../src/identity/interaction/InteractionHandler';
-import type {
-  InteractionCompleter,
-} from '../../../../../../src/identity/interaction/util/InteractionCompleter';
 import { FoundHttpError } from '../../../../../../src/util/errors/FoundHttpError';
 import { createPostJsonOperation } from './Util';
 
 describe('A LoginHandler', (): void => {
   const webId = 'http://alice.test.com/card#me';
   const email = 'alice@test.email';
-  const oidcInteraction: Interaction = {} as any;
+  let oidcInteraction: jest.Mocked<Interaction>;
   let input: Required<InteractionHandlerInput>;
   let accountStore: jest.Mocked<AccountStore>;
-  let interactionCompleter: jest.Mocked<InteractionCompleter>;
   let handler: LoginHandler;
 
   beforeEach(async(): Promise<void> => {
+    oidcInteraction = {
+      exp: 123456,
+      save: jest.fn(),
+    } as any;
+
     input = { oidcInteraction } as any;
 
     accountStore = {
@@ -27,11 +28,18 @@ describe('A LoginHandler', (): void => {
       getSettings: jest.fn().mockResolvedValue({ useIdp: true }),
     } as any;
 
-    interactionCompleter = {
-      handleSafe: jest.fn().mockResolvedValue('http://test.com/redirect'),
-    } as any;
+    handler = new LoginHandler(accountStore);
+  });
+  it('errors if no oidcInteraction is defined on POST requests.', async(): Promise<void> => {
+    const error = expect.objectContaining({
+      statusCode: 400,
+      message: 'This action can only be performed as part of an OIDC authentication flow.',
+      errorCode: 'E0002',
+    });
+    await expect(handler.canHandle({ operation: createPostJsonOperation({}) })).rejects.toThrow(error);
 
-    handler = new LoginHandler(accountStore, interactionCompleter);
+    await expect(handler.canHandle({ operation: createPostJsonOperation({}), oidcInteraction }))
+      .resolves.toBeUndefined();
   });
 
   it('errors on invalid emails.', async(): Promise<void> => {
@@ -61,13 +69,13 @@ describe('A LoginHandler', (): void => {
       .rejects.toThrow('This server is not an identity provider for this account.');
   });
 
-  it('returns the correct completion parameters.', async(): Promise<void> => {
+  it('returns the generated redirect URL.', async(): Promise<void> => {
     input.operation = createPostJsonOperation({ email, password: 'password!' });
     await expect(handler.handle(input)).rejects.toThrow(FoundHttpError);
 
     expect(accountStore.authenticate).toHaveBeenCalledTimes(1);
     expect(accountStore.authenticate).toHaveBeenLastCalledWith(email, 'password!');
-    expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(1);
-    expect(interactionCompleter.handleSafe).toHaveBeenLastCalledWith({ oidcInteraction, webId, shouldRemember: false });
+    expect(oidcInteraction.save).toHaveBeenCalledTimes(1);
+    expect(oidcInteraction.result).toEqual({ login: { accountId: webId, remember: false }});
   });
 });

test/unit/identity/interaction/email-password/util/BaseEmailSender.test.ts

@@ -1,6 +1,6 @@
-import type { EmailSenderArgs } from '../../../../../src/identity/interaction/util/BaseEmailSender';
-import { BaseEmailSender } from '../../../../../src/identity/interaction/util/BaseEmailSender';
-import type { EmailArgs } from '../../../../../src/identity/interaction/util/EmailSender';
+import type { EmailSenderArgs } from '../../../../../../src/identity/interaction/email-password/util/BaseEmailSender';
+import { BaseEmailSender } from '../../../../../../src/identity/interaction/email-password/util/BaseEmailSender';
+import type { EmailArgs } from '../../../../../../src/identity/interaction/email-password/util/EmailSender';
 jest.mock('nodemailer');
 
 describe('A BaseEmailSender', (): void => {

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

@@ -1,56 +0,0 @@
-import type { Interaction } from '../../../../../src/identity/interaction/InteractionHandler';
-import { BaseInteractionCompleter } from '../../../../../src/identity/interaction/util/BaseInteractionCompleter';
-
-jest.useFakeTimers();
-
-describe('A BaseInteractionCompleter', (): void => {
-  const now = Math.floor(Date.now() / 1000);
-  const webId = 'http://alice.test.com/#me';
-  let oidcInteraction: jest.Mocked<Interaction>;
-  let completer: BaseInteractionCompleter;
-
-  beforeEach(async(): Promise<void> => {
-    oidcInteraction = {
-      lastSubmission: {},
-      exp: now + 500,
-      returnTo: 'http://test.com/redirect',
-      save: jest.fn(),
-    } as any;
-
-    completer = new BaseInteractionCompleter();
-  });
-
-  it('stores the correct data in the interaction.', async(): Promise<void> => {
-    await expect(completer.handle({ oidcInteraction, webId, shouldRemember: true }))
-      .resolves.toBe(oidcInteraction.returnTo);
-    expect(oidcInteraction.result).toEqual({
-      login: {
-        account: webId,
-        remember: true,
-        ts: now,
-      },
-      consent: {
-        rejectedScopes: [],
-      },
-    });
-    expect(oidcInteraction.save).toHaveBeenCalledTimes(1);
-    expect(oidcInteraction.save).toHaveBeenLastCalledWith(500);
-  });
-
-  it('rejects offline access if shouldRemember is false.', async(): Promise<void> => {
-    await expect(completer.handle({ oidcInteraction, webId, shouldRemember: false }))
-      .resolves.toBe(oidcInteraction.returnTo);
-    expect(oidcInteraction.result).toEqual({
-      login: {
-        account: webId,
-        remember: false,
-        ts: now,
-      },
-      consent: {
-        rejectedScopes: [ 'offline_access' ],
-      },
-    });
-    expect(oidcInteraction.save).toHaveBeenCalledTimes(1);
-    expect(oidcInteraction.save).toHaveBeenLastCalledWith(500);
-  });
-});