feat(authz): update integration tests

frontend/src/components/ServiceAccountDrawer/OverviewTab.tsx

@@ -16,8 +16,8 @@ interface OverviewTabProps {
 	account: ServiceAccountRow;
 	localName: string;
 	onNameChange: (v: string) => void;
-	localRole: string;
-	onRoleChange: (v: string | undefined) => void;
+	localRoles: string[];
+	onRolesChange: (v: string[]) => void;
 	isDisabled: boolean;
 	availableRoles: AuthtypesRoleDTO[];
 	rolesLoading?: boolean;
@@ -31,8 +31,8 @@ function OverviewTab({
 	account,
 	localName,
 	onNameChange,
-	localRole,
-	onRoleChange,
+	localRoles,
+	onRolesChange,
 	isDisabled,
 	availableRoles,
 	rolesLoading,
@@ -95,10 +95,15 @@ function OverviewTab({
 				{isDisabled ? (
 					<div className="sa-drawer__input-wrapper sa-drawer__input-wrapper--disabled">
 						<div className="sa-drawer__disabled-roles">
-							{localRole ? (
-								<Badge color="vanilla">
-									{availableRoles.find((r) => r.id === localRole)?.name ?? localRole}
-								</Badge>
+							{localRoles.length > 0 ? (
+								localRoles.map((roleId) => {
+									const role = availableRoles.find((r) => r.id === roleId);
+									return (
+										<Badge key={roleId} color="vanilla">
+											{role?.name ?? roleId}
+										</Badge>
+									);
+								})
 							) : (
 								<span className="sa-drawer__input-text">—</span>
 							)}
@@ -108,14 +113,15 @@ function OverviewTab({
 				) : (
 					<RolesSelect
 						id="sa-roles"
+						mode="multiple"
 						roles={availableRoles}
 						loading={rolesLoading}
 						isError={rolesError}
 						error={rolesErrorObj}
 						onRefetch={onRefetchRoles}
-						value={localRole}
-						onChange={onRoleChange}
-						placeholder="Select role"
+						value={localRoles}
+						onChange={onRolesChange}
+						placeholder="Select roles"
 					/>
 				)}
 			</div>

frontend/src/components/ServiceAccountDrawer/ServiceAccountDrawer.tsx

@@ -8,9 +8,7 @@ import { ToggleGroup, ToggleGroupItem } from '@signozhq/ui/toggle-group';
 import { Pagination, Skeleton } from 'antd';
 import { convertToApiError } from 'api/ErrorResponseHandlerForGeneratedAPIs';
 import {
-	getGetServiceAccountRolesQueryKey,
 	getListServiceAccountsQueryKey,
-	useDeleteServiceAccountRole,
 	useGetServiceAccount,
 	useListServiceAccountKeys,
 	useUpdateServiceAccount,
@@ -37,7 +35,7 @@ import {
 	useQueryState,
 } from 'nuqs';
 import APIError from 'types/api/error';
-import { retryOn429, toAPIError } from 'utils/errorUtils';
+import { toAPIError } from 'utils/errorUtils';
 
 import AddKeyModal from './AddKeyModal';
 import DeleteAccountModal from './DeleteAccountModal';
@@ -92,7 +90,7 @@ function ServiceAccountDrawer({
 		parseAsBoolean.withDefault(false),
 	);
 	const [localName, setLocalName] = useState('');
-	const [localRole, setLocalRole] = useState('');
+	const [localRoles, setLocalRoles] = useState<string[]>([]);
 	const [isSaving, setIsSaving] = useState(false);
 	const [saveErrors, setSaveErrors] = useState<SaveError[]>([]);
 
@@ -140,7 +138,7 @@ function ServiceAccountDrawer({
 		if (!account?.id) {
 			roleSessionRef.current = null;
 		} else if (account.id !== roleSessionRef.current && !isRolesLoading) {
-			setLocalRole(currentRoles[0]?.id ?? '');
+			setLocalRoles(currentRoles.map((r) => r.id).filter(Boolean) as string[]);
 			roleSessionRef.current = account.id;
 		}
 	}, [account?.id, currentRoles, isRolesLoading]);
@@ -151,7 +149,13 @@ function ServiceAccountDrawer({
 	const isDirty =
 		account !== null &&
 		(localName !== (account.name ?? '') ||
-			localRole !== (currentRoles[0]?.id ?? ''));
+			JSON.stringify([...localRoles].sort()) !==
+				JSON.stringify(
+					currentRoles
+						.map((r) => r.id)
+						.filter(Boolean)
+						.sort(),
+				));
 
 	const {
 		roles: availableRoles,
@@ -179,27 +183,6 @@ function ServiceAccountDrawer({
 
 	// the retry for this mutation is safe due to the api being idempotent on backend
 	const { mutateAsync: updateMutateAsync } = useUpdateServiceAccount();
-	const { mutateAsync: deleteRole } = useDeleteServiceAccountRole({
-		mutation: {
-			retry: retryOn429,
-		},
-	});
-
-	const executeRolesOperation = useCallback(
-		async (accountId: string): Promise<RoleUpdateFailure[]> => {
-			if (localRole === '' && currentRoles[0]?.id) {
-				await deleteRole({
-					pathParams: { id: accountId, rid: currentRoles[0].id },
-				});
-				await queryClient.invalidateQueries(
-					getGetServiceAccountRolesQueryKey({ id: accountId }),
-				);
-				return [];
-			}
-			return applyDiff([localRole].filter(Boolean), availableRoles);
-		},
-		[localRole, currentRoles, availableRoles, applyDiff, deleteRole, queryClient],
-	);
 
 	const retryNameUpdate = useCallback(async (): Promise<void> => {
 		if (!account) {
@@ -267,7 +250,7 @@ function ServiceAccountDrawer({
 
 	const retryRolesUpdate = useCallback(async (): Promise<void> => {
 		try {
-			const failures = await executeRolesOperation(selectedAccountId ?? '');
+			const failures = await applyDiff([...localRoles], availableRoles);
 			if (failures.length === 0) {
 				setSaveErrors((prev) => prev.filter((e) => e.context !== 'Roles update'));
 			} else {
@@ -283,7 +266,7 @@ function ServiceAccountDrawer({
 				),
 			);
 		}
-	}, [selectedAccountId, executeRolesOperation, failuresToSaveErrors]);
+	}, [localRoles, availableRoles, applyDiff, failuresToSaveErrors]);
 
 	const handleSave = useCallback(async (): Promise<void> => {
 		if (!account || !isDirty) {
@@ -302,7 +285,7 @@ function ServiceAccountDrawer({
 
 			const [nameResult, rolesResult] = await Promise.allSettled([
 				namePromise,
-				executeRolesOperation(account.id),
+				applyDiff([...localRoles], availableRoles),
 			]);
 
 			const errors: SaveError[] = [];
@@ -343,8 +326,10 @@ function ServiceAccountDrawer({
 		account,
 		isDirty,
 		localName,
+		localRoles,
+		availableRoles,
 		updateMutateAsync,
-		executeRolesOperation,
+		applyDiff,
 		refetchAccount,
 		onSuccess,
 		queryClient,
@@ -443,9 +428,9 @@ function ServiceAccountDrawer({
 								account={account}
 								localName={localName}
 								onNameChange={handleNameChange}
-								localRole={localRole}
-								onRoleChange={(role): void => {
-									setLocalRole(role ?? '');
+								localRoles={localRoles}
+								onRolesChange={(roles): void => {
+									setLocalRoles(roles);
 									clearRoleErrors();
 								}}
 								isDisabled={isDeleted}

frontend/src/components/ServiceAccountDrawer/__tests__/ServiceAccountDrawer.test.tsx

@@ -151,7 +151,7 @@ describe('ServiceAccountDrawer', () => {
 		});
 	});
 
-	it('changing roles enables Save; clicking Save sends role add request without delete', async () => {
+	it('adding a role fires POST for the new role and no DELETE for existing roles', async () => {
 		const roleSpy = jest.fn();
 		const deleteSpy = jest.fn();
 		const user = userEvent.setup({ pointerEventsCheck: 0 });
@@ -171,6 +171,7 @@ describe('ServiceAccountDrawer', () => {
 
 		await screen.findByDisplayValue('CI Bot');
 
+		// Add signoz-viewer while keeping signoz-admin selected
 		await user.click(screen.getByLabelText('Roles'));
 		await user.click(await screen.findByTitle('signoz-viewer'));
 
@@ -188,6 +189,43 @@ describe('ServiceAccountDrawer', () => {
 		});
 	});
 
+	it('removing a role fires DELETE for the removed role and no POST', async () => {
+		const roleSpy = jest.fn();
+		const deleteSpy = jest.fn();
+		const user = userEvent.setup({ pointerEventsCheck: 0 });
+
+		server.use(
+			rest.post(SA_ROLES_ENDPOINT, async (req, res, ctx) => {
+				roleSpy(await req.json());
+				return res(ctx.status(200), ctx.json({ status: 'success', data: {} }));
+			}),
+			rest.delete(SA_ROLE_DELETE_ENDPOINT, (_, res, ctx) => {
+				deleteSpy();
+				return res(ctx.status(200), ctx.json({ status: 'success', data: {} }));
+			}),
+		);
+
+		renderDrawer();
+
+		await screen.findByDisplayValue('CI Bot');
+
+		// Remove the signoz-admin tag from the multi-select
+		const adminTag = await screen.findByTitle('signoz-admin');
+		const removeBtn = adminTag.querySelector(
+			'.ant-select-selection-item-remove',
+		) as Element;
+		await user.click(removeBtn);
+
+		const saveBtn = screen.getByRole('button', { name: /Save Changes/i });
+		await waitFor(() => expect(saveBtn).not.toBeDisabled());
+		await user.click(saveBtn);
+
+		await waitFor(() => {
+			expect(deleteSpy).toHaveBeenCalled();
+			expect(roleSpy).not.toHaveBeenCalled();
+		});
+	});
+
 	it('"Delete Service Account" opens confirm dialog; confirming sends delete request', async () => {
 		const deleteSpy = jest.fn();
 		const user = userEvent.setup({ pointerEventsCheck: 0 });

frontend/src/hooks/serviceAccount/useServiceAccountRoleManager.ts

@@ -3,6 +3,7 @@ import { useQueryClient } from 'react-query';
 import {
 	getGetServiceAccountRolesQueryKey,
 	useCreateServiceAccountRole,
+	useDeleteServiceAccountRole,
 	useGetServiceAccountRoles,
 } from 'api/generated/services/serviceaccount';
 import type { AuthtypesRoleDTO } from 'api/generated/services/sigNoz.schemas';
@@ -44,6 +45,9 @@ export function useServiceAccountRoleManager(
 	const { mutateAsync: createRole } = useCreateServiceAccountRole({
 		mutation: { retry: retryOn429 },
 	});
+	const { mutateAsync: deleteRole } = useDeleteServiceAccountRole({
+		mutation: { retry: retryOn429 },
+	});
 
 	const invalidateRoles = useCallback(
 		() =>
@@ -68,14 +72,21 @@ export function useServiceAccountRoleManager(
 			const addedRoles = availableRoles.filter(
 				(r) => r.id && desiredRoleIds.has(r.id) && !currentRoleIds.has(r.id),
 			);
+			const removedRoles = currentRoles.filter(
+				(r) => r.id && !desiredRoleIds.has(r.id),
+			);
 
-			// TODO: re-enable deletes once BE for this is streamlined
 			const allOperations = [
 				...addedRoles.map((role) => ({
 					role,
 					run: (): ReturnType<typeof createRole> =>
 						createRole({ pathParams: { id: accountId }, data: { id: role.id } }),
 				})),
+				...removedRoles.map((role) => ({
+					role,
+					run: (): ReturnType<typeof deleteRole> =>
+						deleteRole({ pathParams: { id: accountId, rid: role.id ?? '' } }),
+				})),
 			];
 
 			const results = await Promise.allSettled(
@@ -106,7 +117,7 @@ export function useServiceAccountRoleManager(
 
 			return failures;
 		},
-		[accountId, currentRoles, createRole, invalidateRoles],
+		[accountId, currentRoles, createRole, deleteRole, invalidateRoles],
 	);
 
 	return {

pkg/modules/serviceaccount/implserviceaccount/module.go

@@ -377,7 +377,7 @@ func (module *module) getOrGetSetIdentity(ctx context.Context, serviceAccountID
 }
 
 func (module *module) setRole(ctx context.Context, orgID valuer.UUID, id valuer.UUID, role *authtypes.Role) error {
-	serviceAccount, err := module.GetWithRoles(ctx, orgID, id)
+	serviceAccount, err := module.Get(ctx, orgID, id)
 	if err != nil {
 		return err
 	}
@@ -387,24 +387,12 @@ func (module *module) setRole(ctx context.Context, orgID valuer.UUID, id valuer.
 		return err
 	}
 
-	err = module.authz.ModifyGrant(ctx, orgID, serviceAccount.RoleNames(), []string{role.Name}, authtypes.MustNewSubject(coretypes.NewResourceServiceAccount(), id.String(), orgID, nil))
+	err = module.authz.Grant(ctx, orgID, []string{role.Name}, authtypes.MustNewSubject(coretypes.NewResourceServiceAccount(), id.String(), orgID, nil))
 	if err != nil {
 		return err
 	}
 
-	err = module.store.RunInTx(ctx, func(ctx context.Context) error {
-		err = module.store.DeleteServiceAccountRoles(ctx, serviceAccount.ID)
-		if err != nil {
-			return err
-		}
-
-		err = module.store.CreateServiceAccountRole(ctx, serviceAccountRole)
-		if err != nil {
-			return err
-		}
-
-		return nil
-	})
+	err = module.store.CreateServiceAccountRole(ctx, serviceAccountRole)
 	if err != nil {
 		return err
 	}

pkg/modules/serviceaccount/implserviceaccount/store.go

@@ -207,21 +207,6 @@ func (store *store) CreateServiceAccountRole(ctx context.Context, serviceAccount
 	return nil
 }
 
-func (store *store) DeleteServiceAccountRoles(ctx context.Context, serviceAccountID valuer.UUID) error {
-	_, err := store.
-		sqlstore.
-		BunDBCtx(ctx).
-		NewDelete().
-		Model(new(serviceaccounttypes.ServiceAccountRole)).
-		Where("service_account_id = ?", serviceAccountID).
-		Exec(ctx)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func (store *store) DeleteServiceAccountRole(ctx context.Context, serviceAccountID valuer.UUID, roleID valuer.UUID) error {
 	_, err := store.
 		sqlstore.

pkg/types/serviceaccounttypes/service_acccount.go

@@ -245,7 +245,6 @@ type Store interface {
 
 	// Service Account Role
 	CreateServiceAccountRole(context.Context, *ServiceAccountRole) error
-	DeleteServiceAccountRoles(context.Context, valuer.UUID) error
 	DeleteServiceAccountRole(context.Context, valuer.UUID, valuer.UUID) error
 
 	// Service Account Factor API Key

tests/fixtures/serviceaccount.py

@@ -73,6 +73,30 @@ def get_first_key_id(signoz: types.SigNoz, token: str, service_account_id: str)
     return resp.json()["data"][0]["id"]
 
 
+def create_service_account_with_roles(signoz: types.SigNoz, token: str, name: str, roles: list[str]) -> str:
+    """Create a service account and assign multiple roles."""
+    resp = requests.post(
+        signoz.self.host_configs["8080"].get(SERVICE_ACCOUNT_BASE),
+        json={"name": name},
+        headers={"Authorization": f"Bearer {token}"},
+        timeout=5,
+    )
+    assert resp.status_code == HTTPStatus.CREATED, resp.text
+    service_account_id = resp.json()["data"]["id"]
+
+    for role in roles:
+        role_id = find_role_by_name(signoz, token, role)
+        role_resp = requests.post(
+            signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
+            json={"id": role_id},
+            headers={"Authorization": f"Bearer {token}"},
+            timeout=5,
+        )
+        assert role_resp.status_code == HTTPStatus.NO_CONTENT, role_resp.text
+
+    return service_account_id
+
+
 def find_service_account_by_name(signoz: types.SigNoz, token: str, name: str) -> dict:
     """Find a service account by name from the list endpoint."""
     list_resp = requests.get(

tests/integration/tests/serviceaccount/04_roles.py

@@ -44,13 +44,13 @@ def test_assign_role_to_service_account(
     create_user_admin: types.Operation,  # pylint: disable=unused-argument
     get_token: Callable[[str, str], str],
 ):
-    """POST /{id}/roles replaces existing role, verify via GET."""
+    """POST /{id}/roles adds a role alongside existing ones."""
     token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
 
     # create service account with viewer role
     service_account_id = create_service_account(signoz, token, "sa-assign-role", role="signoz-viewer")
 
-    # assign editor role (replaces viewer)
+    # assign editor role (additive — viewer stays)
     editor_role_id = find_role_by_name(signoz, token, "signoz-editor")
     assign_resp = requests.post(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
@@ -60,7 +60,7 @@ def test_assign_role_to_service_account(
     )
     assert assign_resp.status_code == HTTPStatus.NO_CONTENT, assign_resp.text
 
-    # verify only editor role is present (viewer was replaced)
+    # verify both viewer and editor roles are present
     roles_resp = requests.get(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
         headers={"Authorization": f"Bearer {token}"},
@@ -68,9 +68,31 @@ def test_assign_role_to_service_account(
     )
     assert roles_resp.status_code == HTTPStatus.OK, roles_resp.text
     role_names = [r["name"] for r in roles_resp.json()["data"]]
-    assert len(role_names) == 1
+    assert len(role_names) == 2
+    assert "signoz-viewer" in role_names
     assert "signoz-editor" in role_names
-    assert "signoz-viewer" not in role_names
+
+    # assign admin role — all three should be present
+    admin_role_id = find_role_by_name(signoz, token, "signoz-admin")
+    assign_resp = requests.post(
+        signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
+        json={"id": admin_role_id},
+        headers={"Authorization": f"Bearer {token}"},
+        timeout=5,
+    )
+    assert assign_resp.status_code == HTTPStatus.NO_CONTENT, assign_resp.text
+
+    roles_resp = requests.get(
+        signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
+        headers={"Authorization": f"Bearer {token}"},
+        timeout=5,
+    )
+    assert roles_resp.status_code == HTTPStatus.OK, roles_resp.text
+    role_names = [r["name"] for r in roles_resp.json()["data"]]
+    assert len(role_names) == 3
+    assert "signoz-viewer" in role_names
+    assert "signoz-editor" in role_names
+    assert "signoz-admin" in role_names
 
 
 def test_assign_role_idempotent(
@@ -103,16 +125,16 @@ def test_assign_role_idempotent(
     assert role_names.count("signoz-viewer") == 1
 
 
-def test_assign_role_replaces_access(
+def test_assign_role_expands_access(
     signoz: types.SigNoz,
     create_user_admin: types.Operation,  # pylint: disable=unused-argument
     get_token: Callable[[str, str], str],
 ):
-    """After role replacement, SA loses old permissions and gains new ones."""
+    """Adding a higher-privilege role expands the SA's access."""
     token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
 
     # create SA with viewer role and an API key
-    service_account_id, api_key = create_service_account_with_key(signoz, token, "sa-role-replace-access", role="signoz-viewer")
+    service_account_id, api_key = create_service_account_with_key(signoz, token, "sa-role-expand-access", role="signoz-viewer")
 
     # viewer should get 403 on admin-only endpoint
     resp = requests.get(
@@ -122,7 +144,7 @@ def test_assign_role_replaces_access(
     )
     assert resp.status_code == HTTPStatus.FORBIDDEN, f"Expected 403 for viewer on admin endpoint, got {resp.status_code}: {resp.text}"
 
-    # assign admin role (replaces viewer)
+    # assign admin role (additive — viewer stays)
     admin_role_id = find_role_by_name(signoz, token, "signoz-admin")
     assign_resp = requests.post(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
@@ -138,9 +160,9 @@ def test_assign_role_replaces_access(
         headers={"SIGNOZ-API-KEY": api_key},
         timeout=5,
     )
-    assert resp.status_code == HTTPStatus.OK, f"Expected 200 for admin on admin endpoint, got {resp.status_code}: {resp.text}"
+    assert resp.status_code == HTTPStatus.OK, f"Expected 200 after adding admin role, got {resp.status_code}: {resp.text}"
 
-    # verify only admin role is present
+    # verify both roles are present
     roles_resp = requests.get(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
         headers={"Authorization": f"Bearer {token}"},
@@ -148,9 +170,9 @@ def test_assign_role_replaces_access(
     )
     assert roles_resp.status_code == HTTPStatus.OK, roles_resp.text
     role_names = [r["name"] for r in roles_resp.json()["data"]]
-    assert len(role_names) == 1
+    assert len(role_names) == 2
     assert "signoz-admin" in role_names
-    assert "signoz-viewer" not in role_names
+    assert "signoz-viewer" in role_names
 
 
 def test_remove_role_from_service_account(
@@ -158,13 +180,22 @@ def test_remove_role_from_service_account(
     create_user_admin: types.Operation,  # pylint: disable=unused-argument
     get_token: Callable[[str, str], str],
 ):
-    """DELETE /{id}/roles/{rid} revokes a role."""
+    """DELETE /{id}/roles/{rid} revokes one role while keeping others."""
     token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
     service_account_id = create_service_account(signoz, token, "sa-remove-role", role="signoz-editor")
 
-    editor_role_id = find_role_by_name(signoz, token, "signoz-editor")
+    # add admin role (now has editor + admin)
+    admin_role_id = find_role_by_name(signoz, token, "signoz-admin")
+    assign_resp = requests.post(
+        signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
+        json={"id": admin_role_id},
+        headers={"Authorization": f"Bearer {token}"},
+        timeout=5,
+    )
+    assert assign_resp.status_code == HTTPStatus.NO_CONTENT, assign_resp.text
 
-    # remove the role
+    # remove editor role
+    editor_role_id = find_role_by_name(signoz, token, "signoz-editor")
     resp = requests.delete(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles/{editor_role_id}"),
         headers={"Authorization": f"Bearer {token}"},
@@ -172,7 +203,7 @@ def test_remove_role_from_service_account(
     )
     assert resp.status_code == HTTPStatus.NO_CONTENT, resp.text
 
-    # verify role is gone
+    # verify editor is gone but admin remains
     roles_resp = requests.get(
         signoz.self.host_configs["8080"].get(f"{SERVICE_ACCOUNT_BASE}/{service_account_id}/roles"),
         headers={"Authorization": f"Bearer {token}"},
@@ -181,6 +212,7 @@ def test_remove_role_from_service_account(
     assert roles_resp.status_code == HTTPStatus.OK, roles_resp.text
     role_names = [r["name"] for r in roles_resp.json()["data"]]
     assert "signoz-editor" not in role_names
+    assert "signoz-admin" in role_names
 
 
 def test_remove_role_verify_access_lost(