fix(server): invalidate old user avatar when updated (#7285)

fix CLOUD-41

packages/backend/server/src/core/storage/config.ts

@@ -7,7 +7,10 @@ export type StorageConfig<Ext = unknown> = {
 } & Ext;
 
 export interface StorageStartupConfigurations {
-  avatar: StorageConfig<{ publicLinkFactory: (key: string) => string }>;
+  avatar: StorageConfig<{
+    publicLinkFactory: (key: string) => string;
+    keyInPublicLink: (link: string) => string;
+  }>;
   blob: StorageConfig;
 }
 
@@ -22,6 +25,7 @@ defineStartupConfig('storages', {
     provider: 'fs',
     bucket: 'avatars',
     publicLinkFactory: key => `/api/avatars/${key}`,
+    keyInPublicLink: link => link.split('/').pop() as string,
   },
   blob: {
     provider: 'fs',

packages/backend/server/src/core/storage/wrappers/avatar.ts

@@ -42,8 +42,8 @@ export class AvatarStorage {
     return this.provider.get(key);
   }
 
-  delete(key: string) {
-    return this.provider.delete(key);
+  delete(link: string) {
+    return this.provider.delete(this.storageConfig.keyInPublicLink(link));
   }
 
   @OnEvent('user.deleted')

packages/backend/server/src/core/user/controller.ts

@@ -2,8 +2,10 @@ import { Controller, Get, Param, Res } from '@nestjs/common';
 import type { Response } from 'express';
 
 import { ActionForbidden, UserAvatarNotFound } from '../../fundamentals';
+import { Public } from '../auth/guard';
 import { AvatarStorage } from '../storage';
 
+@Public()
 @Controller('/api/avatars')
 export class UserAvatarController {
   constructor(private readonly storage: AvatarStorage) {}

packages/backend/server/src/core/user/resolver.ts

@@ -91,18 +91,26 @@ export class UserResolver {
     @Args({ name: 'avatar', type: () => GraphQLUpload })
     avatar: FileUpload
   ) {
+    if (!avatar.mimetype.startsWith('image/')) {
+      throw new Error('Invalid file type');
+    }
+
     if (!user) {
       throw new UserNotFound();
     }
 
     const avatarUrl = await this.storage.put(
-      `${user.id}-avatar`,
+      `${user.id}-avatar-${Date.now()}`,
       avatar.createReadStream(),
       {
         contentType: avatar.mimetype,
       }
     );
 
+    if (user.avatarUrl) {
+      await this.storage.delete(user.avatarUrl);
+    }
+
     return this.users.updateUser(user.id, { avatarUrl });
   }
 

packages/backend/server/tests/user/user.e2e.ts

@@ -0,0 +1,96 @@
+import type { INestApplication } from '@nestjs/common';
+import type { TestFn } from 'ava';
+import ava from 'ava';
+import request from 'supertest';
+
+import { AppModule } from '../../src/app.module';
+import { AuthService, CurrentUser } from '../../src/core/auth';
+import { createTestingApp, gql, internalSignIn } from '../utils';
+
+const test = ava as TestFn<{
+  app: INestApplication;
+  u1: CurrentUser;
+}>;
+
+test.beforeEach(async t => {
+  const { app } = await createTestingApp({
+    imports: [AppModule],
+  });
+
+  t.context.u1 = await app.get(AuthService).signUp('u1', 'u1@affine.pro', '1');
+  t.context.app = app;
+});
+
+test.afterEach.always(async t => {
+  await t.context.app.close();
+});
+
+async function fakeUploadAvatar(
+  app: INestApplication,
+  userId: string,
+  avatar: Buffer
+) {
+  const cookie = await internalSignIn(app, userId);
+
+  return gql(app)
+    .set('Cookie', cookie)
+    .field(
+      'operations',
+      JSON.stringify({
+        name: 'uploadAvatar',
+        query: `mutation uploadAvatar($avatar: Upload!) {
+        uploadAvatar(avatar: $avatar) {
+          avatarUrl
+        }
+      }`,
+        variables: { avatar: null },
+      })
+    )
+    .field('map', JSON.stringify({ '0': ['variables.avatar'] }))
+    .attach('0', avatar, {
+      filename: 'test.png',
+      contentType: 'image/png',
+    });
+}
+
+test('should be able to upload user avatar', async t => {
+  const { app } = t.context;
+
+  const avatar = Buffer.from('test');
+  const res = await fakeUploadAvatar(app, t.context.u1.id, avatar);
+
+  t.is(res.status, 200);
+  const avatarUrl = res.body.data.uploadAvatar.avatarUrl;
+  t.truthy(avatarUrl);
+
+  const avatarRes = await request(app.getHttpServer())
+    .get(new URL(avatarUrl).pathname)
+    .expect(200);
+
+  t.deepEqual(avatarRes.body, Buffer.from('test'));
+});
+
+test('should be able to update user avatar, and invalidate old avatar url', async t => {
+  const { app } = t.context;
+
+  const avatar = Buffer.from('test');
+  let res = await fakeUploadAvatar(app, t.context.u1.id, avatar);
+
+  const oldAvatarUrl = res.body.data.uploadAvatar.avatarUrl;
+
+  const newAvatar = Buffer.from('new');
+  res = await fakeUploadAvatar(app, t.context.u1.id, newAvatar);
+  const newAvatarUrl = res.body.data.uploadAvatar.avatarUrl;
+
+  t.not(oldAvatarUrl, newAvatarUrl);
+
+  await request(app.getHttpServer())
+    .get(new URL(oldAvatarUrl).pathname)
+    .expect(404);
+
+  const avatarRes = await request(app.getHttpServer())
+    .get(new URL(newAvatarUrl).pathname)
+    .expect(200);
+
+  t.deepEqual(avatarRes.body, Buffer.from('new'));
+});

packages/backend/server/tests/utils/utils.ts

@@ -6,6 +6,7 @@ import { PrismaClient } from '@prisma/client';
 import cookieParser from 'cookie-parser';
 import graphqlUploadExpress from 'graphql-upload/graphqlUploadExpress.mjs';
 import type { Response } from 'supertest';
+import supertest from 'supertest';
 
 import { AppModule, FunctionalityModules } from '../../src/app.module';
 import { AuthGuard, AuthModule } from '../../src/core/auth';
@@ -147,3 +148,15 @@ export function handleGraphQLError(resp: Response) {
     throw new Error(stacktrace ? stacktrace.join('\n') : cause.message, cause);
   }
 }
+
+export function gql(app: INestApplication, query?: string) {
+  const req = supertest(app.getHttpServer())
+    .post('/graphql')
+    .set({ 'x-request-id': 'test', 'x-operation-name': 'test' });
+
+  if (query) {
+    return req.send({ query });
+  }
+
+  return req;
+}