From c276f1bb642ae7d9c8da72078ec0e0829b31ec54 Mon Sep 17 00:00:00 2001 From: Nick Phura Date: Fri, 16 Aug 2024 14:01:06 -0700 Subject: [PATCH] TechDebt: Fix Patch System User SQL (#1341) * Fix patch system user sql. Fix self endpoint rejecting users if they are missing a guid (because the patch has not had a chance to run yet). * add missing key prop on project list container --------- Co-authored-by: Macgregor Aubertin-Young --- api/src/paths/user/self.test.ts | 8 ++--- api/src/paths/user/self.ts | 26 ++++----------- .../project/ProjectsListContainer.tsx | 1 + .../src/procedures/api_patch_system_user.ts | 33 ++++++++++--------- 4 files changed, 29 insertions(+), 39 deletions(-) diff --git a/api/src/paths/user/self.test.ts b/api/src/paths/user/self.test.ts index 1a22331455..ee141b77d1 100644 --- a/api/src/paths/user/self.test.ts +++ b/api/src/paths/user/self.test.ts @@ -6,7 +6,7 @@ import * as db from '../../database/db'; import { HTTPError } from '../../errors/http-error'; import { UserService } from '../../services/user-service'; import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db'; -import * as self from './self'; +import { getSelf } from './self'; chai.use(sinonChai); @@ -23,7 +23,7 @@ describe('getUser', () => { sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); try { - const requestHandler = self.getUser(); + const requestHandler = getSelf(); await requestHandler(mockReq, mockRes, mockNext); expect.fail(); @@ -57,7 +57,7 @@ describe('getUser', () => { agency: null }); - const requestHandler = self.getUser(); + const requestHandler = getSelf(); await requestHandler(mockReq, mockRes, mockNext); @@ -84,7 +84,7 @@ describe('getUser', () => { }); try { - const requestHandler = self.getUser(); + const requestHandler = getSelf(); await requestHandler(mockReq, mockRes, mockNext); expect.fail(); diff --git a/api/src/paths/user/self.ts b/api/src/paths/user/self.ts index 9a1333f893..58516a72a5 100644 --- a/api/src/paths/user/self.ts +++ b/api/src/paths/user/self.ts @@ -3,24 +3,12 @@ import { Operation } from 'express-openapi'; import { getDBConnection } from '../../database/db'; import { HTTP400 } from '../../errors/http-error'; import { systemUserSchema } from '../../openapi/schemas/user'; -import { authorizeRequestHandler } from '../../request-handlers/security/authorization'; import { UserService } from '../../services/user-service'; import { getLogger } from '../../utils/logger'; -const defaultLog = getLogger('paths/user/{userId}'); +const defaultLog = getLogger('paths/user/self'); -export const GET: Operation = [ - authorizeRequestHandler(() => { - return { - and: [ - { - discriminator: 'SystemUser' - } - ] - }; - }), - getUser() -]; +export const GET: Operation = [getSelf()]; GET.apiDoc = { description: 'Get user details for the currently authenticated user.', @@ -64,29 +52,29 @@ GET.apiDoc = { * * @returns {RequestHandler} */ -export function getUser(): RequestHandler { +export function getSelf(): RequestHandler { return async (req, res) => { const connection = getDBConnection(req.keycloak_token); try { await connection.open(); - const userId = connection.systemUserId(); + const systemUserId = connection.systemUserId(); - if (!userId) { + if (!systemUserId) { throw new HTTP400('Failed to identify system user ID'); } const userService = new UserService(connection); // Fetch system user record - const userObject = await userService.getUserById(userId); + const userObject = await userService.getUserById(systemUserId); await connection.commit(); return res.status(200).json(userObject); } catch (error) { - defaultLog.error({ label: 'getUser', message: 'error', error }); + defaultLog.error({ label: 'getSelf', message: 'error', error }); await connection.rollback(); throw error; } finally { diff --git a/app/src/features/summary/list-data/project/ProjectsListContainer.tsx b/app/src/features/summary/list-data/project/ProjectsListContainer.tsx index 31ac59fda6..fb46c2b153 100644 --- a/app/src/features/summary/list-data/project/ProjectsListContainer.tsx +++ b/app/src/features/summary/list-data/project/ProjectsListContainer.tsx @@ -159,6 +159,7 @@ const ProjectsListContainer = (props: IProjectsListContainerProps) => { {params.row.members.map((member) => ( { AS $$ DECLARE _system_user system_user%rowtype; - _user_identity_source_id user_identity_source.user_identity_source_id%type; BEGIN -- Attempt to find user based on guid SELECT * INTO _system_user FROM system_user @@ -46,14 +46,15 @@ export async function seed(knex: Knex): Promise { -- Otherwise, attempt to find user based on identifier and identity source IF NOT found THEN - SELECT user_identity_source_id INTO strict _user_identity_source_id FROM user_identity_source - WHERE LOWER(name) = LOWER(p_user_identity_source_name) - AND record_end_date IS NULL; - SELECT * INTO _system_user FROM system_user - WHERE user_identity_source_id = _user_identity_source_id - AND LOWER(user_identifier) = LOWER(p_user_identifier) - LIMIT 1; + WHERE user_identity_source_id = ( + SELECT user_identity_source_id FROM user_identity_source + WHERE LOWER(name) = LOWER(p_user_identity_source_name) + AND record_end_date IS NULL + ) + AND LOWER(user_identifier) = LOWER(p_user_identifier) + AND record_end_date IS NULL + LIMIT 1; END IF; -- If no user found, return and do nothing @@ -73,13 +74,13 @@ export async function seed(knex: Knex): Promise { WHERE system_user_id = _system_user.system_user_id AND ( - user_guid != p_system_user_guid OR - user_identifier != p_user_identifier OR - email != p_email OR - display_name != p_display_name OR - given_name != p_given_name OR - family_name != p_family_name OR - agency != p_agency + user_guid IS DISTINCT FROM p_system_user_guid OR + user_identifier IS DISTINCT FROM p_user_identifier OR + email IS DISTINCT FROM p_email OR + display_name IS DISTINCT FROM p_display_name OR + given_name IS DISTINCT FROM p_given_name OR + family_name IS DISTINCT FROM p_family_name OR + agency IS DISTINCT FROM p_agency ); -- Return system user id of patched record