From ad8896fd11fab9bd100280552798ccdbe6ffba08 Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Sun, 15 Oct 2023 13:20:24 -0700 Subject: [PATCH] Closes #449 - Added in-memory permission caching --- .env-cmdrc-template | 1 + .github/workflows/master.yml | 3 +- .github/workflows/release.yml | 1 + config/.env.dev | 1 + docker-compose.yml | 1 + src/app.js | 1 + src/client/permission-resolvers.js | 9 +++- src/helpers/cache.js | 55 +++++++++++++++++++++++++ src/services/permission.js | 24 +++++++---- tests/client-api.test.js | 30 +++++++++++++- tests/relay.test.js | 2 +- tests/unit-test/cache.test.js | 66 ++++++++++++++++++++++++++++++ tests/unit-test/helpers.test.js | 2 +- 13 files changed, 182 insertions(+), 14 deletions(-) create mode 100644 src/helpers/cache.js create mode 100644 tests/unit-test/cache.test.js diff --git a/.env-cmdrc-template b/.env-cmdrc-template index 54d8285..e8abd04 100644 --- a/.env-cmdrc-template +++ b/.env-cmdrc-template @@ -16,6 +16,7 @@ "MAX_STRATEGY_OPERATION": 100, "RELAY_BYPASS_HTTPS": true, "RELAY_BYPASS_VERIFICATION": true, + "PERMISSION_CACHE_ACTIVATED": true, "HISTORY_ACTIVATED": true, "METRICS_ACTIVATED": true, "METRICS_MAX_PAGE": 50, diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index 8266f0b..4d54317 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -49,6 +49,7 @@ jobs: MAX_STRATEGY_OPERATION: 100 RELAY_BYPASS_HTTPS: true RELAY_BYPASS_VERIFICATION: true + PERMISSION_CACHE_ACTIVATED: true METRICS_ACTIVATED: true METRICS_MAX_PAGE: 50 MAX_REQUEST_PER_MINUTE: 0 @@ -56,7 +57,7 @@ jobs: SWITCHER_API_LOGGER: false - name: SonarCloud Scan - uses: sonarsource/sonarcloud-github-action@v1.8 + uses: sonarsource/sonarcloud-github-action@v2.0.2 env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 129ef75..de598d8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -43,6 +43,7 @@ jobs: MAX_STRATEGY_OPERATION: 100 RELAY_BYPASS_HTTPS: true RELAY_BYPASS_VERIFICATION: true + PERMISSION_CACHE_ACTIVATED: true METRICS_ACTIVATED: true METRICS_MAX_PAGE: 50 MAX_REQUEST_PER_MINUTE: 0 diff --git a/config/.env.dev b/config/.env.dev index 00a603c..b6ca95a 100644 --- a/config/.env.dev +++ b/config/.env.dev @@ -11,6 +11,7 @@ JWT_ADMIN_TOKEN_RENEW_INTERVAL=10m MAX_STRATEGY_OPERATION=100 RELAY_BYPASS_HTTPS=true RELAY_BYPASS_VERIFICATION=true +PERMISSION_CACHE_ACTIVATED=true REGEX_MAX_TIMEOUT=3000 REGEX_MAX_BLACLIST=50 MAX_REQUEST_PER_MINUTE=0 diff --git a/docker-compose.yml b/docker-compose.yml index f75f62d..adba0f0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -54,6 +54,7 @@ services: - MAX_STRATEGY_OPERATION=${MAX_STRATEGY_OPERATION} - RELAY_BYPASS_HTTPS=${RELAY_BYPASS_HTTPS} - RELAY_BYPASS_VERIFICATION=${RELAY_BYPASS_VERIFICATION} + - PERMISSION_CACHE_ACTIVATED=${PERMISSION_CACHE_ACTIVATED} - HISTORY_ACTIVATED=${HISTORY_ACTIVATED} - METRICS_ACTIVATED=${METRICS_ACTIVATED} - METRICS_MAX_PAGE=${METRICS_MAX_PAGE} diff --git a/src/app.js b/src/app.js index 5348fbe..dcc8fca 100644 --- a/src/app.js +++ b/src/app.js @@ -96,6 +96,7 @@ app.get('/check', defaultLimiter, (req, res) => { switcherapi_logger: process.env.SWITCHER_API_LOGGER, relay_bypass_https: process.env.RELAY_BYPASS_HTTPS, relay_bypass_verification: process.env.RELAY_BYPASS_VERIFICATION, + permission_cache: process.env.PERMISSION_CACHE_ACTIVATED, history: process.env.HISTORY_ACTIVATED, metrics: process.env.METRICS_ACTIVATED, max_metrics_pages: process.env.METRICS_MAX_PAGE, diff --git a/src/client/permission-resolvers.js b/src/client/permission-resolvers.js index 19905c0..0c3b7aa 100644 --- a/src/client/permission-resolvers.js +++ b/src/client/permission-resolvers.js @@ -2,6 +2,7 @@ import { verifyOwnership } from '../helpers'; import { RouterTypes } from '../models/permission'; import { getConfigs } from '../services/config'; import { getGroupConfigs } from '../services/group-config'; +import { permissionCache } from '../helpers/cache'; export async function resolvePermission(args, admin) { let elements; @@ -12,6 +13,11 @@ export async function resolvePermission(args, admin) { } else { return []; } + + const cacheKey = permissionCache.permissionKey(admin._id, args.domain, elements, args.actions, args.router); + if (permissionCache.has(cacheKey)) { + return permissionCache.get(cacheKey); + } let result = []; for (const element of elements) { @@ -30,6 +36,7 @@ export async function resolvePermission(args, admin) { } } } - + + permissionCache.set(cacheKey, result); return result; } \ No newline at end of file diff --git a/src/helpers/cache.js b/src/helpers/cache.js new file mode 100644 index 0000000..08cc288 --- /dev/null +++ b/src/helpers/cache.js @@ -0,0 +1,55 @@ +class Cache { + constructor() { + this.cache = new Map(); + } + + set(key, value) { + if (!this.isPermissionCacheActivated()) { + return; + } + + this.cache.set(key, value); + } + + get(key) { + return this.cache.get(key); + } + + permissionKey(adminId, domainId, elements, action, router) { + return JSON.stringify({ + adminId: String(adminId), + domainId: String(domainId), + elements: elements.map(element => String(element._id)), + actions: action, + router: router + }); + } + + permissionReset(domainId, action, router) { + if (!domainId || !action || !router) { + return; + } + + const keys = this.cache.keys(); + for (const key of keys) { + const parsedKey = JSON.parse(key); + if (parsedKey.domainId === String(domainId) && + parsedKey.actions.includes(action) && + parsedKey.router === router) { + this.cache.delete(key); + } + } + } + + isPermissionCacheActivated() { + return process.env.PERMISSION_CACHE_ACTIVATED === 'true'; + } + + has(key) { + return this.cache.has(key); + } +} + +const permissionCache = new Cache(); + +export { permissionCache }; \ No newline at end of file diff --git a/src/services/permission.js b/src/services/permission.js index e06ac6d..6dee5cc 100644 --- a/src/services/permission.js +++ b/src/services/permission.js @@ -3,6 +3,7 @@ import { ActionTypes, Permission, RouterTypes } from '../models/permission'; import { verifyOwnership } from '../helpers'; import { response } from './common'; import { getTeam, getTeams, verifyRequestedTeam } from './team'; +import { permissionCache } from '../helpers/cache'; async function verifyRequestedTeamByPermission(permissionId, admin, action) { let team = await getTeam({ permissions: permissionId }); @@ -48,36 +49,41 @@ export async function createPermission(args, teamId, admin) { const team = await verifyRequestedTeam(teamId, admin, ActionTypes.CREATE); team.permissions.push(permission._id); + permissionCache.permissionReset(team.domain, permission.action, permission.router); + await team.save(); return permission; } export async function updatePermission(args, id, admin) { const permission = await getPermissionById(id); - await verifyRequestedTeamByPermission(permission._id, admin, ActionTypes.UPDATE); + const team = await verifyRequestedTeamByPermission(permission._id, admin, ActionTypes.UPDATE); const updates = Object.keys(args); updates.forEach((update) => permission[update] = args[update]); + + permissionCache.permissionReset(team.domain, permission.action, permission.router); return permission.save(); } export async function deletePermission(id, admin) { const permission = await getPermissionById(id); - await verifyRequestedTeamByPermission(permission._id, admin, ActionTypes.DELETE); + const team = await verifyRequestedTeamByPermission(permission._id, admin, ActionTypes.DELETE); const teams = await getTeams({ permissions: permission._id }); - teams.forEach(team => { - const indexValue = team.permissions.indexOf(permission._id); - team.permissions.splice(indexValue, 1); - team.save(); + teams.forEach(pTeam => { + const indexValue = pTeam.permissions.indexOf(permission._id); + pTeam.permissions.splice(indexValue, 1); + pTeam.save(); }); + permissionCache.permissionReset(team.domain, permission.action, permission.router); return permission.deleteOne(); } export async function addValue(args, id, admin) { const permission = await verifyPermissionValueInput(id, args.value); - await verifyRequestedTeamByPermission(permission._id, admin, ActionTypes.UPDATE); + const team = await verifyRequestedTeamByPermission(permission._id, admin, ActionTypes.UPDATE); const value = args.value.trim(); if (permission.values.includes(value)) { @@ -85,12 +91,13 @@ export async function addValue(args, id, admin) { } permission.values.push(value); + permissionCache.permissionReset(team.domain, permission.action, permission.router); return permission.save(); } export async function removeValue(args, id, admin) { const permission = await verifyPermissionValueInput(id, args.value); - await verifyRequestedTeamByPermission(permission._id, admin, ActionTypes.UPDATE); + const team = await verifyRequestedTeamByPermission(permission._id, admin, ActionTypes.UPDATE); const value = args.value.trim(); const indexValue = permission.values.indexOf(value); @@ -100,5 +107,6 @@ export async function removeValue(args, id, admin) { } permission.values.splice(indexValue, 1); + permissionCache.permissionReset(team.domain, permission.action, permission.router); return permission.save(); } \ No newline at end of file diff --git a/tests/client-api.test.js b/tests/client-api.test.js index 33d4214..b14e1a6 100644 --- a/tests/client-api.test.js +++ b/tests/client-api.test.js @@ -1,6 +1,9 @@ import mongoose from 'mongoose'; import request from 'supertest'; +import sinon from 'sinon'; import app from '../src/app'; +import { ActionTypes, RouterTypes } from '../src/models/permission'; +import { permissionCache } from '../src/helpers/cache'; import Domain from '../src/models/domain'; import GroupConfig from '../src/models/group-config'; import { Config } from '../src/models/config'; @@ -27,11 +30,10 @@ import { adminAccountId, slack } from './fixtures/db_client'; -import { RouterTypes } from '../src/models/permission'; const changeStrategy = async (strategyId, newOperation, status, environment) => { const strategy = await ConfigStrategy.findById(strategyId).exec(); - strategy.operation = newOperation ? newOperation : strategy.operation; + strategy.operation = newOperation || strategy.operation; strategy.activated.set(environment, status !== undefined ? status : strategy.activated.get(environment)); strategy.updatedBy = adminMasterAccountId; await strategy.save(); @@ -969,6 +971,30 @@ describe('Testing domain [Adm-GraphQL] ', () => { expect(JSON.parse(req.text).data.permission[0].permissions).toMatchObject(JSON.parse(exptected)); }); + test('CLIENT_SUITE - Should return list of Groups permissions - from cache', async () => { + const cacheSpy = sinon.spy(permissionCache, 'get'); + permissionCache.permissionReset(domainId, ActionTypes.UPDATE, RouterTypes.GROUP); + + await request(app) + .post('/adm-graphql') + .set('Authorization', `Bearer ${adminMasterAccountToken}`) + .send(graphqlUtils.permissionsQuery(domainId, undefined, `"UPDATE","DELETE"`, RouterTypes.GROUP)); + + expect(cacheSpy.callCount).toBe(0); + + const req = await request(app) + .post('/adm-graphql') + .set('Authorization', `Bearer ${adminMasterAccountToken}`) + .send(graphqlUtils.permissionsQuery(domainId, undefined, `"UPDATE","DELETE"`, RouterTypes.GROUP)); + + const exptected = '[{"action":"UPDATE","result":"ok"},{"action":"DELETE","result":"ok"}]'; + expect(req.statusCode).toBe(200); + expect(cacheSpy.callCount).toBe(1); + expect(JSON.parse(req.text)).not.toBe(null); + expect(JSON.parse(req.text).data.permission[0].name).toBe("Group Test"); + expect(JSON.parse(req.text).data.permission[0].permissions).toMatchObject(JSON.parse(exptected)); + }); + test('CLIENT_SUITE - Should return list of Groups permissions - Unauthorized access', async () => { const req = await request(app) .post('/adm-graphql') diff --git a/tests/relay.test.js b/tests/relay.test.js index f5de809..72d07be 100644 --- a/tests/relay.test.js +++ b/tests/relay.test.js @@ -21,7 +21,7 @@ import { StrategiesType, ConfigStrategy } from '../src/models/config-strategy'; const changeStrategy = async (strategyId, newOperation, status, environment) => { const strategy = await ConfigStrategy.findById(strategyId).exec(); - strategy.operation = newOperation ? newOperation : strategy.operation; + strategy.operation = newOperation || strategy.operation; strategy.activated.set(environment, status !== undefined ? status : strategy.activated.get(environment)); strategy.updatedBy = adminMasterAccountId; await strategy.save(); diff --git a/tests/unit-test/cache.test.js b/tests/unit-test/cache.test.js new file mode 100644 index 0000000..0e059d9 --- /dev/null +++ b/tests/unit-test/cache.test.js @@ -0,0 +1,66 @@ +import { permissionCache } from "../../src/helpers/cache"; +import { ActionTypes, RouterTypes } from "../../src/models/permission"; + +describe("Test permissionCache", () => { + + it("UNIT_CACHE - Should set and get cache", () => { + const cacheKey = permissionCache.permissionKey( + 'adminId', + 'domainId', + ['element1Id', 'element2Id'], + [ActionTypes.UPDATE, ActionTypes.READ], + RouterTypes.GROUP + ); + + permissionCache.set(cacheKey, 'value'); + expect(permissionCache.has(cacheKey)).toBe(true); + const result = permissionCache.get(cacheKey); + expect(result).toEqual('value'); + }); + + it("UNIT_CACHE - Should reload cache", () => { + const cacheKey = permissionCache.permissionKey( + 'adminId', + 'domainId', + ['element1Id', 'element2Id'], + [ActionTypes.UPDATE, ActionTypes.READ], + RouterTypes.GROUP + ); + + permissionCache.set(cacheKey, 'value'); + permissionCache.permissionReset('domainId', ActionTypes.UPDATE, RouterTypes.GROUP); + const result = permissionCache.get(cacheKey); + expect(result).toBeUndefined(); + }); + + it("UNIT_CACHE - Should not reload cache", () => { + const cacheKey = permissionCache.permissionKey( + 'adminId', + 'domainId', + ['element1Id', 'element2Id'], + [ActionTypes.UPDATE, ActionTypes.READ], + RouterTypes.GROUP + ); + + permissionCache.set(cacheKey, 'value'); + permissionCache.permissionReset('domainId', ActionTypes.UPDATE, RouterTypes.CONFIG); + const result = permissionCache.get(cacheKey); + expect(result).toEqual('value'); + }); + + it("UNIT_CACHE - Should not reload cache - empty router/action", () => { + const cacheKey = permissionCache.permissionKey( + 'adminId', + 'domainId', + ['element1Id', 'element2Id'], + [ActionTypes.UPDATE, ActionTypes.READ], + RouterTypes.GROUP + ); + + permissionCache.set(cacheKey, 'value'); + permissionCache.permissionReset(); + const result = permissionCache.get(cacheKey); + expect(result).toEqual('value'); + }); + +}); \ No newline at end of file diff --git a/tests/unit-test/helpers.test.js b/tests/unit-test/helpers.test.js index 1d50e8b..effa7fb 100644 --- a/tests/unit-test/helpers.test.js +++ b/tests/unit-test/helpers.test.js @@ -1,4 +1,4 @@ -const { formatInput, containsValue } = require('../../src/helpers'); +import { formatInput, containsValue } from '../../src/helpers'; describe('Test formatInput', () => {