Skip to content

Commit

Permalink
Merge pull request #3847 from uselagoon/email-case
Browse files Browse the repository at this point in the history 
refactor: lowercase email address search to match keycloak
  • Loading branch information
@tobybellwood
tobybellwood authored Dec 2, 2024
2 parents 91d8c13 + 99ec74d commit 4417358
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 36 deletions.
18 changes: 11 additions & 7 deletions services/api/src/models/user.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ interface UserEdit {
export interface UserModel {
loadAllUsers: () => Promise<User[]>;
loadUserById: (id: string) => Promise<User>;
loadUserByUsername: (email: string) => Promise<User>;
loadUserByIdOrUsername: (userInput: UserEdit) => Promise<User>;
loadUserByEmail: (email: string) => Promise<User>;
loadUserByIdOrEmail: (userInput: UserEdit) => Promise<User>;
loadUsersByOrganizationId: (organizationId: number) => Promise<User[]>;
getAllOrganizationIdsForUser: (userInput: UserEdit) => Promise<number[]>;
getAllGroupsForUser: (userId: string, organization?: number) => Promise<Group[]>;
Expand Down Expand Up @@ -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<User> => {
const loadUserByEmail = async (email: string): Promise<User> => {
// 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
});
Expand All @@ -270,13 +274,13 @@ export const User = (clients: {
return await loadUserById(userId);
};

const loadUserByIdOrUsername = async (userInput: UserEdit): Promise<User> => {
const loadUserByIdOrEmail = async (userInput: UserEdit): Promise<User> => {
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');
Expand Down Expand Up @@ -708,8 +712,8 @@ export const User = (clients: {
return {
loadAllUsers,
loadUserById,
loadUserByUsername,
loadUserByIdOrUsername,
loadUserByEmail,
loadUserByIdOrEmail,
loadUsersByOrganizationId,
getAllOrganizationIdsForUser,
getAllGroupsForUser,
Expand Down
8 changes: 4 additions & 4 deletions services/api/src/resources/group/resolvers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
});
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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)
});
Expand Down
4 changes: 2 additions & 2 deletions services/api/src/resources/organization/resolvers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
});
Expand Down
6 changes: 3 additions & 3 deletions services/api/src/resources/project/resolvers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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({
Expand Down
2 changes: 1 addition & 1 deletion services/api/src/resources/sshKey/resolvers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
});
Expand Down
47 changes: 28 additions & 19 deletions services/api/src/resources/user/resolvers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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', {
Expand Down Expand Up @@ -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),
});
Expand All @@ -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),
});
Expand All @@ -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),
});
Expand Down Expand Up @@ -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),
});
Expand Down Expand Up @@ -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),
});
Expand Down Expand Up @@ -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),
});
Expand Down Expand Up @@ -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),
});
Expand Down

0 comments on commit 4417358

Please sign in to comment.