Skip to content

feat: add unique id to users to be able to match users with external user services #1307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gnyiri wants to merge 5 commits into from
Closed

feat: add unique id to users to be able to match users with external user services #1307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
24929cb
feat: add unique id to users to be able to match users with external …
Jan 12, 2026
9323fda
fix: undefined vs null mismatch
Jan 12, 2026
691c4aa
Merge branch 'develop' into add_unique_id_to_user
Jan 13, 2026
34134e8
Merge branch 'develop' into add_unique_id_to_user
gnyiri Jan 13, 2026
c6490c8
Merge branch 'develop' into add_unique_id_to_user
gnyiri Jan 13, 2026
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
11 changes: 11 additions & 0 deletions apps/backend/db_patches/0203_AlterUsersUniqueId.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
DO
$$
BEGIN
IF register_patch('AlterUsersUniqueId.sql', 'Gergely Nyiri', 'Adding column unique_id to users', '2026-01-12') THEN
BEGIN
ALTER TABLE users ADD COLUMN unique_id varchar(100) UNIQUE;

@yoganandaness yoganandaness Jan 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnyiri Hi Thanks for this PR. I have few fundamental questions and more curious about the answers.

  1. What Authentication provider are you using, that returns the value unique_id?
  2. Is this the standard field(like oidc_sub) from a particular Authentication provider, that other facility can use? How does it differ from the existing oidc_sub?
  3. How are you planning to use this new field in the future? - If this field is not very critical w.r.t business logic and just for auditing purpose, we would recommend going for a JSON column in the users table, where we could dump the OAuth userinfo and use it for auditing purpose.

@gnyiri gnyiri Jan 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. unique_id is a custom field created by our registration service (developed by ELI). It is immutable and unique and is assigned to the registered user upon registration. The Auth service (which is not the same as the registration service) will map this field in UserInfo if the user logs in.
  2. oidc_sub is a standard field in Oauth2 providers but we do not want to rely on it (due to technical reasons)
  3. we are simply using this field so that we can "join" the UOS users table with the users table in the registration service. Currently we are matching the users through email address but this is not ideal as the email addresses can be changed by the user and the email address will not be updated in UOS unless the user logs in into UOS.

I am OK with the JSON approach that could store all auxiliary, locally needed fields in the future.

@mutambaraf mutambaraf Jan 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With JSON will it be possible to enforce database constrains, l am okay with an approach we we may have a generic id that can be used as oidc_sub also.

@yoganandaness yoganandaness Jan 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification.

Just curious about the "join" you have mentioned. Is it some DB join that you are doing on outside UOS?

We are currently in the process of cleaning up the users table(PR) and its column to keep it optimised. Hence, any new column should be carefully discussed and evaluated.

I am trying to evaluate the alternative approaches, as I am only a bit concerned about the impact of the new column(unique_id), that could potentially slow down the table(Not on grand scale, but still significant) due to unique indexing.

As i understand, Since you are not going to use oidc_sub, is it possible for you to use the same column for your use case. If so, we can rename the column oidc_sub to something like auth_identifier and use it according to facility's need. I know this would result in writing your own Authentication like STFC, but that should be fine, as you already now have your special use case. Let me know your thoughts about this.

@yoganandaness yoganandaness Jan 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mutambaraf Agreed. Json is not a fit-in solution here. Thanks.

@gnyiri gnyiri Jan 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yoganandaness : not a real join rather a matching. We have a DB that is a "fusion" of multiple DB-s (UOS, registration, experiments, .etc..). We use this DB for generating reports to the MGMT, e.g. reports showing proposals per affiliation and so on. This unique_id would be needed to be able to match UOS users with the registration DB where the real user data is stored.

@gnyiri gnyiri Jan 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, our original idea was to misuse the oidc_sub column for storing our unique_id but not sure if we can force the auth service to encode the unique_id as oidc_sub in user info (and if so what is the side effect). What else we can do is to encode both unique_id and oidc_sub and change UOS code for ELI to store unique_id instead of oidc_sub.

@gnyiri gnyiri Jan 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yoganandaness: renaming oidc_sub to auth_identifier is a good idea. I think that we can move that way.

END;
END IF;
END;
$$
LANGUAGE plpgsql;
5 changes: 3 additions & 2 deletions apps/backend/src/auth/OAuthAuthorization.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
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';
Expand Down Expand Up @@ -176,7 +176,8 @@ export class OAuthAuthorization extends UserAuthorization {
'',
(userInfo.position as string) ?? '',
userInfo.email,
''
'',
userInfo.unique_id as string

@yoganandaness yoganandaness Jan 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, we need to have a fallback. Facility like ESS, would n't have unique_id returned from our OAuth server. In this case, it would be undefined here.

Could you handle this in safe way?

@gnyiri gnyiri Jan 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it with and without mapping unique_id from the Auth Server and it works safely.

);

const roleID = this.getUserRole(newUser);
Expand Down
5 changes: 3 additions & 2 deletions apps/backend/src/datasources/UserDataSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { BasicUserDetails, User, UserRole } from '../models/User';
import { AddUserRoleArgs } from '../resolvers/mutations/AddUserRoleMutation';
import { CreateUserByEmailInviteArgs } from '../resolvers/mutations/CreateUserByEmailInviteMutation';
import {
UpdateUserByOidcSubArgs,
UpdateUserByIdArgs,
UpdateUserByOidcSubArgs,
} from '../resolvers/mutations/UpdateUserMutation';
import { UsersArgs } from '../resolvers/queries/UsersQuery';

Expand Down Expand Up @@ -72,7 +72,8 @@ export interface UserDataSource {
department: string,
position: string,
email: string,
telephone: string
telephone: string,
unique_id: string | null
): Promise<User>;
ensureDummyUserExists(userId: number): Promise<User>;
ensureDummyUsersExist(userIds: number[]): Promise<User[]>;
Expand Down
2 changes: 2 additions & 0 deletions apps/backend/src/datasources/mockups/DataAccessUsersDataSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const dummyDataAccessFullUser = new User(
'jane.smith',
'Jane',
'jane.smith.oidc',
'jane.smith.uniqueid',
'refresh-token',
'oauth-issuer',
'female',
Expand All @@ -40,6 +41,7 @@ export const dummyDataAccessFullUser2 = new User(
'bob.johnson',
'Bob',
'bob.johnson.oidc',
'bob.johnson.uniqueid',
'refresh-token',
'oauth-issuer',
'male',
Expand Down
8 changes: 7 additions & 1 deletion apps/backend/src/datasources/mockups/UserDataSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const dummyUserOfficer = new User(
'JoDo',
'Hailey',
'324235',
'182082741',
'683142616',
'issuer',
'male',
Expand Down Expand Up @@ -90,6 +91,7 @@ export const dummyUser = new User(
'JaDa',
'Meta',
'12312414',
'182082741',
'568567353',
'issuer',
'male',
Expand Down Expand Up @@ -209,6 +211,7 @@ export const dummyPlaceHolderUser = new User(
'JaDa',
'Meta',
'12312414',
'182082741',
'568567353',
'issuer',
'male',
Expand All @@ -232,6 +235,7 @@ export const dummyUserNotOnProposal = new User(
'NoDO',
'Damion',
'182082741',
'182082741',
'Apricot',
'issuer',
'female',
Expand Down Expand Up @@ -514,7 +518,8 @@ export class UserDataSourceMock implements UserDataSource {
department: string,
position: string,
email: string,
telephone: string
telephone: string,
unique_id: string | null
) {
// Generate a new user ID
const newId = Math.max(...this.mockUsers.map((u) => u.id)) + 1;
Expand All @@ -527,6 +532,7 @@ export class UserDataSourceMock implements UserDataSource {
username,
preferredname || '',
oidc_sub,
unique_id,
oauth_refresh_token,
oauth_issuer,
gender || 'unspecified',
Expand Down
4 changes: 3 additions & 1 deletion apps/backend/src/datasources/postgres/UserDataSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,8 @@ export default class PostgresUserDataSource implements UserDataSource {
department: string,
position: string,
email: string,
telephone: string
telephone: string,
unique_id: string | null
): Promise<User> {
return database
.insert({
Expand All @@ -414,6 +415,7 @@ export default class PostgresUserDataSource implements UserDataSource {
position,
email,
telephone,
unique_id,
})
.returning(['*'])
.into('users')
Expand Down
4 changes: 3 additions & 1 deletion apps/backend/src/datasources/postgres/records.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
ProposalPdfTemplateRecord,
ExperimentSafetyPdfTemplateRecord,
ProposalPdfTemplateRecord,
} from 'knex/types/tables';

import { EmailTemplateId } from '../../eventHandlers/email/essEmailHandler';
Expand Down Expand Up @@ -251,6 +251,7 @@ export interface UserRecord {
readonly username: string;
readonly preferredname: string;
readonly oidc_sub: string | null;
readonly unique_id: string | null;
readonly oauth_refresh_token: string | null;
readonly oauth_issuer: string | null;
readonly gender: string;
Expand Down Expand Up @@ -971,6 +972,7 @@ export const createUserObject = (user: UserRecord) => {
user.username,
user.preferredname,
user.oidc_sub,
user.unique_id,
user.oauth_refresh_token,
user.oauth_issuer,
user.gender,
Expand Down
1 change: 1 addition & 0 deletions apps/backend/src/datasources/stfc/StfcUserDataSource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ function toEssUser(stfcUser: StfcBasicPersonDetails): User {
stfcUser.email ?? '',
stfcUser.firstNameKnownAs ?? stfcUser.givenName,
'',
null,
'',
'',
'',
Expand Down
1 change: 1 addition & 0 deletions apps/backend/src/models/User.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export class User {
public username: string,
public preferredname: string | undefined,
public oidcSub: string | null,
public uniqueId: string | null,
public oauthRefreshToken: string | null,
public oauthIssuer: string | null,
public gender: string,
Expand Down
7 changes: 4 additions & 3 deletions apps/backend/src/mutations/UserMutations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import {
import { AddUserRoleArgs } from '../resolvers/mutations/AddUserRoleMutation';
import { CreateUserByEmailInviteArgs } from '../resolvers/mutations/CreateUserByEmailInviteMutation';
import {
UpdateUserRolesArgs,
UpdateUserByOidcSubArgs,
UpdateUserByIdArgs,
UpdateUserByOidcSubArgs,
UpdateUserRolesArgs,
} from '../resolvers/mutations/UpdateUserMutation';
import { UpsertUserByOidcSubArgs } from '../resolvers/mutations/UpsertUserMutation';
import { signToken, verifyToken } from '../utils/jwt';
Expand Down Expand Up @@ -591,7 +591,8 @@ export default class UserMutations {
department ?? '',
position,
email,
telephone ?? ''
telephone ?? '',
null
);

await this.dataSource.addUserRole({
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/src/resolvers/mutations/UpdateUserMutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {
Arg,
Args,
ArgsType,
Ctx,
Field,
Int,
Mutation,
Resolver,
Arg,
} from 'type-graphql';

import { ResolverContext } from '../../context';
Expand Down
Loading