Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLU-324: transfer tiles ownership #771

Open
wants to merge 2 commits into
base: develop-v2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/backend/src/errors/graphql-errors/forbidden.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { GraphQLError } from 'graphql/error'

const FORBIDDEN_ERROR_CODE = 'FORBIDDEN'

export class ForbiddenError extends GraphQLError {
constructor(message: string) {
super(message, {
extensions: {
code: FORBIDDEN_ERROR_CODE,
message,
http: {
status: 403,
},
},
})
}
}
1 change: 1 addition & 0 deletions packages/backend/src/errors/graphql-errors/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './bad-user-input'
export * from './forbidden'
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, it } from 'vitest'

import { ForbiddenError } from '@/errors/graphql-errors'
import createRow from '@/graphql/mutations/tiles/create-row'
import TableMetadata from '@/models/table-metadata'
import User from '@/models/user'
Expand Down Expand Up @@ -112,6 +113,6 @@ describe('create row mutation', () => {
},
context,
),
).rejects.toThrow('You do not have access to this tile')
).rejects.toThrow(ForbiddenError)
})
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, it } from 'vitest'

import { ForbiddenError } from '@/errors/graphql-errors'
import createRows from '@/graphql/mutations/tiles/create-rows'
import { getTableRows } from '@/models/dynamodb/table-row/functions'
import TableMetadata from '@/models/table-metadata'
Expand Down Expand Up @@ -118,6 +119,6 @@ describe('create row mutation', () => {
},
context,
),
).rejects.toThrow('You do not have access to this tile')
).rejects.toThrow(ForbiddenError)
})
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, it } from 'vitest'

import { ForbiddenError } from '@/errors/graphql-errors'
import deleteRows from '@/graphql/mutations/tiles/delete-rows'
import { createTableRows, getTableRowCount } from '@/models/dynamodb/table-row'
import TableMetadata from '@/models/table-metadata'
Expand Down Expand Up @@ -100,6 +101,6 @@ describe('delete rows mutation', () => {
{ input: { tableId: dummyTable.id, rowIds: slicedRows } },
context,
),
).rejects.toThrow('You do not have access to this tile')
).rejects.toThrow(ForbiddenError)
})
})
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { randomUUID } from 'crypto'
import { beforeEach, describe, expect, it } from 'vitest'

import { ForbiddenError } from '@/errors/graphql-errors'
import deleteTable from '@/graphql/mutations/tiles/delete-table'
import TableMetadata from '@/models/table-metadata'
import User from '@/models/user'
Expand Down Expand Up @@ -64,11 +65,11 @@ describe('delete table mutation', () => {
context.currentUser = editor
await expect(
deleteTable(null, { input: { id: dummyTable.id } }, context),
).rejects.toThrow('You do not have access to this tile')
).rejects.toThrow(ForbiddenError)

context.currentUser = viewer
await expect(
deleteTable(null, { input: { id: dummyTable.id } }, context),
).rejects.toThrow('You do not have access to this tile')
).rejects.toThrow(ForbiddenError)
})
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, it } from 'vitest'

import { ForbiddenError } from '@/errors/graphql-errors'
import updateRow from '@/graphql/mutations/tiles/update-row'
import { createTableRow, TableRow } from '@/models/dynamodb/table-row'
import TableMetadata from '@/models/table-metadata'
Expand Down Expand Up @@ -185,6 +186,6 @@ describe('update row mutation', () => {
},
context,
),
).rejects.toThrow('You do not have access to this tile')
).rejects.toThrow(ForbiddenError)
})
})
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { randomUUID } from 'crypto'
import { beforeEach, describe, expect, it } from 'vitest'

import { ForbiddenError } from '@/errors/graphql-errors'
import updateTable from '@/graphql/mutations/tiles/update-table'
import TableMetadata from '@/models/table-metadata'
import User from '@/models/user'
Expand Down Expand Up @@ -313,6 +314,6 @@ describe('update table mutation', () => {
},
context,
),
).rejects.toThrow('You do not have access to this tile')
).rejects.toThrow(ForbiddenError)
})
})
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, it } from 'vitest'

import { BadUserInputError, ForbiddenError } from '@/errors/graphql-errors'
import TableMetadata from '@/models/table-metadata'
import User from '@/models/user'
import Context from '@/types/express/context'
Expand Down Expand Up @@ -93,22 +94,6 @@ describe('update table collaborators', () => {
expect(addedCollaborator).toHaveProperty('role', 'viewer')
})

it('should not allow adding of owner role', async () => {
await expect(
upsertTableCollaborator(
null,
{
input: {
tableId: dummyTable.id,
email: '[email protected]',
role: 'owner',
},
},
context,
),
).rejects.toThrowError('Cannot set collaborator role as owner')
})

it('should not allow editing role of owner', async () => {
context.currentUser = editor
await expect(
Expand Down Expand Up @@ -173,6 +158,66 @@ describe('update table collaborators', () => {
},
context,
),
).rejects.toThrow('You do not have access to this tile')
).rejects.toThrow(ForbiddenError)
})

describe('transfer ownership', () => {
it('should not allow adding of owner role if you are not owner', async () => {
context.currentUser = editor
await expect(
upsertTableCollaborator(
null,
{
input: {
tableId: dummyTable.id,
email: '[email protected]',
role: 'owner',
},
},
context,
),
).rejects.toThrowError(ForbiddenError)
})

it('should allow transfer of owner role if you are owner, old owner will become editor', async () => {
await expect(
upsertTableCollaborator(
null,
{
input: {
tableId: dummyTable.id,
email: editor.email,
role: 'owner',
},
},
context,
),
).resolves.not.toThrow()
const collaborators = await dummyTable
.$relatedQuery('collaborators')
.where('table_collaborators.deleted_at', null)
expect(
collaborators.find((col) => col.email === editor.email),
).toHaveProperty('role', 'owner')
expect(
collaborators.find((col) => col.email === owner.email),
).toHaveProperty('role', 'editor')
})

it('should not allow transfer of ownership to non-existent user', async () => {
await expect(
upsertTableCollaborator(
null,
{
input: {
tableId: dummyTable.id,
email: '[email protected]',
role: 'owner',
},
},
context,
),
).rejects.toThrowError(BadUserInputError)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getOrCreateUser } from '@/helpers/auth'
import { validateAndParseEmail } from '@/helpers/email-validator'
import logger from '@/helpers/logger'
import TableCollaborator from '@/models/table-collaborators'
import User from '@/models/user'

import type { MutationResolvers } from '../../__generated__/types.generated'

Expand All @@ -25,14 +26,40 @@ const upsertTableCollaborator: MutationResolvers['upsertTableCollaborator'] =
throw new BadUserInputError('Cannot change own role')
}

let collaboratorUser: User

if (role === 'owner') {
throw new BadUserInputError('Cannot set collaborator role as owner')
/**
* If transferring ownership, current user must be owner
* and new user must exist
*/
await TableCollaborator.hasAccess(
context.currentUser.id,
tableId,
'owner',
)
collaboratorUser = await User.query().findOne({ email })
if (!collaboratorUser) {
throw new BadUserInputError(
'User not found. You can only transfer to users who have logged in at least once.',
)
}
} else {
/**
* If mere adding collaborator, current user be editor
* and new user will be created if not exists
*/
await TableCollaborator.hasAccess(
context.currentUser.id,
tableId,
'editor',
)
collaboratorUser = await getOrCreateUser(validatedEmail)
if (!collaboratorUser) {
throw new BadUserInputError('Error creating user')
}
}

await TableCollaborator.hasAccess(context.currentUser.id, tableId, 'editor')

const collaboratorUser = await getOrCreateUser(validatedEmail)

try {
/**
* We check if a table collaborator has been added before (could have been soft deleted)
Expand All @@ -46,6 +73,9 @@ const upsertTableCollaborator: MutationResolvers['upsertTableCollaborator'] =
user_id: collaboratorUser.id,
})
.withSoftDeleted()
/**
* Upsert collaborator here
*/
if (existingCollaborator) {
if (existingCollaborator.role === 'owner') {
throw new BadUserInputError('Cannot change owner role')
Expand All @@ -64,6 +94,17 @@ const upsertTableCollaborator: MutationResolvers['upsertTableCollaborator'] =
role,
})
}

/**
* When transferring ownership, we need to make sure the current owner is set to editor
*/
if (role === 'owner') {
await TableCollaborator.query(trx)
.where({ table_id: tableId, user_id: context.currentUser.id })
.update({
role: 'editor',
})
}
})
} catch (e) {
logger.error({
Expand All @@ -76,7 +117,7 @@ const upsertTableCollaborator: MutationResolvers['upsertTableCollaborator'] =
userId: context.currentUser.id,
error: e,
})
throw new Error('Failed to upsert collaborator')
throw new Error(e.message ?? 'Failed to upsert collaborator')
}

return true
Expand Down
5 changes: 3 additions & 2 deletions packages/backend/src/models/table-collaborators.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { IGlobalVariable, ITableCollabRole } from '@plumber/types'

import { ForbiddenError } from '@/errors/graphql-errors'
import StepError from '@/errors/step'

import Base from './base'
Expand Down Expand Up @@ -70,15 +71,15 @@ class TableCollaborator extends Base {
) {
if ($) {
throw new StepError(
'You do not have access to this tile',
'You do not sufficient permissions to this tile',
`Please ensure that you are ${
role === 'viewer' ? 'a' : 'an'
} ${role} of this tile.`,
$.step.position,
$.app.name,
)
}
throw new Error('You do not have access to this tile.')
throw new ForbiddenError('You do not sufficient permissions to this tile')
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions packages/frontend/src/graphql/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

if (graphQLErrors) {
graphQLErrors.forEach(({ message, locations, path }) => {
console.log('message', message)

Check warning on line 31 in packages/frontend/src/graphql/link.ts

View workflow job for this annotation

GitHub Actions / Test and Build

Unexpected console statement
if (autoSnackbar) {
callback?.(message)
}
Expand All @@ -45,9 +46,7 @@
window.location.href = URLS.UNAUTHORIZED_TILE
}
})
}

if (networkError) {
} else if (networkError) {
if (autoSnackbar) {
callback?.(networkError.toString())
}
Expand Down
Loading
Loading