Skip to content

Commit 46df84f

Browse files
Copilothsluoyz
andcommitted
refactor: address code review feedback
Co-authored-by: hsluoyz <3787410+hsluoyz@users.noreply.github.com>
1 parent 8be36a2 commit 46df84f

1 file changed

Lines changed: 27 additions & 32 deletions

File tree

src/coreEnforcer.ts

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,26 @@ export class CoreEnforcer {
426426
}
427427
}
428428

429+
/**
430+
* Helper method to populate request parameters with JSON parsing support
431+
*/
432+
private populateRequestParameters(parameters: { [key: string]: any }, rTokens: string[] | undefined, rvals: any[]): void {
433+
if (this.acceptJsonRequest) {
434+
// Attempt to parse each request parameter as JSON; continue with string if failed
435+
rTokens?.forEach((token, j) => {
436+
try {
437+
parameters[token] = JSON.parse(rvals[j]);
438+
} catch {
439+
parameters[token] = rvals[j];
440+
}
441+
});
442+
} else {
443+
rTokens?.forEach((token, j): void => {
444+
parameters[token] = rvals[j];
445+
});
446+
}
447+
}
448+
429449
private *privateEnforce(
430450
asyncCompile = true,
431451
explain = false,
@@ -479,10 +499,11 @@ export class CoreEnforcer {
479499

480500
const effectStream = this.eft.newStream(effectExpr);
481501

482-
// Check if the matcher references policy tokens (e.g., p.sub, p.obj)
483-
// Note: escapeAssertion transforms "p." to "p_", so we check for "p_" in the escaped expression
502+
// Check if the matcher references policy tokens (e.g., p.sub, p.obj, p2.sub)
503+
// Note: escapeAssertion transforms "p." to "p_", so we check for "p_" or "p<digit>_" in the escaped expression
484504
// If the matcher doesn't reference policy tokens, we can skip policy iteration (no-policy ABAC)
485-
const matcherUsesPolicyTokens = expString.includes(`${enforceContext.pType}_`);
505+
const policyTokenPattern = new RegExp(`${enforceContext.pType}\\d*_`);
506+
const matcherUsesPolicyTokens = policyTokenPattern.test(expString);
486507

487508
if (policyLen && policyLen !== 0 && matcherUsesPolicyTokens) {
488509
for (let i = 0; i < policyLen; i++) {
@@ -492,20 +513,7 @@ export class CoreEnforcer {
492513
throw new Error(`invalid request size: expected ${rTokensLen}, got ${rvals.length}, rvals: ${rvals}"`);
493514
}
494515

495-
if (this.acceptJsonRequest) {
496-
// Attempt to parse each request parameter as JSON; continue with string if failed
497-
rTokens.forEach((token, j) => {
498-
try {
499-
parameters[token] = JSON.parse(rvals[j]);
500-
} catch {
501-
parameters[token] = rvals[j];
502-
}
503-
});
504-
} else {
505-
rTokens.forEach((token, j) => {
506-
parameters[token] = rvals[j];
507-
});
508-
}
516+
this.populateRequestParameters(parameters, rTokens, rvals);
509517

510518
p?.tokens.forEach((token, j) => {
511519
parameters[token] = p?.policy[i][j];
@@ -578,28 +586,15 @@ export class CoreEnforcer {
578586
}
579587
} else {
580588
// When matcher doesn't use policy tokens or there are no policies
581-
if (HasEval && (!policyLen || policyLen === 0)) {
589+
if (HasEval && !policyLen) {
582590
throw new Error('please make sure rule exists in policy when using eval() in matcher');
583591
}
584592

585593
explainIndex = 0;
586594

587595
const parameters: { [key: string]: any } = {};
588596

589-
if (this.acceptJsonRequest) {
590-
// Attempt to parse each request parameter as JSON; continue with string if failed
591-
rTokens?.forEach((token, j) => {
592-
try {
593-
parameters[token] = JSON.parse(rvals[j]);
594-
} catch {
595-
parameters[token] = rvals[j];
596-
}
597-
});
598-
} else {
599-
rTokens?.forEach((token, j): void => {
600-
parameters[token] = rvals[j];
601-
});
602-
}
597+
this.populateRequestParameters(parameters, rTokens, rvals);
603598

604599
p?.tokens?.forEach((token) => {
605600
parameters[token] = '';

0 commit comments

Comments
 (0)