diff --git a/Backend.Tests/Mocks/UserServiceMock.cs b/Backend.Tests/Mocks/UserServiceMock.cs index 7de3b97466..76910f6ff8 100644 --- a/Backend.Tests/Mocks/UserServiceMock.cs +++ b/Backend.Tests/Mocks/UserServiceMock.cs @@ -105,5 +105,11 @@ public Task ChangePassword(string userId, string password) { return Task.FromResult(ResultOfUpdate.Updated); //TODO: more sophisticated mock } + public void Sanitize(User user) + { + user.Avatar = null; + user.Password = null; + user.Token = null; + } } } diff --git a/Backend/Controllers/AvatarController.cs b/Backend/Controllers/AvatarController.cs index ecc8b0124a..97bb9e74b7 100644 --- a/Backend/Controllers/AvatarController.cs +++ b/Backend/Controllers/AvatarController.cs @@ -80,6 +80,7 @@ public async Task UploadAvatar(string userId, [FromForm] FileUplo // Update the user's avatar file gotUser.Avatar = fileUpload.FilePath; + gotUser.HasAvatar = true; _ = await _userService.Update(userId, gotUser); return new OkResult(); diff --git a/Backend/Interfaces/IUserService.cs b/Backend/Interfaces/IUserService.cs index 8e35ce2d86..fa74ab322c 100644 --- a/Backend/Interfaces/IUserService.cs +++ b/Backend/Interfaces/IUserService.cs @@ -17,5 +17,6 @@ public interface IUserService Task Authenticate(string username, string password); Task MakeJwt(User user); Task ChangePassword(string userId, string password); + void Sanitize(User user); } } diff --git a/Backend/Models/User.cs b/Backend/Models/User.cs index 7e10ef38e2..556c6d0631 100644 --- a/Backend/Models/User.cs +++ b/Backend/Models/User.cs @@ -15,6 +15,9 @@ public class User [BsonElement("avatar")] public string Avatar { get; set; } + [BsonElement("hasAvatar")] + public bool HasAvatar { get; set; } + [BsonElement("name")] public string Name { get; set; } @@ -62,6 +65,7 @@ public User() { Id = ""; Avatar = ""; + HasAvatar = false; Name = ""; Email = ""; Phone = ""; @@ -82,6 +86,7 @@ public User Clone() { Id = Id.Clone() as string, Avatar = Avatar.Clone() as string, + HasAvatar = HasAvatar, Name = Name.Clone() as string, Email = Email.Clone() as string, Phone = Phone.Clone() as string, @@ -113,6 +118,7 @@ public bool ContentEquals(User other) return other.Id.Equals(Id) && other.Avatar.Equals(Avatar) && + other.HasAvatar.Equals(HasAvatar) && other.Name.Equals(Name) && other.Email.Equals(Email) && other.Phone.Equals(Phone) && @@ -145,6 +151,7 @@ public override int GetHashCode() var hash = new HashCode(); hash.Add(Id); hash.Add(Avatar); + hash.Add(HasAvatar); hash.Add(Name); hash.Add(Email); hash.Add(Phone); diff --git a/Backend/Services/UserApiServices.cs b/Backend/Services/UserApiServices.cs index cc7a959372..ba4c3ac2a6 100644 --- a/Backend/Services/UserApiServices.cs +++ b/Backend/Services/UserApiServices.cs @@ -157,6 +157,10 @@ public async Task MakeJwt(User user) new SymmetricSecurityKey(key), SecurityAlgorithms.HmacSha256Signature) }; var token = tokenHandler.CreateToken(tokenDescriptor); + + // Sanitize user to remove password, avatar path, and old token + // Then add updated token. + Sanitize(user); user.Token = tokenHandler.WriteToken(token); if (await Update(user.Id, user) != ResultOfUpdate.Updated) @@ -164,10 +168,6 @@ public async Task MakeJwt(User user) return null; } - // Remove password and avatar filepath before returning - user.Password = ""; - user.Avatar = ""; - return user; } @@ -175,7 +175,8 @@ public async Task MakeJwt(User user) public async Task> GetAllUsers() { var users = await _userDatabase.Users.Find(_ => true).ToListAsync(); - return users.Select(c => { c.Avatar = ""; c.Password = ""; c.Token = ""; return c; }).ToList(); + users.ForEach(Sanitize); + return (users); } /// Removes all s @@ -193,11 +194,8 @@ public async Task GetUser(string userId) var filter = filterDef.Eq(x => x.Id, userId); var userList = await _userDatabase.Users.FindAsync(filter); - var user = userList.FirstOrDefault(); - user.Avatar = ""; - user.Password = ""; - user.Token = ""; + Sanitize(user); return user; } @@ -256,11 +254,7 @@ public async Task Create(User user) // Replace password with encoded, hashed password. user.Password = Convert.ToBase64String(hash); await _userDatabase.Users.InsertOneAsync(user); - - // Important, don't send plaintext password back to user. - user.Password = ""; - user.Avatar = ""; - + Sanitize(user); return user; } @@ -272,6 +266,15 @@ public async Task Delete(string userId) return deleted.DeletedCount > 0; } + /// Removes avatar path, password, and token from + public void Sanitize(User user) + { + // .Avatar or .Token set to "" or null will not be updated in the database + user.Avatar = null; + user.Password = null; + user.Token = null; + } + /// Updates with specified userId /// A enum: success of operation public async Task Update(string userId, User user, bool updateIsAdmin = false) @@ -280,6 +283,7 @@ public async Task Update(string userId, User user, bool updateIs // Note: Nulls out values not in update body var updateDef = Builders.Update + .Set(x => x.HasAvatar, user.HasAvatar) .Set(x => x.Name, user.Name) .Set(x => x.Email, user.Email) .Set(x => x.Phone, user.Phone) @@ -290,11 +294,12 @@ public async Task Update(string userId, User user, bool updateIs .Set(x => x.Username, user.Username) .Set(x => x.UILang, user.UILang); + // If .Avatar or .Token has been set to null or "", + // this prevents it from being erased in the database if (!string.IsNullOrEmpty(user.Avatar)) { updateDef = updateDef.Set(x => x.Avatar, user.Avatar); } - if (!string.IsNullOrEmpty(user.Token)) { updateDef = updateDef.Set(x => x.Token, user.Token); diff --git a/src/components/ProjectSettings/ProjectUsers/ActiveUsers.tsx b/src/components/ProjectSettings/ProjectUsers/ActiveUsers.tsx index 7647d87271..359d993922 100644 --- a/src/components/ProjectSettings/ProjectUsers/ActiveUsers.tsx +++ b/src/components/ProjectSettings/ProjectUsers/ActiveUsers.tsx @@ -36,19 +36,18 @@ export default class ActiveUsers extends React.Component { private populateUsers() { getAllUsersInCurrentProject() - .then((projUsers) => { + .then(async (projUsers) => { this.setState({ projUsers }); - projUsers.forEach((u: User) => { - avatarSrc(u.id) - .then((result) => { - let userAvatar = this.state.userAvatar; - userAvatar[u.id] = result; - this.setState({ userAvatar }); - }) - .catch((err) => console.log(err)); + const userAvatar = this.state.userAvatar; + const promises = projUsers.map(async (u) => { + if (u.hasAvatar) { + userAvatar[u.id] = await avatarSrc(u.id); + } }); + await Promise.all(promises); + this.setState({ userAvatar }); }) - .catch((err) => console.log(err)); + .catch((err) => console.error(err)); } render() { diff --git a/src/components/ProjectSettings/ProjectUsers/ProjectUsers.tsx b/src/components/ProjectSettings/ProjectUsers/ProjectUsers.tsx index bee8587414..6f14c7aec1 100644 --- a/src/components/ProjectSettings/ProjectUsers/ProjectUsers.tsx +++ b/src/components/ProjectSettings/ProjectUsers/ProjectUsers.tsx @@ -73,26 +73,24 @@ class ProjectUsers extends React.Component { this.setState({ projUsers }); backend .getAllUsers() - .then((returnedUsers) => { + .then(async (returnedUsers) => { this.setState((prevState) => ({ allUsers: returnedUsers.filter( (user) => !prevState.projUsers.find((u) => u.id === user.id) ), })); - returnedUsers.forEach((u: User) => { - backend - .avatarSrc(u.id) - .then((result) => { - let userAvatar = this.state.userAvatar; - userAvatar[u.id] = result; - this.setState({ userAvatar }); - }) - .catch((err) => console.log(err)); + const userAvatar = this.state.userAvatar; + const promises = projUsers.map(async (u) => { + if (u.hasAvatar) { + userAvatar[u.id] = await backend.avatarSrc(u.id); + } }); + await Promise.all(promises); + this.setState({ userAvatar }); }) - .catch((err) => console.log(err)); + .catch((err) => console.error(err)); }) - .catch((err) => console.log(err)); + .catch((err) => console.error(err)); } addToProject(user: User) { diff --git a/src/types/user.tsx b/src/types/user.tsx index 2449b0d6e2..d93531f1cd 100644 --- a/src/types/user.tsx +++ b/src/types/user.tsx @@ -1,35 +1,22 @@ import { Hash } from "../goals/MergeDupGoal/MergeDupStep/MergeDupsTree"; export class User { - id: string; - avatar: string; - name: string; - email: string; - phone: string; - otherConnectionField: string; - projectRoles: Hash; - workedProjects: Hash; - agreement: boolean; - password: string; - username: string; - uiLang: string; - token: string; - isAdmin: boolean; + id: string = ""; + avatar: string = ""; + hasAvatar: boolean = false; + email: string = ""; + phone: string = ""; + otherConnectionField: string = ""; + projectRoles: Hash = {}; + workedProjects: Hash = {}; + agreement: boolean = false; + uiLang: string = ""; + token: string = ""; + isAdmin: boolean = false; - constructor(name: string, username: string, password: string) { - this.id = ""; - this.avatar = ""; - this.name = name; - this.email = ""; - this.phone = ""; - this.otherConnectionField = ""; - this.projectRoles = {}; - this.workedProjects = {}; - this.agreement = false; - this.password = password; - this.username = username; - this.uiLang = ""; - this.token = ""; - this.isAdmin = false; - } + constructor( + public name: string, + public username: string, + public password: string + ) {} }