From 5919e03a0445f11db92e5332aec431551d65649f Mon Sep 17 00:00:00 2001 From: AleF83 Date: Wed, 24 Jun 2020 09:32:33 +0300 Subject: [PATCH 1/8] params injection supports deep paths --- services/package-lock.json | 18 +++++++++++++++ services/package.json | 2 ++ services/src/modules/paramInjection.ts | 31 +++++++++++++------------- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/services/package-lock.json b/services/package-lock.json index bfe05280..1728127a 100644 --- a/services/package-lock.json +++ b/services/package-lock.json @@ -1111,6 +1111,14 @@ "integrity": "sha512-eAdu+NW1IkCdmp85SnhyKha+OOREQMT9lXaoICQxa7bhSauRiLzu3WSNt9Mf2piuJvWeXF/G0hGWHr63xNpIRA==", "dev": true }, + "@types/ramda": { + "version": "0.27.6", + "resolved": "https://registry.npmjs.org/@types/ramda/-/ramda-0.27.6.tgz", + "integrity": "sha512-ephagb0ZIAJSoS5I/qMS4Mqo1b/Nd50pWM+o1QO/dz8NF//GsCGPTLDVRqgXlVncy74KShfHzE5rPZXTeek4PA==", + "requires": { + "ts-toolbelt": "^6.3.3" + } + }, "@types/range-parser": { "version": "1.2.3", "resolved": "https://registry.npmjs.org/@types/range-parser/-/range-parser-1.2.3.tgz", @@ -6209,6 +6217,11 @@ "resolved": "https://registry.npmjs.org/quick-format-unescaped/-/quick-format-unescaped-3.0.3.tgz", "integrity": "sha512-dy1yjycmn9blucmJLXOfZDx1ikZJUi6E8bBZLnhPG5gBrVhHXx2xVyqqgKBubVNEXmx51dBACMHpoMQK/N/AXQ==" }, + "ramda": { + "version": "0.27.0", + "resolved": "https://registry.npmjs.org/ramda/-/ramda-0.27.0.tgz", + "integrity": "sha512-pVzZdDpWwWqEVVLshWUHjNwuVP7SfcmPraYuqocJp1yo2U1R7P+5QAfDhdItkuoGqIBnBYrtPp7rEPqDn9HlZA==" + }, "react-is": { "version": "16.12.0", "resolved": "https://registry.npmjs.org/react-is/-/react-is-16.12.0.tgz", @@ -7482,6 +7495,11 @@ } } }, + "ts-toolbelt": { + "version": "6.9.9", + "resolved": "https://registry.npmjs.org/ts-toolbelt/-/ts-toolbelt-6.9.9.tgz", + "integrity": "sha512-5a8k6qfbrL54N4Dw+i7M6kldrbjgDWb5Vit8DnT+gwThhvqMg8KtxLE5Vmnft+geIgaSOfNJyAcnmmlflS+Vdg==" + }, "tsconfig": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/tsconfig/-/tsconfig-7.0.0.tgz", diff --git a/services/package.json b/services/package.json index 633a61e6..f82b33e0 100644 --- a/services/package.json +++ b/services/package.json @@ -15,6 +15,7 @@ "dependencies": { "@apollo/federation": "^0.12.1", "@open-policy-agent/opa-wasm": "^1.1.0", + "@types/ramda": "^0.27.6", "apollo-datasource-rest": "^0.7.0", "apollo-link-context": "^1.0.19", "apollo-link-http": "^1.5.16", @@ -36,6 +37,7 @@ "p-limit": "^2.2.2", "pino": "^5.16.0", "pino-pretty": "^3.6.1", + "ramda": "^0.27.0", "rxjs": "^6.5.4", "tslib": "^1.11.0" }, diff --git a/services/src/modules/paramInjection.ts b/services/src/modules/paramInjection.ts index 777a38ab..0b16fcab 100644 --- a/services/src/modules/paramInjection.ts +++ b/services/src/modules/paramInjection.ts @@ -1,6 +1,7 @@ import {GraphQLResolveInfo} from 'graphql'; import {decode as decodeJwt} from 'jsonwebtoken'; import {RequestContext} from './context'; +import * as R from 'ramda'; interface GraphQLArguments { [key: string]: any; @@ -9,27 +10,27 @@ type jwtData = { [name: string]: any; }; -const paramRegex = /{(\w+\.\w+)}/g; +const paramRegex = /{(source|args|exports)\.(\w+(\.\w+)*)}/g; const authzHeaderPrefix = 'Bearer '; function resolveTemplate( + source: string, template: string, parent: any, args: GraphQLArguments, context: RequestContext, info: GraphQLResolveInfo ) { - const [sourceName, propName] = template.split('.'); - - switch (sourceName.toLowerCase()) { + const propPath = template.split('.'); + switch (source) { case 'source': - return parent && parent[propName]; + return parent && R.path(propPath, parent); case 'args': - return args && args[propName]; + return args && R.path(propPath, args); case 'exports': - return context.exports.resolve(info.parentType, parent, propName); + return context.exports.resolve(info.parentType, parent, template); case 'jwt': - return getJwt(context)[propName]; + return getJwt(context)[template]; default: return null; } @@ -44,13 +45,13 @@ export function injectParameters( ) { let didFindValues = false; let didFindTemplates = false; - const value = template.replace(paramRegex, (_, match) => { - const resolved = resolveTemplate(match, parent, args, context, info); + let value: any = template; + const match = paramRegex.exec(template); + if (match) { + value = resolveTemplate(match[1], match[2], parent, args, context, info); didFindTemplates = true; - didFindValues = didFindValues || (resolved !== null && typeof resolved !== 'undefined'); - return resolved; - }); - + didFindValues = didFindValues || (value !== null && typeof value !== 'undefined'); + } return {value, didFindValues, didFindTemplates}; } @@ -73,7 +74,7 @@ export function resolveParameters( continue; } - parameters[group] = resolveTemplate(group, parent, args, context, info); + parameters[group] = resolveTemplate(group, match[2], parent, args, context, info); } return foundMatches ? parameters : null; From 086c408f0b39f61d9d0457be250a8e74d5463e8e Mon Sep 17 00:00:00 2001 From: AleF83 Date: Wed, 24 Jun 2020 10:58:09 +0300 Subject: [PATCH 2/8] Query model changed --- services/src/modules/directives/policy/opa.ts | 4 +- .../src/modules/directives/policy/types.ts | 6 +-- .../src/modules/resource-repository/types.ts | 22 +++------ services/src/registry.ts | 45 +++---------------- .../src/tests/e2e/tests/authorization.spec.ts | 10 ++--- .../registry/create-resources.spec.ts | 20 +++------ .../registry/update-resources.spec.ts | 16 +++---- 7 files changed, 36 insertions(+), 87 deletions(-) diff --git a/services/src/modules/directives/policy/opa.ts b/services/src/modules/directives/policy/opa.ts index 99fcf13c..fad61886 100644 --- a/services/src/modules/directives/policy/opa.ts +++ b/services/src/modules/directives/policy/opa.ts @@ -25,12 +25,12 @@ function getInput(ctx: PolicyExecutionContext): PolicyInput { const input: PolicyInput = {}; if (ctx.args) input.args = ctx.args; - if (ctx.queries) input.queries = ctx.queries; + if (ctx.query) input.query = ctx.query; return input; } type PolicyInput = { args?: PolicyArgsObject; - queries?: QueriesResults; + query?: QueryResults; }; diff --git a/services/src/modules/directives/policy/types.ts b/services/src/modules/directives/policy/types.ts index f6bd51db..98501d1c 100644 --- a/services/src/modules/directives/policy/types.ts +++ b/services/src/modules/directives/policy/types.ts @@ -12,11 +12,11 @@ export type PolicyExecutionContext = { name: string; policyAttachments: PolicyAttachments; args?: PolicyArgsObject; - queries?: QueriesResults; + query?: QueryResults; }; -export type QueriesResults = { - [name: string]: string; +export type QueryResults = { + [name: string]: any; }; export type PolicyExecutionResult = { diff --git a/services/src/modules/resource-repository/types.ts b/services/src/modules/resource-repository/types.ts index d1dd3b17..83355569 100644 --- a/services/src/modules/resource-repository/types.ts +++ b/services/src/modules/resource-repository/types.ts @@ -44,23 +44,16 @@ export interface Policy extends Resource { type: PolicyType; code: string; args?: PolicyArgsObject; - queries?: PolicyQuery[]; + query?: PolicyQuery; } interface PolicyQuery { - type: PolicyQueryType; - name: string; - graphql?: PolicyQueryGraphQL; - policy?: PolicyQueryPolicy; -} - -interface PolicyQueryGraphQL { - query: string; + source: string; + variables?: PolicyQueryGraphqlVariables; } -interface PolicyQueryPolicy { - policyName: string; - args: PolicyArgsObject; +export interface PolicyQueryGraphqlVariables { + [key: string]: any; } export interface PolicyArgsObject { @@ -87,8 +80,3 @@ enum AuthType { export enum PolicyType { opa = 'opa', } - -export enum PolicyQueryType { - graphql = 'graphql', - policy = 'policy', -} diff --git a/services/src/registry.ts b/services/src/registry.ts index 9a9c8ff0..34316a5d 100644 --- a/services/src/registry.ts +++ b/services/src/registry.ts @@ -11,7 +11,7 @@ import {createSchemaConfig} from './modules/graphqlService'; import * as opaHelper from './modules/opaHelper'; // Importing directly from types because of a typescript or ts-jest bug that re-exported enums cause a runtime error for being undefined // https://github.com/kulshekhar/ts-jest/issues/281 -import {PolicyArgsObject, PolicyType, PolicyQueryType} from './modules/resource-repository/types'; +import {PolicyArgsObject, PolicyType} from './modules/resource-repository/types'; const typeDefs = gql` scalar JSON @@ -108,29 +108,9 @@ const typeDefs = gql` opa } - enum PolicyQueryType { - graphql - policy - } - - input PolicyQueryGraphQLInput { - query: String! - } - - input PolicyQueryPolicyInput { - policyName: String! - args: JSONObject - } - - """ - GraphQL doesn't support unions for input types, otherwise this would be a union of different policy query types. - Instead, the PolicyQueryType enum indicates which policy query type is needed, and there's a property which corresponds to each policy query type, which we validate in the registry. - """ input PolicyQueryInput { - type: PolicyQueryType! - name: String! - graphql: PolicyQueryGraphQLInput - policy: PolicyQueryPolicyInput + source: String! + variables: JSONObject } input PolicyInput { @@ -138,7 +118,7 @@ const typeDefs = gql` type: PolicyType! code: String! args: JSONObject - queries: [PolicyQueryInput!] + query: PolicyQueryInput } `; @@ -187,20 +167,9 @@ interface UpstreamClientCredentialsInput { }; } -interface PolicyQueryGraphQLInput { - query: string; -} - -interface PolicyQueryPolicyInput { - policyName: string; - args: PolicyArgsObject; -} - interface PolicyQueryInput { - type: PolicyQueryType; - name: string; - graphql?: PolicyQueryGraphQLInput; - policy?: PolicyQueryPolicyInput; + source: string; + variables?: {[name: string]: any}; } interface PolicyInput { @@ -208,7 +177,7 @@ interface PolicyInput { type: PolicyType; code: string; args?: PolicyArgsObject; - queries?: PolicyQueryInput[]; + query?: PolicyQueryInput; } const resourceRepository = S3ResourceRepository.fromEnvironment(); diff --git a/services/src/tests/e2e/tests/authorization.spec.ts b/services/src/tests/e2e/tests/authorization.spec.ts index 4bbb5449..e1fb3e79 100644 --- a/services/src/tests/e2e/tests/authorization.spec.ts +++ b/services/src/tests/e2e/tests/authorization.spec.ts @@ -16,13 +16,13 @@ const registryClient = createRegistryClient(); describe('authorization', () => { // This is kind of both the "before" section and a test, but it was weird putting a test in an actual before section it('creates the policy and schema resources', async () => { - const policy1Response = await registryClient.request(createPolicyMutation, {policy: onlyAdminPolicy()}); + const policy1Response: any = await registryClient.request(createPolicyMutation, {policy: onlyAdminPolicy()}); expect(policy1Response.updatePolicies.success).toBe(true); - const policy2Response = await registryClient.request(createPolicyMutation, {policy: jwtNamePolicy()}); + const policy2Response: any = await registryClient.request(createPolicyMutation, {policy: jwtNamePolicy()}); expect(policy2Response.updatePolicies.success).toBe(true); - const schemaResponse = await registryClient.request(createSchemaMutation, {schema: getSchema()}); + const schemaResponse: any = await registryClient.request(createSchemaMutation, {schema: getSchema()}); expect(schemaResponse.updateSchemas.success).toBe(true); // Wait for gateway to update before next tests @@ -30,7 +30,7 @@ describe('authorization', () => { }); it('allows access to a field based on an argument using param injection from source', async () => { - const response = await gatewayClient.request(getUserQuery('userAdmin')); + const response: any = await gatewayClient.request(getUserQuery('userAdmin')); expect(response.userAdmin).toEqual({firstName: 'John', lastName: 'Smith', role: 'admin'}); }); @@ -79,7 +79,7 @@ describe('authorization', () => { it('allows access to a field based on JWT info', async () => { const gatewayClientJwt = createGatewayClient(allowedJwtOptions()); - const response = await gatewayClientJwt.request(getArbitraryDataQuery()); + const response: any = await gatewayClientJwt.request(getArbitraryDataQuery()); expect(response.arbitraryData).toEqual({arbitraryField: 'arbitraryValue'}); }); }); diff --git a/services/src/tests/integration/registry/create-resources.spec.ts b/services/src/tests/integration/registry/create-resources.spec.ts index a92bd13f..6ae27efa 100644 --- a/services/src/tests/integration/registry/create-resources.spec.ts +++ b/services/src/tests/integration/registry/create-resources.spec.ts @@ -9,7 +9,7 @@ import {beforeEachDispose} from '../beforeEachDispose'; import {app} from '../../../registry'; import {mockResourceBucket} from '../resourceBucket'; import {ResourceGroup} from '../../../modules/resource-repository'; -import {PolicyType, PolicyQueryType} from '../../../modules/resource-repository/types'; +import {PolicyType, Policy} from '../../../modules/resource-repository/types'; import {tmpPoliciesDir} from '../../../modules/config'; import mockFsForOpa from '../../helpers/mockFsForOpa'; @@ -43,7 +43,7 @@ const upstreamClientCredentials = { }, }; -const policy = { +const policy: Policy = { metadata: {namespace: 'namespace', name: 'name'}, type: PolicyType.opa, code: `real rego code @@ -53,18 +53,12 @@ const policy = { an: 'arg', another: 'one!', }, - queries: [ - { - type: PolicyQueryType.graphql, - name: 'someGraphqlQuery', - graphql: {query: 'actual gql'}, + query: { + source: 'some gql', + variables: { + a: 'b', }, - { - type: PolicyQueryType.policy, - name: 'somePolicyQuery', - policy: {policyName: 'someOtherPolicy', args: {some: 'arg for the other policy'}}, - }, - ], + }, }; const baseResourceGroup = { diff --git a/services/src/tests/integration/registry/update-resources.spec.ts b/services/src/tests/integration/registry/update-resources.spec.ts index ce6007fb..27fc8f2d 100644 --- a/services/src/tests/integration/registry/update-resources.spec.ts +++ b/services/src/tests/integration/registry/update-resources.spec.ts @@ -9,7 +9,7 @@ import {beforeEachDispose} from '../beforeEachDispose'; import {app, AuthType} from '../../../registry'; import {mockResourceBucket} from '../resourceBucket'; import {ResourceGroup} from '../../../modules/resource-repository'; -import {PolicyType, PolicyQueryType} from '../../../modules/resource-repository/types'; +import {PolicyType, Policy} from '../../../modules/resource-repository/types'; import {tmpPoliciesDir} from '../../../modules/config'; import mockFsForOpa from '../../helpers/mockFsForOpa'; @@ -46,7 +46,7 @@ const upstreamClientCredentials = { }; const upstreamClientCredentialsActiveDirectoryUpdate = {clientSecret: 'myOtherClientSecret'}; -const policy = { +const policy: Policy = { metadata: {namespace: 'namespace', name: 'name'}, type: PolicyType.opa, code: `real rego code @@ -56,14 +56,12 @@ const policy = { an: 'arg', another: 'one!', }, - queries: [ - {type: PolicyQueryType.graphql, name: 'someGraphqlQuery', graphql: {query: 'actual gql'}}, - { - type: PolicyQueryType.policy, - name: 'somePolicyQuery', - policy: {policyName: 'someOtherPolicy', args: {some: 'arg for the other policy'}}, + query: { + source: 'some another gql', + variables: { + c: 'd', }, - ], + }, }; const policyUpdate = {code: 'changed code', args: {just: 'one arg'}}; From cdecf152bf05de6efcff2434bfa7eb7b73e6a36e Mon Sep 17 00:00:00 2001 From: AleF83 Date: Thu, 25 Jun 2020 14:30:44 +0300 Subject: [PATCH 3/8] First working version --- services/src/modules/directives/index.ts | 4 +- .../directives/policy/policy-executor.ts | 23 +- .../modules/directives/policy/policy-query.ts | 35 +++ .../src/modules/directives/policy/types.ts | 4 + services/src/modules/graphqlService.ts | 30 ++- services/src/modules/paramInjection.ts | 4 +- .../src/modules/resource-repository/types.ts | 2 +- .../authorization_with_queries.spec.ts.snap | 83 +++++++ .../tests/authorization_with_queries.spec.ts | 211 ++++++++++++++++++ .../src/tests/e2e/tests/hello-world.spec.ts | 6 +- 10 files changed, 388 insertions(+), 14 deletions(-) create mode 100644 services/src/modules/directives/policy/policy-query.ts create mode 100644 services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap create mode 100644 services/src/tests/e2e/tests/authorization_with_queries.spec.ts diff --git a/services/src/modules/directives/index.ts b/services/src/modules/directives/index.ts index 52b6668a..e0250b86 100644 --- a/services/src/modules/directives/index.ts +++ b/services/src/modules/directives/index.ts @@ -6,6 +6,7 @@ import {sdl as gqlSdl, GqlDirective} from './gql'; import {sdl as exportSdl, ExportDirective} from './export'; import {sdl as selectSdl, SelectDirective} from './select'; import {sdl as policySdl, PolicyDirective} from './policy/policy'; +import {sdl as policyQuerySdl, PolicyQueryDirective} from './policy/policy-query'; export const directiveMap: {[visitorName: string]: typeof SchemaDirectiveVisitor} = { stub: StubDirective, @@ -14,6 +15,7 @@ export const directiveMap: {[visitorName: string]: typeof SchemaDirectiveVisitor export: ExportDirective, select: SelectDirective, policy: PolicyDirective, + policyQuery: PolicyQueryDirective, }; -export const sdl = concatAST([stubSdl, restSdl, gqlSdl, exportSdl, selectSdl, policySdl]); +export const sdl = concatAST([stubSdl, restSdl, gqlSdl, exportSdl, selectSdl, policySdl, policyQuerySdl]); diff --git a/services/src/modules/directives/policy/policy-executor.ts b/services/src/modules/directives/policy/policy-executor.ts index 2da7ec33..d84165a7 100644 --- a/services/src/modules/directives/policy/policy-executor.ts +++ b/services/src/modules/directives/policy/policy-executor.ts @@ -1,7 +1,7 @@ -import {GraphQLResolveInfo} from 'graphql'; +import {GraphQLResolveInfo, graphql} from 'graphql'; import {RequestContext} from '../../context'; import {Policy, GraphQLArguments} from './types'; -import {Policy as PolicyDefinition, PolicyArgsObject, PolicyAttachments} from '../../resource-repository'; +import {Policy as PolicyDefinition, PolicyArgsObject, PolicyAttachments, PolicyQuery} from '../../resource-repository'; import {evaluate as evaluateOpa} from './opa'; import {injectParameters} from '../../paramInjection'; @@ -32,7 +32,8 @@ 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)); const evaluate = typeEvaluators[policyDefinition.type]; if (!evaluate) throw new Error(`Unsupported policy type ${policyDefinition.type}`); @@ -73,4 +74,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 { + let variableValues = + query.variables && + Object.entries(query.variables).reduce<{[key: string]: any}>((policyArgs, [varName, varValue]) => { + if (typeof varValue === 'string') { + varValue = injectParameters(varValue, this.parent, args, this.context, this.info).value; + } + policyArgs[varName] = varValue; + return policyArgs; + }, {}); + + // TODO: Run with admin permissions + const gqlResult = await graphql(this.info.schema, query.source, undefined, this.context, variableValues); + return gqlResult.data; + } } diff --git a/services/src/modules/directives/policy/policy-query.ts b/services/src/modules/directives/policy/policy-query.ts new file mode 100644 index 00000000..9610d95d --- /dev/null +++ b/services/src/modules/directives/policy/policy-query.ts @@ -0,0 +1,35 @@ +import {SchemaDirectiveVisitor} from 'graphql-tools'; +import {GraphQLField, GraphQLResolveInfo} from 'graphql'; +import {RequestContext} from '../../context'; +import {gql} from 'apollo-server-core'; +import {PolicyResult, Policy} from './types'; +import {PolicyExecutor} from './policy-executor'; + +export class PolicyQueryDirective extends SchemaDirectiveVisitor { + visitFieldDefinition(field: GraphQLField) { + field.resolve = async ( + parent: any, + args: any, + context: RequestContext, + info: GraphQLResolveInfo + ): Promise => { + const policy: Policy = { + namespace: this.args.namespace, + name: this.args.name, + args: args, + }; + + const executor = new PolicyExecutor([policy], parent, args, context, info); + try { + await executor.validatePolicies(); + return {allow: true}; + } catch (error) { + return {allow: false}; + } + }; + } +} + +export const sdl = gql` + directive @policyQuery(namespace: String!, name: String!) on FIELD_DEFINITION +`; diff --git a/services/src/modules/directives/policy/types.ts b/services/src/modules/directives/policy/types.ts index 98501d1c..8839b1c1 100644 --- a/services/src/modules/directives/policy/types.ts +++ b/services/src/modules/directives/policy/types.ts @@ -28,6 +28,10 @@ export type PolicyExecutionResult = { }; }; +export interface PolicyResult { + allow: boolean; +} + export type GraphQLArguments = { [name: string]: any; }; diff --git a/services/src/modules/graphqlService.ts b/services/src/modules/graphqlService.ts index 713c3a67..6d4db9cd 100644 --- a/services/src/modules/graphqlService.ts +++ b/services/src/modules/graphqlService.ts @@ -1,5 +1,5 @@ -import {Unsubscriber, GraphQLServiceConfig, SchemaChangeCallback} from 'apollo-server-core'; -import {parse, execute} from 'graphql'; +import {Unsubscriber, GraphQLServiceConfig, SchemaChangeCallback, gql} from 'apollo-server-core'; +import {parse, execute, DocumentNode} from 'graphql'; import {Observable, Subscription} from 'rxjs'; import {shareReplay, map, take, tap, catchError, skip} from 'rxjs/operators'; @@ -55,6 +55,23 @@ export function createGraphQLService(config: {resourceGroups: Observable `${argName}: ${argType}`) + .join(',')})` + : ''; + + return gql` + type PolicyResult { + allow: Boolean! + } + + type Query { + policy___${policy.metadata.namespace}___${policy.metadata.name}${argStr}: PolicyResult! @policyQuery(namespace: "${policy.metadata.namespace}", name: "${policy.metadata.name}") + }`; +} + export function createSchemaConfig(rg: ResourceGroup): GraphQLServiceConfig { const activeDirectoryAuth = new ActiveDirectoryAuth(); const upstreamsByHost = new Map(rg.upstreams.map(u => [u.host, u])); @@ -62,6 +79,7 @@ export function createSchemaConfig(rg: ResourceGroup): GraphQLServiceConfig { rg.upstreamClientCredentials.map(u => [u.activeDirectory.authority, u]) ); const schemas = rg.schemas.length === 0 ? [defaultSchema] : rg.schemas; + const policies = rg.policies ?? []; const authenticationConfig: AuthenticationConfig = { getUpstreamByHost(host: string) { @@ -73,15 +91,19 @@ export function createSchemaConfig(rg: ResourceGroup): GraphQLServiceConfig { activeDirectoryAuth, }; + const schemaTypeDefs = schemas.map(s => [`${s.metadata.namespace}/${s.metadata.name}`, parse(s.schema)]); + const policyQueryTypeDefs = policies.map(p => [ + `policy___${p.metadata.namespace}___${p.metadata.name}`, + buildPolicyGqlQuery(p), + ]); const schema = buildSchemaFromFederatedTypeDefs({ - typeDefs: Object.fromEntries(schemas.map(s => [`${s.metadata.namespace}/${s.metadata.name}`, parse(s.schema)])), + typeDefs: Object.fromEntries([...schemaTypeDefs, ...policyQueryTypeDefs]), baseTypeDefs: baseSchema.baseTypeDef, directiveTypeDefs: directivesSdl, resolvers: baseSchema.resolvers, schemaDirectives: directiveMap, schemaDirectivesContext: {authenticationConfig}, }); - return { schema, executor(requestContext) { diff --git a/services/src/modules/paramInjection.ts b/services/src/modules/paramInjection.ts index 0b16fcab..46b98d3d 100644 --- a/services/src/modules/paramInjection.ts +++ b/services/src/modules/paramInjection.ts @@ -10,7 +10,7 @@ type jwtData = { [name: string]: any; }; -const paramRegex = /{(source|args|exports)\.(\w+(\.\w+)*)}/g; +const paramRegex = /{(source|args|exports)\.(\w+(\.\w+)*)}/; const authzHeaderPrefix = 'Bearer '; function resolveTemplate( @@ -46,7 +46,7 @@ export function injectParameters( let didFindValues = false; let didFindTemplates = false; let value: any = template; - const match = paramRegex.exec(template); + const match = template.match(paramRegex); if (match) { value = resolveTemplate(match[1], match[2], parent, args, context, info); didFindTemplates = true; diff --git a/services/src/modules/resource-repository/types.ts b/services/src/modules/resource-repository/types.ts index 83355569..cd2118aa 100644 --- a/services/src/modules/resource-repository/types.ts +++ b/services/src/modules/resource-repository/types.ts @@ -47,7 +47,7 @@ export interface Policy extends Resource { query?: PolicyQuery; } -interface PolicyQuery { +export interface PolicyQuery { source: string; variables?: PolicyQueryGraphqlVariables; } diff --git a/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap b/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap new file mode 100644 index 00000000..34754ec7 --- /dev/null +++ b/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap @@ -0,0 +1,83 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Authorization with queries Query denied employee 1 1`] = ` +Object { + "data": Object { + "deniedEmployee1": Object { + "address": null, + "id": "2", + "name": "Mark Zuckerberg", + }, + }, + "errors": Array [ + Object { + "extensions": Object { + "code": "INTERNAL_SERVER_ERROR", + "exception": Object { + "stacktrace": Array [ + "Error: Unauthorized by policy notClassified in namespace namespace", + " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:33:19)", + " at async Promise.all (index 0)", + " at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)", + " at async field.resolve (/service/dist/modules/directives/policy/policy.js:13:13)", + ], + }, + }, + "locations": Array [ + Object { + "column": 7, + "line": 6, + }, + ], + "message": "Unauthorized by policy notClassified in namespace namespace", + "path": Array [ + "deniedEmployee1", + "address", + ], + }, + ], + "extensions": Any, + "status": 200, +} +`; + +exports[`Authorization with queries Query denied employee 2 1`] = ` +Object { + "data": Object { + "deniedEmployee2": Object { + "address": null, + "id": "2", + "name": "Tom Baker", + }, + }, + "errors": Array [ + Object { + "extensions": Object { + "code": "INTERNAL_SERVER_ERROR", + "exception": Object { + "stacktrace": Array [ + "Error: Unauthorized by policy notClassified in namespace namespace", + " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:33:19)", + " at async Promise.all (index 0)", + " at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)", + " at async field.resolve (/service/dist/modules/directives/policy/policy.js:13:13)", + ], + }, + }, + "locations": Array [ + Object { + "column": 7, + "line": 6, + }, + ], + "message": "Unauthorized by policy notClassified in namespace namespace", + "path": Array [ + "deniedEmployee2", + "address", + ], + }, + ], + "extensions": Any, + "status": 200, +} +`; diff --git a/services/src/tests/e2e/tests/authorization_with_queries.spec.ts b/services/src/tests/e2e/tests/authorization_with_queries.spec.ts new file mode 100644 index 00000000..91d536f9 --- /dev/null +++ b/services/src/tests/e2e/tests/authorization_with_queries.spec.ts @@ -0,0 +1,211 @@ +import {GraphQLClient} from 'graphql-request'; +import {createSchemaMutation} from '../../helpers/authzSchema'; +import {Policy, PolicyType, Schema} from '../../../modules/resource-repository/types'; +import {sleep} from '../../helpers/utility'; + +const policies: Policy[] = [ + { + metadata: { + name: 'isSenior', + namespace: 'namespace', + }, + type: PolicyType.opa, + code: ` + default allow = false + allow { + input.args.hireDate < 2015 + } + `, + args: { + hireDate: 'Int', + }, + }, + { + metadata: { + name: 'notClassified', + namespace: 'namespace', + }, + type: PolicyType.opa, + code: ` + default allow = false + allow { + input.query.classifiedDepartments[_].id != input.args.departmentId; + input.query.policy___namespace___isSenior.allow + } + `, + args: { + departmentId: 'String', + hireDate: 'Int', + }, + query: { + source: ` + query($hireDate: Int!) { + classifiedDepartments { + id + }, + policy___namespace___isSenior(hireDate: $hireDate) { + allow + } + } + `, + variables: { + hireDate: '{args.hireDate}', + }, + }, + }, +]; + +const schema: Schema = { + metadata: { + name: 'EmployeeSchema', + namespace: 'namespace', + }, + schema: ` + type Department { + id: String + name: String + } + type Employee { + id: String + name: String + hireDate: Int + department: Department! + address: String @policy(policies: [{ + namespace: "namespace", + name: "notClassified", + args: { + departmentId: "{source.department.id}", + hireDate: "{source.hireDate}" + } + }]) + } + + type Query { + allowedEmployee: Employee! @stub(value: { + id: "1" + name: "John Smith" + address: "Tel Aviv" + hireDate: 2010 + department: { + id: "D1" + name: "Sales" + } + }) + deniedEmployee1: Employee! @stub(value: { + id: "2" + name: "Mark Zuckerberg", + department: { + id: "D1000" + name: "VIP" + } + hireDate: 2010 + address: "Facebook HQ" + }) + deniedEmployee2: Employee! @stub(value: { + id: "2" + name: "Tom Baker", + department: { + id: "D2" + name: "VIP" + } + hireDate: 2019 + address: "Atlanta" + }) + classifiedDepartments: [Department!]! @stub(value: [{ + id: "D1000" + name: "VIP" + }]) + } + `, +}; + +const createPolicyMutation = ` + mutation CreatePolicy($policies: [PolicyInput!]!) { + updatePolicies(input: $policies) { + success + } + }`; + +const employeeQuery = (query: string) => ` + query { + ${query} { + id + name + address + } + } +`; + +interface CreatePolicyMutationResponse { + updatePolicies: { + success: boolean; + }; +} + +interface UpdateSchemasMutationResponse { + updateSchemas: { + success: boolean; + }; +} + +interface AllowedEmployeeQueryResponse { + allowedEmployee: { + id: string; + name: string; + address: string; + }; +} + +describe('Authorization with queries', () => { + let gatewayClient: GraphQLClient; + let registryClient: GraphQLClient; + + beforeAll(() => { + gatewayClient = new GraphQLClient('http://localhost:8080/graphql'); + registryClient = new GraphQLClient('http://localhost:8090/graphql'); + }); + + test('Setup policies', async () => { + const policyResponse: CreatePolicyMutationResponse = await registryClient.request(createPolicyMutation, { + policies, + }); + expect(policyResponse.updatePolicies.success).toBeTruthy(); + + const schemaResponse: UpdateSchemasMutationResponse = await registryClient.request(createSchemaMutation, { + schema, + }); + expect(schemaResponse.updateSchemas.success).toBeTruthy(); + + // Wait for gateway to update before next tests + await sleep(500); + }); + + test('Query allowed employee', async () => { + const response: AllowedEmployeeQueryResponse = await gatewayClient.request(employeeQuery('allowedEmployee')); + expect(response.allowedEmployee.address).toBeDefined(); + }); + + test('Query denied employee 1', async () => { + try { + await gatewayClient.request(employeeQuery('deniedEmployee1')); + expect(true).toBeFalsy(); + } catch (e) { + const response = e.response; + expect(response).toMatchSnapshot({ + extensions: expect.any(Object), + }); + } + }); + + test('Query denied employee 2', async () => { + try { + await gatewayClient.request(employeeQuery('deniedEmployee2')); + expect(true).toBeFalsy(); + } catch (e) { + const response = e.response; + expect(response).toMatchSnapshot({ + extensions: expect.any(Object), + }); + } + }); +}); diff --git a/services/src/tests/e2e/tests/hello-world.spec.ts b/services/src/tests/e2e/tests/hello-world.spec.ts index 9d7bffa4..6cbcd1f4 100644 --- a/services/src/tests/e2e/tests/hello-world.spec.ts +++ b/services/src/tests/e2e/tests/hello-world.spec.ts @@ -18,19 +18,19 @@ mutation CreateSchema($schema: SchemaInput!) { describe('Basic flow', () => { test('Default schema works', async () => { - const response = await gatewayClient.request(`query {default}`); + const response: any = await gatewayClient.request(`query {default}`); expect(response.default).toBe('default'); }); test('Gateway updates when updating schema in registry', async () => { - const response1 = await registryClient.request(createSchemaMutation, {schema: nonDefaultSchema}); + const response1: any = await registryClient.request(createSchemaMutation, {schema: nonDefaultSchema}); expect(response1.updateSchemas.success).toBe(true); // Wait for gateway to update await sleep(500); - const response2 = await gatewayClient.request(`query {default}`); + const response2: any = await gatewayClient.request(`query {default}`); expect(response2.default).toBe('NOPE'); }); }); From 3aad83b75014ed7c0318e5a95d16bfd0f320fa6f Mon Sep 17 00:00:00 2001 From: AleF83 Date: Sun, 28 Jun 2020 16:10:41 +0300 Subject: [PATCH 4/8] Second iteration --- services/src/modules/baseSchema.ts | 11 +++- services/src/modules/directives/policy/opa.ts | 8 +-- .../directives/policy/policy-executor.ts | 27 ++++++++-- .../modules/directives/policy/policy-query.ts | 10 ++-- .../src/modules/directives/policy/policy.ts | 6 ++- services/src/modules/graphqlService.ts | 32 ++++++++---- services/src/modules/paramInjection.ts | 8 +-- .../src/modules/resource-repository/types.ts | 6 +-- services/src/registry.ts | 8 +-- .../authorization_with_queries.spec.ts.snap | 50 +++++++++++++++++-- .../tests/authorization_with_queries.spec.ts | 40 +++++++++++++-- .../registry/create-resources.spec.ts | 2 +- .../registry/update-resources.spec.ts | 2 +- 13 files changed, 159 insertions(+), 51 deletions(-) diff --git a/services/src/modules/baseSchema.ts b/services/src/modules/baseSchema.ts index a3b84a30..bd53f677 100644 --- a/services/src/modules/baseSchema.ts +++ b/services/src/modules/baseSchema.ts @@ -3,6 +3,7 @@ import GraphQLJSON, {GraphQLJSONObject} from 'graphql-type-json'; import {GraphQLDate, GraphQLDateTime, GraphQLTime} from 'graphql-iso-date'; import {concatAST} from 'graphql'; import {sdl as directivesSdl} from './directives'; +import {GraphQLResolverMap} from 'apollo-graphql'; export const baseTypeDef = gql` scalar JSON @@ -11,11 +12,19 @@ export const baseTypeDef = gql` scalar Date scalar Time scalar DateTime + + type PolicyResult { + allow: Boolean! + } + + type Policy { + default: PolicyResult! + } `; export const typeDef = concatAST([baseTypeDef, directivesSdl]); -export const resolvers = { +export const resolvers: GraphQLResolverMap<{}> = { JSON: GraphQLJSON, JSONObject: GraphQLJSONObject, Date: GraphQLDate, diff --git a/services/src/modules/directives/policy/opa.ts b/services/src/modules/directives/policy/opa.ts index fad61886..a8174aa1 100644 --- a/services/src/modules/directives/policy/opa.ts +++ b/services/src/modules/directives/policy/opa.ts @@ -1,7 +1,7 @@ // @ts-ignore opa-wasm already has TS typings merged, but not yet published on npm import * as Rego from '@open-policy-agent/opa-wasm'; import {getCompiledFilename} from '../../opaHelper'; -import {PolicyExecutionContext, PolicyExecutionResult, QueriesResults} from './types'; +import {PolicyExecutionContext, PolicyExecutionResult, QueryResults} from './types'; import {PolicyArgsObject} from '../../resource-repository'; export async function evaluate(ctx: PolicyExecutionContext): Promise { @@ -21,8 +21,8 @@ async function getWasmPolicy(ctx: PolicyExecutionContext): Promise { return rego.load_policy(wasm); } -function getInput(ctx: PolicyExecutionContext): PolicyInput { - const input: PolicyInput = {}; +function getInput(ctx: PolicyExecutionContext): PolicyOpaInput { + const input: PolicyOpaInput = {}; if (ctx.args) input.args = ctx.args; if (ctx.query) input.query = ctx.query; @@ -30,7 +30,7 @@ function getInput(ctx: PolicyExecutionContext): PolicyInput { return input; } -type PolicyInput = { +type PolicyOpaInput = { args?: PolicyArgsObject; query?: QueryResults; }; diff --git a/services/src/modules/directives/policy/policy-executor.ts b/services/src/modules/directives/policy/policy-executor.ts index d84165a7..b15cd01c 100644 --- a/services/src/modules/directives/policy/policy-executor.ts +++ b/services/src/modules/directives/policy/policy-executor.ts @@ -1,6 +1,6 @@ import {GraphQLResolveInfo, graphql} from 'graphql'; import {RequestContext} from '../../context'; -import {Policy, GraphQLArguments} from './types'; +import {Policy, GraphQLArguments, QueryResults} from './types'; import {Policy as PolicyDefinition, PolicyArgsObject, PolicyAttachments, PolicyQuery} from '../../resource-repository'; import {evaluate as evaluateOpa} from './opa'; import {injectParameters} from '../../paramInjection'; @@ -28,7 +28,7 @@ export class PolicyExecutor { await Promise.all(this.policies.map(r => this.validatePolicy(r))); } - async validatePolicy(policy: Policy) { + async evaluatePolicy(policy: Policy): Promise { const policyDefinition = this.getPolicyDefinition(policy.namespace, policy.name); const args = policyDefinition.args && this.preparePolicyArgs(policyDefinition.args, policy); @@ -44,7 +44,11 @@ export class PolicyExecutor { policyAttachments: this.policyAttachments, }); if (!done) throw new Error('in-line query evaluation not yet supported'); + return allow || false; + } + async validatePolicy(policy: Policy): Promise { + const allow = await this.evaluatePolicy(policy); if (!allow) throw new Error(`Unauthorized by policy ${policy.name} in namespace ${policy.namespace}`); } @@ -75,7 +79,10 @@ export class PolicyExecutor { return policyDefinition; } - private async evaluatePolicyQuery(query: PolicyQuery, args: PolicyArgsObject = {}): Promise { + private async evaluatePolicyQuery( + query: PolicyQuery, + args: PolicyArgsObject = {} + ): Promise { let variableValues = query.variables && Object.entries(query.variables).reduce<{[key: string]: any}>((policyArgs, [varName, varValue]) => { @@ -87,7 +94,17 @@ export class PolicyExecutor { }, {}); // TODO: Run with admin permissions - const gqlResult = await graphql(this.info.schema, query.source, undefined, this.context, variableValues); - return gqlResult.data; + const context: RequestContext = {...this.context, ignorePolicies: true}; + const gqlResult = await graphql(this.info.schema, query.gql, undefined, context, variableValues); + return gqlResult.data || undefined; + } +} + +declare module '../../context' { + interface RequestContext { + /** + * This flag indicates that request should be resolved without invoking authorization policies evaluation + */ + ignorePolicies: boolean; } } diff --git a/services/src/modules/directives/policy/policy-query.ts b/services/src/modules/directives/policy/policy-query.ts index 9610d95d..da1cca72 100644 --- a/services/src/modules/directives/policy/policy-query.ts +++ b/services/src/modules/directives/policy/policy-query.ts @@ -19,13 +19,9 @@ export class PolicyQueryDirective extends SchemaDirectiveVisitor { args: args, }; - const executor = new PolicyExecutor([policy], parent, args, context, info); - try { - await executor.validatePolicies(); - return {allow: true}; - } catch (error) { - return {allow: false}; - } + const executor = new PolicyExecutor([], parent, args, context, info); + const allow = await executor.evaluatePolicy(policy); + return {allow}; }; } } diff --git a/services/src/modules/directives/policy/policy.ts b/services/src/modules/directives/policy/policy.ts index 02cd3eb3..ea780268 100644 --- a/services/src/modules/directives/policy/policy.ts +++ b/services/src/modules/directives/policy/policy.ts @@ -11,8 +11,10 @@ export class PolicyDirective extends SchemaDirectiveVisitor { const policies = this.args.policies; field.resolve = async (parent: any, args: any, context: RequestContext, info: GraphQLResolveInfo) => { - const executor = new PolicyExecutor(policies, parent, args, context, info); - await executor.validatePolicies(); + if (!context.ignorePolicies) { + const executor = new PolicyExecutor(policies, parent, args, context, info); + await executor.validatePolicies(); + } return originalResolve.call(field, parent, args, context, info); }; diff --git a/services/src/modules/graphqlService.ts b/services/src/modules/graphqlService.ts index 6d4db9cd..b3936418 100644 --- a/services/src/modules/graphqlService.ts +++ b/services/src/modules/graphqlService.ts @@ -4,7 +4,7 @@ import {Observable, Subscription} from 'rxjs'; import {shareReplay, map, take, tap, catchError, skip} from 'rxjs/operators'; import {directiveMap} from './directives'; -import {ResourceGroup, Policy, PolicyAttachments} from './resource-repository'; +import {ResourceGroup, Policy, PolicyAttachments, Schema} from './resource-repository'; import {buildSchemaFromFederatedTypeDefs} from './buildFederatedSchema'; import * as baseSchema from './baseSchema'; import {ActiveDirectoryAuth} from './auth/activeDirectoryAuth'; @@ -63,13 +63,10 @@ function buildPolicyGqlQuery(policy: Policy): DocumentNode { : ''; return gql` - type PolicyResult { - allow: Boolean! + extend type Policy { + ${policy.metadata.namespace}___${policy.metadata.name}${argStr}: PolicyResult! @policyQuery(namespace: "${policy.metadata.namespace}", name: "${policy.metadata.name}") } - - type Query { - policy___${policy.metadata.namespace}___${policy.metadata.name}${argStr}: PolicyResult! @policyQuery(namespace: "${policy.metadata.namespace}", name: "${policy.metadata.name}") - }`; + `; } export function createSchemaConfig(rg: ResourceGroup): GraphQLServiceConfig { @@ -78,7 +75,7 @@ export function createSchemaConfig(rg: ResourceGroup): GraphQLServiceConfig { const upstreamClientCredentialsByAuthority = new Map( rg.upstreamClientCredentials.map(u => [u.activeDirectory.authority, u]) ); - const schemas = rg.schemas.length === 0 ? [defaultSchema] : rg.schemas; + const schemas = rg.schemas.length === 0 ? [defaultSchema] : [initialSchema, ...rg.schemas]; const policies = rg.policies ?? []; const authenticationConfig: AuthenticationConfig = { @@ -93,7 +90,7 @@ export function createSchemaConfig(rg: ResourceGroup): GraphQLServiceConfig { const schemaTypeDefs = schemas.map(s => [`${s.metadata.namespace}/${s.metadata.name}`, parse(s.schema)]); const policyQueryTypeDefs = policies.map(p => [ - `policy___${p.metadata.namespace}___${p.metadata.name}`, + `${p.metadata.namespace}___${p.metadata.name}`, buildPolicyGqlQuery(p), ]); const schema = buildSchemaFromFederatedTypeDefs({ @@ -122,11 +119,26 @@ export function createSchemaConfig(rg: ResourceGroup): GraphQLServiceConfig { }; } -const defaultSchema = { +const defaultSchema: Schema = { metadata: {namespace: '__internal__', name: 'default'}, schema: 'type Query { default: String! @stub(value: "default") }', }; +const initialSchema: Schema = { + metadata: { + namespace: '__internal__', + name: '__initial__', + }, + schema: ` + type Query { + policy: Policy! @stub(value: { + default: { + allow: true + } + }) + }`, +}; + declare module './context' { interface RequestContext { authenticationConfig: AuthenticationConfig; diff --git a/services/src/modules/paramInjection.ts b/services/src/modules/paramInjection.ts index 46b98d3d..95d8244c 100644 --- a/services/src/modules/paramInjection.ts +++ b/services/src/modules/paramInjection.ts @@ -15,22 +15,22 @@ const authzHeaderPrefix = 'Bearer '; function resolveTemplate( source: string, - template: string, + key: string, parent: any, args: GraphQLArguments, context: RequestContext, info: GraphQLResolveInfo ) { - const propPath = template.split('.'); + const propPath = key.split('.'); switch (source) { case 'source': return parent && R.path(propPath, parent); case 'args': return args && R.path(propPath, args); case 'exports': - return context.exports.resolve(info.parentType, parent, template); + return context.exports.resolve(info.parentType, parent, key); case 'jwt': - return getJwt(context)[template]; + return getJwt(context)[key]; default: return null; } diff --git a/services/src/modules/resource-repository/types.ts b/services/src/modules/resource-repository/types.ts index cd2118aa..3a85afdb 100644 --- a/services/src/modules/resource-repository/types.ts +++ b/services/src/modules/resource-repository/types.ts @@ -48,11 +48,11 @@ export interface Policy extends Resource { } export interface PolicyQuery { - source: string; - variables?: PolicyQueryGraphqlVariables; + gql: string; + variables?: PolicyQueryVariables; } -export interface PolicyQueryGraphqlVariables { +export interface PolicyQueryVariables { [key: string]: any; } diff --git a/services/src/registry.ts b/services/src/registry.ts index 34316a5d..5d77445a 100644 --- a/services/src/registry.ts +++ b/services/src/registry.ts @@ -11,7 +11,7 @@ import {createSchemaConfig} from './modules/graphqlService'; import * as opaHelper from './modules/opaHelper'; // Importing directly from types because of a typescript or ts-jest bug that re-exported enums cause a runtime error for being undefined // https://github.com/kulshekhar/ts-jest/issues/281 -import {PolicyArgsObject, PolicyType} from './modules/resource-repository/types'; +import {PolicyArgsObject, PolicyType, PolicyQueryVariables} from './modules/resource-repository/types'; const typeDefs = gql` scalar JSON @@ -109,7 +109,7 @@ const typeDefs = gql` } input PolicyQueryInput { - source: String! + gql: String! variables: JSONObject } @@ -168,8 +168,8 @@ interface UpstreamClientCredentialsInput { } interface PolicyQueryInput { - source: string; - variables?: {[name: string]: any}; + gql: string; + variables?: PolicyQueryVariables; } interface PolicyInput { diff --git a/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap b/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap index 34754ec7..db08b067 100644 --- a/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap +++ b/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap @@ -1,5 +1,47 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Authorization with queries Query allowed employee 1`] = ` +Object { + "data": null, + "errors": Array [ + Object { + "extensions": Object { + "code": "INTERNAL_SERVER_ERROR", + "exception": Object { + "stacktrace": Array [ + "Error: Unauthorized by policy alwaysDenied in namespace namespace", + " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:37:19)", + " at async Promise.all (index 0)", + " at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)", + " at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)", + ], + }, + }, + "locations": Array [ + Object { + "column": 15, + "line": 2, + }, + ], + "message": "Unauthorized by policy alwaysDenied in namespace namespace", + "path": Array [ + "classifiedDepartments", + ], + }, + ], + "extensions": Any, + "status": 200, +} +`; + +exports[`Authorization with queries Query allowed employee 2`] = ` +Object { + "address": "Tel Aviv", + "id": "1", + "name": "John Smith", +} +`; + exports[`Authorization with queries Query denied employee 1 1`] = ` Object { "data": Object { @@ -16,10 +58,10 @@ Object { "exception": Object { "stacktrace": Array [ "Error: Unauthorized by policy notClassified in namespace namespace", - " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:33:19)", + " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:37:19)", " at async Promise.all (index 0)", " at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)", - " at async field.resolve (/service/dist/modules/directives/policy/policy.js:13:13)", + " at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)", ], }, }, @@ -57,10 +99,10 @@ Object { "exception": Object { "stacktrace": Array [ "Error: Unauthorized by policy notClassified in namespace namespace", - " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:33:19)", + " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:37:19)", " at async Promise.all (index 0)", " at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)", - " at async field.resolve (/service/dist/modules/directives/policy/policy.js:13:13)", + " at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)", ], }, }, diff --git a/services/src/tests/e2e/tests/authorization_with_queries.spec.ts b/services/src/tests/e2e/tests/authorization_with_queries.spec.ts index 91d536f9..11404173 100644 --- a/services/src/tests/e2e/tests/authorization_with_queries.spec.ts +++ b/services/src/tests/e2e/tests/authorization_with_queries.spec.ts @@ -4,6 +4,16 @@ import {Policy, PolicyType, Schema} from '../../../modules/resource-repository/t import {sleep} from '../../helpers/utility'; const policies: Policy[] = [ + { + metadata: { + name: 'alwaysDenied', + namespace: 'namespace', + }, + type: PolicyType.opa, + code: ` + default allow = false + `, + }, { metadata: { name: 'isSenior', @@ -30,7 +40,7 @@ const policies: Policy[] = [ default allow = false allow { input.query.classifiedDepartments[_].id != input.args.departmentId; - input.query.policy___namespace___isSenior.allow + input.query.policy.namespace___isSenior.allow } `, args: { @@ -38,13 +48,15 @@ const policies: Policy[] = [ hireDate: 'Int', }, query: { - source: ` + gql: ` query($hireDate: Int!) { classifiedDepartments { id }, - policy___namespace___isSenior(hireDate: $hireDate) { - allow + policy { + namespace___isSenior(hireDate: $hireDate) { + allow + } } } `, @@ -114,6 +126,9 @@ const schema: Schema = { classifiedDepartments: [Department!]! @stub(value: [{ id: "D1000" name: "VIP" + }]) @policy(policies: [{ + namespace: "namespace", + name: "alwaysDenied" }]) } `, @@ -181,8 +196,23 @@ describe('Authorization with queries', () => { }); test('Query allowed employee', async () => { + // TODO: check classified fails + try { + await gatewayClient.request(`query { + classifiedDepartments { + id + name + } + }`); + } catch (e) { + const response = e.response; + expect(response).toMatchSnapshot({ + extensions: expect.any(Object), + }); + } + const response: AllowedEmployeeQueryResponse = await gatewayClient.request(employeeQuery('allowedEmployee')); - expect(response.allowedEmployee.address).toBeDefined(); + expect(response.allowedEmployee).toMatchSnapshot(); }); test('Query denied employee 1', async () => { diff --git a/services/src/tests/integration/registry/create-resources.spec.ts b/services/src/tests/integration/registry/create-resources.spec.ts index 6ae27efa..4e260b8f 100644 --- a/services/src/tests/integration/registry/create-resources.spec.ts +++ b/services/src/tests/integration/registry/create-resources.spec.ts @@ -54,7 +54,7 @@ const policy: Policy = { another: 'one!', }, query: { - source: 'some gql', + gql: 'some gql', variables: { a: 'b', }, diff --git a/services/src/tests/integration/registry/update-resources.spec.ts b/services/src/tests/integration/registry/update-resources.spec.ts index 27fc8f2d..0d335ce0 100644 --- a/services/src/tests/integration/registry/update-resources.spec.ts +++ b/services/src/tests/integration/registry/update-resources.spec.ts @@ -57,7 +57,7 @@ const policy: Policy = { another: 'one!', }, query: { - source: 'some another gql', + gql: 'some another gql', variables: { c: 'd', }, From 3a8678c1e6c43a221cf9882738ccc37fe59273c6 Mon Sep 17 00:00:00 2001 From: AleF83 Date: Mon, 29 Jun 2020 08:28:29 +0300 Subject: [PATCH 5/8] rebase fixed --- services/src/modules/directives/policy/policy-executor.ts | 1 + services/src/modules/paramInjection.ts | 3 +-- .../__snapshots__/authorization_with_queries.spec.ts.snap | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/services/src/modules/directives/policy/policy-executor.ts b/services/src/modules/directives/policy/policy-executor.ts index b15cd01c..436539a0 100644 --- a/services/src/modules/directives/policy/policy-executor.ts +++ b/services/src/modules/directives/policy/policy-executor.ts @@ -41,6 +41,7 @@ export class PolicyExecutor { const {done, allow} = await evaluate({ ...policy, args, + query, policyAttachments: this.policyAttachments, }); if (!done) throw new Error('in-line query evaluation not yet supported'); diff --git a/services/src/modules/paramInjection.ts b/services/src/modules/paramInjection.ts index 95d8244c..78398ffd 100644 --- a/services/src/modules/paramInjection.ts +++ b/services/src/modules/paramInjection.ts @@ -10,7 +10,7 @@ type jwtData = { [name: string]: any; }; -const paramRegex = /{(source|args|exports)\.(\w+(\.\w+)*)}/; +const paramRegex = /{(source|args|jwt|exports)\.(\w+(\.\w+)*)}/; const authzHeaderPrefix = 'Bearer '; function resolveTemplate( @@ -114,7 +114,6 @@ function getJwt(context: RequestContext): jwtData { context.jwt = isAuthzHeaderValid(authzHeader) ? (decodeJwt(authzHeader.substr(authzHeaderPrefix.length), {json: true}) as jwtData) : {}; - return context.jwt; } diff --git a/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap b/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap index db08b067..f7a47bcb 100644 --- a/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap +++ b/services/src/tests/e2e/tests/__snapshots__/authorization_with_queries.spec.ts.snap @@ -10,7 +10,7 @@ Object { "exception": Object { "stacktrace": Array [ "Error: Unauthorized by policy alwaysDenied in namespace namespace", - " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:37:19)", + " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:42:19)", " at async Promise.all (index 0)", " at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)", " at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)", @@ -58,7 +58,7 @@ Object { "exception": Object { "stacktrace": Array [ "Error: Unauthorized by policy notClassified in namespace namespace", - " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:37:19)", + " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:42:19)", " at async Promise.all (index 0)", " at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)", " at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)", @@ -99,7 +99,7 @@ Object { "exception": Object { "stacktrace": Array [ "Error: Unauthorized by policy notClassified in namespace namespace", - " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:37:19)", + " at PolicyExecutor.validatePolicy (/service/dist/modules/directives/policy/policy-executor.js:42:19)", " at async Promise.all (index 0)", " at async PolicyExecutor.validatePolicies (/service/dist/modules/directives/policy/policy-executor.js:20:9)", " at async field.resolve (/service/dist/modules/directives/policy/policy.js:14:17)", From dc9ccf500e0b94559f4330858b11245d5404385e Mon Sep 17 00:00:00 2001 From: AleF83 Date: Mon, 29 Jun 2020 11:55:26 +0300 Subject: [PATCH 6/8] fix 1 --- docs/authorization_spec.md | 66 ++++++++++--------- services/src/modules/paramInjection.ts | 19 +++--- .../gateway/export-directive.spec.ts | 6 +- .../gateway/schema-extensions.spec.ts | 10 +-- .../registry/create-resources.spec.ts | 24 ++++--- .../registry/update-resources.spec.ts | 26 +++++--- 6 files changed, 82 insertions(+), 69 deletions(-) diff --git a/docs/authorization_spec.md b/docs/authorization_spec.md index 5d752a44..cd2ec24a 100644 --- a/docs/authorization_spec.md +++ b/docs/authorization_spec.md @@ -16,7 +16,7 @@ Create a new object policy that describes an access logic that can be attached to a graphql field. Some examples: -### Policy that always allows access: +### Policy that always allows access ```yaml kind: Policy @@ -28,7 +28,7 @@ Spec: code: {'result': 'allow'} ``` -### Policy that allows access only if the issuer is "abc.com": +### Policy that allows access only if the issuer is "abc.com" ```yaml kind: Policy @@ -62,7 +62,7 @@ Spec: _Note the js-expression type is an example of a possible type and not planned to be implemented at this time._ -### Policy that uses a graphql query to fetch additional data and allows based on the results and an argument: +### Policy that uses a graphql query to fetch additional data and allows based on the results and an argument ```yaml kind: Policy @@ -74,29 +74,29 @@ Spec: code: | allow = false allow = { - input.args.userId == input.queries.familyQuery.family.members[_].id + input.args.userId == input.query.user.family.members[_].id } args: userId: ID! - queries: - - type: graphql - name: familyQuery - graphql: - query: | - { - user({jwt.sub}) { - family { - members { - id - } - } - } + sub: ID! + query: + gql: | + query ($sub: String!) { + user(sub: $sub) { + family { + members { + id } + } + } + } + variables: + sub: '{args.sub}' ``` Queries support the standard Stitch parameter injection syntax, but only support the `jwt` and `args` objects for injection -### Same as the previous policy, but evaluates the query while opa is running: +### Same as the previous policy, but evaluates the query while opa is running ```yaml kind: Policy @@ -118,7 +118,7 @@ Spec: This example will always evaluate the graphql query, but generally this approach should be used when conditional side effect evaluation is needed -### Policy that uses a policy query and allows based on the results: +### Policy that uses another policy as query and allows based on the results ```yaml kind: Policy @@ -130,24 +130,26 @@ Spec: code: | allow = false allow = { - input.queries.myUserPolicy == true + input.query.myUserPolicy == true } args: userId: ID! - queries: - - type: policy - name: myUserPolicy - policy: - policyName: my-user - args: - userId: '{args.userId}' + query: + gql: | + query(userId: ID!) { + policy.my_user(userId: $userId) { + allow + } + } + variables: + userId: '{args.userId}' ``` `args` for query policy can use parameter injection from the `jwt` and `args` objects, similarly to the graphql query ### Attach policy to fields: -``` +```gql type User { ID: ID! Picture: String @policy-some-ns-public @@ -163,7 +165,7 @@ Adding args support to policies can ease memoization and remove hidden dependenc Initially, we will support a single `@policy` directive that gets an array of policy names and their args: -``` +```gql type User { Picture: String @policy(policies: [ { namespace: "some-ns", name: “my-user”, args: { userId: “{source.UserId}” } }, @@ -179,13 +181,13 @@ Later on, for convenience and type safety, we will add an alternative syntax or 1. Client requests a graphql query that includes a field that has a `@policy` directive. 2. Server activates the policy resolver and reads the arguments with parameter injection. 3. The resolver passes the args to a policy executor. - The policy executor invokes the right binding (see policy implementation contract) and manages the queries for the policy. + The policy executor invokes the right binding (see policy implementation contract) and manages the query for the policy. 4. The policy executor should return true/false if the user has access or a string describing the failure. 5. If the user has access, return the field value. Otherwise, return an error. ## Policy implementation contract -The naive js contract can be implemented with a generator, that has a result of true/false and can yield queries. +The naive js contract can be implemented with a generator, that has a result of true/false and can yield query. All queries and policy evaluation itself are memoized for a request context for same query/policy arguments. ```typescript @@ -280,7 +282,7 @@ Spec: Usage: -``` +```gql type User { ID: ID! Picture: String @policy-some-ns-has-claims(claims:["issuer", "sub"], values: ["soluto.com", "{source.UserId}"], jwtClaims: "{jwt.claims}") diff --git a/services/src/modules/paramInjection.ts b/services/src/modules/paramInjection.ts index 78398ffd..f7970a84 100644 --- a/services/src/modules/paramInjection.ts +++ b/services/src/modules/paramInjection.ts @@ -10,7 +10,7 @@ type jwtData = { [name: string]: any; }; -const paramRegex = /{(source|args|jwt|exports)\.(\w+(\.\w+)*)}/; +const paramRegex = /{(source|args|jwt|exports)\.(\w+(\.\w+)*)}/g; const authzHeaderPrefix = 'Bearer '; function resolveTemplate( @@ -46,8 +46,8 @@ export function injectParameters( let didFindValues = false; let didFindTemplates = false; let value: any = template; - const match = template.match(paramRegex); - if (match) { + const matches = Array.from(template.matchAll(paramRegex)); + for (const match of matches) { value = resolveTemplate(match[1], match[2], parent, args, context, info); didFindTemplates = true; didFindValues = didFindValues || (value !== null && typeof value !== 'undefined'); @@ -63,20 +63,19 @@ export function resolveParameters( info: GraphQLResolveInfo ) { let foundMatches = false; - const matches = template.matchAll(paramRegex); + const matches = Array.from(template.matchAll(paramRegex)); const parameters: {[key: string]: any} = {}; for (const match of matches) { foundMatches = true; - - const group = match[1]; - if (group in parameters) { + const paramTemplate = match[0]; + const source = match[1]; + const key = match[2]; + if (paramTemplate in parameters) { continue; } - - parameters[group] = resolveTemplate(group, match[2], parent, args, context, info); + parameters[paramTemplate] = resolveTemplate(source, key, parent, args, context, info); } - return foundMatches ? parameters : null; } diff --git a/services/src/tests/integration/gateway/export-directive.spec.ts b/services/src/tests/integration/gateway/export-directive.spec.ts index 610f7ddd..5bb4acba 100644 --- a/services/src/tests/integration/gateway/export-directive.spec.ts +++ b/services/src/tests/integration/gateway/export-directive.spec.ts @@ -5,6 +5,7 @@ import {print} from 'graphql'; import * as nock from 'nock'; import {createStitchGateway} from '../../../modules/gateway'; import {beforeEachDispose} from '../beforeEachDispose'; +import {Schema, ResourceGroup} from '../../../modules/resource-repository'; const organizations = [ { @@ -37,7 +38,7 @@ const organizations = [ }, ]; -const schema = { +const schema: Schema = { metadata: { namespace: 'namespace', name: 'name', @@ -64,8 +65,7 @@ const schema = { `), }; -const resourceGroup = { - etag: 'etag', +const resourceGroup: ResourceGroup = { schemas: [schema], upstreams: [], upstreamClientCredentials: [], diff --git a/services/src/tests/integration/gateway/schema-extensions.spec.ts b/services/src/tests/integration/gateway/schema-extensions.spec.ts index 40323da2..55bb5e74 100644 --- a/services/src/tests/integration/gateway/schema-extensions.spec.ts +++ b/services/src/tests/integration/gateway/schema-extensions.spec.ts @@ -6,6 +6,7 @@ import * as nock from 'nock'; import {basename} from 'path'; import {createStitchGateway} from '../../../modules/gateway'; import {beforeEachDispose} from '../beforeEachDispose'; +import {Schema, ResourceGroup} from '../../../modules/resource-repository'; const organizations = [{name: 'EvilCorp'}, {name: 'GoodCorp'}]; @@ -21,7 +22,7 @@ const employees: {[name: string]: any} = { ReallyEvilTeam: [{name: 'Alex'}], }; -const organizationSchema = { +const organizationSchema: Schema = { metadata: { namespace: 'namespace', name: 'organization', @@ -37,7 +38,7 @@ const organizationSchema = { `), }; -const teamSchema = { +const teamSchema: Schema = { metadata: { namespace: 'namespace', name: 'team', @@ -54,7 +55,7 @@ const teamSchema = { `), }; -const employeeSchema = { +const employeeSchema: Schema = { metadata: { namespace: 'namespace', name: 'employee', @@ -71,8 +72,7 @@ const employeeSchema = { `), }; -const resourceGroup = { - etag: 'etag', +const resourceGroup: ResourceGroup = { schemas: [organizationSchema, teamSchema, employeeSchema], upstreams: [], upstreamClientCredentials: [], diff --git a/services/src/tests/integration/registry/create-resources.spec.ts b/services/src/tests/integration/registry/create-resources.spec.ts index 4e260b8f..ccc62ace 100644 --- a/services/src/tests/integration/registry/create-resources.spec.ts +++ b/services/src/tests/integration/registry/create-resources.spec.ts @@ -6,10 +6,16 @@ import {promises as fs} from 'fs'; import * as path from 'path'; import * as nock from 'nock'; import {beforeEachDispose} from '../beforeEachDispose'; -import {app} from '../../../registry'; +import {app, AuthType} from '../../../registry'; import {mockResourceBucket} from '../resourceBucket'; import {ResourceGroup} from '../../../modules/resource-repository'; -import {PolicyType, Policy} from '../../../modules/resource-repository/types'; +import { + PolicyType, + Policy, + Schema, + Upstream, + UpstreamClientCredentials, +} from '../../../modules/resource-repository/types'; import {tmpPoliciesDir} from '../../../modules/config'; import mockFsForOpa from '../../helpers/mockFsForOpa'; @@ -19,23 +25,23 @@ jest.mock('child_process', () => ({ const mockedExec = mocked(exec, true); -const schema = { +const schema: Schema = { metadata: {namespace: 'namespace', name: 'name'}, schema: 'type Query { something: String! }', }; -const upstream = { +const upstream: Upstream = { metadata: {namespace: 'namespace', name: 'name'}, host: 'test.api', auth: { - type: 'ActiveDirectory', + type: AuthType.ActiveDirectory, activeDirectory: {authority: 'https://authority', resource: 'someResource'}, }, }; -const upstreamClientCredentials = { +const upstreamClientCredentials: UpstreamClientCredentials = { metadata: {namespace: 'namespace', name: 'name'}, - authType: 'ActiveDirectory', + authType: AuthType.ActiveDirectory, activeDirectory: { authority: 'https://authority', clientId: 'myClientId', @@ -50,8 +56,8 @@ const policy: Policy = { with multiple lines`, args: { - an: 'arg', - another: 'one!', + an: 'String', + another: 'String!', }, query: { gql: 'some gql', diff --git a/services/src/tests/integration/registry/update-resources.spec.ts b/services/src/tests/integration/registry/update-resources.spec.ts index 0d335ce0..c188945c 100644 --- a/services/src/tests/integration/registry/update-resources.spec.ts +++ b/services/src/tests/integration/registry/update-resources.spec.ts @@ -9,7 +9,13 @@ import {beforeEachDispose} from '../beforeEachDispose'; import {app, AuthType} from '../../../registry'; import {mockResourceBucket} from '../resourceBucket'; import {ResourceGroup} from '../../../modules/resource-repository'; -import {PolicyType, Policy} from '../../../modules/resource-repository/types'; +import { + PolicyType, + Policy, + Upstream, + Schema, + UpstreamClientCredentials, +} from '../../../modules/resource-repository/types'; import {tmpPoliciesDir} from '../../../modules/config'; import mockFsForOpa from '../../helpers/mockFsForOpa'; @@ -19,13 +25,13 @@ jest.mock('child_process', () => ({ const mockedExec = mocked(exec, true); -const schema = { +const schema: Schema = { metadata: {namespace: 'namespace', name: 'name'}, schema: 'type Query { something: String! }', }; -const schemaUpdate = {schema: 'type Query { somethingElse: String! }'}; +const schemaUpdate: Partial = {schema: 'type Query { somethingElse: String! }'}; -const upstream = { +const upstream: Upstream = { metadata: {namespace: 'namespace', name: 'name'}, host: 'test.api', auth: { @@ -33,9 +39,9 @@ const upstream = { activeDirectory: {authority: 'https://authority', resource: 'someResource'}, }, }; -const upstreamUpdate = {host: 'test2.api'}; +const upstreamUpdate: Partial = {host: 'test2.api'}; -const upstreamClientCredentials = { +const upstreamClientCredentials: UpstreamClientCredentials = { metadata: {namespace: 'namespace', name: 'name'}, authType: AuthType.ActiveDirectory, activeDirectory: { @@ -53,8 +59,8 @@ const policy: Policy = { with multiple lines`, args: { - an: 'arg', - another: 'one!', + an: 'String', + another: 'String!', }, query: { gql: 'some another gql', @@ -63,9 +69,9 @@ const policy: Policy = { }, }, }; -const policyUpdate = {code: 'changed code', args: {just: 'one arg'}}; +const policyUpdate: Partial = {code: 'changed code', args: {just: 'Int'}}; -const baseResourceGroup = { +const baseResourceGroup: ResourceGroup = { schemas: [schema], upstreams: [upstream], upstreamClientCredentials: [upstreamClientCredentials], From 39cd24dde7e7b10db00a35efa860ea9a092037e6 Mon Sep 17 00:00:00 2001 From: AleF83 Date: Mon, 29 Jun 2020 13:58:40 +0300 Subject: [PATCH 7/8] tests fixed --- services/src/modules/paramInjection.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/services/src/modules/paramInjection.ts b/services/src/modules/paramInjection.ts index f7970a84..575b9283 100644 --- a/services/src/modules/paramInjection.ts +++ b/services/src/modules/paramInjection.ts @@ -45,13 +45,12 @@ export function injectParameters( ) { let didFindValues = false; let didFindTemplates = false; - let value: any = template; - const matches = Array.from(template.matchAll(paramRegex)); - for (const match of matches) { - value = resolveTemplate(match[1], match[2], parent, args, context, info); + const value = template.replace(paramRegex, (_, source, key) => { + const resolved = resolveTemplate(source, key, parent, args, context, info); didFindTemplates = true; - didFindValues = didFindValues || (value !== null && typeof value !== 'undefined'); - } + didFindValues = didFindValues || (resolved !== null && typeof resolved !== 'undefined'); + return resolved; + }); return {value, didFindValues, didFindTemplates}; } From aeb25db96255e525ecfd5a09b9c2065f8b6b2fad Mon Sep 17 00:00:00 2001 From: AleF83 Date: Mon, 29 Jun 2020 19:52:39 +0300 Subject: [PATCH 8/8] tests fixed --- .../directives/policy/policy-executor.ts | 59 +++++++++++++------ 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/services/src/modules/directives/policy/policy-executor.ts b/services/src/modules/directives/policy/policy-executor.ts index 436539a0..f2198c16 100644 --- a/services/src/modules/directives/policy/policy-executor.ts +++ b/services/src/modules/directives/policy/policy-executor.ts @@ -3,7 +3,7 @@ import {RequestContext} from '../../context'; import {Policy, GraphQLArguments, QueryResults} from './types'; import {Policy as PolicyDefinition, PolicyArgsObject, PolicyAttachments, PolicyQuery} from '../../resource-repository'; import {evaluate as evaluateOpa} from './opa'; -import {injectParameters} from '../../paramInjection'; +import {injectParameters, resolveParameters} from '../../paramInjection'; const typeEvaluators = { opa: evaluateOpa, @@ -54,21 +54,42 @@ export class PolicyExecutor { } private preparePolicyArgs(supportedPolicyArgs: PolicyArgsObject, policy: Policy): PolicyArgsObject { - return Object.keys(supportedPolicyArgs).reduce((policyArgs, policyArgName) => { - if (policy?.args?.[policyArgName] === undefined) - throw new Error( - `Missing arg ${policyArgName} for policy ${policy.name} in namespace ${policy.namespace}` - ); - - let policyArgValue = policy.args[policyArgName]; - if (typeof policyArgValue === 'string') { - policyArgValue = injectParameters(policyArgValue, this.parent, this.args, this.context, this.info) - .value; - } - - policyArgs[policyArgName] = policyArgValue; - return policyArgs; - }, {}); + return Object.entries(supportedPolicyArgs).reduce( + (policyArgs, [policyArgName, policyArgType]) => { + if (policy?.args?.[policyArgName] === undefined) + throw new Error( + `Missing arg ${policyArgName} for policy ${policy.name} in namespace ${policy.namespace}` + ); + + let policyArgValue = policy.args[policyArgName]; + if (typeof policyArgValue === 'string') { + if (policyArgType === 'String') { + policyArgValue = injectParameters( + policyArgValue, + this.parent, + this.args, + this.context, + this.info + ).value; + } else { + const resolvedArgValue = resolveParameters( + policyArgValue, + this.parent, + this.args, + this.context, + this.info + ); + if (resolvedArgValue) { + policyArgValue = resolvedArgValue[policyArgValue]; + } + } + } + + policyArgs[policyArgName] = policyArgValue; + return policyArgs; + }, + {} + ); } private getPolicyDefinition(namespace: string, name: string) { @@ -88,13 +109,15 @@ export class PolicyExecutor { query.variables && Object.entries(query.variables).reduce<{[key: string]: any}>((policyArgs, [varName, varValue]) => { if (typeof varValue === 'string') { - varValue = injectParameters(varValue, this.parent, args, this.context, this.info).value; + const resolvedValue = resolveParameters(varValue, this.parent, args, this.context, this.info); + if (resolvedValue) { + varValue = resolvedValue[varValue]; + } } policyArgs[varName] = varValue; return policyArgs; }, {}); - // TODO: Run with admin permissions const context: RequestContext = {...this.context, ignorePolicies: true}; const gqlResult = await graphql(this.info.schema, query.gql, undefined, context, variableValues); return gqlResult.data || undefined;