From 99ec74d4259accb944d2cc43cefa4cfdcbabf78c Mon Sep 17 00:00:00 2001 From: shreddedbacon Date: Fri, 22 Nov 2024 08:09:50 +1100 Subject: [PATCH] refactor: lowercase email address search to match keycloak --- services/api/src/models/user.ts | 18 ++++--- services/api/src/resources/group/resolvers.ts | 8 ++-- .../src/resources/organization/resolvers.ts | 4 +- .../api/src/resources/project/resolvers.ts | 6 +-- .../api/src/resources/sshKey/resolvers.ts | 2 +- services/api/src/resources/user/resolvers.ts | 47 +++++++++++-------- 6 files changed, 49 insertions(+), 36 deletions(-) diff --git a/services/api/src/models/user.ts b/services/api/src/models/user.ts index a0b21b296f..31e0eb5497 100644 --- a/services/api/src/models/user.ts +++ b/services/api/src/models/user.ts @@ -43,8 +43,8 @@ interface UserEdit { export interface UserModel { loadAllUsers: () => Promise; loadUserById: (id: string) => Promise; - loadUserByUsername: (email: string) => Promise; - loadUserByIdOrUsername: (userInput: UserEdit) => Promise; + loadUserByEmail: (email: string) => Promise; + loadUserByIdOrEmail: (userInput: UserEdit) => Promise; loadUsersByOrganizationId: (organizationId: number) => Promise; getAllOrganizationIdsForUser: (userInput: UserEdit) => Promise; getAllGroupsForUser: (userId: string, organization?: number) => Promise; @@ -248,7 +248,11 @@ export const User = (clients: { }; // used by project resolver only, so leave this one out of redis for now - const loadUserByUsername = async (email: string): Promise => { + const loadUserByEmail = async (email: string): Promise => { + // keycloak saves usernames and email addresses as lowercase :( + // there isn't a simple way to change that, numerous threads in SO and GH + // from various people with issues with this + email = email.toLocaleLowerCase(); const keycloakUsers = await keycloakAdminClient.users.find({ email }); @@ -270,13 +274,13 @@ export const User = (clients: { return await loadUserById(userId); }; - const loadUserByIdOrUsername = async (userInput: UserEdit): Promise => { + const loadUserByIdOrEmail = async (userInput: UserEdit): Promise => { if (R.prop('id', userInput)) { return loadUserById(R.prop('id', userInput)); } if (R.prop('email', userInput)) { - return loadUserByUsername(R.prop('email', userInput)); + return loadUserByEmail(R.prop('email', userInput)); } throw new Error('You must provide a user id or email'); @@ -708,8 +712,8 @@ export const User = (clients: { return { loadAllUsers, loadUserById, - loadUserByUsername, - loadUserByIdOrUsername, + loadUserByEmail, + loadUserByIdOrEmail, loadUsersByOrganizationId, getAllOrganizationIdsForUser, getAllGroupsForUser, diff --git a/services/api/src/resources/group/resolvers.ts b/services/api/src/resources/group/resolvers.ts index 41f17b3380..5187ad51f7 100644 --- a/services/api/src/resources/group/resolvers.ts +++ b/services/api/src/resources/group/resolvers.ts @@ -532,7 +532,7 @@ export const addUserToGroup: ResolverFn = async ( let createUserErr; try { // check if user already exists - user = await models.UserModel.loadUserByIdOrUsername({ + user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput) }); @@ -568,7 +568,7 @@ export const addUserToGroup: ResolverFn = async ( return e } } else if (createUserErr) { - // otherwise return whatever error loadUserByIdOrUsername returned + // otherwise return whatever error loadUserByIdOrEmail returned return createUserErr } } @@ -580,7 +580,7 @@ export const addUserToGroup: ResolverFn = async ( // otherwise return the error throw new Error('Cannot invite user to group that is not in an organization'); } else if (createUserErr) { - // otherwise return whatever error loadUserByIdOrUsername returned + // otherwise return whatever error loadUserByIdOrEmail returned return createUserErr } } @@ -615,7 +615,7 @@ export const removeUserFromGroup: ResolverFn = async ( throw new Error('You must provide a user id or email'); } - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput) }); diff --git a/services/api/src/resources/organization/resolvers.ts b/services/api/src/resources/organization/resolvers.ts index 52f3770145..7bcf6feef9 100644 --- a/services/api/src/resources/organization/resolvers.ts +++ b/services/api/src/resources/organization/resolvers.ts @@ -429,7 +429,7 @@ export const getUserByEmailAndOrganizationId: ResolverFn = async ( }); try { - const user = await models.UserModel.loadUserByUsername(email); + const user = await models.UserModel.loadUserByEmail(email); const queryUserGroups = await models.UserModel.getAllGroupsForUser(user.id, organization); user.owner = false @@ -945,7 +945,7 @@ export const removeUserFromOrganizationGroups: ResolverFn = async ( throw new Error('You must provide a user id or email'); } - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput) }); diff --git a/services/api/src/resources/project/resolvers.ts b/services/api/src/resources/project/resolvers.ts index 33e7fad500..6fea51e672 100644 --- a/services/api/src/resources/project/resolvers.ts +++ b/services/api/src/resources/project/resolvers.ts @@ -583,7 +583,7 @@ export const deleteProject: ResolverFn = async ( // Remove the default user try { - const user = await models.UserModel.loadUserByUsername( + const user = await models.UserModel.loadUserByEmail( `default-user@${project.name}` ); await models.UserModel.deleteUser(user.id); @@ -740,7 +740,7 @@ export const updateProject: ResolverFn = async ( keyFingerprint: keyPair.fingerprint }) ); - const user = await models.UserModel.loadUserByUsername( + const user = await models.UserModel.loadUserByEmail( `default-user@${oldProject.name}` ); await query( @@ -857,7 +857,7 @@ export const updateProject: ResolverFn = async ( } try { - const user = await models.UserModel.loadUserByUsername( + const user = await models.UserModel.loadUserByEmail( `default-user@${oldProject.name}` ); await models.UserModel.updateUser({ diff --git a/services/api/src/resources/sshKey/resolvers.ts b/services/api/src/resources/sshKey/resolvers.ts index 300498351b..e1e01ff489 100644 --- a/services/api/src/resources/sshKey/resolvers.ts +++ b/services/api/src/resources/sshKey/resolvers.ts @@ -44,7 +44,7 @@ export const addSshKey: ResolverFn = async ( throw new Error('Invalid SSH key format! Please verify keyType + keyValue'); } - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput) }); diff --git a/services/api/src/resources/user/resolvers.ts b/services/api/src/resources/user/resolvers.ts index 9f205dd2d8..400f52e14c 100644 --- a/services/api/src/resources/user/resolvers.ts +++ b/services/api/src/resources/user/resolvers.ts @@ -83,19 +83,28 @@ export const getAllUsers: ResolverFn = async ( ) => { await hasPermission('user', 'viewAll'); - const users = await models.UserModel.loadAllUsers(); - if (id) { - const filteredById = users.filter(function (item) { - return item.id === id; - }); - return filteredById; - } if (email) { - const filteredByEmail = users.filter(function (item) { - return item.email === email; - }); - return filteredByEmail; + try { + // use the model to get a user by email address instead of filtering + const user = await models.UserModel.loadUserByEmail(email); + return [user] + } catch (e) { + // if no user found, return empty user list like before + return [] + } } + if (id) { + try { + // use the model to get a user by id instead of filtering + const user = await models.UserModel.loadUserById(id); + return [user] + } catch (e) { + // if no user found, return empty user list like before + return [] + } + } + // since gitlab users are harder to check, do the all users query and then filter them + const users = await models.UserModel.loadAllUsers(); if (gitlabId) { const filteredByGitlab = users.filter(function (item) { return item.gitlabId === gitlabId; @@ -113,7 +122,7 @@ export const getUserByEmail: ResolverFn = async ( { sqlClientPool, models, hasPermission, keycloakGrant }, ) => { - const user = await models.UserModel.loadUserByUsername(email); + const user = await models.UserModel.loadUserByEmail(email); if (keycloakGrant) { if (keycloakGrant.access_token.content.sub == user.id) { await hasPermission('ssh_key', 'view:user', { @@ -157,7 +166,7 @@ export const updateUser: ResolverFn = async ( throw new Error('Input patch requires at least 1 attribute'); } - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput), }); @@ -184,7 +193,7 @@ export const resetUserPassword: ResolverFn = async ( { input: { user: userInput } }, { models, hasPermission }, ) => { - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput), }); @@ -204,7 +213,7 @@ export const deleteUser: ResolverFn = async ( { input: { user: userInput } }, { models, hasPermission }, ) => { - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput), }); @@ -238,7 +247,7 @@ export const addUserToOrganization: ResolverFn = async ( throw new Error(`Unauthorized: You don't have permission to "${scope}" on "organization"`) } - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput), }); @@ -298,7 +307,7 @@ export const removeUserFromOrganization: ResolverFn = async ( throw new Error(`Unauthorized: You don't have permission to "addOwner" on "organization"`) } - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput), }); @@ -365,7 +374,7 @@ export const addAdminToOrganization: ResolverFn = async ( throw new Error(`Unauthorized: You don't have permission to "${scope}" on "organization"`) } - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput), }); @@ -412,7 +421,7 @@ export const removeAdminFromOrganization: ResolverFn = async ( throw new Error(`Unauthorized: You don't have permission to scope on "organization"`) } - const user = await models.UserModel.loadUserByIdOrUsername({ + const user = await models.UserModel.loadUserByIdOrEmail({ id: R.prop('id', userInput), email: R.prop('email', userInput), });