Skip to content

Commit

Permalink
Merge pull request #235 from glutengo/feature/issue-234
Browse files Browse the repository at this point in the history
Use hash instead of encryption for storing passwords refs #234
  • Loading branch information
Angelo Manganiello committed Jun 7, 2021
2 parents 139a71b + 7a841b0 commit 24d3c63
Show file tree
Hide file tree
Showing 16 changed files with 67 additions and 36 deletions.
1 change: 1 addition & 0 deletions generators/server/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const serverFiles = {
'src/security/guards/roles.guard.ts',
'src/security/guards/auth.guard.ts',
'src/security/role-type.ts',
'src/security/password-util.ts',
'src/security/decorators/auth-user.decorator.ts',
'src/security/decorators/roles.decorator.ts',
'src/security/index.ts',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('Account', () => {
await app.init();
service = moduleFixture.get<UserService>(UserService);
authService = moduleFixture.get<AuthService>(AuthService);
userAuthenticated = await service.save(testUserAuthenticated);
userAuthenticated = await service.save(testUserAuthenticated, true);
});

it('/POST register new user', async () => {
Expand Down
8 changes: 5 additions & 3 deletions generators/server/templates/server/e2e/user.e2e-spec.ts.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,12 @@ describe('User', () => {

it('/PUT update user', async () => {
testUserDTO.login = 'TestUserUpdate';
const savedUser: UserDTO = await service.save(testUserDTO);
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { password: savedPassword, ...savedUser } = await service.save(testUserDTO);
savedUser.firstName = 'Updated Name';

const updatedUser: UserDTO = (
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { password: updatedPassword, ...updatedUser } = (
await request(app.getHttpServer())
.put('/api/admin/users')
.send(savedUser)
Expand All @@ -80,7 +82,7 @@ describe('User', () => {
expect(updatedUser.firstName).toEqual(savedUser.firstName);
<%_ } _%>

await service.delete(savedUser);
await service.delete(savedUser as UserDTO);
});

it('/GET user with a login name', async () => {
Expand Down
7 changes: 6 additions & 1 deletion generators/server/templates/server/package.json.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
<%_ } _%>
"@nestjs/swagger": "3.1.0",
"@nestjs/typeorm": "7.1.4",
<%_ if (authenticationType === 'jwt') { _%>
"bcrypt": "5.0.1",
<%_ } _%>
"class-transformer": "0.3.1",
"class-validator": "0.13.1",
"cloud-config-client": "1.4.2",
Expand Down Expand Up @@ -79,6 +82,7 @@
"typeorm-encrypted": "0.5.6"
},
"devDependencies": {
"@types/bcrypt": "5.0.0",
"@nestjs/testing": "7.5.1",
"@types/express": "4.17.1",
"@types/express-serve-static-core": "4.17.3",
Expand Down Expand Up @@ -123,6 +127,7 @@
"^.+\\.(t|j)s$": "ts-jest"
},
"coverageDirectory": "coverage",
"testEnvironment": "node"
"testEnvironment": "node",
"setupFiles": ["./e2e/setup.test.js"]
}
}
5 changes: 2 additions & 3 deletions generators/server/templates/server/src/config.ts.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export class Config {
'jhipster.security.authentication.jwt.base64-secret' = '';
'jhipster.security.authentication.jwt.token-validity-in-seconds' = 86400;
'jhipster.security.authentication.jwt.token-validity-in-seconds-for-remember-me' = 2592000;
'jhipster.security.authentication.jwt.hash-salt-or-rounds' = 10;
<%_ } else if (authenticationType === 'oauth2') { _%>
'jhipster.security.session.secret' = '';
'jhipster.security.oauth2.client.provider.oidc.issuer-uri' = '';
Expand Down Expand Up @@ -49,9 +50,7 @@ export class Config {
'cloud.config.uri' = 'http://admin:${jhipster.registry.password}@localhost:8761/config';
'cloud.config.name' = '<%= baseName %>';
'cloud.config.profile' = 'prod';
'loud.config.label' = 'master';
'crypto.key' = '3772c1cdbd27c225735d116d1e4c5421a3aec26c919cc7ab457f21a4d16a1821';
'crypto.iv' = '54f3ad979d9262d3a2dd4489531daf34';
'cloud.config.label' = 'master';

constructor(properties) {
this.addAll(properties);
Expand Down
13 changes: 2 additions & 11 deletions generators/server/templates/server/src/domain/user.entity.ts.ejs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { Authority } from './authority.entity';
import { Entity, Column <%_ if (databaseType !== 'mongodb') { _%>, ManyToMany, JoinTable <%_ } _%> } from 'typeorm';
import { Entity, Column<%_ if (databaseType !== 'mongodb') { _%>, ManyToMany, JoinTable <%_ } _%> } from 'typeorm';
import { BaseEntity } from './base/base.entity';
import { config } from '../config';
import { EncryptionTransformer } from "typeorm-encrypted";

@Entity('nhi_user')
export class User extends BaseEntity {
Expand All @@ -29,14 +27,7 @@ export class User extends BaseEntity {
<%_ } _%>

@Column({
type: "varchar",
transformer: new EncryptionTransformer({
key: config.get('crypto.key'),
algorithm: 'aes-256-cbc',
ivLength: 16,
iv: config.get('crypto.iv')
}),
select: false
type: "varchar"
})
password: string;
@Column({ nullable: true })
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { MigrationInterface, QueryRunner, getRepository } from 'typeorm';
<%_ if (authenticationType !== 'oauth2') { _%>
import { User } from '../domain/user.entity';
import { transformPassword } from '../security';
<%_ } _%>
import { Authority } from '../domain/authority.entity';

Expand Down Expand Up @@ -83,8 +84,9 @@ export class SeedUsersRoles1570200490072 implements MigrationInterface {
this.user3.authorities= [adminRole, userRole];
this.user4.authorities= [userRole];
await userRepository.save([this.user1, this.user2, this.user3, this.user4]);
await Promise.all([this.user1, this.user2, this.user3, this.user4].map(u => transformPassword(u)));
await userRepository.save([this.user1, this.user2, this.user3, this.user4]);
<%_ } _%>
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export * from './guards/roles.guard';
export * from './decorators/roles.decorator';
export * from './decorators/auth-user.decorator';
export * from './role-type';
export * from './password-util';
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<%_ if (authenticationType === 'jwt') { _%>
import * as bcrypt from 'bcrypt';
import { config } from '../config';
<%_ } _%>

export async function transformPassword(user: { password?: string }): Promise<void> {
<%_ if (authenticationType === 'jwt') { _%>
if (user.password) {
user.password = await bcrypt.hash(
user.password,
config.get('jhipster.security.authentication.jwt.hash-salt-or-rounds'),
);
}
<%_ } _%>
return Promise.resolve();
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { InjectRepository } from '@nestjs/typeorm';
import { JwtService } from '@nestjs/jwt';
import { UserLoginDTO } from '../service/dto/user-login.dto';
import { Payload } from '../security/payload.interface';
import * as bcrypt from 'bcrypt';
<%_ } _%>
import { AuthorityRepository } from '../repository/authority.repository';
import { UserService } from '../service/user.service';
Expand Down Expand Up @@ -31,8 +32,9 @@ export class AuthService {
const loginUserName = userLogin.username;
const loginPassword = userLogin.password;
const userFind = await this.userService.findByfields({ where: { login: loginUserName, password: loginPassword } });
if (!userFind) {
const userFind = await this.userService.findByfields({ where: { login: loginUserName } });
const validPassword = !!userFind && await bcrypt.compare(loginPassword, userFind.password);
if (!userFind || !validPassword) {
throw new HttpException('Invalid login name or password!', HttpStatus.BAD_REQUEST);
}
Expand Down Expand Up @@ -73,15 +75,16 @@ export class AuthService {
}
async changePassword(userLogin: string, currentClearTextPassword: string, newPassword: string): Promise<void> {
const userFind: UserDTO = await this.userService.findByfields({ where: { login: userLogin }, select: [ 'id', 'password' ] });
const userFind: UserDTO = await this.userService.findByfields({ where: { login: userLogin } });
if (!userFind) {
throw new HttpException('Invalid login name!', HttpStatus.BAD_REQUEST);
}
if (userFind.password !== currentClearTextPassword ) {
if (!(await bcrypt.compare(currentClearTextPassword, userFind.password))) {
throw new HttpException('Invalid password!', HttpStatus.BAD_REQUEST);
}
userFind.password = newPassword;
await this.userService.save(userFind);
await this.userService.save(userFind, true);
return;
}
Expand All @@ -95,7 +98,7 @@ export class AuthService {
throw new HttpException('Email is already in use!', HttpStatus.BAD_REQUEST);
}
newUser.authorities = ['ROLE_USER'];
const user: UserDTO = await this.userService.save(newUser);
const user: UserDTO = await this.userService.save(newUser, true);
return user;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
<%_ if (databaseType === 'mongodb') { _%>
import { Transform } from 'class-transformer';
<%_ } _%>

/**
* A DTO base object.
*/
export class BaseDTO {

<%_ if (databaseType === 'mongodb') { _%>@Transform(id => id?.toHexString ? id?.toHexString() : id, {toPlainOnly: true})<%_ } _%>
id?:<%_ if (getPkType(databaseType) === 'Long') { _%>number<%_ } else { _%>string<%_ } _%>;
createdBy?: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { ApiModelProperty } from '@nestjs/swagger';
import { IsString, IsEmail } from 'class-validator';
import { BaseDTO } from './base.dto';
import { Exclude } from 'class-transformer';

/**
* An User DTO object.
*/
Expand Down Expand Up @@ -33,6 +35,7 @@ export class UserDTO extends BaseDTO {
})
authorities?: any[];

@Exclude()
@ApiModelProperty({ example: 'myuser', description: 'User password' })
password: string;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { UserDTO } from './dto/user.dto';
import { UserMapper } from './mapper/user.mapper';
import { UserRepository } from '../repository/user.repository';
import { FindManyOptions, FindOneOptions } from 'typeorm';
import { transformPassword } from '../security';

@Injectable()
export class UserService {
Expand Down Expand Up @@ -41,8 +42,11 @@ export class UserService {
return resultList;
}

async save(userDTO: UserDTO): Promise<UserDTO | undefined> {
async save(userDTO: UserDTO, updatePassword = false): Promise<UserDTO | undefined> {
const user = this.convertInAuthorities(UserMapper.fromDTOtoEntity(userDTO));
if (updatePassword) {
await transformPassword(user);
}
const result = await this.userRepository.save(user);
return UserMapper.fromEntityToDTO(this.flatAuthorities(result));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable @typescript-eslint/no-unused-vars */
import { <%_ if (authenticationType === 'jwt') { _%> Body, Param, Post, Res, UseGuards, <%_ } _%> Controller, Get, Logger, Req, UseInterceptors } from '@nestjs/common';
import { <%_ if (authenticationType === 'jwt') { _%> Body, Param, Post, Res, UseGuards, <%_ } _%> Controller, Get, Logger, Req, UseInterceptors, ClassSerializerInterceptor, InternalServerErrorException } from '@nestjs/common';
<%_ if (authenticationType === 'jwt') { _%>
import { Response, Request } from 'express';
import { AuthGuard, Roles, RoleType, RolesGuard } from '../../security';
Expand All @@ -11,7 +11,7 @@ import {<%_ if (authenticationType === 'jwt') { _%> ApiBearerAuth, <%_ } _%> Api
import { AuthService } from '../../service/auth.service';

@Controller('api')
@UseInterceptors(LoggingInterceptor)
@UseInterceptors(LoggingInterceptor, ClassSerializerInterceptor)
@ApiUseTags('account-resource')
export class AccountController {
logger = new Logger('AccountController');
Expand All @@ -26,7 +26,7 @@ export class AccountController {
description: 'Registered user',
type: UserDTO,
})
async registerAccount(@Req() req: Request, @Body() userDTO: UserDTO): Promise <any> {
async registerAccount(@Req() req: Request, @Body() userDTO: UserDTO & { password: string }): Promise <any> {
return await this.authService.registerNewUser(userDTO);
}
Expand All @@ -40,7 +40,7 @@ export class AccountController {
description: 'activated',
})
activateAccount(@Param() key: string, @Res() res: Response): any{
return res.sendStatus(500);
throw new InternalServerErrorException();
}
@Get('/authenticate')
Expand Down Expand Up @@ -116,7 +116,7 @@ export class AccountController {
type: 'string',
})
requestPasswordReset(@Req() req: Request, @Body() email: string, @Res() res: Response): any {
return res.sendStatus(500);
throw new InternalServerErrorException();
}
@Post('/account/reset-password/finish')
Expand All @@ -129,7 +129,7 @@ export class AccountController {
type: 'string',
})
finishPasswordReset(@Req() req: Request, @Body() keyAndPassword: string, @Res() res: Response): any {
return res.sendStatus(500);
throw new InternalServerErrorException();
}
<%_ } else if (authenticationType === 'oauth2') { _%>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Body, Controller, Delete, Get, Logger, Param, Post, Put, UseGuards, Req, UseInterceptors } from '@nestjs/common';
import { Body, Controller, Delete, Get, Logger, Param, Post, Put, UseGuards, Req, UseInterceptors, ClassSerializerInterceptor } from '@nestjs/common';
import { Request } from 'express';
import { AuthGuard, Roles, RolesGuard, RoleType } from '../../security';
import { PageRequest, Page } from '../../domain/base/pagination.entity';
Expand All @@ -10,7 +10,7 @@ import { UserService } from '../../service/user.service';

@Controller('api/admin/users')
@UseGuards(AuthGuard, RolesGuard)
@UseInterceptors(LoggingInterceptor)
@UseInterceptors(LoggingInterceptor, ClassSerializerInterceptor)
<%_ if (authenticationType === 'jwt') { _%>
@ApiBearerAuth()
<%_ } else if (authenticationType === 'oauth2') { _%>
Expand Down
2 changes: 1 addition & 1 deletion test-integration/07-run-generated-app-sample.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ launchCurlOrProtractor() {
do
result=0
echo "***${GREEN}run protractor e2e test in client for : "$1
node node_modules/webdriver-manager/bin/webdriver-manager update --gecko false && JHI_E2E_HEADLESS=true npm run e2e
node node_modules/webdriver-manager/bin/webdriver-manager update --gecko false --versions.chrome 90.0.4430.24 && JHI_E2E_HEADLESS=true npm run e2e
result=$?
[ $result -eq 0 ] && break
retryCount=$((retryCount+1))
Expand Down

0 comments on commit 24d3c63

Please sign in to comment.