fix: refactor institution handling in user authorization and mutation

[jekabs-karklins]

Description

This PR refactors the way institution is defined when calling upserUserByOidc

Motivation and Context

This change allows to call upserUserByOidc and specify institution in two ways
Either specifying

RorId
or
{institutionName: string, institutionCountry: string}

Changes

1. Refactored getOrCreateUserInstitution method in OAuthAuthorization to accept different types of input (ROR ID, manual input, institution ID), and split into smaller, more specific methods.
2. Updated getOrCreateUserInstitution method in StfcUserAuthorization to match the new input type.
3. Updated getOrCreateUserInstitution method in UserAuthorization to match the new input type.

How Has This Been Tested

Unit tests have been added and updated and run to ensure the changes work as expected. Additionally, manual testing has been performed to verify correct functionality in a live environment.

Fixes Jira Issue

https://jira.esss.lu.se/browse/SWAP-5157

[Copilot]

Pull request overview

This PR refactors the institution handling in user authorization and mutation logic to support two flexible ways of specifying institution information: either by ROR ID or by manual input (name and country). The changes introduce a new @oneOf GraphQL input type, add a ROR API integration service, and update the authorization flow to handle both input methods.

Changes:

• Introduced InstitutionInput with @oneOf directive to accept either ROR ID or manual institution data
• Added ROR API service to fetch institution details from the ROR registry
• Refactored getOrCreateUserInstitution across authorization classes to handle the new input types
• Made institution country field nullable to support institutions without country information

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
apps/frontend/src/graphql/user/upsertUserByOidcSub.graphql	Updated mutation to use new InstitutionInput type instead of separate fields
apps/e2e/cypress/types/user.d.ts	Added type definition for upsertUserByOidc command
apps/e2e/cypress/support/user.ts	Implemented upsertUserByOidc Cypress command
apps/e2e/cypress/e2e/login.cy.ts	Updated tests to use new institution input structure
apps/backend/src/utils/buildFederatedSchema.ts	Registered @oneOf directive for GraphQL schema
apps/backend/src/services/assetRegistrar/RorApi.spec.ts	Added comprehensive test coverage for ROR API service
apps/backend/src/services/RorApi.ts	Implemented ROR API integration to fetch institution data
apps/backend/src/resolvers/types/User.ts	Added institution field resolver to User type
apps/backend/src/resolvers/types/Institution.ts	Added null check for country field in resolver
apps/backend/src/resolvers/mutations/UpsertUserMutation.ts	Defined InstitutionInput and InstitutionManualInput types with @oneOf directive
apps/backend/src/resolvers/mutations/UpdateInstitutionsMutation.ts	Made country field nullable to align with model changes
apps/backend/src/mutations/UserMutations.ts	Updated to use new institution input structure
apps/backend/src/mutations/UserMutations.spec.ts	Updated tests and registered UserAuthorizationMock properly
apps/backend/src/models/Institution.ts	Made country field nullable
apps/backend/src/datasources/postgres/UserDataSource.ts	Improved error handling to include originalError
apps/backend/src/auth/mockups/UserAuthorization.ts	Updated mock to handle new institution input types
apps/backend/src/auth/UserAuthorization.ts	Updated abstract method signature for getOrCreateUserInstitution
apps/backend/src/auth/StfcUserAuthorization.ts	Updated method signature (not implemented)
apps/backend/src/auth/OAuthAuthorization.ts	Refactored into separate methods for ROR ID and manual input handling
apps/backend/src/auth/OAuthAuthorization.spec.ts	Updated tests to reflect new behavior and API

Comments suppressed due to low confidence (2)

apps/backend/src/auth/OAuthAuthorization.ts:160

• The method doesn't handle the case when input is null. Based on line 167 in the upsertUser method and the type definition, GetOrCreateInstitutionInput can be null when neither rorId nor institutionData is provided. The method should have an explicit null check at the beginning and return null in that case to avoid unexpected behavior.

  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);
    }

    return institution;
  }

apps/backend/src/mutations/UserMutations.spec.ts:272

• There's no test coverage for the case when both rorId and institutionData are null in the institution input. According to the @oneOf directive, this should be invalid, but it should be tested to ensure proper error handling. Consider adding a test case for this scenario.

describe('upsertUserByOidcSub', () => {
  test('A user can be created if OIDC sub does not exist', async () => {
    const newOidcSub = 'new-unique-oidc-sub';
    const result = await userMutations.upsertUserByOidcSub(
      dummyUserOfficerWithRole,
      {
        oidcSub: newOidcSub,
        firstName: 'New',
        lastName: 'User',
        email: 'new.user@example.com',
        userTitle: null,
        preferredName: null,
        institution: {
          rorId: 'dummy-ror-id',
          institutionData: null,
        },
      }
    );
    // Check if the result has the oidcsub
    expect(isRejection(result)).toBe(false);
    expect((result as User).oidcSub).toBe(newOidcSub);
  });
  test('A user will be updated if OIDC sub exists', async () => {
    const result = await userMutations.upsertUserByOidcSub(
      dummyUserOfficerWithRole,
      {
        oidcSub: dummyUser.oidcSub as string,
        firstName: 'UpsertedJane',
        lastName: 'UpsertedDoe',
        email: 'upserted.jane.doe@example.com',
        userTitle: null,
        preferredName: null,
        institution: {
          rorId: '',
          institutionData: null,
        },
      }
    );

    // Check if the result has the oidcsub
    expect(isRejection(result)).toBe(false);
    expect((result as User).oidcSub).toBe(dummyUser.oidcSub);
  });

  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 result = await userMutations.upsertUserByOidcSub(
      dummyUserOfficerWithRole,
      {
        oidcSub: newOidcSub,
        firstName: 'John',
        lastName: 'Scientist',
        email: 'john.scientist@dummy-research.org',
        userTitle: 'Dr.',
        preferredName: 'Johnny',
        institution: {
          rorId: existingRorId, // This should find Dummy Research Institute in our mock
          institutionData: null,
        },
      }
    );

    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);
    expect(createdUser.firstname).toBe('John');
    expect(createdUser.lastname).toBe('Scientist');
    expect(createdUser.email).toBe('john.scientist@dummy-research.org');
    expect(createdUser.oidcSub).toBe(newOidcSub);
  });

  test('A new user can be created where the institution will be created newly for the new institutionRoRId', async () => {
    const newOidcSub = 'user-with-new-institution-ror';
    const newRorId = 'https://ror.org/05a28rw58'; // New ROR ID not in our mock

    const result = await userMutations.upsertUserByOidcSub(
      dummyUserOfficerWithRole,
      {
        oidcSub: newOidcSub,
        firstName: 'Maria',
        lastName: 'Researcher',
        email: 'maria.researcher@newinstitute.edu',
        userTitle: 'Prof.',
        preferredName: 'Maria',
        institution: {
          rorId: newRorId, // This ROR ID doesn't exist in mock
          institutionData: null,
        },
      }
    );

    expect(isRejection(result)).toBe(false);
    const createdUser = result as User;

    // Should create new institution and assign it
    // The new institution should have ID 6 (next available in our mock)
    expect(createdUser.institutionId).toBe(6);
    expect(createdUser.firstname).toBe('Maria');
    expect(createdUser.lastname).toBe('Researcher');
    expect(createdUser.email).toBe('maria.researcher@newinstitute.edu');
    expect(createdUser.oidcSub).toBe(newOidcSub);
  });
});

[yoganandaness]

@jekabs-karklins Should leverage YUP to validate the input data in addition to the Graphql validation.

[jekabs-karklins]

Yes, I added that in the validation schema

[yoganandaness]

As I can see, this mutation is not in use. Could we remove it?

[jekabs-karklins]

After checking, turns out we do still use this in the InstitutionsPage
https://staging.useroffice.ess.eu/Institutions. The mutation however does not match the file name. Mutation is called UpdateInstitutionMutation

[yoganandaness]

Should this be outside assetRegistrar?

[jekabs-karklins]

I cleaned UpdateInstitutionsMutation up as well as the related tests

[yoganandaness]

@jekabs-karklins Should leverage YUP to validate the input data in addition to the Graphql validation.

[yoganandaness]

Will it add more value to validate rorID with more specific rule. Since the rorID is very standard, we can enforce the validation based on its pattern.

https://ror.readme.io/docs/identifier

Just an add-on

[jekabs-karklins]

Good idea, I have added the validation schema to make sure all user input is valid at the get-go