Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create jwt auth service #33

Merged
merged 1 commit into from
Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,15 @@ import {
AuthenticationBindings,
AuthenticationComponent,
} from '@loopback/authentication';
import {JWTAuthenticationBindings, OtherServicesBindings} from './keys';
import {StrategyResolverProvider} from './providers/strategy.resolver.provider';
import {AuthenticateActionProvider} from './providers/custom.authentication.provider';
import {
JWTAuthenticationService,
JWT_SECRET,
} from './services/JWT.authentication.service';
import {hashPassword} from './services/hash.password.bcryptjs';
import {JWTStrategy} from './authentication-strategies/JWT.strategy';

/**
* Information from package.json
Expand All @@ -38,6 +45,7 @@ export class ShoppingApplication extends BootMixin(
// Bind package.json to the application context
this.bind(PackageKey).to(pkg);

// Bind authentication component related elements
this.component(AuthenticationComponent);
this.bind(AuthenticationBindings.AUTH_ACTION).toProvider(
AuthenticateActionProvider,
Expand All @@ -46,6 +54,16 @@ export class ShoppingApplication extends BootMixin(
StrategyResolverProvider,
);

// Bind JWT authentication strategy related elements
this.bind(JWTAuthenticationBindings.STRATEGY).toClass(JWTStrategy);
this.bind(JWTAuthenticationBindings.SECRET).to(JWT_SECRET);
this.bind(JWTAuthenticationBindings.SERVICE).toClass(
JWTAuthenticationService,
);

// Bind other services
this.bind(OtherServicesBindings.HASH_PASSWORD).to(hashPassword);

// Set up the custom sequence
this.sequence(MySequence);

Expand Down
16 changes: 12 additions & 4 deletions src/authentication-strategies/JWT.strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

// Consider turn it to a binding
const SECRET = 'secretforjwt';
import {JWTAuthenticationBindings} from '../keys';
import {Request, HttpErrors} from '@loopback/rest';
import {UserProfile} from '@loopback/authentication';
import {AuthenticationStrategy} from './authentication.strategy';
import {decodeAccessToken} from '../utils/user.authentication';
import {inject} from '@loopback/core';
import {JWTAuthenticationService} from '../services/JWT.authentication.service';

export class JWTStrategy implements AuthenticationStrategy {
constructor(
@inject(JWTAuthenticationBindings.SERVICE)
public jwt_authentication_service: JWTAuthenticationService,
@inject(JWTAuthenticationBindings.SECRET)
public jwt_secret: string,
) {}
async authenticate(request: Request): Promise<UserProfile | undefined> {
let token = request.query.access_token || request.headers['authorization'];
if (!token) throw new HttpErrors.Unauthorized('No access token found!');
Expand All @@ -20,7 +26,9 @@ export class JWTStrategy implements AuthenticationStrategy {
}

try {
const user = await decodeAccessToken(token, SECRET);
const user = await this.jwt_authentication_service.decodeAccessToken(
token,
);
return user;
} catch (err) {
Object.assign(err, {
Expand Down
19 changes: 12 additions & 7 deletions src/controllers/user.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ import {
AuthenticationBindings,
} from '@loopback/authentication';
import {Credentials} from '../repositories/user.repository';
import {
validateCredentials,
getAccessTokenForUser,
hashPassword,
} from '../utils/user.authentication';
import {HashPassword} from '../services/hash.password.bcryptjs';
import {JWTAuthenticationService} from '../services/JWT.authentication.service';
import {JWTAuthenticationBindings, OtherServicesBindings} from '../keys';
import {validateCredentials} from '../services/JWT.authentication.service';
import * as _ from 'lodash';

// TODO(jannyHou): This should be moved to @loopback/authentication
Expand All @@ -40,12 +39,16 @@ export class UserController {
public recommender: RecommenderService,
@inject.setter(AuthenticationBindings.CURRENT_USER)
public setCurrentUser: Setter<UserProfile>,
@inject(OtherServicesBindings.HASH_PASSWORD)
public hashPassword: HashPassword,
@inject(JWTAuthenticationBindings.SERVICE)
public jwt_authentication_service: JWTAuthenticationService,
) {}

@post('/users')
async create(@requestBody() user: User): Promise<User> {
validateCredentials(_.pick(user, ['email', 'password']));
user.password = await hashPassword(user.password, 10);
user.password = await this.hashPassword(user.password, 10);

// Save & Return Result
const savedUser = await this.userRepository.create(user);
Expand Down Expand Up @@ -143,7 +146,9 @@ export class UserController {
@requestBody() credentials: Credentials,
): Promise<{token: string}> {
validateCredentials(credentials);
const token = await getAccessTokenForUser(this.userRepository, credentials);
const token = await this.jwt_authentication_service.getAccessTokenForUser(
credentials,
);
return {token};
}
}
22 changes: 22 additions & 0 deletions src/keys.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {BindingKey} from '@loopback/context';
import {JWTAuthenticationService} from './services/JWT.authentication.service';
import {HashPassword} from './services/hash.password.bcryptjs';
import {JWTStrategy} from './authentication-strategies/JWT.strategy';

// Discussion point for reviewers:
// What would be the good naming conversion for bindings?
export namespace JWTAuthenticationBindings {
export const STRATEGY = BindingKey.create<JWTStrategy>(
'authentication.strategies.jwt.strategy',
);
export const SECRET = BindingKey.create<string>('authentication.jwt.secret');
export const SERVICE = BindingKey.create<JWTAuthenticationService>(
'services.authentication.jwt.service',
);
}

export namespace OtherServicesBindings {
export const HASH_PASSWORD = BindingKey.create<HashPassword>(
'services.hash_password',
);
}
5 changes: 4 additions & 1 deletion src/providers/strategy.resolver.provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ import {
AuthenticationMetadata,
} from '@loopback/authentication';
import {JWTStrategy} from '../authentication-strategies/JWT.strategy';
import {JWTAuthenticationBindings} from '../keys';
export class StrategyResolverProvider
implements Provider<JWTStrategy | undefined> {
constructor(
@inject(AuthenticationBindings.METADATA)
private metadata: AuthenticationMetadata,
@inject(JWTAuthenticationBindings.STRATEGY)
private jwt_strategy: JWTStrategy,
) {}
value(): ValueOrPromise<JWTStrategy | undefined> {
if (!this.metadata) {
Expand All @@ -24,7 +27,7 @@ export class StrategyResolverProvider
const name = this.metadata.strategy;
// This should be extensible
if (name === 'jwt') {
return new JWTStrategy();
return this.jwt_strategy;
} else {
throw new Error(`The strategy ${name} is not available.`);
}
Expand Down
106 changes: 106 additions & 0 deletions src/services/JWT.authentication.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Copyright IBM Corp. 2018, 2019. All Rights Reserved.
// Node module: @loopback4-example-shopping
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import * as _ from 'lodash';
import {Credentials, UserRepository} from '../repositories/user.repository';
import {toJSON} from '@loopback/testlab';
import {promisify} from 'util';
import * as isemail from 'isemail';
import {HttpErrors} from '@loopback/rest';
import {UserProfile} from '@loopback/authentication';
import {compare} from 'bcryptjs';
import {repository} from '@loopback/repository';
import {inject} from '@loopback/core';
import {JWTAuthenticationBindings} from '../keys';
const jwt = require('jsonwebtoken');
const signAsync = promisify(jwt.sign);
const verifyAsync = promisify(jwt.verify);

/**
* Constant for JWT secret string
*/
export const JWT_SECRET = 'jwtsecret';

/**
* A JWT authentication service that could be reused by
* different clients. Usually it can be injected in the
* controller constructor.
* It provides services that handle the logics between the controller layer
* and the repository layer.
*/
export class JWTAuthenticationService {
constructor(
@repository(UserRepository) public userRepository: UserRepository,
@inject(JWTAuthenticationBindings.SECRET) public jwt_secret: string,
) {}

/**
* A function that retrieves the user with given credentials. Generates
* JWT access token using user profile as payload if user found.
*
* Usually a request's corresponding controller function filters the credential
* fields and invokes this function.
*
* @param credentials The user credentials including email and password.
*/
async getAccessTokenForUser(credentials: Credentials): Promise<string> {
const foundUser = await this.userRepository.findOne({
where: {email: credentials.email},
});
if (!foundUser) {
throw new HttpErrors['NotFound'](
`User with email ${credentials.email} not found.`,
);
}
const passwordMatched = await compare(
credentials.password,
foundUser.password,
);
if (!passwordMatched) {
throw new HttpErrors.Unauthorized('The credentials are not correct.');
}

const currentUser = _.pick(toJSON(foundUser), ['id', 'email', 'firstName']);
// Generate user token using JWT
const token = await signAsync(currentUser, this.jwt_secret, {
expiresIn: 300,
});

return token;
}

/**
* Decodes the user's information from a valid JWT access token.
* Then generate a `UserProfile` instance as the returned user.
*
* @param token A JWT access token.
*/
async decodeAccessToken(token: string): Promise<UserProfile> {
const decoded = await verifyAsync(token, this.jwt_secret);
let user = _.pick(decoded, ['id', 'email', 'firstName']);
(user as UserProfile).name = user.firstName;
delete user.firstName;
return user;
}
}

/**
* To be removed in story
* https://github.com/strongloop/loopback4-example-shopping/issues/39
* @param credentials
*/
export function validateCredentials(credentials: Credentials) {
// Validate Email
if (!isemail.validate(credentials.email)) {
throw new HttpErrors.UnprocessableEntity('invalid email');
}

// Validate Password Length
if (credentials.password.length < 8) {
throw new HttpErrors.UnprocessableEntity(
'password must be minimum 8 characters',
);
}
}
Copy link
Contributor

@raymondfeng raymondfeng Jan 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessary to have JWTAuthenticationServiceProvider as JWTAuthenticationService class can receive injections to create instances. You can simply do the following:

export class JWTAuthenticationService {
  constructor(
    @repository(UserRepository) public userRepository: UserRepository,
    @inject('JWT.authentication.secret') protected jwt_secret: string,
  ) {}
}

And

this.bind(JWTAuthenticationBindings.SERVICE).toClass(JWTAuthenticationService);

19 changes: 19 additions & 0 deletions src/services/hash.password.bcryptjs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {genSalt, hash} from 'bcryptjs';

/**
* Service HashPassword using module 'bcryptjs'.
* It takes in a plain password, generates a salt with given
* round and returns the hashed password as a string
*/
export type HashPassword = (
password: string,
rounds: number,
Copy link
Member

@bajtos bajtos Feb 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, consumers of HashPassword should not need to understand specifics of the hashing function used, e.g. number of rounds. Typically, we want to use a different number of rounds when running in test vs. in production. Consumers of HashPassword do not have enough information to be able to pick the right number of rounds.

I am proposing the following API:

export type HashPassword = (password: string) => Promise<string>;

And the following usage:

// in production
app.bind(OtherServicesBindings.HASH_PASSWORD)
  .to(pwd => hasPassword(pwd, 10));

// in test
app.bind(OtherServicesBindings.HASH_PASSWORD)
  .to(pwd => hasPassword(pwd, 4));

Having wrote that, I think we should go one step further. As I see it, the function for computing password hash is tightly coupled with the function for comparing user-provided password with the stored hash, and therefore both functions should be part of the same "service".

interface PasswordHasher {
  hash(password: string): Promise<string>;
  compare(password: string): Promise<boolean>;
}

class BcryptHasher implements PasswordHasher {
  constructor(
    @inject('BcryptHasher.rounds')
    private readonly rounds: number
  ) {}
  // ...
}

app.bind('services.PasswordHasher').toClass(BcryptHasher);

// usage in production
app.bind('BcryptHasher.rounds').to(10);
// usage in tests
app.bind('BcryptHasher.rounds').to(4);

Later in the future we should leverage loopbackio/loopback-next#2259 to control this configuration option.

Feel free to leave this second part (service for hash+compare) for a follow-up pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bajtos

IMO, consumers of HashPassword should not need to understand specifics of the hashing function used, e.g. number of rounds.

Having wrote that, I think we should go one step further. As I see it, the function for computing password hash is tightly coupled with the function for comparing user-provided password with the stored hash, and therefore both functions should be part of the same "service".

Both good points 👍 Do you mind if I address both of them in the next PR as a refactor specific for password hashing? (within the same story #40)

I would like to have this PR focus on the JWT related services first. Then improve the password hash in a second one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough 👍

) => Promise<string>;
// bind function to `services.bcryptjs.HashPassword`
export async function hashPassword(
password: string,
rounds: number,
): Promise<string> {
const salt = await genSalt(rounds);
return await hash(password, salt);
}
81 changes: 0 additions & 81 deletions src/utils/user.authentication.ts

This file was deleted.

Loading