Skip to content

Commit

Permalink
fix(rbac): handle preexisting policies by adding a legacy source
Browse files Browse the repository at this point in the history
  • Loading branch information
PatAKnight committed Jan 12, 2024
1 parent 669e085 commit d85afe9
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 41 deletions.
56 changes: 51 additions & 5 deletions plugins/rbac-backend/migrations/20231212224526_migrations.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,57 @@
* @returns { Promise<void> }
*/
exports.up = async function up(knex) {
await knex.schema.createTable('policy-metadata', table => {
table.increments('id').primary();
table.string('policy').primary();
table.string('source');
});
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 });
}
});
};

/**
Expand Down
36 changes: 31 additions & 5 deletions plugins/rbac-backend/migrations/20231221113214_migrations.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,37 @@
* @returns { Promise<void> }
*/
exports.up = async function up(knex) {
await knex.schema.createTable('role-metadata', table => {
table.increments('id').primary();
table.string('roleEntityRef').primary();
table.string('source');
});
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 = [];
for (const groupPolicy of listGroupPolicies) {
const { v1 } = groupPolicy;
allGroupPolicies.push(v1);
}
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 });
}
});
};

/**
Expand Down
4 changes: 2 additions & 2 deletions plugins/rbac-backend/src/helper.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export function policyToString(policy: string[]): string {
return `[${policy.join(',')}]`;
return `[${policy.join(', ')}]`;
}

export function policiesToString(policies: string[][]): string {
Expand All @@ -10,5 +10,5 @@ export function policiesToString(policies: string[][]): string {
}

export function metadataStringToPolicy(policy: string): string[] {
return policy.replace('[', '').replace(']', '').split(',');
return policy.replace('[', '').replace(']', '').split(', ');
}
32 changes: 16 additions & 16 deletions plugins/rbac-backend/src/service/enforcer-delegate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ export class EnforcerDelegate {
if (!ok) {
throw new Error(`failed to create policy ${policyToString(policy)}`);
}
addMetadataTrx.commit();
await addMetadataTrx.commit();
} catch (err) {
addMetadataTrx.rollback();
await addMetadataTrx.rollback();
throw err;
}
}
Expand All @@ -87,9 +87,9 @@ export class EnforcerDelegate {
`Failed to store policies ${policiesToString(policies)}`,
);
}
addMetadataTrx.commit();
await addMetadataTrx.commit();
} catch (err) {
addMetadataTrx.rollback();
await addMetadataTrx.rollback();
throw err;
}
}
Expand All @@ -107,9 +107,9 @@ export class EnforcerDelegate {
if (!ok) {
throw new Error(`failed to create policy ${policyToString(policy)}`);
}
addMetadataTrx.commit();
await addMetadataTrx.commit();
} catch (err) {
addMetadataTrx.rollback();
await addMetadataTrx.rollback();
throw err;
}
}
Expand All @@ -134,9 +134,9 @@ export class EnforcerDelegate {
`Failed to store policies ${policiesToString(policies)}`,
);
}
addMetadataTrx.commit();
await addMetadataTrx.commit();
} catch (err) {
addMetadataTrx.rollback();
await addMetadataTrx.rollback();
throw err;
}
}
Expand All @@ -152,9 +152,9 @@ export class EnforcerDelegate {
if (!ok) {
throw new Error(`fail to delete policy ${policy}`);
}
rmMetadataTrx.commit();
await rmMetadataTrx.commit();
} catch (err) {
rmMetadataTrx.rollback(err);
await rmMetadataTrx.rollback(err);
throw err;
}
}
Expand All @@ -176,9 +176,9 @@ export class EnforcerDelegate {
`Failed to delete policies ${policiesToString(policies)}`,
);
}
rmMetadataTrx.commit();
await rmMetadataTrx.commit();
} catch (err) {
rmMetadataTrx.rollback(err);
await rmMetadataTrx.rollback(err);
throw err;
}
}
Expand All @@ -196,9 +196,9 @@ export class EnforcerDelegate {
if (!ok) {
throw new Error(`Failed to delete policy ${policyToString(policy)}`);
}
rmMetadataTrx.commit();
await rmMetadataTrx.commit();
} catch (err) {
rmMetadataTrx.rollback(err);
await rmMetadataTrx.rollback(err);
throw err;
}
}
Expand All @@ -219,9 +219,9 @@ export class EnforcerDelegate {
`Failed to delete grouping policies: ${policiesToString(policies)}`,
);
}
rmMetadataTrx.commit();
await rmMetadataTrx.commit();
} catch (err) {
rmMetadataTrx.rollback(err);
await rmMetadataTrx.rollback(err);
throw err;
}
}
Expand Down
12 changes: 6 additions & 6 deletions plugins/rbac-backend/src/service/permission-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const useAdmins = async (
adminRoleName,
trx,
);
trx.commit();
await trx.commit();
}

admins.flatMap(async localConfig => {
Expand Down Expand Up @@ -152,9 +152,9 @@ const addPredefinedPoliciesAndGroupPolicies = async (
await roleMetadataStorage.removeRoleMetadata(roleMeta, delRoleMetaTrx);
}
await enf.removeGroupingPolicies(groupPoliciesToDelete, true);
delRoleMetaTrx.commit();
await delRoleMetaTrx.commit();
} catch (err) {
delRoleMetaTrx.rollback();
await delRoleMetaTrx.rollback();
throw err;
}
await enf.removePolicies(policiesToDelete, true);
Expand Down Expand Up @@ -188,9 +188,9 @@ const addPredefinedPoliciesAndGroupPolicies = async (
trx,
);
await enf.addGroupingPolicy(groupPolicy, 'csv-file');
trx.commit();
await trx.commit();
} catch (err) {
trx.rollback();
await trx.rollback();
throw err;
}
}
Expand Down Expand Up @@ -239,7 +239,7 @@ async function validateGroupingPolicy(
}

const metadata = await roleMetadataStorage.findRoleMetadata(parent);
if (metadata && metadata.source !== source) {
if (metadata && metadata.source !== source && metadata.source !== 'legacy') {
throw new Error(
`You could not add user or group to the role created with source ${metadata.source}`,
);
Expand Down
12 changes: 6 additions & 6 deletions plugins/rbac-backend/src/service/policies-rest-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,9 +367,9 @@ export class PolicesServer {
createTrx,
);
await this.enforcer.addGroupingPolicies(roles, 'rest');
createTrx.commit();
await createTrx.commit();
} catch (trxErr) {
createTrx.rollback();
await createTrx.rollback();
throw trxErr;
}

Expand Down Expand Up @@ -490,9 +490,9 @@ export class PolicesServer {
// So, let's compensate this combination delete + create.
await this.enforcer.removeGroupingPolicies(oldRole, false);
await this.enforcer.addGroupingPolicies(newRole, 'rest');
updateTrx.commit();
await updateTrx.commit();
} catch (trxErr) {
updateTrx.rollback();
await updateTrx.rollback();
throw trxErr;
}

Expand Down Expand Up @@ -548,9 +548,9 @@ export class PolicesServer {
if (roleMembers.length === 0) {
await this.roleMetadata.removeRoleMetadata(roleEntityRef, rmTrx);
}
rmTrx.commit();
await rmTrx.commit();
} catch (trxErr) {
rmTrx.rollback();
await rmTrx.rollback();
throw trxErr;
}

Expand Down
3 changes: 2 additions & 1 deletion plugins/rbac-common/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
export type Source =
| 'rest' // created via REST API
| 'csv-file' // created via policies-csv-file with defined path in the application configuration
| 'configuration'; // created from application configuration
| 'configuration' // created from application configuration
| 'legacy'; // preexisting policies

export type RoleSource = Source | 'default'; // hard coded in the plugin code

Expand Down

0 comments on commit d85afe9

Please sign in to comment.