Merge pull request #164 from q16marvin/main

feat(backend:users): add avatar synchronization for OIDC users

Author: johaven

backend/src/applications/files/services/files-manager.service.ts

@@ -516,7 +516,7 @@ export class FilesManager {
 
     // do
     try {
-      await downloadFile(this.http, downloadDto, rPath, space)
+      await downloadFile(this.http, downloadDto, rPath, { space: space })
     } finally {
       // release lock
       await this.filesLockManager.removeLock(fileLock.key)

backend/src/applications/files/utils/download-file.ts

@@ -35,7 +35,12 @@ const parts = [
 const regExpPrivateIP = new RegExp(`^(?:${parts.join('|')})$`, 'i')
 const errorRegexpPrivateIP = 'Access to internal IP addresses is forbidden'
 
-export async function downloadFile(http: HttpService, downloadDto: DownloadFileDto, dstPath: string, space?: SpaceEnv) {
+export async function downloadFile(
+  http: HttpService,
+  downloadDto: DownloadFileDto,
+  dstPath: string,
+  options?: { space?: SpaceEnv; getContentInfo?: boolean }
+) {
   // dto must be validated by the caller
   const headRes: AxiosResponse = await http.axiosRef({ method: HTTP_METHOD.HEAD, url: downloadDto.url, maxRedirects: 1 })
   if (regExpPrivateIP.test(headRes.request.socket.remoteAddress)) {
@@ -45,18 +50,22 @@ export async function downloadFile(http: HttpService, downloadDto: DownloadFileD
 
   // attempt to retrieve the Content-Length header
   const contentLength = 'content-length' in headRes.headers ? Number(headRes.headers['content-length']) || null : null
+  if (options?.getContentInfo) {
+    return { contentLength: contentLength, contentType: `${headRes.headers['content-type']}`, lastModified: headRes.headers['last-modified'] }
+  }
+
   if (!contentLength) {
     throw new FileError(HttpStatus.BAD_REQUEST, 'Missing "content-length" header')
   }
 
-  if (space) {
-    if (space.willExceedQuota(contentLength)) {
+  if (options?.space) {
+    if (options.space.willExceedQuota(contentLength)) {
       throw new FileError(HttpStatus.INSUFFICIENT_STORAGE, 'Storage quota will be exceeded')
     }
     // tasking
-    if (space.task.cacheKey) {
-      space.task.props.totalSize = contentLength
-      FileTaskEvent.emit('startWatch', space, FILE_OPERATION.DOWNLOAD, dstPath)
+    if (options.space.task?.cacheKey) {
+      options.space.task.props.totalSize = contentLength
+      FileTaskEvent.emit('startWatch', options.space, FILE_OPERATION.DOWNLOAD, dstPath)
     }
   }
 

backend/src/applications/users/services/users-manager.service.spec.ts

@@ -1,11 +1,14 @@
 import { Test, TestingModule } from '@nestjs/testing'
 import bcrypt from 'bcryptjs'
+import fs from 'node:fs/promises'
+import os from 'node:os'
 import path from 'node:path'
 import { Readable } from 'node:stream'
 import { AuthManager } from '../../../authentication/auth.service'
 import { comparePassword } from '../../../common/functions'
 import * as imageModule from '../../../common/image'
 import { pngMimeType, svgMimeType } from '../../../common/image'
+import { configuration } from '../../../configuration/config.environment'
 import { Cache } from '../../../infrastructure/cache/services/cache.service'
 import { DB_TOKEN_PROVIDER } from '../../../infrastructure/database/constants'
 import * as filesUtilsModule from '../../files/utils/files'
@@ -44,6 +47,13 @@ describe(UsersManager.name, () => {
   let usersQueriesService: UsersQueries
   let userTest: UserModel
   let deleteUserDto: DeleteUserDto
+  let testDataPath: string
+  const initialFilesPaths = {
+    dataPath: configuration.applications.files.dataPath,
+    usersPath: configuration.applications.files.usersPath,
+    spacesPath: configuration.applications.files.spacesPath,
+    tmpPath: configuration.applications.files.tmpPath
+  }
   const flush = () => new Promise<void>((r) => setImmediate(r))
   const okStream = (d = 'OK') => {
     const s: any = Readable.from([Buffer.from(d)])
@@ -71,6 +81,12 @@ describe(UsersManager.name, () => {
   }
 
   beforeAll(async () => {
+    testDataPath = await fs.mkdtemp(path.join(os.tmpdir(), 'sync-in-users-manager-spec-'))
+    configuration.applications.files.dataPath = testDataPath
+    configuration.applications.files.usersPath = path.join(testDataPath, 'users')
+    configuration.applications.files.spacesPath = path.join(testDataPath, 'spaces')
+    configuration.applications.files.tmpPath = path.join(testDataPath, 'tmp')
+
     const module: TestingModule = await Test.createTestingModule({
       providers: [
         AdminUsersManager,
@@ -100,6 +116,11 @@ describe(UsersManager.name, () => {
 
   afterAll(async () => {
     await expect(adminUsersManager.deleteUserSpace(userTest.login)).resolves.not.toThrow()
+    configuration.applications.files.dataPath = initialFilesPaths.dataPath
+    configuration.applications.files.usersPath = initialFilesPaths.usersPath
+    configuration.applications.files.spacesPath = initialFilesPaths.spacesPath
+    configuration.applications.files.tmpPath = initialFilesPaths.tmpPath
+    await fs.rm(testDataPath, { recursive: true, force: true })
   })
 
   it('instances + findUser/me/fromUserId + impersonation', async () => {
@@ -271,7 +292,7 @@ describe(UsersManager.name, () => {
 
   it('avatars advanced: generateIsNotExists, failure branches, base64 fallback', async () => {
     await ensurePaths()
-    usersManager.findUser = jest.fn().mockResolvedValue({ getInitials: () => 'UT' } as unknown as UserModel)
+    usersManager.findUser = jest.fn().mockResolvedValue({ login: userTest.login, getInitials: () => 'UT' } as unknown as UserModel)
     const [p, m] = (await usersManager.getAvatar(userTest.login, false, true)) as [string, string]
     expect(fileName(p)).toBe('avatar.png')
     expect(m).toBe(pngMimeType)
@@ -289,7 +310,7 @@ describe(UsersManager.name, () => {
     const t = okStream('OK')
     t.truncated = true
     const mvSpy = jest.spyOn(filesUtilsModule, 'moveFiles').mockResolvedValue(undefined)
-    await expect(usersManager.updateAvatar(mkReq('image/png', t) as any)).rejects.toThrow('Image is too large (5MB max)')
+    await expect(usersManager.updateAvatar(mkReq('image/png', t) as any)).rejects.toThrow('Image is too large')
     expect(mvSpy).not.toHaveBeenCalled()
 
     jest.spyOn(filesUtilsModule, 'moveFiles').mockRejectedValue(new Error('mv fail'))

backend/src/applications/users/services/users-manager.service.ts

@@ -11,7 +11,7 @@ import { FastifyAuthenticatedRequest } from '../../../authentication/interfaces/
 import { JwtIdentityPayload } from '../../../authentication/interfaces/jwt-payload.interface'
 import { ACTION } from '../../../common/constants'
 import { comparePassword, hashPassword } from '../../../common/functions'
-import { generateAvatar, pngMimeType, svgMimeType } from '../../../common/image'
+import { generateAvatar, imgMimeTypePrefix, pngMimeType, svgMimeType } from '../../../common/image'
 import { createLightSlug, genPassword } from '../../../common/shared'
 import { configuration, serverConfig } from '../../../configuration/config.environment'
 import { isPathExists, moveFiles, sanitizeName } from '../../files/utils/files'
@@ -40,7 +40,7 @@ import { UserModel } from '../models/user.model'
 import type { Group } from '../schemas/group.interface'
 import type { UserGroup } from '../schemas/user-group.interface'
 import type { User } from '../schemas/user.interface'
-import { USER_AVATAR_FILE_NAME, USER_AVATAR_MAX_UPLOAD_SIZE, USER_DEFAULT_AVATAR_FILE_PATH } from '../utils/avatar'
+import { saveAvatarMetadata, USER_AVATAR_FILE_NAME, USER_AVATAR_MAX_UPLOAD_SIZE, USER_DEFAULT_AVATAR_FILE_PATH } from '../utils/avatar'
 import { AdminUsersManager } from './admin-users-manager.service'
 import { UsersQueries } from './users-queries.service'
 
@@ -144,7 +144,7 @@ export class UsersManager {
 
   async updateAvatar(req: FastifyAuthenticatedRequest) {
     const part: MultipartFile = await req.file({ limits: { fileSize: USER_AVATAR_MAX_UPLOAD_SIZE } })
-    if (!part.mimetype.startsWith('image/')) {
+    if (!part.mimetype.startsWith(imgMimeTypePrefix)) {
       throw new HttpException('Unsupported file type', HttpStatus.BAD_REQUEST)
     }
     const dstPath = path.join(req.user.tmpPath, USER_AVATAR_FILE_NAME)
@@ -156,10 +156,12 @@ export class UsersManager {
     }
     if (part.file.truncated) {
       this.logger.warn({ tag: this.updateAvatar.name, msg: `image is too large` })
-      throw new HttpException('Image is too large (5MB max)', HttpStatus.PAYLOAD_TOO_LARGE)
+      throw new HttpException('Image is too large', HttpStatus.PAYLOAD_TOO_LARGE)
     }
+    const avatarPath = path.join(req.user.homePath, USER_AVATAR_FILE_NAME)
     try {
-      await moveFiles(dstPath, path.join(req.user.homePath, USER_AVATAR_FILE_NAME), true)
+      await moveFiles(dstPath, avatarPath, true)
+      void saveAvatarMetadata(req.user.login, 'user')
     } catch (e) {
       this.logger.error({ tag: this.updateAvatar.name, msg: `${e}` })
       throw new HttpException('Unable to create avatar', HttpStatus.INTERNAL_SERVER_ERROR)
@@ -214,6 +216,7 @@ export class UsersManager {
     const avatarStream: NodeJS.ReadableStream = await generateAvatar(user.getInitials())
     try {
       await pipeline(avatarStream, avatarFile)
+      void saveAvatarMetadata(user.login, 'local')
     } catch (e) {
       this.logger.error({ tag: this.getAvatar.name, msg: `${e}` })
       throw new HttpException('Unable to create avatar', HttpStatus.INTERNAL_SERVER_ERROR)

backend/src/applications/users/utils/avatar.ts

@@ -3,12 +3,48 @@ import { convertImageToBase64 } from '../../../common/image'
 import { STATIC_ASSETS_PATH } from '../../../configuration/config.constants'
 import { isPathExists } from '../../files/utils/files'
 import { UserModel } from '../models/user.model'
+import { readFile, writeFile } from 'node:fs/promises'
+import fs from 'fs/promises'
 
 export const USER_DEFAULT_AVATAR_FILE_PATH = path.join(STATIC_ASSETS_PATH, 'avatar.svg')
 export const USER_AVATAR_FILE_NAME = 'avatar.png'
+export const USER_AVATAR_INFO = 'avatar.json' // used to determine if the avatar must be updated (oidc/ldap case)
 export const USER_AVATAR_MAX_UPLOAD_SIZE = 1024 * 1024 * 5 // 5MB
 
+export interface AvatarInfo {
+  origin: string
+  size: number
+  lastModified?: string
+}
+
 export async function getAvatarBase64(userLogin: string): Promise<string> {
   const userAvatarPath = path.join(UserModel.getHomePath(userLogin), USER_AVATAR_FILE_NAME)
   return convertImageToBase64((await isPathExists(userAvatarPath)) ? userAvatarPath : USER_DEFAULT_AVATAR_FILE_PATH)
 }
+
+export async function saveAvatarMetadata(userLogin: string, origin: string, size?: number, lastModified?: string): Promise<void> {
+  const userAvatarInfoPath = path.join(UserModel.getHomePath(userLogin), USER_AVATAR_INFO)
+  try {
+    if (size === undefined || lastModified === undefined) {
+      const userAvatarPath = path.join(UserModel.getHomePath(userLogin), USER_AVATAR_FILE_NAME)
+      const stats = await fs.stat(userAvatarPath)
+      size ??= stats.size
+      lastModified ??= stats.mtime.toUTCString()
+    }
+    await writeFile(userAvatarInfoPath, JSON.stringify({ origin, size, lastModified } satisfies AvatarInfo))
+  } catch {
+    // ignore
+  }
+}
+
+export async function isAvatarMetadataUnchanged(userLogin: string, origin: string, size: number, lastModified: string): Promise<boolean> {
+  const userAvatarInfoPath = path.join(UserModel.getHomePath(userLogin), USER_AVATAR_INFO)
+  if (!(await isPathExists(userAvatarInfoPath))) return false
+  let avatarInfo: AvatarInfo
+  try {
+    avatarInfo = JSON.parse(await readFile(userAvatarInfoPath, 'utf8'))
+  } catch {
+    return false
+  }
+  return avatarInfo?.origin === origin && avatarInfo?.size === size && avatarInfo?.lastModified === lastModified
+}

backend/src/authentication/providers/oidc/auth-provider-oidc.service.spec.ts

@@ -1,4 +1,5 @@
 import { HttpStatus } from '@nestjs/common'
+import { HttpService } from '@nestjs/axios'
 import { Test, TestingModule } from '@nestjs/testing'
 import {
   authorizationCodeGrant,
@@ -10,8 +11,13 @@ import {
   randomState
 } from 'openid-client'
 import { USER_ROLE } from '../../../applications/users/constants/user'
+import { UserModel } from '../../../applications/users/models/user.model'
 import { AdminUsersManager } from '../../../applications/users/services/admin-users-manager.service'
 import { UsersManager } from '../../../applications/users/services/users-manager.service'
+import * as avatarUtils from '../../../applications/users/utils/avatar'
+import * as filesUtils from '../../../applications/files/utils/files'
+import * as downloadFileUtils from '../../../applications/files/utils/download-file'
+import * as imageUtils from '../../../common/image'
 import { OAuthCookie } from './auth-oidc.constants'
 import { AuthProviderOIDC } from './auth-provider-oidc.service'
 
@@ -85,6 +91,9 @@ describe(AuthProviderOIDC.name, () => {
     createUserOrGuest: jest.Mock
     updateUserOrGuest: jest.Mock
   }
+  let httpService: {
+    axiosRef: jest.Mock
+  }
 
   const makeConfig = (supportsPKCE = true) => ({
     serverMetadata: () => ({
@@ -110,9 +119,17 @@ describe(AuthProviderOIDC.name, () => {
       createUserOrGuest: jest.fn(),
       updateUserOrGuest: jest.fn()
     }
+    httpService = {
+      axiosRef: jest.fn()
+    }
 
     const module: TestingModule = await Test.createTestingModule({
-      providers: [{ provide: UsersManager, useValue: usersManager }, { provide: AdminUsersManager, useValue: adminUsersManager }, AuthProviderOIDC]
+      providers: [
+        { provide: HttpService, useValue: httpService },
+        { provide: UsersManager, useValue: usersManager },
+        { provide: AdminUsersManager, useValue: adminUsersManager },
+        AuthProviderOIDC
+      ]
     }).compile()
 
     module.useLogger(['fatal'])
@@ -261,4 +278,87 @@ describe(AuthProviderOIDC.name, () => {
     )
     expect(result.role).toBe(USER_ROLE.ADMINISTRATOR)
   })
+
+  describe('updatePictureUrl', () => {
+    const oidcUser = { login: 'alice', tmpPath: '/tmp/sync-in/alice/tmp' } as UserModel
+    const userInfo = (picture = 'https://cdn.example.test/avatar.jpg') => ({ picture }) as any
+
+    it('returns when picture url is invalid', async () => {
+      const downloadSpy = jest.spyOn(downloadFileUtils, 'downloadFile')
+
+      await (service as any).updatePictureUrl(oidcUser, userInfo('not-a-url'))
+
+      expect(downloadSpy).not.toHaveBeenCalled()
+    })
+
+    it('stops when content type is not an image', async () => {
+      const downloadSpy = jest.spyOn(downloadFileUtils, 'downloadFile').mockResolvedValueOnce({
+        contentType: 'text/plain',
+        contentLength: 123,
+        lastModified: 'Mon, 01 Jan 2024 00:00:00 GMT'
+      } as any)
+      const convertSpy = jest.spyOn(imageUtils, 'convertTempImageToPng').mockResolvedValue(undefined)
+
+      await (service as any).updatePictureUrl(oidcUser, userInfo())
+
+      expect(downloadSpy).toHaveBeenCalledTimes(1)
+      expect(convertSpy).not.toHaveBeenCalled()
+    })
+
+    it('skips update when avatar metadata is unchanged', async () => {
+      const downloadSpy = jest.spyOn(downloadFileUtils, 'downloadFile').mockResolvedValueOnce({
+        contentType: 'image/png',
+        contentLength: 128,
+        lastModified: 'Mon, 01 Jan 2024 00:00:00 GMT'
+      } as any)
+      jest.spyOn(avatarUtils, 'isAvatarMetadataUnchanged').mockResolvedValue(true)
+      const convertSpy = jest.spyOn(imageUtils, 'convertTempImageToPng').mockResolvedValue(undefined)
+
+      await (service as any).updatePictureUrl(oidcUser, userInfo())
+
+      expect(downloadSpy).toHaveBeenCalledTimes(1)
+      expect(convertSpy).not.toHaveBeenCalled()
+    })
+
+    it('downloads and converts avatar when checks pass', async () => {
+      const downloadSpy = jest
+        .spyOn(downloadFileUtils, 'downloadFile')
+        .mockResolvedValueOnce({
+          contentType: 'image/png',
+          contentLength: 128,
+          lastModified: 'Mon, 01 Jan 2024 00:00:00 GMT'
+        } as any)
+        .mockResolvedValueOnce(undefined as any)
+      jest.spyOn(avatarUtils, 'isAvatarMetadataUnchanged').mockResolvedValue(false)
+      jest.spyOn(filesUtils, 'fileSize').mockResolvedValue(1024)
+      jest.spyOn(UserModel, 'getHomePath').mockReturnValue('/tmp/sync-in/users/alice')
+      const convertSpy = jest.spyOn(imageUtils, 'convertTempImageToPng').mockResolvedValue(undefined)
+      const metadataSpy = jest.spyOn(avatarUtils, 'saveAvatarMetadata').mockResolvedValue(undefined)
+
+      await (service as any).updatePictureUrl(oidcUser, userInfo())
+
+      expect(downloadSpy).toHaveBeenCalledTimes(2)
+      expect(convertSpy).toHaveBeenCalledWith('/tmp/sync-in/alice/tmp/avatar.png', '/tmp/sync-in/users/alice/avatar.png')
+      expect(metadataSpy).toHaveBeenCalledWith('alice', 'https://cdn.example.test/avatar.jpg', 128, 'Mon, 01 Jan 2024 00:00:00 GMT')
+    })
+
+    it('stops after download when avatar size exceeds limit', async () => {
+      const downloadSpy = jest
+        .spyOn(downloadFileUtils, 'downloadFile')
+        .mockResolvedValueOnce({
+          contentType: 'image/png',
+          contentLength: 128,
+          lastModified: 'Mon, 01 Jan 2024 00:00:00 GMT'
+        } as any)
+        .mockResolvedValueOnce(undefined as any)
+      jest.spyOn(avatarUtils, 'isAvatarMetadataUnchanged').mockResolvedValue(false)
+      jest.spyOn(filesUtils, 'fileSize').mockResolvedValue(avatarUtils.USER_AVATAR_MAX_UPLOAD_SIZE + 1)
+      const convertSpy = jest.spyOn(imageUtils, 'convertTempImageToPng').mockResolvedValue(undefined)
+
+      await (service as any).updatePictureUrl(oidcUser, userInfo())
+
+      expect(downloadSpy).toHaveBeenCalledTimes(2)
+      expect(convertSpy).not.toHaveBeenCalled()
+    })
+  })
 })