Skip to content

Commit e5dda95

Browse files
fix(rbac): reduce the number of permissions returned, add isResourced flag (janus-idp#1474)
* fix(rbac): reduce the number of permissions returned, add isResourced flag Signed-off-by: Oleksandr Andriienko <[email protected]> * fix(rbac): update doc Signed-off-by: Oleksandr Andriienko <[email protected]> --------- Signed-off-by: Oleksandr Andriienko <[email protected]>
1 parent 6f9fe17 commit e5dda95

File tree

4 files changed

+64
-51
lines changed

4 files changed

+64
-51
lines changed

plugins/rbac-backend/docs/apis.md

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -415,37 +415,44 @@ Returns:
415415
[
416416
{
417417
"pluginId": "catalog",
418-
"policies": [
419-
{
420-
"permission": "catalog-entity",
421-
"policy": "read"
422-
},
423-
{
424-
"permission": "catalog.entity.create",
425-
"policy": "create"
426-
},
427-
{
428-
"permission": "catalog-entity",
429-
"policy": "delete"
430-
},
431-
{
432-
"permission": "catalog-entity",
433-
"policy": "update"
434-
},
435-
{
436-
"permission": "catalog.location.read",
437-
"policy": "read"
438-
},
439-
{
440-
"permission": "catalog.location.create",
441-
"policy": "create"
442-
},
443-
{
444-
"permission": "catalog.location.delete",
445-
"policy": "delete"
446-
}
447-
]
448-
},
418+
"policies": [
419+
{
420+
"isResourced": true,
421+
"permission": "catalog-entity",
422+
"policy": "read"
423+
},
424+
{
425+
"isResourced": false,
426+
"permission": "catalog.entity.create",
427+
"policy": "create"
428+
},
429+
{
430+
"isResourced": true,
431+
"permission": "catalog-entity",
432+
"policy": "delete"
433+
},
434+
{
435+
"isResourced": true,
436+
"permission": "catalog-entity",
437+
"policy": "update"
438+
},
439+
{
440+
"isResourced": false,
441+
"permission": "catalog.location.read",
442+
"policy": "read"
443+
},
444+
{
445+
"isResourced": false,
446+
"permission": "catalog.location.create",
447+
"policy": "create"
448+
},
449+
{
450+
"isResourced": false,
451+
"permission": "catalog.location.delete",
452+
"policy": "delete"
453+
}
454+
]
455+
},
449456
...
450457
]
451458
```

plugins/rbac-backend/src/service/plugin-endpoint.test.ts

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ describe('plugin-endpoint', () => {
6969
expect(policiesMetadata.length).toEqual(0);
7070
});
7171

72-
it('should return non empty plugin policies list', async () => {
72+
it('should return non empty plugin policies list with resourced permission', async () => {
7373
backendPluginIDsProviderMock.getPluginIds.mockReturnValue(['permission']);
7474

7575
mockUrlReaderService.readUrl.mockReturnValue(mockReadUrlResponse);
@@ -89,22 +89,19 @@ describe('plugin-endpoint', () => {
8989
expect(policiesMetadata[0].pluginId).toEqual('permission');
9090
expect(policiesMetadata[0].policies).toEqual([
9191
{
92+
isResourced: true,
9293
permission: 'policy-entity',
9394
policy: 'read',
9495
},
95-
{
96-
permission: 'policy.entity.read',
97-
policy: 'read',
98-
},
9996
]);
10097
});
10198

102-
it('should return plugin policies list without resource type permissions', async () => {
99+
it('should return non empty plugin policies list with non resourced permission', async () => {
103100
backendPluginIDsProviderMock.getPluginIds.mockReturnValue(['permission']);
104101

105102
mockUrlReaderService.readUrl.mockReturnValue(mockReadUrlResponse);
106103
bufferMock.toString.mockReturnValueOnce(
107-
'{"permissions":[{"type":"resource","name":"policy.entity.read","attributes":{"action":"read"}}]}',
104+
'{"permissions":[{"type":"basic","name":"catalog.entity.create","attributes":{"action":"create"}}]}',
108105
);
109106

110107
const collector = new PluginPermissionMetadataCollector(
@@ -119,8 +116,9 @@ describe('plugin-endpoint', () => {
119116
expect(policiesMetadata[0].pluginId).toEqual('permission');
120117
expect(policiesMetadata[0].policies).toEqual([
121118
{
122-
permission: 'policy.entity.read',
123-
policy: 'read',
119+
isResourced: false,
120+
permission: 'catalog.entity.create',
121+
policy: 'create',
124122
},
125123
]);
126124
});
@@ -143,7 +141,7 @@ describe('plugin-endpoint', () => {
143141
throw new NotFoundError();
144142
});
145143
bufferMock.toString.mockReturnValueOnce(
146-
'{"permissions":[{"type":"resource","name":"policy.entity.read","attributes":{"action":"read"}}]}',
144+
'{"permissions":[{"type":"resource","resourceType":"policy-entity","name":"policy.entity.read","attributes":{"action":"read"}}]}',
147145
);
148146

149147
const collector = new PluginPermissionMetadataCollector(
@@ -158,7 +156,8 @@ describe('plugin-endpoint', () => {
158156
expect(policiesMetadata[0].pluginId).toEqual('permission');
159157
expect(policiesMetadata[0].policies).toEqual([
160158
{
161-
permission: 'policy.entity.read',
159+
isResourced: true,
160+
permission: 'policy-entity',
162161
policy: 'read',
163162
},
164163
]);
@@ -182,7 +181,7 @@ describe('plugin-endpoint', () => {
182181
throw new Error('Unexpected error');
183182
});
184183
bufferMock.toString.mockReturnValueOnce(
185-
'{"permissions":[{"type":"resource","name":"policy.entity.read","attributes":{"action":"read"}}]}',
184+
'{"permissions":[{"type":"resource","resourceType":"policy-entity","name":"policy.entity.read","attributes":{"action":"read"}}]}',
186185
);
187186

188187
const errorSpy = jest.spyOn(logger, 'error').mockClear();
@@ -199,7 +198,8 @@ describe('plugin-endpoint', () => {
199198
expect(policiesMetadata[0].pluginId).toEqual('permission');
200199
expect(policiesMetadata[0].policies).toEqual([
201200
{
202-
permission: 'policy.entity.read',
201+
isResourced: true,
202+
permission: 'policy-entity',
203203
policy: 'read',
204204
},
205205
]);
@@ -222,7 +222,7 @@ describe('plugin-endpoint', () => {
222222
});
223223
bufferMock.toString
224224
.mockReturnValueOnce(
225-
'{"permissions":[{"type":"resource","name":"policy.entity.read","attributes":{"action":"read"}}]}',
225+
'{"permissions":[{"type":"resource","resourceType":"policy-entity","name":"policy.entity.read","attributes":{"action":"read"}}]}',
226226
)
227227
.mockReturnValueOnce('non json data');
228228

@@ -240,7 +240,8 @@ describe('plugin-endpoint', () => {
240240
expect(policiesMetadata[0].pluginId).toEqual('permission');
241241
expect(policiesMetadata[0].policies).toEqual([
242242
{
243-
permission: 'policy.entity.read',
243+
isResourced: true,
244+
permission: 'policy-entity',
244245
policy: 'read',
245246
},
246247
]);

plugins/rbac-backend/src/service/plugin-endpoints.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,18 +142,22 @@ export class PluginPermissionMetadataCollector {
142142
}
143143

144144
function permissionsToCasbinPolicies(permissions: Permission[]): Policy[] {
145-
const policies = [];
145+
const policies: Policy[] = [];
146146
for (const permission of permissions) {
147147
if (isResourcePermission(permission)) {
148148
policies.push({
149149
permission: permission.resourceType,
150150
policy: permission.attributes.action || 'use',
151+
isResourced: true,
152+
});
153+
} else {
154+
policies.push({
155+
permission: permission.name,
156+
policy: permission.attributes.action || 'use',
157+
isResourced: false,
151158
});
152159
}
153-
policies.push({
154-
permission: permission.name,
155-
policy: permission.attributes.action || 'use',
156-
});
157160
}
161+
158162
return policies;
159163
}

plugins/rbac-common/src/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export type RoleMetadata = {
2121

2222
export type Policy = {
2323
permission?: string;
24+
isResourced?: boolean;
2425
policy?: string;
2526
effect?: string;
2627
metadata?: PermissionPolicyMetadata;

0 commit comments

Comments
 (0)