From 8d0646d8704bf41bb3059ff68359f1d1b8f5ffa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 8 Aug 2017 15:40:45 +0200 Subject: [PATCH 1/3] context: `add @inject.setter` Implement a new decorator for injecting a setter function for (re)binding a new value for the given key. This is useful when implementing Actions that are contributing new Elements. Compared to existing `bindElement`, this new setter approach leads to a less coupled code, because the action invoking the setter does not need to know about the key associated with the binding, in fact a setter function can be used without any Context/Binding classes. This also makes Actions easier to test in isolation. --- packages/context/src/inject.ts | 17 ++++++++++++++++- .../test/acceptance/class-level-bindings.ts | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/context/src/inject.ts b/packages/context/src/inject.ts index 142ce49bf3f9..819c51651ad8 100644 --- a/packages/context/src/inject.ts +++ b/packages/context/src/inject.ts @@ -109,10 +109,25 @@ export namespace inject { ) { return inject(bindingKey, metadata, resolveAsGetter); }; + + export const setter = function injectSetter( + bindingKey: string, + metadata?: Object, + ) { + return inject(bindingKey, metadata, resolveAsSetter); + }; } function resolveAsGetter(ctx: Context, injection: Injection) { - return () => ctx.get(injection.bindingKey); + return function getter() { + return ctx.get(injection.bindingKey); + }; +} + +function resolveAsSetter(ctx: Context, injection: Injection) { + return function setter(value: BoundValue) { + ctx.bind(injection.bindingKey).to(value); + }; } /** diff --git a/packages/context/test/acceptance/class-level-bindings.ts b/packages/context/test/acceptance/class-level-bindings.ts index 759fa5210012..1873ff456643 100644 --- a/packages/context/test/acceptance/class-level-bindings.ts +++ b/packages/context/test/acceptance/class-level-bindings.ts @@ -155,6 +155,22 @@ describe('Context bindings - Injecting dependencies of classes', () => { expect(await store.getter()).to.equal('new-value'); }); + it('injects a setter function', async () => { + class Store { + constructor( + @inject.setter('key') + public setter: (value: string) => void, + ) {} + } + + ctx.bind('store').toClass(Store); + const store = ctx.getSync('store'); + + expect(store.setter).to.be.Function(); + store.setter('a-value'); + expect(ctx.getSync('key')).to.equal('a-value'); + }); + function createContext() { ctx = new Context(); } From 68835a3a00d2e70a821b0b1d1455d89e22710432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 8 Aug 2017 15:58:22 +0200 Subject: [PATCH 2/3] context: add types Setter and Getter Add two new types to make it easier to use `@inject.getter` and `@inject.setter` in TypeScript codebase. These two types represent the function that's being injected. --- packages/context/src/index.ts | 2 +- packages/context/src/inject.ts | 36 +++++++++++++++++++ .../test/acceptance/class-level-bindings.ts | 6 ++-- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/packages/context/src/index.ts b/packages/context/src/index.ts index bb5b70d2c95f..1d2e4d4de658 100644 --- a/packages/context/src/index.ts +++ b/packages/context/src/index.ts @@ -6,7 +6,7 @@ export {Binding, BindingScope, BoundValue, ValueOrPromise} from './binding'; export {Context} from './context'; export {Constructor} from './resolver'; -export {inject} from './inject'; +export {inject, Setter, Getter} from './inject'; export {NamespacedReflect} from './reflect'; export {Provider} from './provider'; export {isPromise} from './is-promise'; diff --git a/packages/context/src/inject.ts b/packages/context/src/inject.ts index 819c51651ad8..352dd1934d6a 100644 --- a/packages/context/src/inject.ts +++ b/packages/context/src/inject.ts @@ -102,7 +102,30 @@ export function inject( }; } +/** + * The function injected by `@inject.getter(key)`. + */ +export type Getter = () => Promise; + +/** + * The function injected by `@inject.setter(key)`. + */ +export type Setter = (value: T) => void; + export namespace inject { + /** + * Inject a function for getting the actual bound value. + * + * This is useful when implementing Actions, where + * the action is instantiated for Sequence constructor, but some + * of action's dependencies become bound only after other actions + * have been executed by the sequence. + * + * See also `Getter`. + * + * @param bindingKey The key of the value we want to eventually get. + * @param metadata Optional metadata to help the injection + */ export const getter = function injectGetter( bindingKey: string, metadata?: Object, @@ -110,6 +133,19 @@ export namespace inject { return inject(bindingKey, metadata, resolveAsGetter); }; + /** + * Inject a function for setting (binding) the given key to a given + * value. (Only static/constant values are supported, it's not possible + * to bind a key to a class or a provider.) + * + * This is useful e.g. when implementing Actions that are contributing + * new Elements. + * + * See also `Setter`. + * + * @param bindingKey The key of the value we want to set. + * @param metadata Optional metadata to help the injection + */ export const setter = function injectSetter( bindingKey: string, metadata?: Object, diff --git a/packages/context/test/acceptance/class-level-bindings.ts b/packages/context/test/acceptance/class-level-bindings.ts index 1873ff456643..24259875973d 100644 --- a/packages/context/test/acceptance/class-level-bindings.ts +++ b/packages/context/test/acceptance/class-level-bindings.ts @@ -4,7 +4,7 @@ // License text available at https://opensource.org/licenses/MIT import {expect} from '@loopback/testlab'; -import {Context, inject, isPromise} from '../..'; +import {Context, inject, isPromise, Setter, Getter} from '../..'; const INFO_CONTROLLER = 'controllers.info'; @@ -140,7 +140,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { class Store { constructor( @inject.getter('key') - public getter: Function, + public getter: Getter, ) {} } @@ -159,7 +159,7 @@ describe('Context bindings - Injecting dependencies of classes', () => { class Store { constructor( @inject.setter('key') - public setter: (value: string) => void, + public setter: Setter, ) {} } From b37bf6dec7e274cd2d74e72c2bdf2fb2b19db829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 8 Aug 2017 15:50:17 +0200 Subject: [PATCH 3/3] authentication: bind current user in the context plus general cleanup. --- packages/authentication/src/keys.ts | 1 + .../src/providers/authenticate.ts | 33 ++++++++++--------- .../test/acceptance/basic-auth.ts | 11 +++---- ...er.test.ts => authenticate-action.test.ts} | 14 ++++++-- ...ator.ts => authenticate-decorator.test.ts} | 0 ...-provider.ts => metadata-provider.test.ts} | 0 6 files changed, 35 insertions(+), 24 deletions(-) rename packages/authentication/test/unit/{provider.test.ts => authenticate-action.test.ts} (89%) rename packages/authentication/test/unit/{decorator.ts => authenticate-decorator.test.ts} (100%) rename packages/authentication/test/unit/{metadata-provider.ts => metadata-provider.test.ts} (100%) diff --git a/packages/authentication/src/keys.ts b/packages/authentication/src/keys.ts index 10e6a7e584c0..fdf87d5c7661 100644 --- a/packages/authentication/src/keys.ts +++ b/packages/authentication/src/keys.ts @@ -11,5 +11,6 @@ export namespace BindingKeys { export const STRATEGY = 'authentication.strategy'; export const AUTH_ACTION = 'authentication.actions.authenticate'; export const METADATA = 'authentication.operationMetadata'; + export const CURRENT_USER = 'authentication.currentUser'; } } diff --git a/packages/authentication/src/providers/authenticate.ts b/packages/authentication/src/providers/authenticate.ts index 6d7475fff3c9..a4ecc8f81584 100644 --- a/packages/authentication/src/providers/authenticate.ts +++ b/packages/authentication/src/providers/authenticate.ts @@ -5,7 +5,7 @@ import * as http from 'http'; import {HttpErrors, inject, ParsedRequest} from '@loopback/core'; -import {Provider} from '@loopback/context'; +import {Provider, Getter, Setter} from '@loopback/context'; import {Strategy} from 'passport'; import {StrategyAdapter} from '../strategy-adapter'; import {BindingKeys} from '../keys'; @@ -44,6 +44,8 @@ export class AuthenticationProvider implements Provider { // is executed. @inject.getter(BindingKeys.Authentication.STRATEGY) readonly getStrategy: () => Promise, + @inject.setter(BindingKeys.Authentication.CURRENT_USER) + readonly setCurrentUser: (value: UserProfile) => void, ) {} /** @@ -51,34 +53,33 @@ export class AuthenticationProvider implements Provider { */ value(): AuthenticateFn { return async (request: ParsedRequest) => { - const strategy = await this.getStrategy(); - return await getAuthenticatedUser(strategy, request); + return await authenticateRequest( + this.getStrategy, + request, + this.setCurrentUser, + ); }; } } /** - * @description a function which accepts (passport-strategy, request) - * and returns a user + * The implementation of authenticate() sequence action. * @param strategy Passport strategy * @param request Parsed Request - * - * @example - * ```ts - * const strategy = new BasicStrategy(async (username, password) => { - * return await findUser(username, password); - * }; - * getAuthenticatedUser(strategy, ParsedRequest); - * ``` + * @param setCurrentUser The setter function to update the current user + * in the per-request Context */ -export async function getAuthenticatedUser( - strategy: Strategy, +async function authenticateRequest( + getStrategy: Getter, request: ParsedRequest, + setCurrentUser: Setter, ): Promise { + const strategy = await getStrategy(); if (!strategy.authenticate) { return Promise.reject(new Error('invalid strategy parameter')); } const strategyAdapter = new StrategyAdapter(strategy); - const user: UserProfile = await strategyAdapter.authenticate(request); + const user = await strategyAdapter.authenticate(request); + setCurrentUser(user); return user; } diff --git a/packages/authentication/test/acceptance/basic-auth.ts b/packages/authentication/test/acceptance/basic-auth.ts index d1ef3cf01015..7053aeee003f 100644 --- a/packages/authentication/test/acceptance/basic-auth.ts +++ b/packages/authentication/test/acceptance/basic-auth.ts @@ -71,7 +71,7 @@ describe('Basic Authentication', () => { users = new UserRepository({ joe : {profile: {id: 'joe'}, password: '12345'}, Simpson: {profile: {id: 'sim123'}, password: 'alpha'}, - Flintstone: {profile: {id: 'Flint'}, password: 'beta'}, + Flinstone: {profile: {id: 'Flint'}, password: 'beta'}, George: {profile: {id: 'Curious'}, password: 'gamma'}, }); } @@ -96,7 +96,10 @@ describe('Basic Authentication', () => { @api(apispec) class MyController { - constructor(@inject('authentication.user') private user: UserProfile) {} + constructor( + @inject(BindingKeys.Authentication.CURRENT_USER) + private user: UserProfile, + ) {} @authenticate('BasicStrategy') async whoAmI() : Promise { @@ -127,10 +130,6 @@ describe('Basic Authentication', () => { // Authenticate const user: UserProfile = await this.authenticateRequest(req); - // User is expected to be returned or an exception should be thrown - if (user) this.bindElement('authentication.user').to(user); - else throw new HttpErrors.InternalServerError('auth error'); - // Authentication successful, proceed to invoke controller const args = await this.parseParams(req, route); const result = await this.invoke(route, args); diff --git a/packages/authentication/test/unit/provider.test.ts b/packages/authentication/test/unit/authenticate-action.test.ts similarity index 89% rename from packages/authentication/test/unit/provider.test.ts rename to packages/authentication/test/unit/authenticate-action.test.ts index b5fd00165a00..e3aa9d61276a 100644 --- a/packages/authentication/test/unit/provider.test.ts +++ b/packages/authentication/test/unit/authenticate-action.test.ts @@ -8,7 +8,6 @@ import {Context, Provider, instantiateClass} from '@loopback/context'; import {ParsedRequest} from '@loopback/core'; import { AuthenticationProvider, - getAuthenticatedUser, AuthenticateFn, UserProfile, BindingKeys, @@ -30,6 +29,7 @@ describe('AuthenticationProvider', () => { describe('value()', () => { let provider: AuthenticationProvider; let strategy: MockStrategy; + let currentUser: UserProfile | undefined; const mockUser: UserProfile = {name: 'user-name', id: 'mock-id'}; @@ -45,6 +45,13 @@ describe('AuthenticationProvider', () => { expect(user).to.be.equal(mockUser); }); + it('updates current user', async () => { + const authenticate = await Promise.resolve(provider.value()); + const request = {}; + await authenticate(request); + expect(currentUser).to.equal(mockUser); + }); + describe('context.get(provider_key)', () => { it('returns a function which authenticates a request and returns a user', async () => { @@ -106,7 +113,10 @@ describe('AuthenticationProvider', () => { function givenAuthenticationProvider() { strategy = new MockStrategy(); strategy.setMockUser(mockUser); - provider = new AuthenticationProvider(() => Promise.resolve(strategy)); + provider = new AuthenticationProvider( + () => Promise.resolve(strategy), + u => currentUser = u); + currentUser = undefined; } }); }); diff --git a/packages/authentication/test/unit/decorator.ts b/packages/authentication/test/unit/authenticate-decorator.test.ts similarity index 100% rename from packages/authentication/test/unit/decorator.ts rename to packages/authentication/test/unit/authenticate-decorator.test.ts diff --git a/packages/authentication/test/unit/metadata-provider.ts b/packages/authentication/test/unit/metadata-provider.test.ts similarity index 100% rename from packages/authentication/test/unit/metadata-provider.ts rename to packages/authentication/test/unit/metadata-provider.test.ts