From f8b37464ede9ba4375fce76733bab80d6421a778 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 18 Nov 2025 00:13:28 +0000 Subject: [PATCH 1/4] build(deps-dev): bump js-yaml from 3.14.1 to 3.14.2 Bumps [js-yaml](https://github.com/nodeca/js-yaml) from 3.14.1 to 3.14.2. - [Changelog](https://github.com/nodeca/js-yaml/blob/master/CHANGELOG.md) - [Commits](https://github.com/nodeca/js-yaml/compare/3.14.1...3.14.2) --- updated-dependencies: - dependency-name: js-yaml dependency-version: 3.14.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] --- package-lock.json | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index aee35af..f39edfd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -183,6 +183,7 @@ "resolved": "https://registry.npmjs.org/@babel/core/-/core-7.22.9.tgz", "integrity": "sha512-G2EgeufBcYw27U4hhoIwFcgc1XU7TlXJ3mv04oOv1WCuo900U/anZSPzEqNjwdjgffkk2Gs0AN0dW1CKVLcG7w==", "dev": true, + "peer": true, "dependencies": { "@ampproject/remapping": "^2.2.0", "@babel/code-frame": "^7.22.5", @@ -905,10 +906,11 @@ } }, "node_modules/@istanbuljs/load-nyc-config/node_modules/js-yaml": { - "version": "3.14.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.14.1.tgz", - "integrity": "sha512-okMH7OXXJ7YrN9Ok3/SXrnu4iX9yOk+25nqX4imS2npuvTYDmo/QEZoqwZkYaIDk3jVvBOTOIEgEhaLOynBS9g==", + "version": "3.14.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.14.2.tgz", + "integrity": "sha512-PMSmkqxr106Xa156c2M265Z+FTrPl+oxd/rgOQy2tijQeK5TxQ43psO1ZCwhVOSdnn+RzkzlRz/eY4BgJBYVpg==", "dev": true, + "license": "MIT", "dependencies": { "argparse": "^1.0.7", "esprima": "^4.0.0" @@ -1672,6 +1674,7 @@ "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-7.16.1.tgz", "integrity": "sha512-SxdPak/5bO0EnGktV05+Hq8oatjAYVY3Zh2bye9pGZy6+jwyR3LG3YKkV4YatlsgqXP28BTeVm9pqwJM96vf2A==", "dev": true, + "peer": true, "dependencies": { "@eslint-community/regexpp": "^4.10.0", "@typescript-eslint/scope-manager": "7.16.1", @@ -2026,6 +2029,7 @@ "version": "8.10.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.10.0.tgz", "integrity": "sha512-F0SAmZ8iUtS//m8DmCTA0jlh6TDKkHQyK6xc6V4KDTyZKA9dnvX9/3sRTVQrWm79glUAZbnmmNcdYwUIHWVybw==", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2516,6 +2520,7 @@ "url": "https://github.com/sponsors/ai" } ], + "peer": true, "dependencies": { "caniuse-lite": "^1.0.30001503", "electron-to-chromium": "^1.4.431", @@ -3482,6 +3487,7 @@ "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.57.1.tgz", "integrity": "sha512-ypowyDxpVSYpkXr9WPv2PAZCtNip1Mv5KTW0SCurXv/9iOpcrH9PaqUElksqEB6pChqHGDRCFTyrZlGhnLNGiA==", "dev": true, + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.6.1", @@ -3537,6 +3543,7 @@ "resolved": "https://registry.npmjs.org/eslint-config-prettier/-/eslint-config-prettier-9.1.0.tgz", "integrity": "sha512-NSWl5BFQWEPi1j4TjVNItzYV7dZXZ+wP6I6ZhrBGpChQhZRUaElihE9uRRkcbRnNb76UMKDF3r+WTmNcGPKsqw==", "dev": true, + "peer": true, "bin": { "eslint-config-prettier": "bin/cli.js" }, @@ -5164,6 +5171,7 @@ "resolved": "https://registry.npmjs.org/jest/-/jest-29.7.0.tgz", "integrity": "sha512-NIy3oAFp9shda19hy4HK0HRTWKtPJmGdnvywu01nOqNC2vZg+Z+fvJDxpMQA88eb2I9EcafcdjYgsDthnYTvGw==", "dev": true, + "peer": true, "dependencies": { "@jest/core": "^29.7.0", "@jest/types": "^29.6.3", @@ -5721,10 +5729,11 @@ "dev": true }, "node_modules/js-yaml": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.0.tgz", - "integrity": "sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==", + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.1.tgz", + "integrity": "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA==", "dev": true, + "license": "MIT", "dependencies": { "argparse": "^2.0.1" }, @@ -7268,6 +7277,7 @@ "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.3.3.tgz", "integrity": "sha512-i2tDNA0O5IrMO757lfrdQZCc2jPNDVntV0m/+4whiDfWaTKfMNgR7Qz0NAeGz/nRqF4m5/6CLzbP4/liHt12Ew==", "dev": true, + "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -9067,6 +9077,7 @@ "resolved": "https://registry.npmjs.org/@babel/core/-/core-7.22.9.tgz", "integrity": "sha512-G2EgeufBcYw27U4hhoIwFcgc1XU7TlXJ3mv04oOv1WCuo900U/anZSPzEqNjwdjgffkk2Gs0AN0dW1CKVLcG7w==", "dev": true, + "peer": true, "requires": { "@ampproject/remapping": "^2.2.0", "@babel/code-frame": "^7.22.5", @@ -9608,9 +9619,9 @@ } }, "js-yaml": { - "version": "3.14.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.14.1.tgz", - "integrity": "sha512-okMH7OXXJ7YrN9Ok3/SXrnu4iX9yOk+25nqX4imS2npuvTYDmo/QEZoqwZkYaIDk3jVvBOTOIEgEhaLOynBS9g==", + "version": "3.14.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-3.14.2.tgz", + "integrity": "sha512-PMSmkqxr106Xa156c2M265Z+FTrPl+oxd/rgOQy2tijQeK5TxQ43psO1ZCwhVOSdnn+RzkzlRz/eY4BgJBYVpg==", "dev": true, "requires": { "argparse": "^1.0.7", @@ -10264,6 +10275,7 @@ "resolved": "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-7.16.1.tgz", "integrity": "sha512-SxdPak/5bO0EnGktV05+Hq8oatjAYVY3Zh2bye9pGZy6+jwyR3LG3YKkV4YatlsgqXP28BTeVm9pqwJM96vf2A==", "dev": true, + "peer": true, "requires": { "@eslint-community/regexpp": "^4.10.0", "@typescript-eslint/scope-manager": "7.16.1", @@ -10474,7 +10486,8 @@ "acorn": { "version": "8.10.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.10.0.tgz", - "integrity": "sha512-F0SAmZ8iUtS//m8DmCTA0jlh6TDKkHQyK6xc6V4KDTyZKA9dnvX9/3sRTVQrWm79glUAZbnmmNcdYwUIHWVybw==" + "integrity": "sha512-F0SAmZ8iUtS//m8DmCTA0jlh6TDKkHQyK6xc6V4KDTyZKA9dnvX9/3sRTVQrWm79glUAZbnmmNcdYwUIHWVybw==", + "peer": true }, "acorn-jsx": { "version": "5.3.2", @@ -10833,6 +10846,7 @@ "resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.21.9.tgz", "integrity": "sha512-M0MFoZzbUrRU4KNfCrDLnvyE7gub+peetoTid3TBIqtunaDJyXlwhakT+/VkvSXcfIzFfK/nkCs4nmyTmxdNSg==", "dev": true, + "peer": true, "requires": { "caniuse-lite": "^1.0.30001503", "electron-to-chromium": "^1.4.431", @@ -11499,6 +11513,7 @@ "resolved": "https://registry.npmjs.org/eslint/-/eslint-8.57.1.tgz", "integrity": "sha512-ypowyDxpVSYpkXr9WPv2PAZCtNip1Mv5KTW0SCurXv/9iOpcrH9PaqUElksqEB6pChqHGDRCFTyrZlGhnLNGiA==", "dev": true, + "peer": true, "requires": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.6.1", @@ -11557,6 +11572,7 @@ "resolved": "https://registry.npmjs.org/eslint-config-prettier/-/eslint-config-prettier-9.1.0.tgz", "integrity": "sha512-NSWl5BFQWEPi1j4TjVNItzYV7dZXZ+wP6I6ZhrBGpChQhZRUaElihE9uRRkcbRnNb76UMKDF3r+WTmNcGPKsqw==", "dev": true, + "peer": true, "requires": {} }, "eslint-import-resolver-node": { @@ -12674,6 +12690,7 @@ "resolved": "https://registry.npmjs.org/jest/-/jest-29.7.0.tgz", "integrity": "sha512-NIy3oAFp9shda19hy4HK0HRTWKtPJmGdnvywu01nOqNC2vZg+Z+fvJDxpMQA88eb2I9EcafcdjYgsDthnYTvGw==", "dev": true, + "peer": true, "requires": { "@jest/core": "^29.7.0", "@jest/types": "^29.6.3", @@ -13102,9 +13119,9 @@ "dev": true }, "js-yaml": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.0.tgz", - "integrity": "sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==", + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.1.tgz", + "integrity": "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA==", "dev": true, "requires": { "argparse": "^2.0.1" @@ -14145,7 +14162,8 @@ "version": "3.3.3", "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.3.3.tgz", "integrity": "sha512-i2tDNA0O5IrMO757lfrdQZCc2jPNDVntV0m/+4whiDfWaTKfMNgR7Qz0NAeGz/nRqF4m5/6CLzbP4/liHt12Ew==", - "dev": true + "dev": true, + "peer": true }, "prettier-linter-helpers": { "version": "1.0.0", From 810ac4f9b2060f9dbcf1c8e29f5c20648899bb83 Mon Sep 17 00:00:00 2001 From: Jay Date: Thu, 4 Dec 2025 14:26:47 +0100 Subject: [PATCH 2/4] fix: update instrument name filter to use case-insensitive matching (#630) --- .../scicatProposal/consumerCallbacks/upsertProposalInScicat.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/upsertProposalInScicat.ts b/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/upsertProposalInScicat.ts index 8b47c59..5d3f300 100644 --- a/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/upsertProposalInScicat.ts +++ b/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/upsertProposalInScicat.ts @@ -201,7 +201,7 @@ const getInstrumentIds = async (instruments: Instrument[]) => { for (const name of instrumentNames) { const instrumentNameLowerCase = encodeURIComponent(name.toLowerCase()); - const url = `${sciCatBaseUrl}/Instruments?filter={"where":{"name":{"like":"${instrumentNameLowerCase}"}}}`; + const url = `${sciCatBaseUrl}/Instruments?filter={"where":{"name":{"ilike":"${instrumentNameLowerCase}"}}}`; try { const res = await request(url, { From 9cfa490e8a6d054f077b08226a8aa62e67ec7425 Mon Sep 17 00:00:00 2001 From: janosbabik <143906591+janosbabik@users.noreply.github.com> Date: Thu, 11 Dec 2025 16:32:53 +0100 Subject: [PATCH 3/4] feat: include data access users in One Identity sync (#631) --- .../OneIdentityIntegrationQueueConsumer.ts | 4 +- src/queue/consumers/oneidentity/README.md | 8 +-- ...osalAndMembersToOneIdentityHandler.spec.ts | 56 ++++++++++----- ...cProposalAndMembersToOneIdentityHandler.ts | 6 +- .../syncVisitToOneIdentityHandler.spec.ts | 8 +-- .../syncVisitToOneIdentityHandler.ts | 1 + .../collectUsersFromProposalMessage.spec.ts | 72 +++++++++++++++++++ .../utils/collectUsersFromProposalMessage.ts | 10 +-- 8 files changed, 133 insertions(+), 32 deletions(-) create mode 100644 src/queue/consumers/utils/collectUsersFromProposalMessage.spec.ts diff --git a/src/queue/consumers/oneidentity/OneIdentityIntegrationQueueConsumer.ts b/src/queue/consumers/oneidentity/OneIdentityIntegrationQueueConsumer.ts index 7f03eb8..0dfaacb 100644 --- a/src/queue/consumers/oneidentity/OneIdentityIntegrationQueueConsumer.ts +++ b/src/queue/consumers/oneidentity/OneIdentityIntegrationQueueConsumer.ts @@ -2,13 +2,13 @@ import { logger } from '@user-office-software/duo-logger'; import { ConsumerCallback } from '@user-office-software/duo-message-broker'; import { isAxiosError } from 'axios'; +import { QueueConsumer } from '../QueueConsumer'; import { syncProposalAndMembersToOneIdentityHandler } from './consumerCallbacks/syncProposalAndMembersToOneIdentityHandler'; import { syncVisitToOneIdentityHandler } from './consumerCallbacks/syncVisitToOneIdentityHandler'; +import { isVisitMessage } from './utils/isVisitMessage'; import { validateProposalMessage } from './utils/validateProposalMessage'; import { Event } from '../../../models/Event'; -import { QueueConsumer } from '../QueueConsumer'; import { hasTriggeringType } from '../utils/hasTriggeringType'; -import { isVisitMessage } from './utils/isVisitMessage'; const ONE_IDENTITY_INTEGRATION_QUEUE_NAME = process.env.ONE_IDENTITY_INTEGRATION_QUEUE_NAME || ''; diff --git a/src/queue/consumers/oneidentity/README.md b/src/queue/consumers/oneidentity/README.md index 3345b5f..48d8cf1 100644 --- a/src/queue/consumers/oneidentity/README.md +++ b/src/queue/consumers/oneidentity/README.md @@ -107,7 +107,7 @@ The handler manages site and system access in One Identity based on visit creati ## One Identity Proposal and Member Sync ### Purpose -The handler synchronizes proposal information and its members (proposer and co-proposers) with One Identity. This ensures that proposals and their associated personnel are accurately represented and connected in One Identity. +The handler synchronizes proposal information and its members (proposer, co-proposers and data access users) with One Identity. This ensures that proposals and their associated personnel are accurately represented and connected in One Identity. ### Process Overview - Triggered by `PROPOSAL_ACCEPTED` and `PROPOSAL_UPDATED` events. @@ -120,18 +120,18 @@ The handler synchronizes proposal information and its members (proposer and co-p - If `PROPOSAL_UPDATED` event: - If the proposal does not exist, the process logs this information and concludes, as there's no existing record to update. - **User Synchronization**: - - Collects all unique user OIDC sub identifiers from the proposal message (proposer and members). + - Collects all unique user OIDC sub identifiers from the proposal message (proposer, members and data access users). - Retrieves the corresponding `UID_Person` for these users from One Identity. - Logs an error if any users from the proposal message are not found in One Identity. - **Connection Management**: - Fetches all existing `PersonHasESET` connections for the identified proposal (`UID_ESet`). - **Remove Old Connections**: - - Identifies connections in One Identity for persons who are no longer part of the current proposal members list. + - Identifies connections in One Identity for persons who are no longer part of the current proposal members/dataAccessUsers list. - Before removing a connection, it checks if the person has "site access" to the proposal (e.g., as a visitor). - If the person has site access, their connection to the proposal is *not* removed. - Otherwise, the outdated connection is removed. - **Add New Connections**: - - Identifies persons in the current proposal members list who are not yet connected to the proposal in One Identity. + - Identifies persons in the current proposal members/dataAccessUsers list who are not yet connected to the proposal in One Identity. - Creates new `PersonHasESET` connections for these persons. - **Logout**: Ensures logout from One Identity in a `finally` block, regardless of success or failure. diff --git a/src/queue/consumers/oneidentity/consumerCallbacks/syncProposalAndMembersToOneIdentityHandler.spec.ts b/src/queue/consumers/oneidentity/consumerCallbacks/syncProposalAndMembersToOneIdentityHandler.spec.ts index ce45e90..7ba30e2 100644 --- a/src/queue/consumers/oneidentity/consumerCallbacks/syncProposalAndMembersToOneIdentityHandler.spec.ts +++ b/src/queue/consumers/oneidentity/consumerCallbacks/syncProposalAndMembersToOneIdentityHandler.spec.ts @@ -8,6 +8,7 @@ import { logger } from '@user-office-software/duo-logger'; import { syncProposalAndMembersToOneIdentityHandler } from './syncProposalAndMembersToOneIdentityHandler'; import { Event } from '../../../../models/Event'; import { ProposalMessageData } from '../../../../models/ProposalMessage'; +import { ProposalUser } from '../../scicat/scicatProposal/dto'; import { ESSOneIdentity } from '../utils/ESSOneIdentity'; import { UID_ESet } from '../utils/interfaces/Eset'; import { PersonHasESET } from '../utils/interfaces/PersonHasESET'; @@ -40,7 +41,7 @@ const setupMocks = (data: { data.getProposalPersonConnections ?? [] ); mockOneIdentity.getPersons.mockResolvedValueOnce( - data.getPersons ?? ['proposer-uid', 'member-uid'] + data.getPersons ?? ['proposer-uid', 'member-uid', 'data-access-uid'] ); if (data.hasPersonSiteAccessToProposalConfig) { mockOneIdentity.hasPersonSiteAccessToProposal.mockImplementation( @@ -57,6 +58,8 @@ const proposalMessage = { shortCode: 'shortCode', proposer: { oidcSub: 'proposer-oidc-sub' }, members: [{ oidcSub: 'member-oidc-sub' }], + dataAccessUsers: [{ oidcSub: 'data-access-oidc-sub' } as ProposalUser], + visitors: [] as ProposalUser[], } as ProposalMessageData; describe('oneIdentityIntegrationHandler', () => { @@ -81,7 +84,7 @@ describe('oneIdentityIntegrationHandler', () => { expect( mockOneIdentity.removeConnectionBetweenPersonAndProposal ).toHaveBeenCalledTimes(0); - expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes(2); + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes(3); expect(mockOneIdentity.connectPersonToProposal).toHaveBeenNthCalledWith( 1, 'proposal-UID_ESet', @@ -92,10 +95,15 @@ describe('oneIdentityIntegrationHandler', () => { 'proposal-UID_ESet', 'member-uid' ); + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenNthCalledWith( + 3, + 'proposal-UID_ESet', + 'data-access-uid' + ); expect(logger.logError).not.toHaveBeenCalled(); expect(logger.logInfo).toHaveBeenCalledWith('Connections updated', { uidESet: 'proposal-UID_ESet', - uidPersons: ['proposer-uid', 'member-uid'], + uidPersons: ['proposer-uid', 'member-uid', 'data-access-uid'], }); expect(mockOneIdentity.logout).toHaveBeenCalled(); }); @@ -104,7 +112,7 @@ describe('oneIdentityIntegrationHandler', () => { setupMocks({ getProposal: undefined, getProposalPersonConnections: [], - getPersons: ['proposer-uid'], + getPersons: ['proposer-oidc-sub'], }); await syncProposalAndMembersToOneIdentityHandler( @@ -115,8 +123,8 @@ describe('oneIdentityIntegrationHandler', () => { expect(logger.logError).toHaveBeenCalledWith( 'Not all users found in One Identity (Investigate). Missing central accounts:', { - centralAccounts: ['member-oidc-sub', 'proposer-oidc-sub'], - foundUsersInOneIdentity: ['proposer-uid'], + missingCentralAccounts: ['member-oidc-sub', 'data-access-oidc-sub'], + foundUsersInOneIdentity: ['proposer-oidc-sub'], } ); }); @@ -159,18 +167,22 @@ describe('oneIdentityIntegrationHandler', () => { expect(mockOneIdentity.createProposal).not.toHaveBeenCalled(); expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes( - 1 + 2 ); expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledWith( 'proposal-UID_ESet', 'member-uid' ); + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledWith( + 'proposal-UID_ESet', + 'data-access-uid' + ); expect( mockOneIdentity.removeConnectionBetweenPersonAndProposal ).not.toHaveBeenCalled(); expect(logger.logInfo).toHaveBeenCalledWith('Connections updated', { uidESet: 'proposal-UID_ESet', - uidPersons: ['proposer-uid', 'member-uid'], + uidPersons: ['proposer-uid', 'member-uid', 'data-access-uid'], }); expect(mockOneIdentity.logout).toHaveBeenCalled(); }); @@ -209,14 +221,18 @@ describe('oneIdentityIntegrationHandler', () => { expect( mockOneIdentity.removeConnectionBetweenPersonAndProposal ).toHaveBeenCalledWith('proposal-UID_ESet', 'old-member-uid'); - expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes(1); + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes(2); expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledWith( 'proposal-UID_ESet', 'member-uid' ); + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledWith( + 'proposal-UID_ESet', + 'data-access-uid' + ); expect(logger.logInfo).toHaveBeenCalledWith('Connections updated', { uidESet: 'proposal-UID_ESet', - uidPersons: ['proposer-uid', 'member-uid'], + uidPersons: ['proposer-uid', 'member-uid', 'data-access-uid'], }); expect(mockOneIdentity.logout).toHaveBeenCalled(); }); @@ -255,14 +271,18 @@ describe('oneIdentityIntegrationHandler', () => { mockOneIdentity.removeConnectionBetweenPersonAndProposal ).toHaveBeenCalledTimes(0); // No connections should be removed in this specific setup - expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes(1); // 'member-uid' is new + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes(2); expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledWith( 'proposal-UID_ESet', 'member-uid' ); + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledWith( + 'proposal-UID_ESet', + 'data-access-uid' + ); expect(logger.logInfo).toHaveBeenCalledWith('Connections updated', { uidESet: 'proposal-UID_ESet', - uidPersons: ['proposer-uid', 'member-uid'], + uidPersons: ['proposer-uid', 'member-uid', 'data-access-uid'], }); expect(mockOneIdentity.logout).toHaveBeenCalled(); }); @@ -284,7 +304,7 @@ describe('oneIdentityIntegrationHandler', () => { UID_Person: 'visitor-member-to-keep-uid', // Keep (not in proposal, but has site access) }, ], - getPersons: ['proposer-uid', 'member-uid'], // Current members in the proposal message + getPersons: ['proposer-uid', 'member-uid', 'data-access-uid'], // Current members in the proposal message hasPersonSiteAccessToProposalConfig: { 'old-member-to-remove-uid': false, 'visitor-member-to-keep-uid': true, @@ -292,7 +312,7 @@ describe('oneIdentityIntegrationHandler', () => { }); await syncProposalAndMembersToOneIdentityHandler( - proposalMessage, // Contains proposer-uid and member-uid + proposalMessage, Event.PROPOSAL_UPDATED ); @@ -317,15 +337,19 @@ describe('oneIdentityIntegrationHandler', () => { // Check additions // 'member-uid' is in proposalMessage.members and not in initial connections that are kept - expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes(1); + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledTimes(2); expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledWith( 'proposal-UID_ESet', 'member-uid' ); + expect(mockOneIdentity.connectPersonToProposal).toHaveBeenCalledWith( + 'proposal-UID_ESet', + 'data-access-uid' + ); expect(logger.logInfo).toHaveBeenCalledWith('Connections updated', { uidESet: 'proposal-UID_ESet', - uidPersons: ['proposer-uid', 'member-uid'], + uidPersons: ['proposer-uid', 'member-uid', 'data-access-uid'], }); expect(mockOneIdentity.logout).toHaveBeenCalled(); }); diff --git a/src/queue/consumers/oneidentity/consumerCallbacks/syncProposalAndMembersToOneIdentityHandler.ts b/src/queue/consumers/oneidentity/consumerCallbacks/syncProposalAndMembersToOneIdentityHandler.ts index 21d8495..481814b 100644 --- a/src/queue/consumers/oneidentity/consumerCallbacks/syncProposalAndMembersToOneIdentityHandler.ts +++ b/src/queue/consumers/oneidentity/consumerCallbacks/syncProposalAndMembersToOneIdentityHandler.ts @@ -80,10 +80,14 @@ async function handleConnectionsBetweenProposalAndPersons( // Log an error if not all users are found in One Identity to be able to investigate if (uidPersons.length !== centralAccounts.length) { + const missingCentralAccounts = centralAccounts.filter( + (account) => !uidPersons.includes(account) + ); + logger.logError( 'Not all users found in One Identity (Investigate). Missing central accounts:', { - centralAccounts, + missingCentralAccounts, foundUsersInOneIdentity: uidPersons, } ); diff --git a/src/queue/consumers/oneidentity/consumerCallbacks/syncVisitToOneIdentityHandler.spec.ts b/src/queue/consumers/oneidentity/consumerCallbacks/syncVisitToOneIdentityHandler.spec.ts index bcd45b9..ce60cc3 100644 --- a/src/queue/consumers/oneidentity/consumerCallbacks/syncVisitToOneIdentityHandler.spec.ts +++ b/src/queue/consumers/oneidentity/consumerCallbacks/syncVisitToOneIdentityHandler.spec.ts @@ -50,10 +50,8 @@ const visitMessage: VisitMessage = { endAt: '2023-01-10T00:00:00.000Z', proposal: { shortCode: 'proposal-short-code', - members: [ - { oidcSub: 'member-oidc-sub' }, - { oidcSub: 'visitor-oidc-sub' }, // Visitor is also a member - ], + members: [{ oidcSub: 'member-oidc-sub' }], + dataAccessUsers: [{ oidcSub: 'visitor-oidc-sub' }], // Visitor is also a data access user } as ProposalMessageData, }; @@ -61,7 +59,7 @@ const visitMessageVisitorNotMember: VisitMessage = { ...visitMessage, proposal: { ...visitMessage.proposal, - members: [{ oidcSub: 'member-oidc-sub' }], // Visitor is NOT a member + dataAccessUsers: [], // remove visitor from data access users } as ProposalMessageData, }; diff --git a/src/queue/consumers/oneidentity/consumerCallbacks/syncVisitToOneIdentityHandler.ts b/src/queue/consumers/oneidentity/consumerCallbacks/syncVisitToOneIdentityHandler.ts index 5479bc9..781f1d7 100644 --- a/src/queue/consumers/oneidentity/consumerCallbacks/syncVisitToOneIdentityHandler.ts +++ b/src/queue/consumers/oneidentity/consumerCallbacks/syncVisitToOneIdentityHandler.ts @@ -218,6 +218,7 @@ async function removeProposalConnection( ); if (isMember) { + // [proposer, members, dataAccessUsers] should always have access to the proposal logger.logInfo('Visitor is a proposal member, skipping removal', { uidPerson, uidESet, diff --git a/src/queue/consumers/utils/collectUsersFromProposalMessage.spec.ts b/src/queue/consumers/utils/collectUsersFromProposalMessage.spec.ts new file mode 100644 index 0000000..690c986 --- /dev/null +++ b/src/queue/consumers/utils/collectUsersFromProposalMessage.spec.ts @@ -0,0 +1,72 @@ +import { collectUsersFromProposalMessage } from './collectUsersFromProposalMessage'; +import { ProposalMessageData } from '../../../models/ProposalMessage'; +import { ProposalUser } from '../scicat/scicatProposal/dto'; + +describe('collectUsersFromProposalMessage', () => { + const createUser = (id: number, suffix: string): ProposalUser => ({ + id, + firstName: `first-${suffix}`, + lastName: `last-${suffix}`, + email: `${suffix}@example.com`, + oidcSub: `oidc-${suffix}`, + }); + + const createBaseMessage = ( + overrides: Partial = {} + ): ProposalMessageData => ({ + proposalPk: 1, + shortCode: 'short', + title: 'title', + abstract: 'abstract', + callId: 2, + submitted: true, + members: [], + ...overrides, + }); + + it('returns members, proposer, and data access users in order', () => { + const member = createUser(1, 'member'); + const proposer = createUser(2, 'proposer'); + const dataAccessUser = createUser(3, 'data'); + + const message = createBaseMessage({ + members: [member], + proposer, + dataAccessUsers: [dataAccessUser], + }); + + const result = collectUsersFromProposalMessage(message); + + expect(result).toEqual([member, proposer, dataAccessUser]); + }); + + it('filters out undefined entries', () => { + const member = createUser(1, 'member'); + + const message = createBaseMessage({ + members: [member], + proposer: undefined, + dataAccessUsers: [undefined as unknown as ProposalUser], + }); + + const result = collectUsersFromProposalMessage(message); + + expect(result).toEqual([member]); + }); + + it('handles missing dataAccessUsers by treating it as empty', () => { + const member = createUser(1, 'member'); + const proposer = createUser(2, 'proposer'); + + const message = { + ...createBaseMessage({ + members: [member], + proposer, + }), + } as ProposalMessageData; + + const result = collectUsersFromProposalMessage(message); + + expect(result).toEqual([member, proposer]); + }); +}); diff --git a/src/queue/consumers/utils/collectUsersFromProposalMessage.ts b/src/queue/consumers/utils/collectUsersFromProposalMessage.ts index a711dab..f2dfa3c 100644 --- a/src/queue/consumers/utils/collectUsersFromProposalMessage.ts +++ b/src/queue/consumers/utils/collectUsersFromProposalMessage.ts @@ -1,10 +1,12 @@ import { ProposalMessageData } from '../../../models/ProposalMessage'; import { ProposalUser } from '../scicat/scicatProposal/dto'; -export function collectUsersFromProposalMessage( - proposalMessage: ProposalMessageData -): ProposalUser[] { - return [...proposalMessage.members, proposalMessage.proposer].filter( +export function collectUsersFromProposalMessage({ + members, + proposer, + dataAccessUsers = [], +}: ProposalMessageData): ProposalUser[] { + return [...members, proposer, ...dataAccessUsers].filter( (user): user is ProposalUser => user !== undefined ); } From 34bee1f9d3e09414b1d3b380cbbc116bee680cc2 Mon Sep 17 00:00:00 2001 From: Jay Date: Thu, 22 Jan 2026 15:30:05 +0100 Subject: [PATCH 4/4] fix: update instrument ID fetching to use encoded filter string (#633) --- .../consumerCallbacks/proposalFoldersCreation.spec.ts | 2 +- .../consumerCallbacks/upsertProposalInScicat.ts | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/proposalFoldersCreation.spec.ts b/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/proposalFoldersCreation.spec.ts index 28ade26..8c5d6f4 100644 --- a/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/proposalFoldersCreation.spec.ts +++ b/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/proposalFoldersCreation.spec.ts @@ -85,7 +85,7 @@ describe('proposalFoldersCreation', () => { expect(exec).toHaveBeenCalledTimes(1); expect(exec).toHaveBeenCalledWith( - 'command shortcode 2025 shortCode group_prefix_shortCode test.proposer@email.com test.member@email.com', + 'command shortcode 2026 shortCode group_prefix_shortCode test.proposer@email.com test.member@email.com', expect.any(Function) ); }); diff --git a/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/upsertProposalInScicat.ts b/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/upsertProposalInScicat.ts index 5d3f300..0e1d5ad 100644 --- a/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/upsertProposalInScicat.ts +++ b/src/queue/consumers/scicat/scicatProposal/consumerCallbacks/upsertProposalInScicat.ts @@ -200,8 +200,13 @@ const getInstrumentIds = async (instruments: Instrument[]) => { const instrumentIds = []; for (const name of instrumentNames) { - const instrumentNameLowerCase = encodeURIComponent(name.toLowerCase()); - const url = `${sciCatBaseUrl}/Instruments?filter={"where":{"name":{"ilike":"${instrumentNameLowerCase}"}}}`; + const instrumentNameLowerCase = name.toLowerCase(); + + const filterString = JSON.stringify({ + where: { name: { ilike: instrumentNameLowerCase } }, + }); + + const url = `${sciCatBaseUrl}/Instruments?filter=${encodeURIComponent(filterString)}`; try { const res = await request(url, {