Merge pull request #2057 from automatisch/aut-1226

feat: write endpoint to update SamlAuthProvidersRoleMappings

packages/backend/src/controllers/api/v1/admin/saml-auth-providers/update-role-mappings.ee.js

@@ -0,0 +1,26 @@
+import { renderObject } from '../../../../../helpers/renderer.js';
+import SamlAuthProvider from '../../../../../models/saml-auth-provider.ee.js';
+
+export default async (request, response) => {
+  const samlAuthProviderId = request.params.samlAuthProviderId;
+
+  const samlAuthProvider = await SamlAuthProvider.query()
+    .findById(samlAuthProviderId)
+    .throwIfNotFound();
+
+  const samlAuthProvidersRoleMappings =
+    await samlAuthProvider.updateRoleMappings(
+      samlAuthProvidersRoleMappingsParams(request)
+    );
+
+  renderObject(response, samlAuthProvidersRoleMappings);
+};
+
+const samlAuthProvidersRoleMappingsParams = (request) => {
+  const roleMappings = request.body;
+
+  return roleMappings.map(({ roleId, remoteRoleName }) => ({
+    roleId,
+    remoteRoleName,
+  }));
+};

packages/backend/src/controllers/api/v1/admin/saml-auth-providers/update-role-mappings.ee.test.js

@@ -0,0 +1,182 @@
+import Crypto from 'node:crypto';
+import { vi, describe, it, expect, beforeEach } from 'vitest';
+import request from 'supertest';
+import app from '../../../../../app.js';
+import createAuthTokenByUserId from '../../../../../helpers/create-auth-token-by-user-id.js';
+import { createRole } from '../../../../../../test/factories/role.js';
+import { createUser } from '../../../../../../test/factories/user.js';
+import { createSamlAuthProvider } from '../../../../../../test/factories/saml-auth-provider.ee.js';
+import { createSamlAuthProvidersRoleMapping } from '../../../../../../test/factories/saml-auth-providers-role-mapping.js';
+import createRoleMappingsMock from '../../../../../../test/mocks/rest/api/v1/admin/saml-auth-providers/update-role-mappings.ee.js';
+import * as license from '../../../../../helpers/license.ee.js';
+
+describe('PATCH /api/v1/admin/saml-auth-providers/:samlAuthProviderId/role-mappings', () => {
+  let samlAuthProvider, currentUser, userRole, token;
+
+  beforeEach(async () => {
+    vi.spyOn(license, 'hasValidLicense').mockResolvedValue(true);
+
+    userRole = await createRole({ name: 'Admin' });
+    currentUser = await createUser({ roleId: userRole.id });
+
+    samlAuthProvider = await createSamlAuthProvider();
+
+    await createSamlAuthProvidersRoleMapping({
+      samlAuthProviderId: samlAuthProvider.id,
+      remoteRoleName: 'Viewer',
+    });
+
+    await createSamlAuthProvidersRoleMapping({
+      samlAuthProviderId: samlAuthProvider.id,
+      remoteRoleName: 'Editor',
+    });
+
+    token = await createAuthTokenByUserId(currentUser.id);
+  });
+
+  it('should update role mappings', async () => {
+    const roleMappings = [
+      {
+        roleId: userRole.id,
+        remoteRoleName: 'Admin',
+      },
+    ];
+
+    const response = await request(app)
+      .patch(
+        `/api/v1/admin/saml-auth-providers/${samlAuthProvider.id}/role-mappings`
+      )
+      .set('Authorization', token)
+      .send(roleMappings)
+      .expect(200);
+
+    const expectedPayload = await createRoleMappingsMock([
+      {
+        roleId: userRole.id,
+        remoteRoleName: 'Admin',
+        id: response.body.data[0].id,
+        samlAuthProviderId: samlAuthProvider.id,
+      },
+    ]);
+
+    expect(response.body).toStrictEqual(expectedPayload);
+  });
+
+  it('should delete role mappings when given empty role mappings', async () => {
+    const existingRoleMappings = await samlAuthProvider.$relatedQuery(
+      'samlAuthProvidersRoleMappings'
+    );
+
+    expect(existingRoleMappings.length).toBe(2);
+
+    const response = await request(app)
+      .patch(
+        `/api/v1/admin/saml-auth-providers/${samlAuthProvider.id}/role-mappings`
+      )
+      .set('Authorization', token)
+      .send([])
+      .expect(200);
+
+    const expectedPayload = await createRoleMappingsMock([]);
+
+    expect(response.body).toStrictEqual({
+      ...expectedPayload,
+      meta: {
+        ...expectedPayload.meta,
+        type: 'Object',
+      },
+    });
+  });
+
+  it('should return internal server error response for not existing role UUID', async () => {
+    const notExistingRoleUUID = Crypto.randomUUID();
+    const roleMappings = [
+      {
+        roleId: notExistingRoleUUID,
+        remoteRoleName: 'Admin',
+      },
+    ];
+
+    await request(app)
+      .patch(
+        `/api/v1/admin/saml-auth-providers/${samlAuthProvider.id}/role-mappings`
+      )
+      .set('Authorization', token)
+      .send(roleMappings)
+      .expect(500);
+  });
+
+  it('should return unprocessable entity response for invalid data', async () => {
+    const roleMappings = [
+      {
+        roleId: userRole.id,
+        remoteRoleName: {},
+      },
+    ];
+
+    const response = await request(app)
+      .patch(
+        `/api/v1/admin/saml-auth-providers/${samlAuthProvider.id}/role-mappings`
+      )
+      .set('Authorization', token)
+      .send(roleMappings)
+      .expect(422);
+
+    expect(response.body).toStrictEqual({
+      errors: {
+        remoteRoleName: ['must be string'],
+      },
+      meta: {
+        type: 'ModelValidation',
+      },
+    });
+  });
+
+  it('should return not found response for not existing SAML auth provider UUID', async () => {
+    const notExistingSamlAuthProviderUUID = Crypto.randomUUID();
+    const roleMappings = [
+      {
+        roleId: userRole.id,
+        remoteRoleName: 'Admin',
+      },
+    ];
+
+    await request(app)
+      .patch(
+        `/api/v1/admin/saml-auth-providers/${notExistingSamlAuthProviderUUID}/role-mappings`
+      )
+      .set('Authorization', token)
+      .send(roleMappings)
+      .expect(404);
+  });
+
+  it('should not delete existing role mapping when error thrown', async () => {
+    const roleMappings = [
+      {
+        roleId: userRole.id,
+        remoteRoleName: {
+          invalid: 'data',
+        },
+      },
+    ];
+
+    const roleMappingsBeforeRequest = await samlAuthProvider.$relatedQuery(
+      'samlAuthProvidersRoleMappings'
+    );
+
+    await request(app)
+      .patch(
+        `/api/v1/admin/saml-auth-providers/${samlAuthProvider.id}/role-mappings`
+      )
+      .set('Authorization', token)
+      .send(roleMappings)
+      .expect(422);
+
+    const roleMappingsAfterRequest = await samlAuthProvider.$relatedQuery(
+      'samlAuthProvidersRoleMappings'
+    );
+
+    expect(roleMappingsBeforeRequest).toStrictEqual(roleMappingsAfterRequest);
+    expect(roleMappingsAfterRequest.length).toBe(2);
+  });
+});

packages/backend/src/graphql/mutation-resolvers.js

@@ -11,7 +11,6 @@ import updateConnection from './mutations/update-connection.js';
 import updateCurrentUser from './mutations/update-current-user.js';
 import updateFlowStatus from './mutations/update-flow-status.js';
 import updateStep from './mutations/update-step.js';
-import upsertSamlAuthProvidersRoleMappings from './mutations/upsert-saml-auth-providers-role-mappings.ee.js';
 
 // Converted mutations
 import executeFlow from './mutations/execute-flow.js';
@@ -40,7 +39,6 @@ const mutationResolvers = {
   updateFlowStatus,
   updateStep,
   updateUser,
-  upsertSamlAuthProvidersRoleMappings,
   verifyConnection,
 };
 

packages/backend/src/graphql/mutations/upsert-saml-auth-providers-role-mappings.ee.js

@@ -1,42 +0,0 @@
-import SamlAuthProvider from '../../models/saml-auth-provider.ee.js';
-import SamlAuthProvidersRoleMapping from '../../models/saml-auth-providers-role-mapping.ee.js';
-import isEmpty from 'lodash/isEmpty.js';
-
-const upsertSamlAuthProvidersRoleMappings = async (
-  _parent,
-  params,
-  context
-) => {
-  context.currentUser.can('update', 'SamlAuthProvider');
-
-  const samlAuthProviderId = params.input.samlAuthProviderId;
-
-  const samlAuthProvider = await SamlAuthProvider.query()
-    .findById(samlAuthProviderId)
-    .throwIfNotFound();
-
-  await samlAuthProvider
-    .$relatedQuery('samlAuthProvidersRoleMappings')
-    .delete();
-
-  if (isEmpty(params.input.samlAuthProvidersRoleMappings)) {
-    return [];
-  }
-
-  const samlAuthProvidersRoleMappingsData =
-    params.input.samlAuthProvidersRoleMappings.map(
-      (samlAuthProvidersRoleMapping) => ({
-        ...samlAuthProvidersRoleMapping,
-        samlAuthProviderId: samlAuthProvider.id,
-      })
-    );
-
-  const samlAuthProvidersRoleMappings =
-    await SamlAuthProvidersRoleMapping.query().insert(
-      samlAuthProvidersRoleMappingsData
-    );
-
-  return samlAuthProvidersRoleMappings;
-};
-
-export default upsertSamlAuthProvidersRoleMappings;

packages/backend/src/graphql/schema.graphql

@@ -20,9 +20,6 @@ type Mutation {
   updateFlowStatus(input: UpdateFlowStatusInput): Flow
   updateStep(input: UpdateStepInput): Step
   updateUser(input: UpdateUserInput): User
-  upsertSamlAuthProvidersRoleMappings(
-    input: UpsertSamlAuthProvidersRoleMappingsInput
-  ): [SamlAuthProvidersRoleMapping]
   verifyConnection(input: VerifyConnectionInput): Connection
 }
 
@@ -247,16 +244,6 @@ input VerifyConnectionInput {
   id: String!
 }
 
-input UpsertSamlAuthProvidersRoleMappingsInput {
-  samlAuthProviderId: String!
-  samlAuthProvidersRoleMappings: [SamlAuthProviderRoleMappingInput]
-}
-
-input SamlAuthProviderRoleMappingInput {
-  roleId: String!
-  remoteRoleName: String!
-}
-
 input CreateFlowInput {
   triggerAppKey: String
   connectionId: String

packages/backend/src/helpers/error-handler.js

@@ -1,8 +1,13 @@
 import logger from './logger.js';
 import objection from 'objection';
 import * as Sentry from './sentry.ee.js';
-const { NotFoundError, DataError, ValidationError, UniqueViolationError } =
+const {
-  objection;
+  NotFoundError,
+  DataError,
+  ForeignKeyViolationError,
+  ValidationError,
+  UniqueViolationError,
+} = objection;
 import NotAuthorizedError from '../errors/not-authorized.js';
 import HttpError from '../errors/http.js';
 import {
@@ -29,6 +34,10 @@ const errorHandler = (error, request, response, next) => {
     renderUniqueViolationError(response, error);
   }
 
+  if (error instanceof ForeignKeyViolationError) {
+    response.status(500).end();
+  }
+
   if (error instanceof DataError) {
     response.status(400).end();
   }

packages/backend/src/models/saml-auth-provider.ee.js

@@ -1,5 +1,6 @@
 import { URL } from 'node:url';
 import { v4 as uuidv4 } from 'uuid';
+import isEmpty from 'lodash/isEmpty.js';
 import appConfig from '../config/app.js';
 import axios from '../helpers/axios-with-proxy.js';
 import Base from './base.js';
@@ -88,7 +89,7 @@ class SamlAuthProvider extends Base {
       entryPoint: this.entryPoint,
       issuer: this.issuer,
       signatureAlgorithm: this.signatureAlgorithm,
-      logoutUrl: this.remoteLogoutUrl
+      logoutUrl: this.remoteLogoutUrl,
     };
   }
 
@@ -101,14 +102,16 @@ class SamlAuthProvider extends Base {
           IssueInstant="${new Date().toISOString()}"
           Destination="${this.remoteLogoutUrl}">
 
-          <saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">${this.issuer}</saml:Issuer>
+          <saml:Issuer xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">${
+            this.issuer
+          }</saml:Issuer>
           <samlp:SessionIndex>${sessionId}</samlp:SessionIndex>
       </samlp:LogoutRequest>
     `;
 
-    const encodedLogoutRequest = Buffer.from(logoutRequest).toString('base64')
+    const encodedLogoutRequest = Buffer.from(logoutRequest).toString('base64');
 
-    return encodedLogoutRequest
+    return encodedLogoutRequest;
   }
 
   async terminateRemoteSession(sessionId) {
@@ -122,12 +125,36 @@ class SamlAuthProvider extends Base {
       {
         headers: {
           'Content-Type': 'application/x-www-form-urlencoded',
-        }
+        },
       }
     );
 
     return response;
   }
+
+  async updateRoleMappings(roleMappings) {
+    return await SamlAuthProvider.transaction(async (trx) => {
+      await this.$relatedQuery('samlAuthProvidersRoleMappings', trx).delete();
+
+      if (isEmpty(roleMappings)) {
+        return [];
+      }
+
+      const samlAuthProvidersRoleMappingsData = roleMappings.map(
+        (samlAuthProvidersRoleMapping) => ({
+          ...samlAuthProvidersRoleMapping,
+          samlAuthProviderId: this.id,
+        })
+      );
+
+      const samlAuthProvidersRoleMappings =
+        await SamlAuthProvidersRoleMapping.query(trx).insertAndFetch(
+          samlAuthProvidersRoleMappingsData
+        );
+
+      return samlAuthProvidersRoleMappings;
+    });
+  }
 }
 
 export default SamlAuthProvider;

packages/backend/src/routes/api/v1/admin/saml-auth-providers.ee.js

@@ -7,6 +7,7 @@ import updateSamlAuthProviderAction from '../../../../controllers/api/v1/admin/s
 import getSamlAuthProvidersAction from '../../../../controllers/api/v1/admin/saml-auth-providers/get-saml-auth-providers.ee.js';
 import getSamlAuthProviderAction from '../../../../controllers/api/v1/admin/saml-auth-providers/get-saml-auth-provider.ee.js';
 import getRoleMappingsAction from '../../../../controllers/api/v1/admin/saml-auth-providers/get-role-mappings.ee.js';
+import updateRoleMappingsAction from '../../../../controllers/api/v1/admin/saml-auth-providers/update-role-mappings.ee.js';
 
 const router = Router();
 
@@ -50,4 +51,12 @@ router.patch(
   updateSamlAuthProviderAction
 );
 
+router.patch(
+  '/:samlAuthProviderId/role-mappings',
+  authenticateUser,
+  authorizeAdmin,
+  checkIsEnterprise,
+  updateRoleMappingsAction
+);
+
 export default router;

packages/backend/test/factories/role.js

@@ -6,6 +6,12 @@ export const createRole = async (params = {}) => {
 
   params.name = params?.name || name;
 
+  const existingRole = await Role.query().findOne({ name }).first();
+
+  if (existingRole) {
+    return await createRole();
+  }
+
   const role = await Role.query().insertAndFetch(params);
 
   return role;

packages/backend/test/factories/saml-auth-providers-role-mapping.js

@@ -0,0 +1,16 @@
+import { faker } from '@faker-js/faker';
+import { createRole } from './role.js';
+import SamlAuthProvidersRoleMapping from '../../src/models/saml-auth-providers-role-mapping.ee.js';
+import { createSamlAuthProvider } from './saml-auth-provider.ee.js';
+
+export const createSamlAuthProvidersRoleMapping = async (params = {}) => {
+  params.roleId = params.roleId || (await createRole()).id;
+  params.samlAuthProviderId =
+    params.samlAuthProviderId || (await createSamlAuthProvider()).id;
+  params.remoteRoleName = params.remoteRoleName || faker.person.jobType();
+
+  const samlAuthProvider =
+    await SamlAuthProvidersRoleMapping.query().insertAndFetch(params);
+
+  return samlAuthProvider;
+};

packages/backend/test/mocks/rest/api/v1/admin/saml-auth-providers/update-role-mappings.ee.js

@@ -0,0 +1,23 @@
+const createRoleMappingsMock = async (roleMappings) => {
+  const data = roleMappings.map((roleMapping) => {
+    return {
+      id: roleMapping.id,
+      samlAuthProviderId: roleMapping.samlAuthProviderId,
+      roleId: roleMapping.roleId,
+      remoteRoleName: roleMapping.remoteRoleName,
+    };
+  });
+
+  return {
+    data: data,
+    meta: {
+      count: data.length,
+      currentPage: null,
+      isArray: true,
+      totalPages: null,
+      type: 'SamlAuthProvidersRoleMapping',
+    },
+  };
+};
+
+export default createRoleMappingsMock;

packages/web/src/graphql/mutations/upsert-saml-auth-providers-role-mappings.js

@@ -1,13 +0,0 @@
-import { gql } from '@apollo/client';
-export const UPSERT_SAML_AUTH_PROVIDERS_ROLE_MAPPINGS = gql`
-  mutation UpsertSamlAuthProvidersRoleMappings(
-    $input: UpsertSamlAuthProvidersRoleMappingsInput
-  ) {
-    upsertSamlAuthProvidersRoleMappings(input: $input) {
-      id
-      samlAuthProviderId
-      roleId
-      remoteRoleName
-    }
-  }
-`;

packages/web/src/hooks/useAdminUpdateSamlAuthProviderRoleMappings.js

@@ -0,0 +1,31 @@
+import { useMutation, useQueryClient } from '@tanstack/react-query';
+import api from 'helpers/api';
+
+export default function useAdminUpdateSamlAuthProviderRoleMappings(
+  samlAuthProviderId,
+) {
+  const queryClient = useQueryClient();
+
+  const query = useMutation({
+    mutationFn: async (payload) => {
+      const { data } = await api.patch(
+        `/v1/admin/saml-auth-providers/${samlAuthProviderId}/role-mappings`,
+        payload,
+      );
+
+      return data;
+    },
+    onSuccess: () => {
+      queryClient.invalidateQueries({
+        queryKey: [
+          'admin',
+          'samlAuthProviders',
+          samlAuthProviderId,
+          'roleMappings',
+        ],
+      });
+    },
+  });
+
+  return query;
+}

packages/web/src/pages/Authentication/RoleMappings.jsx

@@ -1,5 +1,4 @@
 import PropTypes from 'prop-types';
-import { useMutation } from '@apollo/client';
 import LoadingButton from '@mui/lab/LoadingButton';
 import Divider from '@mui/material/Divider';
 import Stack from '@mui/material/Stack';
@@ -8,9 +7,9 @@ import useEnqueueSnackbar from 'hooks/useEnqueueSnackbar';
 import { useMemo } from 'react';
 
 import Form from 'components/Form';
-import { UPSERT_SAML_AUTH_PROVIDERS_ROLE_MAPPINGS } from 'graphql/mutations/upsert-saml-auth-providers-role-mappings';
 import useFormatMessage from 'hooks/useFormatMessage';
 import useAdminSamlAuthProviderRoleMappings from 'hooks/useAdminSamlAuthProviderRoleMappings';
+import useAdminUpdateSamlAuthProviderRoleMappings from 'hooks/useAdminUpdateSamlAuthProviderRoleMappings';
 import RoleMappingsFieldArray from './RoleMappingsFieldsArray';
 
 function generateFormRoleMappings(roleMappings) {
@@ -28,33 +27,26 @@ function RoleMappings({ provider, providerLoading }) {
   const formatMessage = useFormatMessage();
   const enqueueSnackbar = useEnqueueSnackbar();
 
+  const {
+    mutateAsync: updateSamlAuthProvidersRoleMappings,
+    isPending: isUpdateSamlAuthProvidersRoleMappingsPending,
+  } = useAdminUpdateSamlAuthProviderRoleMappings(provider?.id);
+
   const { data, isLoading: isAdminSamlAuthProviderRoleMappingsLoading } =
     useAdminSamlAuthProviderRoleMappings({
       adminSamlAuthProviderId: provider?.id,
     });
   const roleMappings = data?.data;
 
-  const [
-    upsertSamlAuthProvidersRoleMappings,
-    { loading: upsertRoleMappingsLoading },
-  ] = useMutation(UPSERT_SAML_AUTH_PROVIDERS_ROLE_MAPPINGS);
-
   const handleRoleMappingsUpdate = async (values) => {
     try {
       if (provider?.id) {
-        await upsertSamlAuthProvidersRoleMappings({
+        await updateSamlAuthProvidersRoleMappings(
-          variables: {
+          values.roleMappings.map(({ roleId, remoteRoleName }) => ({
-            input: {
-              samlAuthProviderId: provider.id,
-              samlAuthProvidersRoleMappings: values.roleMappings.map(
-                ({ roleId, remoteRoleName }) => ({
             roleId,
             remoteRoleName,
-                }),
+          })),
-              ),
+        );
-            },
-          },
-        });
 
         enqueueSnackbar(formatMessage('roleMappingsForm.successfullySaved'), {
           variant: 'success',
@@ -97,7 +89,7 @@ function RoleMappings({ provider, providerLoading }) {
             variant="contained"
             color="primary"
             sx={{ boxShadow: 2 }}
-            loading={upsertRoleMappingsLoading}
+            loading={isUpdateSamlAuthProvidersRoleMappingsPending}
           >
             {formatMessage('roleMappingsForm.save')}
           </LoadingButton>