diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 2f9d6b725..ee495c48c 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,50 +1,11 @@ -# Overview +## Links to Jira Tickets -## Links to Jira tickets +- {Include a link to all applicable Jira tickets} -- {List all applicable Jira tickets} +## Description of Changes -## Description of relevant changes +- {List all relevant code changes. Include any changes to the business workflow that might not be obvious to the reviewers of this PR.} -- {List all relevant changes, in particular anything that will help the reviewers test/verify this PR} +## Testing Notes -## PR Checklist - -A list of items that are good to consider when making any changes. - -_Note: this list is not exhaustive, and not all items are always applicable._ - -### Code - -- [ ] New files/classes/functions have appropriately descriptive names and comment blocks to describe their use/behaviour -- [ ] I have avoided duplicating code when possible, moving re-usable pieces into functions -- [ ] I have avoided hard-coding values where possible and moved any re-usable constants to a constants file -- [ ] My code is as flat as possible (avoids deeply nested if/else blocks, promise chains, etc) -- [ ] My code changes account for null/undefined values and handle errors appropriately -- [ ] My code uses types/interfaces to help describe values/parameters/etc, help ensure type safety, and improve readability - -### Style - -- [ ] My code follows the established style conventions -- [ ] My code uses native material-ui components/icons/conventions when possible - -### Documentation - -- [ ] I have commented my code sufficiently, such that an unfamiliar developer could understand my code -- [ ] I have added/updated README's and related documentation, as needed - -### Tests - -- [ ] I have added/updated unit tests for any code I've added/updated -- [ ] I have added/updated the Postman requests/tests to account for any API endpoints I've added/updated - -### Linting/Formatting - -- [ ] I have run the linter and fixed any issues, as needed - _See the `lint` commands in package.json_ -- [ ] I have run the formatter and fixed any issues, as needed - _See the `format` commands in package.json_ - -### SonarCloud - -- [ ] I have addressed all SonarCloud Bugs, Vulnerabilities, Security Hotspots, and Code Smells +- {List any relevant testing considerations, necessary pre-reqs, and areas of the app to focus on. Specifically, include anything that will help the reviewers of this PR verify the code is functioning as expected.} diff --git a/api/src/models/index.ts b/api/src/models/index.ts deleted file mode 100644 index 44b3ae674..000000000 --- a/api/src/models/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * as Models from './models'; diff --git a/api/src/models/models.ts b/api/src/models/models.ts deleted file mode 100644 index faa2ca1ef..000000000 --- a/api/src/models/models.ts +++ /dev/null @@ -1 +0,0 @@ -export * as user from './user'; diff --git a/api/src/models/user/index.ts b/api/src/models/user/index.ts deleted file mode 100644 index e5abc8565..000000000 --- a/api/src/models/user/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './user'; diff --git a/api/src/models/user/user.test.ts b/api/src/models/user/user.test.ts deleted file mode 100644 index 88647d502..000000000 --- a/api/src/models/user/user.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { expect } from 'chai'; -import { describe } from 'mocha'; -import { UserObject } from './user'; - -describe('UserObject', () => { - describe('No values provided', () => { - let data: UserObject; - - before(() => { - data = new UserObject(null as unknown as any); - }); - - it('sets id', function () { - expect(data.id).to.equal(null); - }); - - it('sets user_identifier', function () { - expect(data.user_identifier).to.equal(null); - }); - - it('sets role_names', function () { - expect(data.role_names).to.eql([]); - }); - }); - - describe('valid values provided, no roles', () => { - let data: UserObject; - - const userObject = { system_user_id: 1, user_identifier: 'test name', role_ids: [], role_names: [] }; - - before(() => { - data = new UserObject(userObject); - }); - - it('sets id', function () { - expect(data.id).to.equal(1); - }); - - it('sets user_identifier', function () { - expect(data.user_identifier).to.equal('test name'); - }); - - it('sets role_ids', function () { - expect(data.role_ids).to.eql([]); - }); - - it('sets role_names', function () { - expect(data.role_names).to.eql([]); - }); - }); - - describe('valid values provided', () => { - let data: UserObject; - - const userObject = { - system_user_id: 1, - user_identifier: 'test name', - role_ids: [1, 2], - role_names: ['role 1', 'role 2'] - }; - - before(() => { - data = new UserObject(userObject); - }); - - it('sets id', function () { - expect(data.id).to.equal(1); - }); - - it('sets user_identifier', function () { - expect(data.user_identifier).to.equal('test name'); - }); - - it('sets role_ids', function () { - expect(data.role_ids).to.eql([1, 2]); - }); - - it('sets role_names', function () { - expect(data.role_names).to.eql(['role 1', 'role 2']); - }); - }); -}); diff --git a/api/src/models/user/user.ts b/api/src/models/user/user.ts deleted file mode 100644 index 7cb8c8a6e..000000000 --- a/api/src/models/user/user.ts +++ /dev/null @@ -1,19 +0,0 @@ -export class UserObject { - id: number; - user_identifier: string; - user_guid: string; - identity_source: string; - record_end_date: string; - role_ids: number[]; - role_names: string[]; - - constructor(obj?: any) { - this.id = obj?.system_user_id || null; - this.user_identifier = obj?.user_identifier || null; - this.user_guid = obj?.user_guid || null; - this.identity_source = obj?.identity_source || null; - this.record_end_date = obj?.record_end_date || null; - this.role_ids = (obj?.role_ids?.length && obj.role_ids) || []; - this.role_names = (obj?.role_names?.length && obj.role_names) || []; - } -} diff --git a/api/src/paths/administrative/access-request/update.test.ts b/api/src/paths/administrative/access-request/update.test.ts index 2afa9b610..f51460f7e 100644 --- a/api/src/paths/administrative/access-request/update.test.ts +++ b/api/src/paths/administrative/access-request/update.test.ts @@ -4,7 +4,7 @@ import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import * as db from '../../../database/db'; import { HTTPError } from '../../../errors/http-error'; -import { Models } from '../../../models'; +import { SystemUserExtended } from '../../../repositories/user-repository'; import { AdministrativeService } from '../../../services/administrative-service'; import { GCNotifyService } from '../../../services/gcnotify-service'; import { UserService } from '../../../services/user-service'; @@ -68,12 +68,19 @@ describe('updateAccessRequest', () => { const systemUserId = 4; const existingRoleIds = [1, 2]; - const mockSystemUser: Models.user.UserObject = { - id: systemUserId, - user_identifier: '', + const mockSystemUser: SystemUserExtended = { + system_user_id: systemUserId, + user_identity_source_id: 2, + user_identifier: 'username', user_guid: '', - identity_source: '', record_end_date: '', + record_effective_date: '2023-12-08', + create_date: '2023-12-08 14:37:41.315999-08', + create_user: 1, + update_date: null, + update_user: null, + revision_count: 0, + identity_source: 'identitySource', role_ids: existingRoleIds, role_names: [] }; diff --git a/api/src/paths/administrative/access-request/update.ts b/api/src/paths/administrative/access-request/update.ts index 3e4624a3e..d2f9c6945 100644 --- a/api/src/paths/administrative/access-request/update.ts +++ b/api/src/paths/administrative/access-request/update.ts @@ -125,7 +125,7 @@ export function updateAccessRequest(): RequestHandler { if (rolesIdsToAdd?.length) { // Add any missing roles (if any) - await userService.addUserSystemRoles(systemUserObject.id, rolesIdsToAdd); + await userService.addUserSystemRoles(systemUserObject.system_user_id, rolesIdsToAdd); } // Update the access request record status diff --git a/api/src/paths/dwc/spatial/download.test.ts b/api/src/paths/dwc/spatial/download.test.ts index 664672149..e109d66fd 100644 --- a/api/src/paths/dwc/spatial/download.test.ts +++ b/api/src/paths/dwc/spatial/download.test.ts @@ -291,7 +291,7 @@ describe('download', () => { }); it('returns 200', async () => { - const datasetID = 'AAA-BBB-CCC-123'; + const datasetID = '123-456-789'; const dbConnectionObj = getMockDBConnection({ commit: sinon.stub(), release: sinon.stub() }); sinon.stub(db, 'getAPIUserDBConnection').returns(dbConnectionObj); diff --git a/api/src/paths/dwc/submission/{datasetId}/artifacts.test.ts b/api/src/paths/dwc/submission/{datasetId}/artifacts.test.ts index d1c63729f..e79ea17fc 100644 --- a/api/src/paths/dwc/submission/{datasetId}/artifacts.test.ts +++ b/api/src/paths/dwc/submission/{datasetId}/artifacts.test.ts @@ -70,7 +70,7 @@ describe('getArtifactsByDatasetId', () => { const mockResponse = { artifact_id: 1, create_date: '1970-01-01T00:00:00.000Z', - description: 'aaa', + description: 'description', file_name: 'Lecture 5 - Partial Fraction.pdf', file_size: 2405262, file_type: 'Report', diff --git a/api/src/paths/dwc/submission/{datasetId}/related.test.ts b/api/src/paths/dwc/submission/{datasetId}/related.test.ts index bfe58f567..e5a2a6f0a 100644 --- a/api/src/paths/dwc/submission/{datasetId}/related.test.ts +++ b/api/src/paths/dwc/submission/{datasetId}/related.test.ts @@ -284,7 +284,7 @@ describe('getRelatedDatasetsByDatasetId', () => { const submissionServiceStub = sinon .stub(SubmissionService.prototype, 'findRelatedDatasetsByDatasetId') - .resolves([{ datasetId: 'aaa' }, { datasetId: 'bbb' }] as RelatedDataset[]); + .resolves([{ datasetId: '123' }, { datasetId: '456' }] as RelatedDataset[]); const isSystemUserAdminStub = sinon.stub(UserService.prototype, 'isSystemUserAdmin').resolves(true); @@ -307,13 +307,13 @@ describe('getRelatedDatasetsByDatasetId', () => { expect(mockRes.jsonValue).to.eql({ datasetsWithSupplementaryData: [ { - datasetId: 'aaa', + datasetId: '123', supplementaryData: { isPendingReview: true } }, { - datasetId: 'bbb', + datasetId: '456', supplementaryData: { isPendingReview: false } diff --git a/api/src/paths/submission/intake.test.ts b/api/src/paths/submission/intake.test.ts index 4c673ae48..8af7ea298 100644 --- a/api/src/paths/submission/intake.test.ts +++ b/api/src/paths/submission/intake.test.ts @@ -30,7 +30,7 @@ describe('intake', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - id: 'aaaa', + id: '123-456-789', type: 'submission', properties: {}, features: [] @@ -61,7 +61,7 @@ describe('intake', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - id: 'aaaa', + id: '123-456-789', type: 'submission', properties: {}, features: [] @@ -105,7 +105,7 @@ describe('intake', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - id: 'aaaa', + id: '123-456-789', type: 'submission', properties: { additionalInformation: 'test' }, features: [] diff --git a/api/src/paths/user/add.test.ts b/api/src/paths/user/add.test.ts index 34853756e..e8f98f460 100644 --- a/api/src/paths/user/add.test.ts +++ b/api/src/paths/user/add.test.ts @@ -5,7 +5,7 @@ import sinonChai from 'sinon-chai'; import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; import * as db from '../../database/db'; import { HTTPError } from '../../errors/http-error'; -import { Models } from '../../models'; +import { SystemUserExtended } from '../../repositories/user-repository'; import { UserService } from '../../services/user-service'; import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db'; import * as user from './add'; @@ -46,7 +46,7 @@ describe('user', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - userGuid: 'aaaa', + userGuid: '123-456-789', identitySource: SYSTEM_IDENTITY_SOURCE.IDIR, roleId: 1 }; @@ -94,7 +94,7 @@ describe('user', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - userGuid: 'aaaa', + userGuid: '123-456-789', userIdentifier: 'username', roleId: 1 }; @@ -118,7 +118,7 @@ describe('user', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - userGuid: 'aaaa', + userGuid: '123-456-789', userIdentifier: 'username', identitySource: SYSTEM_IDENTITY_SOURCE.IDIR }; @@ -142,18 +142,25 @@ describe('user', () => { const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); mockReq.body = { - userGuid: 'aaaa', + userGuid: '123-456-789', userIdentifier: 'username', identitySource: SYSTEM_IDENTITY_SOURCE.IDIR, roleId: 1 }; - const mockUserObject: Models.user.UserObject = { - id: 1, + const mockUserObject: SystemUserExtended = { + system_user_id: 1, + user_identity_source_id: 2, user_identifier: '', user_guid: '', identity_source: '', + record_effective_date: '', record_end_date: '', + create_date: 'string', + create_user: 1, + update_date: null, + update_user: null, + revision_count: 1, role_ids: [1], role_names: [] }; diff --git a/api/src/paths/user/add.ts b/api/src/paths/user/add.ts index 4bf52b924..0ba9209f6 100644 --- a/api/src/paths/user/add.ts +++ b/api/src/paths/user/add.ts @@ -114,7 +114,7 @@ export function addSystemRoleUser(): RequestHandler { const userObject = await userService.ensureSystemUser(userGuid, userIdentifier, identitySource); if (userObject) { - await userService.addUserSystemRoles(userObject.id, [roleId]); + await userService.addUserSystemRoles(userObject.system_user_id, [roleId]); } await connection.commit(); diff --git a/api/src/paths/user/list.test.ts b/api/src/paths/user/list.test.ts index 0c8f74870..00ffae8ca 100644 --- a/api/src/paths/user/list.test.ts +++ b/api/src/paths/user/list.test.ts @@ -3,6 +3,7 @@ import { describe } from 'mocha'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import * as db from '../../database/db'; +import { SystemUserExtended } from '../../repositories/user-repository'; import { UserService } from '../../services/user-service'; import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db'; import * as users from './list'; @@ -22,13 +23,20 @@ describe('users', () => { sinon.stub(db, 'getDBConnection').returns(mockDBConnection); - const mockResponse = [ + const mockResponse: SystemUserExtended[] = [ { - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'identifier', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [1, 2], role_names: ['System Admin', 'Project Lead'] } diff --git a/api/src/paths/user/self.test.ts b/api/src/paths/user/self.test.ts index 57b31fb1c..7f40eba05 100644 --- a/api/src/paths/user/self.test.ts +++ b/api/src/paths/user/self.test.ts @@ -2,12 +2,9 @@ import chai, { expect } from 'chai'; import { describe } from 'mocha'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; -import { SYSTEM_IDENTITY_SOURCE } from '../../constants/database'; import * as db from '../../database/db'; import { HTTPError } from '../../errors/http-error'; -import { UserRepository } from '../../repositories/user-repository'; import { UserService } from '../../services/user-service'; -import * as keycloakUtils from '../../utils/keycloak-utils'; import { getMockDBConnection, getRequestHandlerMocks } from '../../__mocks__/db'; import * as self from './self'; @@ -32,80 +29,54 @@ describe('getUser', () => { expect.fail(); } catch (actualError) { expect((actualError as HTTPError).status).to.equal(400); - expect((actualError as HTTPError).message).to.equal("Failed to retrieve user's identifier or GUID"); + expect((actualError as HTTPError).message).to.equal('Failed to identify system user ID'); } }); it('should return the user row on success', async () => { - const dbConnectionObj = getMockDBConnection({ systemUserId: () => 1 }); + const dbConnectionObj = getMockDBConnection({ + systemUserId: () => 1 + }); const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); - sinon.stub(keycloakUtils, 'getUserGuid').returns('aaaa'); - sinon.stub(keycloakUtils, 'getUserIdentitySource').returns(SYSTEM_IDENTITY_SOURCE.IDIR); - sinon.stub(keycloakUtils, 'getUserIdentifier').returns('identifier'); - sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); - sinon.stub(UserRepository.prototype, 'getUserByGuid').resolves([ - { - system_user_id: 1, - user_identity_source_id: 1, - user_identifier: 'identifier', - user_guid: 'aaaa', - identity_source: 'idir', - record_effective_date: 'date', - record_end_date: null, - create_date: 'date', - create_user: 1, - update_date: 'date', - update_user: null, - revision_count: 0, - role_ids: [1, 2], - role_names: ['role 1', 'role 2'] - } - ]); + sinon.stub(UserService.prototype, 'getUserById').resolves({ + system_user_id: 1, + user_identity_source_id: 2, + user_identifier: 'testuser', + user_guid: '123456789', + record_effective_date: 'date', + record_end_date: null, + create_date: 'date', + create_user: 1, + update_date: null, + update_user: null, + revision_count: 1, + identity_source: 'idir', + role_ids: [1, 2], + role_names: ['role 1', 'role 2'] + }); const requestHandler = self.getUser(); await requestHandler(mockReq, mockRes, mockNext); - expect(mockRes.jsonValue.id).to.equal(1); - expect(mockRes.jsonValue.user_guid).to.equal('aaaa'); - expect(mockRes.jsonValue.user_identifier).to.equal('identifier'); - expect(mockRes.jsonValue.identity_source).to.equal('idir'); + expect(mockRes.jsonValue.system_user_id).to.equal(1); + expect(mockRes.jsonValue.user_identifier).to.equal('testuser'); expect(mockRes.jsonValue.role_ids).to.eql([1, 2]); expect(mockRes.jsonValue.role_names).to.eql(['role 1', 'role 2']); }); - it('should parse out user guid, identifier and identity source from the keycloak token', async () => { - const dbConnectionObj = getMockDBConnection({ systemUserId: () => 1 }); - - const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); - - mockReq['keycloak_token'] = { - preferred_username: 'aaaa@idir', - identity_source: 'idir', - idir_username: 'username' - }; - - sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); - - const userServiceStub = sinon.stub(UserService.prototype, 'getOrCreateSystemUser'); - - const requestHandler = self.getUser(); - - await requestHandler(mockReq, mockRes, mockNext); - - expect(userServiceStub).to.be.calledWith('aaaa', 'username', SYSTEM_IDENTITY_SOURCE.IDIR); - }); - it('should throw an error when a failure occurs', async () => { - const dbConnectionObj = getMockDBConnection({ systemUserId: () => 1 }); + const dbConnectionObj = getMockDBConnection({ + systemUserId: () => 1 + }); const { mockReq, mockRes, mockNext } = getRequestHandlerMocks(); - const expectedError = new Error("Failed to retrieve user's identifier or GUID"); + const expectedError = new Error('cannot process query'); sinon.stub(db, 'getDBConnection').returns({ ...dbConnectionObj, diff --git a/api/src/paths/user/self.ts b/api/src/paths/user/self.ts index 303a893d6..1221d0f2a 100644 --- a/api/src/paths/user/self.ts +++ b/api/src/paths/user/self.ts @@ -1,15 +1,25 @@ import { RequestHandler } from 'express'; import { Operation } from 'express-openapi'; -import { getAPIUserDBConnection } from '../../database/db'; +import { getDBConnection } from '../../database/db'; import { HTTP400 } from '../../errors/http-error'; -import { defaultErrorResponses } from '../../openapi/schemas/http-responses'; +import { authorizeRequestHandler } from '../../request-handlers/security/authorization'; import { UserService } from '../../services/user-service'; -import { getUserGuid, getUserIdentifier, getUserIdentitySource } from '../../utils/keycloak-utils'; import { getLogger } from '../../utils/logger'; -const defaultLog = getLogger('paths/user/self'); +const defaultLog = getLogger('paths/user/{userId}'); -export const GET: Operation = [getUser()]; +export const GET: Operation = [ + authorizeRequestHandler(() => { + return { + and: [ + { + discriminator: 'SystemUser' + } + ] + }; + }), + getUser() +]; GET.apiDoc = { description: 'Get user details for the currently authenticated user.', @@ -27,13 +37,30 @@ GET.apiDoc = { schema: { title: 'User Response Object', type: 'object', - required: ['id', 'user_identifier', 'user_guid', 'record_end_date', 'role_ids', 'role_names'], + required: [ + 'system_user_id', + 'user_identity_source_id', + 'user_identifier', + 'user_guid', + 'record_effective_date', + 'record_end_date', + 'create_user', + 'update_date', + 'update_user', + 'revision_count', + 'identity_source', + 'role_ids', + 'role_names' + ], properties: { - id: { + system_user_id: { description: 'user id', type: 'integer', minimum: 1 }, + user_identity_source_id: { + type: 'integer' + }, user_identifier: { description: 'The unique user identifier', type: 'string' @@ -43,11 +70,33 @@ GET.apiDoc = { description: 'The GUID for the user.', nullable: true }, + record_effective_date: { + type: 'string' + }, record_end_date: { - oneOf: [{ type: 'object' }, { type: 'string', format: 'date' }], - description: 'Determines if the user record has expired', + type: 'string', nullable: true }, + create_user: { + type: 'integer', + minimum: 1 + }, + update_date: { + type: 'string', + nullable: true + }, + update_user: { + type: 'integer', + minimum: 1, + nullable: true + }, + revision_count: { + type: 'integer', + minimum: 0 + }, + identity_source: { + type: 'string' + }, role_ids: { description: 'list of role ids for the user', type: 'array', @@ -68,40 +117,46 @@ GET.apiDoc = { } } }, - ...defaultErrorResponses + 400: { + $ref: '#/components/responses/400' + }, + 401: { + $ref: '#/components/responses/401' + }, + 403: { + $ref: '#/components/responses/403' + }, + 500: { + $ref: '#/components/responses/500' + }, + default: { + $ref: '#/components/responses/default' + } } }; /** - * Get a user by its user identifier. + * Get the currently logged in user. * * @returns {RequestHandler} */ export function getUser(): RequestHandler { return async (req, res) => { - const keycloakToken = req['keycloak_token']; - - // Use APIUser connection to get or create a new system user if they don't exist - const connection = getAPIUserDBConnection(); + const connection = getDBConnection(req['keycloak_token']); try { await connection.open(); - const userService = new UserService(connection); - - // Parse GUID, user identity from keycloak token - const userGuid = getUserGuid(keycloakToken); - const userIdentifier = getUserIdentifier(keycloakToken); - const userIdentitySource = getUserIdentitySource(keycloakToken); + const userId = connection.systemUserId(); - defaultLog.debug({ label: 'getUser', userGuid, userIdentifier, userIdentitySource }); - - if (!userGuid || !userIdentifier || !userIdentitySource) { - throw new HTTP400("Failed to retrieve user's identifier or GUID"); + if (!userId) { + throw new HTTP400('Failed to identify system user ID'); } - // Retrieves the system user if they exist, or creates a system user if they do not - const userObject = await userService.getOrCreateSystemUser(userGuid, userIdentifier, userIdentitySource); + const userService = new UserService(connection); + + // Fetch system user record + const userObject = await userService.getUserById(userId); await connection.commit(); diff --git a/api/src/paths/user/{userId}/delete.test.ts b/api/src/paths/user/{userId}/delete.test.ts index ce6af3024..ccf90dad4 100644 --- a/api/src/paths/user/{userId}/delete.test.ts +++ b/api/src/paths/user/{userId}/delete.test.ts @@ -26,11 +26,18 @@ describe('removeSystemUser', () => { sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, user_identifier: 'testname', - user_guid: 'aaaa', + user_identity_source_id: 2, + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '2010-10-10', record_end_date: '2010-10-10', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [1, 2], role_names: ['role 1', 'role 2'] }); @@ -58,11 +65,18 @@ describe('removeSystemUser', () => { sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, user_identifier: 'testname', - user_guid: 'aaaa', + user_identity_source_id: 2, + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '2010-10-10', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [1, 2], role_names: ['role 1', 'role 2'] }); @@ -91,11 +105,18 @@ describe('removeSystemUser', () => { sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, user_identifier: 'testname', - user_guid: 'aaaa', + user_identity_source_id: 2, + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '2010-10-10', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [1, 2], role_names: ['role 1', 'role 2'] }); @@ -126,11 +147,18 @@ describe('removeSystemUser', () => { sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, user_identifier: 'testname', - user_guid: 'aaaa', + user_identity_source_id: 2, + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '2010-10-10', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [1, 2], role_names: ['role 1', 'role 2'] }); diff --git a/api/src/paths/user/{userId}/get.test.ts b/api/src/paths/user/{userId}/get.test.ts index 0c2a98148..75809a38d 100644 --- a/api/src/paths/user/{userId}/get.test.ts +++ b/api/src/paths/user/{userId}/get.test.ts @@ -51,11 +51,18 @@ describe('user', () => { }; sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'user_identifier', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '2010-10-10', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [], role_names: [] }); @@ -65,11 +72,18 @@ describe('user', () => { await requestHandler(mockReq, mockRes, mockNext); expect(mockRes.jsonValue).to.eql({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'user_identifier', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '2010-10-10', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [], role_names: [] }); diff --git a/api/src/paths/user/{userId}/system-roles/create.test.ts b/api/src/paths/user/{userId}/system-roles/create.test.ts index 9db3369a4..bcf5704eb 100644 --- a/api/src/paths/user/{userId}/system-roles/create.test.ts +++ b/api/src/paths/user/{userId}/system-roles/create.test.ts @@ -90,11 +90,18 @@ describe('getAddSystemRolesHandler', () => { }; sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'test name', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [11, 22], role_names: ['role 11', 'role 22'] }); @@ -135,11 +142,18 @@ describe('getAddSystemRolesHandler', () => { }); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'test name', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [1, 2], role_names: ['role 1', 'role 2'] }); @@ -175,11 +189,18 @@ describe('getAddSystemRolesHandler', () => { }); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'test name', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [], role_names: ['role 11', 'role 22'] }); diff --git a/api/src/paths/user/{userId}/system-roles/update.test.ts b/api/src/paths/user/{userId}/system-roles/update.test.ts index 188cdd031..7214d670a 100644 --- a/api/src/paths/user/{userId}/system-roles/update.test.ts +++ b/api/src/paths/user/{userId}/system-roles/update.test.ts @@ -30,11 +30,18 @@ describe('updateSystemRolesHandler', () => { }; sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'test name', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [11, 22], role_names: ['role 11', 'role 22'] }); @@ -77,11 +84,18 @@ describe('updateSystemRolesHandler', () => { }); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'test name', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [11, 22], role_names: ['role 11', 'role 22'] }); @@ -114,11 +128,18 @@ describe('updateSystemRolesHandler', () => { sinon.stub(db, 'getDBConnection').returns(dbConnectionObj); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'test name', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [11, 22], role_names: ['role 1', 'role 2'] }); @@ -158,11 +179,18 @@ describe('updateSystemRolesHandler', () => { }); sinon.stub(UserService.prototype, 'getUserById').resolves({ - id: 1, + system_user_id: 1, + user_identity_source_id: 2, user_identifier: 'test name', - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', + record_effective_date: '', record_end_date: '', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [], role_names: [] }); diff --git a/api/src/repositories/spatial-repository.test.ts b/api/src/repositories/spatial-repository.test.ts index 36d19b48a..b276f88bb 100644 --- a/api/src/repositories/spatial-repository.test.ts +++ b/api/src/repositories/spatial-repository.test.ts @@ -7,7 +7,6 @@ import sinonChai from 'sinon-chai'; import SQL from 'sql-template-strings'; import { SPATIAL_COMPONENT_TYPE } from '../constants/spatial'; import { ApiGeneralError } from '../errors/api-error'; -import { UserObject } from '../models/user'; import { Srid3005 } from '../services/geo-service'; import { UserService } from '../services/user-service'; import * as spatialUtils from '../utils/spatial-utils'; @@ -18,6 +17,7 @@ import { ISubmissionSpatialComponent, SpatialRepository } from './spatial-repository'; +import { SystemUserExtended } from './user-repository'; chai.use(sinonChai); @@ -440,7 +440,7 @@ describe('SpatialRepository', () => { it('should call _findSpatialComponentsByCriteria', async () => { const mockDBConnection = getMockDBConnection(); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const findSpatialComponentsByCriteriaAsAdminUserStub = sinon diff --git a/api/src/repositories/user-repository.test.ts b/api/src/repositories/user-repository.test.ts index 041ea8cd8..ae7accefd 100644 --- a/api/src/repositories/user-repository.test.ts +++ b/api/src/repositories/user-repository.test.ts @@ -101,7 +101,7 @@ describe('UserRepository', () => { { system_user_id: 1, user_identifier: 1, - user_guid: 'aaaa', + user_guid: '123-456-789', identity_source: 'idir', record_end_date: 'data', role_ids: [1], @@ -118,7 +118,7 @@ describe('UserRepository', () => { const userRepository = new UserRepository(mockDBConnection); - const response = await userRepository.getUserByGuid('aaaa'); + const response = await userRepository.getUserByGuid('123-456-789'); expect(response).to.equal(mockResponse); }); @@ -153,7 +153,7 @@ describe('UserRepository', () => { system_user_id: 1, user_identity_source_id: 1, user_identifier: 'user', - user_guid: 'aaaa', + user_guid: '123-456-789', record_end_date: 'data', record_effective_date: 'date' } @@ -168,7 +168,7 @@ describe('UserRepository', () => { const userRepository = new UserRepository(mockDBConnection); - const response = await userRepository.addSystemUser('aaaa', 'user', 'idir'); + const response = await userRepository.addSystemUser('123-456-789', 'user', 'idir'); expect(response).to.equal(mockResponse[0]); }); diff --git a/api/src/repositories/user-repository.ts b/api/src/repositories/user-repository.ts index 08e90617a..46d49c6e1 100644 --- a/api/src/repositories/user-repository.ts +++ b/api/src/repositories/user-repository.ts @@ -59,7 +59,6 @@ export class UserRepository extends BaseRepository { /** * Fetch a single system user by their system user ID. * - * * @param {number} systemUserId * @return {*} {Promise} * @memberof UserRepository @@ -166,6 +165,60 @@ export class UserRepository extends BaseRepository { return response.rows; } + /** + * 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` + SELECT + su.*, + 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 + LOWER(su.user_identifier) = ${userIdentifier.toLowerCase()} + AND + uis.name = ${identitySource.toUpperCase()} + GROUP BY + su.system_user_id, + su.user_identity_source_id, + su.user_identifier, + su.user_guid, + su.record_effective_date, + su.record_end_date, + su.create_date, + su.create_user, + su.update_date, + su.update_user, + su.revision_count, + uis.name; + `; + + const response = await this.connection.sql(sqlStatement, SystemUserExtended); + + return response.rows; + } + /** * Adds a new system user. * diff --git a/api/src/request-handlers/security/authorization.test.ts b/api/src/request-handlers/security/authorization.test.ts index e63541492..6fb3482be 100644 --- a/api/src/request-handlers/security/authorization.test.ts +++ b/api/src/request-handlers/security/authorization.test.ts @@ -4,7 +4,7 @@ import { describe } from 'mocha'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { HTTPError } from '../../errors/http-error'; -import { Models } from '../../models'; +import { SystemUserExtended } from '../../repositories/user-repository'; import { AuthorizationService } from '../../services/authorization-service'; import { getRequestHandlerMocks, registerMockDBConnection } from '../../__mocks__/db'; import * as authorization from './authorization'; @@ -67,7 +67,7 @@ describe('authorizeRequest', function () { it('returns false if systemUserObject is null', async function () { registerMockDBConnection(); - const mockSystemUserObject = undefined as unknown as Models.user.UserObject; + const mockSystemUserObject = undefined as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockSystemUserObject); const mockReq = { authorization_scheme: {} } as unknown as Request; @@ -79,7 +79,7 @@ describe('authorizeRequest', function () { it('returns true if the user is a system administrator', async function () { registerMockDBConnection(); - const mockSystemUserObject = { role_names: [] } as unknown as Models.user.UserObject; + const mockSystemUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockSystemUserObject); sinon.stub(AuthorizationService.prototype, 'authorizeSystemAdministrator').resolves(true); @@ -93,7 +93,7 @@ describe('authorizeRequest', function () { it('returns true if the authorization_scheme is undefined', async function () { registerMockDBConnection(); - const mockSystemUserObject = { role_names: [] } as unknown as Models.user.UserObject; + const mockSystemUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockSystemUserObject); sinon.stub(AuthorizationService.prototype, 'authorizeSystemAdministrator').resolves(false); @@ -107,7 +107,7 @@ describe('authorizeRequest', function () { it('returns true if the user is authorized against the authorization_scheme', async function () { registerMockDBConnection(); - const mockSystemUserObject = { role_names: [] } as unknown as Models.user.UserObject; + const mockSystemUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockSystemUserObject); sinon.stub(AuthorizationService.prototype, 'authorizeSystemAdministrator').resolves(false); @@ -123,7 +123,7 @@ describe('authorizeRequest', function () { it('returns false if the user is not authorized against the authorization_scheme', async function () { registerMockDBConnection(); - const mockSystemUserObject = { role_names: [] } as unknown as Models.user.UserObject; + const mockSystemUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockSystemUserObject); sinon.stub(AuthorizationService.prototype, 'authorizeSystemAdministrator').resolves(false); diff --git a/api/src/services/authorization-service.test.ts b/api/src/services/authorization-service.test.ts index 25458200a..aa636549b 100644 --- a/api/src/services/authorization-service.test.ts +++ b/api/src/services/authorization-service.test.ts @@ -5,7 +5,7 @@ import sinonChai from 'sinon-chai'; import { SOURCE_SYSTEM } from '../constants/database'; import { SYSTEM_ROLE } from '../constants/roles'; import * as db from '../database/db'; -import { Models } from '../models'; +import { SystemUserExtended } from '../repositories/user-repository'; import { AuthorizationScheme, AuthorizationService, @@ -131,7 +131,7 @@ describe('authorizeByServiceClient', function () { const mockGetSystemUsersObjectResponse = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] - } as unknown as Models.user.UserObject; + } as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockGetSystemUsersObjectResponse); @@ -166,7 +166,7 @@ describe('authorizeBySystemRole', function () { }; const mockDBConnection = getMockDBConnection(); - const mockGetSystemUsersObjectResponse = null as unknown as Models.user.UserObject; + const mockGetSystemUsersObjectResponse = null as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockGetSystemUsersObjectResponse); const authorizationService = new AuthorizationService(mockDBConnection); @@ -183,7 +183,7 @@ describe('authorizeBySystemRole', function () { }; const mockDBConnection = getMockDBConnection(); - const mockGetSystemUsersObjectResponse = { record_end_date: 'datetime' } as unknown as Models.user.UserObject; + const mockGetSystemUsersObjectResponse = { record_end_date: 'datetime' } as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockGetSystemUsersObjectResponse); const authorizationService = new AuthorizationService(mockDBConnection); @@ -201,7 +201,7 @@ describe('authorizeBySystemRole', function () { const mockDBConnection = getMockDBConnection(); const authorizationService = new AuthorizationService(mockDBConnection, { - systemUser: {} as unknown as Models.user.UserObject + systemUser: {} as unknown as SystemUserExtended }); const isAuthorizedBySystemRole = await authorizationService.authorizeBySystemRole(mockAuthorizeSystemRoles); @@ -217,7 +217,7 @@ describe('authorizeBySystemRole', function () { const mockDBConnection = getMockDBConnection(); const authorizationService = new AuthorizationService(mockDBConnection, { - systemUser: { role_names: [] } as unknown as Models.user.UserObject + systemUser: { role_names: [] } as unknown as SystemUserExtended }); const isAuthorizedBySystemRole = await authorizationService.authorizeBySystemRole(mockAuthorizeSystemRoles); @@ -233,7 +233,7 @@ describe('authorizeBySystemRole', function () { const mockDBConnection = getMockDBConnection(); const authorizationService = new AuthorizationService(mockDBConnection, { - systemUser: { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as Models.user.UserObject + systemUser: { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as SystemUserExtended }); const isAuthorizedBySystemRole = await authorizationService.authorizeBySystemRole(mockAuthorizeSystemRoles); @@ -250,7 +250,7 @@ describe('authorizeBySystemUser', function () { it('returns false if `systemUserObject` is null', async function () { const mockDBConnection = getMockDBConnection(); - const mockGetSystemUsersObjectResponse = null as unknown as Models.user.UserObject; + const mockGetSystemUsersObjectResponse = null as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockGetSystemUsersObjectResponse); const authorizationService = new AuthorizationService(mockDBConnection); @@ -263,11 +263,11 @@ describe('authorizeBySystemUser', function () { it('returns true if `systemUserObject` is not null', async function () { const mockDBConnection = getMockDBConnection(); - const mockGetSystemUsersObjectResponse = null as unknown as Models.user.UserObject; + const mockGetSystemUsersObjectResponse = null as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockGetSystemUsersObjectResponse); const authorizationService = new AuthorizationService(mockDBConnection, { - systemUser: {} as unknown as Models.user.UserObject + systemUser: {} as unknown as SystemUserExtended }); const isAuthorizedBySystemRole = await authorizationService.authorizeBySystemUser(); @@ -333,7 +333,7 @@ describe('authorizeByServiceClient', function () { it('returns true if `systemUserObject` hasAtLeastOneValidValue', async function () { const mockDBConnection = getMockDBConnection(); - const mockGetSystemUsersObjectResponse = null as unknown as Models.user.UserObject; + const mockGetSystemUsersObjectResponse = null as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserObject').resolves(mockGetSystemUsersObjectResponse); const authorizationService = new AuthorizationService(mockDBConnection, { @@ -493,10 +493,10 @@ describe('getSystemUserObject', function () { expect(systemUserObject).to.equal(null); }); - it('returns a `UserObject`', async function () { + it('returns a system user', async function () { const mockDBConnection = getMockDBConnection(); - const mockSystemUserWithRolesResponse = new Models.user.UserObject(); + const mockSystemUserWithRolesResponse = {} as unknown as SystemUserExtended; sinon.stub(AuthorizationService.prototype, 'getSystemUserWithRoles').resolves(mockSystemUserWithRolesResponse); const authorizationService = new AuthorizationService(mockDBConnection); @@ -536,11 +536,11 @@ describe('getSystemUserWithRoles', function () { expect(result).to.be.null; }); - it('returns a UserObject', async function () { + it('returns a system user', async function () { const mockDBConnection = getMockDBConnection(); sinon.stub(db, 'getDBConnection').returns(mockDBConnection); - const userObjectMock = new Models.user.UserObject(); + const userObjectMock = {} as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserByGuid').resolves(userObjectMock); const authorizationService = new AuthorizationService(mockDBConnection, { diff --git a/api/src/services/authorization-service.ts b/api/src/services/authorization-service.ts index cb0db1a8f..6c73875c6 100644 --- a/api/src/services/authorization-service.ts +++ b/api/src/services/authorization-service.ts @@ -1,7 +1,7 @@ import { SOURCE_SYSTEM } from '../constants/database'; import { SYSTEM_ROLE } from '../constants/roles'; import { IDBConnection } from '../database/db'; -import { Models } from '../models'; +import { SystemUserExtended } from '../repositories/user-repository'; import { getKeycloakSource, getUserGuid } from '../utils/keycloak-utils'; import { DBService } from './db-service'; import { UserService } from './user-service'; @@ -61,26 +61,24 @@ export type AuthorizationScheme = AuthorizeConfigAnd | AuthorizeConfigOr; export class AuthorizationService extends DBService { _userService = new UserService(this.connection); - _systemUser: Models.user.UserObject | undefined = undefined; + _systemUser: SystemUserExtended | undefined = undefined; _keycloakToken: object | undefined = undefined; - constructor(connection: IDBConnection, init?: { systemUser?: Models.user.UserObject; keycloakToken?: object }) { + constructor(connection: IDBConnection, init?: { systemUser?: SystemUserExtended; keycloakToken?: object }) { super(connection); this._systemUser = init?.systemUser; this._keycloakToken = init?.keycloakToken; } - get systemUser(): Models.user.UserObject | undefined { + get systemUser(): SystemUserExtended | undefined { return this._systemUser; } /** * Execute the `authorizationScheme` against the current user, and return `true` if they have access, `false` otherwise. * - * @param {UserObject} systemUserObject * @param {AuthorizationScheme} authorizationScheme - * @param {IDBConnection} connection * @return {*} {Promise} `true` if the `authorizationScheme` indicates the user has access, `false` otherwise. */ async executeAuthorizationScheme(authorizationScheme: AuthorizationScheme): Promise { @@ -245,7 +243,7 @@ export class AuthorizationService extends DBService { return false; }; - async getSystemUserObject(): Promise { + async getSystemUserObject(): Promise { let systemUserWithRoles; try { @@ -264,9 +262,9 @@ export class AuthorizationService extends DBService { /** * Finds a single user based on their keycloak token information. * - * @return {*} {(Promise)} + * @return {*} {(Promise)} */ - async getSystemUserWithRoles(): Promise { + async getSystemUserWithRoles(): Promise { if (!this._keycloakToken) { return null; } diff --git a/api/src/services/spatial-service.test.ts b/api/src/services/spatial-service.test.ts index 137047218..c46bd69bd 100644 --- a/api/src/services/spatial-service.test.ts +++ b/api/src/services/spatial-service.test.ts @@ -5,7 +5,6 @@ import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { SYSTEM_ROLE } from '../constants/roles'; import { SPATIAL_COMPONENT_TYPE } from '../constants/spatial'; -import { UserObject } from '../models/user'; import { IGetSecurityTransformRecord, IGetSpatialTransformRecord, @@ -15,6 +14,7 @@ import { ISubmissionSpatialSearchResponseRow, SpatialRepository } from '../repositories/spatial-repository'; +import { SystemUserExtended } from '../repositories/user-repository'; import { getMockDBConnection } from '../__mocks__/db'; import { Srid3005 } from './geo-service'; import { SpatialService } from './spatial-service'; @@ -269,7 +269,7 @@ describe('SpatialService', () => { it('should return spatial component search result rows', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const mockResponseRows = [ @@ -305,7 +305,7 @@ describe('SpatialService', () => { it('should call findSpatialComponentsByCriteriaAsAdminUser as data admin', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [SYSTEM_ROLE.DATA_ADMINISTRATOR] } as unknown as UserObject; + const mockUserObject = { role_names: [SYSTEM_ROLE.DATA_ADMINISTRATOR] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const findSpatialComponentsByCriteriaAsAdminUserStub = sinon @@ -329,7 +329,7 @@ describe('SpatialService', () => { it('should call findSpatialComponentsByCriteriaAsAdminUser as system admin', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as UserObject; + const mockUserObject = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const findSpatialComponentsByCriteriaAsAdminUserStub = sinon @@ -353,7 +353,7 @@ describe('SpatialService', () => { it('should return spatial component search result rows', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const mockResponseRows = [ @@ -446,7 +446,7 @@ describe('SpatialService', () => { it('should return spatial component metadata', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const mockResponseRows: ISpatialComponentFeaturePropertiesRow[] = [ @@ -473,7 +473,7 @@ describe('SpatialService', () => { it('should return spatial component metadata', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const mockResponseRows: ISpatialComponentFeaturePropertiesRow[] = [ @@ -509,7 +509,7 @@ describe('SpatialService', () => { it('should return spatial component metadata as system admin', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as UserObject; + const mockUserObject = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const mockResponseRows: ISpatialComponentFeaturePropertiesRow[] = [ @@ -543,7 +543,7 @@ describe('SpatialService', () => { it('should return spatial component metadata as data admin', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [SYSTEM_ROLE.DATA_ADMINISTRATOR] } as unknown as UserObject; + const mockUserObject = { role_names: [SYSTEM_ROLE.DATA_ADMINISTRATOR] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const mockResponseRows: ISpatialComponentFeaturePropertiesRow[] = [ @@ -612,7 +612,7 @@ describe('SpatialService', () => { it('should return [] as system admin', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as UserObject; + const mockUserObject = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const repo = sinon @@ -628,7 +628,7 @@ describe('SpatialService', () => { it('should return [] as data admin', async () => { const mockDBConnection = getMockDBConnection(); const spatialService = new SpatialService(mockDBConnection); - const mockUserObject = { role_names: [SYSTEM_ROLE.DATA_ADMINISTRATOR] } as unknown as UserObject; + const mockUserObject = { role_names: [SYSTEM_ROLE.DATA_ADMINISTRATOR] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const repo = sinon diff --git a/api/src/services/submission-service.test.ts b/api/src/services/submission-service.test.ts index fd8a3745f..f23b48647 100644 --- a/api/src/services/submission-service.test.ts +++ b/api/src/services/submission-service.test.ts @@ -5,7 +5,6 @@ import { QueryResult } from 'pg'; import sinon from 'sinon'; import sinonChai from 'sinon-chai'; import { ApiExecuteSQLError } from '../errors/api-error'; -import { UserObject } from '../models/user'; import { SECURITY_APPLIED_STATUS } from '../repositories/security-repository'; import { ISourceTransformModel, @@ -21,6 +20,7 @@ import { SUBMISSION_MESSAGE_TYPE, SUBMISSION_STATUS_TYPE } from '../repositories/submission-repository'; +import { SystemUserExtended } from '../repositories/user-repository'; import { EMLFile } from '../utils/media/eml/eml-file'; import { getMockDBConnection } from '../__mocks__/db'; import { SubmissionService } from './submission-service'; @@ -459,7 +459,7 @@ describe('SubmissionService', () => { it('should return submission with count object', async () => { const mockDBConnection = getMockDBConnection(); const submissionService = new SubmissionService(mockDBConnection); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const findSubmissionRecordEMLJSONByDatasetIdStub = sinon @@ -482,7 +482,7 @@ describe('SubmissionService', () => { it('should return submission with count object', async () => { const mockDBConnection = getMockDBConnection(); const submissionService = new SubmissionService(mockDBConnection); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const findSubmissionRecordEMLJSONByDatasetIdStub = sinon @@ -507,7 +507,7 @@ describe('SubmissionService', () => { it('should return null', async () => { const mockDBConnection = getMockDBConnection(); const submissionService = new SubmissionService(mockDBConnection); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const findSubmissionRecordEMLJSONByDatasetIdStub = sinon diff --git a/api/src/services/user-service.test.ts b/api/src/services/user-service.test.ts index 3db53b3f0..0b0bdacfb 100644 --- a/api/src/services/user-service.test.ts +++ b/api/src/services/user-service.test.ts @@ -5,8 +5,6 @@ import sinonChai from 'sinon-chai'; import { SYSTEM_IDENTITY_SOURCE } from '../constants/database'; import { SYSTEM_ROLE } from '../constants/roles'; import { ApiError } from '../errors/api-error'; -import { Models } from '../models'; -import { UserObject } from '../models/user'; import { SystemRoles, SystemUser, SystemUserExtended, UserRepository } from '../repositories/user-repository'; import { getMockDBConnection } from '../__mocks__/db'; import { UserService } from './user-service'; @@ -40,7 +38,7 @@ describe('UserService', () => { sinon.restore(); }); - it('returns a UserObject', async function () { + it('returns a system user', async function () { const mockDBConnection = getMockDBConnection(); const mockResponseRow = { system_user_id: 123 }; @@ -51,7 +49,7 @@ describe('UserService', () => { const result = await userService.getUserById(1); - expect(result).to.eql(new Models.user.UserObject(mockResponseRow)); + expect(result).to.eql(mockResponseRow); expect(mockUserRepository).to.have.been.calledOnce; }); }); @@ -68,13 +66,13 @@ describe('UserService', () => { const userService = new UserService(mockDBConnection); - const result = await userService.getUserByGuid('aaaa'); + const result = await userService.getUserByGuid('123-456-789'); expect(result).to.be.null; expect(mockUserRepository).to.have.been.calledOnce; }); - it('returns a UserObject for the first row of the response', async function () { + it('returns a system user for the first row of the response', async function () { const mockDBConnection = getMockDBConnection(); const mockResponseRow = [{ system_user_id: 123 }]; @@ -83,9 +81,9 @@ describe('UserService', () => { const userService = new UserService(mockDBConnection); - const result = await userService.getUserByGuid('aaaa'); + const result = await userService.getUserByGuid('123-456-789'); - expect(result).to.eql(new Models.user.UserObject(mockResponseRow[0])); + expect(result).to.eql(mockResponseRow[0]); expect(mockUserRepository).to.have.been.calledOnce; }); }); @@ -98,7 +96,7 @@ describe('UserService', () => { it('should not be an admin', async () => { const mockDBConnection = getMockDBConnection(); const userService = new UserService(mockDBConnection); - const mockUserObject = { role_names: [] } as unknown as UserObject; + const mockUserObject = { role_names: [] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const isAdmin = await userService.isSystemUserAdmin(); @@ -108,7 +106,7 @@ describe('UserService', () => { it('should be an admin as data admin', async () => { const mockDBConnection = getMockDBConnection(); const userService = new UserService(mockDBConnection); - const mockUserObject = { role_names: [SYSTEM_ROLE.DATA_ADMINISTRATOR] } as unknown as UserObject; + const mockUserObject = { role_names: [SYSTEM_ROLE.DATA_ADMINISTRATOR] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const isAdmin = await userService.isSystemUserAdmin(); @@ -118,7 +116,7 @@ describe('UserService', () => { it('should be an admin as system admin', async () => { const mockDBConnection = getMockDBConnection(); const userService = new UserService(mockDBConnection); - const mockUserObject = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as UserObject; + const mockUserObject = { role_names: [SYSTEM_ROLE.SYSTEM_ADMIN] } as unknown as SystemUserExtended; sinon.stub(UserService.prototype, 'getUserById').resolves(mockUserObject); const isAdmin = await userService.isSystemUserAdmin(); @@ -136,17 +134,17 @@ describe('UserService', () => { const mockRowObj = { system_user_id: 123 }; const mockUserRepository = sinon.stub(UserRepository.prototype, 'addSystemUser'); - mockUserRepository.resolves(mockRowObj as unknown as SystemUser); + mockUserRepository.resolves(mockRowObj as unknown as SystemUserExtended); const userService = new UserService(mockDBConnection); const userIdentifier = 'username'; - const userGuid = 'aaaa'; + const userGuid = '123-456-789'; const identitySource = SYSTEM_IDENTITY_SOURCE.IDIR; const result = await userService.addSystemUser(userGuid, userIdentifier, identitySource); - expect(result).to.eql(new Models.user.UserObject(mockRowObj)); + expect(result).to.eql(mockRowObj); expect(mockUserRepository).to.have.been.calledOnce; }); }); @@ -168,7 +166,7 @@ describe('UserService', () => { expect(result).to.eql([]); }); - it('returns a UserObject for each row of the response', async function () { + it('returns a system user for each row of the response', async function () { const mockDBConnection = getMockDBConnection(); const mockResponseRows = [{ system_user_id: 123 }, { system_user_id: 456 }, { system_user_id: 789 }]; @@ -179,11 +177,7 @@ describe('UserService', () => { const result = await userService.listSystemUsers(); - expect(result).to.eql([ - new Models.user.UserObject(mockResponseRows[0]), - new Models.user.UserObject(mockResponseRows[1]), - new Models.user.UserObject(mockResponseRows[2]) - ]); + expect(result).to.eql([mockResponseRows[0], mockResponseRows[1], mockResponseRows[2]]); }); }); @@ -202,7 +196,7 @@ describe('UserService', () => { const activateSystemUserStub = sinon.stub(UserService.prototype, 'activateSystemUser'); const userIdentifier = 'username'; - const userGuid = 'aaaa'; + const userGuid = '123-456-789'; const identitySource = SYSTEM_IDENTITY_SOURCE.IDIR; const userService = new UserService(mockDBConnection); @@ -225,11 +219,17 @@ describe('UserService', () => { const existingSystemUser = null; const getUserByGuidStub = sinon.stub(UserService.prototype, 'getUserByGuid').resolves(existingSystemUser); - const addedSystemUser = new Models.user.UserObject({ system_user_id: 2, record_end_date: null }); - const addSystemUserStub = sinon.stub(UserService.prototype, 'addSystemUser').resolves(addedSystemUser); + const addedSystemUser = { system_user_id: 2, record_end_date: null }; + const addSystemUserStub = sinon + .stub(UserService.prototype, 'addSystemUser') + .resolves(addedSystemUser as unknown as SystemUser); const activateSystemUserStub = sinon.stub(UserService.prototype, 'activateSystemUser'); + const getUserById = sinon + .stub(UserService.prototype, 'getUserById') + .resolves(addedSystemUser as unknown as SystemUserExtended); + const userIdentifier = 'username'; const userGuid = 'aaaa'; const identitySource = SYSTEM_IDENTITY_SOURCE.IDIR; @@ -238,24 +238,35 @@ describe('UserService', () => { const result = await userService.ensureSystemUser(userGuid, userIdentifier, identitySource); - expect(result.id).to.equal(2); + expect(result.system_user_id).to.equal(2); expect(result.record_end_date).to.equal(null); expect(getUserByGuidStub).to.have.been.calledOnce; expect(addSystemUserStub).to.have.been.calledOnce; + expect(getUserById).to.have.been.calledOnce; expect(activateSystemUserStub).not.to.have.been.called; }); it('gets an existing system user that is already activate', async () => { const mockDBConnection = getMockDBConnection({ systemUserId: () => 1 }); - const existingInactiveSystemUser = new Models.user.UserObject({ + const existingInactiveSystemUser: SystemUserExtended = { system_user_id: 2, - user_identifier: SYSTEM_IDENTITY_SOURCE.IDIR, + user_identifier: 'username', + user_identity_source_id: 2, + identity_source: SYSTEM_IDENTITY_SOURCE.IDIR, + user_guid: '', + record_effective_date: '2020-10-10', record_end_date: null, role_ids: [1], - role_names: ['Editor'] - }); + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, + role_names: ['Collaborator'] + }; + const getUserByGuidStub = sinon.stub(UserService.prototype, 'getUserByGuid').resolves(existingInactiveSystemUser); const addSystemUserStub = sinon.stub(UserService.prototype, 'addSystemUser'); @@ -270,7 +281,7 @@ describe('UserService', () => { const result = await userService.ensureSystemUser(userGuid, userIdentifier, identitySource); - expect(result.id).to.equal(2); + expect(result.system_user_id).to.equal(2); expect(result.record_end_date).to.equal(null); expect(getUserByGuidStub).to.have.been.calledOnce; @@ -281,26 +292,46 @@ describe('UserService', () => { it('gets an existing system user that is not already active and re-activates it', async () => { const mockDBConnection = getMockDBConnection({ systemUserId: () => 1 }); - const existingSystemUser = new Models.user.UserObject({ + const existingSystemUser: SystemUserExtended = { system_user_id: 2, - user_identifier: SYSTEM_IDENTITY_SOURCE.IDIR, - record_end_date: '2021-11-22', + user_identity_source_id: 2, + user_identifier: 'username', + identity_source: SYSTEM_IDENTITY_SOURCE.IDIR, + user_guid: '', + record_effective_date: '2020-10-10', + record_end_date: '1900-01-01', + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [1], - role_names: ['Editor'] - }); + role_names: ['Collaborator'] + }; + const getUserByGuidStub = sinon.stub(UserService.prototype, 'getUserByGuid').resolves(existingSystemUser); const addSystemUserStub = sinon.stub(UserService.prototype, 'addSystemUser'); const activateSystemUserStub = sinon.stub(UserService.prototype, 'activateSystemUser'); - const activatedSystemUser = new Models.user.UserObject({ + const activatedSystemUser: SystemUserExtended = { system_user_id: 2, - user_identifier: SYSTEM_IDENTITY_SOURCE.IDIR, + user_identity_source_id: 2, + user_identifier: 'username', + identity_source: SYSTEM_IDENTITY_SOURCE.IDIR, + user_guid: '', + record_effective_date: '2020-10-10', record_end_date: null, + create_user: 1, + create_date: '', + update_user: null, + update_date: null, + revision_count: 0, role_ids: [1], - role_names: ['Editor'] - }); + role_names: ['Collaborator'] + }; + const getUserByIdStub = sinon.stub(UserService.prototype, 'getUserById').resolves(activatedSystemUser); const userIdentifier = 'username'; @@ -311,7 +342,7 @@ describe('UserService', () => { const result = await userService.ensureSystemUser(userGuid, userIdentifier, identitySource); - expect(result.id).to.equal(2); + expect(result.system_user_id).to.equal(2); expect(result.record_end_date).to.equal(null); expect(getUserByGuidStub).to.have.been.calledOnce; @@ -321,105 +352,6 @@ describe('UserService', () => { }); }); - describe('getOrCreateSystemUser', () => { - afterEach(() => { - sinon.restore(); - }); - - it('creates a new system user if one does not already exist', async () => { - const mockDBConnection = getMockDBConnection(); - - const existingSystemUser = null; - const getUserByGuidStub = sinon.stub(UserService.prototype, 'getUserByGuid').resolves(existingSystemUser); - - const addedSystemUser = new Models.user.UserObject({ system_user_id: 2, record_end_date: null }); - const addSystemUserStub = sinon.stub(UserService.prototype, 'addSystemUser').resolves(addedSystemUser); - - const activateSystemUserStub = sinon.stub(UserService.prototype, 'activateSystemUser'); - - const userIdentifier = 'username'; - const userGuid = 'aaaa'; - const identitySource = SYSTEM_IDENTITY_SOURCE.IDIR; - - const userService = new UserService(mockDBConnection); - - const result = await userService.getOrCreateSystemUser(userGuid, userIdentifier, identitySource); - - expect(result.id).to.equal(2); - expect(result.record_end_date).to.equal(null); - - expect(getUserByGuidStub).to.have.been.calledOnce; - expect(addSystemUserStub).to.have.been.calledOnce; - expect(activateSystemUserStub).not.to.have.been.called; - }); - - it('gets an existing system user that is activate', async () => { - const mockDBConnection = getMockDBConnection({ systemUserId: () => 1 }); - - const existingInactiveSystemUser = new Models.user.UserObject({ - system_user_id: 2, - user_identifier: SYSTEM_IDENTITY_SOURCE.IDIR, - record_end_date: null, - role_ids: [1], - role_names: ['Editor'] - }); - - const getUserByGuidStub = sinon.stub(UserService.prototype, 'getUserByGuid').resolves(existingInactiveSystemUser); - - const addSystemUserStub = sinon.stub(UserService.prototype, 'addSystemUser'); - - const activateSystemUserStub = sinon.stub(UserService.prototype, 'activateSystemUser'); - - const userIdentifier = 'username'; - const userGuid = 'aaaa'; - const identitySource = SYSTEM_IDENTITY_SOURCE.IDIR; - - const userService = new UserService(mockDBConnection); - - const result = await userService.getOrCreateSystemUser(userGuid, userIdentifier, identitySource); - - expect(result.id).to.equal(2); - expect(result.record_end_date).to.equal(null); - - expect(getUserByGuidStub).to.have.been.calledOnce; - expect(addSystemUserStub).not.to.have.been.called; - expect(activateSystemUserStub).not.to.have.been.called; - }); - - it('gets an existing system user that is not active and does not re-activate it', async () => { - const mockDBConnection = getMockDBConnection({ systemUserId: () => 1 }); - - const existingSystemUser = new Models.user.UserObject({ - system_user_id: 2, - user_identifier: SYSTEM_IDENTITY_SOURCE.IDIR, - record_end_date: '2021-11-22', - role_ids: [1], - role_names: ['Editor'] - }); - - const getUserByGuidStub = sinon.stub(UserService.prototype, 'getUserByGuid').resolves(existingSystemUser); - - const addSystemUserStub = sinon.stub(UserService.prototype, 'addSystemUser'); - - const activateSystemUserStub = sinon.stub(UserService.prototype, 'activateSystemUser'); - - const userIdentifier = 'username'; - const userGuid = 'aaaa'; - const identitySource = SYSTEM_IDENTITY_SOURCE.IDIR; - - const userService = new UserService(mockDBConnection); - - const result = await userService.getOrCreateSystemUser(userGuid, userIdentifier, identitySource); - - expect(result.id).to.equal(2); - expect(result.record_end_date).to.equal('2021-11-22'); - - expect(getUserByGuidStub).to.have.been.calledOnce; - expect(addSystemUserStub).not.to.have.been.called; - expect(activateSystemUserStub).to.not.have.been.calledOnce; - }); - }); - describe('activateSystemUser', function () { afterEach(() => { sinon.restore(); diff --git a/api/src/services/user-service.ts b/api/src/services/user-service.ts index 06e704e2a..ddc467ced 100644 --- a/api/src/services/user-service.ts +++ b/api/src/services/user-service.ts @@ -1,13 +1,9 @@ import { SYSTEM_ROLE } from '../constants/roles'; import { IDBConnection } from '../database/db'; import { ApiExecuteSQLError } from '../errors/api-error'; -import { Models } from '../models'; -import { SystemRoles, UserRepository } from '../repositories/user-repository'; -import { getLogger } from '../utils/logger'; +import { SystemRoles, SystemUser, SystemUserExtended, UserRepository } from '../repositories/user-repository'; import { DBService } from './db-service'; -const defaultLog = getLogger('services/user-service'); - export class UserService extends DBService { userRepository: UserRepository; @@ -30,32 +26,46 @@ export class UserService extends DBService { * Fetch a single system user by their system user ID. * * @param {number} systemUserId - * @return {*} {(Promise)} + * @return {*} {Promise} * @memberof UserService */ - async getUserById(systemUserId: number): Promise { - const response = await this.userRepository.getUserById(systemUserId); - - return new Models.user.UserObject(response); + async getUserById(systemUserId: number): Promise { + return this.userRepository.getUserById(systemUserId); } /** * Get an existing system user by their GUID. * * @param {string} userGuid The user's GUID - * @return {*} {(Promise)} + * @return {*} {(Promise)} * @memberof UserService */ - async getUserByGuid(userGuid: string): Promise { - defaultLog.debug({ label: 'getUserByGuid', userGuid }); - + async getUserByGuid(userGuid: string): Promise { const response = await this.userRepository.getUserByGuid(userGuid); if (response.length !== 1) { return null; } - return new Models.user.UserObject(response[0]); + return response[0]; + } + + /** + * 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 User, or `null` if the user wasn't found. + * @memberof UserService + */ + async getUserByIdentifier(userIdentifier: string, identitySource: string): Promise { + const response = await this.userRepository.getUserByIdentifier(userIdentifier, identitySource); + + if (response.length !== 1) { + return null; + } + + return response[0]; } /** @@ -66,29 +76,21 @@ export class UserService extends DBService { * @param {string} userGuid * @param {string} userIdentifier * @param {string} identitySource - * @return {*} {Promise} + * @return {*} {Promise} * @memberof UserService */ - async addSystemUser( - userGuid: string, - userIdentifier: string, - identitySource: string - ): Promise { - const response = await this.userRepository.addSystemUser(userGuid, userIdentifier, identitySource); - - return new Models.user.UserObject(response); + async addSystemUser(userGuid: string, userIdentifier: string, identitySource: string): Promise { + return this.userRepository.addSystemUser(userGuid, userIdentifier, identitySource); } /** * Get a list of all system users. * - * @return {*} {Promise} + * @return {*} {Promise} * @memberof UserService */ - async listSystemUsers(): Promise { - const response = await this.userRepository.listSystemUsers(); - - return response.map((row) => new Models.user.UserObject(row)); + async listSystemUsers(): Promise { + return this.userRepository.listSystemUsers(); } /** @@ -98,19 +100,21 @@ export class UserService extends DBService { * @param {string} userGuid * @param {string} userIdentifier * @param {string} identitySource - * @return {*} {Promise} + * @return {*} {Promise} * @memberof UserService */ async ensureSystemUser( userGuid: string, userIdentifier: string, identitySource: string - ): Promise { - // Check if the user exists - let userObject = await this.getUserByGuid(userGuid); - - if (!userObject) { - // ID of the current authenticated user + ): Promise { + // Check if the user exists in SIMS + const existingUser = userGuid + ? await this.getUserByGuid(userGuid) + : await this.getUserByIdentifier(userIdentifier, identitySource); + + if (!existingUser) { + // Id of the current authenticated user const systemUserId = this.connection.systemUserId(); if (!systemUserId) { @@ -118,47 +122,22 @@ export class UserService extends DBService { } // Found no existing user, add them - userObject = await this.addSystemUser(userGuid, userIdentifier, identitySource); + const newUserId = await this.addSystemUser(userGuid, userIdentifier, identitySource); + + // fetch the new user object + return this.getUserById(newUserId.system_user_id); } - if (!userObject.record_end_date) { + if (!existingUser.record_end_date) { // system user is already active - return userObject; + return existingUser; } // system user is not active, re-activate them - await this.activateSystemUser(userObject.id); + await this.activateSystemUser(existingUser.system_user_id); // get the newly activated user - return this.getUserById(userObject.id); - } - - /** - * Gets a system user, adding them if they do not already exist. - * - * @param {string} userGuid - * @param {string} userIdentifier - * @param {string} identitySource - * @return {*} {Promise} - * @memberof UserService - */ - async getOrCreateSystemUser( - userGuid: string, - userIdentifier: string, - identitySource: string - ): Promise { - defaultLog.debug({ label: 'getUserByGuid', userGuid, userIdentifier, identitySource }); - - // Check if the user exists - let userObject = await this.getUserByGuid(userGuid); - - if (!userObject) { - // Found no existing user, add them - userObject = await this.addSystemUser(userGuid, userIdentifier, identitySource); - } - - // return the user object - return userObject; + return this.getUserById(existingUser.system_user_id); } /** diff --git a/api/src/utils/keycloak-utils.test.ts b/api/src/utils/keycloak-utils.test.ts index 393fbb9e9..1953fc692 100644 --- a/api/src/utils/keycloak-utils.test.ts +++ b/api/src/utils/keycloak-utils.test.ts @@ -18,9 +18,9 @@ describe('keycloakUtils', () => { }); it('returns their guid', () => { - const response = getUserGuid({ preferred_username: 'aaaaa@idir' }); + const response = getUserGuid({ preferred_username: '123-456-789@idir' }); - expect(response).to.equal('aaaaa'); + expect(response).to.equal('123-456-789'); }); }); @@ -32,19 +32,19 @@ describe('keycloakUtils', () => { }); it('returns null response when a keycloakToken is provided with a missing username field', () => { - const response = getUserIdentifier({ preferred_username: 'aaaaa@idir' }); + const response = getUserIdentifier({ preferred_username: '123-456-789@idir' }); expect(response).to.be.null; }); it('returns the identifier from their IDIR username', () => { - const response = getUserIdentifier({ preferred_username: 'aaaaa@idir', idir_username: 'idiruser' }); + const response = getUserIdentifier({ preferred_username: '123-456-789@idir', idir_username: 'idiruser' }); expect(response).to.equal('idiruser'); }); it('returns the identifier from their BCeID username', () => { - const response = getUserIdentifier({ preferred_username: 'aaaaa@idir', bceid_username: 'bceiduser' }); + const response = getUserIdentifier({ preferred_username: '123-456-789@idir', bceid_username: 'bceiduser' }); expect(response).to.equal('bceiduser'); }); diff --git a/api/src/utils/keycloak-utils.ts b/api/src/utils/keycloak-utils.ts index 8453c158b..05419a1fb 100644 --- a/api/src/utils/keycloak-utils.ts +++ b/api/src/utils/keycloak-utils.ts @@ -4,7 +4,7 @@ import { SOURCE_SYSTEM, SYSTEM_IDENTITY_SOURCE } from '../constants/database'; * Parses out the user's GUID from a keycloak token, which is extracted from the * `preferred_username` property. * - * @example getUserGuid({ preferred_username: 'aaabbaaa@idir' }) // => 'aaabbaaa' + * @example getUserGuid({ preferred_username: '123-456-789@idir' }) // => '123-456-789' * * @param {object} keycloakToken * @return {*} {(string | null)} @@ -25,7 +25,7 @@ export const getUserGuid = (keycloakToken: object): string | null => { * identity source is inferred from the `preferred_username` field as a contingency. * * @example getUserIdentitySource({ ...token, identity_provider: 'bceidbasic' }) => SYSTEM_IDENTITY_SOURCE.BCEID_BASIC - * @example getUserIdentitySource({ preferred_username: 'aaaa@idir' }) => SYSTEM_IDENTITY_SOURCE.IDIR + * @example getUserIdentitySource({ preferred_username: '123-456-789@idir' }) => SYSTEM_IDENTITY_SOURCE.IDIR * * @param {object} keycloakToken * @return {*} {SYSTEM_IDENTITY_SOURCE} diff --git a/app/package-lock.json b/app/package-lock.json index 68083ca84..cac022f28 100644 --- a/app/package-lock.json +++ b/app/package-lock.json @@ -13793,6 +13793,11 @@ "setimmediate": "^1.0.5" } }, + "jwt-decode": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/jwt-decode/-/jwt-decode-4.0.0.tgz", + "integrity": "sha512-+KJGIyHgkGuIq3IEBNftfhW/LfWhXUIY6OmyVWjliu5KH1y0fw7VQ8YndE2O4qZdMSd9SqbnC8GOcZEy0Om7sA==" + }, "keycloak-js": { "version": "20.0.5", "resolved": "https://registry.npmjs.org/keycloak-js/-/keycloak-js-20.0.5.tgz", @@ -14650,6 +14655,14 @@ "integrity": "sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg==", "dev": true }, + "oidc-client-ts": { + "version": "3.0.0-rc.0", + "resolved": "https://registry.npmjs.org/oidc-client-ts/-/oidc-client-ts-3.0.0-rc.0.tgz", + "integrity": "sha512-xJNUNrx+o21+IQjSw6rih3xM1B2wnILA/Sv6TEQrA+ghU4gNwpMHPnT4L2I/tH0KIIs86zca8/8wvoZ7HGOtjw==", + "requires": { + "jwt-decode": "^4.0.0" + } + }, "on-finished": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.3.0.tgz", @@ -16385,6 +16398,11 @@ "prop-types": "^15.7.2" } }, + "react-oidc-context": { + "version": "3.0.0-beta.0", + "resolved": "https://registry.npmjs.org/react-oidc-context/-/react-oidc-context-3.0.0-beta.0.tgz", + "integrity": "sha512-Y3WjJ+2qBpNqkX7DTnYc2WRLhXajX2tCv2S5rSB05J9IOjAOBYPXArIlHbjp6UWnSBW8rGbobmoRB9clrqIcWg==" + }, "react-refresh": { "version": "0.11.0", "resolved": "https://registry.npmjs.org/react-refresh/-/react-refresh-0.11.0.tgz", diff --git a/app/package.json b/app/package.json index 4152e9fb5..53420e121 100644 --- a/app/package.json +++ b/app/package.json @@ -63,6 +63,7 @@ "lodash-es": "~4.17.21", "moment": "~2.29.2", "node-sass": "~4.14.1", + "oidc-client-ts": "^3.0.0-rc.0", "pug": "^3.0.2", "qs": "~6.9.4", "react": "^17.0.2", @@ -70,6 +71,7 @@ "react-dropzone": "~11.3.2", "react-leaflet": "~4.2.1", "react-number-format": "~4.5.2", + "react-oidc-context": "^3.0.0-beta.0", "react-router": "^5.3.3", "react-router-dom": "^5.3.3", "react-window": "~1.8.6", diff --git a/app/src/App.tsx b/app/src/App.tsx index f31a86198..6be1c3372 100644 --- a/app/src/App.tsx +++ b/app/src/App.tsx @@ -1,12 +1,13 @@ import CircularProgress from '@mui/material/CircularProgress'; import { ThemeProvider } from '@mui/material/styles'; -import { ReactKeycloakProvider } from '@react-keycloak/web'; import AppRouter from 'AppRouter'; -import { AuthStateContextProvider } from 'contexts/authStateContext'; +import { AuthStateContext, AuthStateContextProvider } from 'contexts/authStateContext'; import { ConfigContext, ConfigContextProvider } from 'contexts/configContext'; -import Keycloak from 'keycloak-js'; +import { WebStorageStateStore } from 'oidc-client-ts'; +import { AuthProvider, AuthProviderProps } from 'react-oidc-context'; import { BrowserRouter } from 'react-router-dom'; import appTheme from 'themes/appTheme'; +import { buildUrl } from 'utils/Utils'; const App: React.FC = () => { return ( @@ -18,19 +19,44 @@ const App: React.FC = () => { return ; } - const keycloak = new Keycloak(config.KEYCLOAK_CONFIG); + const logoutRedirectUri = config.SITEMINDER_LOGOUT_URL + ? `${config.SITEMINDER_LOGOUT_URL}?returl=${window.location.origin}&retnow=1` + : buildUrl(window.location.origin); + + const authConfig: AuthProviderProps = { + authority: `${config.KEYCLOAK_CONFIG.authority}/realms/${config.KEYCLOAK_CONFIG.realm}/`, + client_id: config.KEYCLOAK_CONFIG.clientId, + resource: config.KEYCLOAK_CONFIG.clientId, + // Default sign in redirect + redirect_uri: buildUrl(window.location.origin), + // Default sign out redirect + post_logout_redirect_uri: logoutRedirectUri, + // Automatically load additional user profile information + loadUserInfo: true, + userStore: new WebStorageStateStore({ store: window.localStorage }), + onSigninCallback: (_): void => { + // See https://github.com/authts/react-oidc-context#getting-started + window.history.replaceState({}, document.title, window.location.pathname); + } + }; return ( - }> + - - - + + {(authState) => { + if (!authState) { + return ; + } + return ( + + + + ); + }} + - + ); }} diff --git a/app/src/AppRouter.tsx b/app/src/AppRouter.tsx index 3cfdf130d..1e6bac1f2 100644 --- a/app/src/AppRouter.tsx +++ b/app/src/AppRouter.tsx @@ -1,5 +1,3 @@ -import { SystemRoleGuard } from 'components/security/Guards'; -import { AuthenticatedRouteGuard, UnAuthenticatedRouteGuard } from 'components/security/RouteGuards'; import { SYSTEM_ROLE } from 'constants/roles'; import AccessDenied from 'features/403/AccessDenied'; import NotFoundPage from 'features/404/NotFoundPage'; @@ -8,9 +6,9 @@ import AdminDashboardRouter from 'features/admin/dashboard/AdminDashboardRouter' import DatasetsRouter from 'features/datasets/DatasetsRouter'; import SearchRouter from 'features/search/SearchRouter'; import SubmissionsRouter from 'features/submissions/SubmissionsRouter'; +import { SystemRoleGuard } from 'guards/Guards'; +import { AuthenticatedRouteGuard } from 'guards/RouteGuards'; import BaseLayout from 'layouts/BaseLayout'; -import LoginPage from 'pages/authentication/LoginPage'; -import LogOutPage from 'pages/authentication/LogOutPage'; import { Redirect, Route, Switch, useLocation } from 'react-router-dom'; import RouteWithTitle from 'utils/RouteWithTitle'; import { getTitle } from 'utils/Utils'; @@ -78,20 +76,6 @@ const AppRouter: React.FC = () => { - - - - - - - - - - - - - - diff --git a/app/src/components/layout/header/Header.test.tsx b/app/src/components/layout/header/Header.test.tsx index 1deaf2d60..f474de34f 100644 --- a/app/src/components/layout/header/Header.test.tsx +++ b/app/src/components/layout/header/Header.test.tsx @@ -1,22 +1,54 @@ +import { SYSTEM_IDENTITY_SOURCE } from 'constants/auth'; import { AuthStateContext } from 'contexts/authStateContext'; import { createMemoryHistory } from 'history'; -import { SYSTEM_IDENTITY_SOURCE } from 'hooks/useKeycloakWrapper'; import { Router } from 'react-router-dom'; import { getMockAuthState, SystemAdminAuthState, SystemUserAuthState } from 'test-helpers/auth-helpers'; -import { render } from 'test-helpers/test-utils'; +import { fireEvent, render, waitFor } from 'test-helpers/test-utils'; import Header from './Header'; const history = createMemoryHistory(); describe('Header', () => { - it('renders correctly with IDIR system admin role', () => { - const mockHasSystemRole = jest.fn(); + it('renders correctly with system admin role (IDIR)', () => { + const authState = getMockAuthState({ base: SystemAdminAuthState }); + + const { getByTestId } = render( + + +
+ + + ); - mockHasSystemRole.mockReturnValueOnce(true); // Return true when the `Manage Users` secure link is parsed + expect(getByTestId('submissions-header-item')).toBeVisible(); + expect(getByTestId('manage-users-header-item')).toBeVisible(); + }); - const authState = getMockAuthState({ base: SystemAdminAuthState }); + it('renders correctly with system admin role (BCeID Business)', () => { + const authState = getMockAuthState({ + base: SystemAdminAuthState, + overrides: { biohubUserWrapper: { identitySource: SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS } } + }); + + const { getByTestId } = render( + + +
+ + + ); + + expect(getByTestId('submissions-header-item')).toBeVisible(); + expect(getByTestId('manage-users-header-item')).toBeVisible(); + }); + + it('renders correctly with system admin role (BCeID Basic)', () => { + const authState = getMockAuthState({ + base: SystemAdminAuthState, + overrides: { biohubUserWrapper: { identitySource: SYSTEM_IDENTITY_SOURCE.BCEID_BASIC } } + }); - const { getByText } = render( + const { getByTestId } = render(
@@ -24,15 +56,14 @@ describe('Header', () => { ); - expect(getByText('Home')).toBeVisible(); - expect(getByText('Submissions')).toBeVisible(); - expect(getByText('Manage Users')).toBeVisible(); + expect(getByTestId('submissions-header-item')).toBeVisible(); + expect(getByTestId('manage-users-header-item')).toBeVisible(); }); it('renders the username and logout button', () => { const authState = getMockAuthState({ base: SystemAdminAuthState, - overrides: { keycloakWrapper: { getIdentitySource: () => SYSTEM_IDENTITY_SOURCE.BCEID_BUSINESS } } + overrides: { biohubUserWrapper: { identitySource: SYSTEM_IDENTITY_SOURCE.BCEID_BASIC } } }); const { getByTestId, getByText } = render( @@ -45,22 +76,66 @@ describe('Header', () => { expect(getByTestId('menu_log_out')).toBeVisible(); - expect(getByText('BCeID Business/testusername')).toBeVisible(); + expect(getByText('BCeID Basic/admin-username')).toBeVisible(); }); - describe('Log Out', () => { - it('redirects to the `/logout` page', async () => { - const authState = getMockAuthState({ base: SystemUserAuthState }); + describe('Log out', () => { + describe('expanded menu button', () => { + it('calls logout', async () => { + const signoutRedirectStub = jest.fn(); + + const authState = getMockAuthState({ + base: SystemUserAuthState, + overrides: { auth: { signoutRedirect: signoutRedirectStub } } + }); + + const { getByTestId } = render( + + +
+ + + ); + + const logoutButton = getByTestId('menu_log_out'); + + expect(logoutButton).toBeInTheDocument(); + + fireEvent.click(logoutButton); + + await waitFor(() => { + expect(signoutRedirectStub).toHaveBeenCalledTimes(1); + }); + }); + }); + + describe('collapsed menu button', () => { + it('calls logout', async () => { + const signoutRedirectStub = jest.fn(); + + const authState = getMockAuthState({ + base: SystemUserAuthState, + overrides: { auth: { signoutRedirect: signoutRedirectStub } } + }); + + const { getByTestId } = render( + + +
+ + + ); + + const logoutButton = getByTestId('collapsed_menu_log_out'); + + expect(logoutButton).toBeInTheDocument(); - const { getByTestId } = render( - - -
- - - ); + fireEvent.click(logoutButton); - expect(getByTestId('menu_log_out')).toHaveAttribute('href', '/logout'); + await waitFor(() => { + expect(signoutRedirectStub).toHaveBeenCalledTimes(1); + }); + }); }); }); }); diff --git a/app/src/components/layout/header/Header.tsx b/app/src/components/layout/header/Header.tsx index 9c0069ba6..39f5b8bbe 100644 --- a/app/src/components/layout/header/Header.tsx +++ b/app/src/components/layout/header/Header.tsx @@ -13,8 +13,8 @@ import Toolbar from '@mui/material/Toolbar'; import Typography from '@mui/material/Typography'; import headerImageLarge from 'assets/images/gov-bc-logo-horiz.png'; import headerImageSmall from 'assets/images/gov-bc-logo-vert.png'; -import { AuthGuard, SystemRoleGuard, UnAuthGuard } from 'components/security/Guards'; import { SYSTEM_ROLE } from 'constants/roles'; +import { AuthGuard, SystemRoleGuard, UnAuthGuard } from 'guards/Guards'; import { useState } from 'react'; import { Link as RouterLink } from 'react-router-dom'; import { LoggedInUser, PublicViewUser } from './UserControls'; @@ -158,7 +158,12 @@ const Header: React.FC = () => { fontWeight: 700 } }}> - + Home @@ -167,16 +172,26 @@ const Header: React.FC = () => { component={RouterLink} to="/admin/dashboard" id="menu_dashboard_sm" - onClick={hideMobileMenu}> + onClick={hideMobileMenu} + data-testid="collapsed_submissions-header-item"> Submissions - + Manage Users - + Support @@ -228,16 +243,16 @@ const Header: React.FC = () => { textTransform: 'none' } }}> - + Home - + Submissions - + Manage Users @@ -249,7 +264,8 @@ const Header: React.FC = () => { sx={{ m: '8px', p: 1 - }}> + }} + data-testid="support-header-item"> Support diff --git a/app/src/components/layout/header/UserControls.tsx b/app/src/components/layout/header/UserControls.tsx index dd94b53b0..1b7231e4c 100644 --- a/app/src/components/layout/header/UserControls.tsx +++ b/app/src/components/layout/header/UserControls.tsx @@ -4,17 +4,16 @@ import Box from '@mui/material/Box'; import Button from '@mui/material/Button'; import Divider from '@mui/material/Divider'; import MenuItem from '@mui/material/MenuItem'; +import { SYSTEM_IDENTITY_SOURCE } from 'constants/auth'; import { useAuthStateContext } from 'hooks/useAuthStateContext'; -import { SYSTEM_IDENTITY_SOURCE } from 'hooks/useKeycloakWrapper'; -import { useMemo } from 'react'; import { getFormattedIdentitySource } from 'utils/Utils'; // Authenticated view export const LoggedInUser = () => { - const { keycloakWrapper } = useAuthStateContext(); + const authStateContext = useAuthStateContext(); - const identitySource = keycloakWrapper?.getIdentitySource() || ''; - const userIdentifier = keycloakWrapper?.getUserIdentifier() || ''; + const identitySource = authStateContext.biohubUserWrapper.identitySource ?? ''; + const userIdentifier = authStateContext.biohubUserWrapper.userIdentifier ?? ''; const formattedUsername = [getFormattedIdentitySource(identitySource as SYSTEM_IDENTITY_SOURCE), userIdentifier] .filter(Boolean) .join('/'); @@ -49,7 +48,7 @@ export const LoggedInUser = () => {