Support secrets management by Manager role (#3801)

* Add allow_managers_manage_secrets server setting

* Fix new clients with old servers

* Update frontend

* Fix tests

* Fix tests

Author: r4victor

frontend/src/pages/Project/Secrets/index.tsx

@@ -3,19 +3,19 @@ import { useTranslation } from 'react-i18next';
 
 import { Button, ButtonWithConfirmation, Header, ListEmptyMessage, Modal, Pagination, SpaceBetween, Table } from 'components';
 
-import { useAppSelector, useCollection, useNotifications, usePermissionGuard } from 'hooks';
+import { useAppSelector, useCollection, useNotifications } from 'hooks';
 import { getServerError } from 'libs';
 import {
     useDeleteSecretsMutation,
     useGetAllSecretsQuery,
     useLazyGetSecretQuery,
     useUpdateSecretMutation,
 } from 'services/secrets';
-import { GlobalUserRole, ProjectUserRole } from 'types';
+import { GlobalUserRole } from 'types';
 
 import { selectUserData } from 'App/slice';
 
-import { getProjectRoleByUserName } from '../utils';
+import { getMemberCanManageSecrets } from '../utils';
 import { SecretForm } from './Form';
 
 import { IProps, TFormValues } from './types';
@@ -30,11 +30,8 @@ export const ProjectSecrets: React.FC<IProps> = ({ project, loading }) => {
     const projectName = project?.project_name ?? '';
     const [pushNotification] = useNotifications();
 
-    const [hasPermissionForSecretsManaging] = usePermissionGuard({
-        allowedProjectRoles: [ProjectUserRole.ADMIN],
-        allowedGlobalRoles: [GlobalUserRole.ADMIN],
-        projectRole: project ? (getProjectRoleByUserName(project, userName) ?? undefined) : undefined,
-    });
+    const hasPermissionForSecretsManaging =
+        userData?.global_role === GlobalUserRole.ADMIN || (project ? getMemberCanManageSecrets(project, userName) : false);
 
     const { data, isLoading, isFetching } = useGetAllSecretsQuery(
         { project_name: projectName },

frontend/src/pages/Project/utils.ts

@@ -4,3 +4,8 @@ export const getProjectRoleByUserName = (
 ): TProjectRole | null => {
     return project.members.find((m) => m.user.username === userName)?.project_role ?? null;
 };
+
+export const getMemberCanManageSecrets = (project: IProject, userName: IProjectMember['user']['username']): boolean => {
+    const member = project.members.find((m) => m.user.username === userName);
+    return member?.permissions?.can_manage_secrets ?? false;
+};

frontend/src/types/project.d.ts

@@ -33,9 +33,15 @@ declare interface IProject {
     templates_repo?: string | null;
 }
 
+declare interface IProjectMemberPermissions {
+    can_manage_ssh_fleets: boolean;
+    can_manage_secrets: boolean;
+}
+
 declare interface IProjectMember {
     project_role: TProjectRole;
     user: IUser | { username: string };
+    permissions?: IProjectMemberPermissions;
 }
 
 declare type TSetProjectMembersParams = {

src/dstack/_internal/core/models/projects.py

@@ -10,6 +10,9 @@
 
 class MemberPermissions(CoreModel):
     can_manage_ssh_fleets: bool
+    can_manage_secrets: bool = False
+    """Default is for client-side compatibility with older servers.
+    Always explicitly set on the server."""
 
 
 class Member(CoreModel):

src/dstack/_internal/server/routers/secrets.py

@@ -12,7 +12,7 @@
     DeleteSecretsRequest,
     GetSecretRequest,
 )
-from dstack._internal.server.security.permissions import ProjectAdmin
+from dstack._internal.server.security.permissions import ProjectManager
 from dstack._internal.server.services import secrets as secrets_services
 from dstack._internal.server.utils.routers import CustomORJSONResponse
 
@@ -25,13 +25,14 @@
 @router.post("/list", response_model=List[Secret])
 async def list_secrets(
     session: AsyncSession = Depends(get_session),
-    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectAdmin()),
+    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManager()),
 ):
-    _, project = user_project
+    user, project = user_project
     return CustomORJSONResponse(
         await secrets_services.list_secrets(
             session=session,
             project=project,
+            user=user,
         )
     )
 
@@ -40,13 +41,14 @@ async def list_secrets(
 async def get_secret(
     body: GetSecretRequest,
     session: AsyncSession = Depends(get_session),
-    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectAdmin()),
+    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManager()),
 ):
-    _, project = user_project
+    user, project = user_project
     secret = await secrets_services.get_secret(
         session=session,
         project=project,
         name=body.name,
+        user=user,
     )
     if secret is None:
         raise ResourceNotExistsError()
@@ -57,7 +59,7 @@ async def get_secret(
 async def create_or_update_secret(
     body: CreateOrUpdateSecretRequest,
     session: AsyncSession = Depends(get_session),
-    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectAdmin()),
+    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManager()),
 ):
     user, project = user_project
     return CustomORJSONResponse(
@@ -75,7 +77,7 @@ async def create_or_update_secret(
 async def delete_secrets(
     body: DeleteSecretsRequest,
     session: AsyncSession = Depends(get_session),
-    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectAdmin()),
+    user_project: Tuple[UserModel, ProjectModel] = Depends(ProjectManager()),
 ):
     user, project = user_project
     await secrets_services.delete_secrets(

src/dstack/_internal/server/services/permissions.py

@@ -23,6 +23,12 @@ class DefaultPermissions(CoreModel):
             )
         ),
     ] = True
+    allow_managers_manage_secrets: Annotated[
+        bool,
+        Field(
+            description=("This flag controls whether project managers can manage project secrets")
+        ),
+    ] = False
 
 
 _default_permissions = DefaultPermissions()

src/dstack/_internal/server/services/projects.py

@@ -716,8 +716,17 @@ def get_member_permissions(member_model: MemberModel) -> MemberPermissions:
             and member_model.project_role != ProjectRole.ADMIN
         ):
             can_manage_ssh_fleets = False
+    can_manage_secrets = (
+        user_model.global_role == GlobalRole.ADMIN
+        or member_model.project_role == ProjectRole.ADMIN
+        or (
+            member_model.project_role == ProjectRole.MANAGER
+            and default_permissions.allow_managers_manage_secrets
+        )
+    )
     return MemberPermissions(
         can_manage_ssh_fleets=can_manage_ssh_fleets,
+        can_manage_secrets=can_manage_secrets,
     )
 
 

src/dstack/_internal/server/services/secrets.py

@@ -9,15 +9,18 @@
 from sqlalchemy.ext.asyncio import AsyncSession
 
 from dstack._internal.core.errors import (
+    ForbiddenError,
     ResourceExistsError,
     ResourceNotExistsError,
     ServerClientError,
 )
 from dstack._internal.core.models.secrets import Secret
+from dstack._internal.core.models.users import GlobalRole
 from dstack._internal.server.db import get_db
 from dstack._internal.server.models import DecryptedString, ProjectModel, SecretModel, UserModel
 from dstack._internal.server.services import events
 from dstack._internal.server.services.locking import get_locker
+from dstack._internal.server.services.projects import get_member, get_member_permissions
 
 _SECRET_NAME_REGEX = "^[A-Za-z0-9-_]{1,200}$"
 _SECRET_VALUE_MAX_LENGTH = 5000
@@ -26,7 +29,9 @@
 async def list_secrets(
     session: AsyncSession,
     project: ProjectModel,
+    user: UserModel,
 ) -> List[Secret]:
+    _check_can_manage_secrets(user=user, project=project)
     secret_models = await list_project_secret_models(session=session, project=project)
     return [secret_model_to_secret(s, include_value=False) for s in secret_models]
 
@@ -43,7 +48,9 @@ async def get_secret(
     session: AsyncSession,
     project: ProjectModel,
     name: str,
+    user: UserModel,
 ) -> Optional[Secret]:
+    _check_can_manage_secrets(user=user, project=project)
     secret_model = await get_project_secret_model_by_name(
         session=session,
         project=project,
@@ -61,6 +68,7 @@ async def create_or_update_secret(
     value: str,
     user: UserModel,
 ) -> Secret:
+    _check_can_manage_secrets(user=user, project=project)
     _validate_secret(name=name, value=value)
     try:
         secret_model = await create_secret(
@@ -87,6 +95,7 @@ async def delete_secrets(
     names: List[str],
     user: UserModel,
 ):
+    _check_can_manage_secrets(user=user, project=project)
     async with get_project_secret_models_by_name_for_update(
         session=session, project=project, names=names
     ) as secret_models:
@@ -243,3 +252,15 @@ def _validate_secret_name(name: str):
 def _validate_secret_value(value: str):
     if len(value) > _SECRET_VALUE_MAX_LENGTH:
         raise ServerClientError(f"Secret value length must not exceed {_SECRET_VALUE_MAX_LENGTH}")
+
+
+def _check_can_manage_secrets(user: UserModel, project: ProjectModel):
+    if user.global_role == GlobalRole.ADMIN:
+        return
+    member = get_member(user=user, project=project)
+    if member is None:
+        raise ForbiddenError()
+    permissions = get_member_permissions(member)
+    if permissions.can_manage_secrets:
+        return
+    raise ForbiddenError()

src/tests/_internal/server/routers/test_projects.py

@@ -976,6 +976,7 @@ async def test_creates_project(self, test_db, session: AsyncSession, client: Asy
                     "project_role": ProjectRole.ADMIN,
                     "permissions": {
                         "can_manage_ssh_fleets": True,
+                        "can_manage_secrets": True,
                     },
                 }
             ],
@@ -1439,6 +1440,7 @@ async def test_returns_project(self, test_db, session: AsyncSession, client: Asy
                     "project_role": ProjectRole.ADMIN,
                     "permissions": {
                         "can_manage_ssh_fleets": True,
+                        "can_manage_secrets": True,
                     },
                 }
             ],
@@ -1669,6 +1671,7 @@ async def test_sets_project_members(self, test_db, session: AsyncSession, client
                 "project_role": ProjectRole.ADMIN,
                 "permissions": {
                     "can_manage_ssh_fleets": True,
+                    "can_manage_secrets": True,
                 },
             },
             {
@@ -1687,6 +1690,7 @@ async def test_sets_project_members(self, test_db, session: AsyncSession, client
                 "project_role": ProjectRole.ADMIN,
                 "permissions": {
                     "can_manage_ssh_fleets": True,
+                    "can_manage_secrets": True,
                 },
             },
             {
@@ -1705,6 +1709,7 @@ async def test_sets_project_members(self, test_db, session: AsyncSession, client
                 "project_role": ProjectRole.USER,
                 "permissions": {
                     "can_manage_ssh_fleets": True,
+                    "can_manage_secrets": True,
                 },
             },
         ]
@@ -1762,6 +1767,7 @@ async def test_sets_project_members_by_email(
                 "project_role": ProjectRole.ADMIN,
                 "permissions": {
                     "can_manage_ssh_fleets": True,
+                    "can_manage_secrets": True,
                 },
             },
         ]