From b5ec6941704d3d47b8ff09344c29654473df931b Mon Sep 17 00:00:00 2001 From: Birk Johansson Date: Fri, 17 Mar 2023 17:08:37 +0100 Subject: [PATCH] feat(appversions): add endpoint for getting a single app-version --- server/src/models/v2/AppVersion.js | 8 +- server/src/query/executeQuery.js | 21 ++++- server/src/routes/v2/appVersions.js | 52 +++++++++++- server/src/services/appVersion.js | 35 +++++++- server/test/routes/v2/appVersions.js | 119 ++++++++++++++++++++++++--- 5 files changed, 214 insertions(+), 21 deletions(-) diff --git a/server/src/models/v2/AppVersion.js b/server/src/models/v2/AppVersion.js index 2177574ee..c73729498 100644 --- a/server/src/models/v2/AppVersion.js +++ b/server/src/models/v2/AppVersion.js @@ -1,10 +1,11 @@ const Joi = require('../../utils/CustomJoi') const { definition: defaultDefinition } = require('./Default') const { createDefaultValidator } = require('./helpers') +const { AppStatuses } = require('../../enums/index.js') const definition = defaultDefinition .append({ - appId: Joi.string(), + appId: Joi.string().guid({ version: 'uuidv4' }), version: Joi.string(), channel: Joi.string().required(), demoUrl: Joi.string().uri().allow(null, ''), @@ -13,14 +14,15 @@ const definition = defaultDefinition minDhisVersion: Joi.string().required(), maxDhisVersion: Joi.string().allow(null, ''), slug: Joi.string(), + status: Joi.string().valid(...AppStatuses), }) .alter({ - db: s => + db: (s) => s .rename('minDhisVersion', 'min_dhis2_version') .rename('maxDhisVersion', 'max_dhis2_version') .rename('demoUrl', 'demo_url'), - external: s => s.strip('slug'), + external: (s) => s.strip('slug'), }) .label('AppVersion') diff --git a/server/src/query/executeQuery.js b/server/src/query/executeQuery.js index c2bedb83c..1c5da0493 100644 --- a/server/src/query/executeQuery.js +++ b/server/src/query/executeQuery.js @@ -10,6 +10,25 @@ const defaultOptions = { pagingStrategy: pagingStrategies.WINDOW, } +const selectMethods = { + select: 'select', + first: 'first', + pluck: 'pluck', +} + +/** + * Check if the query is a select-query + * @param {*} query the knex query + * @returns true if the query is a "select"-query + */ +const isSelectQuery = (query) => { + const method = query._method + // the actual "command" is only available after the query has been executed + // through listening to the `query`-event (see https://knexjs.org/guide/interfaces.html#query-response) + // that would also make it async, so it's more flexible to check the _method property. + return selectMethods[method] !== undefined +} + /** * Executes the knex-query, applying filters and paging if present * @@ -57,7 +76,7 @@ async function executeQuery( } else if (model) { // parse if it's a "getter" - ie is a select-query // else we format it to db-format - if (query._method === 'select') { + if (isSelectQuery(query)) { result = model.parseDatabaseJson(result) } else { result = model.formatDatabaseJson(result) diff --git a/server/src/routes/v2/appVersions.js b/server/src/routes/v2/appVersions.js index ba09aac17..d1b91ec6a 100644 --- a/server/src/routes/v2/appVersions.js +++ b/server/src/routes/v2/appVersions.js @@ -1,14 +1,48 @@ -//const Boom = require('@hapi/boom') +const Boom = require('@hapi/boom') const AppVersionModel = require('../../models/v2/AppVersion') const { withPagingResultSchema, withPagingQuerySchema, } = require('../../query/Pager') const Joi = require('../../utils/CustomJoi') +const App = require('../../services/app') +const { AppStatus } = require('../../enums') const CHANNELS = ['stable', 'development', 'canary'] module.exports = [ + { + method: 'GET', + path: '/v2/appVersions/{appVersionId}', + config: { + tags: ['api', 'v2'], + response: { + modify: true, + schema: AppVersionModel.externalDefinition, + }, + validate: { + params: Joi.object({ + appVersionId: Joi.string() + .guid({ version: 'uuidv4' }) + .required(), + }), + }, + }, + handler: async (request, h) => { + const { db } = h.context + const { appVersionId } = request.params + const { appVersionService } = request.services(true) + + const setDownloadUrl = + appVersionService.createSetDownloadUrl(request) + + const version = await appVersionService.findOne(appVersionId, db) + await checkVersionAccess(version, request, db) + + setDownloadUrl(version) + return version + }, + }, { method: 'GET', path: '/v2/apps/{appId}/versions', @@ -67,3 +101,19 @@ module.exports = [ }, }, ] + +async function checkVersionAccess(version, request, db) { + // check if the user is allowed to see the app + if (version.status !== AppStatus.APPROVED) { + const user = request.getUser() + const canSeeApp = + user && + (user.isManager || + (await App.canEditApp(user.id, version.appId, db))) + + if (!canSeeApp) { + // TODO: should this return a 404 instead of a 403? (to avoid leaking info) + throw Boom.forbidden() + } + } +} diff --git a/server/src/services/appVersion.js b/server/src/services/appVersion.js index 5c6a57f4b..7f74e8b51 100644 --- a/server/src/services/appVersion.js +++ b/server/src/services/appVersion.js @@ -2,8 +2,9 @@ const Schmervice = require('@hapipal/schmervice') const AppVersionModel = require('../models/v2/AppVersion') const { executeQuery } = require('../query/executeQuery') const { getServerUrl } = require('../utils') +const { NotFoundError } = require('../utils/errors') -const getAppVersionQuery = knex => +const getAppVersionQuery = (knex) => knex('app_version') .innerJoin( knex.ref('app_version_localised').as('avl'), @@ -33,16 +34,42 @@ const getAppVersionQuery = knex => knex.ref('ac.min_dhis2_version').as('minDhisVersion'), knex.ref('ac.max_dhis2_version').as('maxDhisVersion'), knex.ref('avl.slug'), - knex.ref('app_version.download_count').as('downloadCount') + knex.ref('app_version.download_count').as('downloadCount'), + knex.ref('as.status').as('status') ) .where('language_code', 'en') // only english is supported for now .orderBy('app_version.created_at', 'desc') +const TABLE_NAME = 'app_version' + class AppVersionService extends Schmervice.Service { constructor(server, schmerviceOptions) { super(server, schmerviceOptions) } + async findOneByColumn( + columnValue, + { tableName = TABLE_NAME, columnName = 'id' } = {}, + knex + ) { + const whereColumn = `${tableName}.${columnName}` + const query = getAppVersionQuery(knex) + .where(whereColumn, columnValue) + .first() + + const { result } = await executeQuery(query, { model: AppVersionModel }) + + if (!result) { + throw new NotFoundError('App Version Not Found.') + } + + return result + } + + async findOne(id, knex) { + return this.findOneByColumn(id, { columnName: 'id' }, knex) + } + async findByAppId(appId, { filters, pager } = {}, knex) { const query = getAppVersionQuery(knex).where( 'app_version.app_id', @@ -81,7 +108,7 @@ class AppVersionService extends Schmervice.Service { const { result } = await executeQuery(query) - return result.map(c => c.name) + return result.map((c) => c.name) } getDownloadUrl(request, appVersion) { @@ -98,7 +125,7 @@ class AppVersionService extends Schmervice.Service { * @returns a function with signature (appVersion) => appVersionWithDownloadUrl */ createSetDownloadUrl(request) { - return appVersion => { + return (appVersion) => { appVersion.downloadUrl = this.getDownloadUrl(request, appVersion) return appVersion } diff --git a/server/test/routes/v2/appVersions.js b/server/test/routes/v2/appVersions.js index 1237cb042..d5b773cfb 100644 --- a/server/test/routes/v2/appVersions.js +++ b/server/test/routes/v2/appVersions.js @@ -50,8 +50,8 @@ describe('v2/appVersions', () => { const result = res.result.result - const resIds = result.map(r => r.id) - expect(resIds).to.include(dhis2AppVersions.map(v => v.id)) + const resIds = result.map((r) => r.id) + expect(resIds).to.include(dhis2AppVersions.map((v) => v.id)) }) it('should return a result that complies with AppVersionModel', async () => { @@ -69,7 +69,7 @@ describe('v2/appVersions', () => { Joi.assert(result, Joi.array().items(AppVersionModel.def)) // check some keys as well - result.map(v => { + result.map((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() expect(v.minDhisVersion).to.be.a.string() @@ -95,7 +95,7 @@ describe('v2/appVersions', () => { expect(result.pager.pageSize).to.equal(2) expect(versions.length).to.equal(2) // check some keys as well - versions.map(v => { + versions.map((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() expect(v.minDhisVersion).to.be.a.string() @@ -119,7 +119,7 @@ describe('v2/appVersions', () => { Joi.assert(versions, Joi.array().items(AppVersionModel.def)) expect(result.pager).to.be.an.object() // check some keys as well - versions.forEach(v => { + versions.forEach((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() expect(v.channel).to.be.equal('development') @@ -140,7 +140,7 @@ describe('v2/appVersions', () => { const versions = result.result Joi.assert(versions, Joi.array().items(AppVersionModel.def)) - versions.forEach(v => { + versions.forEach((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() expect(v.minDhisVersion).to.be.equal('2.29') @@ -161,7 +161,7 @@ describe('v2/appVersions', () => { const versions = result.result Joi.assert(versions, Joi.array().items(AppVersionModel.def)) - versions.forEach(v => { + versions.forEach((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() expect(v.minDhisVersion).to.be.below('2.29') @@ -183,7 +183,7 @@ describe('v2/appVersions', () => { expect(versions.length).to.be.above(0) Joi.assert(versions, Joi.array().items(AppVersionModel.def)) - versions.forEach(v => { + versions.forEach((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() if (v.maxDhisVersion) { @@ -210,7 +210,7 @@ describe('v2/appVersions', () => { expect(versions.length).to.be.above(0) Joi.assert(versions, Joi.array().items(AppVersionModel.def)) - versions.forEach(v => { + versions.forEach((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() if (v.maxDhisVersion) { @@ -237,7 +237,7 @@ describe('v2/appVersions', () => { expect(versions.length).to.be.above(0) Joi.assert(versions, Joi.array().items(AppVersionModel.def)) - versions.forEach(v => { + versions.forEach((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() if (v.maxDhisVersion) { @@ -266,7 +266,7 @@ describe('v2/appVersions', () => { expect(versions.length).to.be.above(0) Joi.assert(versions, Joi.array().items(AppVersionModel.def)) - versions.forEach(v => { + versions.forEach((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() expect(v.minDhisVersion).to.be.below(31) @@ -295,7 +295,7 @@ describe('v2/appVersions', () => { expect(versions.length).to.be.above(0) Joi.assert(versions, Joi.array().items(AppVersionModel.def)) - versions.forEach(v => { + versions.forEach((v) => { expect(v.appId).to.be.a.string() expect(v.version).to.be.a.string() if (v.maxDhisVersion) { @@ -349,4 +349,99 @@ describe('v2/appVersions', () => { }) }) }) + + describe('get single appVersion', () => { + it('should successfully return an appVersion if it exist', async () => { + const versionId = dhis2AppVersions[0].id + const request = { + method: 'GET', + url: `/api/v2/appVersions/${versionId}`, + } + + const res = await server.inject(request) + + expect(res.statusCode).to.equal(200) + + const version = res.result + + expect(version.id).to.equal(versionId) + Joi.assert(version, AppVersionModel.def) + }) + + it('should return 404 if appVersion does not exist', async () => { + const validRandomUuid = '7f282e24-2d16-44f5-bd38-a7e708b84735' + const request = { + method: 'GET', + url: `/api/v2/appVersions/${validRandomUuid}`, + } + + const res = await server.inject(request) + + expect(res.statusCode).to.equal(404) + }) + + it('should return 400: bad request if id is not an uuid', async () => { + const request = { + method: 'GET', + url: `/api/v2/appVersions/not-an-uuid`, + } + + const res = await server.inject(request) + + expect(res.statusCode).to.equal(400) + }) + + it('should return 403: forbidden if user is not authorized to view app', async () => { + // pending app so version should not be public + const pendingAppId = '02cb663c-5112-400b-8a93-0353187d337b' + const pendingVersions = appVersions.find((versions) => + versions.find((v) => v.app_id === pendingAppId) + ) + + expect(pendingVersions).to.be.an.array() + const pendingVersion = pendingVersions[0] + expect(pendingVersion.id).to.be.a.string() + expect(pendingVersion.app_id).to.be.equal(pendingAppId) + + const request = { + method: 'GET', + url: `/api/v2/appVersions/${pendingVersion.id}`, + auth: { + strategy: 'no-auth', + credentials: { + // some user that does not exist + userId: '2557234e-38d8-4037-9429-80c986632800', + roles: [], + }, + }, + } + const res = await server.inject(request) + + expect(res.statusCode).to.equal(403) + }) + it('should return 403: forbidden if user is not logged in and app is not public', async () => { + // pending app so version should not be public + const pendingAppId = '02cb663c-5112-400b-8a93-0353187d337b' + const pendingVersions = appVersions.find((versions) => + versions.find((v) => v.app_id === pendingAppId) + ) + + expect(pendingVersions).to.be.an.array() + const pendingVersion = pendingVersions[0] + expect(pendingVersion.id).to.be.a.string() + expect(pendingVersion.app_id).to.be.equal(pendingAppId) + + const request = { + method: 'GET', + url: `/api/v2/appVersions/${pendingVersion.id}`, + auth: { + strategy: 'no-auth', + credentials: {}, + }, + } + const res = await server.inject(request) + + expect(res.statusCode).to.equal(403) + }) + }) })