From ff4e70d909ff9a6616d80cb17f78a17fc0617a2d Mon Sep 17 00:00:00 2001 From: AleF83 Date: Sun, 28 Jun 2020 16:10:41 +0300 Subject: [PATCH] Second iteration --- services/src/modules/baseSchema.ts | 11 +++- services/src/modules/directives/policy/opa.ts | 6 +-- .../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 | 6 +-- .../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, 157 insertions(+), 49 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 5cf8d56a..2288933d 100644 --- a/services/src/modules/directives/policy/opa.ts +++ b/services/src/modules/directives/policy/opa.ts @@ -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.jwt) input.jwt = ctx.jwt; if (ctx.args) input.args = ctx.args; @@ -31,7 +31,7 @@ function getInput(ctx: PolicyExecutionContext): PolicyInput { return input; } -type PolicyInput = { +type PolicyOpaInput = { jwt?: JwtInput; 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 6803b725..e17442e2 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'; @@ -29,7 +29,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); @@ -41,7 +41,11 @@ 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'); + 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}`); } @@ -72,7 +76,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]) => { @@ -84,7 +91,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 6f834486..9696be7c 100644 --- a/services/src/modules/paramInjection.ts +++ b/services/src/modules/paramInjection.ts @@ -10,20 +10,20 @@ const paramRegex = /{(source|args|exports)\.(\w+(\.\w+)*)}/; 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); 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', },