diff --git a/.gitguardian.yml b/.gitguardian.yml index 837d08b2c..8de8c5ef2 100644 --- a/.gitguardian.yml +++ b/.gitguardian.yml @@ -10,4 +10,6 @@ secret: name: '.circleci/deployment/manifests/speckle-server.secret.yaml - test session_secret' - match: 9bf360c5ce31170e8e3cb30e275b2c00224dd97b93282491c60fb1665fac3845 name: local test license + - match: 7a4ab6f7bfbcc0a37aa3a0fb00fd5b6edd1d524f393a6054e242eb28f5c06be5 + name: 'packages/server/modules/core/tests/graph.spec.js - test secret' version: 2 diff --git a/packages/server/modules/auth/graph/resolvers/apps.js b/packages/server/modules/auth/graph/resolvers/apps.js index a8662c9be..975f90d7e 100644 --- a/packages/server/modules/auth/graph/resolvers/apps.js +++ b/packages/server/modules/auth/graph/resolvers/apps.js @@ -73,6 +73,12 @@ module.exports = { async appDelete(parent, args, context) { const app = await getApp({ id: args.appId }) + if (!app) { + //Possibly ould have been an UserInputError, but + //we do not want to leak the existence of any app + //the user may not own or have access to. + throw new ForbiddenError('You are not authorized to edit this app.') + } if (!app.author && context.role !== Roles.Server.Admin) throw new ForbiddenError('You are not authorized to edit this app.') diff --git a/packages/server/modules/auth/tests/apps.graphql.spec.js b/packages/server/modules/auth/tests/apps.graphql.spec.js index d5aeb163f..dacfbb0bb 100644 --- a/packages/server/modules/auth/tests/apps.graphql.spec.js +++ b/packages/server/modules/auth/tests/apps.graphql.spec.js @@ -97,6 +97,7 @@ describe('GraphQL @apps-api', () => { const res = await sendRequest(null, { query, variables }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('FORBIDDEN') }) it('Should get app info', async () => { @@ -177,12 +178,14 @@ describe('GraphQL @apps-api', () => { }) it('Should not delete app if request is not authenticated/user is app owner', async () => { - const query = `mutation del { appDelete( id: "${testAppId}" ) }` + const query = `mutation del { appDelete( appId: "${testAppId}" ) }` const res = await sendRequest(null, { query }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('FORBIDDEN') const res2 = await sendRequest(testToken2, { query }) expect(res2.body.errors).to.exist + expect(res2.body.errors[0].extensions?.code).to.equal('FORBIDDEN') }) it('Should get the apps that i have created', async () => { diff --git a/packages/server/modules/core/services/tokens.ts b/packages/server/modules/core/services/tokens.ts index 1d4c805a5..a9023125d 100644 --- a/packages/server/modules/core/services/tokens.ts +++ b/packages/server/modules/core/services/tokens.ts @@ -16,6 +16,7 @@ import { import { getTokenAppInfo } from '@/modules/core/repositories/tokens' import { Optional, ServerRoles } from '@speckle/shared' import { TokenResourceIdentifierInput } from '@/modules/core/graph/generated/graphql' +import { UserInputError } from '@/modules/core/errors/userinput' /* Tokens @@ -162,8 +163,7 @@ export async function validateToken( export async function revokeToken(tokenId: string, userId: string) { tokenId = tokenId.slice(0, 10) const delCount = await ApiTokens.knex().where({ id: tokenId, owner: userId }).del() - - if (delCount === 0) throw new Error('Did not revoke token') + if (delCount === 0) throw new UserInputError('Did not revoke token') return true } diff --git a/packages/server/modules/core/tests/graph.spec.js b/packages/server/modules/core/tests/graph.spec.js index 5f3764af2..e934d0080 100644 --- a/packages/server/modules/core/tests/graph.spec.js +++ b/packages/server/modules/core/tests/graph.spec.js @@ -16,6 +16,7 @@ const { removeStreamCollaborator } = require('@/modules/core/services/streams/streamAccessService') const { Roles, Scopes } = require('@speckle/shared') +const cryptoRandomString = require('crypto-random-string') let app let server @@ -212,6 +213,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('USER_INPUT_ERROR') }) it('Should fail to create a stream with an invalid scope token', async () => { @@ -221,6 +223,7 @@ describe('GraphQL API Core @core-api', () => { 'mutation { streamCreate(stream: { name: "INVALID TS1 (u A) Private", description: "Hello World", isPublic:false } ) }' }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('FORBIDDEN') }) it('Should edit my profile', async () => { @@ -238,7 +241,7 @@ describe('GraphQL API Core @core-api', () => { it('Should delete my account', async () => { const userDelete = { name: 'delete', - email: 'delete@speckle.systems', + email: `${cryptoRandomString({ length: 10 })}@example.org`, password: 'wowwowwowwowwow' } userDelete.id = await createUser(userDelete) @@ -264,12 +267,18 @@ describe('GraphQL API Core @core-api', () => { variables: { user: { email: 'wrongEmail@email.com' } } }) expect(badTokenScopesBadEmail.body.errors).to.exist + expect(badTokenScopesBadEmail.body.errors[0].extensions?.code).to.equal( + 'FORBIDDEN' + ) const badTokenScopesGoodEmail = await sendRequest(userDelete.token, { query: 'mutation($user:UserDeleteInput!) { userDelete( userConfirmation: $user) } ', variables: { user: { email: userDelete.email } } }) expect(badTokenScopesGoodEmail.body.errors).to.exist + expect(badTokenScopesGoodEmail.body.errors[0].extensions?.code).to.equal( + 'FORBIDDEN' + ) userDelete.token = `Bearer ${await createPersonalAccessToken( userDelete.id, @@ -293,6 +302,9 @@ describe('GraphQL API Core @core-api', () => { variables: { user: { email: 'wrongEmail@email.com' } } }) expect(goodTokenScopesBadEmail.body.errors).to.exist + expect(goodTokenScopesBadEmail.body.errors[0].extensions?.code).to.equal( + 'BAD_USER_INPUT' + ) const goodTokenScopesGoodEmail = await sendRequest(userDelete.token, { query: 'mutation($user:UserDeleteInput!) { userDelete( userConfirmation: $user) } ', @@ -330,6 +342,7 @@ describe('GraphQL API Core @core-api', () => { query: ` { otherUser(id:"${userB.id}") { id name role } }` }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('FORBIDDEN') expect(queriedUserB.body.data.otherUser.role).to.equal(Roles.Server.User) }) }) @@ -338,7 +351,7 @@ describe('GraphQL API Core @core-api', () => { it('Only admins can delete user', async () => { const userDelete = { name: 'delete', - email: 'delete@speckle.systems', + email: `${cryptoRandomString({ length: 10 })}@example.org`, password: 'wowwowwowwowwow' } userDelete.id = await createUser(userDelete) @@ -372,6 +385,7 @@ describe('GraphQL API Core @core-api', () => { const query = `mutation { adminDeleteUser( userConfirmation: { email: "${userA.email}" } ) } ` const res = await sendRequest(userA.token, { query }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('USER_INPUT_ERROR') expect(res.body.errors[0].message).to.equal( 'Cannot remove the last admin role from the server' ) @@ -492,6 +506,9 @@ describe('GraphQL API Core @core-api', () => { }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal( + 'STREAM_INVALID_ACCESS_ERROR' + ) }) it('Should fail to grant myself permissions', async () => { @@ -500,6 +517,9 @@ describe('GraphQL API Core @core-api', () => { }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal( + 'STREAM_INVALID_ACCESS_ERROR' + ) }) it('Should revoke permissions', async () => { @@ -530,6 +550,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(resNotAuth).to.be.json expect(resNotAuth.body.errors).to.exist + expect(resNotAuth.body.errors[0].extensions?.code).to.equal('FORBIDDEN') }) it('Should fail to edit/write on a public stream if no access is provided', async () => { @@ -538,6 +559,7 @@ describe('GraphQL API Core @core-api', () => { query: `mutation { streamUpdate(stream: {id:"${ts4}" name: "HACK", description: "Hello World, Again!", isPublic:false } ) }` }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('FORBIDDEN') }) it('Should fail editing a private stream if no access has been granted', async () => { @@ -546,6 +568,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('FORBIDDEN') }) it('Should fail to delete a stream because of permissions', async () => { @@ -755,6 +778,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res2).to.be.json expect(res2.body.errors).to.exist + expect(res2.body.errors[0].extensions?.code).to.equal('FORBIDDEN') }) it('Should create a read receipt', async () => { @@ -803,6 +827,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions?.code).to.equal('FORBIDDEN') const res2 = await sendRequest(userA.token, { query: @@ -934,6 +959,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('BRANCH_UPDATE_ERROR') expect(res.body.errors[0].message).to.equal('Branch not found') const res1 = await sendRequest(userC.token, { @@ -943,6 +969,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res1).to.be.json expect(res1.body.errors).to.exist + expect(res1.body.errors[0].extensions.code).to.equal('BRANCH_UPDATE_ERROR') expect(res1.body.errors[0].message).to.equal( 'Only the branch creator or stream owners are allowed to delete branches' ) @@ -1017,6 +1044,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res2).to.be.json expect(res2.body.errors).to.exist + expect(res2.body.errors[0].extensions.code).to.equal('BRANCH_UPDATE_ERROR') expect(res2.body.errors[0].message).to.equal( 'The branch ID and stream ID do not match, please check your inputs' ) @@ -1035,6 +1063,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('BRANCH_UPDATE_ERROR') expect(res.body.errors[0].message).to.equal( 'The branch ID and stream ID do not match, please check your inputs' ) @@ -1240,12 +1269,14 @@ describe('GraphQL API Core @core-api', () => { let res = await sendRequest(userB.token, { query: queryLim }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('BAD_USER_INPUT') const queryPagination = 'query { userSearch( query: "matteo", limit: 200 ) { cursor items { id name } } } ' res = await sendRequest(userB.token, { query: queryPagination }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('BAD_USER_INPUT') }) }) @@ -1682,6 +1713,7 @@ describe('GraphQL API Core @core-api', () => { const res = await sendRequest(userB.token, { query, variables }) expect(res).to.be.json expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') }) }) @@ -1749,6 +1781,7 @@ describe('GraphQL API Core @core-api', () => { // WHY NOT 401 ??? // expect( res ).to.have.status( 401 ) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1772,6 +1805,7 @@ describe('GraphQL API Core @core-api', () => { query: `query { stream(id:"${streamId}") { id name } }` }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1781,6 +1815,7 @@ describe('GraphQL API Core @core-api', () => { '{ user { streams( limit: 30 ) { totalCount cursor items { id name } } } }' }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1789,6 +1824,7 @@ describe('GraphQL API Core @core-api', () => { query: `mutation { streamDelete( id:"${streamId}")}` }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1797,6 +1833,7 @@ describe('GraphQL API Core @core-api', () => { query: `mutation { streamUpdate(stream: {id:"${streamId}" name: "HACK", description: "Hello World, Again!", isPublic:false } ) }` }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1817,6 +1854,7 @@ describe('GraphQL API Core @core-api', () => { } }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1832,6 +1870,7 @@ describe('GraphQL API Core @core-api', () => { } }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1853,6 +1892,7 @@ describe('GraphQL API Core @core-api', () => { const res = await sendRequest(archivedUser.token, { query, variables }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1865,6 +1905,7 @@ describe('GraphQL API Core @core-api', () => { variables: { input: { email: 'cabbages@speckle.systems', message: 'wow!' } } }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1879,6 +1920,7 @@ describe('GraphQL API Core @core-api', () => { }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) @@ -1897,6 +1939,7 @@ describe('GraphQL API Core @core-api', () => { variables: { myCommit: commit } }) expect(res.body.errors).to.exist + expect(res.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res.body.errors[0].message).to.equal( 'You do not have the required server role' ) diff --git a/packages/server/modules/webhooks/tests/webhooks.spec.js b/packages/server/modules/webhooks/tests/webhooks.spec.js index 808477ef8..0e44ec48b 100644 --- a/packages/server/modules/webhooks/tests/webhooks.spec.js +++ b/packages/server/modules/webhooks/tests/webhooks.spec.js @@ -280,6 +280,7 @@ describe('Webhooks @webhooks', () => { query: `mutation { webhookDelete(webhook: { id: "${webhookTwo.id}", streamId: "${streamOne.id}" } ) }` }) expect(res1.body.errors).to.exist + expect(res1.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res1.body.errors[0].message).to.equal( 'The webhook id and stream id do not match. Please check your inputs.' ) @@ -289,6 +290,7 @@ describe('Webhooks @webhooks', () => { query: `mutation { webhookUpdate(webhook: { id: "${webhookTwo.id}", streamId: "${streamOne.id}", description: "updated webhook", enabled: false }) }` }) expect(res2.body.errors).to.exist + expect(res2.body.errors[0].extensions.code).to.equal('FORBIDDEN') expect(res2.body.errors[0].message).to.equal( 'The webhook id and stream id do not match. Please check your inputs.' )