feat(xo-server-auth-oidc): import user groups from OIDC (#9206)

CHANGELOG.unreleased.md

@@ -11,15 +11,16 @@
 
 > Users must be able to say: “Nice enhancement, I'm eager to test it”
 
-
 - [HUB Recipe] Support custom cluster CIDR and Xo CCM (Cloud Controller Manager) in Pyrgos recipe
 - [Disk] Add warning before disconnecting a VBD (PR [#9211](https://github.com/vatesfr/xen-orchestra/pull/9211))
+- [Plugins/OIDC] Import user groups from OIDC (PR [#9206](https://github.com/vatesfr/xen-orchestra/pull/9206))
 
 - **XO 6:**
   - [New/VM] Display unsupported features information message (PR [#9203](https://github.com/vatesfr/xen-orchestra/pull/9203))
   - [i18n] Update Czech, German, French, Italian, Dutch, Portuguese (Brazil), and Ukrainian translations, and add Danish translation (PR [#9165](https://github.com/vatesfr/xen-orchestra/pull/9165))
 
 ### Bug fixes
+
 - [V2V] fix transfer failing at 99% for unaligned disk (PR [#9233](https://github.com/vatesfr/xen-orchestra/pull/9233))
 
 ### Packages to release
@@ -40,6 +41,7 @@
 
 - @vates/nbd-client patch
 - vhd-lib patch
+- xo-server-auth-oidc minor
 - xo-web minor
 
 <!--packages-end-->

packages/xo-server-auth-oidc/index.js

@@ -1,5 +1,9 @@
 'use strict'
 
+/**
+ * @typedef {import('@vates/types').XoUser} XoUser
+ */
+
 const { join } = require('node:path/posix')
 const { Strategy } = require('passport-openidconnect')
 
@@ -118,16 +122,20 @@ class AuthOidc {
     }
 
     this.#unregisterPassportStrategy = xo.registerPassportStrategy(
-      new Strategy(conf, async (issuer, profile, done) => {
+      new Strategy(conf, async (issuer, profile, context, idToken, done) => {
         try {
+          const claims = JSON.parse(Buffer.from(idToken.split('.')[1], 'base64').toString())
+          const groups = claims.groups ? claims.groups : []
+
           // See https://github.com/jaredhanson/passport-openidconnect/blob/master/lib/profile.js
           const { id } = profile
-          done(
-            null,
-            await xo.registerUser2('oidc:' + issuer, {
-              user: { id, name: usernameField === 'email' ? profile.emails[0].value : profile[usernameField] },
-            })
-          )
+          const user = await xo.registerUser2('oidc:' + issuer, {
+            user: { id, name: usernameField === 'email' ? profile.emails[0].value : profile[usernameField] },
+          })
+
+          await this._synchronizeGroups(user, groups)
+
+          done(null, user)
         } catch (error) {
           done(error.message)
         }
@@ -136,6 +144,43 @@ class AuthOidc {
     )
   }
 
+  /**
+   * Synchronize user's groups.
+   * Not private in order to be testable, but should be private.
+   *
+   * @param {XoUser} user
+   * @param {string[]} oidcGroups
+   *
+   * @returns {Promise<void>}
+   */
+  async _synchronizeGroups(user, oidcGroups) {
+    const xoGroups = await this.#xo.getAllGroups()
+
+    for (const xoGroup of xoGroups) {
+      // If the user is in a XO group that he is not a part of in OIDC, we remove him.
+      if (xoGroup.provider === 'oidc' && xoGroup.users.includes(user.id) && !oidcGroups.includes(xoGroup.name)) {
+        await this.#xo.removeUserFromGroup(user.id, xoGroup.id)
+      }
+    }
+
+    for (const oidcGroupName of oidcGroups) {
+      // Try to find the OIDC group in the XO groups by name.
+      let xoGroup = xoGroups.find(group => group.provider === 'oidc' && group.name === oidcGroupName)
+      if (xoGroup === undefined) {
+        // If the OIDC group does not exist we create it.
+        xoGroup = await this.#xo.createGroup({
+          name: oidcGroupName,
+          provider: 'oidc',
+        })
+      }
+
+      // If the user is not part of the group, add him.
+      if (xoGroup.users.find(xoGroupUser => xoGroupUser === user.id) === undefined) {
+        await this.#xo.addUserToGroup(user.id, xoGroup.id)
+      }
+    }
+  }
+
   unload() {
     this.#unregisterPassportStrategy()
   }

packages/xo-server-auth-oidc/index.test.js

@@ -0,0 +1,117 @@
+'use strict'
+
+const { beforeEach, describe, it, mock } = require('node:test')
+const assert = require('node:assert/strict')
+
+let mockData
+let mockXo
+let authOidc
+
+beforeEach(() => {
+  mockData = { xoGroups: [] }
+
+  mockXo = {
+    getAllGroups: mock.fn(async () => mockData.xoGroups),
+    createGroup: mock.fn(async group => {
+      const newGroup = { id: mockData.xoGroups.length + 1, users: [], ...group }
+      mockData.xoGroups.push(newGroup)
+      return newGroup
+    }),
+    addUserToGroup: mock.fn(async (userId, groupId) => {
+      const group = mockData.xoGroups.find(group => group.id === groupId)
+      if (!group) throw new Error(`Group ${groupId} not found`)
+      if (!group.users.includes(userId)) group.users.push(userId)
+    }),
+    removeUserFromGroup: mock.fn(async (userId, groupId) => {
+      const group = mockData.xoGroups.find(group => group.id === groupId)
+      if (!group) throw new Error(`Group ${groupId} not found`)
+      if (group.users.includes(userId)) group.users.splice(group.users.indexOf(userId), 1)
+    }),
+  }
+
+  authOidc = require('./index.js').default({ xo: mockXo })
+})
+
+describe('AuthOidc._synchronizeGroups', () => {
+  it('synchronizeGroups should create the user groups and add the user', async () => {
+    mockData.xoGroups = []
+    const mockUser = { id: 'id1' }
+    const mockOidcGroups = ['group-1', 'group-2']
+
+    await authOidc._synchronizeGroups(mockUser, mockOidcGroups)
+
+    // We created 2 groups and added 1 user to each.
+    assert.equal(mockXo.createGroup.mock.calls.length, 2)
+    assert.equal(mockXo.addUserToGroup.mock.calls.length, 2)
+    assert.equal(mockXo.removeUserFromGroup.mock.calls.length, 0)
+
+    assert.deepEqual(mockData.xoGroups, [
+      { id: 1, users: ['id1'], name: 'group-1', provider: 'oidc' },
+      { id: 2, users: ['id1'], name: 'group-2', provider: 'oidc' },
+    ])
+  })
+
+  it('synchronizeGroups should not recreate existing groups but still add the user', async () => {
+    mockData.xoGroups = [
+      { id: 1, users: [], name: 'group-1', provider: 'oidc' },
+      { id: 2, users: [], name: 'group-2', provider: 'oidc' },
+    ]
+    const mockUser = { id: 'id1' }
+    const mockOidcGroups = ['group-1', 'group-2']
+
+    await authOidc._synchronizeGroups(mockUser, mockOidcGroups)
+
+    // We added 1 user to each group.
+    assert.equal(mockXo.createGroup.mock.calls.length, 0)
+    assert.equal(mockXo.addUserToGroup.mock.calls.length, 2)
+    assert.equal(mockXo.removeUserFromGroup.mock.calls.length, 0)
+
+    assert.deepEqual(mockData.xoGroups, [
+      { id: 1, users: ['id1'], name: 'group-1', provider: 'oidc' },
+      { id: 2, users: ['id1'], name: 'group-2', provider: 'oidc' },
+    ])
+  })
+
+  it('synchronizeGroups should check the provider of the existing groups', async () => {
+    mockData.xoGroups = [{ id: 1, users: [], name: 'group-1', provider: 'ldap' }]
+    const mockUser = { id: 'id1' }
+    const mockOidcGroups = ['group-1']
+
+    await authOidc._synchronizeGroups(mockUser, mockOidcGroups)
+
+    // We created 1 group and added 1 user to it.
+    assert.equal(mockXo.createGroup.mock.calls.length, 1)
+    assert.equal(mockXo.addUserToGroup.mock.calls.length, 1)
+    assert.equal(mockXo.removeUserFromGroup.mock.calls.length, 0)
+
+    assert.deepEqual(mockData.xoGroups, [
+      { id: 1, users: [], name: 'group-1', provider: 'ldap' },
+      { id: 2, users: ['id1'], name: 'group-1', provider: 'oidc' },
+    ])
+  })
+
+  it('synchronizeGroups should remove user from groups he is not a part of', async () => {
+    mockData.xoGroups = [
+      { id: 1, users: ['id1'], name: 'group-1', provider: 'oidc' },
+      { id: 2, users: [], name: 'group-2', provider: 'oidc' },
+      { id: 3, users: [], name: 'group-3', provider: 'oidc' },
+      { id: 4, users: ['id1'], name: 'group-4', provider: 'ldap' },
+    ]
+    const mockUser = { id: 'id1' }
+    const mockOidcGroups = ['group-2']
+
+    await authOidc._synchronizeGroups(mockUser, mockOidcGroups)
+
+    // We removed the user from 1 group and added him to 1 group.
+    assert.equal(mockXo.createGroup.mock.calls.length, 0)
+    assert.equal(mockXo.addUserToGroup.mock.calls.length, 1)
+    assert.equal(mockXo.removeUserFromGroup.mock.calls.length, 1)
+
+    assert.deepEqual(mockData.xoGroups, [
+      { id: 1, users: [], name: 'group-1', provider: 'oidc' },
+      { id: 2, users: ['id1'], name: 'group-2', provider: 'oidc' },
+      { id: 3, users: [], name: 'group-3', provider: 'oidc' },
+      { id: 4, users: ['id1'], name: 'group-4', provider: 'ldap' },
+    ])
+  })
+})

packages/xo-server-auth-oidc/package.json

@@ -20,6 +20,9 @@
     "type": "git",
     "url": "https://github.com/vatesfr/xen-orchestra.git"
   },
+  "scripts": {
+    "test": "node --test"
+  },
   "author": {
     "name": "Vates SAS",
     "url": "https://vates.fr"