diff --git a/src/Asset/Asset.model.ts b/src/Asset/Asset.model.ts index 910f6b4e..328e510d 100644 --- a/src/Asset/Asset.model.ts +++ b/src/Asset/Asset.model.ts @@ -32,11 +32,37 @@ export class Asset extends Model { return counts[0].count > 0 } - static findByIds(ids: string[]) { - return this.query(SQL` - SELECT * + static findByIds(ids: string[], ethAddresses?: string[]) { + const query = SQL` + SELECT a.* FROM ${SQL.raw(this.tableName)} a` + + if (ethAddresses) { + query.append(SQL` + INNER JOIN ${SQL.raw(AssetPack.tableName)} ap ON a.asset_pack_id = ap.id + WHERE a.id = ANY(${ids}) + AND ap.eth_address = ANY(${ethAddresses}) + AND ap.is_deleted = FALSE`) + } else { + query.append(SQL` + INNER JOIN ${SQL.raw(AssetPack.tableName)} ap ON a.asset_pack_id = ap.id + WHERE a.id = ANY(${ids}) + AND ap.is_deleted = FALSE`) + } + + return this.query(query) + } + + static async existsAnyWithADifferentAssetPackId( + ids: string[], + assetPackId: string + ): Promise { + const counts = await this.query(SQL` + SELECT COUNT(*) as count FROM ${SQL.raw(this.tableName)} - WHERE id = ANY(${ids})`) + WHERE id = ANY(${ids}) + AND asset_pack_id != ${assetPackId}`) + + return counts[0].count > 0 } static findByAssetPackId(assetPackId: string, limit: number | null = null) { diff --git a/src/Asset/Asset.router.spec.ts b/src/Asset/Asset.router.spec.ts new file mode 100644 index 00000000..d441f296 --- /dev/null +++ b/src/Asset/Asset.router.spec.ts @@ -0,0 +1,149 @@ +import { ExpressApp } from '../common/ExpressApp' +import { AssetRouter } from './Asset.router' +import { Asset } from './Asset.model' +import { getDefaultEthAddress } from '../AssetPack/utils' + +jest.mock('./Asset.model') +jest.mock('../AssetPack/utils') + +const anOwnerAddress = 'anOwnerAddress' + +const anAsset = { + id: 'anAssetId', + asset_pack_id: 'anAssetPackId', + name: 'anAsset', + model: 'model.glb', + category: 'decorations', + contents: {}, + tags: [], + metrics: { + triangles: 0, + materials: 0, + textures: 0, + meshes: 0, + bodies: 0, + entities: 0, + }, +} + +const anotherAsset = { + id: 'anotherAssetId', + asset_pack_id: 'anotherAssetPackId', + name: 'anotherAsset', + model: 'another.glb', + category: 'decorations', + contents: {}, + tags: [], + metrics: { + triangles: 0, + materials: 0, + textures: 0, + meshes: 0, + bodies: 0, + entities: 0, + }, +} + +describe('Asset router', () => { + let router: AssetRouter + let req: { + params: Record + query: Record + auth: { ethAddress: string } + } + + beforeEach(() => { + router = new AssetRouter(new ExpressApp()) + req = { params: {}, query: {}, auth: { ethAddress: anOwnerAddress } } + }) + + afterEach(() => { + jest.resetAllMocks() + }) + + describe('when getting a single asset', () => { + beforeEach(() => { + req.params = { id: anAsset.id } + }) + + describe('when the asset belongs to the caller', () => { + beforeEach(() => { + ;(Asset.findByIds as jest.Mock).mockResolvedValueOnce([anAsset]) + }) + + it('should call findByIds with the caller and default addresses', async () => { + await (router as any).getAsset(req) + expect(Asset.findByIds).toHaveBeenCalledWith( + [anAsset.id], + [anOwnerAddress, getDefaultEthAddress()] + ) + }) + + it('should return the asset', async () => { + const result = await (router as any).getAsset(req) + expect(result).toEqual(anAsset) + }) + }) + + describe('when the asset does not belong to the caller or the default address', () => { + beforeEach(() => { + ;(Asset.findByIds as jest.Mock).mockResolvedValueOnce([]) + }) + + it('should return null', async () => { + const result = await (router as any).getAsset(req) + expect(result).toBeNull() + }) + }) + }) + + describe('when getting multiple assets', () => { + beforeEach(() => { + req.query = { id: [anAsset.id, anotherAsset.id] } + }) + + describe('when all assets belong to the caller or the default address', () => { + beforeEach(() => { + ;(Asset.findByIds as jest.Mock).mockResolvedValueOnce([ + anAsset, + anotherAsset, + ]) + }) + + it('should call findByIds with the caller and default addresses', async () => { + await (router as any).getAssets(req) + expect(Asset.findByIds).toHaveBeenCalledWith( + [anAsset.id, anotherAsset.id], + [anOwnerAddress, getDefaultEthAddress()] + ) + }) + + it('should return all visible assets', async () => { + const result = await (router as any).getAssets(req) + expect(result).toEqual([anAsset, anotherAsset]) + }) + }) + + describe('when some assets belong to another user', () => { + beforeEach(() => { + ;(Asset.findByIds as jest.Mock).mockResolvedValueOnce([anAsset]) + }) + + it('should return only the visible assets', async () => { + const result = await (router as any).getAssets(req) + expect(result).toEqual([anAsset]) + }) + }) + + describe('when all assets belong to another user', () => { + beforeEach(() => { + ;(Asset.findByIds as jest.Mock).mockResolvedValueOnce([]) + }) + + it('should return an empty array', async () => { + const result = await (router as any).getAssets(req) + expect(result).toEqual([]) + }) + }) + }) +}) diff --git a/src/Asset/Asset.router.ts b/src/Asset/Asset.router.ts index 182b8490..3c60a6c4 100644 --- a/src/Asset/Asset.router.ts +++ b/src/Asset/Asset.router.ts @@ -10,8 +10,11 @@ import { } from '../middleware' import { getUploader, S3Content } from '../S3' import { AssetPack } from '../AssetPack' +import { getDefaultEthAddress } from '../AssetPack/utils' import { Asset } from './Asset.model' -import { withAuthentication } from '../middleware/authentication' +import { withAuthentication, AuthRequest } from '../middleware/authentication' + +const DEFAULT_ETH_ADDRESS = getDefaultEthAddress() export class AssetRouter extends Router { mount() { @@ -54,6 +57,7 @@ export class AssetRouter extends Router { this.router.get( '/assets/:id', withCors, + withAuthentication, withAssetExists, server.handleRequest(this.getAsset) ) @@ -61,7 +65,12 @@ export class AssetRouter extends Router { /** * Get a multiple assets */ - this.router.get('/assets', withCors, server.handleRequest(this.getAssets)) + this.router.get( + '/assets', + withCors, + withAuthentication, + server.handleRequest(this.getAssets) + ) } async assetBelongsToPackMiddleware(req: Request) { @@ -80,14 +89,20 @@ export class AssetRouter extends Router { // This handler is only here so `server.handleRequest` has a valid callback and it can return the appropiate formated response } - private getAsset(req: Request) { + private async getAsset(req: AuthRequest) { const id = server.extractFromReq(req, 'id') - return Asset.findOne(id) + const eth_address = req.auth.ethAddress + const [asset] = await Asset.findByIds( + [id], + [eth_address, DEFAULT_ETH_ADDRESS] + ) + return asset ?? null } - private getAssets(req: Request) { + private async getAssets(req: AuthRequest) { const reqIds = server.extractFromReq(req, 'id') const ids: string[] = Array.isArray(reqIds) ? reqIds : [reqIds] - return Asset.findByIds(ids) + const eth_address = req.auth.ethAddress + return Asset.findByIds(ids, [eth_address, DEFAULT_ETH_ADDRESS]) } } diff --git a/src/AssetPack/AssetPack.router.spec.ts b/src/AssetPack/AssetPack.router.spec.ts index 5c890427..97f67006 100644 --- a/src/AssetPack/AssetPack.router.spec.ts +++ b/src/AssetPack/AssetPack.router.spec.ts @@ -2,9 +2,11 @@ import { ILoggerComponent } from '@well-known-components/interfaces' import { ExpressApp } from '../common/ExpressApp' import { AssetPackRouter } from './AssetPack.router' import { AssetPack } from './AssetPack.model' +import { Asset } from '../Asset' import { getDefaultEthAddress } from './utils' jest.mock('./AssetPack.model') +jest.mock('../Asset/Asset.model') const anAssetPack = { id: 'anId', @@ -375,4 +377,85 @@ describe('AssetPack router', () => { }) }) }) + + describe('when upserting an asset pack', () => { + const anAssetPackId = '8c251928-fb34-40e9-86d2-868a60d2fa78' + const anotherAssetPackId = '49c9ae13-6779-4ced-b208-dd352c9b7541' + const anOwnerAddress = 'anOwnerAddress' + let upsertReq: any + let assetUpsertSpy: jest.SpyInstance + let upsertedAssetAttrs: any[] + + beforeEach(() => { + upsertedAssetAttrs = [] + const mockUpsert = jest.fn().mockResolvedValue({}) + ;((Asset as unknown) as jest.Mock).mockImplementation((attrs: any) => { + upsertedAssetAttrs.push(attrs) + return { upsert: mockUpsert, attributes: attrs } + }) + assetUpsertSpy = mockUpsert + ;(AssetPack.count as jest.Mock).mockResolvedValue(0) + ;(Asset.existsAnyWithADifferentEthAddress as jest.Mock).mockResolvedValue( + false + ) + ;(Asset.existsAnyWithADifferentAssetPackId as jest.Mock).mockResolvedValue( + false + ) + ;(AssetPack.findOneWithAssets as jest.Mock).mockResolvedValue(null) + ;((AssetPack as unknown) as jest.Mock).mockImplementation( + (attrs: any) => ({ + upsert: jest.fn().mockResolvedValue(attrs), + attributes: attrs, + }) + ) + }) + + describe('when an asset already belongs to a different asset pack', () => { + beforeEach(() => { + ;(Asset.existsAnyWithADifferentAssetPackId as jest.Mock).mockResolvedValue( + true + ) + upsertReq = { + params: { id: anAssetPackId }, + body: { + assetPack: { + id: anAssetPackId, + title: 'an-asset-pack', + assets: [ + { + id: '1e27cbda-5582-4219-8f83-2db817344cc1', + asset_pack_id: anotherAssetPackId, + name: 'an-asset', + model: 'model.glb', + category: 'decorations', + contents: {}, + tags: ['test'], + metrics: { + triangles: 0, + materials: 0, + textures: 0, + meshes: 0, + bodies: 0, + entities: 0, + }, + }, + ], + }, + }, + auth: { ethAddress: anOwnerAddress }, + } + }) + + it('should throw an error', async () => { + await expect(router.upsertAssetPack(upsertReq)).rejects.toThrow( + "One of the assets you're trying to upload is invalid" + ) + }) + + it('should not upsert any assets', async () => { + await expect(router.upsertAssetPack(upsertReq)).rejects.toThrow() + expect(assetUpsertSpy).not.toHaveBeenCalled() + }) + }) + }) }) diff --git a/src/AssetPack/AssetPack.router.ts b/src/AssetPack/AssetPack.router.ts index b9fcb819..cb014cfe 100644 --- a/src/AssetPack/AssetPack.router.ts +++ b/src/AssetPack/AssetPack.router.ts @@ -279,8 +279,17 @@ export class AssetPackRouter extends Router { const assetIds = assets.map((asset) => asset.id) if (await Asset.existsAnyWithADifferentEthAddress(assetIds, eth_address)) { throw new HTTPError( - "One of the assets you're trying to upload belongs to a different address. Check the ids", - { eth_address, assetIds } + "One of the assets you're trying to upload is invalid", + {}, + STATUS_CODES.badRequest + ) + } + + if (await Asset.existsAnyWithADifferentAssetPackId(assetIds, id)) { + throw new HTTPError( + "One of the assets you're trying to upload is invalid", + {}, + STATUS_CODES.badRequest ) }