fix: Update owner or manager logic (#656)

* fix: Update owner or manager logic

* revert: Item isCollectionOwner or isManager logic

* feat: Update tests

spec/utils.ts

@@ -9,10 +9,10 @@ import { Collection } from '../src/Collection'
 import { ItemCuration } from '../src/Curation/ItemCuration'
 import { Ownable } from '../src/Ownable/Ownable'
 import { Item } from '../src/Item/Item.model'
+import { ItemAttributes } from '../src/Item/Item.types'
 import { thirdPartyAPI } from '../src/ethereum/api/thirdParty'
 import { wallet } from './mocks/wallet'
 import { dbCollectionMock } from './mocks/collections'
-import { dbItemMock } from './mocks/items'
 import { tpWearableMock } from './mocks/peer'
 
 export function buildURL(
@@ -221,30 +221,32 @@ export function mockThirdPartyURNExists(
  * by mocking all the function calls to the Collection model and the TP requests.
  * This mock requires the Item model findOne method to be mocked first.
  *
- * @param id - The id of the item to be authorized.
+ * @param item - The item to be authorized.
  * @param ethAddress - The ethAddress of the user that will be requesting authorization to the item.
  * @param isThirdParty - If the mock is for a third party item.
  * @param isAuthorized - If the user should be authorized or not. This is useful to test the response of the middleware.
  */
 export function mockItemAuthorizationMiddleware(
-  id: string,
+  item: ItemAttributes,
   eth_address: string,
   isThirdParty = false,
   isAuthorized = true
 ) {
   const itemToReturn = {
-    ...dbItemMock,
+    ...item,
     urn_suffix: isThirdParty ? 'third-party' : null,
     eth_address,
   }
 
   ;(Item.findOne as jest.Mock).mockImplementationOnce((givenId) =>
-    Promise.resolve(givenId === id && isAuthorized ? itemToReturn : undefined)
+    Promise.resolve(
+      givenId === item.id && isAuthorized ? itemToReturn : undefined
+    )
   )
 
-  if (isThirdParty) {
+  if (isThirdParty && item.collection_id) {
     mockCollectionAuthorizationMiddleware(
-      dbItemMock.collection_id!,
+      item.collection_id,
       eth_address,
       isThirdParty,
       isAuthorized

src/Collection/Collection.service.ts

@@ -532,16 +532,11 @@ export class CollectionService {
     return !!remoteCollection
   }
 
-  public async isDCLManager(
-    id: string,
-    ethAddress: string
-  ): Promise<boolean> {
+  public async isDCLManager(id: string, ethAddress: string): Promise<boolean> {
     const collection = await this.getCollection(id)
 
     if (collection) {
-      return collection.managers.some(
-        manager => manager === ethAddress
-      )
+      return collection.managers.some((manager) => manager === ethAddress)
     }
 
     return false
@@ -555,7 +550,10 @@ export class CollectionService {
     if (collection && isTPCollection(collection)) {
       return thirdPartyAPI.isManager(collection.third_party_id!, ethAddress)
     } else if (collection) {
-      return collection.eth_address === ethAddress
+      return (
+        collection.eth_address === ethAddress ||
+        this.isDCLManager(id, ethAddress)
+      )
     }
 
     return false

src/Item/Item.router.spec.ts

@@ -2037,12 +2037,7 @@ describe('Item router', () => {
       describe('and the user is not authorized', () => {
         beforeEach(() => {
           mockExistsMiddleware(Item, dbItem.id)
-          mockItemAuthorizationMiddleware(
-            dbItem.id,
-            wallet.address,
-            false,
-            false
-          )
+          mockItemAuthorizationMiddleware(dbItem, wallet.address, false, false)
         })
 
         it('should respond with a 401 and a message signaling that the user is not authorized', () => {
@@ -2065,16 +2060,17 @@ describe('Item router', () => {
 
       describe("and the item doesn't have a collection", () => {
         beforeEach(() => {
-          mockExistsMiddleware(Item, dbItem.id)
+          const mockDbItem = { ...dbItem, collection_id: null }
+          mockExistsMiddleware(Item, mockDbItem.id)
           mockItemAuthorizationMiddleware(
-            dbItem.id,
+            mockDbItem,
             wallet.address,
             false,
             true
           )
           ;(Item.findOne as jest.MockedFunction<
             typeof Item.findOne
-          >).mockResolvedValueOnce({ ...dbItem, collection_id: null })
+          >).mockResolvedValueOnce(mockDbItem)
         })
 
         it('should respond with a 200 and delete the item', () => {
@@ -2096,15 +2092,13 @@ describe('Item router', () => {
       describe('and the item has a collection', () => {
         beforeEach(() => {
           mockExistsMiddleware(Item, dbItem.id)
-          mockItemAuthorizationMiddleware(
-            dbItem.id,
-            wallet.address,
-            false,
-            true
-          )
+          mockItemAuthorizationMiddleware(dbItem, wallet.address, false, true)
           ;(Item.findOne as jest.MockedFunction<
             typeof Item.findOne
           >).mockResolvedValueOnce(dbItem)
+          ;(Collection.findOne as jest.MockedFunction<
+            typeof Collection.findOne
+          >).mockResolvedValueOnce(dbCollectionMock)
           ;(Collection.findByIds as jest.MockedFunction<
             typeof Collection.findByIds
           >).mockResolvedValueOnce([
@@ -2148,12 +2142,7 @@ describe('Item router', () => {
       describe('and the collection of the item is not part of a third party collection', () => {
         beforeEach(() => {
           mockExistsMiddleware(Item, dbTPItem.id)
-          mockItemAuthorizationMiddleware(
-            dbTPItem.id,
-            wallet.address,
-            true,
-            true
-          )
+          mockItemAuthorizationMiddleware(dbTPItem, wallet.address, true, true)
           ;(Item.findOne as jest.MockedFunction<
             typeof Item.findOne
           >).mockResolvedValueOnce(dbTPItem)
@@ -2190,12 +2179,7 @@ describe('Item router', () => {
       describe('and the collection of the item is locked', () => {
         beforeEach(() => {
           mockExistsMiddleware(Item, dbTPItem.id)
-          mockItemAuthorizationMiddleware(
-            dbTPItem.id,
-            wallet.address,
-            true,
-            true
-          )
+          mockItemAuthorizationMiddleware(dbTPItem, wallet.address, true, true)
           ;(Item.findOne as jest.MockedFunction<
             typeof Item.findOne
           >).mockResolvedValueOnce(dbTPItem)
@@ -2239,12 +2223,7 @@ describe('Item router', () => {
       describe('and the item exists in the catalyst', () => {
         beforeEach(() => {
           mockExistsMiddleware(Item, dbTPItem.id)
-          mockItemAuthorizationMiddleware(
-            dbTPItem.id,
-            wallet.address,
-            true,
-            true
-          )
+          mockItemAuthorizationMiddleware(dbTPItem, wallet.address, true, true)
           ;(Item.findOne as jest.MockedFunction<
             typeof Item.findOne
           >).mockResolvedValueOnce(dbTPItem)
@@ -2282,12 +2261,7 @@ describe('Item router', () => {
       describe("and the item doesn't exist in the catalayst and the third party collection is not locked", () => {
         beforeEach(() => {
           mockExistsMiddleware(Item, dbTPItem.id)
-          mockItemAuthorizationMiddleware(
-            dbTPItem.id,
-            wallet.address,
-            true,
-            true
-          )
+          mockItemAuthorizationMiddleware(dbTPItem, wallet.address, true, true)
           ;(Item.findOne as jest.MockedFunction<
             typeof Item.findOne
           >).mockResolvedValueOnce(dbTPItem)

src/Item/Item.service.spec.ts

@@ -1,8 +1,11 @@
+import { dbCollectionMock } from '../../spec/mocks/collections'
 import { dbItemMock } from '../../spec/mocks/items'
+import { Collection } from '../Collection/Collection.model'
 import { Item } from './Item.model'
 import { ItemService } from './Item.service'
 import { ItemAttributes } from './Item.types'
 
+jest.mock('../Collection/Collection.model')
 jest.mock('./Item.model')
 
 describe('Item Service', () => {
@@ -17,6 +20,10 @@ describe('Item Service', () => {
       beforeEach(() => {
         dbItem = { ...dbItemMock, eth_address: '0xoriginalAddress' }
         ;(Item.findOne as jest.Mock).mockResolvedValueOnce(dbItem)
+        ;(Collection.findOne as jest.Mock).mockResolvedValueOnce({
+          ...dbCollectionMock,
+          eth_address: '0xoriginalAddress',
+        })
       })
 
       it('should return true', async () => {

src/Item/Item.service.ts

@@ -120,7 +120,7 @@ export class ItemService {
     const dbItem = await Item.findOne<ItemAttributes>(id)
     if (!dbItem) {
       return false
-    } else if (isTPItem(dbItem)) {
+    } else if (dbItem.collection_id) {
       return this.collectionService.isOwnedOrManagedBy(
         dbItem.collection_id,
         ethAddress
@@ -466,6 +466,7 @@ export class ItemService {
       isCollectionOwner ||
       (await new Ownable(Item).canUpsert(item.id, eth_address)) ||
       isManager
+
     if (!canUpsert) {
       throw new UnauthorizedToUpsertError(item.id, eth_address)
     }