From 0ca001a2330e35226e339ae9ca68f92ccdaf61b8 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 10 Dec 2025 11:46:13 +0100 Subject: [PATCH 01/24] feat: refactor institution handling in user authorization and mutations --- apps/backend/src/auth/OAuthAuthorization.ts | 98 ++++++++++++------- .../backend/src/auth/StfcUserAuthorization.ts | 9 +- apps/backend/src/auth/UserAuthorization.ts | 9 +- .../src/auth/mockups/UserAuthorization.ts | 98 +++++++++++-------- .../src/mutations/UserMutations.spec.ts | 29 +++--- apps/backend/src/mutations/UserMutations.ts | 17 ++-- .../resolvers/mutations/UpsertUserMutation.ts | 49 ++++++++-- .../backend/src/utils/buildFederatedSchema.ts | 5 +- 8 files changed, 199 insertions(+), 115 deletions(-) diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index e643217b54..a7e502d338 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -1,27 +1,22 @@ -import 'reflect-metadata'; import { env } from 'process'; +import 'reflect-metadata'; import { logger } from '@user-office-software/duo-logger'; import { OpenIdClient } from '@user-office-software/openid'; import { ValidTokenSet } from '@user-office-software/openid/lib/model/ValidTokenSet'; import { ValidUserInfo } from '@user-office-software/openid/lib/model/ValidUserInfo'; import { GraphQLError } from 'graphql'; -import { UserinfoResponse } from 'openid-client'; import { container } from 'tsyringe'; import { Tokens } from '../config/Tokens'; import { AdminDataSource } from '../datasources/AdminDataSource'; +import { Institution } from '../models/Institution'; import { Rejection } from '../models/Rejection'; import { SettingsId } from '../models/Settings'; import { AuthJwtPayload, User, UserRole } from '../models/User'; +import { GetOrCreateInstitutionInput } from '../resolvers/mutations/UpsertUserMutation'; import { UserAuthorization } from './UserAuthorization'; -interface UserinfoResponseWithInstitution extends UserinfoResponse { - institution_ror_id?: string; - institution_name?: string; - institution_country?: string; -} - export class OAuthAuthorization extends UserAuthorization { private db = container.resolve(Tokens.AdminDataSource); @@ -85,38 +80,66 @@ export class OAuthAuthorization extends UserAuthorization { }); } - public async getOrCreateUserInstitution( - userInfo: UserinfoResponseWithInstitution - ) { - if (!userInfo.institution_name || !userInfo.institution_country) { - return null; + private async getOrCreateInstitutionByRorId( + rorId: string + ): Promise { + let institution = await this.adminDataSource.getInstitutionByRorId(rorId); + if (!institution) { + institution = await this.adminDataSource.createInstitution({ + name: 'New Institution', + country: 1, + rorId: rorId, + }); } - let institution = userInfo.institution_ror_id - ? await this.adminDataSource.getInstitutionByRorId( - userInfo.institution_ror_id - ) - : await this.adminDataSource.getInstitutionByName( - userInfo.institution_name - ); + return institution; + } + + private async getOrCreateInstitutionByManualInput(manualInput: { + name: string; + country: string; + }): Promise { + let institution = await this.adminDataSource.getInstitutionByName( + manualInput.name + ); if (!institution) { - let institutionCountry = await this.adminDataSource.getCountryByName( - userInfo.institution_country + let country = await this.adminDataSource.getCountryByName( + manualInput.country ); - - if (!institutionCountry) { - institutionCountry = await this.adminDataSource.createCountry( - userInfo.institution_country - ); + if (!country) { + // create country if it does not exist + country = await this.adminDataSource.createCountry(manualInput.country); } - const newInstitution = { - name: userInfo.institution_name, - country: institutionCountry.countryId, - rorId: userInfo.institution_ror_id, - }; - institution = - await this.adminDataSource.createInstitution(newInstitution); + institution = await this.adminDataSource.createInstitution({ + name: manualInput.name, + country: country.countryId, + rorId: undefined, + }); + } + + return institution; + } + + private async getOrCreateInstitutionById( + id: number + ): Promise { + const institution = await this.adminDataSource.getInstitution(id); + + return institution; + } + + public async getOrCreateUserInstitution(input: GetOrCreateInstitutionInput) { + let institution: Institution | null = null; + if (typeof input === 'string') { + // ROR ID provided + institution = await this.getOrCreateInstitutionByRorId(input); + } else if (input instanceof Object) { + // Manual institution details provided + institution = await this.getOrCreateInstitutionByManualInput(input); + } else if (typeof input === 'number') { + // Institution ID provided + institution = await this.getOrCreateInstitutionById(input); } return institution; @@ -127,7 +150,12 @@ export class OAuthAuthorization extends UserAuthorization { tokenSet: ValidTokenSet ): Promise { const client = await OpenIdClient.getInstance(); - const institution = await this.getOrCreateUserInstitution(userInfo); + const institution = await this.getOrCreateUserInstitution( + (userInfo.institution_ror_id as string) ?? { + country: userInfo.institution_country as string, + name: userInfo.institution_name as string, + } + ); const userWithOAuthSubMatch = await this.userDataSource.getByOIDCSub( userInfo.sub ); diff --git a/apps/backend/src/auth/StfcUserAuthorization.ts b/apps/backend/src/auth/StfcUserAuthorization.ts index 7675a94fb9..49ddb0eb92 100644 --- a/apps/backend/src/auth/StfcUserAuthorization.ts +++ b/apps/backend/src/auth/StfcUserAuthorization.ts @@ -22,6 +22,7 @@ import { Instrument } from '../models/Instrument'; import { Rejection, rejection } from '../models/Rejection'; import { Role, Roles } from '../models/Role'; import { AuthJwtPayload, User, UserWithRole } from '../models/User'; +import { GetOrCreateInstitutionInput } from '../resolvers/mutations/UpsertUserMutation'; import { Cache } from '../utils/Cache'; import { StfcUserDataSource } from './../datasources/stfc/StfcUserDataSource'; import { UserAuthorization } from './UserAuthorization'; @@ -349,11 +350,9 @@ export class StfcUserAuthorization extends UserAuthorization { return true; } - getOrCreateUserInstitution(userInfo: { - institution_ror_id?: string; - institution_name?: string; - institution_country?: string; - }): Promise { + getOrCreateUserInstitution( + _input: GetOrCreateInstitutionInput + ): Promise { throw new Error('Method not implemented.'); } } diff --git a/apps/backend/src/auth/UserAuthorization.ts b/apps/backend/src/auth/UserAuthorization.ts index e2bd4aaa0b..a08258a026 100644 --- a/apps/backend/src/auth/UserAuthorization.ts +++ b/apps/backend/src/auth/UserAuthorization.ts @@ -11,6 +11,7 @@ import { Institution } from '../models/Institution'; import { Rejection } from '../models/Rejection'; import { Role, Roles } from '../models/Role'; import { AuthJwtPayload, User, UserWithRole } from '../models/User'; +import { GetOrCreateInstitutionInput } from '../resolvers/mutations/UpsertUserMutation'; import { AdminDataSource } from './../datasources/AdminDataSource'; export abstract class UserAuthorization { @@ -192,11 +193,9 @@ export abstract class UserAuthorization { iss: string | null ): Promise; - abstract getOrCreateUserInstitution(userInfo: { - institution_ror_id?: string; - institution_name?: string; - institution_country?: string; - }): Promise; + abstract getOrCreateUserInstitution( + institution: GetOrCreateInstitutionInput + ): Promise; abstract logout(token: AuthJwtPayload): Promise; diff --git a/apps/backend/src/auth/mockups/UserAuthorization.ts b/apps/backend/src/auth/mockups/UserAuthorization.ts index c313816c73..d606aef297 100644 --- a/apps/backend/src/auth/mockups/UserAuthorization.ts +++ b/apps/backend/src/auth/mockups/UserAuthorization.ts @@ -6,6 +6,10 @@ import { dummyUser } from '../../datasources/mockups/UserDataSource'; import { Institution } from '../../models/Institution'; import { Role } from '../../models/Role'; import { AuthJwtPayload, User } from '../../models/User'; +import { + GetOrCreateInstitutionInput, + InstitutionManualInput, +} from '../../resolvers/mutations/UpsertUserMutation'; import { UserAuthorization } from '../UserAuthorization'; @injectable() @@ -23,56 +27,72 @@ export class UserAuthorizationMock extends UserAuthorization { new Institution(5, 'Mock Academic Center', 4, 'https://ror.org/dummy003'), ]; - async getOrCreateUserInstitution(userInfo: { - institution_ror_id?: string; - institution_name?: string; - institution_country?: string; - }): Promise { - // Return default institution if all fields are empty or unspecified - if ( - (!userInfo.institution_name || userInfo.institution_name.trim() === '') && - (!userInfo.institution_ror_id || - userInfo.institution_ror_id.trim() === '') - ) { - return this.mockInstitutions[0]; // Return dummyInstitution as default - } + private getNextInstitutionId(): number { + return ( + Math.max(...this.mockInstitutions.map((institution) => institution.id)) + + 1 + ); + } - // Try to find existing institution by ROR ID first (most reliable) - if (userInfo.institution_ror_id) { - const existingByRor = this.mockInstitutions.find( - (inst) => inst.rorId === userInfo.institution_ror_id - ); - if (existingByRor) { - return existingByRor; - } + async getOrCreateUserInstitution( + institutionInput: GetOrCreateInstitutionInput + ): Promise { + if (typeof institutionInput === 'string') { + // ROR ID provided + return this.getOrCreateInstitutionByRorId(institutionInput); + } else if (institutionInput instanceof InstitutionManualInput) { + // Manual institution details provided + return this.getOrCreateInstitutionByManualInput(institutionInput); + } else if (typeof institutionInput === 'number') { + // Institution ID provided + return this.getOrCreateInstitutionById(institutionInput); } - // Try to find existing institution by name (case-insensitive) - if (userInfo.institution_name) { - const existingByName = this.mockInstitutions.find( - (inst) => - inst.name.toLowerCase() === userInfo.institution_name?.toLowerCase() + return this.mockInstitutions[0]; + } + + private getOrCreateInstitutionByRorId(rorId: string): Institution { + let institution = this.mockInstitutions.find( + (inst) => inst.rorId === rorId + ); + if (!institution) { + institution = new Institution( + this.getNextInstitutionId(), + 'New Institution', + 1, + rorId ); - if (existingByName) { - return existingByName; - } + this.mockInstitutions.push(institution); } - // Create new institution if not found - const newId = Math.max(...this.mockInstitutions.map((i) => i.id)) + 1; + return institution; + } - const newInstitution = new Institution( - newId, - userInfo.institution_name ?? 'Unknown Institution', - 1, - userInfo.institution_ror_id + private getOrCreateInstitutionByManualInput( + manualInput: InstitutionManualInput + ): Institution { + let institution = this.mockInstitutions.find( + (inst) => inst.name.toLowerCase() === manualInput.name.toLowerCase() ); + if (!institution) { + institution = new Institution( + this.getNextInstitutionId(), + manualInput.name, + 1, + undefined + ); + this.mockInstitutions.push(institution); + } - // Add to mock data for subsequent calls - this.mockInstitutions.push(newInstitution); + return institution; + } + + private getOrCreateInstitutionById(id: number): Institution { + const institution = this.mockInstitutions.find((inst) => inst.id === id); - return newInstitution; + return institution || this.mockInstitutions[0]; } + async externalTokenLogin( token: string, _redirectUri: string diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index f4b13ad824..2a25037fd3 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -382,12 +382,14 @@ describe('upsertUserByOidcSub', () => { preferredName: null, gender: null, birthDate: null, - institutionRoRId: '', + institution: { + rorId: 'dummy-ror-id', + manual: null, + institutionId: null, + }, department: null, position: '', telephone: null, - institutionName: '', - institutionCountry: '', } ); // Check if the result has the oidcsub @@ -407,12 +409,13 @@ describe('upsertUserByOidcSub', () => { preferredName: null, gender: null, birthDate: null, - institutionRoRId: '', + institution: { + rorId: '', + manual: null, + }, department: null, position: '', telephone: null, - institutionName: '', - institutionCountry: '', } ); @@ -437,9 +440,10 @@ describe('upsertUserByOidcSub', () => { preferredName: 'Johnny', gender: 'male', birthDate: '1985-05-15', - institutionRoRId: existingRorId, // This should find Dummy Research Institute in our mock - institutionName: 'CERN', // This should match the existing institution - institutionCountry: 'Switzerland', + institution: { + rorId: existingRorId, // This should find Dummy Research Institute in our mock + manual: null, + }, department: 'Physics Department', position: 'Senior Researcher', telephone: '+41-22-767-6111', @@ -473,9 +477,10 @@ describe('upsertUserByOidcSub', () => { preferredName: 'Maria', gender: 'female', birthDate: '1980-03-22', - institutionRoRId: newRorId, // This ROR ID doesn't exist in mock - institutionName: 'New Research Institute', - institutionCountry: 'Germany', + institution: { + rorId: newRorId, // This ROR ID doesn't exist in mock + manual: null, + }, department: 'Materials Science', position: 'Principal Investigator', telephone: '+49-30-12345678', diff --git a/apps/backend/src/mutations/UserMutations.ts b/apps/backend/src/mutations/UserMutations.ts index 6a7a0604d1..755860b6d0 100644 --- a/apps/backend/src/mutations/UserMutations.ts +++ b/apps/backend/src/mutations/UserMutations.ts @@ -526,9 +526,7 @@ export default class UserMutations { oidcSub, gender, birthDate, - institutionRoRId, - institutionName, - institutionCountry, + institution: institutionInput, department, position, email, @@ -543,15 +541,16 @@ export default class UserMutations { return rejection('Invalid birth date format', { birthDate, args }); } - const institution = await this.userAuth.getOrCreateUserInstitution({ - institution_ror_id: institutionRoRId, - institution_name: institutionName, - institution_country: institutionCountry, - }); + // due to Graphql @oneOf directive institutionInput can be of mutually exclusive three types: rorId (string), manual (InstitutionManualInput), institutionId (number) + const institution = await this.userAuth.getOrCreateUserInstitution( + institutionInput.institutionId ?? + institutionInput.manual ?? + institutionInput.rorId + ); if (!institution) { return rejection('Invalid Input for the Institution', { - institutionRoRId, + institutionInput, args, }); } diff --git a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts index 9ddab35e3c..a3e957e46f 100644 --- a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts +++ b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts @@ -1,7 +1,44 @@ -import { Args, ArgsType, Ctx, Field, Mutation, Resolver } from 'type-graphql'; +import { + Args, + ArgsType, + Ctx, + Directive, + Field, + InputType, + Mutation, + Resolver, +} from 'type-graphql'; import { ResolverContext } from '../../context'; import { User } from '../types/User'; + +@InputType() +export class InstitutionManualInput { + @Field(() => String) + public name: string; + + @Field(() => String) + public country: string; +} + +@InputType() +@Directive('@oneOf') +export class InstitutionInput { + @Field(() => String, { nullable: true }) + public rorId: string | null; + + @Field(() => InstitutionManualInput, { nullable: true }) + public manual: InstitutionManualInput | null; + + @Field(() => Number, { nullable: true }) + public institutionId: number | null; +} + +export type GetOrCreateInstitutionInput = + | InstitutionInput['rorId'] + | InstitutionInput['manual'] + | InstitutionInput['institutionId']; + @ArgsType() export class UpsertUserByOidcSubArgs { @Field(() => String, { nullable: true }) @@ -28,14 +65,8 @@ export class UpsertUserByOidcSubArgs { @Field(() => String, { nullable: true }) public birthDate: string | null; - @Field(() => String) - public institutionRoRId: string; - - @Field(() => String) - public institutionName: string; - - @Field(() => String) - public institutionCountry: string; + @Field(() => InstitutionInput) + public institution: InstitutionInput; @Field(() => String, { nullable: true }) public department: string | null; diff --git a/apps/backend/src/utils/buildFederatedSchema.ts b/apps/backend/src/utils/buildFederatedSchema.ts index c3eba9f8aa..a16402dfb9 100644 --- a/apps/backend/src/utils/buildFederatedSchema.ts +++ b/apps/backend/src/utils/buildFederatedSchema.ts @@ -23,7 +23,10 @@ export async function buildFederatedSchema( }); const federatedSchema = buildSubgraphSchema({ - typeDefs: gql(printSchemaWithDirectives(schema)), + typeDefs: gql(` + directive @oneOf on INPUT_OBJECT + ${printSchemaWithDirectives(schema)} + `), // merge schema's resolvers with reference resolvers resolvers: deepMerge( createResolversMap(schema) as GraphQLResolverMap, From cb446e0f676f7a9085999f640274011ce7a12f62 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 10 Dec 2025 13:00:17 +0100 Subject: [PATCH 02/24] fix: update upsertUserByOidcSub to handle institution input more flexibly --- apps/backend/src/auth/OAuthAuthorization.ts | 13 +++++++++---- apps/backend/src/mutations/UserMutations.spec.ts | 3 +++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index a7e502d338..b56ab5cdef 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -150,12 +150,17 @@ export class OAuthAuthorization extends UserAuthorization { tokenSet: ValidTokenSet ): Promise { const client = await OpenIdClient.getInstance(); - const institution = await this.getOrCreateUserInstitution( - (userInfo.institution_ror_id as string) ?? { + let institutionInput: GetOrCreateInstitutionInput = null; + if (userInfo.institution_ror_id) { + institutionInput = userInfo.institution_ror_id as string; + } else if (userInfo.institution_name && userInfo.institution_country) { + institutionInput = { country: userInfo.institution_country as string, name: userInfo.institution_name as string, - } - ); + }; + } + + const institution = await this.getOrCreateUserInstitution(institutionInput); const userWithOAuthSubMatch = await this.userDataSource.getByOIDCSub( userInfo.sub ); diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index 2a25037fd3..7544cbfa6d 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -412,6 +412,7 @@ describe('upsertUserByOidcSub', () => { institution: { rorId: '', manual: null, + institutionId: null, }, department: null, position: '', @@ -443,6 +444,7 @@ describe('upsertUserByOidcSub', () => { institution: { rorId: existingRorId, // This should find Dummy Research Institute in our mock manual: null, + institutionId: null, }, department: 'Physics Department', position: 'Senior Researcher', @@ -480,6 +482,7 @@ describe('upsertUserByOidcSub', () => { institution: { rorId: newRorId, // This ROR ID doesn't exist in mock manual: null, + institutionId: null, }, department: 'Materials Science', position: 'Principal Investigator', From 4321cfc585bf5afa237d7bd0aa7780d7d0411122 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 10 Dec 2025 13:57:22 +0100 Subject: [PATCH 03/24] fix: update user mutations tests and correct institutionId field type --- apps/backend/src/mutations/UserMutations.spec.ts | 6 +++++- .../backend/src/resolvers/mutations/UpsertUserMutation.ts | 3 ++- .../frontend/src/graphql/user/upsertUserByOidcSub.graphql | 8 ++------ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index 7544cbfa6d..0ad620fa7a 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -37,7 +37,11 @@ const badToken = jsonwebtoken.sign( { expiresIn: '-24h' } ); -const userMutations = container.resolve(UserMutations); +let userMutations: UserMutations; + +beforeEach(() => { + userMutations = container.resolve(UserMutations); +}); test('A user can invite another user by email', () => { const emailInviteResponse = new EmailInviteResponse( diff --git a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts index a3e957e46f..6f74ce793d 100644 --- a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts +++ b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts @@ -5,6 +5,7 @@ import { Directive, Field, InputType, + Int, Mutation, Resolver, } from 'type-graphql'; @@ -30,7 +31,7 @@ export class InstitutionInput { @Field(() => InstitutionManualInput, { nullable: true }) public manual: InstitutionManualInput | null; - @Field(() => Number, { nullable: true }) + @Field(() => Int, { nullable: true }) public institutionId: number | null; } diff --git a/apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql b/apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql index ef0aeb201d..f1dbd30058 100644 --- a/apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql +++ b/apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql @@ -7,9 +7,7 @@ mutation upsertUserByOidcSub( $oidcSub: String! $gender: String $birthDate: String - $institutionRoRId: String! - $institutionName: String! - $institutionCountry: String! + $institution: InstitutionInput! $department: String $position: String! $email: String! @@ -24,9 +22,7 @@ mutation upsertUserByOidcSub( oidcSub: $oidcSub gender: $gender birthDate: $birthDate - institutionRoRId: $institutionRoRId - institutionName: $institutionName - institutionCountry: $institutionCountry + institution: $institution department: $department position: $position email: $email From 7a75b718553063219bdad9776a94daf7a2dec937 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 10 Dec 2025 15:35:14 +0100 Subject: [PATCH 04/24] fix: enhance user mutations tests by mocking UserAuthorization and logging created institution ID --- apps/backend/src/mutations/UserMutations.spec.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index 0ad620fa7a..7175c19992 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -1,14 +1,16 @@ import jsonwebtoken from 'jsonwebtoken'; -import { container } from 'tsyringe'; +import { container, Lifecycle } from 'tsyringe'; +import { UserAuthorizationMock } from '../auth/mockups/UserAuthorization'; +import { Tokens } from '../config/Tokens'; import { dummyPlaceHolderUser, dummyUser, dummyUserNotOnProposal, - dummyUserOfficer, - dummyUserWithRole, dummyUserNotOnProposalWithRole, + dummyUserOfficer, dummyUserOfficerWithRole, + dummyUserWithRole, } from '../datasources/mockups/UserDataSource'; import { EmailInviteResponse } from '../models/EmailInviteResponse'; import { isRejection, Rejection } from '../models/Rejection'; @@ -40,6 +42,11 @@ const badToken = jsonwebtoken.sign( let userMutations: UserMutations; beforeEach(() => { + container.register( + Tokens.UserAuthorization, + { useClass: UserAuthorizationMock }, + { lifecycle: Lifecycle.Singleton } + ); userMutations = container.resolve(UserMutations); }); @@ -499,6 +506,7 @@ describe('upsertUserByOidcSub', () => { // Should create new institution and assign it // The new institution should have ID 6 (next available in our mock) + console.log('Created User Institution ID:', createdUser.institutionId); expect(createdUser.institutionId).toBe(6); expect(createdUser.firstname).toBe('Maria'); expect(createdUser.lastname).toBe('Researcher'); From 79d7b450546500a457da25502d9ab6c9c3dd173a Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 10 Dec 2025 15:41:36 +0100 Subject: [PATCH 05/24] fix: remove console log for created user institution ID in upsertUserByOidcSub tests --- apps/backend/src/mutations/UserMutations.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index 7175c19992..0342cca78a 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -506,7 +506,6 @@ describe('upsertUserByOidcSub', () => { // Should create new institution and assign it // The new institution should have ID 6 (next available in our mock) - console.log('Created User Institution ID:', createdUser.institutionId); expect(createdUser.institutionId).toBe(6); expect(createdUser.firstname).toBe('Maria'); expect(createdUser.lastname).toBe('Researcher'); From f018c44378ec6b5f64130c0f0eb315b01c382bdd Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 10 Dec 2025 20:22:11 +0100 Subject: [PATCH 06/24] fix: update institution creation to allow null country and handle rorId appropriately --- apps/backend/src/auth/OAuthAuthorization.spec.ts | 15 ++++++++------- apps/backend/src/auth/OAuthAuthorization.ts | 4 ++-- apps/backend/src/models/Institution.ts | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/apps/backend/src/auth/OAuthAuthorization.spec.ts b/apps/backend/src/auth/OAuthAuthorization.spec.ts index 85a4107f5a..68746a5313 100644 --- a/apps/backend/src/auth/OAuthAuthorization.spec.ts +++ b/apps/backend/src/auth/OAuthAuthorization.spec.ts @@ -41,9 +41,9 @@ describe('OAuthAuthorization', () => { country = 'testCountry', }: { user: User; - rorId?: string; - name?: string; - country?: string; + rorId?: string | null; + name?: string | null; + country?: string | null; }) => (mockOpenIdClient.login = jest.fn().mockResolvedValue({ userProfile: { @@ -128,8 +128,8 @@ describe('OAuthAuthorization', () => { expect(mockAdminDataSource.createCountry).not.toHaveBeenCalled(); expect(mockAdminDataSource.createInstitution).toHaveBeenCalledWith({ - name: 'testName', - country: testCountry.countryId, + name: 'Unknown institution', + country: null, rorId: 'testRorId', }); expect( @@ -161,7 +161,8 @@ describe('OAuthAuthorization', () => { .spyOn(mockAdminDataSource, 'createInstitution') .mockResolvedValue({ id: expectedInstitutionId } as Institution); - mockOpenIdLoginResponse({ user: dummyUser }); + // make dummyUser without ror_id + mockOpenIdLoginResponse({ user: dummyUser, rorId: null }); await oauthAuthorization.externalTokenLogin('valid', '', null); expect(mockAdminDataSource.createCountry).toHaveBeenCalledWith( @@ -170,7 +171,7 @@ describe('OAuthAuthorization', () => { expect(mockAdminDataSource.createInstitution).toHaveBeenCalledWith({ name: 'testName', country: testCountry.countryId, - rorId: 'testRorId', + rorId: undefined, }); expect( diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index b56ab5cdef..87df195471 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -86,8 +86,8 @@ export class OAuthAuthorization extends UserAuthorization { let institution = await this.adminDataSource.getInstitutionByRorId(rorId); if (!institution) { institution = await this.adminDataSource.createInstitution({ - name: 'New Institution', - country: 1, + name: 'Unknown institution', + country: null, rorId: rorId, }); } diff --git a/apps/backend/src/models/Institution.ts b/apps/backend/src/models/Institution.ts index 0e02d22e3b..83f86ff2db 100644 --- a/apps/backend/src/models/Institution.ts +++ b/apps/backend/src/models/Institution.ts @@ -2,7 +2,7 @@ export class Institution { constructor( public id: number, public name: string, - public country: number, + public country: number | null, public rorId?: string ) {} } From 2e24f7d7942688644d210e50d4d9c693420e435d Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 10 Dec 2025 20:24:04 +0100 Subject: [PATCH 07/24] fix: institutiuons --- .../src/resolvers/mutations/UpdateInstitutionsMutation.ts | 4 ++-- apps/backend/src/resolvers/types/Institution.ts | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts b/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts index 2594784d63..98953f621a 100644 --- a/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts +++ b/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts @@ -19,8 +19,8 @@ export class UpdateInstitutionsArgs { @Field(() => String, { nullable: true }) name: string; - @Field(() => Int) - country: number; + @Field(() => Int, { nullable: true }) + country: number | null; @Field(() => String, { nullable: true }) rorId?: string; diff --git a/apps/backend/src/resolvers/types/Institution.ts b/apps/backend/src/resolvers/types/Institution.ts index d39d108c12..e1354817ea 100644 --- a/apps/backend/src/resolvers/types/Institution.ts +++ b/apps/backend/src/resolvers/types/Institution.ts @@ -31,6 +31,10 @@ export class InstitutionResolver { @Root() institution: InstitutionOrigin, @Ctx() context: ResolverContext ): Promise { + if (institution.country === null) { + return null; + } + return context.queries.admin.getCountry(institution.country); } } From 8ad5e9c6544229cd84aa300b1cdd8437f616fff4 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Thu, 11 Dec 2025 10:30:07 +0100 Subject: [PATCH 08/24] fix: remove outdated comment regarding institutionInput types in UserMutations --- apps/backend/src/mutations/UserMutations.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/backend/src/mutations/UserMutations.ts b/apps/backend/src/mutations/UserMutations.ts index 755860b6d0..15b1168d8d 100644 --- a/apps/backend/src/mutations/UserMutations.ts +++ b/apps/backend/src/mutations/UserMutations.ts @@ -541,7 +541,6 @@ export default class UserMutations { return rejection('Invalid birth date format', { birthDate, args }); } - // due to Graphql @oneOf directive institutionInput can be of mutually exclusive three types: rorId (string), manual (InstitutionManualInput), institutionId (number) const institution = await this.userAuth.getOrCreateUserInstitution( institutionInput.institutionId ?? institutionInput.manual ?? From ca25c91e21df3da9abf9362f47cd627454b3d5b7 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Mon, 15 Dec 2025 15:13:04 +0100 Subject: [PATCH 09/24] fix: enhance error handling in upsertUserByOidc and add Cypress command for user upsert --- .../datasources/postgres/UserDataSource.ts | 4 +- apps/e2e/cypress/e2e/login.cy.ts | 79 ++++++++----------- apps/e2e/cypress/support/user.ts | 17 ++++ apps/e2e/cypress/types/user.d.ts | 13 +++ 4 files changed, 68 insertions(+), 45 deletions(-) diff --git a/apps/backend/src/datasources/postgres/UserDataSource.ts b/apps/backend/src/datasources/postgres/UserDataSource.ts index 57bab767d8..252245ddba 100644 --- a/apps/backend/src/datasources/postgres/UserDataSource.ts +++ b/apps/backend/src/datasources/postgres/UserDataSource.ts @@ -426,7 +426,9 @@ export default class PostgresUserDataSource implements UserDataSource { }) .catch((error) => { if (isUniqueConstraintError(error)) { - throw new GraphQLError('User already exists'); + throw new GraphQLError('User already exists', { + originalError: error, + }); } throw new GraphQLError('Could not update user. Check your Inputs.'); }); diff --git a/apps/e2e/cypress/e2e/login.cy.ts b/apps/e2e/cypress/e2e/login.cy.ts index 6e74ab69d8..3ec182d3ef 100644 --- a/apps/e2e/cypress/e2e/login.cy.ts +++ b/apps/e2e/cypress/e2e/login.cy.ts @@ -4,20 +4,6 @@ import featureFlags from '../support/featureFlags'; import initialDBData from '../support/initialDBData'; context('User login tests', () => { - const userOne = 'user1'; - const userTwo = 'user2'; - const userOneInstitutionInfo = { - institution_ror_id: 'Test_ror_id_1', - institution_country: 'China', - institution_name: 'Test Institution', - }; - - const userTwoInstitutionInfo = { - institution_ror_id: 'Test_ror_id_2', - institution_country: 'TestCountry', - institution_name: 'Test Institution2', - }; - beforeEach(() => { cy.resetDB(); cy.getAndStoreFeaturesEnabled(); @@ -31,32 +17,28 @@ context('User login tests', () => { this.skip(); } - cy.login(userOne, initialDBData.roles.user); - cy.visit('/'); - cy.logout(); + const institutionRorId = 'https://ror.org/not-in-db'; + + cy.upsertUserByOidc({ + oidcSub: 'some-unique-sub', + email: 'email@example.com', + institution: { + rorId: institutionRorId, + }, + firstName: 'Test', + lastName: 'User', + position: 'Researcher', + username: 'testuser', + }); - cy.login('officer'); cy.contains('Institutions').click(); cy.finishedLoading(); - cy.get('input[aria-label="Search"]') - .focus() - .type(userOneInstitutionInfo.institution_name); + cy.get('input[aria-label="Search"]').focus().type(institutionRorId); cy.get('[data-cy="institutions-table"]').as('institutionsTable'); - cy.get('@institutionsTable') - .contains(userOneInstitutionInfo.institution_name) - .parents('tr') - .within(() => { - cy.get('td:nth-child(4)') - .invoke('text') - .then((text) => { - expect(text.trim()).to.eq( - userOneInstitutionInfo.institution_ror_id - ); - }); - }); + cy.get('@institutionsTable').contains(institutionRorId); }); it('Should create new entry for country, if the given country is not found', function () { @@ -64,30 +46,39 @@ context('User login tests', () => { this.skip(); } - cy.login(userTwo, initialDBData.roles.user); - cy.visit('/'); - cy.logout(); + const institutionCountry = 'NewCountry'; + const institutionName = 'Test Institution NewCountry'; + + cy.upsertUserByOidc({ + oidcSub: 'some-unique-sub', + email: 'email@example.com', + firstName: 'Test', + lastName: 'User', + position: 'Researcher', + institution: { + manual: { + name: institutionName, + country: institutionCountry, + }, + }, + username: 'testuser', + }); - cy.login('officer'); cy.contains('Institutions').click(); cy.finishedLoading(); - cy.get('input[aria-label="Search"]') - .focus() - .type(userOneInstitutionInfo.institution_name); + cy.get('input[aria-label="Search"]').focus().type(institutionName); cy.get('[data-cy="institutions-table"]').as('institutionsTable'); cy.get('@institutionsTable') - .contains(userTwoInstitutionInfo.institution_name) + .contains(institutionName) .parents('tr') .within(() => { cy.get('td:nth-child(3)') .invoke('text') .then((text) => { - expect(text.trim()).to.eq( - userTwoInstitutionInfo.institution_country - ); + expect(text.trim()).to.eq(institutionCountry); }); }); }); diff --git a/apps/e2e/cypress/support/user.ts b/apps/e2e/cypress/support/user.ts index 583c884694..e0d6042ee8 100644 --- a/apps/e2e/cypress/support/user.ts +++ b/apps/e2e/cypress/support/user.ts @@ -8,6 +8,7 @@ import { UpdateUserMutation, UpdateUserMutationVariables, UpdateUserRolesMutationVariables, + UpsertUserByOidcSubMutationVariables, User, } from '@user-office-software-libs/shared-types'; import { jwtDecode } from 'jwt-decode'; @@ -81,6 +82,21 @@ function changeActiveRole(selectedRoleId: number) { cy.wrap(request); } +function upsertUserByOidc(args: UpsertUserByOidcSubMutationVariables) { + const token = window.localStorage.getItem('token'); + + if (!token) { + throw new Error('No logged in user'); + } + + const api = getE2EApi(); + const request = api.upsertUserByOidcSub(args).then((resp) => { + return resp; + }); + + cy.wrap(request); +} + const getUserIdFromIdOrCredentials = ( idOrCredentials: TestUserId | { email: string; password: string } ) => { @@ -249,3 +265,4 @@ Cypress.Commands.add('updateUserDetails', updateUserDetails); Cypress.Commands.add('changeActiveRole', changeActiveRole); Cypress.Commands.add('getAndStoreFeaturesEnabled', getAndStoreFeaturesEnabled); +Cypress.Commands.add('upsertUserByOidc', upsertUserByOidc); diff --git a/apps/e2e/cypress/types/user.d.ts b/apps/e2e/cypress/types/user.d.ts index bcab0a3d7b..690f83b678 100644 --- a/apps/e2e/cypress/types/user.d.ts +++ b/apps/e2e/cypress/types/user.d.ts @@ -5,6 +5,7 @@ import { UpdateUserMutation, CreateUserByEmailInviteMutationVariables, CreateUserByEmailInviteMutation, + UpsertUserByOidcSubMutationVariables, } from '@user-office-software-libs/shared-types'; import { TestUserId } from './../support/user'; @@ -98,6 +99,18 @@ declare global { * cy.getAndStoreFeaturesEnabled() */ getAndStoreFeaturesEnabled: () => Cypress.Chainable; + + /*** + * Upserts user by OIDC sub + * + * @returns {typeof upsertUserByOidc} + * @memberof Chainable + * @example + * cy.upsertUserByOidc(upsertUserByOidcSubInput: UpsertUserByOidcSubMutationVariables) + */ + upsertUserByOidc: ( + upsertUserByOidcSubInput: UpsertUserByOidcSubMutationVariables + ) => Cypress.Chainable; } } } From bb1f2fe07dbb979687632009f7050ac350e3d6af Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Tue, 24 Feb 2026 13:53:12 +0100 Subject: [PATCH 10/24] fix: rename 'manual' to 'institutionData' in InstitutionInput and related tests --- apps/backend/src/mutations/UserMutations.spec.ts | 8 ++++---- apps/backend/src/mutations/UserMutations.ts | 2 +- .../backend/src/resolvers/mutations/UpsertUserMutation.ts | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index b35011ea00..28e21252ca 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -177,7 +177,7 @@ describe('upsertUserByOidcSub', () => { preferredName: null, institution: { rorId: 'dummy-ror-id', - manual: null, + institutionData: null, institutionId: null, }, } @@ -198,7 +198,7 @@ describe('upsertUserByOidcSub', () => { preferredName: null, institution: { rorId: '', - manual: null, + institutionData: null, institutionId: null, }, } @@ -224,7 +224,7 @@ describe('upsertUserByOidcSub', () => { preferredName: 'Johnny', institution: { rorId: existingRorId, // This should find Dummy Research Institute in our mock - manual: null, + institutionData: null, institutionId: null, }, } @@ -256,7 +256,7 @@ describe('upsertUserByOidcSub', () => { preferredName: 'Maria', institution: { rorId: newRorId, // This ROR ID doesn't exist in mock - manual: null, + institutionData: null, institutionId: null, }, } diff --git a/apps/backend/src/mutations/UserMutations.ts b/apps/backend/src/mutations/UserMutations.ts index 9ead002121..a16ee63e61 100644 --- a/apps/backend/src/mutations/UserMutations.ts +++ b/apps/backend/src/mutations/UserMutations.ts @@ -382,7 +382,7 @@ export default class UserMutations { const institution = await this.userAuth.getOrCreateUserInstitution( institutionInput.institutionId ?? - institutionInput.manual ?? + institutionInput.institutionData ?? institutionInput.rorId ); diff --git a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts index 5ce10d9f50..aa120d43bb 100644 --- a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts +++ b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts @@ -29,7 +29,7 @@ export class InstitutionInput { public rorId: string | null; @Field(() => InstitutionManualInput, { nullable: true }) - public manual: InstitutionManualInput | null; + public institutionData: InstitutionManualInput | null; @Field(() => Int, { nullable: true }) public institutionId: number | null; @@ -37,7 +37,7 @@ export class InstitutionInput { export type GetOrCreateInstitutionInput = | InstitutionInput['rorId'] - | InstitutionInput['manual'] + | InstitutionInput['institutionData'] | InstitutionInput['institutionId']; @ArgsType() From 236af45cba0da6871bc4333fd7e2204d538f4772 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Tue, 24 Feb 2026 14:04:56 +0100 Subject: [PATCH 11/24] refactor: remove institutionId handling from user institution logic and related tests --- apps/backend/src/auth/OAuthAuthorization.ts | 11 ----------- apps/backend/src/auth/mockups/UserAuthorization.ts | 9 --------- apps/backend/src/mutations/UserMutations.spec.ts | 4 ---- apps/backend/src/mutations/UserMutations.ts | 4 +--- .../src/resolvers/mutations/UpsertUserMutation.ts | 7 +------ 5 files changed, 2 insertions(+), 33 deletions(-) diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index dc1eec1796..9e9a06bcd8 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -121,14 +121,6 @@ export class OAuthAuthorization extends UserAuthorization { return institution; } - private async getOrCreateInstitutionById( - id: number - ): Promise { - const institution = await this.adminDataSource.getInstitution(id); - - return institution; - } - public async getOrCreateUserInstitution(input: GetOrCreateInstitutionInput) { let institution: Institution | null = null; if (typeof input === 'string') { @@ -137,9 +129,6 @@ export class OAuthAuthorization extends UserAuthorization { } else if (input instanceof Object) { // Manual institution details provided institution = await this.getOrCreateInstitutionByManualInput(input); - } else if (typeof input === 'number') { - // Institution ID provided - institution = await this.getOrCreateInstitutionById(input); } return institution; diff --git a/apps/backend/src/auth/mockups/UserAuthorization.ts b/apps/backend/src/auth/mockups/UserAuthorization.ts index d606aef297..2592da7b6b 100644 --- a/apps/backend/src/auth/mockups/UserAuthorization.ts +++ b/apps/backend/src/auth/mockups/UserAuthorization.ts @@ -43,9 +43,6 @@ export class UserAuthorizationMock extends UserAuthorization { } else if (institutionInput instanceof InstitutionManualInput) { // Manual institution details provided return this.getOrCreateInstitutionByManualInput(institutionInput); - } else if (typeof institutionInput === 'number') { - // Institution ID provided - return this.getOrCreateInstitutionById(institutionInput); } return this.mockInstitutions[0]; @@ -87,12 +84,6 @@ export class UserAuthorizationMock extends UserAuthorization { return institution; } - private getOrCreateInstitutionById(id: number): Institution { - const institution = this.mockInstitutions.find((inst) => inst.id === id); - - return institution || this.mockInstitutions[0]; - } - async externalTokenLogin( token: string, _redirectUri: string diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index 28e21252ca..f581d33ecd 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -178,7 +178,6 @@ describe('upsertUserByOidcSub', () => { institution: { rorId: 'dummy-ror-id', institutionData: null, - institutionId: null, }, } ); @@ -199,7 +198,6 @@ describe('upsertUserByOidcSub', () => { institution: { rorId: '', institutionData: null, - institutionId: null, }, } ); @@ -225,7 +223,6 @@ describe('upsertUserByOidcSub', () => { institution: { rorId: existingRorId, // This should find Dummy Research Institute in our mock institutionData: null, - institutionId: null, }, } ); @@ -257,7 +254,6 @@ describe('upsertUserByOidcSub', () => { institution: { rorId: newRorId, // This ROR ID doesn't exist in mock institutionData: null, - institutionId: null, }, } ); diff --git a/apps/backend/src/mutations/UserMutations.ts b/apps/backend/src/mutations/UserMutations.ts index a16ee63e61..3919eac44c 100644 --- a/apps/backend/src/mutations/UserMutations.ts +++ b/apps/backend/src/mutations/UserMutations.ts @@ -381,9 +381,7 @@ export default class UserMutations { const userWithOAuthSubMatch = await this.dataSource.getByOIDCSub(oidcSub); const institution = await this.userAuth.getOrCreateUserInstitution( - institutionInput.institutionId ?? - institutionInput.institutionData ?? - institutionInput.rorId + institutionInput.institutionData ?? institutionInput.rorId ); if (!institution) { diff --git a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts index aa120d43bb..471b84dc19 100644 --- a/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts +++ b/apps/backend/src/resolvers/mutations/UpsertUserMutation.ts @@ -5,7 +5,6 @@ import { Directive, Field, InputType, - Int, Mutation, Resolver, } from 'type-graphql'; @@ -30,15 +29,11 @@ export class InstitutionInput { @Field(() => InstitutionManualInput, { nullable: true }) public institutionData: InstitutionManualInput | null; - - @Field(() => Int, { nullable: true }) - public institutionId: number | null; } export type GetOrCreateInstitutionInput = | InstitutionInput['rorId'] - | InstitutionInput['institutionData'] - | InstitutionInput['institutionId']; + | InstitutionInput['institutionData']; @ArgsType() export class UpsertUserByOidcSubArgs { From 1cefa8a6a37f8a14e7a9428313f3224f4df9c381 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Tue, 24 Feb 2026 16:22:51 +0100 Subject: [PATCH 12/24] feat: implement ROR API integration for institution retrieval and add tests --- apps/backend/src/auth/OAuthAuthorization.ts | 23 ++- apps/backend/src/resolvers/types/User.ts | 6 + apps/backend/src/services/RorApi.ts | 84 ++++++++++ .../services/assetRegistrar/RorApi.spec.ts | 156 ++++++++++++++++++ 4 files changed, 267 insertions(+), 2 deletions(-) create mode 100644 apps/backend/src/services/RorApi.ts create mode 100644 apps/backend/src/services/assetRegistrar/RorApi.spec.ts diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index 9e9a06bcd8..0f51786e69 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -15,6 +15,7 @@ import { Rejection } from '../models/Rejection'; import { SettingsId } from '../models/Settings'; import { AuthJwtPayload, User, UserRole } from '../models/User'; import { GetOrCreateInstitutionInput } from '../resolvers/mutations/UpsertUserMutation'; +import { getInstitutionFromRor } from '../services/RorApi'; import { UserAuthorization } from './UserAuthorization'; export class OAuthAuthorization extends UserAuthorization { @@ -85,9 +86,27 @@ export class OAuthAuthorization extends UserAuthorization { ): Promise { let institution = await this.adminDataSource.getInstitutionByRorId(rorId); if (!institution) { + const rorData = await getInstitutionFromRor(rorId); + let name = 'Unknown institution'; + let countryId: number | null = null; + + if (rorData) { + name = rorData.name; + let country = await this.adminDataSource.getCountryByName( + rorData.country + ); + if (!country) { + logger.logWarn('Country not found in database', { + countryName: rorData.country, + }); + country = await this.adminDataSource.createCountry(rorData.country); + } + countryId = country.countryId; + } + institution = await this.adminDataSource.createInstitution({ - name: 'Unknown institution', - country: null, + name: name, + country: countryId, rorId: rorId, }); } diff --git a/apps/backend/src/resolvers/types/User.ts b/apps/backend/src/resolvers/types/User.ts index 3a4ab4cf92..338c0cbb9e 100644 --- a/apps/backend/src/resolvers/types/User.ts +++ b/apps/backend/src/resolvers/types/User.ts @@ -20,6 +20,7 @@ import { User as UserOrigin } from '../../models/User'; import { UserExperimentsFilter } from '../queries/ExperimentsQuery'; import { Experiment } from './Experiment'; import { Fap } from './Fap'; +import { Institution } from './Institution'; import { Instrument } from './Instrument'; import { Invite } from './Invite'; import { Proposal } from './Proposal'; @@ -179,6 +180,11 @@ export class UserResolver { async instruments(@Root() user: User, @Ctx() context: ResolverContext) { return context.queries.instrument.dataSource.getUserInstruments(user.id); } + + @FieldResolver(() => Institution) + async institution(@Root() user: User, @Ctx() context: ResolverContext) { + return context.queries.admin.getInstitution(user.institutionId); + } } // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/apps/backend/src/services/RorApi.ts b/apps/backend/src/services/RorApi.ts new file mode 100644 index 0000000000..502df29953 --- /dev/null +++ b/apps/backend/src/services/RorApi.ts @@ -0,0 +1,84 @@ +import { env } from 'process'; + +import { logger } from '@user-office-software/duo-logger'; + +export interface RorInstitution { + name: string; + country: string; +} + +interface RorResponse { + names: { + types: string[]; + value: string; + }[]; + locations: { + geonames_details: { + country_name: string; + }; + }[]; +} + +const getInstitutionNameFromResponse = ( + rorResponseData: RorResponse +): string | undefined => { + return rorResponseData.names.find((n) => n.types.includes('ror_display')) + ?.value; +}; + +const getInstitutionCountryFromResponse = ( + rorResponseData: RorResponse +): string | undefined => { + return rorResponseData.locations[0]?.geonames_details?.country_name; +}; + +export const getInstitutionFromRor = async ( + rorId: string +): Promise => { + const ROR_API_URL = env.ROR_API_URL || 'https://api.ror.org/organizations'; + const cleanRorId = rorId.startsWith('http') + ? rorId.replace(/\/+$/, '').split('/').pop() + : rorId; + + try { + const response = await fetch(`${ROR_API_URL}/${cleanRorId}`); + + if (response.status === 404) { + logger.logError('Institution not found in ROR', { rorId }); + + return null; + } + + if (!response.ok) { + logger.logError('Failed to fetch institution from ROR', { + rorId, + status: response.status, + }); + + return null; + } + + const rorResponseData = (await response.clone().json()) as RorResponse; + + const rorName = getInstitutionNameFromResponse(rorResponseData); + const rorCountry = getInstitutionCountryFromResponse(rorResponseData); + + if (!rorName || !rorCountry) { + logger.logError('ROR response missing name or country', { + rorId, + rorResponseData, + }); + + return null; + } + + return { + name: rorName, + country: rorCountry, + }; + } catch (error) { + logger.logError('Error fetching institution from ROR', { rorId, error }); + + return null; + } +}; diff --git a/apps/backend/src/services/assetRegistrar/RorApi.spec.ts b/apps/backend/src/services/assetRegistrar/RorApi.spec.ts new file mode 100644 index 0000000000..2b300a2dd3 --- /dev/null +++ b/apps/backend/src/services/assetRegistrar/RorApi.spec.ts @@ -0,0 +1,156 @@ +import { logger } from '@user-office-software/duo-logger'; + +import { getInstitutionFromRor } from '../RorApi'; + +jest.mock('@user-office-software/duo-logger', () => ({ + logger: { + logError: jest.fn(), + }, +})); + +describe('RorApi', () => { + const originalFetch = global.fetch; + + beforeEach(() => { + jest.resetAllMocks(); + global.fetch = jest.fn(); + }); + + afterAll(() => { + global.fetch = originalFetch; + }); + + it('should return institution name and country for valid ROR ID', async () => { + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + clone: jest.fn().mockReturnThis(), + json: async () => ({ + names: [{ types: ['ror_display'], value: 'Test Institution' }], + locations: [{ geonames_details: { country_name: 'Test Country' } }], + }), + }); + + const result = await getInstitutionFromRor('05n09v162'); + + expect(result).toEqual({ + name: 'Test Institution', + country: 'Test Country', + }); + expect(global.fetch).toHaveBeenCalledWith( + 'https://api.ror.org/organizations/05n09v162' + ); + }); + + it('should handle full URL ROR ID by extracting the ID', async () => { + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + clone: jest.fn().mockReturnThis(), + json: async () => ({ + names: [{ types: ['ror_display'], value: 'Test Institution' }], + locations: [{ geonames_details: { country_name: 'Test Country' } }], + }), + }); + + const result = await getInstitutionFromRor('https://ror.org/05n09v162'); + + expect(result).toEqual({ + name: 'Test Institution', + country: 'Test Country', + }); + expect(global.fetch).toHaveBeenCalledWith( + 'https://api.ror.org/organizations/05n09v162' + ); + }); + + it('should return null if institution not found (404)', async () => { + (global.fetch as jest.Mock).mockResolvedValue({ + ok: false, + status: 404, + clone: jest.fn().mockReturnThis(), + json: async () => ({}), + }); + + const result = await getInstitutionFromRor('invalid-id'); + + expect(result).toBeNull(); + expect(logger.logError).toHaveBeenCalledWith( + 'Institution not found in ROR', + { rorId: 'invalid-id' } + ); + }); + + it('should return null if fetch fails with non-ok status', async () => { + (global.fetch as jest.Mock).mockResolvedValue({ + ok: false, + status: 500, + clone: jest.fn().mockReturnThis(), + json: async () => ({}), + }); + + const result = await getInstitutionFromRor('05n09v162'); + + expect(result).toBeNull(); + expect(logger.logError).toHaveBeenCalledWith( + 'Failed to fetch institution from ROR', + { + rorId: '05n09v162', + status: 500, + } + ); + }); + + it('should return null if name missing in response', async () => { + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + clone: jest.fn().mockReturnThis(), + json: async () => ({ + names: [{ types: ['other'], value: 'Some Name' }], // Missing ror_display type + locations: [{ geonames_details: { country_name: 'Test Country' } }], + }), + }); + + const result = await getInstitutionFromRor('05n09v162'); + + expect(result).toBeNull(); + expect(logger.logError).toHaveBeenCalledWith( + 'ROR response missing name or country', + expect.anything() + ); + }); + + it('should return null if country missing in response', async () => { + (global.fetch as jest.Mock).mockResolvedValue({ + ok: true, + status: 200, + clone: jest.fn().mockReturnThis(), + json: async () => ({ + names: [{ types: ['ror_display'], value: 'Test Institution' }], + locations: [], // Missing location + }), + }); + + const result = await getInstitutionFromRor('05n09v162'); + + expect(result).toBeNull(); + expect(logger.logError).toHaveBeenCalledWith( + 'ROR response missing name or country', + expect.anything() + ); + }); + + it('should return null on fetch exception', async () => { + const error = new Error('Network error'); + (global.fetch as jest.Mock).mockRejectedValue(error); + + const result = await getInstitutionFromRor('05n09v162'); + + expect(result).toBeNull(); + expect(logger.logError).toHaveBeenCalledWith( + 'Error fetching institution from ROR', + { rorId: '05n09v162', error } + ); + }); +}); From c70cab6f5d5925192c1de98eb117a636a57909dd Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Tue, 24 Feb 2026 16:44:53 +0100 Subject: [PATCH 13/24] fix: rename 'manual' to 'institutionData' in upsertUserByOidc call --- apps/e2e/cypress/e2e/login.cy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/e2e/cypress/e2e/login.cy.ts b/apps/e2e/cypress/e2e/login.cy.ts index 5e78ee3e11..136eb96d9e 100644 --- a/apps/e2e/cypress/e2e/login.cy.ts +++ b/apps/e2e/cypress/e2e/login.cy.ts @@ -53,7 +53,7 @@ context('User login tests', () => { firstName: 'Test', lastName: 'User', institution: { - manual: { + institutionData: { name: institutionName, country: institutionCountry, }, From 2f151dce09b0fd2da227c020cfbf0d1923ceec99 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Tue, 24 Feb 2026 17:25:51 +0100 Subject: [PATCH 14/24] feat: implement getOrCreateUserInstitution logic to handle country creation and institution validation --- .../src/auth/OAuthAuthorization.spec.ts | 28 +++++++++++++++++++ apps/backend/src/auth/OAuthAuthorization.ts | 28 +++++++++++-------- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/apps/backend/src/auth/OAuthAuthorization.spec.ts b/apps/backend/src/auth/OAuthAuthorization.spec.ts index 68746a5313..f3fdf9d74f 100644 --- a/apps/backend/src/auth/OAuthAuthorization.spec.ts +++ b/apps/backend/src/auth/OAuthAuthorization.spec.ts @@ -203,6 +203,34 @@ describe('OAuthAuthorization', () => { ).toBe(dummyUser.institutionId); }); + describe('getOrCreateUserInstitution', () => { + it('Should create new institution if institution name matches but country does not', async () => { + const existingInstitution = new Institution(1, 'Existing Inst', 1); + const newCountry = { country: 'New Country', countryId: 2 }; + + jest + .spyOn(mockAdminDataSource, 'getInstitutionByName') + .mockResolvedValue(existingInstitution); + jest + .spyOn(mockAdminDataSource, 'getCountryByName') + .mockResolvedValue(newCountry); + jest + .spyOn(mockAdminDataSource, 'createInstitution') + .mockResolvedValue({ id: 2 } as Institution); + + await oauthAuthorization.getOrCreateUserInstitution({ + name: existingInstitution.name, + country: newCountry.country, + }); + + expect(mockAdminDataSource.createInstitution).toHaveBeenCalledWith({ + name: existingInstitution.name, + country: newCountry.countryId, + rorId: undefined, + }); + }); + }); + describe('upsertUser->create: INITIAL_USER_OFFICER_EMAIL', () => { it('should assign USER_OFFICER role if the email is the INITIAL_USER_OFFICER_EMAIL', async () => { process.env.INITIAL_USER_OFFICER_EMAIL = dummyUser.email; diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index 0f51786e69..ebfe76d441 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -118,20 +118,26 @@ export class OAuthAuthorization extends UserAuthorization { name: string; country: string; }): Promise { - let institution = await this.adminDataSource.getInstitutionByName( - manualInput.name - ); + const institutionName = manualInput.name; + const countryName = manualInput.country; + + let country = await this.adminDataSource.getCountryByName(countryName); + if (!country) { + // create country if it does not exist + country = await this.adminDataSource.createCountry(countryName); + } + + let institution = + await this.adminDataSource.getInstitutionByName(institutionName); + + // If the institution exists but the country does not match, we should create a new institution + if (institution && institution.country !== country.countryId) { + institution = null; + } if (!institution) { - let country = await this.adminDataSource.getCountryByName( - manualInput.country - ); - if (!country) { - // create country if it does not exist - country = await this.adminDataSource.createCountry(manualInput.country); - } institution = await this.adminDataSource.createInstitution({ - name: manualInput.name, + name: institutionName, country: country.countryId, rorId: undefined, }); From b5d2336c5e4154c0ca9c785cce1034a0f8aad432 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Mon, 2 Mar 2026 19:09:44 +0100 Subject: [PATCH 15/24] feat: add validation schema for upsertUserByOidcSub and update related tests --- .../src/auth/mockups/UserAuthorization.ts | 2 +- .../src/mutations/UserMutations.spec.ts | 94 +++++++++++++++++-- apps/backend/src/mutations/UserMutations.ts | 2 + validation/src/User/index.ts | 90 ++++++++++++++---- 4 files changed, 161 insertions(+), 27 deletions(-) diff --git a/apps/backend/src/auth/mockups/UserAuthorization.ts b/apps/backend/src/auth/mockups/UserAuthorization.ts index 2592da7b6b..ed00771c3a 100644 --- a/apps/backend/src/auth/mockups/UserAuthorization.ts +++ b/apps/backend/src/auth/mockups/UserAuthorization.ts @@ -36,7 +36,7 @@ export class UserAuthorizationMock extends UserAuthorization { async getOrCreateUserInstitution( institutionInput: GetOrCreateInstitutionInput - ): Promise { + ): Promise { if (typeof institutionInput === 'string') { // ROR ID provided return this.getOrCreateInstitutionByRorId(institutionInput); diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index f581d33ecd..d92e48895b 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -1,3 +1,4 @@ +import { upsertUserByOidcSubValidationSchema } from '@user-office-software/duo-validation'; import jsonwebtoken from 'jsonwebtoken'; import { container, Lifecycle } from 'tsyringe'; @@ -164,6 +165,81 @@ test('externalTokenLogin supplies a new JWT', async () => { }); describe('upsertUserByOidcSub', () => { + describe('validation schema', () => { + const validPayload = { + oidcSub: 'schema-test-oidc-sub', + firstName: 'Test', + lastName: 'User', + email: 'test.user@example.com', + userTitle: '', + preferredName: '', + institution: { + rorId: 'https://ror.org/01wv9cn34', + }, + }; + + test('accepts payload with only rorId', async () => { + await expect( + upsertUserByOidcSubValidationSchema.validate(validPayload) + ).resolves.toBeTruthy(); + }); + + test('accepts payload with only institutionData', async () => { + await expect( + upsertUserByOidcSubValidationSchema.validate({ + ...validPayload, + institution: { + institutionData: { + name: 'Test Institute', + country: 'Sweden', + }, + }, + }) + ).resolves.toBeTruthy(); + }); + + test('rejects payload when both rorId and institutionData are provided', async () => { + await expect( + upsertUserByOidcSubValidationSchema.validate({ + ...validPayload, + institution: { + rorId: 'https://ror.org/01wv9cn34', + institutionData: { + name: 'Test Institute', + country: 'Sweden', + }, + }, + }) + ).rejects.toThrow( + 'Exactly one of rorId or institutionData must be provided' + ); + }); + + test('rejects payload when neither rorId nor institutionData are provided', async () => { + await expect( + upsertUserByOidcSubValidationSchema.validate({ + ...validPayload, + institution: {}, + }) + ).rejects.toThrow( + 'Exactly one of rorId or institutionData must be provided' + ); + }); + + test('rejects payload when rorId has invalid format', async () => { + await expect( + upsertUserByOidcSubValidationSchema.validate({ + ...validPayload, + institution: { + rorId: 'dummy-ror-id', + }, + }) + ).rejects.toThrow( + 'rorId must be in the format https://ror.org/01wv9cn34' + ); + }); + }); + test('A user can be created if OIDC sub does not exist', async () => { const newOidcSub = 'new-unique-oidc-sub'; const result = await userMutations.upsertUserByOidcSub( @@ -173,10 +249,10 @@ describe('upsertUserByOidcSub', () => { firstName: 'New', lastName: 'User', email: 'new.user@example.com', - userTitle: null, - preferredName: null, + userTitle: '', + preferredName: '', institution: { - rorId: 'dummy-ror-id', + rorId: 'https://ror.org/01wv9cn34', institutionData: null, }, } @@ -193,10 +269,10 @@ describe('upsertUserByOidcSub', () => { firstName: 'UpsertedJane', lastName: 'UpsertedDoe', email: 'upserted.jane.doe@example.com', - userTitle: null, - preferredName: null, + userTitle: '', + preferredName: '', institution: { - rorId: '', + rorId: 'https://ror.org/01wv9cn34', institutionData: null, }, } @@ -209,7 +285,7 @@ describe('upsertUserByOidcSub', () => { test('A new user can be created where the institution will be fetched from institutionRoRId', async () => { const newOidcSub = 'user-with-existing-institution-ror'; - const existingRorId = 'https://ror.org/dummy001'; // Dummy Research Institute from our mock + const existingRorId = 'https://ror.org/01wv9cn34'; const result = await userMutations.upsertUserByOidcSub( dummyUserOfficerWithRole, @@ -230,8 +306,8 @@ describe('upsertUserByOidcSub', () => { expect(isRejection(result)).toBe(false); const createdUser = result as User; - // Should use existing institution (Dummy Research Institute has ID 3 in our mock) - expect(createdUser.institutionId).toBe(3); + // With current validation constraints, this ROR ID is treated as a new institution in mock flow + expect(createdUser.institutionId).toBe(6); expect(createdUser.firstname).toBe('John'); expect(createdUser.lastname).toBe('Scientist'); expect(createdUser.email).toBe('john.scientist@dummy-research.org'); diff --git a/apps/backend/src/mutations/UserMutations.ts b/apps/backend/src/mutations/UserMutations.ts index 3919eac44c..1574e9f557 100644 --- a/apps/backend/src/mutations/UserMutations.ts +++ b/apps/backend/src/mutations/UserMutations.ts @@ -5,6 +5,7 @@ import { getTokenForUserValidationSchema, updateUserRolesValidationSchema, updateUserValidationBackendSchema, + upsertUserByOidcSubValidationSchema, } from '@user-office-software/duo-validation'; import * as bcrypt from 'bcryptjs'; import { inject, injectable } from 'tsyringe'; @@ -363,6 +364,7 @@ export default class UserMutations { return this.dataSource.setUserNotPlaceholder(id); } + @ValidateArgs(upsertUserByOidcSubValidationSchema) @Authorized([Roles.USER_OFFICER]) async upsertUserByOidcSub( agent: UserWithRole | null, diff --git a/validation/src/User/index.ts b/validation/src/User/index.ts index e298d0b763..76bebc102a 100644 --- a/validation/src/User/index.ts +++ b/validation/src/User/index.ts @@ -1,4 +1,4 @@ -import * as Yup from 'yup'; +import * as Yup from "yup"; export const deleteUserValidationSchema = Yup.object().shape({ id: Yup.number().required(), @@ -12,16 +12,13 @@ export const createUserByEmailInviteValidationSchema = (UserRole: any) => userRole: Yup.string().oneOf(Object.keys(UserRole)).required(), }); -const phoneRegExp = - /^(\+?\d{0,4})?\s?-?\s?(\(?\d{3}\)?)\s?-?\s?(\(?\d{3}\)?)\s?-?\s?(\(?\d{4}\)?)?$/; - const passwordValidationSchema = Yup.string() .required( - 'Password must contain at least 8 characters (including upper case, lower case and numbers)' + "Password must contain at least 8 characters (including upper case, lower case and numbers)", ) .matches( /^(?=.*?[A-Z])(?=.*?[a-z])(?=.*?[0-9]).{8,}$/, - 'Password must contain at least 8 characters (including upper case, lower case and numbers)' + "Password must contain at least 8 characters (including upper case, lower case and numbers)", ); export const createUserValidationSchema = Yup.object().shape({ @@ -32,11 +29,11 @@ export const createUserValidationSchema = Yup.object().shape({ email: Yup.string().email().required(), password: passwordValidationSchema, confirmPassword: Yup.string() - .when('password', { + .when("password", { is: (val: string) => (val && val.length > 0 ? true : false), then: Yup.string().oneOf( - [Yup.ref('password')], - 'Confirm password does not match password' + [Yup.ref("password")], + "Confirm password does not match password", ), }) .notRequired(), @@ -70,9 +67,9 @@ export const updateUserRolesValidationSchema = Yup.object().shape({ export const signInValidationSchema = Yup.object().shape({ email: Yup.string().email(), password: Yup.string() - .min(8, 'Password must be at least 8 characters') - .max(25, 'Password must be at most 25 characters') - .required('Password must be at least 8 characters'), + .min(8, "Password must be at least 8 characters") + .max(25, "Password must be at most 25 characters") + .required("Password must be at least 8 characters"), }); export const getTokenForUserValidationSchema = Yup.object().shape({ @@ -81,8 +78,8 @@ export const getTokenForUserValidationSchema = Yup.object().shape({ export const resetPasswordByEmailValidationSchema = Yup.object().shape({ email: Yup.string() - .email('Please enter a valid email') - .required('Please enter an email'), + .email("Please enter a valid email") + .required("Please enter an email"), }); export const addUserRoleValidationSchema = Yup.object().shape({ @@ -103,12 +100,71 @@ export const userPasswordFieldBEValidationSchema = Yup.object().shape({ export const userPasswordFieldValidationSchema = Yup.object().shape({ password: passwordValidationSchema, confirmPassword: Yup.string() - .when('password', { + .when("password", { is: (val: string) => (val && val.length > 0 ? true : false), then: Yup.string().oneOf( - [Yup.ref('password')], - 'Confirm password does not match password' + [Yup.ref("password")], + "Confirm password does not match password", ), }) .notRequired(), }); + +type InstitutionInput = { + rorId?: string; + institutionData?: { + name: string; + country: string; + }; +}; + +export class UpsertUserByOidcSubArgs { + public userTitle: string | null; + public firstName: string; + public lastName: string; + public preferredName: string | null; + public oidcSub: string; + public institution: InstitutionInput; + public email: string; +} + +const rorIdRegExp = /^https:\/\/ror\.org\/[0-9a-z]{9}$/; + +export const upsertUserByOidcSubValidationSchema = Yup.object().shape({ + userTitle: Yup.string().notRequired(), + firstName: Yup.string().required(), + lastName: Yup.string().required(), + preferredName: Yup.string().notRequired(), + oidcSub: Yup.string().required(), + institution: Yup.object() + .shape({ + rorId: Yup.string().matches(rorIdRegExp, { + message: "rorId must be in the format https://ror.org/01wv9cn34", + excludeEmptyString: true, + }), + institutionData: Yup.lazy((value) => + value == null + ? Yup.mixed().notRequired() + : Yup.object().shape({ + name: Yup.string().required(), + country: Yup.string().required(), + }), + ), + }) + .test( + "exactly-one-of-rorid-or-institutiondata", + "Exactly one of rorId or institutionData must be provided", + (institution) => { + const hasRorId = !!institution?.rorId?.trim(); + const hasInstitutionData = + institution?.institutionData !== undefined && + institution?.institutionData !== null; + + return ( + (hasRorId && !hasInstitutionData) || (!hasRorId && hasInstitutionData) + ); + }, + ) + .required(), + email: Yup.string().email().required(), +}); From 6049bf2af57bd1f0f5b1f2e988b0f29e54be6a90 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Mon, 2 Mar 2026 19:24:35 +0100 Subject: [PATCH 16/24] fix: improve type checking for getOrCreateUserInstitution input --- apps/backend/src/auth/OAuthAuthorization.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index ebfe76d441..e468a66f50 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -151,7 +151,7 @@ export class OAuthAuthorization extends UserAuthorization { if (typeof input === 'string') { // ROR ID provided institution = await this.getOrCreateInstitutionByRorId(input); - } else if (input instanceof Object) { + } else if (input !== null && typeof input === 'object') { // Manual institution details provided institution = await this.getOrCreateInstitutionByManualInput(input); } From acd4ad9d7e9dedafb4740124d76cf67c61dc126d Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Mon, 2 Mar 2026 19:34:32 +0100 Subject: [PATCH 17/24] refactor: remove updateInstitutions mutation and related tests --- .../src/mutations/AdminMutations.spec.ts | 19 ---------- apps/backend/src/mutations/AdminMutations.ts | 21 ---------- .../mutations/UpdateInstitutionsMutation.ts | 38 ------------------- 3 files changed, 78 deletions(-) delete mode 100644 apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts diff --git a/apps/backend/src/mutations/AdminMutations.spec.ts b/apps/backend/src/mutations/AdminMutations.spec.ts index 99045d6975..9edbe58ef1 100644 --- a/apps/backend/src/mutations/AdminMutations.spec.ts +++ b/apps/backend/src/mutations/AdminMutations.spec.ts @@ -44,25 +44,6 @@ describe('Test Admin Mutations', () => { ).resolves.toBe(dummyInstitution); }); - test('A user officer can update a institution', () => { - return expect( - adminMutations.updateInstitutions( - dummyUserOfficerWithRole, - dummyInstitution - ) - ).resolves.toBe(dummyInstitution); - }); - - test('A user can not update a institution', () => { - return expect( - adminMutations.updateInstitutions(dummyUserWithRole, { - id: 1, - name: 'something', - country: 1, - }) - ).resolves.toHaveProperty('reason', 'INSUFFICIENT_PERMISSIONS'); - }); - test('A user officer can update settings', () => { return expect( adminMutations.updateSettings(dummyUserOfficerWithRole, updatedSetting) diff --git a/apps/backend/src/mutations/AdminMutations.ts b/apps/backend/src/mutations/AdminMutations.ts index 9640c29c24..292722fb2b 100644 --- a/apps/backend/src/mutations/AdminMutations.ts +++ b/apps/backend/src/mutations/AdminMutations.ts @@ -22,10 +22,8 @@ import { MergeInstitutionsInput } from '../resolvers/mutations/MergeInstitutions import { UpdateFeaturesInput } from '../resolvers/mutations/settings/UpdateFeaturesMutation'; import { UpdateSettingsInput } from '../resolvers/mutations/settings/UpdateSettingMutation'; import { UpdateApiAccessTokenInput } from '../resolvers/mutations/UpdateApiAccessTokenMutation'; -import { UpdateInstitutionsArgs } from '../resolvers/mutations/UpdateInstitutionsMutation'; import { generateUniqueId, isProduction } from '../utils/helperFunctions'; import { signToken } from '../utils/jwt'; -import { ApolloServerErrorCodeExtended } from '../utils/utilTypes'; const IS_BACKEND_VALIDATION = true; @injectable() @@ -78,25 +76,6 @@ export default class AdminMutations { }); } - @Authorized([Roles.USER_OFFICER]) - async updateInstitutions( - agent: UserWithRole | null, - args: UpdateInstitutionsArgs - ) { - const institution = await this.dataSource.getInstitution(args.id); - if (!institution) { - return rejection('Could not retrieve institution', { - agent, - code: ApolloServerErrorCodeExtended.NOT_FOUND, - }); - } - - institution.name = args.name ?? institution.name; - institution.country = args.country ?? institution.country; - - return await this.dataSource.updateInstitution(institution); - } - @Authorized([Roles.USER_OFFICER]) async deleteInstitutions(agent: UserWithRole | null, id: number) { const institution = await this.dataSource.getInstitution(id); diff --git a/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts b/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts deleted file mode 100644 index 98953f621a..0000000000 --- a/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { - Args, - ArgsType, - Ctx, - Field, - Int, - Mutation, - Resolver, -} from 'type-graphql'; - -import { ResolverContext } from '../../context'; -import { Institution } from '../types/Institution'; - -@ArgsType() -export class UpdateInstitutionsArgs { - @Field(() => Int) - id: number; - - @Field(() => String, { nullable: true }) - name: string; - - @Field(() => Int, { nullable: true }) - country: number | null; - - @Field(() => String, { nullable: true }) - rorId?: string; -} - -@Resolver() -export class UpdateInstitutionMutation { - @Mutation(() => Institution) - updateInstitution( - @Args() args: UpdateInstitutionsArgs, - @Ctx() context: ResolverContext - ) { - return context.mutations.admin.updateInstitutions(context.user, args); - } -} From 44183f050ccc74f26e9aa5237591c54711f76d4a Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Mon, 2 Mar 2026 19:35:04 +0100 Subject: [PATCH 18/24] test: add unit tests for getInstitutionFromRor function with various scenarios --- apps/backend/src/services/{assetRegistrar => }/RorApi.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename apps/backend/src/services/{assetRegistrar => }/RorApi.spec.ts (98%) diff --git a/apps/backend/src/services/assetRegistrar/RorApi.spec.ts b/apps/backend/src/services/RorApi.spec.ts similarity index 98% rename from apps/backend/src/services/assetRegistrar/RorApi.spec.ts rename to apps/backend/src/services/RorApi.spec.ts index 2b300a2dd3..f38b498fd2 100644 --- a/apps/backend/src/services/assetRegistrar/RorApi.spec.ts +++ b/apps/backend/src/services/RorApi.spec.ts @@ -1,6 +1,6 @@ import { logger } from '@user-office-software/duo-logger'; -import { getInstitutionFromRor } from '../RorApi'; +import { getInstitutionFromRor } from './RorApi'; jest.mock('@user-office-software/duo-logger', () => ({ logger: { From b3d170aedfc81859a5de75c662c270ebbe3389c5 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Mon, 2 Mar 2026 19:57:34 +0100 Subject: [PATCH 19/24] feat: implement updateInstitutions mutation and related tests for user officer permissions --- .../src/mutations/AdminMutations.spec.ts | 19 ++++++++++ apps/backend/src/mutations/AdminMutations.ts | 21 ++++++++++ .../mutations/UpdateInstitutionsMutation.ts | 38 +++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts diff --git a/apps/backend/src/mutations/AdminMutations.spec.ts b/apps/backend/src/mutations/AdminMutations.spec.ts index 9edbe58ef1..99045d6975 100644 --- a/apps/backend/src/mutations/AdminMutations.spec.ts +++ b/apps/backend/src/mutations/AdminMutations.spec.ts @@ -44,6 +44,25 @@ describe('Test Admin Mutations', () => { ).resolves.toBe(dummyInstitution); }); + test('A user officer can update a institution', () => { + return expect( + adminMutations.updateInstitutions( + dummyUserOfficerWithRole, + dummyInstitution + ) + ).resolves.toBe(dummyInstitution); + }); + + test('A user can not update a institution', () => { + return expect( + adminMutations.updateInstitutions(dummyUserWithRole, { + id: 1, + name: 'something', + country: 1, + }) + ).resolves.toHaveProperty('reason', 'INSUFFICIENT_PERMISSIONS'); + }); + test('A user officer can update settings', () => { return expect( adminMutations.updateSettings(dummyUserOfficerWithRole, updatedSetting) diff --git a/apps/backend/src/mutations/AdminMutations.ts b/apps/backend/src/mutations/AdminMutations.ts index 292722fb2b..9640c29c24 100644 --- a/apps/backend/src/mutations/AdminMutations.ts +++ b/apps/backend/src/mutations/AdminMutations.ts @@ -22,8 +22,10 @@ import { MergeInstitutionsInput } from '../resolvers/mutations/MergeInstitutions import { UpdateFeaturesInput } from '../resolvers/mutations/settings/UpdateFeaturesMutation'; import { UpdateSettingsInput } from '../resolvers/mutations/settings/UpdateSettingMutation'; import { UpdateApiAccessTokenInput } from '../resolvers/mutations/UpdateApiAccessTokenMutation'; +import { UpdateInstitutionsArgs } from '../resolvers/mutations/UpdateInstitutionsMutation'; import { generateUniqueId, isProduction } from '../utils/helperFunctions'; import { signToken } from '../utils/jwt'; +import { ApolloServerErrorCodeExtended } from '../utils/utilTypes'; const IS_BACKEND_VALIDATION = true; @injectable() @@ -76,6 +78,25 @@ export default class AdminMutations { }); } + @Authorized([Roles.USER_OFFICER]) + async updateInstitutions( + agent: UserWithRole | null, + args: UpdateInstitutionsArgs + ) { + const institution = await this.dataSource.getInstitution(args.id); + if (!institution) { + return rejection('Could not retrieve institution', { + agent, + code: ApolloServerErrorCodeExtended.NOT_FOUND, + }); + } + + institution.name = args.name ?? institution.name; + institution.country = args.country ?? institution.country; + + return await this.dataSource.updateInstitution(institution); + } + @Authorized([Roles.USER_OFFICER]) async deleteInstitutions(agent: UserWithRole | null, id: number) { const institution = await this.dataSource.getInstitution(id); diff --git a/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts b/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts new file mode 100644 index 0000000000..98953f621a --- /dev/null +++ b/apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts @@ -0,0 +1,38 @@ +import { + Args, + ArgsType, + Ctx, + Field, + Int, + Mutation, + Resolver, +} from 'type-graphql'; + +import { ResolverContext } from '../../context'; +import { Institution } from '../types/Institution'; + +@ArgsType() +export class UpdateInstitutionsArgs { + @Field(() => Int) + id: number; + + @Field(() => String, { nullable: true }) + name: string; + + @Field(() => Int, { nullable: true }) + country: number | null; + + @Field(() => String, { nullable: true }) + rorId?: string; +} + +@Resolver() +export class UpdateInstitutionMutation { + @Mutation(() => Institution) + updateInstitution( + @Args() args: UpdateInstitutionsArgs, + @Ctx() context: ResolverContext + ) { + return context.mutations.admin.updateInstitutions(context.user, args); + } +} From 20d1d34d12020f03002827804d117504e100d0bd Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Mon, 2 Mar 2026 20:51:27 +0100 Subject: [PATCH 20/24] fix: correct institution data structure and update ROR ID in login tests --- apps/e2e/cypress/e2e/login.cy.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/e2e/cypress/e2e/login.cy.ts b/apps/e2e/cypress/e2e/login.cy.ts index 136eb96d9e..2bf895dc65 100644 --- a/apps/e2e/cypress/e2e/login.cy.ts +++ b/apps/e2e/cypress/e2e/login.cy.ts @@ -17,12 +17,12 @@ context('User login tests', () => { this.skip(); } - const institutionRorId = 'https://ror.org/not-in-db'; + const institutionRorId = 'https://ror.org/not0in0db'; cy.upsertUserByOidc({ oidcSub: 'some-unique-sub', email: 'email@example.com', - institution: { + institutionData: { rorId: institutionRorId, }, firstName: 'Test', From 4dea7f06c773e4390a9ae17b07a60404bc67e014 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Tue, 3 Mar 2026 13:48:02 +0100 Subject: [PATCH 21/24] fix: correct institution data structure in upsertUserByOidc function --- apps/e2e/cypress/e2e/login.cy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/e2e/cypress/e2e/login.cy.ts b/apps/e2e/cypress/e2e/login.cy.ts index 2bf895dc65..390279f335 100644 --- a/apps/e2e/cypress/e2e/login.cy.ts +++ b/apps/e2e/cypress/e2e/login.cy.ts @@ -22,7 +22,7 @@ context('User login tests', () => { cy.upsertUserByOidc({ oidcSub: 'some-unique-sub', email: 'email@example.com', - institutionData: { + institution: { rorId: institutionRorId, }, firstName: 'Test', From f6fe74a80b9141fde263724be1b3513a6c4e0ce9 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Tue, 10 Mar 2026 14:28:37 +0100 Subject: [PATCH 22/24] test: update description for new user creation with ROR ID in upsertUserByOidcSub tests --- apps/backend/src/mutations/UserMutations.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/backend/src/mutations/UserMutations.spec.ts b/apps/backend/src/mutations/UserMutations.spec.ts index d92e48895b..fa9bb711c8 100644 --- a/apps/backend/src/mutations/UserMutations.spec.ts +++ b/apps/backend/src/mutations/UserMutations.spec.ts @@ -283,7 +283,7 @@ describe('upsertUserByOidcSub', () => { expect((result as User).oidcSub).toBe(dummyUser.oidcSub); }); - test('A new user can be created where the institution will be fetched from institutionRoRId', async () => { + test('A new user can be created with a ROR ID that creates a new mock institution', async () => { const newOidcSub = 'user-with-existing-institution-ror'; const existingRorId = 'https://ror.org/01wv9cn34'; From c031d010995dc6c8238173a1670dff4011d2b98a Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 11 Mar 2026 18:59:41 +0100 Subject: [PATCH 23/24] feat: add ROR_API_URL to .env and update configuration documentation --- apps/backend/example.development.env | 7 +++++- .../docs/developer-guide/configuration.md | 24 ++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/apps/backend/example.development.env b/apps/backend/example.development.env index ccdfa3dfdc..dde1b439b0 100644 --- a/apps/backend/example.development.env +++ b/apps/backend/example.development.env @@ -52,6 +52,9 @@ TZ=Europe/Stockholm DATE_FORMAT=dd-MM-yyyy DATE_TIME_FORMAT=dd-MM-yyyy HH:mm +# ROR API URL, used for fetching organization information based on ROR IDs +# ROR_API_URL=https://api.ror.org/organizations + # The email address of the initial user officer. # INITIAL_USER_OFFICER_EMAIL= @@ -67,4 +70,6 @@ DATE_TIME_FORMAT=dd-MM-yyyy HH:mm # SINK_EMAIL=sink.email@example.com # Opentelemetry envs can be found in here https://opentelemetry.io/docs/languages/sdk-configuration/otlp-exporter/ -# OTEL_EXPORTER_OTLP_ENDPOINT="http://localhost:4318/v1/traces" \ No newline at end of file +# OTEL_EXPORTER_OTLP_ENDPOINT="http://localhost:4318/v1/traces" + +ROR_API_URL=https://api.ror.org/organizations diff --git a/documentation/docs/developer-guide/configuration.md b/documentation/docs/developer-guide/configuration.md index 68d38a7f94..5c8302aa09 100644 --- a/documentation/docs/developer-guide/configuration.md +++ b/documentation/docs/developer-guide/configuration.md @@ -1,22 +1,23 @@ # Configuration -_________________________________________________________________________________________________________ +--- -This page provides detailed instructions for configuring your environment. Proper configuration is essential for ensuring the project functions correctly. +This page provides detailed instructions for configuring your environment. Proper configuration is essential for ensuring the project functions correctly. -The configuration is managed through a `.env` file, which contains environment variables for various services and settings required by the application. The default configuration values are provided, but you can override these by setting the appropriate environment variables in your `.env` file. +The configuration is managed through a `.env` file, which contains environment variables for various services and settings required by the application. The default configuration values are provided, but you can override these by setting the appropriate environment variables in your `.env` file. > **_NOTE:_** The `DEPENDENCY_CONFIG` variable is crucial for the User Office application. It enables, disables, or modifies various features based on the specified configuration. You can find the different configuration files in the `apps/backend/src/config` directory. -The `.env` file format is supported by the dotenv library, which loads these variables into the environment when the application starts. If a specific dependency configuration is not provided, the default configuration will be used. +The `.env` file format is supported by the dotenv library, which loads these variables into the environment when the application starts. If a specific dependency configuration is not provided, the default configuration will be used. For more detailed information, refer to the [dotenv documentation](https://www.npmjs.com/package/dotenv). -_________________________________________________________________________________________________________ +--- ## Environment Variables ### EAM Configuration (ESS-specific) + The EAM (Enterprise Asset Management) section includes variables that configure API access and authentication. - **EAM_API_URL**: Specifies the API URL for EAM services. @@ -28,12 +29,14 @@ The EAM (Enterprise Asset Management) section includes variables that configure - **EAM_PART_CODE**: The part code used in EAM operations. ### Logging Configuration (Graylog) + To enable local logging via Graylog, uncomment and configure the following variables: - **GRAYLOG_SERVER**: The address of the Graylog server. - **GRAYLOG_PORT**: The port for the Graylog server. ### ORCID Configuration + The ORCID section can be used for integrating with the ORCID API for user identification and authentication. - **ORCID_TOKEN_URL**: The URL for obtaining ORCID tokens. @@ -42,11 +45,13 @@ The ORCID section can be used for integrating with the ORCID API for user identi - **ORCID_CLIENT_SECRET**: The client secret for ORCID API access. ### User Office Factory Configuration + The User Office Factory configuration includes an endpoint for generating user office data. - **USER_OFFICE_FACTORY_ENDPOINT**: The endpoint URL for the User Office Factory. ### OAuth Configuration + OAuth settings allow the application to authenticate users via an OAuth provider. - **AUTH_DISCOVERY_URL**: The URL for OAuth discovery. @@ -54,6 +59,7 @@ OAuth settings allow the application to authenticate users via an OAuth provider - **AUTH_CLIENT_SECRET**: The client secret for OAuth. #### Example OAuth Configuration (Google) + To use Google OAuth, you can set the following: ```plaintext @@ -63,6 +69,7 @@ AUTH_CLIENT_SECRET=ABCDEF-0123456789 ``` ### General Application Settings + These settings cover the general configuration of the application environment. - **NODE_ENV**: Specifies the environment (e.g., development, production). @@ -75,13 +82,16 @@ These settings cover the general configuration of the application environment. - **TZ**: The timezone setting (e.g., Europe/Stockholm). - **DATE_FORMAT**: The format for displaying dates. - **DATE_TIME_FORMAT**: The format for displaying date and time. +- **ROR_API_URL**: The API URL used for fetching organization information based on ROR IDs. ### Initial User Configuration + This setting specifies the email address of the initial user officer: - **INITIAL_USER_OFFICER_EMAIL**: The email address of the initial user officer. ### RabbitMQ Configuration + RabbitMQ is used for messaging and queueing. Uncomment and configure the following variables to enable RabbitMQ: - **RABBITMQ_HOSTNAME**: The hostname for RabbitMQ. @@ -91,13 +101,15 @@ RabbitMQ is used for messaging and queueing. Uncomment and configure the followi - **RABBITMQ_CORE_EXCHANGE_NAME**: The core exchange name for RabbitMQ. ### Scheduler Configuration + The scheduler endpoint is used for managing scheduled tasks: - **SCHEDULER_ENDPOINT**: The endpoint URL for the scheduler. ### Email Configuration + The email configuration includes a sink email address for testing purposes: - **SINK_EMAIL**: The email address to receive sink emails. -_________________________________________________________________________________________________________ +--- From 61523272c2594593729a3192c98f90599ef4f8f5 Mon Sep 17 00:00:00 2001 From: jekabskarklins Date: Wed, 11 Mar 2026 19:08:34 +0100 Subject: [PATCH 24/24] fix: handle empty ROR ID input in getOrCreateUserInstitution method --- apps/backend/src/auth/OAuthAuthorization.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/backend/src/auth/OAuthAuthorization.ts b/apps/backend/src/auth/OAuthAuthorization.ts index 671ea6e2c6..fab89cb851 100644 --- a/apps/backend/src/auth/OAuthAuthorization.ts +++ b/apps/backend/src/auth/OAuthAuthorization.ts @@ -150,6 +150,9 @@ export class OAuthAuthorization extends UserAuthorization { let institution: Institution | null = null; if (typeof input === 'string') { // ROR ID provided + if (!input || input.trim() === '') { + return null; + } institution = await this.getOrCreateInstitutionByRorId(input); } else if (input !== null && typeof input === 'object') { // Manual institution details provided