From 3b9e725750011fbd6e13398ff9a2f2a005cec57a Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Fri, 16 Aug 2024 11:58:13 -0400 Subject: [PATCH 1/7] Allow callers to specify the 'protect' flag. --- README.md | 1 + lib/kerberos.js | 1 + src/kerberos.cc | 7 +++++++ src/kerberos.h | 2 ++ src/unix/kerberos_unix.cc | 3 +-- src/win32/kerberos_win32.cc | 2 +- 6 files changed, 13 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b6623648..53578547 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,7 @@ Processes a single kerberos client-side step using the supplied server challenge | challenge | string | The response returned after calling `unwrap` | | [options] | object | Optional settings | | [options.user] | string | The user to authorize | +| [options.protect] | boolean | Indicates if the wrap should request message confidentiality | | [callback] | function | | Perform the client side kerberos wrap step. diff --git a/lib/kerberos.js b/lib/kerberos.js index 09f8425f..2768e305 100644 --- a/lib/kerberos.js +++ b/lib/kerberos.js @@ -52,6 +52,7 @@ KerberosClient.prototype.step = defineOperation(KerberosClient.prototype.step, [ * @param {string} challenge The response returned after calling `unwrap` * @param {object} [options] Optional settings * @param {string} [options.user] The user to authorize + * @param {string} [options.protect] Indicates if the wrap should request message confidentiality * @param {function} [callback] * @return {Promise} returns Promise if no callback passed */ diff --git a/src/kerberos.cc b/src/kerberos.cc index 7a760423..a1abc591 100644 --- a/src/kerberos.cc +++ b/src/kerberos.cc @@ -37,6 +37,13 @@ std::string ToStringWithNonStringAsEmpty(Napi::Value value) { return value.As(); } +int BooleanToIntWithNonIntAsFalse(Napi::Value value) { + if (!value.IsBoolean()) { + return 0; + } + return value.As().Value() ? 1 : 0; +} + Function KerberosClient::Init(Napi::Env env) { return DefineClass(env, diff --git a/src/kerberos.h b/src/kerberos.h index 7f1c2096..0ca7e98f 100644 --- a/src/kerberos.h +++ b/src/kerberos.h @@ -72,6 +72,8 @@ void TestMethod(const Napi::CallbackInfo& info); std::string ToStringWithNonStringAsEmpty(Napi::Value value); +int BooleanToIntWithNonIntAsFalse(Napi::Value value); + } #endif // KERBEROS_NATIVE_EXTENSION_H diff --git a/src/unix/kerberos_unix.cc b/src/unix/kerberos_unix.cc index 57a67ae8..78659452 100644 --- a/src/unix/kerberos_unix.cc +++ b/src/unix/kerberos_unix.cc @@ -74,8 +74,7 @@ void KerberosClient::WrapData(const CallbackInfo& info) { Object options = info[1].ToObject(); Function callback = info[2].As(); std::string user = ToStringWithNonStringAsEmpty(options["user"]); - - int protect = 0; // NOTE: this should be an option + int protect = BooleanToIntWithNonIntAsFalse(options["protect"]); KerberosWorker::Run(callback, "kerberos:ClientWrap", [=](KerberosWorker::SetOnFinishedHandler onFinished) { gss_result result = authenticate_gss_client_wrap( diff --git a/src/win32/kerberos_win32.cc b/src/win32/kerberos_win32.cc index aa653d43..1ec698fe 100644 --- a/src/win32/kerberos_win32.cc +++ b/src/win32/kerberos_win32.cc @@ -92,7 +92,7 @@ void KerberosClient::WrapData(const CallbackInfo& info) { Object options = info[1].ToObject(); Function callback = info[2].As(); std::string user = ToStringWithNonStringAsEmpty(options["user"]); - int protect = 0; // NOTE: this should be an option + int protect = BooleanToIntWithNonIntAsFalse(options["protect"]); if (isStringTooLong(user)) { throw Error::New(info.Env(), "User name is too long"); From 24893d91fd28da33b610545878e0dfae108cee18 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Tue, 27 Aug 2024 11:25:43 -0400 Subject: [PATCH 2/7] feat(NODE-6333): Throw an error if parameter is not a boolean. --- src/kerberos.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kerberos.cc b/src/kerberos.cc index a1abc591..9e1e415e 100644 --- a/src/kerberos.cc +++ b/src/kerberos.cc @@ -39,7 +39,7 @@ std::string ToStringWithNonStringAsEmpty(Napi::Value value) { int BooleanToIntWithNonIntAsFalse(Napi::Value value) { if (!value.IsBoolean()) { - return 0; + throw Error::New(value.Env(), "Expected a boolean value"); } return value.As().Value() ? 1 : 0; } From c6d6917b413871c38786536a8ae654f9393078ac Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Thu, 29 Aug 2024 13:20:07 -0400 Subject: [PATCH 3/7] feat(NODE-6333): Fixing function name to reflect new behavior. --- src/kerberos.cc | 4 ++-- src/kerberos.h | 2 +- src/unix/kerberos_unix.cc | 2 +- src/win32/kerberos_win32.cc | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/kerberos.cc b/src/kerberos.cc index 9e1e415e..28174909 100644 --- a/src/kerberos.cc +++ b/src/kerberos.cc @@ -37,9 +37,9 @@ std::string ToStringWithNonStringAsEmpty(Napi::Value value) { return value.As(); } -int BooleanToIntWithNonIntAsFalse(Napi::Value value) { +int BooleanToIntWithNonIntAsError(Napi::Value value) { if (!value.IsBoolean()) { - throw Error::New(value.Env(), "Expected a boolean value"); + throw TypeError::New(value.Env(), "Expected a boolean value"); } return value.As().Value() ? 1 : 0; } diff --git a/src/kerberos.h b/src/kerberos.h index 0ca7e98f..a6f29065 100644 --- a/src/kerberos.h +++ b/src/kerberos.h @@ -72,7 +72,7 @@ void TestMethod(const Napi::CallbackInfo& info); std::string ToStringWithNonStringAsEmpty(Napi::Value value); -int BooleanToIntWithNonIntAsFalse(Napi::Value value); +int BooleanToIntWithNonIntAsError(Napi::Value value); } diff --git a/src/unix/kerberos_unix.cc b/src/unix/kerberos_unix.cc index 78659452..1d893c02 100644 --- a/src/unix/kerberos_unix.cc +++ b/src/unix/kerberos_unix.cc @@ -74,7 +74,7 @@ void KerberosClient::WrapData(const CallbackInfo& info) { Object options = info[1].ToObject(); Function callback = info[2].As(); std::string user = ToStringWithNonStringAsEmpty(options["user"]); - int protect = BooleanToIntWithNonIntAsFalse(options["protect"]); + int protect = BooleanToIntWithNonIntAsError(options["protect"]); KerberosWorker::Run(callback, "kerberos:ClientWrap", [=](KerberosWorker::SetOnFinishedHandler onFinished) { gss_result result = authenticate_gss_client_wrap( diff --git a/src/win32/kerberos_win32.cc b/src/win32/kerberos_win32.cc index 1ec698fe..d6c6e29d 100644 --- a/src/win32/kerberos_win32.cc +++ b/src/win32/kerberos_win32.cc @@ -92,7 +92,7 @@ void KerberosClient::WrapData(const CallbackInfo& info) { Object options = info[1].ToObject(); Function callback = info[2].As(); std::string user = ToStringWithNonStringAsEmpty(options["user"]); - int protect = BooleanToIntWithNonIntAsFalse(options["protect"]); + int protect = BooleanToIntWithNonIntAsError(options["protect"]); if (isStringTooLong(user)) { throw Error::New(info.Env(), "User name is too long"); From fc2c81a31fb4b6cf99af15150e412c684dab6f26 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 3 Sep 2024 15:20:24 -0600 Subject: [PATCH 4/7] comments --- src/kerberos.cc | 12 ++++++---- src/kerberos.h | 4 ++-- src/unix/kerberos_unix.cc | 2 +- src/win32/kerberos_win32.cc | 2 +- test/kerberos_tests.js | 46 +++++++++++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/kerberos.cc b/src/kerberos.cc index 28174909..a79dfdd4 100644 --- a/src/kerberos.cc +++ b/src/kerberos.cc @@ -37,11 +37,15 @@ std::string ToStringWithNonStringAsEmpty(Napi::Value value) { return value.As(); } -int BooleanToIntWithNonIntAsError(Napi::Value value) { - if (!value.IsBoolean()) { - throw TypeError::New(value.Env(), "Expected a boolean value"); +int KerberosClient::ParseWrapOptionsProtect(const Napi::Object& options) { + if (!options.Has("protect")) return 0; + + if (!options.Get("protect").IsBoolean()) { + throw TypeError::New(options.Env(), "options.protect must be a boolean."); } - return value.As().Value() ? 1 : 0; + + bool protect = options.Get("protect").As(); + return protect ? 1 : 0; } Function KerberosClient::Init(Napi::Env env) { diff --git a/src/kerberos.h b/src/kerberos.h index a6f29065..deec6e33 100644 --- a/src/kerberos.h +++ b/src/kerberos.h @@ -55,6 +55,8 @@ class KerberosClient : public Napi::ObjectWrap { void UnwrapData(const Napi::CallbackInfo& info); void WrapData(const Napi::CallbackInfo& info); + int ParseWrapOptionsProtect(const Napi::Object& options); + private: friend class Napi::ObjectWrap; explicit KerberosClient(const Napi::CallbackInfo& info); @@ -72,8 +74,6 @@ void TestMethod(const Napi::CallbackInfo& info); std::string ToStringWithNonStringAsEmpty(Napi::Value value); -int BooleanToIntWithNonIntAsError(Napi::Value value); - } #endif // KERBEROS_NATIVE_EXTENSION_H diff --git a/src/unix/kerberos_unix.cc b/src/unix/kerberos_unix.cc index 1d893c02..fafa937b 100644 --- a/src/unix/kerberos_unix.cc +++ b/src/unix/kerberos_unix.cc @@ -74,7 +74,7 @@ void KerberosClient::WrapData(const CallbackInfo& info) { Object options = info[1].ToObject(); Function callback = info[2].As(); std::string user = ToStringWithNonStringAsEmpty(options["user"]); - int protect = BooleanToIntWithNonIntAsError(options["protect"]); + int protect = ParseWrapOptionsProtect(options); KerberosWorker::Run(callback, "kerberos:ClientWrap", [=](KerberosWorker::SetOnFinishedHandler onFinished) { gss_result result = authenticate_gss_client_wrap( diff --git a/src/win32/kerberos_win32.cc b/src/win32/kerberos_win32.cc index d6c6e29d..21977799 100644 --- a/src/win32/kerberos_win32.cc +++ b/src/win32/kerberos_win32.cc @@ -92,7 +92,7 @@ void KerberosClient::WrapData(const CallbackInfo& info) { Object options = info[1].ToObject(); Function callback = info[2].As(); std::string user = ToStringWithNonStringAsEmpty(options["user"]); - int protect = BooleanToIntWithNonIntAsError(options["protect"]); + int protect = ParseWrapOptionsProtect(options); if (isStringTooLong(user)) { throw Error::New(info.Env(), "User name is too long"); diff --git a/test/kerberos_tests.js b/test/kerberos_tests.js index 18263bff..1827a00f 100644 --- a/test/kerberos_tests.js +++ b/test/kerberos_tests.js @@ -122,4 +122,50 @@ describe('Kerberos', function () { }); }); }); + + describe('Client.wrap()', function () { + async function establishConext() { + const service = `HTTP@${hostname}`; + client = await kerberos.initializeClient(service, {}); + server = await kerberos.initializeServer(service); + const clientResponse = await client.step(''); + const serverResponse = await server.step(clientResponse); + await client.step(serverResponse); + expect(client.contextComplete).to.be.true; + return { client, server }; + } + + let client; + let server; + + before(establishConext); + describe('options.protect', function () { + context('valid values for `protect`', function () { + test('no options provided', async function () { + await client.wrap('challenge'); + }); + + test('options provided (protect omitted)', async function () { + await client.wrap('challenge', {}); + }); + + test('protect = false', async function () { + await client.wrap('challenge', { protect: false }); + }); + + test('protect = true', async function () { + await client.wrap('challenge', { protect: true }); + }); + }); + + context('when set to an invalid value', function () { + it('successfully wraps a payload', async function () { + const error = await client.wrap('challenge', { protect: 'non-boolean' }).catch(e => e); + expect(error) + .to.be.instanceOf(TypeError) + .to.match(/options.protect must be a boolean/); + }); + }); + }); + }); }); From e43e09f66478231398c5f1ebbc280234926f78f4 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Tue, 3 Sep 2024 15:33:36 -0600 Subject: [PATCH 5/7] fix tests --- test/kerberos_tests.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/kerberos_tests.js b/test/kerberos_tests.js index 1827a00f..4be9de67 100644 --- a/test/kerberos_tests.js +++ b/test/kerberos_tests.js @@ -4,6 +4,7 @@ const request = require('request'); const chai = require('chai'); const expect = chai.expect; const os = require('os'); +const { test } = require('mocha'); chai.use(require('chai-string')); // environment variables @@ -159,7 +160,7 @@ describe('Kerberos', function () { }); context('when set to an invalid value', function () { - it('successfully wraps a payload', async function () { + it('throws a TypeError', async function () { const error = await client.wrap('challenge', { protect: 'non-boolean' }).catch(e => e); expect(error) .to.be.instanceOf(TypeError) From b7e97b26521786261fef1ac12a6c0102a64e3421 Mon Sep 17 00:00:00 2001 From: arabull <22479725+arabull@users.noreply.github.com> Date: Thu, 5 Sep 2024 11:48:59 -0400 Subject: [PATCH 6/7] Update lib/kerberos.js Co-authored-by: Durran Jordan --- lib/kerberos.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kerberos.js b/lib/kerberos.js index 2768e305..f203d891 100644 --- a/lib/kerberos.js +++ b/lib/kerberos.js @@ -52,7 +52,7 @@ KerberosClient.prototype.step = defineOperation(KerberosClient.prototype.step, [ * @param {string} challenge The response returned after calling `unwrap` * @param {object} [options] Optional settings * @param {string} [options.user] The user to authorize - * @param {string} [options.protect] Indicates if the wrap should request message confidentiality + * @param {boolean} [options.protect] Indicates if the wrap should request message confidentiality * @param {function} [callback] * @return {Promise} returns Promise if no callback passed */ From da50dca3327943334e98120ea332091a52ea3364 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 5 Sep 2024 08:51:16 -0600 Subject: [PATCH 7/7] ci(NODE-6359): specify build-from-source once (#203) --- .evergreen/config.yml | 4 ---- .evergreen/install-dependencies.sh | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/.evergreen/config.yml b/.evergreen/config.yml index 5b3271b0..4b4ca848 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -68,7 +68,6 @@ functions: PROJECT_DIRECTORY: ${PROJECT_DIRECTORY} PROJECT: ${project} GYP_DEFINES: ${GYP_DEFINES|} - NPM_OPTIONS: ${NPM_OPTIONS|} args: - run - '--interactive' @@ -80,8 +79,6 @@ functions: - PROJECT_DIRECTORY=/app - '--env' - GYP_DEFINES - - '--env' - - NPM_OPTIONS - 'ubuntu:22.04' - /bin/bash - /app/.evergreen/run-tests-ubuntu.sh @@ -125,7 +122,6 @@ tasks: - func: run tests ubuntu vars: GYP_DEFINES: kerberos_use_rtld=false - NPM_OPTIONS: --build-from-source - name: run-prebuild commands: - func: install dependencies diff --git a/.evergreen/install-dependencies.sh b/.evergreen/install-dependencies.sh index ac6673ec..0b5fc2b6 100644 --- a/.evergreen/install-dependencies.sh +++ b/.evergreen/install-dependencies.sh @@ -104,5 +104,5 @@ fi echo "npm location: $(which npm)" echo "npm version: $(npm -v)" -npm install "${NPM_OPTIONS}" --build-from-source +npm install --build-from-source ldd build/*/kerberos.node || true