diff --git a/api/v1/controllers/login.mjs b/api/v1/controllers/login.mjs index 2be5882..9330585 100644 --- a/api/v1/controllers/login.mjs +++ b/api/v1/controllers/login.mjs @@ -18,6 +18,7 @@ import * as JV from '../../../lib/jsonvalidator.mjs' import * as Settings from '../../../lib/settings.mjs' import * as ApiKey from '../../../model/apikey.mjs' import * as Metrics from '../../../lib/metrics.mjs' +import * as Folder from '../../../model/folder.mjs' import DB from '../../../lib/db.mjs' @@ -208,6 +209,9 @@ export async function login (req, res, next) { Metrics.counterInc(isapikey ? Const.METRICS_LOGIN_APIKEYS : Const.METRICS_LOGIN_USERS) + // Create user tree cache + await Folder.userTree(user.id) + Events.add(user.id, Const.EV_ACTION_LOGIN, Const.EV_ENTITY_USER, user.id) res.send(R.ok({ jwt: token })) } diff --git a/lib/cache.mjs b/lib/cache.mjs index f2e9afc..ee89fbc 100644 --- a/lib/cache.mjs +++ b/lib/cache.mjs @@ -15,6 +15,7 @@ let Cache const prefix = 'pwapi' export const foldersTreeKey = 'ft' export const foldersReadableKey = 'fr' +export const foldersWritableKey = 'fw' export const groupsTreeKey = 'gt' export const itemTypesKey = 'it' @@ -41,17 +42,26 @@ export async function init () { * @param {string} user If provided, only cache for this user will be reset */ export async function resetFoldersTree (user) { + // Reset the tree cache let k = `${prefix}:${foldersTreeKey}` if (user) { k += `.${user}.` } await Cache.del(k) + // Reset the readable cache k = `${prefix}:${foldersReadableKey}` if (user) { k += `.${user}.` } await Cache.del(k) + + // Reset the writable cache + k = `${prefix}:${foldersWritableKey}` + if (user) { + k += `.${user}.` + } + await Cache.del(k) } /** diff --git a/model/folder.mjs b/model/folder.mjs index 86da748..1ec9f46 100644 --- a/model/folder.mjs +++ b/model/folder.mjs @@ -177,6 +177,15 @@ export async function permissions (id, user) { write: false } + // Check for cache existance + const cacheRead = await Cache.get(user, Cache.foldersReadableKey) + const cacheWrite = await Cache.get(user, Cache.foldersWritableKey) + if (cacheRead && cacheWrite) { + ret.read = cacheRead.find(elem => elem === id) !== undefined + ret.write = cacheWrite.find(elem => elem === id) !== undefined + return ret + } + // Extracts the parents, and all the permissions for any group where user is a member const pPerms = await DB.$queryRaw` with recursive folder_parents as @@ -274,17 +283,13 @@ export async function groupPermissions (folderid, groupid) { } /** - * Return the tree structure of folders visible to the user. - * + * Return the tree structure of folders visible to the user and generates the cache * @param {string} user User ID - * @param {string} getpermissions If true, also permissions are extracted */ -export async function userTree (user, getpermissions) { - if (getpermissions !== 'true') { - const cache = await Cache.get(user, Cache.foldersTreeKey) - if (cache) { - return cache - } +export async function userTree (user) { + const cache = await Cache.get(user, Cache.foldersTreeKey) + if (cache) { + return cache } // Get folders for cache @@ -292,7 +297,7 @@ export async function userTree (user, getpermissions) { // Explicitly allowed folders, plus personal folder const readFolders = await DB.$queryRaw` - select f.* + select f.*, p.read, p.write from folders f join folderspermissions p on f.id = p.folderid @@ -303,13 +308,14 @@ export async function userTree (user, getpermissions) { where p.read = true and ug.userid = ${user} union - select pf.* + select pf.*, true, true from folders pf where pf.personal = true and pf.userid = ${user}` // For each allowed folder, add all parents and children const readable = new Map() + const writable = new Map() const data = [] const added = new Map() for (const folder of readFolders) { @@ -317,6 +323,9 @@ export async function userTree (user, getpermissions) { const aparents = await parents(folder.id, allFolders) readable.set(folder.id, folder.id) + if (folder.write) { + writable.set(folder.id, folder.id) + } // Each child is also added to read-permitted folders for caching for (const el of achildren) { @@ -326,22 +335,31 @@ export async function userTree (user, getpermissions) { } if (!added.get(el.id)) { - if (getpermissions === 'true') { - el.permissions = await permissions(el.id, user) - } + el.permissions = await permissions(el.id, user) data.push(el) added.set(el.id, el.id) readable.set(el.id, el.id) + if (el.permissions.write) { + writable.set(el.id, el.id) + } } } + + // Scan parents and add to the tree, for representation sake for (const el of aparents) { if (!added.get(el.id)) { - if (getpermissions === 'true') { - el.permissions = await permissions(el.id, user) - } + el.permissions = await permissions(el.id, user) data.push(el) added.set(el.id, el.id) + + // On parent folders, permissions need to be checked explicitly + if (el.read) { + readable.set(el.id, el.id) + } + if (el.write) { + writable.set(el.id, el.id) + } } } } @@ -370,11 +388,12 @@ export async function userTree (user, getpermissions) { await Cache.set(user, Cache.foldersTreeKey, tree) await Cache.set(user, Cache.foldersReadableKey, Array.from(readable.keys())) + await Cache.set(user, Cache.foldersWritableKey, Array.from(writable.keys())) return tree } /** - * Return the tree structure of folders visible to the user. + * Return the tree structure of folders visible to the group. * * @param {string} group Group */ diff --git a/test/folderspermissions.cjs b/test/folderspermissions.cjs index 1bae29c..3ce0f90 100644 --- a/test/folderspermissions.cjs +++ b/test/folderspermissions.cjs @@ -190,4 +190,80 @@ describe('Folders permissions', () => { assert.strictEqual(res4.status, 200) }) + + it('Change permissions on a folder and try read/write operations', async () => { + // Create a subfolder on root + const res1 = await agent + .post(`${global.host}/api/v1/folders/0/folders`) + .set('Authorization', `Bearer ${global.adminJWT}`) + .send(global.folderCreateData) + .catch(v => v) + + assert.strictEqual(res1.status, 201) + const folder = res1.body.data.id + + // Add read+write permissions to Everyone + const res2 = await agent + .post(`${global.host}/api/v1/folders/${folder}/groups/E`) + .set('Authorization', `Bearer ${global.adminJWT}`) + .send({ read: true, write: true }) + .catch(v => v) + assert.strictEqual(res2.status, 200) + + // Create a new item + const res3 = await agent + .post(`${global.host}/api/v1/folders/${folder}/items`) + .set('Authorization', `Bearer ${global.userJWT}`) + .send(global.itemCreateData) + .catch(v => v) + assert.strictEqual(res3.status, 201) + + const itemId = res3.body.data.id + + // Delete the item + const res4 = await agent + .delete(`${global.host}/api/v1/folders/${folder}/items/${itemId}`) + .set('Authorization', `Bearer ${global.userJWT}`) + .catch(v => v) + assert.strictEqual(res4.status, 200) + + // Change to readonly + const res5 = await agent + .patch(`${global.host}/api/v1/folders/${folder}/groups/E`) + .set('Authorization', `Bearer ${global.adminJWT}`) + .send({ read: true, write: false }) + .catch(v => v) + assert.strictEqual(res5.status, 200) + + // Try to create a new item, it must be unauthorized + const res6 = await agent + .post(`${global.host}/api/v1/folders/${folder}/items`) + .set('Authorization', `Bearer ${global.userJWT}`) + .send(global.itemCreateData) + .catch(v => v) + assert.strictEqual(res6.status, 403) + + // Change to no access + const res7 = await agent + .patch(`${global.host}/api/v1/folders/${folder}/groups/E`) + .set('Authorization', `Bearer ${global.adminJWT}`) + .send({ read: false, write: false }) + .catch(v => v) + assert.strictEqual(res7.status, 200) + + // Get items list, must be unauthorized + const res8 = await agent + .get(`${global.host}/api/v1/folders/${folder}/items`) + .set('Authorization', `Bearer ${global.userJWT}`) + .catch(v => v) + assert.strictEqual(res8.status, 403) + + // Cleanup the folder + const res9 = await agent + .delete(`${global.host}/api/v1/folders/${folder}`) + .set('Authorization', `Bearer ${global.adminJWT}`) + .catch(v => v) + + assert.strictEqual(res9.status, 200) + }) }) diff --git a/test/items.spec.cjs b/test/items.spec.cjs index d34e4d4..0cae02e 100644 --- a/test/items.spec.cjs +++ b/test/items.spec.cjs @@ -330,4 +330,40 @@ describe('Items', () => { .catch(v => v) assert.strictEqual(res6.status, 200) }) + + it('Get item 100 times and measure time (below 2 seconds)', async () => { + // Create an item to fetch + const res1 = await agent + .post(`${global.host}/api/v1/folders/sample1/items`) + .set('Authorization', `Bearer ${global.userJWT}`) + .send(global.itemCreateData) + .catch(v => v) + + assert.strictEqual(res1.status, 201) + const itemid = res1.body.data.id + + const start = Date.now() + + for (let i = 0; i < 100; i++) { + const res = await agent + .get(`${global.host}/api/v1/items/${itemid}?key=${global.key}`) + .set('Authorization', `Bearer ${global.userJWT}`) + .catch(v => v) + + assert.strictEqual(res.status, 200) + } + + const end = Date.now() + const elapsed = end - start + console.log(`Fetching the item 100 times took ${elapsed} ms`) + + assert(elapsed < 2000, 'Fetching took too long') + + // Cleanup + const res2 = await agent + .delete(`${global.host}/api/v1/items/${itemid}`) + .set('Authorization', `Bearer ${global.userJWT}`) + .catch(v => v) + assert.strictEqual(res2.status, 200) + }) })