Skip to content

Commit 3581237

Browse files
mcasimirgribnoysup
andauthored
fix: remove empty db and collection from privileges (#2583)
* fix: remove undefined from db and collection list * allow empty collection names * chore(data-service): Clarify what exactly we are skipping when extracting dbs and colls from privileges Co-authored-by: Sergey Petushkov <[email protected]>
1 parent 584f0e9 commit 3581237

File tree

3 files changed

+93
-17
lines changed

3 files changed

+93
-17
lines changed

packages/data-service/src/instance-detail-helper.spec.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,65 @@ describe('instance-detail-helper', function () {
253253
expect(dbs).to.have.nested.property('foo.bar').deep.eq([]);
254254
});
255255

256+
context('with known resources', function () {
257+
it('ignores cluster privileges', function () {
258+
const dbs = extractPrivilegesByDatabaseAndCollection([
259+
{ resource: { db: 'foo', collection: 'bar' }, actions: [] },
260+
{ resource: { db: 'buz', collection: 'bla' }, actions: [] },
261+
{ resource: { cluster: true }, actions: [] },
262+
]);
263+
264+
expect(dbs).to.deep.eq({
265+
foo: {
266+
bar: [],
267+
},
268+
buz: {
269+
bla: [],
270+
},
271+
});
272+
});
273+
274+
it('ignores anyResource privileges', function () {
275+
const dbs = extractPrivilegesByDatabaseAndCollection([
276+
{ resource: { db: 'foo', collection: 'bar' }, actions: [] },
277+
{ resource: { db: 'buz', collection: 'bla' }, actions: [] },
278+
{ resource: { anyResource: true }, actions: [] },
279+
]);
280+
281+
expect(dbs).to.deep.eq({
282+
foo: {
283+
bar: [],
284+
},
285+
buz: {
286+
bla: [],
287+
},
288+
});
289+
});
290+
});
291+
292+
context('with unknown resources', function () {
293+
it("ignores everything that doesn't have database and collection in resource", function () {
294+
const dbs = extractPrivilegesByDatabaseAndCollection([
295+
{ resource: { db: 'foo', collection: 'bar' }, actions: [] },
296+
{ resource: { db: 'buz', collection: 'bla' }, actions: [] },
297+
{ resource: { this: true }, actions: [] },
298+
{ resource: { is: true }, actions: [] },
299+
{ resource: { not: true }, actions: [] },
300+
{ resource: { valid: true }, actions: [] },
301+
{ resource: { resource: true }, actions: [] },
302+
] as any);
303+
304+
expect(dbs).to.deep.eq({
305+
foo: {
306+
bar: [],
307+
},
308+
buz: {
309+
bla: [],
310+
},
311+
});
312+
});
313+
});
314+
256315
it('keeps records for all collections in a database', function () {
257316
const dbs = extractPrivilegesByDatabaseAndCollection([
258317
{ resource: { db: 'foo', collection: 'bar' }, actions: [] },

packages/data-service/src/instance-detail-helper.ts

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -176,14 +176,26 @@ export function extractPrivilegesByDatabaseAndCollection(
176176

177177
const result: DatabaseCollectionPrivileges = {};
178178

179-
for (const {
180-
resource: { db, collection },
181-
actions,
182-
} of filteredPrivileges) {
183-
if (result[db]) {
184-
Object.assign(result[db], { [collection]: actions });
185-
} else {
186-
result[db] = { [collection]: actions };
179+
for (const { resource, actions } of filteredPrivileges) {
180+
// Documented resources include roles for dbs/colls, cluster, or in rare cases
181+
// anyResource, additionally there seem to be undocumented ones like
182+
// system_buckets and who knows what else. To make sure we are only cover
183+
// cases that we can meaningfully handle here, roles for the
184+
// databases/collections, we are skipping all roles where these are
185+
// undefined
186+
//
187+
// See: https://docs.mongodb.com/manual/reference/resource-document/#std-label-resource-document
188+
if (
189+
typeof resource.db !== 'undefined' &&
190+
typeof resource.collection !== 'undefined'
191+
) {
192+
const { db, collection } = resource;
193+
194+
if (result[db]) {
195+
Object.assign(result[db], { [collection]: actions });
196+
} else {
197+
result[db] = { [collection]: actions };
198+
}
187199
}
188200
}
189201

@@ -216,14 +228,6 @@ function isNotAuthorized(err: AnyError) {
216228
return new RegExp('not (authorized|allowed)').test(msg);
217229
}
218230

219-
function isMongosLocalException(err: AnyError) {
220-
if (!err) {
221-
return false;
222-
}
223-
const msg = err.message || JSON.stringify(err);
224-
return new RegExp('database through mongos').test(msg);
225-
}
226-
227231
function ignoreNotAuthorized<T>(fallback: T): (err: AnyError) => Promise<T> {
228232
return (err: AnyError) => {
229233
if (isNotAuthorized(err)) {

packages/data-service/src/run-command.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,20 @@ export type ConnectionStatus = {
1111
export type ConnectionStatusWithPrivileges = ConnectionStatus & {
1212
authInfo: {
1313
authenticatedUserPrivileges: {
14-
resource: { db: string; collection: string };
14+
resource:
15+
| { db?: never; collection?: never; cluster: true; anyResource?: never }
16+
| {
17+
db: string;
18+
collection: string;
19+
cluster?: never;
20+
anyResource?: never;
21+
}
22+
| {
23+
db?: never;
24+
collection?: never;
25+
cluster?: never;
26+
anyResource: true;
27+
};
1528
actions: string[];
1629
}[];
1730
};

0 commit comments

Comments
 (0)