EW-167 Refactor infrastructure (#3685)

Various replacements of directs access to account collection from feathers stack. Major changes include password recovery, password recovery link generation, JWT support.

apps/server/src/modules/account/services/account.service.spec.ts

@@ -1,6 +1,7 @@
 import { MikroORM } from '@mikro-orm/core';
 import { ObjectId } from '@mikro-orm/mongodb';
 import { Test, TestingModule } from '@nestjs/testing';
+import bcrypt from 'bcryptjs';
 import { EntityNotFoundError } from '@shared/common';
 import { Account, EntityId, Role, School, User, RoleName, Permission } from '@shared/domain';
 import { AccountRepo } from '@shared/repo';
@@ -404,6 +405,20 @@ describe('AccountService', () => {
 		});
 	});
 
+	describe('updatePassword', () => {
+		it('should update password', async () => {
+			const newPassword = 'newPassword';
+			const ret = await accountService.updatePassword(mockTeacherAccount.id, newPassword);
+
+			expect(ret).toBeDefined();
+			if (ret.password) {
+				await expect(bcrypt.compare(newPassword, ret.password)).resolves.toBe(true);
+			} else {
+				fail('return password is undefined');
+			}
+		});
+	});
+
 	describe('delete', () => {
 		it('should delete account via repo', async () => {
 			await accountService.delete(mockTeacherAccount.id);

apps/server/src/modules/account/services/account.service.ts

@@ -89,6 +89,14 @@ export class AccountService {
 		return AccountEntityToDtoMapper.mapToDto(account);
 	}
 
+	async updatePassword(accountId: EntityId, password: string): Promise<AccountDto> {
+		const account = await this.accountRepo.findById(accountId);
+		account.password = await this.encryptPassword(password);
+		account.updatedAt = new Date();
+		await this.accountRepo.save(account);
+		return AccountEntityToDtoMapper.mapToDto(account);
+	}
+
 	delete(id: EntityId): Promise<void> {
 		return this.accountRepo.deleteById(id);
 	}

apps/server/src/shared/domain/entity/system.entity.ts

@@ -102,6 +102,6 @@ export class System extends BaseEntityWithTimestamps {
 	@Property({ nullable: true })
 	ldapConfig?: Record<string, unknown>;
 
-	@Property()
+	@Property({ nullable: true })
 	provisioningUrl?: string;
 }

src/hooks/index.js

@@ -1,13 +1,13 @@
 /* eslint-disable no-param-reassign */
 
-const { Forbidden, GeneralError, NotFound, BadRequest, TypeError } = require('../errors');
 const { authenticate } = require('@feathersjs/authentication');
 
 const { v4: uuidv4 } = require('uuid');
 
 const { Configuration } = require('@hpi-schul-cloud/commons');
 const _ = require('lodash');
 const mongoose = require('mongoose');
+const { Forbidden, GeneralError, NotFound, BadRequest, TypeError } = require('../errors');
 const { equal: equalIds } = require('../helper/compare').ObjectId;
 
 const logger = require('../logger');
@@ -201,11 +201,13 @@ exports.hasPermissionNoHook = (context, userId, permissionName) => {
 
 exports.hasRoleNoHook = (context, userId, roleName, account = false) => {
 	const userService = context.app.service('/users/');
-	const accountService = context.app.service('/accounts/');
+	const accountService = context.app.service('nest-account-service');
+
 	// What is the account flag for?
 	// if statement can be remove because it has no effect?!
 	if (account) {
-		return accountService.get(userId).then((_account) =>
+		return accountService.findByUserId(userId).then((_account) =>
+			// eslint-disable-next-line promise/no-nesting
 			userService.find({ query: { _id: _account.userId || '', $populate: 'roles' } }).then((user) => {
 				user.data[0].roles = Array.from(user.data[0].roles);
 

src/services/account/hooks/index.js

@@ -272,7 +272,7 @@ const restrictToUsersSchool = async (context) => {
 	const userService = context.app.service('users');
 	const { schoolId: usersSchoolId } = await userService.get(context.params.account.userId);
 
-	const targetAccount = await context.app.service('accountModel').get(context.id);
+	const targetAccount = await context.app.service('nest-account-service').findById(context.id);
 	const { schoolId: targetSchoolId } = await userService.get(targetAccount.userId);
 	if (!equalIds(usersSchoolId, targetSchoolId)) {
 		throw new NotFound('this account doesnt exist');
@@ -281,8 +281,16 @@ const restrictToUsersSchool = async (context) => {
 };
 
 const validateUserName = async (context) => {
-	const accountService = context.app.service('accounts');
-	const { systemId } = context.method === 'create' ? context.data : await accountService.get(context.id);
+	let account;
+
+	if (context.method === 'create') {
+		account = context.data;
+	} else {
+		account = await context.app.service('nest-account-service').findById(context.id);
+	}
+
+	const { systemId } = account;
+
 	if (systemId) {
 		return context;
 	}

src/services/account/services/SupportJWTService.js

@@ -7,7 +7,6 @@ const { ObjectId } = require('mongoose').Types;
 const { BadRequest } = require('../../../errors');
 const { hasPermission } = require('../../../hooks/index');
 const { authenticationSecret, audience: audienceName } = require('../../authentication/logic');
-const accountModel = require('../model');
 const logger = require('../../../logger');
 
 const { addTokenToWhitelistWithIdAndJti } = require('../../authentication/logic/whitelist');
@@ -133,8 +132,8 @@ class SupportJWTService {
 			const requestedUserId = userId.toString();
 			const currentUserId = params.account.userId.toString();
 
-			const account = await accountModel.findOne({ userId }).select(['_id', 'systemId']).lean().exec();
-			if (!account && !account._id) {
+			const account = await this.app.service('nest-account-service').findByUserId(userId);
+			if (!account && !account.id) {
 				throw new Error(`Account for user with the id ${userId} does not exist.`);
 			}
 

src/services/activation/services/eMailAddress/index.js

@@ -101,11 +101,7 @@ class EMailAddressActivationService {
 
 	async update(id, data, params) {
 		const { entry, user } = data;
-		const [account] = await this.app.service('/accounts').find({
-			query: {
-				userId: user._id,
-			},
-		});
+		const account = await this.app.service('nest-account-service').findByUserId(user._id);
 		if (!account) throw new Forbidden(customErrorMessages.NOT_AUTHORIZED);
 
 		const email = entry.quarantinedObject;
@@ -119,7 +115,7 @@ class EMailAddressActivationService {
 
 			// update user and account
 			await this.app.service('users').patch(account.userId, { email });
-			await this.app.service('/accounts').patch(account._id, { username: email });
+			await this.app.service('nest-account-service').updateUsername(account._id, email);
 			// set activation link as consumed
 			await setEntryState(this, entry._id, STATE.SUCCESS);
 		} catch (error) {

src/services/authentication/index.js

@@ -10,13 +10,14 @@ const { authConfig } = require('./configuration');
 class SCAuthenticationService extends AuthenticationService {
 	async getUserData(user, account) {
 		return {
-			accountId: account._id,
+			accountId: account._id ? account._id : account.id,
 			systemId: account.systemId,
 			userId: user._id,
 			schoolId: user.schoolId,
 			roles: user.roles,
 		};
 	}
+
 	async getPayload(authResult, params) {
 		let payload = await super.getPayload(authResult, params);
 		const user = await this.app.service('usersModel').get(authResult.account.userId);

src/services/link/index.js

@@ -78,7 +78,7 @@ module.exports = function setup() {
 					if (user && user.importHash) linkData.hash = user.importHash;
 					else {
 						if (user) {
-							const account = ((await app.service('accounts').find({ query: { userId: user._id } })) || {})[0];
+							const account = await app.service('nest-account-service').findByUserId(user._id);
 							if (account && account.userId) {
 								throw new BadRequest(`User already has an account.`);
 							}

src/services/passwordRecovery/index.js

@@ -2,7 +2,6 @@ const { static: staticContent } = require('@feathersjs/express');
 const path = require('path');
 
 const passwordRecovery = require('./model');
-const AccountModel = require('../account/model');
 const { ChangePasswordService, hooks: changePasswordServiceHooks } = require('./services/ChangePasswordService');
 const {
 	GenerateRecoveryPasswordTokenService,
@@ -13,7 +12,7 @@ module.exports = function setup() {
 	const app = this;
 
 	app.use('/passwordRecovery/api', staticContent(path.join(__dirname, '/docs/openapi.yaml')));
-	app.use('/passwordRecovery/reset', new ChangePasswordService(passwordRecovery, AccountModel));
+	app.use('/passwordRecovery/reset', new ChangePasswordService(passwordRecovery, app));
 	app.use('/passwordRecovery', new GenerateRecoveryPasswordTokenService(passwordRecovery));
 
 	const passwordRecoveryService = app.service('/passwordRecovery');

src/services/passwordRecovery/services/ChangePasswordService.js

@@ -3,14 +3,14 @@ const local = require('@feathersjs/authentication-local');
 const { BadRequest, GeneralError, SilentError } = require('../../../errors');
 const logger = require('../../../logger/index');
 const globalHooks = require('../../../hooks');
-const { ObjectId } = require('../../../helper/compare');
 
 const MAX_LIVE_TIME = 6 * 60 * 60 * 1000; // 6 hours
 
 class ChangePasswordService {
-	constructor(passwordRecoveryModel, accountModel) {
+	constructor(passwordRecoveryModel, app) {
 		this.passwordRecoveryModel = passwordRecoveryModel;
-		this.accountModel = accountModel;
+		this.app = app;
+
 		this.errors = {
 			inputValidation: 'Malformed request body.',
 			expired: 'Token expired!',
@@ -39,7 +39,7 @@ class ChangePasswordService {
 		}
 		try {
 			await Promise.all([
-				this.accountModel.updateOne({ _id: pwrecover.account }, { $set: { password } }).lean().exec(),
+				this.app.service('nest-account-service').updatePassword(pwrecover.account, password),
 				this.passwordRecoveryModel
 					.updateOne({ token }, { $set: { changed: true } })
 					.lean()

src/services/passwordRecovery/services/GenerateRecoveryPasswordTokenService.js

@@ -17,7 +17,7 @@ class GenerateRecoveryPasswordTokenService {
 	}
 
 	setup(app) {
-		this.accountsService = app.service('/accounts');
+		this.accountsService = app.service('nest-account-service');
 	}
 
 	async get(id) {
@@ -29,16 +29,14 @@ class GenerateRecoveryPasswordTokenService {
 	async create(data) {
 		if (data && data.username) {
 			const { username } = data;
-			const accounts = await this.accountsService.find({
-				query: {
-					username,
-				},
-			});
+			const accountsResult = await this.accountsService.searchByUsernameExactMatch(username);
+			const { accounts, total } = accountsResult;
 
-			if (Array.isArray(accounts) && accounts.length !== 0 && accounts[0]._id) {
-				data.account = accounts[0]._id;
+			if (total !== 0 && Array.isArray(accounts) && accounts[0].id) {
+				data.account = accounts[0].id;
+				data.user = accounts[0].userId;
 				const recoveryModel = await PasswordRecoveryModel.create({
-					account: accounts[0]._id,
+					account: accounts[0].id,
 					token: randomStringAsBase64Url(24),
 				});
 				return recoveryModel;
@@ -52,29 +50,25 @@ class GenerateRecoveryPasswordTokenService {
 const sendInfo = (context) => {
 	if (context.path === 'passwordRecovery') {
 		return context.app
-			.service('/accounts')
-			.get(context.data.account, {
-				query: {
-					$populate: ['userId'],
-				},
-			})
-			.then((account) => {
+			.service('/users')
+			.get(context.data.user)
+			.then((user) => {
 				const recoveryLink = `${HOST}/pwrecovery/${context.result.token}`;
-				const mailContent = `Sehr geehrte/r ${account.userId.firstName} ${account.userId.lastName}, \n
-Bitte setzen Sie Ihr Passwort unter folgendem Link zurück:
-${recoveryLink}\n
-Bitte beachten Sie das der Link nur für 6 Stunden gültig ist. Danach müssen sie ein neuen Link anfordern.\n
-Mit Freundlichen Grüßen
-Ihr ${SC_SHORT_TITLE} Team`;
+				const mailContent = `Sehr geehrte/r ${user.firstName} ${user.lastName}, \n
+							Bitte setzen Sie Ihr Passwort unter folgendem Link zurück:
+							${recoveryLink}\n
+							Bitte beachten Sie das der Link nur für 6 Stunden gültig ist. Danach müssen sie ein neuen Link anfordern.\n
+							Mit Freundlichen Grüßen
+							Ihr ${SC_SHORT_TITLE} Team`;
 
 				globalHooks.sendEmail(context, {
 					subject: `Passwort zurücksetzen für die ${SC_SHORT_TITLE}`,
-					emails: [account.userId.email],
+					emails: [user.email],
 					content: {
 						text: mailContent,
 					},
 				});
-				logger.info(`send password recovery information to userId ${account.userId._id}`);
+				logger.info(`send password recovery information to userId ${user._id}`);
 				return context;
 			})
 			.catch((err) => {

test/services/passwordRecovery/index.test.js

@@ -6,8 +6,6 @@ const testObjects = require('../helpers/testObjects')(appPromise());
 const { setupNestServices, closeNestServices } = require('../../utils/setup.nest.services');
 const passwordRecovery = require('../../../src/services/passwordRecovery/model');
 
-const PORT = 0;
-
 describe('passwordRecovery service', () => {
 	let app;
 	let nestServices;
@@ -27,9 +25,9 @@ describe('passwordRecovery service', () => {
 
 	before(async () => {
 		app = await appPromise();
-		passwordRecoveryService = app.service('passwordRecovery');
-		server = await app.listen(0);
 		nestServices = await setupNestServices(app);
+		server = await app.listen(0);
+		passwordRecoveryService = app.service('passwordRecovery');
 		savedUser = await testObjects.createTestUser();
 		savedAccount = await testObjects.createTestAccount(newAccount, null, savedUser);
 		await passwordRecoveryService.create({ username: recoveryUsername });

test/services/teams/helper/helper.user.js

@@ -1,9 +1,8 @@
 const { ObjectId } = require('mongoose').Types;
 
 const { BadRequest } = require('../../../../src/errors');
-const rolesModel = require('../../../../src/services/role/model.js');
+const rolesModel = require('../../../../src/services/role/model');
 const { userModel } = require('../../../../src/services/user/model');
-const accountModel = require('../../../../src/services/account/model.js');
 const appPromise = require('../../../../src/app');
 
 const { TEST_PW, TEST_HASH } = require('../../../../config/globals');
@@ -57,13 +56,15 @@ const createUser = async (userId, roleName = 'student', schoolId = '5f2987e02083
 	});
 };
 
-const createAccount = (userId) =>
-	accountModel.create({
+const createAccount = async (userId) => {
+	const app = await appPromise();
+	return app.service('nest-account-service').save({
 		username: userId + AT,
 		password: TEST_HASH,
 		userId,
 		activated: true,
 	});
+};
 
 const setupUser = async (roleName, schoolId) => {
 	const userId = new ObjectId();
@@ -81,9 +82,10 @@ const setupUser = async (roleName, schoolId) => {
 const deleteUser = async (userId) => {
 	if (typeof userId === 'object' && userId.userId !== undefined) userId = userId.userId;
 
+	const app = await appPromise();
 	const email = userId + AT;
 	await userModel.deleteOne({ email }); // todo: add error handling if not exist
-	await accountModel.deleteOne({ username: email });
+	await app.service('nest-account-service').deleteByUserId(userId);
 };
 
 module.exports = {

test/services/teams/hooks/helpers.test.js

@@ -13,18 +13,22 @@ const {
 } = require('../../../../src/services/teams/hooks/helpers.js');
 
 const { teamUserModel } = require('../../../../src/services/teams/model');
+const { setupNestServices, closeNestServices } = require('../../../utils/setup.nest.services');
 
 describe('hook helpers', () => {
 	let app;
 	let server;
+	let nestServices;
 
 	before(async () => {
 		app = await appPromise();
 		server = await app.listen(0);
+		nestServices = await setupNestServices(app);
 	});
 
-	after((done) => {
-		server.close(done);
+	after(async () => {
+		await server.close();
+		await closeNestServices(nestServices);
 	});
 
 	describe('ifSuperhero', () => {