From dcdbacd281052a7baf898f1129ce6801ae930d22 Mon Sep 17 00:00:00 2001 From: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com> Date: Fri, 6 Sep 2024 13:44:09 -0500 Subject: [PATCH] UI: Fix no data read within namespaces (#28311) * Add test for capabilities within namespace * update capabilities fetchMultiplePaths so that the resulting records have the non-prefixed path as ID --- ui/app/adapters/capabilities.js | 41 +++++--- ui/app/serializers/capabilities.js | 7 +- ui/app/services/capabilities.ts | 7 +- ui/tests/unit/services/capabilities-test.js | 103 ++++++++++++++++++++ 4 files changed, 140 insertions(+), 18 deletions(-) diff --git a/ui/app/adapters/capabilities.js b/ui/app/adapters/capabilities.js index b9c56ad5f4..ba18bcd133 100644 --- a/ui/app/adapters/capabilities.js +++ b/ui/app/adapters/capabilities.js @@ -6,7 +6,7 @@ import AdapterError from '@ember-data/adapter/error'; import { set } from '@ember/object'; import ApplicationAdapter from './application'; -import { sanitizePath } from 'core/utils/sanitize-path'; +import { sanitizePath, sanitizeStart } from 'core/utils/sanitize-path'; export default class CapabilitiesAdapter extends ApplicationAdapter { pathForType() { @@ -14,9 +14,9 @@ export default class CapabilitiesAdapter extends ApplicationAdapter { } /* - users don't always have access to the capabilities-self endpoint, + users don't always have access to the capabilities-self endpoint in the current namespace, this can happen when logging in to a namespace and then navigating to a child namespace. - adding "relativeNamespace" to the path and/or "this.namespaceService.userRootNamespace" + adding "relativeNamespace" to the path and/or "this.namespaceService.userRootNamespace" to the request header ensures we are querying capabilities-self in the user's root namespace, which is where they are most likely to have their policy/permissions. */ @@ -26,7 +26,7 @@ export default class CapabilitiesAdapter extends ApplicationAdapter { return path; } // ensure original path doesn't have leading slash - return `${relativeNamespace}/${path.replace(/^\//, '')}`; + return `${relativeNamespace}/${sanitizeStart(path)}`; } async findRecord(store, type, id) { @@ -54,15 +54,30 @@ export default class CapabilitiesAdapter extends ApplicationAdapter { } query(store, type, query) { - const paths = query?.paths.map((p) => this._formatPath(p)); - return this.ajax(this.buildURL(type), 'POST', { - data: { paths }, - namespace: sanitizePath(this.namespaceService.userRootNamespace), - }).catch((e) => { - if (e instanceof AdapterError) { - set(e, 'policyPath', 'sys/capabilities-self'); + const pathMap = query?.paths.reduce((mapping, path) => { + const withNs = this._formatPath(path); + if (withNs) { + mapping[withNs] = path; } - throw e; - }); + return mapping; + }, {}); + + return this.ajax(this.buildURL(type), 'POST', { + data: { paths: Object.keys(pathMap) }, + namespace: sanitizePath(this.namespaceService.userRootNamespace), + }) + .then((queryResult) => { + if (queryResult) { + // send the pathMap with the response so the serializer can normalize the paths to be relative to the namespace + queryResult.pathMap = pathMap; + } + return queryResult; + }) + .catch((e) => { + if (e instanceof AdapterError) { + set(e, 'policyPath', 'sys/capabilities-self'); + } + throw e; + }); } } diff --git a/ui/app/serializers/capabilities.js b/ui/app/serializers/capabilities.js index dd53d9c0a2..0454078114 100644 --- a/ui/app/serializers/capabilities.js +++ b/ui/app/serializers/capabilities.js @@ -15,7 +15,12 @@ export default ApplicationSerializer.extend({ if (requestType === 'query') { // each key on the response is a path with an array of capabilities as its value - response = Object.keys(payload.data).map((path) => ({ capabilities: payload.data[path], path })); + response = Object.keys(payload.data).map((fullPath) => { + // we use pathMap to normalize a namespace-prefixed path back to the relative path + // this is okay because we clear capabilities when moving between namespaces + const path = payload.pathMap ? payload.pathMap[fullPath] : fullPath; + return { capabilities: payload.data[fullPath], path }; + }); } else { response = { ...payload.data, path: payload.path }; } diff --git a/ui/app/services/capabilities.ts b/ui/app/services/capabilities.ts index f33e45285b..d7fbb6df36 100644 --- a/ui/app/services/capabilities.ts +++ b/ui/app/services/capabilities.ts @@ -40,14 +40,13 @@ export default class CapabilitiesService extends Service { return assert('query object must contain "paths" or "path" key', false); } - async fetchMultiplePaths(paths: string[]): MultipleCapabilities | AdapterError { + async fetchMultiplePaths(paths: string[]): Promise { // if the request to capabilities-self fails, silently catch // all of path capabilities default to "true" - const resp: Array | [] = await this.request({ paths }).catch(() => []); - + const resp: CapabilitiesModel[] = await this.request({ paths }).catch(() => []); return paths.reduce((obj: MultipleCapabilities, apiPath: string) => { // path is the model's primaryKey (id) - const model: CapabilitiesModel | undefined = resp.find((m) => m.path === apiPath); + const model = resp.find((m) => m.path === apiPath); if (model) { const { canCreate, canDelete, canList, canPatch, canRead, canSudo, canUpdate } = model; obj[apiPath] = { canCreate, canDelete, canList, canPatch, canRead, canSudo, canUpdate }; diff --git a/ui/tests/unit/services/capabilities-test.js b/ui/tests/unit/services/capabilities-test.js index 1e884aed8c..0964db1d43 100644 --- a/ui/tests/unit/services/capabilities-test.js +++ b/ui/tests/unit/services/capabilities-test.js @@ -239,4 +239,107 @@ module('Unit | Service | capabilities', function (hooks) { }); }); }); + + module('within namespace', function (hooks) { + // capabilities within namespaces are queried at the user's root namespace with a path that includes + // the relative namespace. The capabilities record is saved at the path without the namespace. + hooks.beforeEach(function () { + this.nsSvc = this.owner.lookup('service:namespace'); + this.nsSvc.path = 'ns1'; + this.store.unloadAll('capabilities'); + }); + + test('fetchPathCapabilities works as expected', async function (assert) { + const ns = this.nsSvc.path; + const path = '/my/api/path'; + const expectedAttrs = { + // capabilities has ID at non-namespaced path + id: path, + canCreate: false, + canDelete: false, + canList: true, + canPatch: false, + canRead: true, + canSudo: false, + canUpdate: false, + }; + this.server.post('/sys/capabilities-self', (schema, req) => { + const actual = JSON.parse(req.requestBody); + assert.strictEqual(req.url, '/v1/sys/capabilities-self', 'request made to capabilities-self'); + assert.propEqual( + actual.paths, + [`${ns}/my/api/path`], + `request made with path: ${JSON.stringify(actual)}` + ); + return this.generateResponse({ + path: `${ns}${path}`, + capabilities: ['read', 'list'], + }); + }); + const actual = await this.capabilities.fetchPathCapabilities(path); + assert.strictEqual(this.store.peekAll('capabilities').length, 1, 'adds 1 record'); + + Object.keys(expectedAttrs).forEach(function (key) { + assert.strictEqual( + actual[key], + expectedAttrs[key], + `record has expected value for ${key}: ${actual[key]}` + ); + }); + }); + + test('fetchMultiplePaths works as expected', async function (assert) { + const ns = this.nsSvc.path; + const paths = ['/my/api/path', '/another/api/path']; + const expectedPayload = paths.map((p) => `${ns}${p}`); + + this.server.post('/sys/capabilities-self', (schema, req) => { + const actual = JSON.parse(req.requestBody); + assert.strictEqual(req.url, '/v1/sys/capabilities-self', 'request made to capabilities-self'); + assert.propEqual(actual.paths, expectedPayload, `request made with paths: ${JSON.stringify(actual)}`); + const resp = this.generateResponse({ + paths: expectedPayload, + capabilities: { + [`${ns}/my/api/path`]: ['read', 'list'], + [`${ns}/another/api/path`]: ['update', 'patch'], + }, + }); + return resp; + }); + const actual = await this.capabilities.fetchMultiplePaths(paths); + const expected = { + '/my/api/path': { + canCreate: false, + canDelete: false, + canList: true, + canPatch: false, + canRead: true, + canSudo: false, + canUpdate: false, + }, + '/another/api/path': { + canCreate: false, + canDelete: false, + canList: false, + canPatch: true, + canRead: false, + canSudo: false, + canUpdate: true, + }, + }; + assert.deepEqual(actual, expected, 'method returns expected response'); + assert.strictEqual(this.store.peekAll('capabilities').length, 2, 'adds 2 records'); + Object.keys(expected).forEach((path) => { + const record = this.store.peekRecord('capabilities', path); + assert.strictEqual(record.id, path, `record exists with id: ${record.id}`); + Object.keys(expected[path]).forEach((attr) => { + assert.strictEqual( + record[attr], + expected[path][attr], + `record has correct value for ${attr}: ${record[attr]}` + ); + }); + }); + }); + }); });