From db43d8f22516f74ae1869ec1771ea8747c48f3c4 Mon Sep 17 00:00:00 2001 From: Tomer Eskenazi Date: Sun, 28 Jun 2020 18:27:33 +0300 Subject: [PATCH] allow jwt in param injection (policy authorization can use it through args) --- .gitignore | 2 +- docs/authorization_spec.md | 22 +++-- services/package-lock.json | 93 +++++++++++++++++++ services/package.json | 2 + services/src/modules/directives/policy/opa.ts | 4 +- .../directives/policy/policy-executor.ts | 7 +- .../src/modules/directives/policy/types.ts | 5 - services/src/modules/paramInjection.ts | 30 ++++++ services/src/tests/e2e/jest-e2e-runner.js | 1 + .../src/tests/e2e/tests/authorization.spec.ts | 70 +++++++++++++- services/src/tests/helpers/authzSchema.ts | 34 +++++++ 11 files changed, 246 insertions(+), 24 deletions(-) diff --git a/.gitignore b/.gitignore index d4036fd3..697cdfba 100644 --- a/.gitignore +++ b/.gitignore @@ -7,4 +7,4 @@ lerna-debug.log* node_modules/ *.tsbuildinfo -.yarn-integrity \ No newline at end of file +.yarn-integrity diff --git a/docs/authorization_spec.md b/docs/authorization_spec.md index 3c9702de..5d752a44 100644 --- a/docs/authorization_spec.md +++ b/docs/authorization_spec.md @@ -37,10 +37,13 @@ metadata: name: abc-user Spec: type: js-expression - code: { "result": (data.jwt.issuer === "abc.com") ? "allow" : "deny" } + code: { "result": (input.args.issuer === "abc.com") ? "allow" : "deny" } + args: + issuer: String! ``` -`jwt` is available on the data object to validate fields from the web token +The `args` are available to use on the input object. +`jwt` is available for parameter injection in args using `issuer: "{jwt.issuer}"` ### Policy that allows access only if the subject matches the provided userId argument: @@ -51,13 +54,12 @@ metadata: name: my-user Spec: type: js-expression - code: { "result": (input.jwt.sub === input.args.userId) ? "allow" : "deny"} + code: { "result": (input.args.sub === input.args.userId) ? "allow" : "deny"} args: userId: ID! + sub: ID! ``` -The `args` are available to use on the input object - _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: @@ -104,13 +106,14 @@ metadata: Spec: type: opa code: | - query = sprintf(“graphql { user(%s) {family { members { id} } } }”, input.jwt.sub) + query = sprintf(“graphql { user(%s) {family { members { id} } } }”, input.args.sub) allow = false allow = { input.args.userId == input.query.family.members[_].id } args: userId: ID! + sub: ID! ``` This example will always evaluate the graphql query, but generally this approach should be used when conditional side effect evaluation is needed @@ -149,7 +152,7 @@ type User { ID: ID! Picture: String @policy-some-ns-public Friends: [User] @policy-some-ns-abc-user - Email: String @policy-some-ns-my-user(userId: "{source.UserId}") + Email: String @policy-some-ns-my-user(userId: "{source.UserId}", sub: "{jwt.sub}") NickName: @policy-some-ns-my-user-family(userId: "{source.UserId}") } ``` @@ -267,11 +270,12 @@ Spec: code: | allow = false allow { - input.jwt.claims[input.args.claims[i]] == input.args.values[i] + input.args.jwtClaims[input.args.claims[i]] == input.args.values[i] } args: claims: [String] values: [String] + jwtClaims: JSONObject ``` Usage: @@ -279,6 +283,6 @@ Usage: ``` type User { ID: ID! - Picture: String @policy-some-ns-has-claims(claims:["issuer", "sub"], values: ["soluto.com", "{source.UserId}"] + Picture: String @policy-some-ns-has-claims(claims:["issuer", "sub"], values: ["soluto.com", "{source.UserId}"], jwtClaims: "{jwt.claims}") } ``` diff --git a/services/package-lock.json b/services/package-lock.json index 65878ca5..bfe05280 100644 --- a/services/package-lock.json +++ b/services/package-lock.json @@ -1036,6 +1036,15 @@ "pretty-format": "^25.1.0" } }, + "@types/jsonwebtoken": { + "version": "8.5.0", + "resolved": "https://registry.npmjs.org/@types/jsonwebtoken/-/jsonwebtoken-8.5.0.tgz", + "integrity": "sha512-9bVao7LvyorRGZCw0VmH/dr7Og+NdjYSsKAxB43OQoComFbBgsEpoR9JW6+qSq/ogwVBg8GI2MfAlk4SYI4OLg==", + "dev": true, + "requires": { + "@types/node": "*" + } + }, "@types/keygrip": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/@types/keygrip/-/keygrip-1.0.2.tgz", @@ -1987,6 +1996,11 @@ "isarray": "^1.0.0" } }, + "buffer-equal-constant-time": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/buffer-equal-constant-time/-/buffer-equal-constant-time-1.0.1.tgz", + "integrity": "sha1-+OcRMvf/5uAaXJaXpMbz5I1cyBk=" + }, "buffer-from": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/buffer-from/-/buffer-from-1.1.1.tgz", @@ -2513,6 +2527,14 @@ "safer-buffer": "^2.1.0" } }, + "ecdsa-sig-formatter": { + "version": "1.0.11", + "resolved": "https://registry.npmjs.org/ecdsa-sig-formatter/-/ecdsa-sig-formatter-1.0.11.tgz", + "integrity": "sha512-nagl3RYrbNv6kQkeJIpt6NJZy8twLB/2vtz6yN9Z4vRKHN4/QZJIEbqohALSgwKdnksuY3k5Addp5lg8sVoVcQ==", + "requires": { + "safe-buffer": "^5.0.1" + } + }, "emoji-regex": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz", @@ -5146,6 +5168,23 @@ "minimist": "^1.2.0" } }, + "jsonwebtoken": { + "version": "8.5.1", + "resolved": "https://registry.npmjs.org/jsonwebtoken/-/jsonwebtoken-8.5.1.tgz", + "integrity": "sha512-XjwVfRS6jTMsqYs0EsuJ4LGxXV14zQybNd4L2r0UvbVnSF9Af8x7p5MzbJ90Ioz/9TI41/hTCvznF/loiSzn8w==", + "requires": { + "jws": "^3.2.2", + "lodash.includes": "^4.3.0", + "lodash.isboolean": "^3.0.3", + "lodash.isinteger": "^4.0.4", + "lodash.isnumber": "^3.0.3", + "lodash.isplainobject": "^4.0.6", + "lodash.isstring": "^4.0.1", + "lodash.once": "^4.0.0", + "ms": "^2.1.1", + "semver": "^5.6.0" + } + }, "jsprim": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/jsprim/-/jsprim-1.4.1.tgz", @@ -5158,6 +5197,25 @@ "verror": "1.10.0" } }, + "jwa": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/jwa/-/jwa-1.4.1.tgz", + "integrity": "sha512-qiLX/xhEEFKUAJ6FiBMbes3w9ATzyk5W7Hvzpa/SLYdxNtng+gcurvrI7TbACjIXlsJyr05/S1oUhZrc63evQA==", + "requires": { + "buffer-equal-constant-time": "1.0.1", + "ecdsa-sig-formatter": "1.0.11", + "safe-buffer": "^5.0.1" + } + }, + "jws": { + "version": "3.2.2", + "resolved": "https://registry.npmjs.org/jws/-/jws-3.2.2.tgz", + "integrity": "sha512-YHlZCB6lMTllWDtSPHz/ZXTsi8S00usEV6v1tjq8tOUZzw7DpSDWVXjXDre6ed1w/pd495ODpHZYSdkRTsa0HA==", + "requires": { + "jwa": "^1.4.1", + "safe-buffer": "^5.0.1" + } + }, "keyv": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/keyv/-/keyv-3.1.0.tgz", @@ -5219,12 +5277,47 @@ "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.15.tgz", "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==" }, + "lodash.includes": { + "version": "4.3.0", + "resolved": "https://registry.npmjs.org/lodash.includes/-/lodash.includes-4.3.0.tgz", + "integrity": "sha1-YLuYqHy5I8aMoeUTJUgzFISfVT8=" + }, + "lodash.isboolean": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/lodash.isboolean/-/lodash.isboolean-3.0.3.tgz", + "integrity": "sha1-bC4XHbKiV82WgC/UOwGyDV9YcPY=" + }, + "lodash.isinteger": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/lodash.isinteger/-/lodash.isinteger-4.0.4.tgz", + "integrity": "sha1-YZwK89A/iwTDH1iChAt3sRzWg0M=" + }, + "lodash.isnumber": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/lodash.isnumber/-/lodash.isnumber-3.0.3.tgz", + "integrity": "sha1-POdoEMWSjQM1IwGsKHMX8RwLH/w=" + }, + "lodash.isplainobject": { + "version": "4.0.6", + "resolved": "https://registry.npmjs.org/lodash.isplainobject/-/lodash.isplainobject-4.0.6.tgz", + "integrity": "sha1-fFJqUtibRcRcxpC4gWO+BJf1UMs=" + }, + "lodash.isstring": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/lodash.isstring/-/lodash.isstring-4.0.1.tgz", + "integrity": "sha1-1SfftUVuynzJu5XV2ur4i6VKVFE=" + }, "lodash.memoize": { "version": "4.1.2", "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz", "integrity": "sha1-vMbEmkKihA7Zl/Mj6tpezRguC/4=", "dev": true }, + "lodash.once": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/lodash.once/-/lodash.once-4.1.1.tgz", + "integrity": "sha1-DdOXEhPHxW34gJd9UEyI+0cal6w=" + }, "lodash.sortby": { "version": "4.7.0", "resolved": "https://registry.npmjs.org/lodash.sortby/-/lodash.sortby-4.7.0.tgz", diff --git a/services/package.json b/services/package.json index 478d30a1..633a61e6 100644 --- a/services/package.json +++ b/services/package.json @@ -29,6 +29,7 @@ "graphql-iso-date": "^3.6.1", "graphql-tools": "^4.0.7", "graphql-type-json": "^0.3.1", + "jsonwebtoken": "^8.5.1", "node-cache": "^5.1.0", "node-fetch": "^2.6.0", "openid-client": "^3.13.0", @@ -42,6 +43,7 @@ "@types/graphql-iso-date": "^3.3.3", "@types/graphql-type-json": "^0.3.2", "@types/jest": "^25.1.3", + "@types/jsonwebtoken": "^8.5.0", "@types/node": "^13.7.4", "@types/pino": "^5.15.5", "@types/xml2js": "^0.4.5", diff --git a/services/src/modules/directives/policy/opa.ts b/services/src/modules/directives/policy/opa.ts index 14ff7a1d..99fcf13c 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, JwtInput} from './types'; +import {PolicyExecutionContext, PolicyExecutionResult, QueriesResults} from './types'; import {PolicyArgsObject} from '../../resource-repository'; export async function evaluate(ctx: PolicyExecutionContext): Promise { @@ -24,7 +24,6 @@ async function getWasmPolicy(ctx: PolicyExecutionContext): Promise { function getInput(ctx: PolicyExecutionContext): PolicyInput { const input: PolicyInput = {}; - if (ctx.jwt) input.jwt = ctx.jwt; if (ctx.args) input.args = ctx.args; if (ctx.queries) input.queries = ctx.queries; @@ -32,7 +31,6 @@ function getInput(ctx: PolicyExecutionContext): PolicyInput { } type PolicyInput = { - jwt?: JwtInput; args?: PolicyArgsObject; queries?: QueriesResults; }; diff --git a/services/src/modules/directives/policy/policy-executor.ts b/services/src/modules/directives/policy/policy-executor.ts index ed6a7063..2da7ec33 100644 --- a/services/src/modules/directives/policy/policy-executor.ts +++ b/services/src/modules/directives/policy/policy-executor.ts @@ -20,7 +20,6 @@ export class PolicyExecutor { protected context: RequestContext, protected info: GraphQLResolveInfo ) { - // TODO: add jwt data this.policyDefinitions = context.policies; this.policyAttachments = context.policyAttachments; } @@ -38,7 +37,11 @@ export class PolicyExecutor { const evaluate = typeEvaluators[policyDefinition.type]; if (!evaluate) throw new Error(`Unsupported policy type ${policyDefinition.type}`); - const {done, allow} = await evaluate({...policy, args, policyAttachments: this.policyAttachments}); + const {done, allow} = await evaluate({ + ...policy, + args, + policyAttachments: this.policyAttachments, + }); if (!done) throw new Error('in-line query evaluation not yet supported'); if (!allow) throw new Error(`Unauthorized by policy ${policy.name} in namespace ${policy.namespace}`); diff --git a/services/src/modules/directives/policy/types.ts b/services/src/modules/directives/policy/types.ts index 48977786..f6bd51db 100644 --- a/services/src/modules/directives/policy/types.ts +++ b/services/src/modules/directives/policy/types.ts @@ -11,7 +11,6 @@ export type PolicyExecutionContext = { namespace: string; name: string; policyAttachments: PolicyAttachments; - jwt?: JwtInput; args?: PolicyArgsObject; queries?: QueriesResults; }; @@ -20,10 +19,6 @@ export type QueriesResults = { [name: string]: string; }; -export type JwtInput = { - [name: string]: string; -}; - export type PolicyExecutionResult = { done: boolean; allow?: boolean; diff --git a/services/src/modules/paramInjection.ts b/services/src/modules/paramInjection.ts index 942e186d..777a38ab 100644 --- a/services/src/modules/paramInjection.ts +++ b/services/src/modules/paramInjection.ts @@ -1,10 +1,16 @@ import {GraphQLResolveInfo} from 'graphql'; +import {decode as decodeJwt} from 'jsonwebtoken'; import {RequestContext} from './context'; interface GraphQLArguments { [key: string]: any; } +type jwtData = { + [name: string]: any; +}; + const paramRegex = /{(\w+\.\w+)}/g; +const authzHeaderPrefix = 'Bearer '; function resolveTemplate( template: string, @@ -22,6 +28,8 @@ function resolveTemplate( return args && args[propName]; case 'exports': return context.exports.resolve(info.parentType, parent, propName); + case 'jwt': + return getJwt(context)[propName]; default: return null; } @@ -96,3 +104,25 @@ export function deepInjectParameters( function isObject(val: any): val is object { return typeof val === 'object' && val !== null; } + +function getJwt(context: RequestContext): jwtData { + if (context.jwt) return context.jwt; + + const authzHeader = context?.request?.headers?.authorization; + + context.jwt = isAuthzHeaderValid(authzHeader) + ? (decodeJwt(authzHeader.substr(authzHeaderPrefix.length), {json: true}) as jwtData) + : {}; + + return context.jwt; +} + +function isAuthzHeaderValid(authzHeader: any): boolean { + return authzHeader && authzHeader.startsWith(authzHeaderPrefix); +} + +declare module './context' { + interface RequestContext { + jwt?: jwtData; + } +} diff --git a/services/src/tests/e2e/jest-e2e-runner.js b/services/src/tests/e2e/jest-e2e-runner.js index d7ea28ed..d04cc7f0 100644 --- a/services/src/tests/e2e/jest-e2e-runner.js +++ b/services/src/tests/e2e/jest-e2e-runner.js @@ -15,6 +15,7 @@ class SerialJestRunner extends DefaultJestRunner { } async teardown() { + // await dockerCompose.logs(['gateway', 'registry'], {cwd: __dirname, log: true}); await dockerCompose.down({cwd: __dirname, log: true}); await Promise.all([waitFor.gatewayStop(10000), waitFor.registryStop(10000)]); } diff --git a/services/src/tests/e2e/tests/authorization.spec.ts b/services/src/tests/e2e/tests/authorization.spec.ts index c5478b68..4bbb5449 100644 --- a/services/src/tests/e2e/tests/authorization.spec.ts +++ b/services/src/tests/e2e/tests/authorization.spec.ts @@ -6,16 +6,21 @@ import { createSchemaMutation, createPolicyMutation, onlyAdminPolicy, + jwtNamePolicy, + getArbitraryDataQuery, } from '../../helpers/authzSchema'; -const gatewayClient = new GraphQLClient('http://localhost:8080/graphql'); -const registryClient = new GraphQLClient('http://localhost:8090/graphql'); +const gatewayClient = createGatewayClient(); +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 policyResponse = await registryClient.request(createPolicyMutation, {policy: onlyAdminPolicy()}); - expect(policyResponse.updatePolicies.success).toBe(true); + const policy1Response = await registryClient.request(createPolicyMutation, {policy: onlyAdminPolicy()}); + expect(policy1Response.updatePolicies.success).toBe(true); + + const policy2Response = await registryClient.request(createPolicyMutation, {policy: jwtNamePolicy()}); + expect(policy2Response.updatePolicies.success).toBe(true); const schemaResponse = await registryClient.request(createSchemaMutation, {schema: getSchema()}); expect(schemaResponse.updateSchemas.success).toBe(true); @@ -42,4 +47,61 @@ describe('authorization', () => { expect(response.errors[0].path).toEqual(['user', 'lastName']); expect(response.data.user).toEqual({firstName: 'John', lastName: null, role: 'normal'}); }); + + it('rejects access to a field based on JWT info when no JWT is sent', async () => { + let response; + try { + await gatewayClient.request(getArbitraryDataQuery()); + } catch (err) { + response = err.response; + } + + expect(response.errors).toHaveLength(1); + expect(response.errors[0].message).toBe('Unauthorized by policy jwtName in namespace ns'); + expect(response.errors[0].path).toEqual(['arbitraryData', 'arbitraryField']); + expect(response.data.arbitraryData).toEqual({arbitraryField: null}); + }); + + it('rejects access to a field based on JWT info', async () => { + const gatewayClientJwt = createGatewayClient(disallowedJwtOptions()); + let response; + try { + await gatewayClientJwt.request(getArbitraryDataQuery()); + } catch (err) { + response = err.response; + } + + expect(response.errors).toHaveLength(1); + expect(response.errors[0].message).toBe('Unauthorized by policy jwtName in namespace ns'); + expect(response.errors[0].path).toEqual(['arbitraryData', 'arbitraryField']); + expect(response.data.arbitraryData).toEqual({arbitraryField: null}); + }); + + it('allows access to a field based on JWT info', async () => { + const gatewayClientJwt = createGatewayClient(allowedJwtOptions()); + const response = await gatewayClientJwt.request(getArbitraryDataQuery()); + expect(response.arbitraryData).toEqual({arbitraryField: 'arbitraryValue'}); + }); }); + +function allowedJwtOptions() { + // Claims: { "name": "Varg", "sub": "1234567890", "iat": 1516239022 } + const encodedJwt = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IlZhcmciLCJpYXQiOjE1MTYyMzkwMjJ9.MEXFHfBaci44tuGREvCK1vZ-KqYL7L-TYqO7v0jfsYM'; + return {headers: {authorization: `Bearer ${encodedJwt}`}}; +} + +function disallowedJwtOptions() { + // Claims: { "name": "Orm", "sub": "1234567890", "iat": 1516239022 } + const encodedJwt = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6Ik9ybSIsImlhdCI6MTUxNjIzOTAyMn0.ZQ__h2OJR8A7uSKTZ3QC0jP5P3U3a1JuMU9xVnzWTIM'; + return {headers: {authorization: `Bearer ${encodedJwt}`}}; +} + +function createRegistryClient(options = {}) { + return new GraphQLClient('http://localhost:8090/graphql', options); +} + +function createGatewayClient(options = {}) { + return new GraphQLClient('http://localhost:8080/graphql', options); +} diff --git a/services/src/tests/helpers/authzSchema.ts b/services/src/tests/helpers/authzSchema.ts index 1e7afb61..6bf40e9d 100644 --- a/services/src/tests/helpers/authzSchema.ts +++ b/services/src/tests/helpers/authzSchema.ts @@ -12,6 +12,21 @@ export const onlyAdminPolicy = () => ({ }, }); +export const jwtNamePolicy = () => ({ + metadata: {namespace: 'ns', name: 'jwtName'}, + type: 'opa', + code: ` + default allow = false + allow { + input.args.allowedName == input.args.jwtName + } +`, + args: { + allowedName: 'String', + jwtName: 'String', + }, +}); + export const createPolicyMutation = ` mutation CreatePolicy($policy: PolicyInput!) { updatePolicies(input: [$policy]) { @@ -29,9 +44,20 @@ export const getSchema = () => ({ ]) role: String } + + type ArbitraryData { + arbitraryField: String @policy(policies: [ + { namespace: "ns", name: "jwtName", args: { + jwtName: "{jwt.name}", + allowedName: "Varg" + }} + ]) + } + type Query { user: User! @stub(value: ${userQueryStub('normal')}) userAdmin: User! @stub(value: ${userQueryStub('admin')}) + arbitraryData: ArbitraryData! @stub(value: { arbitraryField: "arbitraryValue" }) } `, }); @@ -58,3 +84,11 @@ export const getUserQuery = (queryType: string) => ` } } `; + +export const getArbitraryDataQuery = () => ` + query { + arbitraryData { + arbitraryField + } + } +`;