diff --git a/.changeset/dull-comics-camp.md b/.changeset/dull-comics-camp.md new file mode 100644 index 0000000000..2f9a9eca83 --- /dev/null +++ b/.changeset/dull-comics-camp.md @@ -0,0 +1,5 @@ +--- +'@directus/api': patch +--- + +Added validation for IP Access in role settings diff --git a/api/src/services/roles.test.ts b/api/src/services/roles.test.ts index 5c020ea65f..94b18f2481 100644 --- a/api/src/services/roles.test.ts +++ b/api/src/services/roles.test.ts @@ -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', () => { diff --git a/api/src/services/roles.ts b/api/src/services/roles.ts index c44e344e0f..1bb0900ea7 100644 --- a/api/src/services/roles.ts +++ b/api/src/services/roles.ts @@ -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, opts?: MutationOptions): Promise { + 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): 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, opts?: MutationOptions): Promise { + this.assertValidIpAccess(data); + + return super.createOne(data, opts); + } + + override async createMany(data: Partial[], opts?: MutationOptions): Promise { + for (const partialItem of data) { + this.assertValidIpAccess(partialItem); + } + + return super.createMany(data, opts); + } + + override async updateOne(key: PrimaryKey, data: Partial, opts?: MutationOptions): Promise { + 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[], opts?: MutationOptions): Promise { - const primaryKeyField = this.schema.collections[this.collection]!.primary; + override async updateBatch(data: Partial[], opts?: MutationOptions): Promise { + 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, - opts?: MutationOptions, - ): Promise { + override async updateMany(keys: PrimaryKey[], data: Partial, opts?: MutationOptions): Promise { + 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, + opts?: MutationOptions | undefined, + ): Promise { + this.assertValidIpAccess(data); + + return super.updateByQuery(query, data, opts); + } + override async deleteOne(key: PrimaryKey): Promise { await this.deleteMany([key]); return key;