Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"test": "TS_NODE_PROJECT='tests/tsconfig.json' TS_NODE_FILES=true nyc mocha --opts ./.mocha.opts",
"check-node-version": "check-node-version --npm 11.6.2 --print",
"test:ci": "npm test -- --forbid-only",
"eslint": "eslint '{,!(node_modules|dist)/**/}*.js'",
"eslint": "eslint '{,!(node_modules|dist)/**/}*.{js,ts}'",
"markdownlint": "markdownlint-cli2",
"standards": "npm run commitlint && npm run markdownlint && npm run eslint",
"release:preview": "node ./node_modules/@silvermine/standardization/scripts/release.js preview",
Expand Down
6 changes: 6 additions & 0 deletions src/SubjectAuthorizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import {
PolicyEffect,
PolicyWithID,
ISubjectAuthorizerOpts,
HasPolicyGrantingOpts,
} from './fsaba-types';
import isAllowed from './utils/is-allowed';
import makeSubjectSpecificPolicies from './utils/make-subject-specific-policies';
import hasPolicyGranting from './utils/has-policy-granting';

export class SubjectAuthorizer implements ISubjectAuthorizer {

Expand Down Expand Up @@ -43,4 +45,8 @@ export class SubjectAuthorizer implements ISubjectAuthorizer {
return isAllowed(this._policies, action, resource, opts);
}

public hasPolicyGranting(action: string, opts?: HasPolicyGrantingOpts): boolean {
return hasPolicyGranting(this._policies, action, opts);
}

}
91 changes: 88 additions & 3 deletions src/fsaba-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,28 @@ export interface IAuthorizerFactory {
*/
export interface IsAllowedOpts {
context: StringMap;

/**
* When true, policy conditions are bypassed, only action and resource patterns are
* evaluated. Useful for UI scenarios where you need to know if a user *could* have
* access to an action (e.g., showing a button) before the runtime context required by
* conditions is available.
*/
ignoreConditions: boolean;
}

export interface HasPolicyGrantingOpts {

/**
* An optional resource prefix pattern (e.g. 'budgets:kazoo/*'). If provided, the
* authorizer checks if any policy grants the action on a resource that matches this
* prefix.
*
* Note: This must end with a wildcard (`*`) and cannot contain wildcards elsewhere.
*/
resourcePrefixPattern?: string;
}

/**
* A subject authorizer is used to determine what actions each subject can perform.
*/
Expand All @@ -276,12 +295,78 @@ export interface ISubjectAuthorizer {
* Is the subject allowed to perform this action on the given resource? Optionally,
* provide a context that may be evaluated by policy conditions. In rare cases, you may
* want to ignore conditions when determining if the user would have access to an
* action.
*
* TODO: document more on when you'd ignore conditions.
* action—for example, to decide whether to render a UI element before runtime context
* is available.
*/
isAllowed(action: string, resource: string, opts?: Partial<IsAllowedOpts>): boolean;

/**
* Perform an early check to see if the user has a policy allowing them to perform the
* provided action at all, with an optional resource prefix. This is useful for APIs to
* perform an early check before doing expensive processing to load any data needed to
* determine if the user has access to perform the action.
*
* This does not replace the need to call `isAllowed` to determine if the user has
* access to perform the action. This is a tool that can be used to reduce the
* information disclosed (e.g. via timing-based probing) to users who don't have access
* to perform an action at all.
Copy link
Member

Choose a reason for hiding this comment

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

Because this is a more complex security subject, could we add an example scenario where this should be used, clearly showing where isAllowed is still needed?

Example: A request is made for budgets/it-dept/fy2025/q1. Your code needs to: (1) validate query params, and (2) load the IT Department budget for Q1 of FY2025 to see who has permission to view it (before an isAllowed call could be made). If the query params seem invalid, or if there is no budget for those params, you'd return a Bad Response or Not Found response. But, if the user doesn't have budget:View for any combination of departments, etc, then there's no reason to even perform those steps and potentially expose information through the Bad Request or Not Found responses.

Instead, you could follow this flow:

  • hasPolicyGranting('budget:View') - if they don't, then stop right there; if they do have a policy granting them view rights to a budget, it still may not be for this budget, but you can now continue your flow ...
  • validate query params; okay to return Bad Response
  • look up the requested budget; may or may not be okay to return a 404 Not Found here, depending on your security requirements
  • if you did find that budget item, now, check if they have permission to view this budget with isAllowed

*
* Scenario: A request is made for `budgets/it-dept/fy2025/q1`. Your code needs to:
* (1) validate query params, and (2) load the IT Department budget for Q1 of FY2025
* to see who has permission to view it (before an `isAllowed` call could be made). If
* the query params seem invalid, or if there is no budget for those params, you'd
* return a "Bad Request" or "Not Found" response. But, if the user doesn't have
* `budget:View` for any combination of departments, etc, then there's no reason to
* even perform those steps and potentially expose information through the "Bad
* Request" or "Not Found" responses.
*
* Using this method, you could follow this flow:
*
* 1. hasPolicyGranting('budget:View') - if they don't have this, then stop right
* there; if they do have a policy granting them view rights to a budget, they
* might not be for this budget, but you can now continue your flow
* 2. Validate query params; may be okay to return Bad Response depending on your
* security requirements
* 3. Look up the requested budget; may or may not be okay to return a 404 Not Found
* depending on your security requirements
* 4. If you did find that budget item, now, check if they have permission to view
* this budget with isAllowed
*
* In pseudocode, this might look like:
*
* ```
* // A request is made for budgets/it-dept/fy2025/q1.
* // 1. Check if the user has ANY policy granting 'budget:View'
* if (!authorizer.hasPolicyGranting('budget:View')) {
* return 403 Forbidden;
* }
*
* // 2. Validate query params
* if (!params.isValid()) {
* // Possibly safe to let the user know the request is invalid because we know
* // they have SOME view rights
* return 400 Bad Request;
* }
*
* // 3. Load the specific resource
* const budget = await loadBudget('it-dept', 'fy2025', 'q1');
*
* if (!budget) {
* // Returning a 403 Forbidden vs 404 Not Found here depends on your security
* // requirements.
* return 403 Forbidden;
* }
*
* // 4. Perform final check for this specific budget
* if (!authorizer.isAllowed('budget:View', budget.id, { context: budget.context })) {
* return 403 Forbidden;
* }
*
* return 200 OK;
* ```
*/
hasPolicyGranting(action: string, opts?: HasPolicyGrantingOpts): boolean;

}

/**
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from './fsaba-types';
export * from './AuthorizerFactory';
export * from './SubjectAuthorizer';
35 changes: 35 additions & 0 deletions src/utils/has-policy-granting.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { HasPolicyGrantingOpts, PolicyWithID } from '../fsaba-types';
import stringMatchesPattern from './string-matches-pattern';

/**
* Perform an early check to see if the user has a policy to perform an action at all,
* with an optional resource prefix.
*/
export default function hasPolicyGranting(policies: { allow: readonly PolicyWithID[] }, action: string, opts?: HasPolicyGrantingOpts): boolean {
return policies.allow.some((policy) => {
const actionMatches = policy.actions.some((policyAction) => {
return stringMatchesPattern(policyAction, action);
});

if (!actionMatches) {
return false;
}

const resourcePrefixPattern = opts?.resourcePrefixPattern;

if (!resourcePrefixPattern) {
return true;
}

if (!resourcePrefixPattern.endsWith('*') || resourcePrefixPattern.indexOf('*') !== resourcePrefixPattern.length - 1) {
throw new Error('resourcePrefixPattern must end with a wildcard and cannot contain wildcards elsewhere');
}

const prefix = resourcePrefixPattern.slice(0, -1);

return policy.resources.some((policyResource) => {
return stringMatchesPattern(policyResource, resourcePrefixPattern)
|| policyResource.startsWith(prefix);
});
});
}
95 changes: 82 additions & 13 deletions tests/SubjectAuthorizer.test.ts
Original file line number Diff line number Diff line change
@@ -1,42 +1,111 @@
import { expect } from 'chai';
import { AuthorizerFactory, Claims } from '../src';
import { ADMINISTER_OWN_AUTH, ADMINISTER_OTHER_AUTH, DO_NEARLY_EVERYTHING } from './sample-data';
import { AuthorizerFactory, Claims, SubjectAuthorizer } from '../src';
import {
ADMINISTER_OWN_AUTH,
ADMINISTER_OTHER_AUTH,
DO_NEARLY_EVERYTHING,
ALL_ROLES,
BUDGET_VIEWER_KAZOO_PM,
BUDGET_VIEWER_SINGLE,
BUDGET_VIEWER_MFG_HEAD,
} from './sample-data';


describe('SubjectAuthorizer', () => {
const ALL_ROLES_1 = [
const allRoles1 = [
ADMINISTER_OWN_AUTH,
ADMINISTER_OTHER_AUTH,
];

const ALL_ROLES_2 = [
const allRoles2 = [
ADMINISTER_OWN_AUTH,
ADMINISTER_OTHER_AUTH,
DO_NEARLY_EVERYTHING,
];

const USER_ID = '73885b55-2e0d-40bd-8cb3-2e59cf78ed87';
const userID = '73885b55-2e0d-40bd-8cb3-2e59cf78ed87';

const USER: Claims = {
subjectID: USER_ID,
const user: Claims = {
subjectID: userID,
roles: [
{ roleID: ADMINISTER_OWN_AUTH.roleID },
{ roleID: ADMINISTER_OTHER_AUTH.roleID, contextValue: '8e0fa760-9a1d-43ea-8686-768318d923b4' },
{ roleID: DO_NEARLY_EVERYTHING.roleID },
],
};

const factory1 = new AuthorizerFactory(ALL_ROLES_1),
factory2 = new AuthorizerFactory(ALL_ROLES_2);
const factory1 = new AuthorizerFactory(allRoles1),
factory2 = new AuthorizerFactory(allRoles2);

it('ignores unknown roles when no opts provided', () => {
expect(factory1.makeAuthorizerForSubject(USER)).to.be.ok; // eslint-disable-line no-unused-expressions
expect(factory2.makeAuthorizerForSubject(USER)).to.be.ok; // eslint-disable-line no-unused-expressions
expect(factory1.makeAuthorizerForSubject(user)).to.be.ok; // eslint-disable-line no-unused-expressions
expect(factory2.makeAuthorizerForSubject(user)).to.be.ok; // eslint-disable-line no-unused-expressions
});

it('throws an error on unknown roles when requested', () => {
expect(() => { factory1.makeAuthorizerForSubject(USER, { throwOnUnknownRole: true }); }).to.throw();
expect(factory2.makeAuthorizerForSubject(USER, { throwOnUnknownRole: true })).to.be.ok; // eslint-disable-line no-unused-expressions
expect(() => { factory1.makeAuthorizerForSubject(user, { throwOnUnknownRole: true }); }).to.throw();
expect(factory2.makeAuthorizerForSubject(user, { throwOnUnknownRole: true })).to.be.ok; // eslint-disable-line no-unused-expressions
});

describe('hasPolicyGranting', () => {

it('returns true if the user has a policy for the action', () => {
const authorizer = new SubjectAuthorizer(ALL_ROLES, BUDGET_VIEWER_SINGLE);

expect(authorizer.hasPolicyGranting('budget:View')).to.strictlyEqual(true);
});

it('returns false if the user does not have a policy for the action', () => {
const authorizer = new SubjectAuthorizer(ALL_ROLES, BUDGET_VIEWER_SINGLE);

expect(authorizer.hasPolicyGranting('budget:Create')).to.strictlyEqual(false);
});

it('returns true if the user has a policy matching resource prefix', () => {
const authorizer = new SubjectAuthorizer(ALL_ROLES, BUDGET_VIEWER_KAZOO_PM);

expect(authorizer.hasPolicyGranting('budget:View', { resourcePrefixPattern: 'budget:kazoo/*' })).to.strictlyEqual(true);
});

it('returns true if the policy resource is broader than the requested prefix', () => {
const authorizer = new SubjectAuthorizer(ALL_ROLES, BUDGET_VIEWER_MFG_HEAD);

// BUDGET_VIEWER_MFG_HEAD has budget:*
expect(authorizer.hasPolicyGranting('budget:View', { resourcePrefixPattern: 'budget:kazoo/mktg/*' })).to.strictlyEqual(true);
});

it('returns false if the user has the action but on different resources', () => {
const authorizer = new SubjectAuthorizer(ALL_ROLES, BUDGET_VIEWER_KAZOO_PM);

// KAZOO PM has budget:View on budget:* but we can test with a non-matching
// prefix.
expect(authorizer.hasPolicyGranting('budget:View', { resourcePrefixPattern: 'budget-v2/*' })).to.strictlyEqual(false);
});

it('throws an error if resourcePrefixPattern does not end with a wildcard or contains internal wildcards', () => {
const authorizer = new SubjectAuthorizer(ALL_ROLES, BUDGET_VIEWER_KAZOO_PM);

expect(() => { authorizer.hasPolicyGranting('budget:View', { resourcePrefixPattern: 'budget:kazoo' }); }).to.throw();

expect(() => { authorizer.hasPolicyGranting('budget:View', { resourcePrefixPattern: 'budget:*/foo' }); }).to.throw();

expect(() => { authorizer.hasPolicyGranting('budget:View', { resourcePrefixPattern: '*budget:foo' }); }).to.throw();
});

it('correctly handles prefix match', () => {
const authorizer = new SubjectAuthorizer(ALL_ROLES, BUDGET_VIEWER_KAZOO_PM);

// BUDGET_VIEWER_KAZOO_PM has policy for budget:kazoo/*. The requested prefix is
// budget:kazoo/mktg/*. This should be true because the policy resource
// (budget:kazoo/*) matches the requested prefix (budget:kazoo/mktg/*).
expect(authorizer.hasPolicyGranting('budget:View', { resourcePrefixPattern: 'budget:kazoo/mktg/*' })).to.strictlyEqual(true);

// Now, if the requested prefix is budget:kaz*, it should still be true because
// the requested resource prefix (budget:kaz*) matches the policy resource
// (budget:kazoo/*).
expect(authorizer.hasPolicyGranting('budget:View', { resourcePrefixPattern: 'budget:kaz*' })).to.strictlyEqual(true);
});

});

});