feat: upsert user by oidc (#1224)

Co-authored-by: Jekabs Karklins <58165815+jekabs-karklins@users.noreply.github.com>
Co-authored-by: Fredrik Bolmsten <fredrik.bolmsten@ess.eu>

Author: 3 people

apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql

@@ -0,0 +1,65 @@
+
+DO
+$$
+DECLARE
+    duplicate_count INTEGER;
+    duplicate_details TEXT;
+BEGIN
+    IF register_patch('AddUniqueConstraintToOidcSub.sql', 'yoganandanpandiyan', 'Unique constraint', '2025-10-13') THEN
+        BEGIN
+            -- Clean up empty string values in oidc_sub column by setting them to NULL
+            -- This prevents empty strings from conflicting with the unique constraint
+            UPDATE users
+            SET oidc_sub = NULL
+            WHERE oidc_sub = '';
+
+            -- Check for duplicate oidc_sub values and throw error if any are found
+            SELECT COUNT(*)
+            INTO duplicate_count
+            FROM (
+                SELECT oidc_sub
+                FROM users
+                WHERE oidc_sub IS NOT NULL
+                GROUP BY oidc_sub
+                HAVING COUNT(*) > 1
+            ) duplicates;
+
+            IF duplicate_count > 0 THEN
+                WITH duplicate_groups AS (
+                    SELECT 
+                        oidc_sub,
+                        STRING_AGG(user_id::TEXT, ', ' ORDER BY user_id) AS user_ids
+                    FROM users
+                    WHERE oidc_sub IS NOT NULL
+                      AND oidc_sub IN (
+                          SELECT oidc_sub
+                          FROM users
+                          WHERE oidc_sub IS NOT NULL
+                          GROUP BY oidc_sub
+                          HAVING COUNT(*) > 1
+                      )
+                    GROUP BY oidc_sub
+                )
+                SELECT STRING_AGG(
+                    FORMAT('oidc_sub: "%s" (user_ids: %s)', oidc_sub, user_ids), 
+                    E'\n'
+                )
+                INTO duplicate_details
+                FROM duplicate_groups;
+
+                RAISE EXCEPTION 'Cannot add unique constraint to oidc_sub column. Found % duplicate oidc_sub value(s) that must be manually resolved:
+%
+
+Please manually fix these duplicate values before running this migration again. 
+OIDC subject identifiers should be unique as they come from identity providers.',
+                    duplicate_count, duplicate_details;
+            END IF;
+
+            -- Add unique constraint to prevent duplicate oidc_sub values in the future
+            ALTER TABLE users 
+            ADD CONSTRAINT unique_oidc_sub UNIQUE (oidc_sub);
+        END;
+    END IF;
+END;
+$$
+LANGUAGE plpgsql;

apps/backend/src/auth/OAuthAuthorization.ts

@@ -16,7 +16,7 @@ import { SettingsId } from '../models/Settings';
 import { AuthJwtPayload, User, UserRole } from '../models/User';
 import { UserAuthorization } from './UserAuthorization';
 
-interface UserinfoResponseWithInssitution extends UserinfoResponse {
+interface UserinfoResponseWithInstitution extends UserinfoResponse {
   institution_ror_id?: string;
   institution_name?: string;
   institution_country?: string;
@@ -85,11 +85,11 @@ export class OAuthAuthorization extends UserAuthorization {
     });
   }
 
-  private async getUserInstitutionId(
-    userInfo: UserinfoResponseWithInssitution
+  public async getOrCreateUserInstitution(
+    userInfo: UserinfoResponseWithInstitution
   ) {
     if (!userInfo.institution_name || !userInfo.institution_country) {
-      return undefined;
+      return null;
     }
 
     let institution = userInfo.institution_ror_id
@@ -119,15 +119,15 @@ export class OAuthAuthorization extends UserAuthorization {
         await this.adminDataSource.createInstitution(newInstitution);
     }
 
-    return institution?.id;
+    return institution;
   }
 
   private async upsertUser(
     userInfo: ValidUserInfo,
     tokenSet: ValidTokenSet
   ): Promise<User> {
     const client = await OpenIdClient.getInstance();
-    const institutionId = await this.getUserInstitutionId(userInfo);
+    const institution = await this.getOrCreateUserInstitution(userInfo);
     const userWithOAuthSubMatch = await this.userDataSource.getByOIDCSub(
       userInfo.sub
     );
@@ -152,7 +152,7 @@ export class OAuthAuthorization extends UserAuthorization {
         oauthIssuer: client.issuer.metadata.issuer,
         oauthRefreshToken: tokenSet.refresh_token ?? '',
         oidcSub: userInfo.sub,
-        institutionId: institutionId ?? user.institutionId,
+        institutionId: institution?.id ?? user.institutionId,
         position: userInfo.position as string,
         preferredname: userInfo.preferred_username,
         telephone: userInfo.phone_number,
@@ -172,7 +172,7 @@ export class OAuthAuthorization extends UserAuthorization {
         client.issuer.metadata.issuer,
         userInfo.gender ?? 'unspecified',
         new Date(),
-        institutionId ?? 1,
+        institution?.id ?? 1,
         '',
         (userInfo.position as string) ?? '',
         userInfo.email,

apps/backend/src/auth/StfcUserAuthorization.ts

@@ -17,6 +17,7 @@ import {
   toStfcBasicPersonDetails,
 } from '../datasources/stfc/StfcUserDataSource';
 import { createUOWSClient } from '../datasources/stfc/UOWSClient';
+import { Institution } from '../models/Institution';
 import { Instrument } from '../models/Instrument';
 import { Rejection, rejection } from '../models/Rejection';
 import { Role, Roles } from '../models/Role';
@@ -347,4 +348,12 @@ export class StfcUserAuthorization extends UserAuthorization {
   async canBeAssignedToFap(userId: number): Promise<boolean> {
     return true;
   }
+
+  getOrCreateUserInstitution(userInfo: {
+    institution_ror_id?: string;
+    institution_name?: string;
+    institution_country?: string;
+  }): Promise<Institution | null> {
+    throw new Error('Method not implemented.');
+  }
 }

apps/backend/src/auth/UserAuthorization.ts

@@ -7,6 +7,7 @@ import { InternalReviewDataSource } from '../datasources/InternalReviewDataSourc
 import { ProposalDataSource } from '../datasources/ProposalDataSource';
 import { UserDataSource } from '../datasources/UserDataSource';
 import { VisitDataSource } from '../datasources/VisitDataSource';
+import { Institution } from '../models/Institution';
 import { Rejection } from '../models/Rejection';
 import { Role, Roles } from '../models/Role';
 import { AuthJwtPayload, User, UserWithRole } from '../models/User';
@@ -191,6 +192,12 @@ export abstract class UserAuthorization {
     iss: string | null
   ): Promise<User | null>;
 
+  abstract getOrCreateUserInstitution(userInfo: {
+    institution_ror_id?: string;
+    institution_name?: string;
+    institution_country?: string;
+  }): Promise<Institution | null>;
+
   abstract logout(token: AuthJwtPayload): Promise<string | Rejection>;
 
   abstract isExternalTokenValid(

apps/backend/src/auth/mockups/UserAuthorization.ts

@@ -1,13 +1,78 @@
 import 'reflect-metadata';
 import { injectable } from 'tsyringe';
 
+import { dummyInstitution } from '../../datasources/mockups/AdminDataSource';
 import { dummyUser } from '../../datasources/mockups/UserDataSource';
+import { Institution } from '../../models/Institution';
 import { Role } from '../../models/Role';
 import { AuthJwtPayload, User } from '../../models/User';
 import { UserAuthorization } from '../UserAuthorization';
 
 @injectable()
 export class UserAuthorizationMock extends UserAuthorization {
+  // Mock institution data for testing
+  private mockInstitutions: Institution[] = [
+    dummyInstitution,
+    new Institution(
+      3,
+      'Dummy Research Institute',
+      3,
+      'https://ror.org/dummy001'
+    ),
+    new Institution(4, 'Test University', 4, 'https://ror.org/dummy002'),
+    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<Institution | null> {
+    // 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
+    }
+
+    // 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;
+      }
+    }
+
+    // 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()
+      );
+      if (existingByName) {
+        return existingByName;
+      }
+    }
+
+    // Create new institution if not found
+    const newId = Math.max(...this.mockInstitutions.map((i) => i.id)) + 1;
+
+    const newInstitution = new Institution(
+      newId,
+      userInfo.institution_name ?? 'Unknown Institution',
+      1,
+      userInfo.institution_ror_id
+    );
+
+    // Add to mock data for subsequent calls
+    this.mockInstitutions.push(newInstitution);
+
+    return newInstitution;
+  }
   async externalTokenLogin(
     token: string,
     _redirectUri: string

apps/backend/src/datasources/mockups/UserDataSource.ts

@@ -28,6 +28,7 @@ export const basicDummyUser = new BasicUserDetails(
   false,
   'test@email.com',
   '',
+  '',
   ''
 );
 
@@ -43,6 +44,7 @@ export const basicDummyUserNotOnProposal = new BasicUserDetails(
   false,
   'test@email.com',
   '',
+  '',
   ''
 );
 
@@ -259,8 +261,21 @@ export class UserDataSourceMock implements UserDataSource {
   async addUserRole(args: AddUserRoleArgs): Promise<boolean> {
     return true;
   }
+  // Mock user storage for testing upsertUserByOidcSub
+  private mockUsers: User[] = [
+    dummyUser,
+    dummyUserNotOnProposal,
+    dummyUserOfficer,
+    dummyPlaceHolderUser,
+  ];
+
   async getByOIDCSub(oidcSub: string): Promise<User | null> {
-    return dummyUser;
+    // Check if user exists with this OIDC sub
+    const existingUser = this.mockUsers.find(
+      (user) => user.oidcSub === oidcSub
+    );
+
+    return existingUser || null;
   }
   async createInviteUser(args: CreateUserByEmailInviteArgs): Promise<number> {
     return 5;
@@ -300,6 +315,7 @@ export class UserDataSourceMock implements UserDataSource {
       false,
       'test@email.com',
       '',
+      '',
       ''
     );
   }
@@ -483,8 +499,53 @@ export class UserDataSourceMock implements UserDataSource {
     return true;
   }
 
-  async create(firstname: string, lastname: string) {
-    return dummyUser;
+  async create(
+    user_title: string | undefined,
+    firstname: string,
+    lastname: string,
+    username: string,
+    preferredname: string | undefined,
+    oidc_sub: string,
+    oauth_refresh_token: string,
+    oauth_issuer: string,
+    gender: string,
+    birthdate: Date,
+    institution_id: number,
+    department: string,
+    position: string,
+    email: string,
+    telephone: string
+  ) {
+    // Generate a new user ID
+    const newId = Math.max(...this.mockUsers.map((u) => u.id)) + 1;
+
+    const newUser = new User(
+      newId,
+      user_title || 'unspecified',
+      firstname,
+      lastname,
+      username,
+      preferredname || '',
+      oidc_sub,
+      oauth_refresh_token,
+      oauth_issuer,
+      gender || 'unspecified',
+      birthdate,
+      institution_id || 1,
+      'Test institution',
+      department,
+      position,
+      email,
+      telephone,
+      false,
+      new Date().toISOString(),
+      new Date().toISOString()
+    );
+
+    // Add to mock users collection
+    this.mockUsers.push(newUser);
+
+    return newUser;
   }
 
   async ensureDummyUserExists(userId: number): Promise<User> {