Skip to content

Commit

Permalink
fix(rbac): split policies and roles by source (janus-idp#1042)
Browse files Browse the repository at this point in the history
* fix(rbac): split policies by source

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): imporove code

Rename location to source
Add grouping validation for policy file.
Don't allow to modify roles and permission policy from permission policy file.
Fix some bugs.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): add transactions for role and policies metadata

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): cleanup policies which gone from csv policies file

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): handle preexisting policies by adding a legacy source

* fix(rbac): add unit tests for role-metadata.ts

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): fix rest api unit tests

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): fmt code

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): fix failing tests

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): update knex-mock-client to align with knex 3.0 from main

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): avoid nested transactions for sqlite3

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): handle code review feedback.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): handle code review feedback
Move predefined policies validation from permission-policy.ts file to policies-validation.ts.
Add validation for admin users defined in the application config file.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): improve clean up csv file policies and add tests around that

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): handle code review feedback about has method for Set

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): don't allow to modify configuration permission policies

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): add more unit tests

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): add the ability to update legacy policies

* fix(rbac): fix validation mistake

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): complete fix bug janus-idp#1103
Complete fix bug, when after removing admin from app configuration, admin still present.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): fix compilation and tests after rebase

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): fix legacy migration for admin group policies

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): fix sonarcloud issues

* fix(rbac): fix migration issue with multiple admins

* fix(rbac): remove console log

* fix(rbac): fix bug with hanging transaction on sqlite3

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

* fix(rbac): fix tests after rebase

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>

---------

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Co-authored-by: Patrick Knight <pknight@redhat.com>
  • Loading branch information
AndrienkoAleksandr and PatAKnight authored Jan 31, 2024
1 parent f56d072 commit 03a678d
Show file tree
Hide file tree
Showing 21 changed files with 3,407 additions and 318 deletions.
1 change: 1 addition & 0 deletions plugins/rbac-backend/config.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export interface Config {
permission: {
rbac: {
'policies-csv-file'?: string;
/**
* Optional configuration for admins, can declare individual users and / or groups
* @visibility frontend
Expand Down
65 changes: 65 additions & 0 deletions plugins/rbac-backend/migrations/20231212224526_migrations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/**
* @param { import("knex").Knex } knex
* @returns { Promise<void> }
*/
exports.up = async function up(knex) {
const casbinDoesExist = await knex.schema.hasTable('casbin_rule');
let policies = [];
let groupPolicies = [];

if (casbinDoesExist) {
policies = await knex
.select('*')
.from('casbin_rule')
.where('ptype', 'p')
.then(listPolicies => {
const allPolicies = [];
for (const policy of listPolicies) {
const { v0, v1, v2, v3 } = policy;
allPolicies.push(`[${v0}, ${v1}, ${v2}, ${v3}]`);
}
return allPolicies;
});
groupPolicies = await knex
.select('*')
.from('casbin_rule')
.where('ptype', 'g')
.then(listGroupPolicies => {
const allGroupPolicies = [];
for (const groupPolicy of listGroupPolicies) {
const { v0, v1 } = groupPolicy;
allGroupPolicies.push(`[${v0}, ${v1}]`);
}
return allGroupPolicies;
});
}

await knex.schema
.createTable('policy-metadata', table => {
table.increments('id').primary();
table.string('policy').primary();
table.string('source');
})
.then(async () => {
for (const policy of policies) {
await knex
.table('policy-metadata')
.insert({ source: 'legacy', policy: policy });
}
})
.then(async () => {
for (const groupPolicy of groupPolicies) {
await knex
.table('policy-metadata')
.insert({ source: 'legacy', policy: groupPolicy });
}
});
};

/**
* @param { import("knex").Knex } knex
* @returns { Promise<void> }
*/
exports.down = async function down(knex) {
await knex.schema.dropTable('policy-metadata');
};
53 changes: 53 additions & 0 deletions plugins/rbac-backend/migrations/20231221113214_migrations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* @param { import("knex").Knex } knex
* @returns { Promise<void> }
*/
exports.up = async function up(knex) {
const casbinDoesExist = await knex.schema.hasTable('casbin_rule');
let groupPolicies = [];

if (casbinDoesExist) {
groupPolicies = await knex
.select('*')
.from('casbin_rule')
.where('ptype', 'g')
.then(listGroupPolicies => {
const allGroupPolicies = [];
let rbacFlag = false;
for (const groupPolicy of listGroupPolicies) {
const { v1 } = groupPolicy;
if (v1 === 'role:default/rbac_admin') {
rbacFlag = true;
continue;
}
allGroupPolicies.push(v1);
}
if (rbacFlag) {
allGroupPolicies.push('role:default/rbac_admin');
}
return allGroupPolicies;
});
}

await knex.schema
.createTable('role-metadata', table => {
table.increments('id').primary();
table.string('roleEntityRef').primary();
table.string('source');
})
.then(async () => {
for (const groupPolicy of groupPolicies) {
await knex
.table('role-metadata')
.insert({ source: 'legacy', roleEntityRef: groupPolicy });
}
});
};

/**
* @param { import("knex").Knex } knex
* @returns { Promise<void> }
*/
exports.down = async function down(knex) {
await knex.schema.dropTable('role-metadata');
};
5 changes: 3 additions & 2 deletions plugins/rbac-backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
},
"dependencies": {
"@backstage/backend-common": "^0.19.8",
"@backstage/backend-plugin-api": "^0.5.4",
"@backstage/backend-plugin-api": "^0.6.6",
"@backstage/backend-test-utils": "^0.2.7",
"@backstage/catalog-client": "^1.4.5",
"@backstage/catalog-model": "^1.4.3",
"@backstage/config": "^1.1.1",
Expand All @@ -51,7 +52,7 @@
"@types/express": "4.17.20",
"@types/node": "18.18.5",
"@types/supertest": "2.0.16",
"knex-mock-client": "2.0.0",
"knex-mock-client": "2.0.1",
"msw": "1.3.2",
"supertest": "6.3.3"
},
Expand Down
22 changes: 0 additions & 22 deletions plugins/rbac-backend/src/database/conditional-storage.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
import {
PluginDatabaseManager,
resolvePackagePath,
} from '@backstage/backend-common';
import { ConflictError, InputError, NotFoundError } from '@backstage/errors';
import {
AuthorizeResult,
Expand All @@ -11,10 +7,6 @@ import {
import { Knex } from 'knex';

const CONDITIONAL_TABLE = 'policy-conditions';
const migrationsDir = resolvePackagePath(
'@janus-idp/backstage-plugin-rbac-backend', // Package name
'migrations', // Migrations directory
);

interface ConditionalPolicyDecisionDAO {
result: AuthorizeResult.CONDITIONAL;
Expand Down Expand Up @@ -46,20 +38,6 @@ export interface ConditionalStorage {
export class DataBaseConditionalStorage implements ConditionalStorage {
public constructor(private readonly knex: Knex<any, any[]>) {}

static async create(
databaseManager: PluginDatabaseManager,
): Promise<ConditionalStorage> {
const knex = await databaseManager.getClient();

if (!databaseManager.migrations?.skip) {
await knex.migrate.latest({
directory: migrationsDir,
});
}

return new DataBaseConditionalStorage(knex);
}

async getConditions(
pluginId: string,
resourceType: string,
Expand Down
19 changes: 19 additions & 0 deletions plugins/rbac-backend/src/database/migration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {
PluginDatabaseManager,
resolvePackagePath,
} from '@backstage/backend-common';

const migrationsDir = resolvePackagePath(
'@janus-idp/backstage-plugin-rbac-backend', // Package name
'migrations', // Migrations directory
);

export async function migrate(databaseManager: PluginDatabaseManager) {
const knex = await databaseManager.getClient();

if (!databaseManager.migrations?.skip) {
await knex.migrate.latest({
directory: migrationsDir,
});
}
}
Loading

0 comments on commit 03a678d

Please sign in to comment.