Skip to content
Closed
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@
"path-to-regexp": "^6.2.1",
"pino": "8.11.0",
"pino-pretty": "10.2.0",
"require-in-the-middle": "^5.1.0",
"url-parse": "^1.5.10"
"require-in-the-middle": "^5.1.0"
},
"devDependencies": {
"@ava/typescript": "^1.1.1",
Expand All @@ -76,7 +75,6 @@
"@types/express": "^4.17.9",
"@types/lodash": "^4.14.166",
"@types/node": "^14.14.14",
"@types/url-parse": "^1.4.11",
"@typescript-eslint/eslint-plugin": "^4.0.1",
"@typescript-eslint/parser": "^4.0.1",
"ava": "^3.12.1",
Expand Down
37 changes: 31 additions & 6 deletions src/enforcement/enforcer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import axios, { AxiosInstance } from 'axios';
import { Logger } from 'pino';
import URL from 'url-parse';

import { IPermitConfig } from '../config';
import { CheckConfig, Context, ContextStore } from '../utils/context';
Expand Down Expand Up @@ -120,6 +119,34 @@ export interface IEnforcer {
): Promise<TenantDetails[]>;
}

/**
* Builds the OPA client base URL from the configured PDP URL by forcing the OPA
* port (8181) and appending the OPA data path, using the native WHATWG `URL`
* (Node >= 10) in place of the previous `url-parse` dependency (#106).
*
* @param pdp - The configured PDP base URL (e.g. `http://localhost:7766`).
* @returns The OPA base URL (e.g. `http://localhost:8181/v1/data/permit/`). A PDP
* path with no trailing slash is glued to the data path (`/prefix` ->
* `/prefixv1/data/permit/`), preserved from `url-parse` and locked by a test.
* @throws {PermitError} on input without a scheme (e.g. `localhost`); a `host:port`
* value like `localhost:7766` is misparsed, not rejected — pass a full URL. The
* error omits the configured value because it may carry credentials.
*/
export function buildOpaBaseUrl(pdp: string): string {
let opaBaseUrl: URL;
try {
opaBaseUrl = new URL(pdp);
} catch {
throw new PermitError(
'Invalid PDP URL in the "pdp" option: expected an absolute http(s) URL, ' +
'e.g. "http://localhost:7766".',
);
}
opaBaseUrl.port = '8181';
opaBaseUrl.pathname = `${opaBaseUrl.pathname}v1/data/permit/`;
return opaBaseUrl.toString();
}

/**
* The {@link Enforcer} class is responsible for performing permission checks against the PDP.
* It implements the {@link IEnforcer} interface.
Expand All @@ -135,9 +162,7 @@ export class Enforcer implements IEnforcer {
* @param logger - The logger instance for logging.
*/
constructor(private config: IPermitConfig, private logger: Logger) {
const opaBaseUrl = new URL(this.config.pdp);
opaBaseUrl.set('port', '8181');
opaBaseUrl.set('pathname', `${opaBaseUrl.pathname}v1/data/permit/`);
const opaBaseUrl = buildOpaBaseUrl(this.config.pdp);
const version = process.env.npm_package_version ?? 'unknown';
// PDP gets its own dedicated axios instance so PDP-only POST retries never
// apply to the shared REST API client (config.axiosInstance) — REST writes
Expand All @@ -148,11 +173,11 @@ export class Enforcer implements IEnforcer {
});
if (config.opaAxiosInstance) {
this.opaClient = config.opaAxiosInstance;
this.opaClient.defaults.baseURL = opaBaseUrl.toString();
this.opaClient.defaults.baseURL = opaBaseUrl;
this.opaClient.defaults.headers.common['X-Permit-SDK-Version'] = `node:${version}`;
} else {
this.opaClient = axios.create({
baseURL: opaBaseUrl.toString(),
baseURL: opaBaseUrl,
headers: {
'X-Permit-SDK-Version': `node:${version}`,
},
Expand Down
111 changes: 111 additions & 0 deletions src/tests/unit/enforcer.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import test from 'ava';
import axios, { AxiosInstance, InternalAxiosRequestConfig } from 'axios';
import pino from 'pino';

import { buildOpaBaseUrl } from '../../enforcement/enforcer';
import { Permit, PermitError } from '../../index';

// The OPA client base URL is derived from the configured PDP URL by forcing the
// OPA port (8181) and appending the OPA data path. This was previously built
// with the `url-parse` package and is now built with the native WHATWG `URL`
// (#106). For absolute http(s) PDP URLs in canonical form the result matches what
// url-parse produced, and these assertions lock that string. WHATWG parsing also
// normalizes input that url-parse kept verbatim (dot segments, percent-encoding,
// IDN hosts); the dot-segment test below pins that intended difference.

test('buildOpaBaseUrl: default PDP', (t) => {
t.is(buildOpaBaseUrl('http://localhost:7766'), 'http://localhost:8181/v1/data/permit/');
});

test('buildOpaBaseUrl: trailing slash yields the same URL as no trailing slash', (t) => {
t.is(buildOpaBaseUrl('http://localhost:7766/'), 'http://localhost:8181/v1/data/permit/');
});

test('buildOpaBaseUrl: https host without an explicit port', (t) => {
t.is(buildOpaBaseUrl('https://pdp.example.com'), 'https://pdp.example.com:8181/v1/data/permit/');
});

test('buildOpaBaseUrl: an existing port is overridden with 8181', (t) => {
t.is(
buildOpaBaseUrl('https://pdp.example.com:1234'),
'https://pdp.example.com:8181/v1/data/permit/',
);
});

test('buildOpaBaseUrl: a path prefix preserves the existing concatenation behaviour', (t) => {
// Pre-existing url-parse quirk: a path with no trailing slash glues onto the data path.
t.is(
buildOpaBaseUrl('http://localhost:7766/prefix'),
'http://localhost:8181/prefixv1/data/permit/',
);
t.is(
buildOpaBaseUrl('http://localhost:7766/prefix/'),
'http://localhost:8181/prefix/v1/data/permit/',
);
});

test('buildOpaBaseUrl: dot segments in the PDP path are resolved before the data path', (t) => {
// url-parse kept them verbatim and produced `/a/..v1/data/permit/`.
t.is(
buildOpaBaseUrl('https://pdp.example.com/a/..'),
'https://pdp.example.com:8181/v1/data/permit/',
);
});

test('buildOpaBaseUrl: a PDP without a scheme throws a PermitError naming the pdp option', (t) => {
// Bare hosts and `//host:port` throw at construction; `localhost:7766` is misparsed,
// not rejected.
for (const pdp of ['localhost', '//localhost:7766']) {
t.throws(() => buildOpaBaseUrl(pdp), {
instanceOf: PermitError,
message: /"pdp" option.*absolute http\(s\) URL/,
});
}
});

test('new Permit does not expose credentials from an invalid PDP URL', (t) => {
const error = t.throws(
() => new Permit({ token: 'test-token', pdp: 'http://pdp-user:pdp-secret@localhost:bad' }),
{ instanceOf: PermitError },
);
// Serialize the way the SDK's pino logger would, so enumerable error fields are covered too.
t.false(JSON.stringify(pino.stdSerializers.err(error)).includes('pdp-secret'));
});

// A non-default scheme, port and path, so a constructor that ignores the configured
// PDP (or an OPA client that ignores the derived base URL) fails the tests below.
const CONFIGURED_PDP = 'https://pdp.example.com:1234/prefix/';
const EXPECTED_OPA_CHECK_URL = 'https://pdp.example.com:8181/prefix/v1/data/permit/root';

// Reaches the SDK-created OPA client, as retry-interceptor.spec.ts does for enforcer.client.
interface PermitInternals {
enforcer: { opaClient: AxiosInstance };
}

// Records the URL axios would request and answers with an OPA allow decision, so the
// check runs end to end without a network call.
function captureRequestUrls(instance: AxiosInstance): string[] {
const urls: string[] = [];
instance.defaults.adapter = async (config: InternalAxiosRequestConfig) => {
urls.push(axios.getUri(config));
return { status: 200, statusText: 'OK', headers: {}, config, data: { allow: true } };
};
return urls;
}

test('a useOpa check posts to the OPA root derived from the configured PDP', async (t) => {
const permit = new Permit({ token: 'test-token', pdp: CONFIGURED_PDP });
const urls = captureRequestUrls((permit as unknown as PermitInternals).enforcer.opaClient);

t.true(await permit.check('user', 'read', 'document', {}, { useOpa: true }));
t.deepEqual(urls, [EXPECTED_OPA_CHECK_URL]);
});

test('a useOpa check through an injected opaAxiosInstance posts to the OPA root', async (t) => {
const opaAxiosInstance = axios.create();
const urls = captureRequestUrls(opaAxiosInstance);
const permit = new Permit({ token: 'test-token', pdp: CONFIGURED_PDP, opaAxiosInstance });

t.true(await permit.check('user', 'read', 'document', {}, { useOpa: true }));
t.deepEqual(urls, [EXPECTED_OPA_CHECK_URL]);
});
23 changes: 0 additions & 23 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -799,11 +799,6 @@
"@types/mime" "*"
"@types/node" "*"

"@types/url-parse@^1.4.11":
version "1.4.11"
resolved "https://registry.npmjs.org/@types/url-parse/-/url-parse-1.4.11.tgz"
integrity sha512-FKvKIqRaykZtd4n47LbK/W/5fhQQ1X7cxxzG9A48h0BGN+S04NH7ervcCjM8tyR0lyGru83FAHSmw2ObgKoESg==

"@typescript-eslint/eslint-plugin@^4.0.1":
version "4.33.0"
resolved "https://registry.npmjs.org/@typescript-eslint/eslint-plugin/-/eslint-plugin-4.33.0.tgz"
Expand Down Expand Up @@ -5317,11 +5312,6 @@ q@^1.5.1:
resolved "https://registry.npmjs.org/q/-/q-1.5.1.tgz"
integrity sha512-kV/CThkXo6xyFEZUugw/+pIOywXcDbFYgSct5cT3gqlbkBE1SJdwy6UQoZvodiWF/ckQLZyDE/Bu1M6gVu5lVw==

querystringify@^2.1.1:
version "2.2.0"
resolved "https://registry.npmjs.org/querystringify/-/querystringify-2.2.0.tgz"
integrity sha512-FIqgj2EUvTa7R50u0rGsyTftzjYmv/a3hO345bZNrqabNqjtgiDMgmo4mkUjd+nzU5oF3dClKqFIPUKybUyqoQ==

queue-microtask@^1.2.2:
version "1.2.3"
resolved "https://registry.npmjs.org/queue-microtask/-/queue-microtask-1.2.3.tgz"
Expand Down Expand Up @@ -5533,11 +5523,6 @@ require-main-filename@^2.0.0:
resolved "https://registry.npmjs.org/require-main-filename/-/require-main-filename-2.0.0.tgz"
integrity sha512-NKN5kMDylKuldxYLSUfrbo5Tuzh4hd+2E8NPPX02mZtn1VuREQToYe/ZdlJy+J3uCpfaiGF05e7B8W0iXbQHmg==

requires-port@^1.0.0:
version "1.0.0"
resolved "https://registry.npmjs.org/requires-port/-/requires-port-1.0.0.tgz"
integrity sha512-KigOCHcocU3XODJxsu8i/j8T9tzT4adHiecwORRQ0ZZFcp7ahwXuRU1m+yuO90C5ZUyGeGfocHDI14M3L3yDAQ==

resolve-cwd@^3.0.0:
version "3.0.0"
resolved "https://registry.npmjs.org/resolve-cwd/-/resolve-cwd-3.0.0.tgz"
Expand Down Expand Up @@ -6705,14 +6690,6 @@ url-parse-lax@^3.0.0:
dependencies:
prepend-http "^2.0.0"

url-parse@^1.5.10:
version "1.5.10"
resolved "https://registry.npmjs.org/url-parse/-/url-parse-1.5.10.tgz"
integrity sha512-WypcfiRhfeUP9vvF0j6rw0J3hrWrw6iZv3+22h6iRMJ/8z1Tj6XfLP4DsUix5MhMPnXpiHDoKyoZ/bdCkwBCiQ==
dependencies:
querystringify "^2.1.1"
requires-port "^1.0.0"

urlgrey@1.0.0:
version "1.0.0"
resolved "https://registry.npmjs.org/urlgrey/-/urlgrey-1.0.0.tgz"
Expand Down
Loading