Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
55190f1
feat: upsert user by oidc
yoganandaness Oct 13, 2025
fb3d7db
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Oct 13, 2025
1deedc1
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Oct 15, 2025
68d8a64
Changed error message and optimised error handling
yoganandaness Oct 15, 2025
0f2d3e7
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Oct 15, 2025
5328964
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Oct 15, 2025
2270958
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Oct 16, 2025
4b3004f
converted empty values to null for oidc_sub and appended unique suffi…
yoganandaness Oct 17, 2025
267b79e
fix: aligning the type of users based on the original db schema
yoganandaness Oct 19, 2025
d0c68f6
Merge branch 'develop' of github.com:UserOfficeProject/user-office-co…
yoganandaness Oct 21, 2025
3ed5764
added leftjoin to users query to return users without institutions
yoganandaness Oct 24, 2025
ef41aeb
added leftjoin to users query to return users without institutions
yoganandaness Oct 24, 2025
60cb5d6
Merge branch 'SWAP-5056-user-office-one-identity-synchronisation' of …
yoganandaness Oct 24, 2025
ec3a5c0
Merge branch 'SWAP-5056-user-office-one-identity-synchronisation' of …
yoganandaness Oct 24, 2025
75659c7
Merge branch 'SWAP-5056-user-office-one-identity-synchronisation' of …
yoganandaness Oct 24, 2025
81c6af7
Merge branch 'SWAP-5056-user-office-one-identity-synchronisation' of …
yoganandaness Oct 24, 2025
74816c4
Merge branch 'SWAP-5056-user-office-one-identity-synchronisation' of …
yoganandaness Oct 24, 2025
8724d5a
Merge branch 'SWAP-5056-user-office-one-identity-synchronisation' of …
yoganandaness Oct 24, 2025
f97f7f0
Merge branch 'SWAP-5056-user-office-one-identity-synchronisation' of …
yoganandaness Oct 24, 2025
5044060
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Oct 27, 2025
07dfeff
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
jekabs-karklins Oct 28, 2025
e128aae
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
bolmsten Oct 28, 2025
946c6a2
fixed linting issue
yoganandaness Oct 28, 2025
d470d3d
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
bolmsten Oct 29, 2025
8bc927f
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Oct 30, 2025
26755bf
throw explicit error instead of appending sequence at the end of ever…
yoganandaness Oct 31, 2025
9d423ca
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Nov 4, 2025
015fcaf
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Nov 4, 2025
42c9dd6
Merge branch 'develop' into SWAP-5056-user-office-one-identity-synchr…
yoganandaness Nov 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions apps/backend/db_patches/0201_AddUniqueConstraintToOidcSub.sql
Original file line number Diff line number Diff line change
@@ -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;
16 changes: 8 additions & 8 deletions apps/backend/src/auth/OAuthAuthorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
);
Expand All @@ -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,
Expand All @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions apps/backend/src/auth/StfcUserAuthorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.');
}
}
7 changes: 7 additions & 0 deletions apps/backend/src/auth/UserAuthorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand Down
65 changes: 65 additions & 0 deletions apps/backend/src/auth/mockups/UserAuthorization.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
67 changes: 64 additions & 3 deletions apps/backend/src/datasources/mockups/UserDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const basicDummyUser = new BasicUserDetails(
false,
'test@email.com',
'',
'',
''
);

Expand All @@ -43,6 +44,7 @@ export const basicDummyUserNotOnProposal = new BasicUserDetails(
false,
'test@email.com',
'',
'',
''
);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -300,6 +315,7 @@ export class UserDataSourceMock implements UserDataSource {
false,
'test@email.com',
'',
'',
''
);
}
Expand Down Expand Up @@ -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> {
Expand Down
Loading
Loading