From 614141bba6d680e534430760251a67928d0a4e7a Mon Sep 17 00:00:00 2001 From: Anemy Date: Wed, 6 Apr 2022 17:57:45 -0400 Subject: [PATCH 1/2] feat: use databases where the authenticated user has built in roles for listDatabases --- .../data-service/src/data-service.spec.ts | 68 ++++++++++++++++++- packages/data-service/src/data-service.ts | 48 +++++++++++-- .../src/instance-detail-helper.spec.ts | 51 +++++++++++++- .../src/instance-detail-helper.ts | 25 +++++++ packages/database-model/lib/model.js | 1 + 5 files changed, 185 insertions(+), 8 deletions(-) diff --git a/packages/data-service/src/data-service.spec.ts b/packages/data-service/src/data-service.spec.ts index 371c5831bd9..72d6fbfbf8a 100644 --- a/packages/data-service/src/data-service.spec.ts +++ b/packages/data-service/src/data-service.spec.ts @@ -1396,6 +1396,56 @@ describe('DataService', function () { expect(dbs).to.deep.eq(['foo']); }); + it('returns databases with `read`, `readWrite`, `dbAdmin`, `dbOwner` roles roles', async function () { + const dataService = createDataServiceWithMockedClient({ + commands: { + connectionStatus: { + authInfo: { + authenticatedUserPrivileges: [], + authenticatedUserRoles: [ + { + role: 'readWrite', + db: 'pineapple', + }, + { + role: 'dbAdmin', + db: 'pineapple', + }, + { + role: 'dbAdmin', + db: 'readerOfPineapple', + }, + { + role: 'dbOwner', + db: 'pineappleBoss', + }, + { + role: 'customRole', + db: 'mint', + }, + { + role: 'read', + db: 'foo', + }, + { + role: 'readWrite', + db: 'watermelon', + }, + ], + }, + }, + }, + }); + const dbs = (await dataService.listDatabases()).map((db) => db.name); + expect(dbs).to.deep.eq([ + 'pineapple', + 'readerOfPineapple', + 'pineappleBoss', + 'foo', + 'watermelon', + ]); + }); + it('filters out databases with no name from privileges', async function () { const dataService = createDataServiceWithMockedClient({ commands: { @@ -1419,7 +1469,7 @@ describe('DataService', function () { expect(dbs).to.deep.eq(['bar']); }); - it('merges databases from listDatabases and privileges', async function () { + it('merges databases from listDatabases, privileges, and roles', async function () { const dataService = createDataServiceWithMockedClient({ commands: { listDatabases: { databases: [{ name: 'foo' }, { name: 'bar' }] }, @@ -1435,12 +1485,26 @@ describe('DataService', function () { actions: ['find'], }, ], + authenticatedUserRoles: [ + { + role: 'readWrite', + db: 'pineapple', + }, + { + role: 'dbAdmin', + db: 'pineapple', + }, + { + role: 'customRole', + db: 'mint', + }, + ], }, }, }, }); const dbs = (await dataService.listDatabases()).map((db) => db.name); - expect(dbs).to.deep.eq(['foo', 'buz', 'bar']); + expect(dbs).to.deep.eq(['pineapple', 'foo', 'buz', 'bar']); }); it('returns result from privileges even if listDatabases threw any error', async function () { diff --git a/packages/data-service/src/data-service.ts b/packages/data-service/src/data-service.ts index 861081c26cc..e88ed4e404a 100644 --- a/packages/data-service/src/data-service.ts +++ b/packages/data-service/src/data-service.ts @@ -58,6 +58,7 @@ import { getPrivilegesByDatabaseAndCollection, checkIsCSFLEConnection, getInstance, + getDatabasesByRoles, } from './instance-detail-helper'; import { redactConnectionString } from './redact'; import connectMongoClient from './connect-mongo-client'; @@ -341,9 +342,25 @@ class DataService extends EventEmitter { const { authInfo: { authenticatedUserPrivileges }, } = await this.connectionStatus(); + return authenticatedUserPrivileges; } + private async _getRolesOrFallback( + roles: + | ConnectionStatusWithPrivileges['authInfo']['authenticatedUserRoles'] + | null = null + ) { + if (roles) { + return roles; + } + const { + authInfo: { authenticatedUserRoles }, + } = await this.connectionStatus(); + + return authenticatedUserRoles; + } + /** * List all collections for a database. */ @@ -442,11 +459,15 @@ class DataService extends EventEmitter { async listDatabases({ nameOnly, privileges = null, + roles = null, }: { nameOnly?: true; privileges?: | ConnectionStatusWithPrivileges['authInfo']['authenticatedUserPrivileges'] | null; + roles?: + | ConnectionStatusWithPrivileges['authInfo']['authenticatedUserRoles'] + | null; } = {}): Promise<{ _id: string; name: string }[]> { const logop = this._startLogOp( mongoLogId(1_001_000_033), @@ -500,16 +521,33 @@ class DataService extends EventEmitter { .map((name) => ({ name })); }; + const getDatabasesFromRoles = async () => { + const databases = getDatabasesByRoles( + await this._getRolesOrFallback(roles), + // https://jira.mongodb.org/browse/HELP-32199 + // Atlas shared tier MongoDB server version v5+ does not return + // `authenticatedUserPrivileges` as part of the `connectionStatus`. + // As a workaround we show the databases the user has + // certain general built-in roles for. + // This does not cover custom user roles which can + // have custom privileges that we can't currently fetch. + ['read', 'readWrite', 'dbAdmin', 'dbOwner'] + ); + return databases.map((name) => ({ name })); + }; + try { - const [listedDatabases, databasesFromPrivileges] = await Promise.all([ - listDatabases(), - getDatabasesFromPrivileges(), - ]); + const [listedDatabases, databasesFromPrivileges, databasesFromRoles] = + await Promise.all([ + listDatabases(), + getDatabasesFromPrivileges(), + getDatabasesFromRoles(), + ]); const databases = uniqueBy( // NB: Order is important, we want listed collections to take precedence // if they were fetched successfully - [...databasesFromPrivileges, ...listedDatabases], + [...databasesFromRoles, ...databasesFromPrivileges, ...listedDatabases], 'name' ).map((db) => { return { _id: db.name, name: db.name, ...adaptDatabaseInfo(db) }; diff --git a/packages/data-service/src/instance-detail-helper.spec.ts b/packages/data-service/src/instance-detail-helper.spec.ts index fa051eafd60..52b8979e69f 100644 --- a/packages/data-service/src/instance-detail-helper.spec.ts +++ b/packages/data-service/src/instance-detail-helper.spec.ts @@ -4,6 +4,7 @@ import type { ConnectionOptions } from './connection-options'; import type { InstanceDetails } from './instance-detail-helper'; import { checkIsCSFLEConnection } from './instance-detail-helper'; import { + getDatabasesByRoles, getPrivilegesByDatabaseAndCollection, getInstance, } from './instance-detail-helper'; @@ -306,7 +307,55 @@ describe('instance-detail-helper', function () { }); }); - describe('#extractPrivilegesByDatabaseAndCollection', function () { + describe('#getDatabasesByRoles', function () { + it('returns a list of databases matching the roles', function () { + const dbs = getDatabasesByRoles( + [ + { db: 'not-test', role: 'write' }, + { db: 'test', role: 'read' }, + { db: 'pineapple', role: 'customRole123' }, + { db: 'pineapple', role: 'customRole12' }, + { db: 'theater', role: 'dbAdmin' }, + ], + ['read', 'readWrite', 'dbAdmin', 'dbOwner'] + ); + + expect(dbs).to.deep.eq(['test', 'theater']); + }); + + it('handles an empty list', function () { + const dbs = getDatabasesByRoles(); + + expect(dbs).to.deep.eq([]); + }); + + it('handles an empty list with roles', function () { + const dbs = getDatabasesByRoles( + [], + ['read', 'readWrite', 'dbAdmin', 'dbOwner'] + ); + + expect(dbs).to.deep.eq([]); + }); + + it('does not return a duplicate database entry', function () { + const dbs = getDatabasesByRoles( + [ + { db: 'test', role: 'read' }, + { db: 'pineapple', role: 'customRole123' }, + { db: 'pineapple', role: 'customRole12' }, + { db: 'theater', role: 'readWrite' }, + { db: 'theater', role: 'customRole1' }, + { db: 'test', role: 'readWrite' }, + ], + ['read', 'readWrite', 'dbAdmin', 'dbOwner'] + ); + + expect(dbs).to.deep.eq(['test', 'theater']); + }); + }); + + describe('#getPrivilegesByDatabaseAndCollection', function () { it('returns a tree of databases and collections from privileges', function () { const dbs = getPrivilegesByDatabaseAndCollection([ { resource: { db: 'foo', collection: 'bar' }, actions: [] }, diff --git a/packages/data-service/src/instance-detail-helper.ts b/packages/data-service/src/instance-detail-helper.ts index b572689410c..6ea069b6d5b 100644 --- a/packages/data-service/src/instance-detail-helper.ts +++ b/packages/data-service/src/instance-detail-helper.ts @@ -273,6 +273,31 @@ export function getPrivilegesByDatabaseAndCollection( return result; } +// Return a list of the databases which have a role matching one of the roles. +export function getDatabasesByRoles( + authenticatedUserRoles: + | ConnectionStatusWithPrivileges['authInfo']['authenticatedUserRoles'] + | null = null, + possibleRoles: string[] | null = null +): string[] { + const roles = authenticatedUserRoles ?? []; + + const results = new Set(); + + const filteredRoles = + possibleRoles && possibleRoles.length > 0 + ? roles.filter(({ role }) => { + return possibleRoles.includes(role); + }) + : roles; + + for (const { db } of filteredRoles) { + results.add(db); + } + + return [...results]; +} + function isNotAuthorized(err: AnyError) { if (!err) { return false; diff --git a/packages/database-model/lib/model.js b/packages/database-model/lib/model.js index 49c4ee6b897..985b0a8a0fc 100644 --- a/packages/database-model/lib/model.js +++ b/packages/database-model/lib/model.js @@ -228,6 +228,7 @@ const DatabaseCollection = AmpersandCollection.extend( const dbs = await dataService.listDatabases({ nameOnly: true, privileges: instanceModel.auth.privileges, + roles: instanceModel.auth.roles }); this.set(dbs.map(({ _id, name }) => ({ _id, name }))); From 58d81b0f8d8169b3d7338e1e262a2c95c1ae74fd Mon Sep 17 00:00:00 2001 From: Anemy Date: Wed, 6 Apr 2022 18:09:46 -0400 Subject: [PATCH 2/2] fixup: minimize diff, remove extra newline --- packages/data-service/src/data-service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/data-service/src/data-service.ts b/packages/data-service/src/data-service.ts index e88ed4e404a..e929aa8689b 100644 --- a/packages/data-service/src/data-service.ts +++ b/packages/data-service/src/data-service.ts @@ -342,7 +342,6 @@ class DataService extends EventEmitter { const { authInfo: { authenticatedUserPrivileges }, } = await this.connectionStatus(); - return authenticatedUserPrivileges; }