fix(identities): clean up partner permissions, limits and directories on identity webhooks

api/package.json

@@ -21,7 +21,7 @@
     "@types/ws": "^8.5.12"
   },
   "dependencies": {
-    "@data-fair/lib-express": "^1.25.0",
+    "@data-fair/lib-express": "^1.26.0",
     "@data-fair/lib-node": "^2.13.3",
     "@data-fair/lib-node-registry": "^0.7.1",
     "@data-fair/lib-utils": "^1.13.1",

api/src/misc/routers/identities.ts

@@ -1,9 +1,10 @@
-// Webhooks for Simple Directory
+// Webhooks for Simple Directory: keep the copies of identity data in sync and clean up after a deletion
 
 import Debug from 'debug'
 import { createIdentitiesRouter } from '@data-fair/lib-express/identities/index.js'
 import config from '#config'
 import mongo from '#mongo'
+import { deleteProcessing } from '../../runs/service.ts'
 
 const debug = Debug('webhooks-simple-directory')
 
@@ -20,46 +21,81 @@ export default createIdentitiesRouter(
   // onUpdate
   async (identity) => {
     debug('Incoming sd webhook for update', identity)
+    const { type, id, name } = identity
 
     // Update owner name
-    await updateAllCollections(
-      { 'owner.type': identity.type, 'owner.id': identity.id },
-      { $set: { 'owner.name': identity.name } }
-    )
+    await updateAllCollections({ 'owner.type': type, 'owner.id': id }, { $set: { 'owner.name': name } })
+    await mongo.limits.updateMany({ type, id }, { $set: { name } })
 
-    if (identity.type === 'user') {
+    if (type === 'user') {
       // Update created/updated name
       await Promise.all([
-        updateAllCollections(
-          { 'created.id': identity.id },
-          { $set: { 'created.name': identity.name } }
-        ),
-        updateAllCollections(
-          { 'updated.id': identity.id },
-          { $set: { 'updated.name': identity.name } }
-        )
+        updateAllCollections({ 'created.id': id }, { $set: { 'created.name': name } }),
+        updateAllCollections({ 'updated.id': id }, { $set: { 'updated.name': name } })
       ])
     }
 
+    if (type === 'organization') {
+      // the organization as a partner granted permissions on the processings of others
+      await mongo.processings.updateMany(
+        { permissions: { $elemMatch: { 'target.type': 'partner', 'target.organization.id': id } } },
+        { $set: { 'permissions.$[p].target.organization.name': name } },
+        { arrayFilters: [{ 'p.target.type': 'partner', 'p.target.organization.id': id }] }
+      )
+      // partner permissions are only meaningful inside a partnership: the directory sends the complete
+      // list of partners, what is not in it was withdrawn
+      if (identity.partners) {
+        const partnerIds = identity.partners.map(p => p.id)
+        await mongo.processings.updateMany(
+          { 'owner.type': 'organization', 'owner.id': id, permissions: { $elemMatch: { 'target.type': 'partner', 'target.organization.id': { $nin: partnerIds } } } },
+          { $pull: { permissions: { 'target.type': 'partner', 'target.organization.id': { $nin: partnerIds } } } } as any
+        )
+      }
+    }
+
     // If the identity has departments, update the department names in processings and runs
     if (identity.departments) {
       const departmentUpdates = identity.departments.map(department =>
         updateAllCollections(
-          { 'owner.type': identity.type, 'owner.id': identity.id, 'owner.department': department.id },
+          { 'owner.type': type, 'owner.id': id, 'owner.department': department.id },
           { $set: { 'owner.departmentName': department.name } }
         )
       )
       await Promise.all(departmentUpdates)
+      // the directory sends the complete list of departments: a department missing from it was
+      // deleted, its resources keep the id (still reachable by the organization admins) but not the name
+      await updateAllCollections(
+        { 'owner.type': type, 'owner.id': id, 'owner.department': { $exists: true, $nin: identity.departments.map(d => d.id) } },
+        { $unset: { 'owner.departmentName': 1 } }
+      )
     }
   },
 
   // onDelete
   async (identity) => {
     debug('Incoming sd webhook for delete', identity)
-    // Delete all processings and runs for this identity
-    const filter = { 'owner.type': identity.type, 'owner.id': identity.id }
-    await mongo.processings.deleteMany(filter)
-    await mongo.runs.deleteMany(filter)
-    // When departments are deleted, do nothing
+    const { type, id } = identity
+
+    // Delete all processings for this identity, with their runs and their directory
+    for await (const processing of mongo.processings.find({ 'owner.type': type, 'owner.id': id })) {
+      await mongo.processings.deleteOne({ _id: processing._id })
+      await deleteProcessing(mongo, processing)
+    }
+    await mongo.runs.deleteMany({ 'owner.type': type, 'owner.id': id })
+    await mongo.limits.deleteMany({ type, id })
+
+    if (type === 'organization') {
+      await mongo.processings.updateMany(
+        { permissions: { $elemMatch: { 'target.type': 'partner', 'target.organization.id': id } } },
+        { $pull: { permissions: { 'target.type': 'partner', 'target.organization.id': id } } } as any
+      )
+    }
+    if (type === 'user') {
+      // what the user authored on the processings of others keeps the trace of the action without the name
+      await Promise.all([
+        updateAllCollections({ 'created.id': id }, { $unset: { 'created.name': 1 } }),
+        updateAllCollections({ 'updated.id': id }, { $unset: { 'updated.name': 1 } })
+      ])
+    }
   }
 )

api/types/processing/schema.js

@@ -48,9 +48,9 @@ export default {
     created: {
       type: 'object',
       additionalProperties: false,
+      // no name once the user is deleted from the directory
       required: [
         'id',
-        'name',
         'date'
       ],
       readOnly: true,
@@ -128,7 +128,6 @@ export default {
       readOnly: true,
       required: [
         'id',
-        'name',
         'date'
       ],
       properties: {

docker-compose.yml

@@ -39,6 +39,8 @@ services:
       - CONTACT=contact@test.com
       - ADMINS=["superadmin@test.com","test_superadmin@test.com"]
       - PUBLIC_URL=http://${DEV_HOST}:${NGINX_PORT1}/simple-directory
+      # in dev simple-directory only notifies this service, directly (the shared identities router refuses calls through the proxy)
+      - IDENTITIES_WEBHOOKS=[{"base":"http://localhost:${DEV_API_PORT}/api/identities","key":"secret-identities"}]
       - MAILDEV_ACTIVE=true
       - MONGO_URL=mongodb://localhost:${MONGO_PORT}/simple-directory
       - STORAGE_TYPE=file

package.json

@@ -34,6 +34,10 @@
     "worker",
     "shared"
   ],
+  "relativeDependencies": {
+    "@data-fair/lib-vue": "../lib/packages/vue",
+    "@data-fair/lib-vuetify": "../lib/packages/vuetify"
+  },
   "bugs": {
     "url": "https://github.com/data-fair/processings/issues"
   },

tests/features/processings/identities.api.spec.ts

@@ -0,0 +1,91 @@
+import { test, expect } from '@playwright/test'
+import fs from 'node:fs'
+import path from 'node:path'
+import { axios, anonymousAx, axiosAuth, apiUrl, clean } from '../../support/axios.ts'
+import { publishFixturePlugin } from '../../support/registry.ts'
+
+// identity webhooks are internal calls: simple-directory reaches the API directly, not through the proxy
+const axIdentities = axios({ baseURL: apiUrl, headers: { 'x-secret-key': 'secret-identities' } })
+const rawProcessing = async (id: string) => (await anonymousAx.get(`${apiUrl}/api/v1/test-env/raw-processing/${id}`, { validateStatus: () => true }))
+const processingsDir = path.resolve(import.meta.dirname, '../../../data/development/processings')
+
+const seedProcessing = async () => {
+  const plugin = await publishFixturePlugin({ name: '@data-fair/processing-hello-world', version: '1.2.2' })
+  const admin = await axiosAuth({ email: 'test_admin1@test.com', org: 'test_org1' })
+  const processing = (await admin.post('/api/v1/processings', { title: 'Identities processing', plugin: plugin.pluginId })).data
+  await admin.patch(`/api/v1/processings/${processing._id}`, {
+    permissions: [
+      { profile: 'read', target: { type: 'partner', organization: { name: 'Test Org 2', id: 'test_org2' }, roles: ['admin'] } },
+      { profile: 'read', target: { type: 'partner', organization: { name: 'Test Org 3', id: 'test_org3' }, roles: ['admin'] } },
+      { profile: 'read', target: { type: 'userEmail', email: 'test_alone@test.com' } }
+    ]
+  })
+  return processing
+}
+
+test.describe('identity webhooks', () => {
+  test.beforeEach(clean)
+  test.afterAll(clean)
+
+  test('should follow renames and the end of a partnership', async () => {
+    const processing = await seedProcessing()
+
+    await axIdentities.post('/api/identities/user/test_admin1', { name: 'Renamed Admin', organizations: [{ id: 'test_org1', role: 'admin' }] })
+    let raw = (await rawProcessing(processing._id)).data
+    expect(raw.created.name).toBe('Renamed Admin')
+    expect(raw.updated.name).toBe('Renamed Admin')
+
+    await axIdentities.post('/api/identities/organization/test_org2', { name: 'Renamed Org 2' })
+    raw = (await rawProcessing(processing._id)).data
+    expect(raw.permissions[0].target.organization.name).toBe('Renamed Org 2')
+
+    // test_org3 is no longer a partner of the owner, the user email permission is not concerned
+    await axIdentities.post('/api/identities/organization/test_org1', { name: 'Test Org 1', partners: [{ id: 'test_org2', name: 'Renamed Org 2' }] })
+    raw = (await rawProcessing(processing._id)).data
+    expect(raw.permissions.map((p: any) => p.target.type + ':' + (p.target.organization?.id ?? p.target.email))).toEqual(['partner:test_org2', 'userEmail:test_alone@test.com'])
+  })
+
+  test('should rename a department and forget the name of a deleted one', async () => {
+    const plugin = await publishFixturePlugin({ name: '@data-fair/processing-hello-world', version: '1.2.2' })
+    const depAdmin = await axiosAuth({ email: 'test_dep_admin@test.com', org: 'test_org1', dep: 'dep1' })
+    const processing = (await depAdmin.post('/api/v1/processings', { title: 'Department processing', plugin: plugin.pluginId })).data
+    expect(processing.owner.department).toBe('dep1')
+
+    await axIdentities.post('/api/identities/organization/test_org1', { name: 'Test Org 1', departments: [{ id: 'dep1', name: 'Renamed Department' }] })
+    let raw = (await rawProcessing(processing._id)).data
+    expect(raw.owner.departmentName).toBe('Renamed Department')
+
+    // dep1 is missing from the complete list of departments: it was deleted, only its id remains
+    await axIdentities.post('/api/identities/organization/test_org1', { name: 'Test Org 1', departments: [{ id: 'dep2', name: 'Department 2' }] })
+    raw = (await rawProcessing(processing._id)).data
+    expect(raw.owner.department).toBe('dep1')
+    expect(raw.owner.departmentName).toBeUndefined()
+  })
+
+  test('should keep only the id of a deleted user and drop a deleted partner', async () => {
+    const processing = await seedProcessing()
+
+    await axIdentities.delete('/api/identities/user/test_admin1')
+    let raw = (await rawProcessing(processing._id)).data
+    expect(raw.created.name).toBeUndefined()
+    expect(raw.created.id).toBe('test_admin1')
+    expect(raw.updated.name).toBeUndefined()
+
+    await axIdentities.delete('/api/identities/organization/test_org2')
+    raw = (await rawProcessing(processing._id)).data
+    expect(raw.permissions.map((p: any) => p.target.organization?.id ?? p.target.email)).toEqual(['test_org3', 'test_alone@test.com'])
+  })
+
+  test('should delete everything a deleted organization owns, its directory included', async () => {
+    const processing = await seedProcessing()
+    const superadmin = await axiosAuth({ email: 'test_superadmin@test.com', adminMode: true })
+    await superadmin.post('/api/v1/limits/organization/test_org1', { lastUpdate: new Date().toISOString(), processings_seconds: { limit: 100 } })
+    fs.mkdirSync(path.join(processingsDir, processing._id), { recursive: true })
+    fs.writeFileSync(path.join(processingsDir, processing._id, 'log.txt'), 'kept between runs')
+
+    await axIdentities.delete('/api/identities/organization/test_org1')
+    expect((await rawProcessing(processing._id)).status).toBe(404)
+    expect(fs.existsSync(path.join(processingsDir, processing._id))).toBe(false)
+    expect((await superadmin.get('/api/v1/limits', { params: { type: 'organization', id: 'test_org1' } })).data.results).toHaveLength(0)
+  })
+})

ui/package.json

@@ -18,8 +18,8 @@
   "dependencies": {
     "@data-fair/frame": "^0.18.4",
     "@data-fair/lib-utils": "^1.13.1",
-    "@data-fair/lib-vue": "^1.30.1",
-    "@data-fair/lib-vuetify": "^2.4.3",
+    "@data-fair/lib-vue": "^1.31.1",
+    "@data-fair/lib-vuetify": "^2.5.1",
     "@data-fair/processings-shared": "*",
     "@intlify/unplugin-vue-i18n": "^11.0.7",
     "@koumoul/vjsf": "^4.2.0",

ui/src/components/processing/processing-activity.vue

@@ -7,13 +7,13 @@
   <v-list-item
     v-if="processing.updated"
     :prepend-icon="mdiPencil"
-    :title="processing.updated.name"
+    :title="processing.updated.name ?? t('formerUser')"
     :subtitle="dayjs(processing.updated.date).format('D MMM YYYY à HH:mm')"
   />
   <v-list-item
     v-if="processing.created"
     :prepend-icon="mdiPlusCircleOutline"
-    :title="processing.created.name"
+    :title="processing.created.name ?? t('formerUser')"
     :subtitle="dayjs(processing.created.date).format('D MMM YYYY à HH:mm')"
   />
   <v-list-item
@@ -25,6 +25,8 @@
 <script setup lang="ts">
 import type { Processing } from '#api/types'
 
+const { t } = useI18n()
+const { departmentLabel } = useDisplayOwner()
 const { dayjs } = useLocaleDayjs()
 
 const { processing, pluginTitle } = defineProps<{
@@ -35,7 +37,7 @@ const { processing, pluginTitle } = defineProps<{
 const ownerName = computed(() => {
   if (!processing.owner) return ''
   const baseName = processing.owner.name || processing.owner.id
-  const departmentInfo = processing.owner.departmentName || processing.owner.department
+  const departmentInfo = departmentLabel(processing.owner.department, processing.owner.departmentName)
   return departmentInfo
     ? `${baseName} - ${departmentInfo}`
     : baseName
@@ -47,5 +49,12 @@ const avatarUrl = computed(() => {
 
 </script>
 
+<i18n lang="yaml">
+  en:
+    formerUser: Former user
+  fr:
+    formerUser: Ancien utilisateur
+</i18n>
+
 <style scoped>
 </style>

ui/src/components/processings-actions.vue

@@ -103,6 +103,7 @@ import SearchField from '@data-fair/lib-vuetify/search-field.vue'
 import '@data-fair/frame/lib/d-frame.js'
 
 const { t } = useI18n()
+const { departmentLabel } = useDisplayOwner()
 const router = useRouter()
 const session = useSessionAuthenticated()
 const processingsProps = defineProps<{
@@ -206,7 +207,7 @@ const ownersItems = computed(() => {
         owner.departments.forEach(department => {
           // Ajout d'un élément pour chaque département
           items.push({
-            display: `${owner.name} - ${department.departmentName || department.department} (${department.count})`,
+            display: `${owner.name} - ${departmentLabel(department.department, department.departmentName)} (${department.count})`,
             ownerKey: `organization:${owner.id}:${department.department}`
           })
         })