Add validation for IP Access in Role settings (#21444)

This commit is contained in:
Daniel Biegler
2024-02-19 23:19:55 +01:00
committed by GitHub
parent 1e656c627c
commit d577b44231
3 changed files with 108 additions and 12 deletions

View File

@@ -0,0 +1,5 @@
---
'@directus/api': patch
---
Added validation for IP Access in role settings

View File

@@ -2,7 +2,7 @@ import { ForbiddenError, UnprocessableContentError } from '@directus/errors';
import type { SchemaOverview } from '@directus/types';
import type { Knex } from 'knex';
import knex from 'knex';
import { createTracker, MockClient, Tracker, type RawQuery } from 'knex-mock-client';
import { MockClient, Tracker, createTracker, type RawQuery } from 'knex-mock-client';
import {
afterEach,
beforeAll,
@@ -11,8 +11,8 @@ import {
expect,
it,
vi,
type MockedFunction,
type MockInstance,
type MockedFunction,
} from 'vitest';
import { ItemsService, PermissionsService, PresetsService, RolesService, UsersService } from './index.js';
@@ -697,6 +697,12 @@ describe('Integration Tests', () => {
await service.createOne({});
expect(checkForOtherAdminRolesSpy).not.toBeCalled();
});
it('should throw due to invalid ip_access', async () => {
await expect(service.createOne({ ip_access: ['invalid_ip'] })).rejects.toThrow(
'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
);
});
});
describe('createMany', () => {
@@ -704,6 +710,12 @@ describe('Integration Tests', () => {
await service.createMany([{}]);
expect(checkForOtherAdminRolesSpy).not.toBeCalled();
});
it('should throw due to invalid ip_access', async () => {
await expect(service.createMany([{ ip_access: ['invalid_ip'] }])).rejects.toThrow(
'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
);
});
});
describe('updateOne', () => {
@@ -723,6 +735,12 @@ describe('Integration Tests', () => {
expect(checkForOtherAdminRolesSpy).toBeCalledTimes(1);
expect(checkForOtherAdminUsersSpy).toBeCalledTimes(1);
});
it('should throw due to invalid ip_access', async () => {
await expect(service.updateOne(1, { ip_access: ['invalid_ip'] })).rejects.toThrow(
'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
);
});
});
describe('updateMany', () => {
@@ -735,6 +753,12 @@ describe('Integration Tests', () => {
await service.updateMany([1], { admin_access: false });
expect(checkForOtherAdminRolesSpy).toBeCalledTimes(1);
});
it('should throw due to invalid ip_access', async () => {
await expect(service.updateMany([1], { ip_access: ['invalid_ip'] })).rejects.toThrow(
'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
);
});
});
describe('updateBatch', () => {
@@ -747,6 +771,12 @@ describe('Integration Tests', () => {
await service.updateBatch([{ id: 1, admin_access: false }]);
expect(checkForOtherAdminRolesSpy).toBeCalledTimes(1);
});
it('should throw due to invalid ip_access', async () => {
await expect(service.updateBatch([{ id: 1, ip_access: ['invalid_ip'] }])).rejects.toThrow(
'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
);
});
});
describe('updateByQuery', () => {
@@ -763,6 +793,12 @@ describe('Integration Tests', () => {
await service.updateByQuery({}, { admin_access: false });
expect(checkForOtherAdminRolesSpy).toBeCalledTimes(1);
});
it('should throw due to invalid ip_access', async () => {
await expect(service.updateByQuery({}, { ip_access: ['invalid_ip'] })).rejects.toThrow(
'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
);
});
});
describe('deleteOne', () => {

View File

@@ -1,6 +1,7 @@
import { ForbiddenError, UnprocessableContentError } from '@directus/errors';
import { ForbiddenError, InvalidPayloadError, UnprocessableContentError } from '@directus/errors';
import type { Query, User } from '@directus/types';
import type { AbstractServiceOptions, Alterations, MutationOptions, PrimaryKey } from '../types/index.js';
import { getMatch } from 'ip-matching';
import type { AbstractServiceOptions, Alterations, Item, MutationOptions, PrimaryKey } from '../types/index.js';
import { ItemsService } from './items.js';
import { PermissionsService } from './permissions.js';
import { PresetsService } from './presets.js';
@@ -149,7 +150,50 @@ export class RolesService extends ItemsService {
return;
}
override async updateOne(key: PrimaryKey, data: Record<string, any>, opts?: MutationOptions): Promise<PrimaryKey> {
private isIpAccessValid(value?: any[] | null): boolean {
if (value === undefined) return false;
if (value === null) return true;
if (Array.isArray(value) && value.length === 0) return true;
for (const ip of value) {
if (typeof ip !== 'string' || ip.includes('*')) return false;
try {
const match = getMatch(ip);
if (match.type == 'IPMask') return false;
} catch {
return false;
}
}
return true;
}
private assertValidIpAccess(partialItem: Partial<Item>): void {
if ('ip_access' in partialItem && !this.isIpAccessValid(partialItem['ip_access'])) {
throw new InvalidPayloadError({
reason: 'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
});
}
}
override async createOne(data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey> {
this.assertValidIpAccess(data);
return super.createOne(data, opts);
}
override async createMany(data: Partial<Item>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
for (const partialItem of data) {
this.assertValidIpAccess(partialItem);
}
return super.createMany(data, opts);
}
override async updateOne(key: PrimaryKey, data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey> {
this.assertValidIpAccess(data);
try {
if ('users' in data) {
await this.checkForOtherAdminUsers(key, data['users']);
@@ -161,9 +205,12 @@ export class RolesService extends ItemsService {
return super.updateOne(key, data, opts);
}
override async updateBatch(data: Record<string, any>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
const primaryKeyField = this.schema.collections[this.collection]!.primary;
override async updateBatch(data: Partial<Item>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
for (const partialItem of data) {
this.assertValidIpAccess(partialItem);
}
const primaryKeyField = this.schema.collections[this.collection]!.primary;
const keys = data.map((item) => item[primaryKeyField]);
const setsToNoAdmin = data.some((item) => item['admin_access'] === false);
@@ -178,11 +225,9 @@ export class RolesService extends ItemsService {
return super.updateBatch(data, opts);
}
override async updateMany(
keys: PrimaryKey[],
data: Record<string, any>,
opts?: MutationOptions,
): Promise<PrimaryKey[]> {
override async updateMany(keys: PrimaryKey[], data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey[]> {
this.assertValidIpAccess(data);
try {
if ('admin_access' in data && data['admin_access'] === false) {
await this.checkForOtherAdminRoles(keys);
@@ -194,6 +239,16 @@ export class RolesService extends ItemsService {
return super.updateMany(keys, data, opts);
}
override async updateByQuery(
query: Query,
data: Partial<Item>,
opts?: MutationOptions | undefined,
): Promise<PrimaryKey[]> {
this.assertValidIpAccess(data);
return super.updateByQuery(query, data, opts);
}
override async deleteOne(key: PrimaryKey): Promise<PrimaryKey> {
await this.deleteMany([key]);
return key;