Skip to content

allow multiple policies #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
charliemitchell merged 2 commits into from
Nov 17, 2025
Merged

allow multiple policies #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58565d3
allow multiple policies
charliemitchell Nov 17, 2025
ad718b7
Update classes/policy.js
charliemitchell Nov 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions classes/policy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,44 +12,75 @@ module.exports = class Policy {
this.policies = policies;
this.logging = logging;
this.debug = debug('glad');
this.policyList = this.normalizePolicies(policy);
this.rejected = false;
}

normalizePolicies (policy) {
if (!policy && policy !== 0) {
return [];
}

if (Array.isArray(policy)) {
return policy.filter(Boolean);
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent handling of 0 as a policy identifier. Line 20 explicitly allows 0 as a valid policy when passed as a single value (policy !== 0), but line 25 filters it out when it's in an array since filter(Boolean) removes all falsy values including 0.

If 0 should be a valid policy identifier, consider using a more explicit filter:

return policy.filter(p => p !== null && p !== undefined && p !== false && p !== '');

Or if 0 shouldn't be valid, remove the special case check on line 20.

Suggested change
return policy.filter(Boolean);
return policy.filter(p => p !== null && p !== undefined && p !== false && p !== '');

Copilot uses AI. Check for mistakes.
}

return [policy];
}

restrict (req, res) {
this.debug('Policy: evaluating > %s', req.id);
req.controller = this.controller.name;
req.action = this.action;
this.rejected = false;

if (this.logging) {
chalk.info(`Routing ${req.id} to ${this.controller.name}#${this.action}`);
}

if (this.policy) {
this.debug('Policy: lookup %s > %s', this.policy, req.id);
if (this.policies[this.policy]) {
this.debug('Policy: apply %s > %s', this.policy, req.id);
this.policies[this.policy](req, res, this.acceptor(req, res), this.rejector(req, res));
} else {
this.debug('Policy: %s not found [error] %s', this.policy, req.id);
chalk.error(`Policy Error: ${req.id || ''} The policy "${this.policy}" does not exist, therefore the request was denied`);
this.policies.onFailure(req, res);
if (this.policyList.length) {
const reject = this.rejector(req, res);

for (let i = 0; i < this.policyList.length; i++) {
const name = this.policyList[i];

this.debug('Policy: lookup %s > %s', name, req.id);

if (!this.policies[name]) {
this.debug('Policy: %s not found [error] %s', name, req.id);
chalk.error(`Policy Error: ${req.id || ''} The policy "${name}" does not exist, therefore the request was denied`);
reject();
break;
}

const accept = this.acceptor(req, res, i);
this.debug('Policy: apply %s > %s', name, req.id);
this.policies[name](req, res, accept, reject);
}
Comment on lines +44 to 59
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All policies in the array are invoked synchronously in the loop, even if an earlier policy calls reject(). The rejected flag only prevents the controller from being executed, but subsequent policies will still run.

This means if correctPolicy accepts and rejectingPolicy rejects, secondPolicy (if present after it) will still be invoked and execute its logic, even though the request has already been rejected.

Consider checking this.rejected status before invoking each policy, or breaking out of the loop when a policy rejects:

if (this.rejected) {
  break;
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

@charliemitchell charliemitchell Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Copy link
Copy Markdown
Collaborator Author

@charliemitchell charliemitchell Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Copy link
Copy Markdown
Collaborator Author

@charliemitchell charliemitchell Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

Copy link
Copy Markdown
Collaborator Author

@charliemitchell charliemitchell Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

} else {
this.debug('Policy: no policy to apply > %s', req.id);
this.runControllerMethod(req, res);
}
}

acceptor (req, res) {
acceptor (req, res, index) {
return () => {
this.debug('Policy:accept > %s', req.id);
this.runControllerMethod(req, res);
const isLastPolicy = this.policyList.length === index + 1;
if (isLastPolicy && !this.rejected) {
this.runControllerMethod(req, res);
}
};
}

rejector (req, res) {
return (custom) => {
if (this.rejected) {
return;
}
this.debug('Policy:reject > %s', req.id);
return this.policies.onFailure(req, res, custom)
this.rejected = true;
return this.policies.onFailure(req, res, custom);
};
}

Expand Down
Loading