From b6693606d565bf1ccc07070e96fafbeae76486dc Mon Sep 17 00:00:00 2001
From: Jose Semedo <jose.semedo@cern.ch>
Date: Thu, 28 Apr 2022 10:58:53 +0200
Subject: [PATCH] [#148] Lowercase username and email on Login/Add channel
member/Send notification to target
---
src/models/channel.ts | 5 ++---
src/models/user.ts | 4 ++--
src/services/impl/auth/login/Login.ts | 11 +----------
src/services/impl/channels/add-member-to-channel.ts | 5 +++--
src/services/impl/notifications/send-notification.ts | 2 +-
src/services/impl/users-service-impl.ts | 9 ++-------
src/services/impl/users/get-or-create-user.ts | 5 ++---
src/services/impl/users/get-user-from-external.ts | 3 +--
8 files changed, 14 insertions(+), 30 deletions(-)
diff --git a/src/models/channel.ts b/src/models/channel.ts
index ce11786f..f3f1a28f 100644
--- a/src/models/channel.ts
+++ b/src/models/channel.ts
@@ -456,11 +456,10 @@ export class Channel extends ApiKeyObject {
}
}
}
-
return members;
}
- async isMember(user: User) {
+ async isMember(user: User): Promise<boolean> {
if (!user) return false;
if (user.username) {
return (await this.getMembers(false)).map(member => member.username).includes(user.username);
@@ -470,7 +469,7 @@ export class Channel extends ApiKeyObject {
return false;
}
- async isMemberWithCache(user: User, userGroups) {
+ async isMemberWithCache(user: User, userGroups): Promise<boolean> {
if (!user) return false;
if (ChannelHelpers.memoizedIsInMembers(this.members, user)) {
diff --git a/src/models/user.ts b/src/models/user.ts
index b71fb37c..1771753a 100644
--- a/src/models/user.ts
+++ b/src/models/user.ts
@@ -73,8 +73,8 @@ export class User {
constructor(user: any) {
if (user) {
this.id = user.id;
- this.username = user.username;
- this.email = user.email;
+ this.username = user.username?.toLowerCase().trim();
+ this.email = user.email?.toLowerCase().trim();
this.enabled = true;
}
}
diff --git a/src/services/impl/auth/login/Login.ts b/src/services/impl/auth/login/Login.ts
index 9d16e526..d9b58a30 100644
--- a/src/services/impl/auth/login/Login.ts
+++ b/src/services/impl/auth/login/Login.ts
@@ -29,11 +29,6 @@ export class Login implements Command {
email: payload.email,
});
- // let foundUser = await transactionManager.findOne(
- // User,
- // { username: user.username }
- // );
-
// Search user on username OR email
let foundUser = await transactionManager
.getRepository(User)
@@ -49,11 +44,7 @@ export class Login implements Command {
foundUser = await this.authService.createUserWithDefaults(user);
} else {
// username is set, update mail if different
- if (
- foundUser.username &&
- user.email &&
- user.email !== foundUser.email
- ) {
+ if (foundUser.username && user.email && user.email !== foundUser.email) {
await this.authService.updateUserEmail(foundUser, user.email);
}
// no username, means external email that never logged in
diff --git a/src/services/impl/channels/add-member-to-channel.ts b/src/services/impl/channels/add-member-to-channel.ts
index a71b37b0..13b11798 100644
--- a/src/services/impl/channels/add-member-to-channel.ts
+++ b/src/services/impl/channels/add-member-to-channel.ts
@@ -18,7 +18,7 @@ export class AddMemberToChannel implements Command {
get newUser(): User {
if (!this.membername) return null;
- if (this.membername.indexOf('@') > 0) return new User({ email: this.membername });
+ if (this.membername.includes('@')) return new User({ email: this.membername });
return new User({ username: this.membername });
}
@@ -39,7 +39,8 @@ export class AddMemberToChannel implements Command {
throw new ForbiddenError(`User ${this.membername} is already a member or in a member group.`);
}
- const userToAdd = await this.usersService.GetOrCreateUserFromUsernameOrEmail(this.membername);
+ const userToAdd = await this.usersService.GetOrCreateUserFromUsernameOrEmail(this.membername.toLowerCase().trim());
+
//if resolved new user is already a direct member or a group member
if (await channel.isMember(userToAdd)) {
throw new ForbiddenError(
diff --git a/src/services/impl/notifications/send-notification.ts b/src/services/impl/notifications/send-notification.ts
index bea94e6c..298c6867 100644
--- a/src/services/impl/notifications/send-notification.ts
+++ b/src/services/impl/notifications/send-notification.ts
@@ -231,7 +231,7 @@ export class SendNotification implements Command {
const targetUsers = [];
await Promise.all(
this.notification.targetUsers.map(async (_targetUser: User) => {
- const userIdentifier = _targetUser.email.trim();
+ const userIdentifier = _targetUser.email.toLowerCase().trim();
let user;
try {
user = await this.usersService.GetOrCreateUserFromUsernameOrEmail(userIdentifier);
diff --git a/src/services/impl/users-service-impl.ts b/src/services/impl/users-service-impl.ts
index 75bf934f..acade439 100644
--- a/src/services/impl/users-service-impl.ts
+++ b/src/services/impl/users-service-impl.ts
@@ -4,14 +4,9 @@ import { User } from '../../models/user';
import { GetUserFromExternal } from './users/get-user-from-external';
import { GetOrCreateUserFromUsernameOrEmail } from './users/get-or-create-user';
-export class UsersServiceImpl
- extends AbstractService
- implements UsersServiceInterface
-{
+export class UsersServiceImpl extends AbstractService implements UsersServiceInterface {
GetOrCreateUserFromUsernameOrEmail(user: string): Promise<User> {
- return this.commandExecutor.execute(
- new GetOrCreateUserFromUsernameOrEmail(user),
- );
+ return this.commandExecutor.execute(new GetOrCreateUserFromUsernameOrEmail(user));
}
GetUserFromExternal(user: string): Promise<User> {
diff --git a/src/services/impl/users/get-or-create-user.ts b/src/services/impl/users/get-or-create-user.ts
index 203aaec6..86e62e8c 100644
--- a/src/services/impl/users/get-or-create-user.ts
+++ b/src/services/impl/users/get-or-create-user.ts
@@ -25,7 +25,7 @@ export class GetOrCreateUserFromUsernameOrEmail implements Command {
// email alias resolved to primary, let's use that one now
this.userIdentifier = userFromCernAlias.email;
userToAdd = await transactionManager.findOne(User, {
- where: [{ username: this.userIdentifier }, { email: this.userIdentifier }],
+ where: this.userIdentifier ? { username: this.userIdentifier } : { email: this.userIdentifier },
});
} else {
const username = usernameFromEmailIfValid(this.userIdentifier);
@@ -41,11 +41,10 @@ export class GetOrCreateUserFromUsernameOrEmail implements Command {
const user = await this.usersService.GetUserFromExternal(this.userIdentifier);
// Check again in DB if user already there in case it's a mapped user in Auth systems
userToAdd = await transactionManager.findOne(User, {
- where: [{ username: user.username }, { email: user.email }],
+ where: user.username ? { username: user.username } : { email: user.email },
});
if (!userToAdd) userToAdd = await this.authService.createUserWithDefaults(user);
}
-
return userToAdd;
}
}
diff --git a/src/services/impl/users/get-user-from-external.ts b/src/services/impl/users/get-user-from-external.ts
index 4b47f768..7280a76a 100644
--- a/src/services/impl/users/get-user-from-external.ts
+++ b/src/services/impl/users/get-user-from-external.ts
@@ -17,8 +17,7 @@ export class GetUserFromExternal implements Command {
}
// User not found, and lookup is not an email so we can't create an email only user
- if (!acUser && this.userIdentifier.indexOf('@') <= 0)
- throw new NotFoundError('The user does not exist');
+ if (!acUser && !this.userIdentifier.includes('@')) throw new NotFoundError('The user does not exist');
if (acUser) {
console.debug('Return known user', acUser);
--
GitLab