fix: Update updateUser code to reject empty emails (#1210)

* fix: Update updateUser code to reject empty emails

This fixes an issue where the updateUser call would allow null email
addresses, then update the email to null and then raise an exception,
leaving the db in a state where no user could be resolved.

* fix: remove username/email requirement in user.ts

Co-authored-by: Ivar Conradi Østhus <ivarconr@gmail.com>

src/lib/services/user-service.ts

@@ -235,9 +235,7 @@ class UserService {
         { id, name, email, rootRole }: IUpdateUser,
         updatedBy?: User,
     ): Promise<IUser> {
-        if (email) {
-            Joi.assert(email, Joi.string().email(), 'Email');
-        }
+        Joi.assert(email, Joi.string().email(), 'Email');
 
         const preUser = await this.store.get(id);
 

src/lib/types/user.test.ts

@@ -24,15 +24,13 @@ test('should create user, all fields', () => {
     );
 });
 
-test('should require email or username', () => {
-    expect(() => {
-        const user = new User({ id: 11 }); // eslint-disable-line
-    }).toThrowError(Error);
-    expect(() => {
-        const user = new User({ id: 11 }); // eslint-disable-line
-    }).toThrow('Username or Email is required');
-});
+test('Should create user with only id defined', () => {
+    const user = new User({ id: 123 });
 
+    expect(user.id).toBe(123);
+    expect(user.email).toBeUndefined();
+    expect(user.username).toBeUndefined();
+});
 test('Should create user with only email defined', () => {
     const user = new User({ id: 123, email: 'some@email.com' });
 

src/lib/types/user.ts

@@ -60,9 +60,6 @@ export default class User implements IUser {
         if (!id) {
             throw new TypeError('Id is required');
         }
-        if (!username && !email) {
-            throw new TypeError('Username or Email is required');
-        }
         Joi.assert(email, Joi.string().email(), 'Email');
         Joi.assert(username, Joi.string(), 'Username');
         Joi.assert(name, Joi.string(), 'Name');
@@ -78,7 +75,7 @@ export default class User implements IUser {
     }
 
     generateImageUrl(): string {
-        return gravatarUrl(this.email || this.username, {
+        return gravatarUrl(this.email || this.username || '' + this.id, {
             size: 42,
             default: 'retro',
         });

src/test/e2e/services/user-service.e2e.test.ts

@@ -180,6 +180,27 @@ test("deleting a user should delete the user's sessions", async () => {
     ).rejects.toThrow(NotFoundError);
 });
 
+test('updating a user without an email should not strip the email', async () => {
+    const email = 'some@test.com';
+    const user = await userService.createUser({
+        email,
+        password: 'A very strange P4ssw0rd_',
+        rootRole: adminRole.id,
+    });
+
+    try {
+        await userService.updateUser({
+            id: user.id,
+            email: null,
+            name: 'some',
+        });
+    } catch (e) {}
+
+    const updatedUser = await userService.getUser(user.id);
+
+    expect(updatedUser.email).toBe(email);
+});
+
 test('should login and create user via SSO', async () => {
     const email = 'some@test.com';
     const user = await userService.loginUserSSO({