From 651d45ab5193b88062077adc181d92d7cf710450 Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Fri, 27 Jan 2023 13:55:35 -0800 Subject: [PATCH 01/13] BHBC-2144: Initial commit --- api/src/database/db.ts | 8 +++-- api/src/paths/user/add.ts | 12 +++---- api/src/paths/user/list.ts | 8 ++++- .../queries/database/user-context-queries.ts | 27 ++++++++++++-- api/src/repositories/user-repository.ts | 6 ++-- api/src/services/user-service.ts | 10 +++--- .../admin/users/AddSystemUsersForm.tsx | 4 +-- app/src/hooks/api/useAdminApi.ts | 7 ++-- app/src/interfaces/useAdminApi.interface.ts | 2 +- ...0230127000000_user_guid_null_constraint.ts | 35 +++++++++++++++++++ database/src/seeds/01_db_system_users.ts | 2 +- 11 files changed, 92 insertions(+), 29 deletions(-) create mode 100644 database/src/migrations/20230127000000_user_guid_null_constraint.ts diff --git a/api/src/database/db.ts b/api/src/database/db.ts index a6e3f69942..e7b1b522bb 100644 --- a/api/src/database/db.ts +++ b/api/src/database/db.ts @@ -3,7 +3,7 @@ import * as pg from 'pg'; import { SQLStatement } from 'sql-template-strings'; import { ApiExecuteSQLError, ApiGeneralError } from '../errors/api-error'; import { queries } from '../queries/queries'; -import { getUserGuid, getUserIdentitySource } from '../utils/keycloak-utils'; +import { getUserGuid, getUserIdentifier, getUserIdentitySource } from '../utils/keycloak-utils'; import { getLogger } from '../utils/logger'; const defaultLog = getLogger('database/db'); @@ -314,14 +314,16 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection { */ const _setUserContext = async () => { const userGuid = getUserGuid(_token); + const userIdentifier = getUserIdentifier(_token); const userIdentitySource = getUserIdentitySource(_token); + defaultLog.debug({ label: '_setUserContext', userGuid, userIdentifier, userIdentitySource }); - if (!userGuid || !userIdentitySource) { + if (!userGuid || !userIdentifier || !userIdentitySource) { throw new ApiGeneralError('Failed to identify authenticated user'); } // Set the user context for all queries made using this connection - const setSystemUserContextSQLStatement = queries.database.setSystemUserContextSQL(userGuid, userIdentitySource); + const setSystemUserContextSQLStatement = queries.database.setSystemUserContextSQL(userGuid, userIdentifier, userIdentitySource); if (!setSystemUserContextSQLStatement) { throw new ApiExecuteSQLError('Failed to build SQL user context statement'); diff --git a/api/src/paths/user/add.ts b/api/src/paths/user/add.ts index 8fff5ada6c..d8cb53b372 100644 --- a/api/src/paths/user/add.ts +++ b/api/src/paths/user/add.ts @@ -39,7 +39,7 @@ POST.apiDoc = { schema: { title: 'User Response Object', type: 'object', - required: ['userGuid', 'userIdentifier', 'identitySource', 'roleId'], + required: ['userIdentifier', 'identitySource', 'roleId'], properties: { userGuid: { type: 'string', @@ -97,16 +97,12 @@ export function addSystemRoleUser(): RequestHandler { return async (req, res) => { const connection = getDBConnection(req['keycloak_token']); - const userGuid = req.body?.userGuid || null; - const userIdentifier = req.body?.userIdentifier || null; - const identitySource = req.body?.identitySource || null; + const userGuid: string | null = req.body?.userGuid || null; + const userIdentifier: string | null = req.body?.userIdentifier || null; + const identitySource: string | null = req.body?.identitySource || null; const roleId = req.body?.roleId || null; - if (!userGuid) { - throw new HTTP400('Missing required body param: userGuid'); - } - if (!userIdentifier) { throw new HTTP400('Missing required body param: userIdentifier'); } diff --git a/api/src/paths/user/list.ts b/api/src/paths/user/list.ts index acfd96d617..3636b28f9a 100644 --- a/api/src/paths/user/list.ts +++ b/api/src/paths/user/list.ts @@ -40,17 +40,23 @@ GET.apiDoc = { items: { title: 'User Response Object', type: 'object', + required: ['id', 'user_identifier', 'identity_source', 'role_ids', 'role_names'], properties: { id: { type: 'number' }, user_guid: { type: 'string', - description: 'The GUID for the user.' + description: 'The GUID for the user.', + nullable: true }, user_identifier: { type: 'string' }, + identity_source: { + type: 'string', + description: 'The identity source of the user' + }, role_ids: { type: 'array', items: { diff --git a/api/src/queries/database/user-context-queries.ts b/api/src/queries/database/user-context-queries.ts index ca1e64c934..28bc915ad3 100644 --- a/api/src/queries/database/user-context-queries.ts +++ b/api/src/queries/database/user-context-queries.ts @@ -1,13 +1,36 @@ import { SQL, SQLStatement } from 'sql-template-strings'; import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; +/** + * Returns an SQL query for setting the user's context. As a side-effect, it first updates the user's + * `user_guid` to reflect the given `userGuid` value, but only if it is currently `null` (which happens + * when new system users are created and a GUID is not provided). + * + * @param {string} userGuid the GUID of the user + * @param {string} userIdentifier the user's identifier + * @param {string} systemUserType The user type + * @returns {*} {SQLStatement | null} The SQL statement for setting the user's context, or `null` if + * userGuid or userIdentifier are undefinedd. + */ export const setSystemUserContextSQL = ( userGuid: string, + userIdentifier: string, systemUserType: SYSTEM_IDENTITY_SOURCE ): SQLStatement | null => { - if (!userGuid) { + if (!userGuid || !userIdentifier) { return null; } - return SQL`select api_set_context(${userGuid}, ${systemUserType});`; + return SQL` + UPDATE + system_user + SET + user_guid = ${userGuid} + WHERE + user_guid IS NULL + AND + user_identifier = ${userIdentifier} + ; + SELECT api_set_context(${userGuid}, ${systemUserType}); + `; }; diff --git a/api/src/repositories/user-repository.ts b/api/src/repositories/user-repository.ts index 8faa612b9e..b694b56058 100644 --- a/api/src/repositories/user-repository.ts +++ b/api/src/repositories/user-repository.ts @@ -153,13 +153,13 @@ export class UserRepository extends BaseRepository { * * Note: Will fail if the system user already exists. * - * @param {string} userGuid + * @param {string | null} userGuid * @param {string} userIdentifier * @param {string} identitySource * @return {*} {Promise} * @memberof UserRepository */ - async addSystemUser(userGuid: string, userIdentifier: string, identitySource: string): Promise { + async addSystemUser(userGuid: string | null, userIdentifier: string, identitySource: string): Promise { const sqlStatement = SQL` INSERT INTO system_user @@ -170,7 +170,7 @@ export class UserRepository extends BaseRepository { record_effective_date ) VALUES ( - ${userGuid.toLowerCase()}, + ${userGuid ? userGuid.toLowerCase() : null}, ( SELECT user_identity_source_id diff --git a/api/src/services/user-service.ts b/api/src/services/user-service.ts index 8a40347d1a..f95c25a944 100644 --- a/api/src/services/user-service.ts +++ b/api/src/services/user-service.ts @@ -65,13 +65,13 @@ export class UserService extends DBService { * * Note: Will fail if the system user already exists. * - * @param {string} userGuid + * @param {string | null} userGuid * @param {string} userIdentifier * @param {string} identitySource * @return {*} {Promise} * @memberof UserService */ - async addSystemUser(userGuid: string, userIdentifier: string, identitySource: string): Promise { + async addSystemUser(userGuid: string | null, userIdentifier: string, identitySource: string): Promise { const response = await this.userRepository.addSystemUser(userGuid, userIdentifier, identitySource); return new UserObject(response); @@ -93,15 +93,15 @@ export class UserService extends DBService { * Gets a system user, adding them if they do not already exist, or activating them if they had been deactivated (soft * deleted). * - * @param {string} userGuid + * @param {string | null} userGuid * @param {string} userIdentifier * @param {string} identitySource * @return {*} {Promise} * @memberof UserService */ - async ensureSystemUser(userGuid: string, userIdentifier: string, identitySource: string): Promise { + async ensureSystemUser(userGuid: string | null, userIdentifier: string, identitySource: string): Promise { // Check if the user exists in SIMS - let userObject = await this.getUserByGuid(userGuid); + let userObject = userGuid && await this.getUserByGuid(userGuid); if (!userObject) { // Id of the current authenticated user diff --git a/app/src/features/admin/users/AddSystemUsersForm.tsx b/app/src/features/admin/users/AddSystemUsersForm.tsx index 62e8d6c17f..21587b57d6 100644 --- a/app/src/features/admin/users/AddSystemUsersForm.tsx +++ b/app/src/features/admin/users/AddSystemUsersForm.tsx @@ -16,7 +16,7 @@ import React from 'react'; import yup from 'utils/YupSchema'; export interface IAddSystemUsersFormArrayItem { - userGuid: string; + userGuid: string | null; userIdentifier: string; identitySource: string; systemRole: number; @@ -41,7 +41,7 @@ export const AddSystemUsersFormYupSchema = yup.object().shape({ systemUsers: yup.array().of( yup.object().shape({ userIdentifier: yup.string().required('Username is required'), - userGuid: yup.string().required('GUID is required'), + userGuid: yup.string(), // .required('GUID is required'), identitySource: yup.string().required('Login Method is required'), systemRole: yup.number().required('Role is required') }) diff --git a/app/src/hooks/api/useAdminApi.ts b/app/src/hooks/api/useAdminApi.ts index 0d361aaf00..c85755565c 100644 --- a/app/src/hooks/api/useAdminApi.ts +++ b/app/src/hooks/api/useAdminApi.ts @@ -57,7 +57,7 @@ const useAdminApi = (axios: AxiosInstance) => { const approveAccessRequest = async ( administrativeActivityId: number, - userGuid: string, + userGuid: string | null, userIdentifier: string, identitySource: string, roleIds: number[] = [] @@ -108,14 +108,15 @@ const useAdminApi = (axios: AxiosInstance) => { * * Note: Will fail if the system user already exists. * - * @param {string} userGuid + * @param {string | null} userGuid The user's GUID. If `null` is given, we expect it to be updated upon the user's + * next login. * @param {string} userIdentifier * @param {string} identitySource * @param {number} roleId * @return {*} {boolean} True if the user is successfully added, false otherwise. */ const addSystemUser = async ( - userGuid: string, + userGuid: string | null, userIdentifier: string, identitySource: string, roleId: number diff --git a/app/src/interfaces/useAdminApi.interface.ts b/app/src/interfaces/useAdminApi.interface.ts index 025b484421..8c407134c0 100644 --- a/app/src/interfaces/useAdminApi.interface.ts +++ b/app/src/interfaces/useAdminApi.interface.ts @@ -14,7 +14,7 @@ export type IBCeIDBusinessAccessRequestDataObject = { export type IAccessRequestDataObject = { name: string; - userGuid: string; + userGuid: string | null; username: string; email: string; identitySource: string; diff --git a/database/src/migrations/20230127000000_user_guid_null_constraint.ts b/database/src/migrations/20230127000000_user_guid_null_constraint.ts new file mode 100644 index 0000000000..df7cf77652 --- /dev/null +++ b/database/src/migrations/20230127000000_user_guid_null_constraint.ts @@ -0,0 +1,35 @@ +import { Knex } from 'knex'; + +const DB_SCHEMA = process.env.DB_SCHEMA; +const DB_SCHEMA_DAPI_V1 = process.env.DB_SCHEMA_DAPI_V1; + +/** + * Removes the NULL constraint on user_guid from the system_user table. + * + * @export + * @param {Knex} knex + * @return {*} {Promise} + */ +export async function up(knex: Knex): Promise { + await knex.raw(` + SET search_path = ${DB_SCHEMA_DAPI_V1}; + + DROP VIEW system_user; + + SET search_path = ${DB_SCHEMA}; + + ALTER TABLE system_user ALTER COLUMN user_guid DROP NOT NULL; + + SET search_path = ${DB_SCHEMA_DAPI_V1}; + + CREATE OR REPLACE VIEW system_user AS SELECT * FROM ${DB_SCHEMA}.system_user; + `); +} + +/** + * Not implemented. + * @param knex + */ +export async function down(knex: Knex): Promise { + await knex.raw(``); +} diff --git a/database/src/seeds/01_db_system_users.ts b/database/src/seeds/01_db_system_users.ts index 804bb629c7..668e99ca22 100644 --- a/database/src/seeds/01_db_system_users.ts +++ b/database/src/seeds/01_db_system_users.ts @@ -144,7 +144,7 @@ const insertSystemUserSQL = (userIdentifier: string, userType: string, userGuid: SELECT user_identity_source_id, '${userIdentifier}', - '${userGuid}', + '${userGuid.toLowerCase()}', now(), now(), (SELECT system_user_id from system_user where user_identifier = '${DB_ADMIN}') From 01c8bf473757c3adb0f4bdbda5a66d29c7622acf Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Fri, 27 Jan 2023 14:23:11 -0800 Subject: [PATCH 02/13] BHBC-2144: Update frontend to remove user GUID --- api/src/database/db.ts | 1 + .../queries/database/user-context-queries.ts | 21 +++++++++++-------- api/src/utils/keycloak-utils.ts | 5 ++++- .../features/admin/users/ActiveUsersList.tsx | 1 - .../admin/users/AddSystemUsersForm.tsx | 20 ++---------------- app/src/hooks/api/useAdminApi.ts | 9 ++++---- 6 files changed, 24 insertions(+), 33 deletions(-) diff --git a/api/src/database/db.ts b/api/src/database/db.ts index e7b1b522bb..4bef6a8ae3 100644 --- a/api/src/database/db.ts +++ b/api/src/database/db.ts @@ -364,6 +364,7 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection { export const getAPIUserDBConnection = (): IDBConnection => { return getDBConnection({ preferred_username: `${DB_USERNAME}@database`, + sims_system_username: DB_USERNAME, identity_provider: 'database' }); }; diff --git a/api/src/queries/database/user-context-queries.ts b/api/src/queries/database/user-context-queries.ts index 28bc915ad3..337c5d9629 100644 --- a/api/src/queries/database/user-context-queries.ts +++ b/api/src/queries/database/user-context-queries.ts @@ -22,15 +22,18 @@ export const setSystemUserContextSQL = ( } return SQL` - UPDATE - system_user - SET - user_guid = ${userGuid} - WHERE - user_guid IS NULL - AND - user_identifier = ${userIdentifier} - ; + WITH patch_user_guid AS ( + UPDATE + system_user + SET + user_guid = ${userGuid} + WHERE + user_guid IS NULL + AND + user_identifier = ${userIdentifier} + RETURNING + * + ) SELECT api_set_context(${userGuid}, ${systemUserType}); `; }; diff --git a/api/src/utils/keycloak-utils.ts b/api/src/utils/keycloak-utils.ts index b69ee19eb1..ad2f923ccc 100644 --- a/api/src/utils/keycloak-utils.ts +++ b/api/src/utils/keycloak-utils.ts @@ -79,7 +79,10 @@ export const coerceUserIdentitySource = (userIdentitySource: string | null): SYS * @return {*} {(string | null)} */ export const getUserIdentifier = (keycloakToken: object): string | null => { - const userIdentifier = keycloakToken?.['idir_username'] || keycloakToken?.['bceid_username']; + const userIdentifier = + keycloakToken?.['idir_username'] || + keycloakToken?.['bceid_username'] || + keycloakToken?.['sims_system_username']; if (!userIdentifier) { return null; diff --git a/app/src/features/admin/users/ActiveUsersList.tsx b/app/src/features/admin/users/ActiveUsersList.tsx index d5e4d19a65..0cc90da0b7 100644 --- a/app/src/features/admin/users/ActiveUsersList.tsx +++ b/app/src/features/admin/users/ActiveUsersList.tsx @@ -210,7 +210,6 @@ const ActiveUsersList: React.FC = (props) => { try { for (const systemUser of values.systemUsers) { await biohubApi.admin.addSystemUser( - systemUser.userGuid, systemUser.userIdentifier, systemUser.identitySource, systemUser.systemRole diff --git a/app/src/features/admin/users/AddSystemUsersForm.tsx b/app/src/features/admin/users/AddSystemUsersForm.tsx index 21587b57d6..59a8ab5267 100644 --- a/app/src/features/admin/users/AddSystemUsersForm.tsx +++ b/app/src/features/admin/users/AddSystemUsersForm.tsx @@ -16,7 +16,6 @@ import React from 'react'; import yup from 'utils/YupSchema'; export interface IAddSystemUsersFormArrayItem { - userGuid: string | null; userIdentifier: string; identitySource: string; systemRole: number; @@ -27,7 +26,6 @@ export interface IAddSystemUsersForm { } export const AddSystemUsersFormArrayItemInitialValues: IAddSystemUsersFormArrayItem = { - userGuid: '', userIdentifier: '', identitySource: '', systemRole: ('' as unknown) as number @@ -41,7 +39,6 @@ export const AddSystemUsersFormYupSchema = yup.object().shape({ systemUsers: yup.array().of( yup.object().shape({ userIdentifier: yup.string().required('Username is required'), - userGuid: yup.string(), // .required('GUID is required'), identitySource: yup.string().required('Login Method is required'), systemRole: yup.number().required('Role is required') }) @@ -63,7 +60,6 @@ const AddSystemUsersForm: React.FC = (props) => { {values.systemUsers?.map((systemUser: IAddSystemUsersFormArrayItem, index: number) => { - const userGuidMeta = getFieldMeta(`systemUsers.[${index}].userGuid`); const userIdentifierMeta = getFieldMeta(`systemUsers.[${index}].userIdentifier`); const identitySourceMeta = getFieldMeta(`systemUsers.[${index}].identitySource`); const systemRoleMeta = getFieldMeta(`systemUsers.[${index}].roleId`); @@ -71,7 +67,7 @@ const AddSystemUsersForm: React.FC = (props) => { return ( - + = (props) => { }} /> - - - @@ -128,7 +112,7 @@ const AddSystemUsersForm: React.FC = (props) => { {identitySourceMeta.touched && identitySourceMeta.error} - + System Role diff --git a/app/src/hooks/api/useAdminApi.ts b/app/src/hooks/api/useAdminApi.ts index c85755565c..40ffa9a4aa 100644 --- a/app/src/hooks/api/useAdminApi.ts +++ b/app/src/hooks/api/useAdminApi.ts @@ -108,20 +108,21 @@ const useAdminApi = (axios: AxiosInstance) => { * * Note: Will fail if the system user already exists. * - * @param {string | null} userGuid The user's GUID. If `null` is given, we expect it to be updated upon the user's - * next login. * @param {string} userIdentifier * @param {string} identitySource * @param {number} roleId * @return {*} {boolean} True if the user is successfully added, false otherwise. */ const addSystemUser = async ( - userGuid: string | null, userIdentifier: string, identitySource: string, roleId: number ): Promise => { - const { status } = await axios.post(`/api/user/add`, { userGuid, identitySource, userIdentifier, roleId }); + const { status } = await axios.post(`/api/user/add`, { + identitySource, + userIdentifier, + roleId + }); return status === 200; }; From 7b7584e965ada7a9afb5c6d63a61e73226acbcae Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Fri, 27 Jan 2023 15:16:35 -0800 Subject: [PATCH 03/13] BHBC-2144: Update tests --- api/src/database/db.test.ts | 2 +- .../queries/database/user-context-queries.test.ts | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/api/src/database/db.test.ts b/api/src/database/db.test.ts index f9eaaada5f..b14f6670af 100644 --- a/api/src/database/db.test.ts +++ b/api/src/database/db.test.ts @@ -84,7 +84,7 @@ describe('db', () => { expect(getDBPoolStub).to.have.been.calledOnce; expect(connectStub).to.have.been.calledOnce; - const expectedSystemUserContextSQL = setSystemUserContextSQL('testguid', SYSTEM_IDENTITY_SOURCE.IDIR); + const expectedSystemUserContextSQL = setSystemUserContextSQL('testguid', 'testidentifier', SYSTEM_IDENTITY_SOURCE.IDIR); expect(queryStub).to.have.been.calledWith( expectedSystemUserContextSQL?.text, expectedSystemUserContextSQL?.values diff --git a/api/src/queries/database/user-context-queries.test.ts b/api/src/queries/database/user-context-queries.test.ts index 4d3f822163..d888f1337a 100644 --- a/api/src/queries/database/user-context-queries.test.ts +++ b/api/src/queries/database/user-context-queries.test.ts @@ -4,26 +4,32 @@ import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; import { setSystemUserContextSQL } from './user-context-queries'; describe('setSystemUserContextSQL', () => { + it('has empty userGuid', () => { + const response = setSystemUserContextSQL('', 'user-identifier', SYSTEM_IDENTITY_SOURCE.IDIR); + + expect(response).to.be.null; + }); + it('has empty userIdentifier', () => { - const response = setSystemUserContextSQL('', SYSTEM_IDENTITY_SOURCE.IDIR); + const response = setSystemUserContextSQL('user-guid', '', SYSTEM_IDENTITY_SOURCE.IDIR); expect(response).to.be.null; }); it('identifies an IDIR user', () => { - const response = setSystemUserContextSQL('idir-user', SYSTEM_IDENTITY_SOURCE.IDIR); + const response = setSystemUserContextSQL('idir-user', 'user-identifier', SYSTEM_IDENTITY_SOURCE.IDIR); expect(response).not.to.be.null; }); it('identifies a BCEID basic user', () => { - const response = setSystemUserContextSQL('bceid-basic-user', SYSTEM_IDENTITY_SOURCE.BCEID_BASIC); + const response = setSystemUserContextSQL('bceid-basic-user', 'user-identifier', SYSTEM_IDENTITY_SOURCE.BCEID_BASIC); expect(response).not.to.be.null; }); it('identifies a BCEID business user', () => { - const response = setSystemUserContextSQL('bceid-business-user', SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS); + const response = setSystemUserContextSQL('bceid-business-user', 'user-identifier', SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS); expect(response).not.to.be.null; }); From 179b49f3b21a16c2e869830e60d7b67f3b7e00ad Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Fri, 27 Jan 2023 22:00:52 -0800 Subject: [PATCH 04/13] BHBC-2144: Separate queries --- api/src/database/db.test.ts | 2 +- api/src/database/db.ts | 14 +++++- .../{projectId}/participants/create.ts | 25 +++++----- .../database/user-context-queries.test.ts | 18 ++++--- .../queries/database/user-context-queries.ts | 48 ++++++++++++------- api/src/services/user-service.ts | 4 +- 6 files changed, 68 insertions(+), 43 deletions(-) diff --git a/api/src/database/db.test.ts b/api/src/database/db.test.ts index b14f6670af..f9eaaada5f 100644 --- a/api/src/database/db.test.ts +++ b/api/src/database/db.test.ts @@ -84,7 +84,7 @@ describe('db', () => { expect(getDBPoolStub).to.have.been.calledOnce; expect(connectStub).to.have.been.calledOnce; - const expectedSystemUserContextSQL = setSystemUserContextSQL('testguid', 'testidentifier', SYSTEM_IDENTITY_SOURCE.IDIR); + const expectedSystemUserContextSQL = setSystemUserContextSQL('testguid', SYSTEM_IDENTITY_SOURCE.IDIR); expect(queryStub).to.have.been.calledWith( expectedSystemUserContextSQL?.text, expectedSystemUserContextSQL?.values diff --git a/api/src/database/db.ts b/api/src/database/db.ts index 4bef6a8ae3..5965fa89ec 100644 --- a/api/src/database/db.ts +++ b/api/src/database/db.ts @@ -322,14 +322,26 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection { throw new ApiGeneralError('Failed to identify authenticated user'); } + // Patch user GUID + const patchUserGuidSqlStatement = queries.database.patchUserGuidSQL(userGuid, userIdentifier); + + if (!patchUserGuidSqlStatement) { + throw new ApiExecuteSQLError('Failed to build SQL patch user GUID statement'); + } + // Set the user context for all queries made using this connection - const setSystemUserContextSQLStatement = queries.database.setSystemUserContextSQL(userGuid, userIdentifier, userIdentitySource); + const setSystemUserContextSQLStatement = queries.database.setSystemUserContextSQL(userGuid, userIdentitySource); if (!setSystemUserContextSQLStatement) { throw new ApiExecuteSQLError('Failed to build SQL user context statement'); } try { + await _client.query( + patchUserGuidSqlStatement.text, + patchUserGuidSqlStatement.values + ); + const response = await _client.query( setSystemUserContextSQLStatement.text, setSystemUserContextSQLStatement.values diff --git a/api/src/paths/project/{projectId}/participants/create.ts b/api/src/paths/project/{projectId}/participants/create.ts index 94838b2708..4f6e9d065a 100644 --- a/api/src/paths/project/{projectId}/participants/create.ts +++ b/api/src/paths/project/{projectId}/participants/create.ts @@ -56,12 +56,8 @@ POST.apiDoc = { type: 'array', items: { type: 'object', - required: ['userGuid', 'userIdentifier', 'identitySource', 'roleId'], + required: ['userIdentifier', 'identitySource', 'roleId'], properties: { - userGuid: { - type: 'string', - description: 'The GUID for the user.' - }, userIdentifier: { description: 'A IDIR or BCEID username.', type: 'string' @@ -108,6 +104,8 @@ POST.apiDoc = { } }; +type Participant = { userIdentifier: string; identitySource: string; roleId: number } + export function createProjectParticipants(): RequestHandler { return async (req, res) => { const projectId = Number(req.params.projectId); @@ -123,16 +121,17 @@ export function createProjectParticipants(): RequestHandler { const connection = getDBConnection(req['keycloak_token']); try { - const participants: { userGuid: string; userIdentifier: string; identitySource: string; roleId: number }[] = - req.body.participants; + const participants: Participant[] = req.body.participants; await connection.open(); - const promises: Promise[] = []; - - participants.forEach((participant) => - promises.push(ensureSystemUserAndProjectParticipantUser(projectId, participant, connection)) - ); + const promises: Promise[] = participants.map((participant) => { + return ensureSystemUserAndProjectParticipantUser( + projectId, + { ...participant, userGuid: null }, + connection + ); + }); await Promise.all(promises); @@ -150,7 +149,7 @@ export function createProjectParticipants(): RequestHandler { export const ensureSystemUserAndProjectParticipantUser = async ( projectId: number, - participant: { userGuid: string; userIdentifier: string; identitySource: string; roleId: number }, + participant: Participant & { userGuid: string | null }, connection: IDBConnection ) => { const userService = new UserService(connection); diff --git a/api/src/queries/database/user-context-queries.test.ts b/api/src/queries/database/user-context-queries.test.ts index d888f1337a..38393419af 100644 --- a/api/src/queries/database/user-context-queries.test.ts +++ b/api/src/queries/database/user-context-queries.test.ts @@ -5,32 +5,30 @@ import { setSystemUserContextSQL } from './user-context-queries'; describe('setSystemUserContextSQL', () => { it('has empty userGuid', () => { - const response = setSystemUserContextSQL('', 'user-identifier', SYSTEM_IDENTITY_SOURCE.IDIR); - - expect(response).to.be.null; - }); - - it('has empty userIdentifier', () => { - const response = setSystemUserContextSQL('user-guid', '', SYSTEM_IDENTITY_SOURCE.IDIR); + const response = setSystemUserContextSQL('', SYSTEM_IDENTITY_SOURCE.IDIR); expect(response).to.be.null; }); it('identifies an IDIR user', () => { - const response = setSystemUserContextSQL('idir-user', 'user-identifier', SYSTEM_IDENTITY_SOURCE.IDIR); + const response = setSystemUserContextSQL('idir-user', SYSTEM_IDENTITY_SOURCE.IDIR); expect(response).not.to.be.null; }); it('identifies a BCEID basic user', () => { - const response = setSystemUserContextSQL('bceid-basic-user', 'user-identifier', SYSTEM_IDENTITY_SOURCE.BCEID_BASIC); + const response = setSystemUserContextSQL('bceid-basic-user', SYSTEM_IDENTITY_SOURCE.BCEID_BASIC); expect(response).not.to.be.null; }); it('identifies a BCEID business user', () => { - const response = setSystemUserContextSQL('bceid-business-user', 'user-identifier', SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS); + const response = setSystemUserContextSQL('bceid-business-user', SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS); expect(response).not.to.be.null; }); }); + +describe('patchUserGuidSQL', () => { + // +}); diff --git a/api/src/queries/database/user-context-queries.ts b/api/src/queries/database/user-context-queries.ts index 337c5d9629..7a9806f60c 100644 --- a/api/src/queries/database/user-context-queries.ts +++ b/api/src/queries/database/user-context-queries.ts @@ -8,32 +8,46 @@ import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; * * @param {string} userGuid the GUID of the user * @param {string} userIdentifier the user's identifier - * @param {string} systemUserType The user type + * @param {string} userIdentitySource The user's identity source * @returns {*} {SQLStatement | null} The SQL statement for setting the user's context, or `null` if * userGuid or userIdentifier are undefinedd. */ export const setSystemUserContextSQL = ( userGuid: string, - userIdentifier: string, - systemUserType: SYSTEM_IDENTITY_SOURCE + userIdentitySource: SYSTEM_IDENTITY_SOURCE ): SQLStatement | null => { - if (!userGuid || !userIdentifier) { + if (!userGuid) { return null; } return SQL` - WITH patch_user_guid AS ( - UPDATE - system_user - SET - user_guid = ${userGuid} - WHERE - user_guid IS NULL - AND - user_identifier = ${userIdentifier} - RETURNING - * - ) - SELECT api_set_context(${userGuid}, ${systemUserType}); + SELECT api_set_context(${userGuid}, ${userIdentitySource}); `; }; + +/** + * + * @param userGuid + * @param userIdentifier + * @returns + */ +export const patchUserGuidSQL = ( + userGuid: string, + userIdentifier: string, +): SQLStatement | null => { + if (!userGuid || !userIdentifier) { + return null; + } + + return SQL` + UPDATE + system_user + SET + user_guid = ${userGuid.toLowerCase()} + WHERE + user_guid IS NULL + AND + user_identifier = ${userIdentifier.toLowerCase()} + ; + ` +} diff --git a/api/src/services/user-service.ts b/api/src/services/user-service.ts index f95c25a944..aa4cd252ab 100644 --- a/api/src/services/user-service.ts +++ b/api/src/services/user-service.ts @@ -101,7 +101,9 @@ export class UserService extends DBService { */ async ensureSystemUser(userGuid: string | null, userIdentifier: string, identitySource: string): Promise { // Check if the user exists in SIMS - let userObject = userGuid && await this.getUserByGuid(userGuid); + let userObject = userGuid + ? await this.getUserByGuid(userGuid) + : await this.getUserByIdentifier(); if (!userObject) { // Id of the current authenticated user From dfb7c018caea771e568f3dc13dbdbe72c08cbc1c Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Sat, 28 Jan 2023 21:23:41 -0800 Subject: [PATCH 05/13] BHBC-2144: Fix project participant bug --- .../{projectId}/participants/create.ts | 2 +- .../project/{projectId}/participants/get.ts | 3 +- api/src/repositories/user-repository.ts | 46 +++++++++++++++++++ api/src/services/user-service.ts | 19 +++++++- 4 files changed, 67 insertions(+), 3 deletions(-) diff --git a/api/src/paths/project/{projectId}/participants/create.ts b/api/src/paths/project/{projectId}/participants/create.ts index 4f6e9d065a..6b0e8ee114 100644 --- a/api/src/paths/project/{projectId}/participants/create.ts +++ b/api/src/paths/project/{projectId}/participants/create.ts @@ -154,7 +154,7 @@ export const ensureSystemUserAndProjectParticipantUser = async ( ) => { const userService = new UserService(connection); - // Add a system user, unless they already have one + // Create or activate the system user const systemUserObject = await userService.ensureSystemUser( participant.userGuid, participant.userIdentifier, diff --git a/api/src/paths/project/{projectId}/participants/get.ts b/api/src/paths/project/{projectId}/participants/get.ts index 47bd3cd7bb..c26d28d8b2 100644 --- a/api/src/paths/project/{projectId}/participants/get.ts +++ b/api/src/paths/project/{projectId}/participants/get.ts @@ -73,7 +73,8 @@ GET.apiDoc = { }, user_guid: { type: 'string', - description: 'The GUID for the user.' + description: 'The GUID for the user.', + nullable: true }, user_identifier: { type: 'string' diff --git a/api/src/repositories/user-repository.ts b/api/src/repositories/user-repository.ts index b694b56058..dbbd480e41 100644 --- a/api/src/repositories/user-repository.ts +++ b/api/src/repositories/user-repository.ts @@ -148,6 +148,52 @@ export class UserRepository extends BaseRepository { return response.rows; } + /** + * @TODO jsdoc + * @param userIdentifier + * @param identitySource + */ + async getUserByIdentifier(userIdentifier: string, identitySource: string): Promise { + const sqlStatement = SQL` + SELECT + su.system_user_id, + su.user_identifier, + su.user_guid, + su.record_end_date, + uis.name AS identity_source, + array_remove(array_agg(sr.system_role_id), NULL) AS role_ids, + array_remove(array_agg(sr.name), NULL) AS role_names + FROM + system_user su + LEFT JOIN + system_user_role sur + ON + su.system_user_id = sur.system_user_id + LEFT JOIN + system_role sr + ON + sur.system_role_id = sr.system_role_id + LEFT JOIN + user_identity_source uis + ON + uis.user_identity_source_id = su.user_identity_source_id + WHERE + su.user_identifier = ${userIdentifier} + AND + uis.name = ${identitySource} + GROUP BY + su.system_user_id, + su.record_end_date, + su.user_identifier, + su.user_guid, + uis.name; + `; + + const response = await this.connection.sql(sqlStatement); + + return response.rows; + } + /** * Adds a new system user. * diff --git a/api/src/services/user-service.ts b/api/src/services/user-service.ts index aa4cd252ab..12f383c374 100644 --- a/api/src/services/user-service.ts +++ b/api/src/services/user-service.ts @@ -60,6 +60,23 @@ export class UserService extends DBService { return new UserObject(response[0]); } + /** + * @TODO jsdoc + * @param userIdentifier + * @param identitySource + */ + async getUserByIdentifier(userIdentifier: string, identitySource: string): Promise { + defaultLog.debug({ label: 'getUserByIdentifier', userIdentifier, identitySource }); + + const response = await this.userRepository.getUserByIdentifier(userIdentifier, identitySource); + + if (response.length !== 1) { + return null; + } + + return new UserObject(response[0]); + } + /** * Adds a new system user. * @@ -103,7 +120,7 @@ export class UserService extends DBService { // Check if the user exists in SIMS let userObject = userGuid ? await this.getUserByGuid(userGuid) - : await this.getUserByIdentifier(); + : await this.getUserByIdentifier(userIdentifier, identitySource); if (!userObject) { // Id of the current authenticated user From 0d620eb36c1c10fe2940fc3e6db1dc386dcc0091 Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Sat, 28 Jan 2023 21:28:57 -0800 Subject: [PATCH 06/13] BHBC-2144: Lint fixes --- api/src/database/db.ts | 5 +---- .../{projectId}/participants/create.ts | 8 ++------ api/src/paths/user/add.ts | 4 ++-- .../queries/database/user-context-queries.ts | 19 ++++++++----------- api/src/repositories/user-repository.ts | 4 ++-- api/src/services/user-service.ts | 4 ++-- api/src/utils/keycloak-utils.ts | 4 +--- app/src/hooks/api/useAdminApi.ts | 6 +----- ...0230127000000_user_guid_null_constraint.ts | 2 +- 9 files changed, 20 insertions(+), 36 deletions(-) diff --git a/api/src/database/db.ts b/api/src/database/db.ts index 5965fa89ec..37bc74c961 100644 --- a/api/src/database/db.ts +++ b/api/src/database/db.ts @@ -337,10 +337,7 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection { } try { - await _client.query( - patchUserGuidSqlStatement.text, - patchUserGuidSqlStatement.values - ); + await _client.query(patchUserGuidSqlStatement.text, patchUserGuidSqlStatement.values); const response = await _client.query( setSystemUserContextSQLStatement.text, diff --git a/api/src/paths/project/{projectId}/participants/create.ts b/api/src/paths/project/{projectId}/participants/create.ts index 6b0e8ee114..310059c074 100644 --- a/api/src/paths/project/{projectId}/participants/create.ts +++ b/api/src/paths/project/{projectId}/participants/create.ts @@ -104,7 +104,7 @@ POST.apiDoc = { } }; -type Participant = { userIdentifier: string; identitySource: string; roleId: number } +type Participant = { userIdentifier: string; identitySource: string; roleId: number }; export function createProjectParticipants(): RequestHandler { return async (req, res) => { @@ -126,11 +126,7 @@ export function createProjectParticipants(): RequestHandler { await connection.open(); const promises: Promise[] = participants.map((participant) => { - return ensureSystemUserAndProjectParticipantUser( - projectId, - { ...participant, userGuid: null }, - connection - ); + return ensureSystemUserAndProjectParticipantUser(projectId, { ...participant, userGuid: null }, connection); }); await Promise.all(promises); diff --git a/api/src/paths/user/add.ts b/api/src/paths/user/add.ts index d8cb53b372..336d1ced52 100644 --- a/api/src/paths/user/add.ts +++ b/api/src/paths/user/add.ts @@ -98,8 +98,8 @@ export function addSystemRoleUser(): RequestHandler { const connection = getDBConnection(req['keycloak_token']); const userGuid: string | null = req.body?.userGuid || null; - const userIdentifier: string | null = req.body?.userIdentifier || null; - const identitySource: string | null = req.body?.identitySource || null; + const userIdentifier: string | null = req.body?.userIdentifier || null; + const identitySource: string | null = req.body?.identitySource || null; const roleId = req.body?.roleId || null; diff --git a/api/src/queries/database/user-context-queries.ts b/api/src/queries/database/user-context-queries.ts index 7a9806f60c..fcc28d15fc 100644 --- a/api/src/queries/database/user-context-queries.ts +++ b/api/src/queries/database/user-context-queries.ts @@ -5,7 +5,7 @@ import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; * Returns an SQL query for setting the user's context. As a side-effect, it first updates the user's * `user_guid` to reflect the given `userGuid` value, but only if it is currently `null` (which happens * when new system users are created and a GUID is not provided). - * + * * @param {string} userGuid the GUID of the user * @param {string} userIdentifier the user's identifier * @param {string} userIdentitySource The user's identity source @@ -26,15 +26,12 @@ export const setSystemUserContextSQL = ( }; /** - * - * @param userGuid - * @param userIdentifier - * @returns + * + * @param userGuid + * @param userIdentifier + * @returns */ -export const patchUserGuidSQL = ( - userGuid: string, - userIdentifier: string, -): SQLStatement | null => { +export const patchUserGuidSQL = (userGuid: string, userIdentifier: string): SQLStatement | null => { if (!userGuid || !userIdentifier) { return null; } @@ -49,5 +46,5 @@ export const patchUserGuidSQL = ( AND user_identifier = ${userIdentifier.toLowerCase()} ; - ` -} + `; +}; diff --git a/api/src/repositories/user-repository.ts b/api/src/repositories/user-repository.ts index dbbd480e41..6553e61c65 100644 --- a/api/src/repositories/user-repository.ts +++ b/api/src/repositories/user-repository.ts @@ -150,8 +150,8 @@ export class UserRepository extends BaseRepository { /** * @TODO jsdoc - * @param userIdentifier - * @param identitySource + * @param userIdentifier + * @param identitySource */ async getUserByIdentifier(userIdentifier: string, identitySource: string): Promise { const sqlStatement = SQL` diff --git a/api/src/services/user-service.ts b/api/src/services/user-service.ts index 12f383c374..3710435c34 100644 --- a/api/src/services/user-service.ts +++ b/api/src/services/user-service.ts @@ -62,8 +62,8 @@ export class UserService extends DBService { /** * @TODO jsdoc - * @param userIdentifier - * @param identitySource + * @param userIdentifier + * @param identitySource */ async getUserByIdentifier(userIdentifier: string, identitySource: string): Promise { defaultLog.debug({ label: 'getUserByIdentifier', userIdentifier, identitySource }); diff --git a/api/src/utils/keycloak-utils.ts b/api/src/utils/keycloak-utils.ts index ad2f923ccc..b7869932b2 100644 --- a/api/src/utils/keycloak-utils.ts +++ b/api/src/utils/keycloak-utils.ts @@ -80,9 +80,7 @@ export const coerceUserIdentitySource = (userIdentitySource: string | null): SYS */ export const getUserIdentifier = (keycloakToken: object): string | null => { const userIdentifier = - keycloakToken?.['idir_username'] || - keycloakToken?.['bceid_username'] || - keycloakToken?.['sims_system_username']; + keycloakToken?.['idir_username'] || keycloakToken?.['bceid_username'] || keycloakToken?.['sims_system_username']; if (!userIdentifier) { return null; diff --git a/app/src/hooks/api/useAdminApi.ts b/app/src/hooks/api/useAdminApi.ts index 40ffa9a4aa..2923710163 100644 --- a/app/src/hooks/api/useAdminApi.ts +++ b/app/src/hooks/api/useAdminApi.ts @@ -113,11 +113,7 @@ const useAdminApi = (axios: AxiosInstance) => { * @param {number} roleId * @return {*} {boolean} True if the user is successfully added, false otherwise. */ - const addSystemUser = async ( - userIdentifier: string, - identitySource: string, - roleId: number - ): Promise => { + const addSystemUser = async (userIdentifier: string, identitySource: string, roleId: number): Promise => { const { status } = await axios.post(`/api/user/add`, { identitySource, userIdentifier, diff --git a/database/src/migrations/20230127000000_user_guid_null_constraint.ts b/database/src/migrations/20230127000000_user_guid_null_constraint.ts index df7cf77652..c1da4fbbdb 100644 --- a/database/src/migrations/20230127000000_user_guid_null_constraint.ts +++ b/database/src/migrations/20230127000000_user_guid_null_constraint.ts @@ -28,7 +28,7 @@ export async function up(knex: Knex): Promise { /** * Not implemented. - * @param knex + * @param knex */ export async function down(knex: Knex): Promise { await knex.raw(``); From c7f3b8f04abf49b82534478eb99605861fabf527 Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Sat, 28 Jan 2023 21:41:58 -0800 Subject: [PATCH 07/13] BHBC-2144: Fix some tests, user model defn --- api/src/database/db.test.ts | 5 +++ api/src/models/user.ts | 2 +- api/src/paths/user/add.test.ts | 56 +++++++++++++++---------- api/src/repositories/user-repository.ts | 11 +++-- api/src/services/user-service.ts | 9 ++-- 5 files changed, 53 insertions(+), 30 deletions(-) diff --git a/api/src/database/db.test.ts b/api/src/database/db.test.ts index f9eaaada5f..8323be7f91 100644 --- a/api/src/database/db.test.ts +++ b/api/src/database/db.test.ts @@ -360,6 +360,10 @@ describe('db', () => { }); describe('getAPIUserDBConnection', () => { + beforeEach(() => { + process.env.DB_USER_API = 'example_db_username'; + }); + afterEach(() => { Sinon.restore(); }); @@ -375,6 +379,7 @@ describe('db', () => { expect(getDBConnectionStub).to.have.been.calledWith({ preferred_username: `${DB_USERNAME}@database`, + sims_system_username: DB_USERNAME, identity_provider: 'database' }); }); diff --git a/api/src/models/user.ts b/api/src/models/user.ts index 5d63e1e53f..98e63af509 100644 --- a/api/src/models/user.ts +++ b/api/src/models/user.ts @@ -1,7 +1,7 @@ export class UserObject { id: number; user_identifier: string; - user_guid: string; + user_guid: string | null; identity_source: string; record_end_date: string; role_ids: number[]; diff --git a/api/src/paths/user/add.test.ts b/api/src/paths/user/add.test.ts index bf6f9b84a2..06afc0a3ea 100644 --- a/api/src/paths/user/add.test.ts +++ b/api/src/paths/user/add.test.ts @@ -34,7 +34,7 @@ describe('user', () => { expect.fail(); } catch (actualError) { expect((actualError as HTTPError).status).to.equal(400); - expect((actualError as HTTPError).message).to.equal('Missing required body param: userGuid'); + expect((actualError as HTTPError).message).to.equal('Missing required body param: userIdentifier'); } }); @@ -62,7 +62,7 @@ describe('user', () => { } }); - it('should throw a 400 error when no userGuid', async () => { + it('should throw a 400 error when no identitySource', async () => { const dbConnectionObj = getMockDBConnection(); sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); @@ -70,7 +70,7 @@ describe('user', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - identitySource: SYSTEM_IDENTITY_SOURCE.IDIR, + userGuid: 'aaaa', userIdentifier: 'username', roleId: 1 }; @@ -82,11 +82,11 @@ describe('user', () => { expect.fail(); } catch (actualError) { expect((actualError as HTTPError).status).to.equal(400); - expect((actualError as HTTPError).message).to.equal('Missing required body param: userGuid'); + expect((actualError as HTTPError).message).to.equal('Missing required body param: identitySource'); } }); - it('should throw a 400 error when no identitySource', async () => { + it('should throw a 400 error when no roleId', async () => { const dbConnectionObj = getMockDBConnection(); sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); @@ -96,7 +96,7 @@ describe('user', () => { mockReq.body = { userGuid: 'aaaa', userIdentifier: 'username', - roleId: 1 + identitySource: SYSTEM_IDENTITY_SOURCE.IDIR }; try { @@ -106,11 +106,11 @@ describe('user', () => { expect.fail(); } catch (actualError) { expect((actualError as HTTPError).status).to.equal(400); - expect((actualError as HTTPError).message).to.equal('Missing required body param: identitySource'); + expect((actualError as HTTPError).message).to.equal('Missing required body param: roleId'); } }); - it('should throw a 400 error when no roleId', async () => { + it('adds a system user and returns 200 on success', async () => { const dbConnectionObj = getMockDBConnection(); sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); @@ -120,21 +120,33 @@ describe('user', () => { mockReq.body = { userGuid: 'aaaa', userIdentifier: 'username', - identitySource: SYSTEM_IDENTITY_SOURCE.IDIR + identitySource: SYSTEM_IDENTITY_SOURCE.IDIR, + roleId: 1 }; - try { - const requestHandler = user.addSystemRoleUser(); + const mockUserObject: UserObject = { + id: 1, + user_identifier: '', + user_guid: '', + identity_source: '', + record_end_date: '', + role_ids: [1], + role_names: [] + }; - await requestHandler(mockReq, mockRes, mockNext); - expect.fail(); - } catch (actualError) { - expect((actualError as HTTPError).status).to.equal(400); - expect((actualError as HTTPError).message).to.equal('Missing required body param: roleId'); - } + const ensureSystemUserStub = sinon.stub(UserService.prototype, 'ensureSystemUser').resolves(mockUserObject); + + const adduserSystemRolesStub = sinon.stub(UserService.prototype, 'addUserSystemRoles'); + + const requestHandler = user.addSystemRoleUser(); + + await requestHandler(mockReq, mockRes, mockNext); + + expect(ensureSystemUserStub).to.have.been.calledOnce; + expect(adduserSystemRolesStub).to.have.been.calledOnce; }); - it('adds a system user and returns 200 on success', async () => { + it('should success when no userGuid', async () => { const dbConnectionObj = getMockDBConnection(); sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); @@ -142,16 +154,15 @@ describe('user', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - userGuid: 'aaaa', - userIdentifier: 'username', identitySource: SYSTEM_IDENTITY_SOURCE.IDIR, + userIdentifier: 'username', roleId: 1 }; const mockUserObject: UserObject = { id: 1, user_identifier: '', - user_guid: '', + user_guid: null, identity_source: '', record_end_date: '', role_ids: [1], @@ -161,11 +172,10 @@ describe('user', () => { const ensureSystemUserStub = sinon.stub(UserService.prototype, 'ensureSystemUser').resolves(mockUserObject); const adduserSystemRolesStub = sinon.stub(UserService.prototype, 'addUserSystemRoles'); - + const requestHandler = user.addSystemRoleUser(); await requestHandler(mockReq, mockRes, mockNext); - expect(ensureSystemUserStub).to.have.been.calledOnce; expect(adduserSystemRolesStub).to.have.been.calledOnce; }); diff --git a/api/src/repositories/user-repository.ts b/api/src/repositories/user-repository.ts index 6553e61c65..53b706471c 100644 --- a/api/src/repositories/user-repository.ts +++ b/api/src/repositories/user-repository.ts @@ -148,10 +148,15 @@ export class UserRepository extends BaseRepository { return response.rows; } + /** - * @TODO jsdoc - * @param userIdentifier - * @param identitySource + * Get an existing system user by their user identifier and identity source. + * + * @param userIdentifier the user's identifier + * @param identitySource the user's identity source, e.g. `'IDIR'` + * @return {*} {(Promise)} Promise resolving an array containing the user, if they match the + * search criteria. + * @memberof UserService */ async getUserByIdentifier(userIdentifier: string, identitySource: string): Promise { const sqlStatement = SQL` diff --git a/api/src/services/user-service.ts b/api/src/services/user-service.ts index 3710435c34..c5dcf923d2 100644 --- a/api/src/services/user-service.ts +++ b/api/src/services/user-service.ts @@ -61,9 +61,12 @@ export class UserService extends DBService { } /** - * @TODO jsdoc - * @param userIdentifier - * @param identitySource + * Get an existing system user by their user identifier and identity source. + * + * @param userIdentifier the user's identifier + * @param identitySource the user's identity source, e.g. `'IDIR'` + * @return {*} {(Promise)} Promise resolving the UserObject, or `null` if the user wasn't found. + * @memberof UserService */ async getUserByIdentifier(userIdentifier: string, identitySource: string): Promise { defaultLog.debug({ label: 'getUserByIdentifier', userIdentifier, identitySource }); From e9badb3552dd4d625c0f7eae9a7f892539341373 Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Sat, 28 Jan 2023 21:49:12 -0800 Subject: [PATCH 08/13] BHBC-2144: Added tests --- api/src/database/db.test.ts | 3 +- api/src/repositories/user-repository.test.ts | 48 ++++++++++++++++++++ api/src/services/user-service.test.ts | 34 ++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/api/src/database/db.test.ts b/api/src/database/db.test.ts index 8323be7f91..4fec6b8f04 100644 --- a/api/src/database/db.test.ts +++ b/api/src/database/db.test.ts @@ -368,11 +368,12 @@ describe('db', () => { Sinon.restore(); }); - it('calls getDBConnection for the biohub_api user', () => { + it.only('calls getDBConnection for the biohub_api user', () => { const getDBConnectionStub = Sinon.stub(db, 'getDBConnection').returns( ('stubbed DBConnection object' as unknown) as IDBConnection ); + process.env.DB_USER_API = 'example_db_username'; getAPIUserDBConnection(); const DB_USERNAME = process.env.DB_USER_API; diff --git a/api/src/repositories/user-repository.test.ts b/api/src/repositories/user-repository.test.ts index 4f23be9f39..6b0e03fccd 100644 --- a/api/src/repositories/user-repository.test.ts +++ b/api/src/repositories/user-repository.test.ts @@ -124,6 +124,54 @@ describe('UserRepository', () => { }); }); + describe('getUserByIdentifier', () => { + afterEach(() => { + sinon.restore(); + }); + it('should return empty array when no user found', async () => { + const mockQueryResponse = ({ rowCount: 1, rows: [] } as any) as Promise>; + + const mockDBConnection = getMockDBConnection({ + sql: async () => { + return mockQueryResponse; + } + }); + + const userRepository = new UserRepository(mockDBConnection); + + const response = await userRepository.getUserByIdentifier('user', 'source'); + + expect(response).to.eql([]); + }); + + it('should get user by identifier', async () => { + const mockResponse = [ + { + system_user_id: 1, + user_identifier: 'username', + user_guid: 'aaaa', + identity_source: 'idir', + record_end_date: 'data', + role_ids: [1], + role_names: ['admin'] + } + ]; + const mockQueryResponse = ({ rowCount: 1, rows: mockResponse } as any) as Promise>; + + const mockDBConnection = getMockDBConnection({ + sql: async () => { + return mockQueryResponse; + } + }); + + const userRepository = new UserRepository(mockDBConnection); + + const response = await userRepository.getUserByIdentifier('username', 'idir'); + + expect(response).to.equal(mockResponse); + }); + }); + describe('addSystemUser', () => { afterEach(() => { sinon.restore(); diff --git a/api/src/services/user-service.test.ts b/api/src/services/user-service.test.ts index 2cf50eb5c6..a5f08ada38 100644 --- a/api/src/services/user-service.test.ts +++ b/api/src/services/user-service.test.ts @@ -67,6 +67,40 @@ describe('UserService', () => { }); }); + describe('getUserByIdentifier', function () { + afterEach(() => { + sinon.restore(); + }); + + it('returns null if the query response has no rows', async function () { + const mockDBConnection = getMockDBConnection(); + const mockUserRepository = sinon.stub(UserRepository.prototype, 'getUserByIdentifier'); + mockUserRepository.resolves([]); + + const userService = new UserService(mockDBConnection); + + const result = await userService.getUserByIdentifier('aaaa', 'bbbb'); + + expect(result).to.be.null; + expect(mockUserRepository).to.have.been.calledOnce; + }); + + it('returns a UserObject for the first row of the response', async function () { + const mockDBConnection = getMockDBConnection(); + + const mockResponseRow = [{ system_user_id: 123 }]; + const mockUserRepository = sinon.stub(UserRepository.prototype, 'getUserByIdentifier'); + mockUserRepository.resolves((mockResponseRow as unknown) as IGetUser[]); + + const userService = new UserService(mockDBConnection); + + const result = await userService.getUserByIdentifier('aaaa', 'bbbb'); + + expect(result).to.eql(new UserObject(mockResponseRow[0])); + expect(mockUserRepository).to.have.been.calledOnce; + }); + }); + describe('addSystemUser', () => { afterEach(() => { sinon.restore(); From 241bd1bac0d68490d918a44b7850f66f5d40e9eb Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Sun, 29 Jan 2023 22:50:01 -0800 Subject: [PATCH 09/13] BHBC-2144: Add getters to db class --- api/src/database/db.test.ts | 1 - api/src/database/db.ts | 24 ++++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/api/src/database/db.test.ts b/api/src/database/db.test.ts index 4fec6b8f04..7f76a4c8c2 100644 --- a/api/src/database/db.test.ts +++ b/api/src/database/db.test.ts @@ -373,7 +373,6 @@ describe('db', () => { ('stubbed DBConnection object' as unknown) as IDBConnection ); - process.env.DB_USER_API = 'example_db_username'; getAPIUserDBConnection(); const DB_USERNAME = process.env.DB_USER_API; diff --git a/api/src/database/db.ts b/api/src/database/db.ts index 37bc74c961..ad94c0e10e 100644 --- a/api/src/database/db.ts +++ b/api/src/database/db.ts @@ -8,11 +8,11 @@ import { getLogger } from '../utils/logger'; const defaultLog = getLogger('database/db'); -const DB_HOST = process.env.DB_HOST; -const DB_PORT = Number(process.env.DB_PORT); -const DB_USERNAME = process.env.DB_USER_API; -const DB_PASSWORD = process.env.DB_USER_API_PASS; -const DB_DATABASE = process.env.DB_DATABASE; +const getDbHost = () => process.env.DB_HOST; +const getDbPort = () => Number(process.env.DB_PORT); +const getDbUsername = () => process.env.DB_USER_API; +const getDbPassword = () => process.env.DB_USER_API_PASS; +const getDbDatabase = () => process.env.DB_DATABASE; const DB_POOL_SIZE: number = Number(process.env.DB_POOL_SIZE) || 20; const DB_CONNECTION_TIMEOUT: number = Number(process.env.DB_CONNECTION_TIMEOUT) || 0; @@ -21,11 +21,11 @@ const DB_IDLE_TIMEOUT: number = Number(process.env.DB_IDLE_TIMEOUT) || 10000; const DB_CLIENT = 'pg'; export const defaultPoolConfig: pg.PoolConfig = { - user: DB_USERNAME, - password: DB_PASSWORD, - database: DB_DATABASE, - port: DB_PORT, - host: DB_HOST, + user: getDbUsername(), + password: getDbPassword(), + database: getDbDatabase(), + port: getDbPort(), + host: getDbHost(), max: DB_POOL_SIZE, connectionTimeoutMillis: DB_CONNECTION_TIMEOUT, idleTimeoutMillis: DB_IDLE_TIMEOUT @@ -372,8 +372,8 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection { */ export const getAPIUserDBConnection = (): IDBConnection => { return getDBConnection({ - preferred_username: `${DB_USERNAME}@database`, - sims_system_username: DB_USERNAME, + preferred_username: `${getDbUsername()}@database`, + sims_system_username: getDbUsername(), identity_provider: 'database' }); }; From b9e7d82662b02b99a5489aa1702dbe32de972f9e Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Sun, 29 Jan 2023 23:38:58 -0800 Subject: [PATCH 10/13] BHBC-2144: Added tests --- api/src/database/db.test.ts | 2 +- api/src/database/db.ts | 2 +- .../database/user-context-queries.test.ts | 32 +++++++++++++++++-- .../queries/database/user-context-queries.ts | 29 +++++++++++++---- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/api/src/database/db.test.ts b/api/src/database/db.test.ts index 7f76a4c8c2..8323be7f91 100644 --- a/api/src/database/db.test.ts +++ b/api/src/database/db.test.ts @@ -368,7 +368,7 @@ describe('db', () => { Sinon.restore(); }); - it.only('calls getDBConnection for the biohub_api user', () => { + it('calls getDBConnection for the biohub_api user', () => { const getDBConnectionStub = Sinon.stub(db, 'getDBConnection').returns( ('stubbed DBConnection object' as unknown) as IDBConnection ); diff --git a/api/src/database/db.ts b/api/src/database/db.ts index ad94c0e10e..877c693f32 100644 --- a/api/src/database/db.ts +++ b/api/src/database/db.ts @@ -323,7 +323,7 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection { } // Patch user GUID - const patchUserGuidSqlStatement = queries.database.patchUserGuidSQL(userGuid, userIdentifier); + const patchUserGuidSqlStatement = queries.database.patchUserGuidSQL(userGuid, userIdentifier, userIdentitySource); if (!patchUserGuidSqlStatement) { throw new ApiExecuteSQLError('Failed to build SQL patch user GUID statement'); diff --git a/api/src/queries/database/user-context-queries.test.ts b/api/src/queries/database/user-context-queries.test.ts index 38393419af..fcc380ba01 100644 --- a/api/src/queries/database/user-context-queries.test.ts +++ b/api/src/queries/database/user-context-queries.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { describe } from 'mocha'; import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; -import { setSystemUserContextSQL } from './user-context-queries'; +import { patchUserGuidSQL, setSystemUserContextSQL } from './user-context-queries'; describe('setSystemUserContextSQL', () => { it('has empty userGuid', () => { @@ -30,5 +30,33 @@ describe('setSystemUserContextSQL', () => { }); describe('patchUserGuidSQL', () => { - // + it('returns null if no userGuid is provided', () => { + const response = patchUserGuidSQL('', 'user-identifier', SYSTEM_IDENTITY_SOURCE.IDIR); + expect(response).not.to.be.null; + }); + + it('returns null if no userIdentifier is provided', () => { + const response = patchUserGuidSQL('user-guid', '', SYSTEM_IDENTITY_SOURCE.IDIR); + expect(response).not.to.be.null; + }); + + it('returns null if no identity source is provided', () => { + const response = patchUserGuidSQL('user-guid', 'user-identifier', null as unknown as SYSTEM_IDENTITY_SOURCE); + expect(response).not.to.be.null; + }); + + it('identifies an IDIR user', () => { + const response = patchUserGuidSQL('user-guid', 'user-identifier', SYSTEM_IDENTITY_SOURCE.IDIR); + expect(response).not.to.be.null; + }); + + it('identifies a BCEID basic user', () => { + const response = patchUserGuidSQL('user-guid', 'user-identifier', SYSTEM_IDENTITY_SOURCE.BCEID_BASIC); + expect(response).not.to.be.null; + }); + + it('identifies a BCEID business user', () => { + const response = patchUserGuidSQL('user-guid', 'user-identifier', SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS); + expect(response).not.to.be.null; + }); }); diff --git a/api/src/queries/database/user-context-queries.ts b/api/src/queries/database/user-context-queries.ts index fcc28d15fc..7ff7eeb88b 100644 --- a/api/src/queries/database/user-context-queries.ts +++ b/api/src/queries/database/user-context-queries.ts @@ -26,13 +26,15 @@ export const setSystemUserContextSQL = ( }; /** - * + * @TODO jsdoc + * * @param userGuid * @param userIdentifier + * @param {SYSTEM_IDENTITY_SOURCE} userIdentitySource * @returns */ -export const patchUserGuidSQL = (userGuid: string, userIdentifier: string): SQLStatement | null => { - if (!userGuid || !userIdentifier) { +export const patchUserGuidSQL = (userGuid: string, userIdentifier: string, userIdentitySource: SYSTEM_IDENTITY_SOURCE): SQLStatement | null => { + if (!userGuid || !userIdentifier || !userIdentitySource) { return null; } @@ -42,9 +44,22 @@ export const patchUserGuidSQL = (userGuid: string, userIdentifier: string): SQLS SET user_guid = ${userGuid.toLowerCase()} WHERE - user_guid IS NULL - AND - user_identifier = ${userIdentifier.toLowerCase()} - ; + system_user_id + IN ( + SELECT + su.system_user_id + FROM + system_user su + LEFT JOIN + user_identity_source uis + ON + uis.user_identity_source_id = su.user_identity_source_id + WHERE + suuser_identifier = ${userIdentifier.toLowerCase()} + AND + uis.name = ${userIdentitySource} + AND + user_guid IS NULL + ); `; }; From 7ffefefbd05819a773a70bbccf7932e7aa99a575 Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Mon, 30 Jan 2023 12:17:03 -0800 Subject: [PATCH 11/13] BHBC-2144: Amend context db query --- api/src/queries/database/user-context-queries.ts | 4 ++-- .../migrations/20230127000000_user_guid_null_constraint.ts | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/api/src/queries/database/user-context-queries.ts b/api/src/queries/database/user-context-queries.ts index 7ff7eeb88b..510dffa91b 100644 --- a/api/src/queries/database/user-context-queries.ts +++ b/api/src/queries/database/user-context-queries.ts @@ -55,9 +55,9 @@ export const patchUserGuidSQL = (userGuid: string, userIdentifier: string, userI ON uis.user_identity_source_id = su.user_identity_source_id WHERE - suuser_identifier = ${userIdentifier.toLowerCase()} + su.user_identifier ILIKE ${userIdentifier.toLowerCase()} AND - uis.name = ${userIdentitySource} + uis.name ILIKE ${userIdentitySource} AND user_guid IS NULL ); diff --git a/database/src/migrations/20230127000000_user_guid_null_constraint.ts b/database/src/migrations/20230127000000_user_guid_null_constraint.ts index c1da4fbbdb..a06e52644a 100644 --- a/database/src/migrations/20230127000000_user_guid_null_constraint.ts +++ b/database/src/migrations/20230127000000_user_guid_null_constraint.ts @@ -19,6 +19,8 @@ export async function up(knex: Knex): Promise { SET search_path = ${DB_SCHEMA}; ALTER TABLE system_user ALTER COLUMN user_guid DROP NOT NULL; + + CREATE UNIQUE INDEX user_guid_uk1 ON system_user (user_guid); SET search_path = ${DB_SCHEMA_DAPI_V1}; From 34d028e66f9c645a26659ded538ea6ee79610c2f Mon Sep 17 00:00:00 2001 From: Curtis Upshall Date: Mon, 30 Jan 2023 12:18:22 -0800 Subject: [PATCH 12/13] BHBC-2144: Remove toLower in db query --- api/src/queries/database/user-context-queries.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/queries/database/user-context-queries.ts b/api/src/queries/database/user-context-queries.ts index 510dffa91b..9fca7cef3a 100644 --- a/api/src/queries/database/user-context-queries.ts +++ b/api/src/queries/database/user-context-queries.ts @@ -55,7 +55,7 @@ export const patchUserGuidSQL = (userGuid: string, userIdentifier: string, userI ON uis.user_identity_source_id = su.user_identity_source_id WHERE - su.user_identifier ILIKE ${userIdentifier.toLowerCase()} + su.user_identifier ILIKE ${userIdentifier} AND uis.name ILIKE ${userIdentitySource} AND From 1fa86fd413e5fa437794b3259a951e6be3d0771b Mon Sep 17 00:00:00 2001 From: Kjartan Date: Mon, 30 Jan 2023 12:52:50 -0800 Subject: [PATCH 13/13] Fix test, remove query file --- api/src/database/db.test.ts | 7 -- api/src/database/db.ts | 40 ++++++++---- api/src/paths/user/add.test.ts | 2 +- api/src/queries/database/index.ts | 3 - .../database/user-context-queries.test.ts | 62 ------------------ .../queries/database/user-context-queries.ts | 65 ------------------- api/src/queries/queries.ts | 2 - api/src/repositories/user-repository.ts | 1 - 8 files changed, 29 insertions(+), 153 deletions(-) delete mode 100644 api/src/queries/database/index.ts delete mode 100644 api/src/queries/database/user-context-queries.test.ts delete mode 100644 api/src/queries/database/user-context-queries.ts diff --git a/api/src/database/db.test.ts b/api/src/database/db.test.ts index 8323be7f91..e55e62f82f 100644 --- a/api/src/database/db.test.ts +++ b/api/src/database/db.test.ts @@ -6,7 +6,6 @@ import SQL from 'sql-template-strings'; import { SYSTEM_IDENTITY_SOURCE } from '../constants/database'; import { ApiExecuteSQLError } from '../errors/api-error'; import { HTTPError } from '../errors/http-error'; -import { setSystemUserContextSQL } from '../queries/database/user-context-queries'; import * as db from './db'; import { getAPIUserDBConnection, getDBConnection, getDBPool, getKnex, IDBConnection, initDBPool } from './db'; @@ -84,12 +83,6 @@ describe('db', () => { expect(getDBPoolStub).to.have.been.calledOnce; expect(connectStub).to.have.been.calledOnce; - const expectedSystemUserContextSQL = setSystemUserContextSQL('testguid', SYSTEM_IDENTITY_SOURCE.IDIR); - expect(queryStub).to.have.been.calledWith( - expectedSystemUserContextSQL?.text, - expectedSystemUserContextSQL?.values - ); - expect(queryStub).to.have.been.calledWith('BEGIN'); }); }); diff --git a/api/src/database/db.ts b/api/src/database/db.ts index 877c693f32..a40b7a0541 100644 --- a/api/src/database/db.ts +++ b/api/src/database/db.ts @@ -1,8 +1,7 @@ import knex, { Knex } from 'knex'; import * as pg from 'pg'; -import { SQLStatement } from 'sql-template-strings'; +import SQL, { SQLStatement } from 'sql-template-strings'; import { ApiExecuteSQLError, ApiGeneralError } from '../errors/api-error'; -import { queries } from '../queries/queries'; import { getUserGuid, getUserIdentifier, getUserIdentitySource } from '../utils/keycloak-utils'; import { getLogger } from '../utils/logger'; @@ -323,18 +322,35 @@ export const getDBConnection = function (keycloakToken: object): IDBConnection { } // Patch user GUID - const patchUserGuidSqlStatement = queries.database.patchUserGuidSQL(userGuid, userIdentifier, userIdentitySource); - - if (!patchUserGuidSqlStatement) { - throw new ApiExecuteSQLError('Failed to build SQL patch user GUID statement'); - } + const patchUserGuidSqlStatement = SQL` + UPDATE + system_user + SET + user_guid = ${userGuid.toLowerCase()} + WHERE + system_user_id + IN ( + SELECT + su.system_user_id + FROM + system_user su + LEFT JOIN + user_identity_source uis + ON + uis.user_identity_source_id = su.user_identity_source_id + WHERE + su.user_identifier ILIKE ${userIdentifier} + AND + uis.name ILIKE ${userIdentitySource} + AND + user_guid IS NULL + ); + `; // Set the user context for all queries made using this connection - const setSystemUserContextSQLStatement = queries.database.setSystemUserContextSQL(userGuid, userIdentitySource); - - if (!setSystemUserContextSQLStatement) { - throw new ApiExecuteSQLError('Failed to build SQL user context statement'); - } + const setSystemUserContextSQLStatement = SQL` + SELECT api_set_context(${userGuid}, ${userIdentitySource}); + `; try { await _client.query(patchUserGuidSqlStatement.text, patchUserGuidSqlStatement.values); diff --git a/api/src/paths/user/add.test.ts b/api/src/paths/user/add.test.ts index 06afc0a3ea..e8461fe867 100644 --- a/api/src/paths/user/add.test.ts +++ b/api/src/paths/user/add.test.ts @@ -172,7 +172,7 @@ describe('user', () => { const ensureSystemUserStub = sinon.stub(UserService.prototype, 'ensureSystemUser').resolves(mockUserObject); const adduserSystemRolesStub = sinon.stub(UserService.prototype, 'addUserSystemRoles'); - + const requestHandler = user.addSystemRoleUser(); await requestHandler(mockReq, mockRes, mockNext); diff --git a/api/src/queries/database/index.ts b/api/src/queries/database/index.ts deleted file mode 100644 index b596c80ee2..0000000000 --- a/api/src/queries/database/index.ts +++ /dev/null @@ -1,3 +0,0 @@ -import * as userContext from './user-context-queries'; - -export default { ...userContext }; diff --git a/api/src/queries/database/user-context-queries.test.ts b/api/src/queries/database/user-context-queries.test.ts deleted file mode 100644 index fcc380ba01..0000000000 --- a/api/src/queries/database/user-context-queries.test.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { expect } from 'chai'; -import { describe } from 'mocha'; -import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; -import { patchUserGuidSQL, setSystemUserContextSQL } from './user-context-queries'; - -describe('setSystemUserContextSQL', () => { - it('has empty userGuid', () => { - const response = setSystemUserContextSQL('', SYSTEM_IDENTITY_SOURCE.IDIR); - - expect(response).to.be.null; - }); - - it('identifies an IDIR user', () => { - const response = setSystemUserContextSQL('idir-user', SYSTEM_IDENTITY_SOURCE.IDIR); - - expect(response).not.to.be.null; - }); - - it('identifies a BCEID basic user', () => { - const response = setSystemUserContextSQL('bceid-basic-user', SYSTEM_IDENTITY_SOURCE.BCEID_BASIC); - - expect(response).not.to.be.null; - }); - - it('identifies a BCEID business user', () => { - const response = setSystemUserContextSQL('bceid-business-user', SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS); - - expect(response).not.to.be.null; - }); -}); - -describe('patchUserGuidSQL', () => { - it('returns null if no userGuid is provided', () => { - const response = patchUserGuidSQL('', 'user-identifier', SYSTEM_IDENTITY_SOURCE.IDIR); - expect(response).not.to.be.null; - }); - - it('returns null if no userIdentifier is provided', () => { - const response = patchUserGuidSQL('user-guid', '', SYSTEM_IDENTITY_SOURCE.IDIR); - expect(response).not.to.be.null; - }); - - it('returns null if no identity source is provided', () => { - const response = patchUserGuidSQL('user-guid', 'user-identifier', null as unknown as SYSTEM_IDENTITY_SOURCE); - expect(response).not.to.be.null; - }); - - it('identifies an IDIR user', () => { - const response = patchUserGuidSQL('user-guid', 'user-identifier', SYSTEM_IDENTITY_SOURCE.IDIR); - expect(response).not.to.be.null; - }); - - it('identifies a BCEID basic user', () => { - const response = patchUserGuidSQL('user-guid', 'user-identifier', SYSTEM_IDENTITY_SOURCE.BCEID_BASIC); - expect(response).not.to.be.null; - }); - - it('identifies a BCEID business user', () => { - const response = patchUserGuidSQL('user-guid', 'user-identifier', SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS); - expect(response).not.to.be.null; - }); -}); diff --git a/api/src/queries/database/user-context-queries.ts b/api/src/queries/database/user-context-queries.ts deleted file mode 100644 index 9fca7cef3a..0000000000 --- a/api/src/queries/database/user-context-queries.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { SQL, SQLStatement } from 'sql-template-strings'; -import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; - -/** - * Returns an SQL query for setting the user's context. As a side-effect, it first updates the user's - * `user_guid` to reflect the given `userGuid` value, but only if it is currently `null` (which happens - * when new system users are created and a GUID is not provided). - * - * @param {string} userGuid the GUID of the user - * @param {string} userIdentifier the user's identifier - * @param {string} userIdentitySource The user's identity source - * @returns {*} {SQLStatement | null} The SQL statement for setting the user's context, or `null` if - * userGuid or userIdentifier are undefinedd. - */ -export const setSystemUserContextSQL = ( - userGuid: string, - userIdentitySource: SYSTEM_IDENTITY_SOURCE -): SQLStatement | null => { - if (!userGuid) { - return null; - } - - return SQL` - SELECT api_set_context(${userGuid}, ${userIdentitySource}); - `; -}; - -/** - * @TODO jsdoc - * - * @param userGuid - * @param userIdentifier - * @param {SYSTEM_IDENTITY_SOURCE} userIdentitySource - * @returns - */ -export const patchUserGuidSQL = (userGuid: string, userIdentifier: string, userIdentitySource: SYSTEM_IDENTITY_SOURCE): SQLStatement | null => { - if (!userGuid || !userIdentifier || !userIdentitySource) { - return null; - } - - return SQL` - UPDATE - system_user - SET - user_guid = ${userGuid.toLowerCase()} - WHERE - system_user_id - IN ( - SELECT - su.system_user_id - FROM - system_user su - LEFT JOIN - user_identity_source uis - ON - uis.user_identity_source_id = su.user_identity_source_id - WHERE - su.user_identifier ILIKE ${userIdentifier} - AND - uis.name ILIKE ${userIdentitySource} - AND - user_guid IS NULL - ); - `; -}; diff --git a/api/src/queries/queries.ts b/api/src/queries/queries.ts index 25f0fd4937..eaac7f1533 100644 --- a/api/src/queries/queries.ts +++ b/api/src/queries/queries.ts @@ -1,6 +1,5 @@ import administrativeActivity from './administrative-activity'; import codes from './codes'; -import database from './database'; import project from './project'; import projectParticipation from './project-participation'; import search from './search'; @@ -10,7 +9,6 @@ import users from './users'; export const queries = { administrativeActivity, codes, - database, project, projectParticipation, search, diff --git a/api/src/repositories/user-repository.ts b/api/src/repositories/user-repository.ts index 53b706471c..50a4831539 100644 --- a/api/src/repositories/user-repository.ts +++ b/api/src/repositories/user-repository.ts @@ -148,7 +148,6 @@ export class UserRepository extends BaseRepository { return response.rows; } - /** * Get an existing system user by their user identifier and identity source. *