1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-08-13 13:48:59 +02:00

Fix/project role permission grant (#8084) (#8132)

Cherry pick the fix merged in #8084

Co-authored-by: Fredrik Strand Oseberg <fredrikstrandoseberg@Fredrik-sin-MacBook-Pro.local>
This commit is contained in:
Fredrik Strand Oseberg 2024-09-11 18:12:48 +02:00 committed by GitHub
parent b7301dc3dd
commit 36e780bdb9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 247 additions and 43 deletions

1
.gitignore vendored
View File

@ -3,6 +3,7 @@ lerna-debug
npm-debug
.DS_Store
/dist
.vscode
# Logs
logs

View File

@ -0,0 +1,121 @@
import { canGrantProjectRole } from './can-grant-project-role';
describe('canGrantProjectRole', () => {
test('should return true if the granter has all the permissions the receiver needs', () => {
const granterPermissions = [
{
project: 'test',
environment: undefined,
permission: 'CREATE_FEATURE',
},
{
project: 'test',
environment: 'production',
permission: 'UPDATE_FEATURE_ENVIRONMENT',
},
{
project: 'test',
environment: 'production',
permission: 'APPROVE_CHANGE_REQUEST',
},
];
const receiverPermissions = [
{
id: 28,
name: 'UPDATE_FEATURE_ENVIRONMENT',
environment: 'production',
displayName: 'Enable/disable flags',
type: 'environment',
},
{
id: 29,
name: 'APPROVE_CHANGE_REQUEST',
environment: 'production',
displayName: 'Enable/disable flags',
type: 'environment',
},
];
canGrantProjectRole(granterPermissions, receiverPermissions);
});
test('should return false if the granter and receiver permissions have different environments', () => {
const granterPermissions = [
{
project: 'test',
environment: 'production',
permission: 'UPDATE_FEATURE_ENVIRONMENT',
},
{
project: 'test',
environment: 'production',
permission: 'APPROVE_CHANGE_REQUEST',
},
];
const receiverPermissions = [
{
id: 28,
name: 'UPDATE_FEATURE_ENVIRONMENT',
environment: 'development',
displayName: 'Enable/disable flags',
type: 'environment',
},
{
id: 29,
name: 'APPROVE_CHANGE_REQUEST',
environment: 'development',
displayName: 'Enable/disable flags',
type: 'environment',
},
];
expect(
canGrantProjectRole(granterPermissions, receiverPermissions),
).toBeFalsy();
});
test('should return false if the granter does not have all receiver permissions', () => {
const granterPermissions = [
{
project: 'test',
environment: 'production',
permission: 'UPDATE_FEATURE_ENVIRONMENT',
},
{
project: 'test',
environment: 'production',
permission: 'APPROVE_CHANGE_REQUEST',
},
];
const receiverPermissions = [
{
id: 28,
name: 'UPDATE_FEATURE_ENVIRONMENT',
environment: 'production',
displayName: 'Enable/disable flags',
type: 'environment',
},
{
id: 29,
name: 'APPROVE_CHANGE_REQUEST',
environment: 'production',
displayName: 'Enable/disable flags',
type: 'environment',
},
{
id: 26,
name: 'UPDATE_FEATURE_STRATEGY',
environment: 'production',
displayName: 'Update activation strategies',
type: 'environment',
},
];
expect(
canGrantProjectRole(granterPermissions, receiverPermissions),
).toBeFalsy();
});
});

View File

@ -0,0 +1,21 @@
import type { IPermission } from '../../types';
import type { IUserPermission } from '../../types/stores/access-store';
export const canGrantProjectRole = (
granterPermissions: IUserPermission[],
receiverPermissions: IPermission[],
) => {
return receiverPermissions.every((receiverPermission) => {
return granterPermissions.some((granterPermission) => {
if (granterPermission.environment) {
return (
granterPermission.permission === receiverPermission.name &&
granterPermission.environment ===
receiverPermission.environment
);
}
return granterPermission.permission === receiverPermission.name;
});
});
};

View File

@ -772,7 +772,8 @@ describe('Managing Project access', () => {
),
);
});
test('Users can not assign roles they do not have to a user through explicit roles endpoint', async () => {
test('Users can not assign roles where they do not hold the same permissions', async () => {
const project = {
id: 'user_fail_assign_to_user',
name: 'user_fail_assign_to_user',
@ -780,64 +781,83 @@ describe('Managing Project access', () => {
mode: 'open' as const,
defaultStickiness: 'clientId',
};
const auditUser = extractAuditInfoFromUser(user);
await projectService.createProject(project, user, auditUser);
const projectUser = await stores.userStore.insert({
name: 'Some project user',
email: 'fail_assign_role_to_user@example.com',
});
const projectAuditUser = extractAuditInfoFromUser(projectUser);
const secondUser = await stores.userStore.insert({
name: 'Some other user',
email: 'otheruser_no_roles@example.com',
});
const customRole = await stores.roleStore.create({
name: 'role_that_noone_has',
roleType: 'custom',
description:
'Used to prove that you can not assign a role you do not have via setRolesForUser',
});
const customRoleUserAccess = await accessService.createRole(
{
name: 'Project-permissions-lead',
description: 'Role',
permissions: [
{
name: 'PROJECT_USER_ACCESS_WRITE',
},
],
createdByUserId: SYSTEM_USER_ID,
},
SYSTEM_USER_AUDIT,
);
const customRoleUpdateEnvironments = await accessService.createRole(
{
name: 'Project Lead',
description: 'Role',
permissions: [
{
name: 'UPDATE_FEATURE_ENVIRONMENT',
environment: 'production',
},
{
name: 'CREATE_FEATURE_STRATEGY',
environment: 'production',
},
],
createdByUserId: SYSTEM_USER_ID,
},
SYSTEM_USER_AUDIT,
);
await projectService.setRolesForUser(
project.id,
projectUser.id,
[customRoleUserAccess.id],
auditUser,
);
const auditProjectUser = extractAuditInfoFromUser(projectUser);
await expect(
projectService.setRolesForUser(
project.id,
secondUser.id,
[customRole.id],
projectAuditUser,
[customRoleUpdateEnvironments.id],
auditProjectUser,
),
).rejects.toThrow(
new InvalidOperationError(
'User tried to assign a role they did not have access to',
),
);
});
test('Users can not assign roles they do not have to a group through explicit roles endpoint', async () => {
const project = {
id: 'user_fail_assign_to_group',
name: 'user_fail_assign_to_group',
description: '',
mode: 'open' as const,
defaultStickiness: 'clientId',
};
await projectService.createProject(project, user, auditUser);
const projectUser = await stores.userStore.insert({
name: 'Some project user',
email: 'fail_assign_role_to_group@example.com',
});
const projectAuditUser = extractAuditInfoFromUser(projectUser);
const group = await stores.groupStore.create({
name: 'Some group_awaiting_role',
});
const customRole = await stores.roleStore.create({
name: 'role_that_noone_has_fail_assign_group',
roleType: 'custom',
description:
'Used to prove that you can not assign a role you do not have via setRolesForGroup',
});
return expect(
await expect(
projectService.setRolesForGroup(
project.id,
group.id,
[customRole.id],
projectAuditUser,
[customRoleUpdateEnvironments.id],
auditProjectUser,
),
).rejects.toThrow(
new InvalidOperationError(

View File

@ -89,6 +89,7 @@ import { throwExceedsLimitError } from '../../error/exceeds-limit-error';
import type EventEmitter from 'events';
import type { ApiTokenService } from '../../services/api-token-service';
import type { TransitionalProjectData } from './project-read-model-type';
import { canGrantProjectRole } from './can-grant-project-role';
type Days = number;
type Count = number;
@ -940,16 +941,39 @@ export default class ProjectService {
userAddingAccess.id,
projectId,
);
if (
this.isAdmin(userAddingAccess.id, userRoles) ||
this.isProjectOwner(userRoles, projectId)
) {
return true;
}
// Users may have access to multiple projects, so we need to filter out the permissions based on this project.
// Since the project roles are just collections of permissions that are not tied to a project in the database
// not filtering here might lead to false positives as they may have the permission in another project.
if (this.flagResolver.isEnabled('projectRoleAssignment')) {
const filteredUserPermissions = userPermissions.filter(
(permission) => permission.project === projectId,
);
const rolesToBeAssignedData = await Promise.all(
rolesBeingAdded.map((role) => this.accessService.getRole(role)),
);
const rolesToBeAssignedPermissions = rolesToBeAssignedData.flatMap(
(role) => role.permissions,
);
return canGrantProjectRole(
filteredUserPermissions,
rolesToBeAssignedPermissions,
);
} else {
return rolesBeingAdded.every((roleId) =>
userRoles.some((userRole) => userRole.id === roleId),
);
}
}
async addAccess(
projectId: string,

View File

@ -13,7 +13,10 @@ import { createRequestSchema } from '../../../openapi/util/create-request-schema
import { createResponseSchema } from '../../../openapi/util/create-response-schema';
import { meSchema, type MeSchema } from '../../../openapi/spec/me-schema';
import { serializeDates } from '../../../types/serialize-dates';
import type { IUserPermission } from '../../../types/stores/access-store';
import type {
IRole,
IUserPermission,
} from '../../../types/stores/access-store';
import type { PasswordSchema } from '../../../openapi/spec/password-schema';
import {
emptyResponse,
@ -28,6 +31,7 @@ import {
rolesSchema,
type RolesSchema,
} from '../../../openapi/spec/roles-schema';
import type { IFlagResolver } from '../../../types';
class UserController extends Controller {
private accessService: AccessService;
@ -42,6 +46,8 @@ class UserController extends Controller {
private projectService: ProjectService;
private flagResolver: IFlagResolver;
constructor(
config: IUnleashConfig,
{
@ -68,6 +74,7 @@ class UserController extends Controller {
this.userSplashService = userSplashService;
this.openApiService = openApiService;
this.projectService = projectService;
this.flagResolver = config.flagResolver;
this.route({
method: 'get',
@ -174,10 +181,15 @@ class UserController extends Controller {
): Promise<void> {
const { projectId } = req.query;
if (projectId) {
const roles = await this.accessService.getAllProjectRolesForUser(
let roles: IRole[];
if (this.flagResolver.isEnabled('projectRoleAssignment')) {
roles = await this.accessService.getProjectRoles();
} else {
roles = await this.accessService.getAllProjectRolesForUser(
req.user.id,
projectId,
);
}
this.openApiService.respondWithValidation(
200,
res,

View File

@ -62,7 +62,8 @@ export type IFlagKey =
| 'useProjectReadModel'
| 'addonUsageMetrics'
| 'onboardingMetrics'
| 'onboardingUI';
| 'onboardingUI'
| 'projectRoleAssignment';
export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>;
@ -307,6 +308,10 @@ const flags: IFlags = {
process.env.UNLEASH_EXPERIMENTAL_ONBOARDING_UI,
false,
),
projectRoleAssignment: parseEnvVarBoolean(
process.env.UNLEASH_EXPERIMENTAL_PROJECT_ROLE_ASSIGNMENT,
false,
),
};
export const defaultExperimentalOptions: IExperimentalOptions = {