🔖 feat: Enhance Bookmarks UX, add RBAC, toggle via librechat.yaml (d…

…anny-avila#3747) * chore: update package version to 0.7.416 * chore: Update Role.js imports order * refactor: move updateTagsInConvo to tags route, add RBAC for tags * refactor: add updateTagsInConvoOptions * fix: loading state for bookmark form * refactor: update primaryText class in TitleButton component * refactor: remove duplicate bookmarks and theming * refactor: update EditIcon component to use React.forwardRef * refactor: add _id field to tConversationTagSchema * refactor: remove promises * refactor: move mutation logic from BookmarkForm -> BookmarkEditDialog * refactor: update button class in BookmarkForm component * fix: conversation mutations and add better logging to useConversationTagMutation * refactor: update logger message in BookmarkEditDialog component * refactor: improve UI consistency in BookmarkNav and NewChat components * refactor: update logger message in BookmarkEditDialog component * refactor: Add tags prop to BookmarkForm component * refactor: Update BookmarkForm to avoid tag mutation if the tag already exists; also close dialog on submission programmatically * refactor: general role helper function to support updating access permissions for different permission types * refactor: Update getLatestText function to handle undefined values in message.content * refactor: Update useHasAccess hook to handle null role values for authenticated users * feat: toggle bookmarks access * refactor: Update PromptsCommand to handle access permissions for prompts * feat: updateConversationSelector * refactor: rename `vars` to `tagToDelete` for clarity * fix: prevent recreation of deleted tags in BookmarkMenu on Item Click * ci: mock updateBookmarksAccess function * ci: mock updateBookmarksAccess function
kukoboris · Aug 22, 2024 · f86e9dd · f86e9dd
1 parent 366e4c5
commit f86e9dd
Show file tree

Hide file tree

Showing 39 changed files with 527 additions and 295 deletions.
diff --git a/api/models/Role.js b/api/models/Role.js
@@ -1,10 +1,11 @@
 const {
-  SystemRoles,
   CacheKeys,
+  SystemRoles,
   roleDefaults,
   PermissionTypes,
-  Permissions,
+  removeNullishValues,
   promptPermissionsSchema,
+  bookmarkPermissionsSchema,
 } = require('librechat-data-provider');
 const getLogStores = require('~/cache/getLogStores');
 const Role = require('~/models/schema/roleSchema');
@@ -69,37 +70,52 @@ const updateRoleByName = async function (roleName, updates) {
   }
 };
 
+const permissionSchemas = {
+  [PermissionTypes.PROMPTS]: promptPermissionsSchema,
+  [PermissionTypes.BOOKMARKS]: bookmarkPermissionsSchema,
+};
+
 /**
- * Updates the Prompt access for a specific role.
- * @param {SystemRoles} roleName - The role to update the prompt access for.
- * @param {boolean | undefined} [value] - The new value for the prompt access.
+ * Updates access permissions for a specific role and permission type.
+ * @param {SystemRoles} roleName - The role to update.
+ * @param {PermissionTypes} permissionType - The type of permission to update.
+ * @param {Object.<Permissions, boolean>} permissions - Permissions to update and their values.
  */
-async function updatePromptsAccess(roleName, value) {
-  if (typeof value === 'undefined') {
+async function updateAccessPermissions(roleName, permissionType, _permissions) {
+  const permissions = removeNullishValues(_permissions);
+  if (Object.keys(permissions).length === 0) {
     return;
   }
 
   try {
-    const parsedUpdates = promptPermissionsSchema.partial().parse({ [Permissions.USE]: value });
     const role = await getRoleByName(roleName);
-    if (!role) {
+    if (!role || !permissionSchemas[permissionType]) {
       return;
     }
 
-    const mergedUpdates = {
-      [PermissionTypes.PROMPTS]: {
-        ...role[PermissionTypes.PROMPTS],
-        ...parsedUpdates,
+    await updateRoleByName(roleName, {
+      [permissionType]: {
+        ...role[permissionType],
+        ...permissionSchemas[permissionType].partial().parse(permissions),
       },
-    };
+    });
 
-    await updateRoleByName(roleName, mergedUpdates);
-    logger.info(`Updated '${roleName}' role prompts 'USE' permission to: ${value}`);
+    Object.entries(permissions).forEach(([permission, value]) =>
+      logger.info(
+        `Updated '${roleName}' role ${permissionType} '${permission}' permission to: ${value}`,
+      ),
+    );
   } catch (error) {
-    logger.error('Failed to update USER role prompts USE permission:', error);
+    logger.error(`Failed to update ${roleName} role ${permissionType} permissions:`, error);
   }
 }
 
+const updatePromptsAccess = (roleName, permissions) =>
+  updateAccessPermissions(roleName, PermissionTypes.PROMPTS, permissions);
+
+const updateBookmarksAccess = (roleName, permissions) =>
+  updateAccessPermissions(roleName, PermissionTypes.BOOKMARKS, permissions);
+
 /**
  * Initialize default roles in the system.
  * Creates the default roles (ADMIN, USER) if they don't exist in the database.
@@ -123,4 +139,5 @@ module.exports = {
   initializeRoles,
   updateRoleByName,
   updatePromptsAccess,
+  updateBookmarksAccess,
 };
diff --git a/api/models/schema/roleSchema.js b/api/models/schema/roleSchema.js
@@ -8,6 +8,12 @@ const roleSchema = new mongoose.Schema({
     unique: true,
     index: true,
   },
+  [PermissionTypes.BOOKMARKS]: {
+    [Permissions.USE]: {
+      type: Boolean,
+      default: true,
+    },
+  },
   [PermissionTypes.PROMPTS]: {
     [Permissions.SHARED_GLOBAL]: {
       type: Boolean,

diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js
@@ -8,7 +8,6 @@ const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
 const { forkConversation } = require('~/server/utils/import/fork');
 const { importConversations } = require('~/server/utils/import');
 const { createImportLimiters } = require('~/server/middleware');
-const { updateTagsForConversation } = require('~/models/ConversationTag');
 const getLogStores = require('~/cache/getLogStores');
 const { sleep } = require('~/server/utils');
 const { logger } = require('~/config');
@@ -174,18 +173,4 @@ router.post('/fork', async (req, res) => {
   }
 });
 
-router.put('/tags/:conversationId', async (req, res) => {
-  try {
-    const conversationTags = await updateTagsForConversation(
-      req.user.id,
-      req.params.conversationId,
-      req.body.tags,
-    );
-    res.status(200).json(conversationTags);
-  } catch (error) {
-    logger.error('Error updating conversation tags', error);
-    res.status(500).send('Error updating conversation tags');
-  }
-});
-
 module.exports = router;
diff --git a/api/server/routes/tags.js b/api/server/routes/tags.js
@@ -1,13 +1,21 @@
 const express = require('express');
+const { PermissionTypes, Permissions } = require('librechat-data-provider');
 const {
   getConversationTags,
   updateConversationTag,
   createConversationTag,
   deleteConversationTag,
+  updateTagsForConversation,
 } = require('~/models/ConversationTag');
-const requireJwtAuth = require('~/server/middleware/requireJwtAuth');
+const { requireJwtAuth, generateCheckAccess } = require('~/server/middleware');
+const { logger } = require('~/config');
+
 const router = express.Router();
+
+const checkBookmarkAccess = generateCheckAccess(PermissionTypes.BOOKMARKS, [Permissions.USE]);
+
 router.use(requireJwtAuth);
+router.use(checkBookmarkAccess);
 
 /**
  * GET /
@@ -24,7 +32,7 @@ router.get('/', async (req, res) => {
       res.status(404).end();
     }
   } catch (error) {
-    console.error('Error getting conversation tags:', error);
+    logger.error('Error getting conversation tags:', error);
     res.status(500).json({ error: 'Internal server error' });
   }
 });
@@ -40,7 +48,7 @@ router.post('/', async (req, res) => {
     const tag = await createConversationTag(req.user.id, req.body);
     res.status(200).json(tag);
   } catch (error) {
-    console.error('Error creating conversation tag:', error);
+    logger.error('Error creating conversation tag:', error);
     res.status(500).json({ error: 'Internal server error' });
   }
 });
@@ -60,7 +68,7 @@ router.put('/:tag', async (req, res) => {
       res.status(404).json({ error: 'Tag not found' });
     }
   } catch (error) {
-    console.error('Error updating conversation tag:', error);
+    logger.error('Error updating conversation tag:', error);
     res.status(500).json({ error: 'Internal server error' });
   }
 });
@@ -80,9 +88,29 @@ router.delete('/:tag', async (req, res) => {
       res.status(404).json({ error: 'Tag not found' });
     }
   } catch (error) {
-    console.error('Error deleting conversation tag:', error);
+    logger.error('Error deleting conversation tag:', error);
     res.status(500).json({ error: 'Internal server error' });
   }
 });
 
+/**
+ * PUT /convo/:conversationId
+ * Updates the tags for a conversation.
+ * @param {Object} req - Express request object
+ * @param {Object} res - Express response object
+ */
+router.put('/convo/:conversationId', async (req, res) => {
+  try {
+    const conversationTags = await updateTagsForConversation(
+      req.user.id,
+      req.params.conversationId,
+      req.body.tags,
+    );
+    res.status(200).json(conversationTags);
+  } catch (error) {
+    logger.error('Error updating conversation tags', error);
+    res.status(500).send('Error updating conversation tags');
+  }
+});
+
 module.exports = router;
diff --git a/api/server/services/AppService.spec.js b/api/server/services/AppService.spec.js
@@ -24,6 +24,7 @@ jest.mock('./Files/Firebase/initialize', () => ({
 jest.mock('~/models/Role', () => ({
   initializeRoles: jest.fn(),
   updatePromptsAccess: jest.fn(),
+  updateBookmarksAccess: jest.fn(),
 }));
 jest.mock('./ToolService', () => ({
   loadAndFormatTools: jest.fn().mockReturnValue({

diff --git a/api/server/services/start/interface.js b/api/server/services/start/interface.js
@@ -1,5 +1,5 @@
-const { SystemRoles, removeNullishValues } = require('librechat-data-provider');
-const { updatePromptsAccess } = require('~/models/Role');
+const { SystemRoles, Permissions, removeNullishValues } = require('librechat-data-provider');
+const { updatePromptsAccess, updateBookmarksAccess } = require('~/models/Role');
 const { logger } = require('~/config');
 
 /**
@@ -24,10 +24,12 @@ async function loadDefaultInterface(config, configDefaults, roleName = SystemRol
     sidePanel: interfaceConfig?.sidePanel ?? defaults.sidePanel,
     privacyPolicy: interfaceConfig?.privacyPolicy ?? defaults.privacyPolicy,
     termsOfService: interfaceConfig?.termsOfService ?? defaults.termsOfService,
+    bookmarks: interfaceConfig?.bookmarks ?? defaults.bookmarks,
     prompts: interfaceConfig?.prompts ?? defaults.prompts,
   });
 
-  await updatePromptsAccess(roleName, loadedInterface.prompts);
+  await updatePromptsAccess(roleName, { [Permissions.USE]: loadedInterface.prompts });
+  await updateBookmarksAccess(roleName, { [Permissions.USE]: loadedInterface.bookmarks });
 
   let i = 0;
   const logSettings = () => {

diff --git a/api/server/services/start/interface.spec.js b/api/server/services/start/interface.spec.js
@@ -1,9 +1,10 @@
-const { SystemRoles } = require('librechat-data-provider');
+const { SystemRoles, Permissions } = require('librechat-data-provider');
 const { updatePromptsAccess } = require('~/models/Role');
 const { loadDefaultInterface } = require('./interface');
 
 jest.mock('~/models/Role', () => ({
   updatePromptsAccess: jest.fn(),
+  updateBookmarksAccess: jest.fn(),
 }));
 
 describe('loadDefaultInterface', () => {
@@ -13,7 +14,7 @@ describe('loadDefaultInterface', () => {
 
     await loadDefaultInterface(config, configDefaults);
 
-    expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, true);
+    expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, { [Permissions.USE]: true });
   });
 
   it('should call updatePromptsAccess with false when prompts is false', async () => {
@@ -22,7 +23,9 @@ describe('loadDefaultInterface', () => {
 
     await loadDefaultInterface(config, configDefaults);
 
-    expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, false);
+    expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, {
+      [Permissions.USE]: false,
+    });
   });
 
   it('should call updatePromptsAccess with undefined when prompts is not specified in config', async () => {
@@ -31,7 +34,9 @@ describe('loadDefaultInterface', () => {
 
     await loadDefaultInterface(config, configDefaults);
 
-    expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, undefined);
+    expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, {
+      [Permissions.USE]: undefined,
+    });
   });
 
   it('should call updatePromptsAccess with undefined when prompts is explicitly undefined', async () => {
@@ -40,6 +45,8 @@ describe('loadDefaultInterface', () => {
 
     await loadDefaultInterface(config, configDefaults);
 
-    expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, undefined);
+    expect(updatePromptsAccess).toHaveBeenCalledWith(SystemRoles.USER, {
+      [Permissions.USE]: undefined,
+    });
   });
 });