diff --git a/packages/wabe/src/database/DatabaseController.ts b/packages/wabe/src/database/DatabaseController.ts index 1de8fa09..a5b8dbe0 100644 --- a/packages/wabe/src/database/DatabaseController.ts +++ b/packages/wabe/src/database/DatabaseController.ts @@ -1,3 +1,4 @@ +import { selectFieldsWithoutPrivateFields } from 'src/utils/helper' import type { WabeTypes } from '../..' import { OperationType, initializeHook } from '../hooks' import type { SchemaInterface } from '../schema' @@ -57,7 +58,9 @@ export class DatabaseController { realClass.fields[fieldName]?.type === 'Relation', ) - return Object.entries(select).reduce( + return Object.entries( + context.isRoot ? select : selectFieldsWithoutPrivateFields(select), + ).reduce( (acc, [fieldName, value]) => { // If not pointer or relation if (!pointerOrRelationFields.includes(fieldName)) @@ -644,12 +647,8 @@ export class DatabaseController { return this.getObject({ className, - context: { - ...context, - // @ts-expect-error - user: className === 'User' ? res : context.user, - }, - select, + context: contextWithRoot(context), + select: selectFieldsWithoutPrivateFields(select), id: res.id, }) } diff --git a/packages/wabe/src/database/index.test.ts b/packages/wabe/src/database/index.test.ts index ccfd0528..e3ba3ab6 100644 --- a/packages/wabe/src/database/index.test.ts +++ b/packages/wabe/src/database/index.test.ts @@ -13,6 +13,7 @@ import type { Wabe } from '../server' import { type DevWabeTypes, getAdminUserClient, + getAnonymousClient, getGraphqlClient, } from '../utils/helper' import { setupTests, closeTests } from '../utils/testHelper' @@ -134,6 +135,60 @@ describe('Database', () => { spyGetObjects.mockClear() }) + it('should allow to signUp with anonymous user and read his own data', async () => { + const setup = await setupTests([ + { + name: 'User', + fields: {}, + searchableFields: ['email', 'firstName', 'lastName'], + permissions: { + read: { + authorizedRoles: ['Admin', 'Client'], + requireAuthentication: true, + }, + update: { + authorizedRoles: ['Admin', 'Client'], + requireAuthentication: true, + }, + delete: { + authorizedRoles: ['Admin', 'Client'], + requireAuthentication: true, + }, + }, + }, + ]) + + const client = getAnonymousClient(setup.port) + + const res = await client.request( + gql` + mutation signUpWith($input: SignUpWithInput!) { + signUpWith(input: $input) { + id + accessToken + refreshToken + csrfToken + } + } + `, + { + input: { + authentication: { + emailPassword: { + email: 'test@example.com', + password: 'password', + }, + }, + }, + }, + ) + + expect(res.signUpWith.id).toBeDefined() + expect(res.signUpWith.accessToken).toBeDefined() + expect(res.signUpWith.refreshToken).toBeDefined() + expect(res.signUpWith.csrfToken).toBeDefined() + }) + it('should return id of a relation if set to true in select on getObject', async () => { const rootClient = getGraphqlClient(wabe.config.port) diff --git a/packages/wabe/src/hooks/permissions.test.ts b/packages/wabe/src/hooks/permissions.test.ts index 6cffe7c8..7a5415a9 100644 --- a/packages/wabe/src/hooks/permissions.test.ts +++ b/packages/wabe/src/hooks/permissions.test.ts @@ -107,7 +107,7 @@ describe('Permissions', () => { select: {}, }) - expect(_checkCLP(obj, OperationType.BeforeRead)).rejects.toThrow( + expect(() => _checkCLP(obj, OperationType.BeforeRead)).toThrow( 'Permission denied to read class TestClass4', ) }) @@ -141,13 +141,13 @@ describe('Permissions', () => { select: {}, }) - expect(_checkCLP(obj, OperationType.BeforeRead)).rejects.toThrow( + expect(() => _checkCLP(obj, OperationType.BeforeRead)).toThrow( 'Permission denied to read class TestClass5', ) }) - it('should get the permission for a given className', async () => { - const permission = await _getPermissionPropertiesOfAClass({ + it('should get the permission for a given className', () => { + const permission = _getPermissionPropertiesOfAClass({ className: 'TestClass', operation: 'read', context, @@ -158,7 +158,7 @@ describe('Permissions', () => { authorizedRoles: [RoleEnum.Admin], }) - const permission2 = await _getPermissionPropertiesOfAClass({ + const permission2 = _getPermissionPropertiesOfAClass({ className: 'TestClass2', operation: 'read', context, @@ -185,7 +185,7 @@ describe('Permissions', () => { select: {}, }) - expect(_checkCLP(obj, OperationType.BeforeRead)).rejects.toThrow( + expect(() => _checkCLP(obj, OperationType.BeforeRead)).toThrow( 'Permission denied to read class TestClass', ) }) @@ -251,7 +251,7 @@ describe('Permissions', () => { select: {}, }) - expect(_checkCLP(obj, OperationType.BeforeRead)).rejects.toThrow( + expect(() => _checkCLP(obj, OperationType.BeforeRead)).toThrow( 'Permission denied to read class TestClass3', ) }) @@ -288,7 +288,7 @@ describe('Permissions', () => { select: {}, }) - expect(_checkCLP(obj, OperationType.BeforeRead)).rejects.toThrow( + expect(() => _checkCLP(obj, OperationType.BeforeRead)).toThrow( 'Permission denied to read class TestClass', ) }) @@ -351,7 +351,7 @@ describe('Permissions', () => { const spyBeforeRead = spyOn( permissions, 'defaultCheckPermissionOnRead', - ).mockResolvedValue() + ).mockReturnValue() permissions.defaultCheckPermissionOnRead({} as never) @@ -365,7 +365,7 @@ describe('Permissions', () => { const spyBeforeCreate = spyOn( permissions, 'defaultCheckPermissionOnCreate', - ).mockResolvedValue() + ).mockReturnValue() permissions.defaultCheckPermissionOnCreate({ sessionId: 'sessionId', @@ -385,7 +385,7 @@ describe('Permissions', () => { const spyBeforeUpdate = spyOn( permissions, 'defaultCheckPermissionOnUpdate', - ).mockResolvedValue() + ).mockReturnValue() permissions.defaultCheckPermissionOnUpdate({ sessionId: 'sessionId', @@ -405,7 +405,7 @@ describe('Permissions', () => { const spyBeforeDelete = spyOn( permissions, 'defaultCheckPermissionOnDelete', - ).mockResolvedValue() + ).mockReturnValue() permissions.defaultCheckPermissionOnDelete({ sessionId: 'sessionId', diff --git a/packages/wabe/src/hooks/permissions.ts b/packages/wabe/src/hooks/permissions.ts index 0e38f253..128a401f 100644 --- a/packages/wabe/src/hooks/permissions.ts +++ b/packages/wabe/src/hooks/permissions.ts @@ -38,7 +38,7 @@ export const _getPermissionPropertiesOfAClass = ({ return permission } -export const _checkCLP = async ( +export const _checkCLP = ( object: HookObject, operationType: OperationType, ) => { @@ -48,7 +48,7 @@ export const _checkCLP = async ( if (!permissionOperation) throw new Error('Bad operation type provided') - const permissionProperties = await _getPermissionPropertiesOfAClass({ + const permissionProperties = _getPermissionPropertiesOfAClass({ className: object.className, operation: permissionOperation, context: object.context, @@ -69,10 +69,11 @@ export const _checkCLP = async ( return // User is not corrected but requireAuthentication is on true - if (!sessionId || !object.getUser()) + if (!sessionId || !object.getUser()) { throw new Error( `Permission denied to ${permissionOperation} class ${object.className}`, ) + } if (permissionProperties.authorizedRoles?.includes('everyone')) return @@ -88,10 +89,11 @@ export const _checkCLP = async ( const roleName = object.context.user?.role?.name // No role name found - if (!roleName) + if (!roleName) { throw new Error( `Permission denied to ${permissionOperation} class ${object.className}`, ) + } // The role of the user is not included in the authorizedRoles if (!permissionProperties.authorizedRoles?.includes(roleName)) diff --git a/packages/wabe/src/hooks/setupAcl.test.ts b/packages/wabe/src/hooks/setupAcl.test.ts index 34db9176..dcf4fb40 100644 --- a/packages/wabe/src/hooks/setupAcl.test.ts +++ b/packages/wabe/src/hooks/setupAcl.test.ts @@ -348,7 +348,7 @@ describe('setupAcl', () => { rootClient, }) - const res = await userClient.request(gql` + const setupResult = await userClient.request(gql` mutation createSetupACL8 { createSetupACL8(input: {fields: {test: "test"}}) { setupACL8 { @@ -370,20 +370,37 @@ describe('setupAcl', () => { } `) + const res = await rootClient.request(gql` + query setupACL8 { + setupACL8(id: "${setupResult.createSetupACL8.setupACL8.id}") { + id + acl { + users { + userId + read + write + } + roles { + roleId + read + write + } + } + } + } + `) + // User - expect(res.createSetupACL8.setupACL8.acl.users[0].userId).toEqual(userId) - expect(res.createSetupACL8.setupACL8.acl.users[0].read).toEqual(true) - expect(res.createSetupACL8.setupACL8.acl.users[0].write).toEqual(true) + expect(res.setupACL8.acl.users[0].userId).toEqual(userId) + expect(res.setupACL8.acl.users[0].read).toEqual(true) + expect(res.setupACL8.acl.users[0].write).toEqual(true) // Role expect( - await getRoleNameFromId( - res.createSetupACL8.setupACL8.acl.roles[0].roleId, - rootClient, - ), + await getRoleNameFromId(res.setupACL8.acl.roles[0].roleId, rootClient), ).toEqual('Client') - expect(res.createSetupACL8.setupACL8.acl.roles[0].read).toEqual(true) - expect(res.createSetupACL8.setupACL8.acl.roles[0].write).toEqual(true) + expect(res.setupACL8.acl.roles[0].read).toEqual(true) + expect(res.setupACL8.acl.roles[0].write).toEqual(true) }) it('should not update acl object if the acl function is not present in permissions in the class', async () => { @@ -469,7 +486,7 @@ describe('setupAcl', () => { }) it('should get different role id for read and write if roles are different', async () => { - const res = await rootClient.request(gql` + const setupResult = await rootClient.request(gql` mutation createSetupACL5 { createSetupACL5(input: {fields: {test: "test"}}) { setupACL5 { @@ -491,21 +508,35 @@ describe('setupAcl', () => { } `) - expect(res.createSetupACL5.setupACL5.acl.users).toHaveLength(0) + const res = await rootClient.request(gql` + query setupACL5 { + setupACL5(id: "${setupResult.createSetupACL5.setupACL5.id}") { + id + acl { + users { + userId + read + write + } + roles { + roleId + read + write + } + } + } + } + `) + + expect(res.setupACL5.acl.users).toHaveLength(0) // Role expect( - await getRoleNameFromId( - res.createSetupACL5.setupACL5.acl.roles[0].roleId, - rootClient, - ), + await getRoleNameFromId(res.setupACL5.acl.roles[0].roleId, rootClient), ).toEqual('Client') expect( - await getRoleNameFromId( - res.createSetupACL5.setupACL5.acl.roles[1].roleId, - rootClient, - ), + await getRoleNameFromId(res.setupACL5.acl.roles[1].roleId, rootClient), ).toEqual('Client2') }) diff --git a/packages/wabe/src/schema/Schema.ts b/packages/wabe/src/schema/Schema.ts index bcdb1a1a..07329461 100644 --- a/packages/wabe/src/schema/Schema.ts +++ b/packages/wabe/src/schema/Schema.ts @@ -8,6 +8,8 @@ import type { HookObject } from '../hooks/HookObject' import { signUpWithResolver } from '../authentication/resolvers/signUpWithResolver' import { signInWithResolver } from '../authentication/resolvers/signInWithResolver' +export const defaultPrivateFields = ['acl'] + export type WabePrimaryTypes = | 'String' | 'Int' diff --git a/packages/wabe/src/security.test.ts b/packages/wabe/src/security.test.ts index 6e1e1e8e..87cb5469 100644 --- a/packages/wabe/src/security.test.ts +++ b/packages/wabe/src/security.test.ts @@ -11,6 +11,7 @@ import { getUserClient, } from './utils/helper' import { setupTests, closeTests } from './utils/testHelper' +import { RoleEnum } from 'generated/wabe' describe('Security tests', () => { let wabe: Wabe | undefined @@ -25,6 +26,125 @@ describe('Security tests', () => { wabe = undefined }) + it('should not return private fields (acl) on createObject, getObject and getObjects if not root', async () => { + const setup = await setupTests([ + { + name: 'Secret', + fields: { + name: { type: 'String' }, + }, + permissions: { + create: { + requireAuthentication: true, + authorizedRoles: ['Client'], + }, + read: { + requireAuthentication: true, + authorizedRoles: ['Client'], + }, + acl: async (hookObject) => { + await hookObject.addACL('roles', { + read: true, + write: true, + role: RoleEnum.Client, + }) + }, + }, + }, + ]) + + wabe = setup.wabe + port = setup.port + client = getAnonymousClient(port) + rootClient = getGraphqlClient(port) + + const { userClient } = await createUserAndUpdateRole({ + anonymousClient: client, + port, + roleName: 'Client', + rootClient, + }) + + const res = await userClient.request(gql` + mutation createSecret { + createSecret( + input: { fields: { name: "n", } } + ) { + secret { + id + acl { + roles { + roleId + read + write + } + } + } + } + } + `) + + expect(res.createSecret.secret.acl).toBeNull() + + const res2 = await userClient.request(gql` + query secret { + secret(id: "${res.createSecret.secret.id}") { + id + acl { + roles { + roleId + read + write + } + } + } + } + `) + + expect(res2.secret.acl).toBeNull() + + const res3 = await userClient.request(gql` + query secrets { + secrets{ + edges { + node { + id + acl { + roles { + roleId + read + write + } + } + } + } + } + } + `) + + expect(res3.secrets.edges[0].node.acl).toBeNull() + + const res4 = await rootClient.request(gql` + query secret { + secret(id: "${res.createSecret.secret.id}") { + id + name + acl { + roles { + roleId + read + write + } + } + } + } + `) + + expect(res4.secret.acl.roles).not.toBeNull() + + await closeTests(wabe) + }) + it('should block requests without a valid CSRF token', async () => { const setup = await setupTests( [ @@ -200,7 +320,7 @@ describe('Security tests', () => { rootClient, }) - await expect( + expect( userClient.request(gql` mutation createSecret { createSecret( @@ -218,58 +338,6 @@ describe('Security tests', () => { await closeTests(wabe) }) - it('should not return created object when ACL denies read access', async () => { - const setup = await setupTests([ - { - name: 'NoReadAfterCreate', - fields: { - name: { type: 'String' }, - }, - permissions: { - create: { - requireAuthentication: true, - authorizedRoles: ['Client'], - }, - read: { - requireAuthentication: true, - authorizedRoles: ['Client'], - }, - acl: async (hookObject) => { - await hookObject.addACL('users', null) - await hookObject.addACL('roles', null) - }, - }, - }, - ]) - - wabe = setup.wabe - port = setup.port - client = getAnonymousClient(port) - rootClient = getGraphqlClient(port) - - const { userClient } = await createUserAndUpdateRole({ - anonymousClient: client, - port, - roleName: 'Client', - rootClient, - }) - - await expect( - userClient.request(gql` - mutation createNoReadAfterCreate { - createNoReadAfterCreate(input: { fields: { name: "n" } }) { - noReadAfterCreate { - id - name - } - } - } - `), - ).rejects.toThrow(/Permission denied|Object not found/) - - await closeTests(wabe) - }) - it('should apply ACL filtering to count queries', async () => { const setup = await setupTests([ { diff --git a/packages/wabe/src/utils/helper.ts b/packages/wabe/src/utils/helper.ts index c43ff1fa..df40eadb 100644 --- a/packages/wabe/src/utils/helper.ts +++ b/packages/wabe/src/utils/helper.ts @@ -7,6 +7,7 @@ import { type WabeSchemaTypes, } from '../../generated/wabe' import type { Wabe, WabeTypes } from '../server' +import { defaultPrivateFields } from 'src/schema' export interface DevWabeTypes extends WabeTypes { types: WabeSchemaTypes @@ -15,6 +16,18 @@ export interface DevWabeTypes extends WabeTypes { where: WabeSchemaWhereTypes } +export const selectFieldsWithoutPrivateFields = >( + select?: T, +): T => + Object.entries(select || {}).reduce((acc, [key, value]) => { + if (defaultPrivateFields.includes(key)) return acc + + return { + ...acc, + [key]: value, + } + }, {} as T) + export const firstLetterUpperCase = (str: string): string => str.charAt(0).toUpperCase() + str.slice(1) @@ -140,7 +153,7 @@ export const createUserAndUpdateRole = async ({ id accessToken refreshToken - csrfToken + csrfToken } } `,