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

Use hash instead of encryption for storing passwords refs #234 #235

Merged
4 commits merged into from
Jun 7, 2021
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
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