diff --git a/packages/backend/src/controllers/api/v1/admin/saml-auth-providers/update-role-mappings.ee.js b/packages/backend/src/controllers/api/v1/admin/saml-auth-providers/update-role-mappings.ee.js new file mode 100644 index 00000000..c0e6afb8 --- /dev/null +++ b/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, + })); +}; diff --git a/packages/backend/src/controllers/api/v1/admin/saml-auth-providers/update-role-mappings.ee.test.js b/packages/backend/src/controllers/api/v1/admin/saml-auth-providers/update-role-mappings.ee.test.js new file mode 100644 index 00000000..f06a3ba6 --- /dev/null +++ b/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); + }); +}); diff --git a/packages/backend/src/graphql/mutation-resolvers.js b/packages/backend/src/graphql/mutation-resolvers.js index 8483637a..60c8b8ea 100644 --- a/packages/backend/src/graphql/mutation-resolvers.js +++ b/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, }; diff --git a/packages/backend/src/graphql/mutations/upsert-saml-auth-providers-role-mappings.ee.js b/packages/backend/src/graphql/mutations/upsert-saml-auth-providers-role-mappings.ee.js deleted file mode 100644 index 0040d48e..00000000 --- a/packages/backend/src/graphql/mutations/upsert-saml-auth-providers-role-mappings.ee.js +++ /dev/null @@ -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; diff --git a/packages/backend/src/graphql/schema.graphql b/packages/backend/src/graphql/schema.graphql index 5598f59a..4958b2f0 100644 --- a/packages/backend/src/graphql/schema.graphql +++ b/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 diff --git a/packages/backend/src/helpers/error-handler.js b/packages/backend/src/helpers/error-handler.js index ecfa9c18..63c80bbb 100644 --- a/packages/backend/src/helpers/error-handler.js +++ b/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 } = - objection; +const { + 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(); } diff --git a/packages/backend/src/models/saml-auth-provider.ee.js b/packages/backend/src/models/saml-auth-provider.ee.js index 431153f9..744da1a8 100644 --- a/packages/backend/src/models/saml-auth-provider.ee.js +++ b/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}"> - ${this.issuer} + ${ + this.issuer + } ${sessionId} `; - 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; diff --git a/packages/backend/src/routes/api/v1/admin/saml-auth-providers.ee.js b/packages/backend/src/routes/api/v1/admin/saml-auth-providers.ee.js index 57e333f8..eb243b82 100644 --- a/packages/backend/src/routes/api/v1/admin/saml-auth-providers.ee.js +++ b/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; diff --git a/packages/backend/test/factories/role.js b/packages/backend/test/factories/role.js index 7db3eff2..28ac9960 100644 --- a/packages/backend/test/factories/role.js +++ b/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; diff --git a/packages/backend/test/factories/saml-auth-providers-role-mapping.js b/packages/backend/test/factories/saml-auth-providers-role-mapping.js new file mode 100644 index 00000000..72b23c06 --- /dev/null +++ b/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; +}; diff --git a/packages/backend/test/mocks/rest/api/v1/admin/saml-auth-providers/update-role-mappings.ee.js b/packages/backend/test/mocks/rest/api/v1/admin/saml-auth-providers/update-role-mappings.ee.js new file mode 100644 index 00000000..2ff8ca16 --- /dev/null +++ b/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; diff --git a/packages/web/src/graphql/mutations/upsert-saml-auth-providers-role-mappings.js b/packages/web/src/graphql/mutations/upsert-saml-auth-providers-role-mappings.js deleted file mode 100644 index 2d9180e0..00000000 --- a/packages/web/src/graphql/mutations/upsert-saml-auth-providers-role-mappings.js +++ /dev/null @@ -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 - } - } -`; diff --git a/packages/web/src/hooks/useAdminUpdateSamlAuthProviderRoleMappings.js b/packages/web/src/hooks/useAdminUpdateSamlAuthProviderRoleMappings.js new file mode 100644 index 00000000..4909e540 --- /dev/null +++ b/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; +} diff --git a/packages/web/src/pages/Authentication/RoleMappings.jsx b/packages/web/src/pages/Authentication/RoleMappings.jsx index a2605eef..7a46c35a 100644 --- a/packages/web/src/pages/Authentication/RoleMappings.jsx +++ b/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({ - variables: { - input: { - samlAuthProviderId: provider.id, - samlAuthProvidersRoleMappings: values.roleMappings.map( - ({ roleId, remoteRoleName }) => ({ - roleId, - remoteRoleName, - }), - ), - }, - }, - }); + await updateSamlAuthProvidersRoleMappings( + 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')}