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 9, 2024
1 parent 669e085 commit 2612a3d
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 18 deletions.
54 changes: 38 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 Expand Up @@ -265,4 +265,26 @@ export class EnforcerDelegate {
): Promise<PermissionPolicyMetadataDao[]> {
return await this.metadataStorage.findPolicyMetadataBySource(source);
}

async getPoliciesWithoutSource(): Promise<string[][]> {
const policiesWithoutSource: string[][] = [];
const allPolicies = await this.enforcer.getPolicy();
for (const policy of allPolicies) {
const sourcePolicy =
await this.metadataStorage.findPolicyMetadata(policy);
if (!sourcePolicy) {
policiesWithoutSource.push(policy);
}
}
return policiesWithoutSource;
}

async migratePreexistingPolicies(enforcer: Enforcer): Promise<void> {
const policies = await this.getPoliciesWithoutSource();

for (const policy of policies) {
await enforcer.removePolicy(...policy);
await this.addPolicy(policy, 'legacy');
}
}
}
23 changes: 22 additions & 1 deletion plugins/rbac-backend/src/service/permission-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ async function addRoleMetadata(
}
}

const legacyPolicies = async (enf: EnforcerDelegate, policy: string[]) => {
if (
(await enf.hasPolicy(...policy)) &&
(await enf.getMetadata(policy)).source === 'legacy'
) {
await enf.removePolicy(policy);
}
};

const useAdmins = async (
admins: Config[],
enf: EnforcerDelegate,
Expand All @@ -55,7 +64,7 @@ const useAdmins = async (
if (!adminRoleMeta) {
const trx = await knex.transaction();
await roleMetadataStorage.createRoleMetadata(
{ source: 'default' },
{ source: 'configuration' },
adminRoleName,
trx,
);
Expand All @@ -71,15 +80,20 @@ const useAdmins = async (
});

const adminReadPermission = [adminRoleName, 'policy-entity', 'read', 'allow'];

await legacyPolicies(enf, adminReadPermission);
if (!(await enf.hasPolicy(...adminReadPermission))) {
await enf.addPolicy(adminReadPermission, 'configuration');
}

const adminCreatePermission = [
adminRoleName,
'policy-entity',
'create',
'allow',
];

await legacyPolicies(enf, adminCreatePermission);
if (!(await enf.hasPolicy(...adminCreatePermission))) {
await enf.addPolicy(adminCreatePermission, 'configuration');
}
Expand All @@ -90,6 +104,8 @@ const useAdmins = async (
'delete',
'allow',
];

await legacyPolicies(enf, adminDeletePermission);
if (!(await enf.hasPolicy(...adminDeletePermission))) {
await enf.addPolicy(adminDeletePermission, 'configuration');
}
Expand All @@ -100,6 +116,8 @@ const useAdmins = async (
'update',
'allow',
];

await legacyPolicies(enf, adminUpdatePermission);
if (!(await enf.hasPolicy(...adminUpdatePermission))) {
await enf.addPolicy(adminUpdatePermission, 'configuration');
}
Expand Down Expand Up @@ -166,6 +184,9 @@ const addPredefinedPoliciesAndGroupPolicies = async (
`Failed to validate policy from file ${preDefinedPoliciesFile}. Cause: ${err.message}`,
);
}

await legacyPolicies(enf, policy);

if (!(await enf.hasPolicy(...policy))) {
await enf.addPolicy(policy, 'csv-file');
}
Expand Down
2 changes: 2 additions & 0 deletions plugins/rbac-backend/src/service/policy-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ export class PolicyBuilder {
knex,
);

await enforcerDelegate.migratePreexistingPolicies(enf);

const options: RouterOptions = {
config: env.config,
logger: env.logger,
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 2612a3d

Please sign in to comment.