Support for policy query #143
Conversation
| @@ -33,12 +33,13 @@ export class PolicyExecutor { | |||
| const policyDefinition = this.getPolicyDefinition(policy.namespace, policy.name); | |||
|
|
|||
| const args = policyDefinition.args && this.preparePolicyArgs(policyDefinition.args, policy); | |||
| // TODO: evaluate queries | |||
|
|
|||
| const query = policyDefinition.query && (await this.evaluatePolicyQuery(policyDefinition.query, args)); | |||
There was a problem hiding this comment.
It should also support param injection from jwt in addition to args, though this is something I'm adding in my next PR. We should probably wait until after both PRs are merged and then add jwt injection support to queries.
Probably worth adding a TODO for it though to be safe
| @@ -70,4 +71,20 @@ export class PolicyExecutor { | |||
| if (!policyDefinition) throw new Error(`The policy ${name} in namespace ${namespace} was not found`); | |||
| return policyDefinition; | |||
| } | |||
|
|
|||
| private async evaluatePolicyQuery(query: PolicyQuery, args: PolicyArgsObject = {}): Promise<any> { | |||
There was a problem hiding this comment.
The return value should be Promise<QueryResults> instead of Promise<any>
|
|
||
| private async evaluatePolicyQuery(query: PolicyQuery, args: PolicyArgsObject = {}): Promise<any> { | ||
| let variableValues = | ||
| query.variables && |
There was a problem hiding this comment.
The query definition is very different from the spec, if you make changes you should also update the spec to match this
| case 'exports': | ||
| return context.exports.resolve(info.parentType, parent, propName); | ||
| return context.exports.resolve(info.parentType, parent, template); |
There was a problem hiding this comment.
I don't think template is a good name for this variable after the changes, maybe propPath (and change the existing propPath to propPathArray or something)
|
|
||
| test('Query allowed employee', async () => { | ||
| const response: AllowedEmployeeQueryResponse = await gatewayClient.request(employeeQuery('allowedEmployee')); | ||
| expect(response.allowedEmployee.address).toBeDefined(); |
There was a problem hiding this comment.
Why not check the actual value of the address? should be pretty simple here
There was a problem hiding this comment.
I'll add snapshot validation
AleF83
force-pushed
the
auth-policy-queries
branch
from
ff4e70d
to
3a8678c
Compare
| case 'jwt': | ||
| return getJwt(context)[propName]; | ||
| return getJwt(context)[key]; |
There was a problem hiding this comment.
| return getJwt(context)[key]; | |
| return R.path(propPath, getJwt(context)); |
We can support the deep nesting here as well.
getJwt will return an empty object if there is not JWT so there is no need for an initial existence check.
* Authorization - fully implemented registry part (#133) This includes: Create/update policy resource Attachments support for policy resource (with support for writing the attachment to both s3 and fs repositories) Opa policy type implementation, including compiling rego code to wasm and adding that to the policy as an attachment * Authorization gateway - basic features (#138) implemented full flow with basic features Implement local policy attachment caching for all resource repositories * Add policy definitions and attachments to request context, change pol… (#141) * Add policy definitions and attachments to request context, change policy executor to use them from context instead of directly from repo * PR comments * allow jwt in param injection (policy authorization can use it through args) (#144) * Support for policy query (#143) * Policy directive - accept only a single policy (#146) * change policy directive to accept only a single policy * Refactored PolicyExecutor API to only expose static methods Co-authored-by: Tomer Eskenazi <tomeresk@gmail.com>
No description provided.