Merge pull request #363 from dhis2/fix/developer-permission

fix: verify developer permission correctly on several API endpoints
dhis2 · Aug 26, 2020 · 24afc70 · 24afc70
2 parents c55573e + 5987bd0
commit 24afc70
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 18 deletions.
diff --git a/server/src/routes/v1/apps/handlers/deleteAppVersion.js b/server/src/routes/v1/apps/handlers/deleteAppVersion.js
@@ -1,11 +1,17 @@
 const Boom = require('@hapi/boom')
+const debug = require('debug')(
+    'apphub:server:routes:handlers:v1:deleteAppVersion'
+)
 
 const {
     getCurrentUserFromRequest,
     currentUserIsManager,
 } = require('../../../../security')
 
-const { getAppDeveloperId, deleteAppVersion } = require('../../../../data')
+const {
+    deleteAppVersion,
+    getOrganisationAppsByUserId,
+} = require('../../../../data')
 
 const { deleteFile } = require('../../../../utils')
 
@@ -32,11 +38,15 @@ module.exports = {
         const { appId, versionId } = request.params
 
         const currentUser = await getCurrentUserFromRequest(request, db)
-        const appDeveloperId = await getAppDeveloperId(appId, db)
 
         const isManager = currentUserIsManager(request)
+        const userApps = await getOrganisationAppsByUserId(currentUser.id, db)
+        const userCanDeleteVersion =
+            isManager || userApps.map(app => app.app_id).indexOf(appId) !== -1
 
-        if (isManager || appDeveloperId === currentUser.id) {
+        debug('isManager:', isManager)
+        debug('userCanDeleteVersion:', userCanDeleteVersion)
+        if (isManager || userCanDeleteVersion) {
             //can edit app
             const transaction = await db.transaction()
 

diff --git a/server/src/routes/v1/apps/handlers/deleteImage.js b/server/src/routes/v1/apps/handlers/deleteImage.js
@@ -1,11 +1,16 @@
 const Boom = require('@hapi/boom')
 
+const debug = require('debug')('apphub:server:routes:handlers:v1:deleteImage')
+
 const {
     getCurrentUserFromRequest,
     currentUserIsManager,
 } = require('../../../../security')
 
-const { getAppDeveloperId, deleteAppMedia } = require('../../../../data')
+const {
+    deleteAppMedia,
+    getOrganisationAppsByUserId,
+} = require('../../../../data')
 
 const { deleteFile } = require('../../../../utils')
 
@@ -32,11 +37,16 @@ module.exports = {
         const { appMediaId, appId } = request.params
 
         const currentUser = await getCurrentUserFromRequest(request, db)
-        const appDeveloperId = await getAppDeveloperId(appId, db)
-
         const isManager = currentUserIsManager(request)
 
-        if (isManager || appDeveloperId === currentUser.id) {
+        const userApps = await getOrganisationAppsByUserId(currentUser.id, db)
+        const canDeleteImage =
+            isManager || userApps.map(app => app.app_id).indexOf(appId) !== -1
+
+        debug('isManager:', isManager)
+        debug('canDeleteImage:', canDeleteImage)
+
+        if (canDeleteImage) {
             //can edit app
             const transaction = await db.transaction()
 

diff --git a/server/src/routes/v1/apps/handlers/uploadImageToApp.js b/server/src/routes/v1/apps/handlers/uploadImageToApp.js
@@ -1,3 +1,7 @@
+const debug = require('debug')(
+    'apphub:server:routes:handlers:v1:uploadImageToApp'
+)
+
 const { MediaType } = require('../../../../enums')
 const { saveFile } = require('../../../../utils')
 
@@ -6,7 +10,7 @@ const {
     currentUserIsManager,
 } = require('../../../../security')
 
-const { addAppMedia, getAppDeveloperId } = require('../../../../data')
+const { addAppMedia, getOrganisationAppsByUserId } = require('../../../../data')
 
 module.exports = {
     method: 'POST',
@@ -36,17 +40,19 @@ module.exports = {
         request.logger.info('In handler %s', request.path)
 
         const knex = h.context.db
+        const appId = request.params.appId
 
         const currentUser = await getCurrentUserFromRequest(request, knex)
-        const appDeveloperId = await getAppDeveloperId(
-            request.params.appId,
-            knex
-        )
+        const isManager = currentUserIsManager(request)
+
+        const userApps = await getOrganisationAppsByUserId(currentUser.id, knex)
+        const canUploadMedia =
+            isManager || userApps.map(app => app.app_id).indexOf(appId) !== -1
 
-        if (
-            !currentUserIsManager(request) &&
-            appDeveloperId !== currentUser.id
-        ) {
+        debug('isManager:', isManager)
+        debug('canUploadMedia:', canUploadMedia)
+
+        if (!canUploadMedia) {
             return h
                 .response({ message: `You don't have access to edit that app` })
                 .code(401)
@@ -59,8 +65,6 @@ module.exports = {
 
         let imageId = null
 
-        const appId = request.params.appId
-
         const { id } = await addAppMedia(
             {
                 userId: currentUser.id,