From f7321fb90101f7818fc37c17da31a95790104620 Mon Sep 17 00:00:00 2001 From: Maximilian Kreuzkam Date: Fri, 12 Jan 2024 13:27:40 +0100 Subject: [PATCH] Check usage of ?? ''. --- .../account/controller/mapper/account-response.mapper.ts | 2 +- .../modules/account/repo/account.repo.integration.spec.ts | 5 +---- apps/server/src/modules/account/review-comments.md | 3 +-- .../src/modules/account/services/account-db.service.spec.ts | 2 +- .../account/services/account.service.integration.spec.ts | 2 +- .../modules/account/services/account.validation.service.ts | 2 +- apps/server/src/modules/account/uc/account.uc.ts | 2 +- .../src/modules/authentication/strategy/ldap.strategy.ts | 4 ++-- .../src/modules/authentication/strategy/local.strategy.ts | 6 +++--- .../src/modules/authentication/strategy/oauth2.strategy.ts | 2 +- apps/server/src/modules/user/service/user.service.ts | 6 +----- 11 files changed, 14 insertions(+), 22 deletions(-) diff --git a/apps/server/src/modules/account/controller/mapper/account-response.mapper.ts b/apps/server/src/modules/account/controller/mapper/account-response.mapper.ts index 000847506de..bac41a1d698 100644 --- a/apps/server/src/modules/account/controller/mapper/account-response.mapper.ts +++ b/apps/server/src/modules/account/controller/mapper/account-response.mapper.ts @@ -7,7 +7,7 @@ export class AccountResponseMapper { id: resolvedAccount.id as string, userId: resolvedAccount.userId, activated: resolvedAccount.activated, - username: resolvedAccount.username ?? '', + username: resolvedAccount.username, updatedAt: resolvedAccount.updatedAt, }); } diff --git a/apps/server/src/modules/account/repo/account.repo.integration.spec.ts b/apps/server/src/modules/account/repo/account.repo.integration.spec.ts index 5c0efb790d3..47f50d53248 100644 --- a/apps/server/src/modules/account/repo/account.repo.integration.spec.ts +++ b/apps/server/src/modules/account/repo/account.repo.integration.spec.ts @@ -60,10 +60,7 @@ describe('account repo', () => { it('should return account', async () => { const accountToFind = await setup(); - const account = await repo.findByUsernameAndSystemId( - accountToFind.username ?? '', - accountToFind.systemId ?? '' - ); + const account = await repo.findByUsernameAndSystemId(accountToFind.username, accountToFind.systemId ?? ''); expect(account?.username).toEqual(accountToFind.username); }); }); diff --git a/apps/server/src/modules/account/review-comments.md b/apps/server/src/modules/account/review-comments.md index ff80a63946f..0e5a0e07af8 100644 --- a/apps/server/src/modules/account/review-comments.md +++ b/apps/server/src/modules/account/review-comments.md @@ -1,7 +1,6 @@ # Review Comments 14.7.23 -- check ?? '' -- check names (fix dto naming) +- check TODOs - Remove lookup service, move to idm and db - Move Account Entity diff --git a/apps/server/src/modules/account/services/account-db.service.spec.ts b/apps/server/src/modules/account/services/account-db.service.spec.ts index d35844b6400..f05d0f7800b 100644 --- a/apps/server/src/modules/account/services/account-db.service.spec.ts +++ b/apps/server/src/modules/account/services/account-db.service.spec.ts @@ -188,7 +188,7 @@ describe('AccountDbService', () => { const { mockAccountWithSystemId } = setup(); const resultAccount = await accountService.findByUsernameAndSystemId( mockAccountWithSystemId.username, - 'nonExistentSystemId' ?? '' + 'nonExistentSystemId' ); expect(resultAccount).toBeNull(); }); diff --git a/apps/server/src/modules/account/services/account.service.integration.spec.ts b/apps/server/src/modules/account/services/account.service.integration.spec.ts index d47bceec64e..ec35a425738 100644 --- a/apps/server/src/modules/account/services/account.service.integration.spec.ts +++ b/apps/server/src/modules/account/services/account.service.integration.spec.ts @@ -165,7 +165,7 @@ describe('AccountService Integration', () => { it('should create a new account', async () => { if (!isIdmReachable) return; const account = await accountService.save(testAccount); - await compareDbAccount(account.id ?? '', account); + await compareDbAccount(account.id, account); await compareIdmAccount(account.idmReferenceId ?? '', account); }); }); diff --git a/apps/server/src/modules/account/services/account.validation.service.ts b/apps/server/src/modules/account/services/account.validation.service.ts index a6f22280b9a..f1d04f91827 100644 --- a/apps/server/src/modules/account/services/account.validation.service.ts +++ b/apps/server/src/modules/account/services/account.validation.service.ts @@ -24,7 +24,7 @@ export class AccountValidationService { filteredAccounts.length > 1 || // paranoid 'toString': legacy code may call userId or accountId as ObjectID (foundUsers.length === 1 && foundUsers[0].id.toString() !== userId?.toString()) || - (filteredAccounts.length === 1 && (filteredAccounts[0].id ?? '').toString() !== accountId?.toString()) + (filteredAccounts.length === 1 && filteredAccounts[0].id.toString() !== accountId?.toString()) ); } diff --git a/apps/server/src/modules/account/uc/account.uc.ts b/apps/server/src/modules/account/uc/account.uc.ts index 96d8063ee90..f0987879772 100644 --- a/apps/server/src/modules/account/uc/account.uc.ts +++ b/apps/server/src/modules/account/uc/account.uc.ts @@ -199,7 +199,7 @@ export class AccountUc { throw new ForbiddenOperationError('Current user is not authorized to delete an account.'); } const account: Account = await this.accountService.findById(params.id); - await this.accountService.delete(account.id ?? ''); + await this.accountService.delete(account.id); return AccountUcMapper.mapToResolvedAccountDto(account); } diff --git a/apps/server/src/modules/authentication/strategy/ldap.strategy.ts b/apps/server/src/modules/authentication/strategy/ldap.strategy.ts index 04c24266715..3513e29d50e 100644 --- a/apps/server/src/modules/authentication/strategy/ldap.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/ldap.strategy.ts @@ -49,7 +49,7 @@ export class LdapStrategy extends PassportStrategy(Strategy, 'ldap') { await this.checkCredentials(account, system, ldapDn, password); - const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(account.id ?? '', user, true, systemId); + const currentUser: ICurrentUser = CurrentUserMapper.userToICurrentUser(account.id, user, true, systemId); return currentUser; } @@ -83,7 +83,7 @@ export class LdapStrategy extends PassportStrategy(Strategy, 'ldap') { await this.ldapService.checkLdapCredentials(system, ldapDn, password); } catch (error) { if (error instanceof UnauthorizedException) { - await this.authenticationService.updateLastTriedFailedLogin(account.id ?? ''); + await this.authenticationService.updateLastTriedFailedLogin(account.id); } throw error; } diff --git a/apps/server/src/modules/authentication/strategy/local.strategy.ts b/apps/server/src/modules/authentication/strategy/local.strategy.ts index 53be846af35..2572df205db 100644 --- a/apps/server/src/modules/authentication/strategy/local.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/local.strategy.ts @@ -36,10 +36,10 @@ export class LocalStrategy extends PassportStrategy(Strategy) { const accountUserId = GuardAgainst.nullOrUndefined( account.userId, - new Error(`login failing, because account ${account.id ?? ''} has no userId`) + new Error(`login failing, because account ${account.id} has no userId`) ); const user = await this.userRepo.findById(accountUserId, true); - const currentUser = CurrentUserMapper.userToICurrentUser(account.id ?? '', user, false); + const currentUser = CurrentUserMapper.userToICurrentUser(account.id, user, false); return currentUser; } @@ -58,7 +58,7 @@ export class LocalStrategy extends PassportStrategy(Strategy) { ): Promise { this.authenticationService.checkBrutForce(account); if (!(await bcrypt.compare(enteredPassword, savedPassword))) { - await this.authenticationService.updateLastTriedFailedLogin(account.id ?? ''); + await this.authenticationService.updateLastTriedFailedLogin(account.id); throw new UnauthorizedException(); } } diff --git a/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts b/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts index f6c9d26017e..816e7533e96 100644 --- a/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts +++ b/apps/server/src/modules/authentication/strategy/oauth2.strategy.ts @@ -33,7 +33,7 @@ export class Oauth2Strategy extends PassportStrategy(Strategy, 'oauth2') { } const currentUser: OauthCurrentUser = CurrentUserMapper.mapToOauthCurrentUser( - account.id ?? '', + account.id, user, systemId, tokenDto.idToken diff --git a/apps/server/src/modules/user/service/user.service.ts b/apps/server/src/modules/user/service/user.service.ts index d52326e0714..73fce105268 100644 --- a/apps/server/src/modules/user/service/user.service.ts +++ b/apps/server/src/modules/user/service/user.service.ts @@ -49,11 +49,7 @@ export class UserService { const user: UserDO = await this.findById(userId); const account: Account = await this.accountService.findByUserIdOrFail(userId); - const resolvedUser: OauthCurrentUser = CurrentUserMapper.mapToOauthCurrentUser( - account.id ?? '', - user, - account.systemId - ); + const resolvedUser: OauthCurrentUser = CurrentUserMapper.mapToOauthCurrentUser(account.id, user, account.systemId); return resolvedUser; }