Skip to content

Commit

Permalink
Merge pull request #3827 from uselagoon/group-attributes-db-fixes
Browse files Browse the repository at this point in the history 
fix: use api-db for project and org group attributes instead of keycloak
  • Loading branch information
@tobybellwood
tobybellwood authored Oct 22, 2024
2 parents 3ca81ad + 47e16dd commit c253d6d
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 103 deletions.
77 changes: 30 additions & 47 deletions services/api/src/models/group.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ interface GroupEdit {
attributes?: object;
}

interface AttributeFilterFn {
(attribute: { name: string; value: string[] }): boolean;
}

export class GroupExistsError extends Error {
constructor(message: string) {
super(message);
Expand All @@ -102,25 +98,6 @@ export class GroupNotFoundError extends Error {
// Group types that are for internal use only.
const internalGroupTypes = [GroupType.ROLE_SUBGROUP];

const attrLens = R.lensPath(['attributes']);
const lagoonProjectsLens = R.lensPath(['lagoon-projects']);

const attrLagoonProjectsLens = R.compose(
// @ts-ignore
attrLens,
lagoonProjectsLens,
R.lensPath([0])
);

const getProjectIdsFromGroup = R.pipe(
// @ts-ignore
R.view(attrLagoonProjectsLens),
R.defaultTo(''),
R.split(','),
R.reject(R.isEmpty),
R.map(id => parseInt(id, 10))
);

export const isRoleSubgroup = R.pathEq(
['attributes', 'type', 0],
'role-subgroup'
Expand Down Expand Up @@ -156,7 +133,7 @@ export interface GroupModel {
loadGroupByName: (name: string) => Promise<Group>
loadGroupByIdOrName: (groupInput: GroupInput) => Promise<Group>
loadParentGroup: (groupInput: Group) => Promise<Group>
createGroupFromKeycloak: (group: KeycloakLagoonGroup) => SparseGroup
createGroupFromKeycloak: (group: KeycloakLagoonGroup, projectIdsArray: number[], organizationAttr: number) => SparseGroup
getProjectsFromGroupAndParents: (group: Group) => Promise<number[]>
getProjectsFromGroupAndSubgroups: (group: Group) => Promise<number[]>
getProjectsFromGroup: (group: Group) => Promise<number[]>
Expand Down Expand Up @@ -186,17 +163,17 @@ export const Group = (clients: {
keycloakGroups: GroupRepresentation[]
): Promise<Group[]> => {
// Map from keycloak object to group object
const groups = keycloakGroups.map(
(keycloakGroup: GroupRepresentation): Group => ({
const groups = await Promise.all(keycloakGroups.map(
async (keycloakGroup: GroupRepresentation): Promise<Group> => ({
id: keycloakGroup.id,
name: keycloakGroup.name,
type: attributeKVOrNull('type', keycloakGroup),
path: keycloakGroup.path,
attributes: keycloakGroup.attributes,
subGroups: keycloakGroup.subGroups,
organization: parseInt(attributeKVOrNull('lagoon-organization', keycloakGroup), 10) || null, // if it exists set it or null
organization: await groupHelpers(sqlClientPool).selectOrganizationIdByGroupId(keycloakGroup.id), // if it exists set it or null
})
);
));

let groupsWithGroupsAndMembers = [];

Expand All @@ -216,17 +193,8 @@ export const Group = (clients: {
return groupsWithGroupsAndMembers;
};

const createGroupFromKeycloak = (group: KeycloakLagoonGroup): SparseGroup => {
const createGroupFromKeycloak = (group: KeycloakLagoonGroup, projectsArray: number[], organizationAttr: number): SparseGroup => {
const groupAttr = group.attributes?.['type']?.[0];
const projectsAttr = group.attributes?.['lagoon-projects']?.[0];
const projectsArray = projectsAttr
? projectsAttr
.split(',')
.map((id) => id.trim())
.filter((id) => !!id)
.map(toNumber)
: [];
const organizationAttr = group.attributes?.['lagoon-organization']?.[0];

if (!group.id) {
throw new Error('Missing group id');
Expand All @@ -239,7 +207,7 @@ export const Group = (clients: {
return {
id: group.id,
name: group.name,
organization: organizationAttr ? toNumber(organizationAttr) : null,
organization: organizationAttr,
parentGroupId: group.parentId ?? null,
projects: new Set(projectsArray),
subGroupCount: group.subGroupCount ?? 0,
Expand All @@ -257,7 +225,11 @@ export const Group = (clients: {
throw new GroupNotFoundError(`Group not found: ${id}`);
}

return createGroupFromKeycloak(keycloakGroup);
// get the projectids from the group_project database table instead of group attributes
const projectIds = await groupHelpers(sqlClientPool).selectProjectIdsByGroupID(keycloakGroup.id);
const organizationId = await groupHelpers(sqlClientPool).selectOrganizationIdByGroupId(keycloakGroup.id);

return createGroupFromKeycloak(keycloakGroup, projectIds, organizationId);
};

/**
Expand Down Expand Up @@ -324,7 +296,11 @@ export const Group = (clients: {
);
}

return createGroupFromKeycloak(keycloakGroups[0]);
// get the projectids from the group_project database table instead of group attributes
const projectIds = await groupHelpers(sqlClientPool).selectProjectIdsByGroupID(keycloakGroups[0].id);
const organizationId = await groupHelpers(sqlClientPool).selectOrganizationIdByGroupId(keycloakGroups[0].id);

return createGroupFromKeycloak(keycloakGroups[0], projectIds, organizationId);
};

/**
Expand Down Expand Up @@ -456,7 +432,11 @@ export const Group = (clients: {

let subGroups: HieararchicalGroup[] = [];
for (const keycloakGroup of keycloakGroups) {
const subGroup = createGroupFromKeycloak(keycloakGroup);
// get the projectids from the group_project database table instead of group attributes
const projectIds = await groupHelpers(sqlClientPool).selectProjectIdsByGroupID(keycloakGroup.id);
const organizationId = await groupHelpers(sqlClientPool).selectOrganizationIdByGroupId(keycloakGroup.id);

const subGroup = createGroupFromKeycloak(keycloakGroup, projectIds, organizationId);

const groupWithSubgroups = await loadSubGroups(subGroup, depth ? depth -1 : null);
subGroups.push({
Expand Down Expand Up @@ -526,7 +506,8 @@ export const Group = (clients: {
const getProjectsFromGroupAndParents = async (
group: Group
): Promise<number[]> => {
const projectIds = getProjectIdsFromGroup(group);
// get the projectids from the group_project database table instead of group attributes
const projectIds = await groupHelpers(sqlClientPool).selectProjectIdsByGroupID(group.id);

const parentGroup = await loadParentGroup(group);
const parentProjectIds = parentGroup
Expand All @@ -545,7 +526,8 @@ export const Group = (clients: {
group: Group
): Promise<number[]> => {
try {
const groupProjectIds = getProjectIdsFromGroup(group);
// get the projectids from the group_project database table instead of group attributes
const groupProjectIds = await groupHelpers(sqlClientPool).selectProjectIdsByGroupID(group.id);

let subGroupProjectIds = [];
// @TODO: check is `groups.groups` ever used? it always appears to be empty
Expand Down Expand Up @@ -577,7 +559,8 @@ export const Group = (clients: {
group: Group
): Promise<number[]> => {
try {
const groupProjectIds = getProjectIdsFromGroup(group);
// get the projectids from the group_project database table instead of group attributes
const groupProjectIds = await groupHelpers(sqlClientPool).selectProjectIdsByGroupID(group.id);
// remove deleted projects from the result to prevent null errors in user queries
const existingProjects = await projectHelpers(sqlClientPool).getAllProjectsIn(groupProjectIds);
let existingProjectsIds = [];
Expand Down Expand Up @@ -913,10 +896,10 @@ export const Group = (clients: {
if (group.type) {
attributes.type = [group.type];
}
// lagoon-organization and lagoon-projects/project-ids attributes are added for legacy reasons only, theses values are stored in the api-db now
if (group.organization) {
attributes['lagoon-organization'] = [group.organization.toString()];
}

attributes['lagoon-projects'] = [Array.from(group.projects).join(',')];
attributes['group-lagoon-project-ids'] = [
`{${JSON.stringify(group.name)}:[${Array.from(group.projects).join(',')}]}`,
Expand Down Expand Up @@ -957,10 +940,10 @@ export const Group = (clients: {
if (group.type) {
attributes.type = [group.type];
}
// lagoon-organization and lagoon-projects/project-ids attributes are added for legacy reasons only, theses values are stored in the api-db now
if (group.organization) {
attributes['lagoon-organization'] = [group.organization.toString()];
}

attributes['lagoon-projects'] = [Array.from(group.projects).join(',')];
attributes['group-lagoon-project-ids'] = [
`{${JSON.stringify(group.name)}:[${Array.from(group.projects).join(',')}]}`,
Expand Down
12 changes: 8 additions & 4 deletions services/api/src/models/user.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { toNumber } from '../util/func';
import { Group, GroupType, KeycloakLagoonGroup } from './group';
import { Sql } from '../resources/user/sql';
import { getConfigFromEnv } from '../util/config';
import { Helpers as groupHelpers } from '../resources/group/helpers';

interface IUserAttributes {
comment?: [string];
Expand Down Expand Up @@ -356,9 +357,12 @@ export const User = (clients: {
id: userId,
briefRepresentation: false,
})) as KeycloakLagoonGroup[];
const roleSubgroups = keycloakGroups.map(
GroupModel.createGroupFromKeycloak,
);
let roleSubgroups = [];
for (const keycloakGroup of keycloakGroups){
const projectIds = await groupHelpers(sqlClientPool).selectProjectIdsByGroupID(keycloakGroup.id);
const organizationId = await groupHelpers(sqlClientPool).selectOrganizationIdByGroupId(keycloakGroup.id);
roleSubgroups.push(GroupModel.createGroupFromKeycloak(keycloakGroup, projectIds, organizationId))
}

let userGroups: {
[key: string]: Group;
Expand All @@ -370,7 +374,7 @@ export const User = (clients: {

if (!userGroups[ug.parentGroupId]) {
const parentGroup = await GroupModel.loadGroupById(ug.parentGroupId);
const parentOrg = parentGroup.attributes?.['lagoon-organization']?.[0];
const parentOrg = await groupHelpers(sqlClientPool).selectOrganizationIdByGroupId(parentGroup.id);
if (organization && parentOrg && toNumber(parentOrg) != organization) {
continue;
}
Expand Down
35 changes: 30 additions & 5 deletions services/api/src/resources/group/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,24 @@ import { logger } from '../../loggers/logger';

export const Helpers = (sqlClientPool: Pool) => {
return {
selectProjectIdsByGroupIDs: async (groupIds: string[]) => {
selectProjectIdsByGroupIDs: async (groupIds: string[]): Promise<number[]> => {
const projectIdsArray = await query(
sqlClientPool,
Sql.selectProjectIdsByGroupIDs(groupIds)
)
if (projectIdsArray[0]["projectIds"] != null) {
const values = projectIdsArray[0]["projectIds"].split(',');
const values = projectIdsArray[0]["projectIds"].split(',').map(Number);
return values
}
return []
},
selectProjectIdsByGroupID: async (groupId: string) => {
selectProjectIdsByGroupID: async (groupId: string): Promise<number[]> => {
const projectIdsArray = await query(
sqlClientPool,
Sql.selectProjectIdsByGroupID(groupId)
)
if (projectIdsArray[0]["projectIds"] != null) {
const values = projectIdsArray[0]["projectIds"].split(',');
const values = projectIdsArray[0]["projectIds"].split(',').map(Number);
return values
}
return []
Expand Down Expand Up @@ -68,10 +68,24 @@ export const Helpers = (sqlClientPool: Pool) => {
return []
},
selectOrganizationByGroupId: async (groupId: string) => {
return await query(
const organization = await query(
sqlClientPool,
Sql.selectOrganizationByGroupId(groupId)
)
if (organization[0] != null) {
return organization[0]
}
return null
},
selectOrganizationIdByGroupId: async (groupId: string) => {
const organization = await query(
sqlClientPool,
Sql.selectOrganizationByGroupId(groupId)
)
if (organization[0] != null) {
return organization[0].organizationId
}
return null
},
removeProjectFromGroup: async (projectId: number, groupId: string) => {
await query(
Expand Down Expand Up @@ -99,6 +113,17 @@ export const Helpers = (sqlClientPool: Pool) => {
return null
}
},
removeGroupFromOrganization: async (groupId: string) => {
try {
// delete the reference for the group from the group_organization table
await query(
sqlClientPool,
Sql.deleteOrganizationGroup(groupId)
);
} catch (err) {
return null
}
},
deleteGroup: async (groupId: string) => {
await query(
sqlClientPool,
Expand Down
Loading

0 comments on commit c253d6d

Please sign in to comment.