Update OIDC advanced permissions check to only perform an update on changes

- Update permissions example to use UUIDv4 strings for allowedLibraries
- More validation on advanced permission JSON to ensure arrays are array of strings
- Only set allowedTags and allowedLibraries if the corresponding access all permission is false

server/Auth.js

@@ -100,24 +100,24 @@ class Auth {
     }, async (tokenset, userinfo, done) => {
       try {
         Logger.debug(`[Auth] openid callback userinfo=`, JSON.stringify(userinfo, null, 2))
-    
+
         if (!userinfo.sub) {
           throw new Error('Invalid userinfo, no sub')
         }
-    
+
         if (!this.validateGroupClaim(userinfo)) {
           throw new Error(`Group claim ${Database.serverSettings.authOpenIDGroupClaim} not found or empty in userinfo`)
         }
-    
+
         let user = await this.findOrCreateUser(userinfo)
-    
+
-        if (!user || !user.isActive) {
+        if (!user?.isActive) {
           throw new Error('User not active or not found')
         }
-    
+
         await this.setUserGroup(user, userinfo)
         await this.updateUserPermissions(user, userinfo)
-    
+
         // We also have to save the id_token for later (used for logout) because we cannot set cookies here
         user.openid_id_token = tokenset.id_token
 
@@ -229,62 +229,68 @@ class Auth {
     return true
   }
 
-/**
+  /**
- * Sets the user group based on group claim in userinfo.
+   * Sets the user group based on group claim in userinfo.
- */
+   * 
-async setUserGroup(user, userinfo) {
+   * @param {import('./objects/user/User')} user
-  const groupClaimName = Database.serverSettings.authOpenIDGroupClaim
+   * @param {Object} userinfo
-  if (!groupClaimName) // No group claim configured, don't set anything
+   */
-    return
+  async setUserGroup(user, userinfo) {
+    const groupClaimName = Database.serverSettings.authOpenIDGroupClaim
+    if (!groupClaimName) // No group claim configured, don't set anything
+      return
 
-  if (!userinfo[groupClaimName])
+    if (!userinfo[groupClaimName])
-    throw new Error(`Group claim ${groupClaimName} not found in userinfo`)
+      throw new Error(`Group claim ${groupClaimName} not found in userinfo`)
 
-  const groupsList = userinfo[groupClaimName].map(group => group.toLowerCase())
+    const groupsList = userinfo[groupClaimName].map(group => group.toLowerCase())
-  const rolesInOrderOfPriority = ['admin', 'user', 'guest']
+    const rolesInOrderOfPriority = ['admin', 'user', 'guest']
 
-  let userType = rolesInOrderOfPriority.find(role => groupsList.includes(role))
+    let userType = rolesInOrderOfPriority.find(role => groupsList.includes(role))
-  if (userType) {
+    if (userType) {
-    if (user.type === 'root') {
+      if (user.type === 'root') {
-      // Check OpenID Group
+        // Check OpenID Group
-      if (userType !== 'admin') {
+        if (userType !== 'admin') {
-        throw new Error(`Root user "${user.username}" cannot be downgraded to ${userType}. Denying login.`)
+          throw new Error(`Root user "${user.username}" cannot be downgraded to ${userType}. Denying login.`)
-      } else {
+        } else {
-        // If root user is logging in via OpenID, we will not change the type
+          // If root user is logging in via OpenID, we will not change the type
-        return
+          return
+        }
       }
+
+      if (user.type !== userType) {
+        Logger.info(`[Auth] openid callback: Updating user "${user.username}" type to "${userType}" from "${user.type}"`)
+        user.type = userType
+        await Database.userModel.updateFromOld(user)
+      }
+    } else {
+      throw new Error(`No valid group found in userinfo: ${JSON.stringify(userinfo[groupClaimName], null, 2)}`)
     }
+  }
 
-    Logger.debug(`[Auth] openid callback: Setting user ${user.username} type to ${userType}`)
+  /**
+   * Updates user permissions based on the advanced permissions claim.
+   * 
+   * @param {import('./objects/user/User')} user
+   * @param {Object} userinfo
+   */
+  async updateUserPermissions(user, userinfo) {
+    const absPermissionsClaim = Database.serverSettings.authOpenIDAdvancedPermsClaim
+    if (!absPermissionsClaim) // No advanced permissions claim configured, don't set anything
+      return
 
-    if (user.type !== userType) {
+    if (user.type === 'admin' || user.type === 'root')
-      user.type = userType
+      return
+
+    const absPermissions = userinfo[absPermissionsClaim]
+    if (!absPermissions)
+      throw new Error(`Advanced permissions claim ${absPermissionsClaim} not found in userinfo`)
+
+    if (user.updatePermissionsFromExternalJSON(absPermissions)) {
+      Logger.debug(`[Auth] openid callback: Updating advanced perms for user "${user.username}" using "${JSON.stringify(absPermissions)}"`)
       await Database.userModel.updateFromOld(user)
     }
-  } else {
-    throw new Error(`No valid group found in userinfo: ${JSON.stringify(userinfo[groupClaimName], null, 2)}`)
   }
-}
-
-/**
- * Updates user permissions based on the advanced permissions claim.
- */
-async updateUserPermissions(user, userinfo) {
-  const absPermissionsClaim = Database.serverSettings.authOpenIDAdvancedPermsClaim
-  if (!absPermissionsClaim) // No advanced permissions claim configured, don't set anything
-    return
-
-  if (user.type === 'admin' || user.type === 'root')
-    return
-
-  const absPermissions = userinfo[absPermissionsClaim]
-  if (!absPermissions)
-    throw new Error(`Advanced permissions claim ${absPermissionsClaim} not found in userinfo`)
-
-  Logger.debug(`[Auth] openid callback: Updating advanced perms for user ${user.username} to ${JSON.stringify(absPermissions)}`)
-  user.updatePermissionsFromExternalJSON(absPermissions)
-  await Database.userModel.updateFromOld(user)
-}
 
   /**
    * Unuse strategy

server/objects/user/User.js

@@ -280,64 +280,97 @@ class User {
     tagsAreDenylist: 'selectedTagsNotAccessible',
     // Direct mapping for array-based permissions
     allowedLibraries: 'librariesAccessible',
-    allowedTags: 'itemTagsSelected',
+    allowedTags: 'itemTagsSelected'
   }
 
   /**
-   * Update user from external JSON
+   * Update user permissions from external JSON
    * 
-   * @param {object} absPermissions JSON containg user permissions
+   * @param {Object} absPermissions JSON containing user permissions
+   * @returns {boolean} true if updates were made
    */
   updatePermissionsFromExternalJSON(absPermissions) {
+    let hasUpdates = false
+    let updatedUserPermissions = {}
+
     // Initialize all permissions to false first
     Object.keys(User.permissionMapping).forEach(mappingKey => {
-      const userPermKey = User.permissionMapping[mappingKey];
+      const userPermKey = User.permissionMapping[mappingKey]
       if (typeof this.permissions[userPermKey] === 'boolean') {
-        this.permissions[userPermKey] = false; // Default to false for boolean permissions
+        updatedUserPermissions[userPermKey] = false // Default to false for boolean permissions
-      } else {
-        this[userPermKey] = []; // Default to empty array for other properties
       }
-    });
+    })
 
+    // Map the boolean permissions from absPermissions
     Object.keys(absPermissions).forEach(absKey => {
       const userPermKey = User.permissionMapping[absKey]
       if (!userPermKey) {
         throw new Error(`Unexpected permission property: ${absKey}`)
       }
 
-      // Update the user's permissions based on absPermissions
+      if (updatedUserPermissions[userPermKey] !== undefined) {
-      this.permissions[userPermKey] = absPermissions[absKey]
+        updatedUserPermissions[userPermKey] = !!absPermissions[absKey]
-    });
+      }
+    })
 
-    // Handle allowedLibraries and allowedTags separately if needed
+    // Update user permissions if changes were made
-    if (absPermissions.allowedLibraries) {
+    if (JSON.stringify(this.permissions) !== JSON.stringify(updatedUserPermissions)) {
+      this.permissions = updatedUserPermissions
+      hasUpdates = true
+    }
+
+    // Handle allowedLibraries
+    if (this.permissions.accessAllLibraries) {
+      if (this.librariesAccessible.length) {
+        this.librariesAccessible = []
+        hasUpdates = true
+      }
+    } else if (absPermissions.allowedLibraries?.length && absPermissions.allowedLibraries.join(',') !== this.librariesAccessible.join(',')) {
+      if (absPermissions.allowedLibraries.some(lid => typeof lid !== 'string')) {
+        throw new Error('Invalid permission property "allowedLibraries", expecting array of strings')
+      }
       this.librariesAccessible = absPermissions.allowedLibraries
+      hasUpdates = true
     }
-    if (absPermissions.allowedTags) {
+
+    // Handle allowedTags
+    if (this.permissions.accessAllTags) {
+      if (this.itemTagsSelected.length) {
+        this.itemTagsSelected = []
+        hasUpdates = true
+      }
+    } else if (absPermissions.allowedTags?.length && absPermissions.allowedTags.join(',') !== this.itemTagsSelected.join(',')) {
+      if (absPermissions.allowedTags.some(tag => typeof tag !== 'string')) {
+        throw new Error('Invalid permission property "allowedTags", expecting array of strings')
+      }
       this.itemTagsSelected = absPermissions.allowedTags
+      hasUpdates = true
     }
+
+    return hasUpdates
   }
 
+
   /**
    * Get a sample to show how a JSON for updatePermissionsFromExternalJSON should look like 
    * 
-   * @returns JSON string
+   * @returns {string} JSON string
    */
   static getSampleAbsPermissions() {
     // Start with a template object where all permissions are false for simplicity
     const samplePermissions = Object.keys(User.permissionMapping).reduce((acc, key) => {
       // For array-based permissions, provide a sample array
       if (key === 'allowedLibraries') {
-        acc[key] = [`ExampleLibrary`, `AnotherLibrary`];
+        acc[key] = [`5406ba8a-16e1-451d-96d7-4931b0a0d966`, `918fd848-7c1d-4a02-818a-847435a879ca`]
       } else if (key === 'allowedTags') {
-        acc[key] = [`ExampleTag`, `AnotherTag`, `ThirdTag`];
+        acc[key] = [`ExampleTag`, `AnotherTag`, `ThirdTag`]
       } else {
-        acc[key] = false;
+        acc[key] = false
       }
-      return acc;
+      return acc
-    }, {});
+    }, {})
 
-    return JSON.stringify(samplePermissions, null, 2); // Pretty print the JSON
+    return JSON.stringify(samplePermissions, null, 2) // Pretty print the JSON
   }
 
   /**